Skip to content

feat: Add ability to customize notification handler #2117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

feat: Add ability to customize notification handler #2117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1e8f057
feat: Add ability to customize notification handler
ghostbuster91 Apr 9, 2023
c5c09eb
Remove nvim-notify auto detection
ghostbuster91 Apr 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions doc/nvim-tree-lua.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,7 @@ applying configuration.
},
notify = {
threshold = vim.log.levels.INFO,
handler = "default",
},
ui = {
confirm = {
Expand Down Expand Up @@ -1224,6 +1225,19 @@ Configuration for notification.
`INFO:` information only e.g. file copy path confirmation.
`DEBUG:` not used.

*nvim-tree.notify.handler*
Change the default notification handler, can be a string `"default"` or a function.
The default handler will use |vim.notify()|
Type: `string` | `function`, Default: `"default"`

Parameters: ~
• {msg} (string) See |vim.notify()|
• {level} (number) See |vim.notify()|
• {opts} (table) Optional parameters

Options: ~
• {title} (string) Usually `"NvimTree"`

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about:

    *nvim-tree.notify.handler*
    Change the default notification handler, can be a string `"default"` or a function.
    The default handler will check if the nvim-notify plugin is present on the
    runtime path and use it, otherwise it will fallback to use |vim.notify()|
      Type: `string` | `function`, Default: `"default"`

    Parameters: ~
      • {msg}   (string) See |vim.notify()|
      • {level} (number) See |vim.notify()|
      • {opts}  (table)  Optional parameters

    Options: ~
      • {title} (string) Usually `"NvimTree"`

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we checking for nvim-notify at all? Wasn't idea behind vim.notify such that it handles notifications send to it per users' handler, if it happens to be nvim-notify so be it.

Not near my computer so can't check much easily.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see! We set handler directly to vim.notify.

Much cleaner. Just have to ensure that the arguments line up.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we checking for nvim-notify at all? Wasn't idea behind vim.notify such that it handles notifications send to it per users' handler, if it happens to be nvim-notify so be it.

Yes, that's correct. I did it like that only to preserve the previous behavior, but I would happy to simplify it. Is there any issue/way of notifying users of such change?

Much cleaner. Just have to ensure that the arguments line up.

They do :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working:
handler = require("notify").notify,
and
handler = vim.notify,

the latter case does not prepend the title, however that's expected.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's correct. I did it like that only to preserve the previous behavior, but I would happy to simplify it. Is there any issue/way of notifying users of such change?

If we were building this today, the user would need to specify handler = require("notify").notify however the existing auto-detection behaviour exists and I don't like to break behaviour.

How about:

  • notify.handler is nil or "default" AND
  • require("notify") successful

"nvim-notify plugin detected: please set notify.handler = require("notify").notify. See :help nvim-tree.notify"

Options:

  1. Preserve existing behaviour, defaulting to nvim-notify
  2. Remove nvim-notify fallback, requiring the user to explicitly set notify.handler

Your call @gegoune :)

*nvim-tree.ui*
General UI configuration.

Expand Down
2 changes: 2 additions & 0 deletions lua/nvim-tree.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,7 @@ local DEFAULT_OPTS = { -- BEGIN_DEFAULT_OPTS
},
notify = {
threshold = vim.log.levels.INFO,
handler = "default",
},
ui = {
confirm = {
Expand Down Expand Up @@ -674,6 +675,7 @@ local FIELD_OVERRIDE_TYPECHECK = {
sort_by = { ["function"] = true, string = true },
root_folder_label = { ["function"] = true, string = true },
picker = { ["function"] = true, string = true },
handler = { ["function"] = true, string = true },
}

local function validate_options(conf)
Expand Down
16 changes: 9 additions & 7 deletions lua/nvim-tree/notify.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
local M = {}

local default_handler = function(msg, level, opts)
vim.notify(string.format("[%s] %s", opts.title, vim.inspect(msg)), level)
end

local config = {
threshold = vim.log.levels.INFO,
handler = default_handler,
}

local modes = {
Expand All @@ -13,19 +18,13 @@ local modes = {
}

do
local has_notify, notify_plugin = pcall(require, "notify")

local dispatch = function(level, msg)
if level < config.threshold then
return
end

vim.schedule(function()
if has_notify and notify_plugin then
notify_plugin(msg, level, { title = "NvimTree" })
else
vim.notify(string.format("[NvimTree] %s", vim.inspect(msg)), level)
end
config.handler(msg, level, { title = "NvimTree" })
end)
end

Expand All @@ -39,6 +38,9 @@ end
function M.setup(opts)
opts = opts or {}
config.threshold = opts.notify.threshold or vim.log.levels.INFO
if type(opts.notify.handler) == "function" then
config.handler = opts.notify.handler
end
end

return M