Skip to content
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.

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

feat request: possibility to update notification window #43

Closed
hassec opened this issue Dec 17, 2021 · 12 comments
Closed

feat request: possibility to update notification window #43

hassec opened this issue Dec 17, 2021 · 12 comments

Comments

@hassec
Copy link
Contributor

hassec commented Dec 17, 2021

A possible use case I was thinking about is using a notification popup to show the status of the LSP startup progress.
E.g at first "LSP Started" and then it would update to show the progress of the background indexing.

I'm not sure if something like that would already be possible?

I could think about two possible interfaces:

  1. Add a "key" in opts and only allow these keys to be unique, thus a 2nd call to notify with the same key will update the old notification and reset the timeout
  2. have notify() return a function that can be called to update the notification and reset timeout

WDYT, @rcarriga, does any of the above sound reasonable to you?

@rcarriga
Copy link
Owner

Hmm I do like this idea. I definitely prefer the 2nd option you've suggested (the first would require some awkward state handling). As you suggested it could reset the timeout each time that it is called but that has the disadvantage of requiring the caller to constantly update to avoid it timing out.

Alternatively the timeout parameter could be set to false which would require the caller to use the returned function to close the window.
Something like the following:

local notify_obj = notify("Hello world", "info", {timeout = false})

notify_obj.update("This is the new text to show!")

local timer = vim.loop.new_timer()
timer:start(2000, 0, notify_obj.update) -- Closes the window by not providing update text

I'll probably put some more thought into it before implementing it, The API could instead have an update and close function to be more explicit. Not sure if there would be a need for further actions in the future also

@zbirenbaum
Copy link

I'll probably put some more thought into it before implementing it, The API could instead have an update and close function to be more explicit. Not sure if there would be a need for further actions in the future also

This would be super useful. I just spent a couple hours making it so that my LSP diagnostics for a line appear in a notify window on hover and refresh on line change and the hacky stuff I had to put together with global variables and nvim_win_close is honestly terrifying.

@vogelsgesang
Copy link

vogelsgesang commented Jan 29, 2022

[...] The API could instead have an update and close function to be more explicit [...]

I think the update method should not only be able to update the message body, but also be able to change the title, the icon and to set a timeout.

My use case is similar to @hassec's, i.e. I want to show the progress of the background indexing for an LSP server.
I imagine this to work something like this:

LSP sends `$/progress; kind: begin`
  -> notify_obj = notify("Indexing started", {level: "info", title: "Indexing", timeout = false, icon: "spinner", timeout: false})
LSP sends `$/progress`; kind: report; progress: 1%`
  -> notify_obj.update("1% done") -- title stays "Indexing", icon still "spinner"
LSP sends `$/progress`; kind: report; progress: 2%`
  -> notify_obj.update("2% done") -- title stays "Indexing", icon still "spinner"
[...]
LSP sends `$/progress`; kind: report; progress: 99%`
  -> notify_obj.update("99% done") -- title stays "Indexing", icon still "spinner"
LSP sends `$/progress`; kind: end; final message: "indexed 982 files"`
  -> notify_obj.update("Indexed 982 files", {title: "Indexing | Finished", timeout: 5000, icon: "info"}) -- title & icon changed, also sets a timeout now

I would even go as far and say: update should accept all the same parameters as the original notify call.
A close on the original notification + a new notify would get me pretty close to that behavior, but have two shortcomings:

  1. a close + notify would place the new notification at the bottom of the stack. In contrast, an "updated" notification keeps the same position in the notification stack. E.g., if I have two progress indicators open at the same time, with close + notify being called repeatedly on both of them, the notifications would constantly swap places in the notification stack. Not what I want.
  2. In contrast to a new notification, the update inherits properties such as title and icon from the updated message.

Aspect (2) is optional in my opinion, it's just convenience and I could work this out outside of nvim-notify in my LSP $/progress callback. For aspect (1) I don't see how to work around it without support from nvim-notify.

As an additional feature, I would like to see all the in-between progress notifications in my notification history (:Telescope notify), e.g., in order to lookup how long indexing took. As such, I think notify_obj.update should not overwrite the previous notification completely. It should only replace the notification displayed on screen, but not replace the original notification in the notification history.

@rcarriga
Copy link
Owner

Sorry there's been no movement on this, been away and quite busy but glad to see some discussion on it!

I would even go as far and say: update should accept all the same parameters as the original notify call.

As an additional feature, I would like to see all the in-between progress notifications in my notification history (:Telescope notify), e.g., in order to lookup how long indexing took. As such, I think notify_obj.update should not overwrite the previous notification completely. It should only replace the notification displayed on screen, but not replace the original notification in the notification history.

This actually changes my mind on how this might work. Internally these would be totally separate notifications, the only difference being that they share the same window. I'd like to keep this transparent so instead of the returned object having properties to call, there would be another module function replace which would take the notify return value along with the same args as the default notify function and inherit any that aren't provided. So

local notify = require("notify")

local notify_obj = notify("1st message", "info", {
  title = "First Title",
  timeout = false, -- Won't close itself
  on_close = function()
    print("This is closed when replaced")
  end,
})

local next_notify_obj = notify.replace(notify_obj, "2nd message", nil, {
  on_close = function()
    print("This is also closed when replaced")
  end,
})

notify.replace(next_notify_obj, "3rd message", nil, {
  title = "Last Title",
  timeout = 1000,
  on_close = function()
    print("This is closed when timeout occurs")
  end,
})

@vogelsgesang
Copy link

Sorry there's been no movement on this, been away and quite busy but glad to see some discussion on it!

No worries. nvim-notify as it is today is already a great improvement to my workflow. Thank you for the time you already spent on it, and for being open to extend it further, e.g., based on the discussions in this feature request :)

Your latest API draft sounds great to me, and afaict would suit my use case perfectly.
Out of curiosity: Is there any reason why you prefer a new function over a new option?
E.g., I could also imagine:

local notify = require("notify")

local notify_obj = notify("1st message", "info", {
  title = "First Title",
  timeout = false, -- Won't close itself
  on_close = function()
    print("This is closed when replaced")
  end,
})

local next_notify_obj = notify(notify_obj, "2nd message", nil, {
  replace = notify_obj
  on_close = function()
    print("This is also closed when replaced")
  end,
})

notify(next_notify_obj, "3rd message", nil, {
  title = "Last Title",
  timeout = 1000,
  replace = next_notify_obj
  on_close = function()
    print("This is closed when timeout occurs")
  end,
})

I don't really see any big benefits of "new option" over "new separate function", so I am posting this mostly to make sure we explored a larger design space before committing to a new API.

This would be very close to option (1) from @hassec's original post, except the "key" object would be generated by the notify-library instead of by the caller. But, I guess with key objects generated by the library the "awkward state handling" (mentioned in the 2nd post) would no longer be an issue

@rcarriga
Copy link
Owner

rcarriga commented Feb 5, 2022

OK so I've got a branch with this feature. I've gone with the replace key in the options as it allows using the same function for all calls which I prefer to a new function, thanks for the suggestion @vogelsgesang. As this seems to be a popular request I'll give it time for people to test it out and give feedback. Here is the PR for it #59.

the hacky stuff I had to put together with global variables and nvim_win_close is honestly terrifying.

Fear no more @zbirenbaum, went with the same use case for an example of the functionality. Not the cleanest in the world but it works 😁

replace

local client_notifs = {}

local spinner_frames = { "", "", "", "", "", "", "", "" }

local function update_spinner(client_id, token)
  local notif_data = client_notifs[client_id][token]
  if notif_data and notif_data.spinner then
    local new_spinner = (notif_data.spinner + 1) % #spinner_frames
    local new_notif = vim.notify(
      nil,
      nil,
      { hide_from_history = true, icon = spinner_frames[new_spinner], replace = notif_data.notification }
    )
    client_notifs[client_id][token] = {
      notification = new_notif,
      spinner = new_spinner,
    }
    vim.defer_fn(function()
      update_spinner(client_id, token)
    end, 100)
  end
end

local function format_title(title, client)
  return client.name .. (#title > 0 and ": " .. title or "")
end

local function format_message(message, percentage)
  return (percentage and percentage .. "%\t" or "") .. (message or "")
end

vim.lsp.handlers["$/progress"] = function(_, result, ctx)
  local client_id = ctx.client_id
  local val = result.value
  if val.kind then
    if not client_notifs[client_id] then
      client_notifs[client_id] = {}
    end
    local notif_data = client_notifs[client_id][result.token]
    if val.kind == "begin" then
      local message = format_message(val.message, val.percentage)
      local notification = vim.notify(message, "info", {
        title = format_title(val.title, vim.lsp.get_client_by_id(client_id)),
        icon = spinner_frames[1],
        timeout = false,
        hide_from_history,
      })
      client_notifs[client_id][result.token] = {
        notification = notification,
        spinner = 1,
      }
      update_spinner(client_id, result.token)
    elseif val.kind == "report" and notif_data then
      local new_notif = vim.notify(
        format_message(val.message, val.percentage),
        "info",
        { replace = notif_data.notification, hide_from_history = false }
      )
      client_notifs[client_id][result.token] = {
        notification = new_notif,
        spinner = notif_data.spinner,
      }
    elseif val.kind == "end" and notif_data then
      local new_notif = vim.notify(
        val.message and format_message(val.message) or "Complete",
        "info",
        { icon = "", replace = notif_data.notification, timeout = 3000 }
      )
      client_notifs[client_id][result.token] = {
        notification = new_notif,
      }
    end
  end
end

@zbirenbaum
Copy link

OK so I've got a branch with this feature. I've gone with the replace key in the options as it allows using the same function for all calls which I prefer to a new function, thanks for the suggestion @vogelsgesang. As this seems to be a popular request I'll give it time for people to test it out and give feedback. Here is the PR for it #59.

the hacky stuff I had to put together with global variables and nvim_win_close is honestly terrifying.

Fear no more @zbirenbaum, went with the same use case for an example of the functionality. Not the cleanest in the world but it works 😁

replace

local client_notifs = {}



local spinner_frames = { "", "", "", "", "", "", "", "" }



local function update_spinner(client_id, token)

  local notif_data = client_notifs[client_id][token]

  if notif_data and notif_data.spinner then

    local new_spinner = (notif_data.spinner + 1) % #spinner_frames

    local new_notif = vim.notify(

      nil,

      nil,

      { hidden = true, icon = spinner_frames[new_spinner], replace = notif_data.notification }

    )

    client_notifs[client_id][token] = {

      notification = new_notif,

      spinner = new_spinner,

    }

    vim.defer_fn(function()

      update_spinner(client_id, token)

    end, 100)

  end

end



local function format_title(title, client)

  return client.name .. (#title > 0 and ": " .. title or "")

end



local function format_message(message, percentage)

  return (percentage and percentage .. "%\t" or "") .. (message or "")

end



vim.lsp.handlers["$/progress"] = function(_, result, ctx)

  local client_id = ctx.client_id

  local val = result.value

  if val.kind then

    if not client_notifs[client_id] then

      client_notifs[client_id] = {}

    end

    local notif_data = client_notifs[client_id][result.token]

    if val.kind == "begin" then

      local message = format_message(val.message, val.percentage)

      local notification = vim.notify(message, "info", {

        title = format_title(val.title, vim.lsp.get_client_by_id(client_id)),

        icon = spinner_frames[1],

        timeout = false,

        hidden = false,

      })

      client_notifs[client_id][result.token] = {

        notification = notification,

        spinner = 1,

      }

      update_spinner(client_id, result.token)

    elseif val.kind == "report" and notif_data then

      local new_notif = vim.notify(

        format_message(val.message, val.percentage),

        "info",

        { replace = notif_data.notification, hidden = false }

      )

      client_notifs[client_id][result.token] = {

        notification = new_notif,

        spinner = notif_data.spinner,

      }

    elseif val.kind == "end" and notif_data then

      local new_notif = vim.notify(

        val.message and format_message(val.message) or "Complete",

        "info",

        { icon = "", replace = notif_data.notification, timeout = 3000 }

      )

      client_notifs[client_id][result.token] = {

        notification = new_notif,

      }

    end

  end

end

Awesome!! Can't wait to test it out. Thanks so much for the features

@vogelsgesang
Copy link

I have been using your exact example snippet for a couple of days now.
Works like a charm! :)
Didn't have time yet to play around with other use cases, though...

@rcarriga
Copy link
Owner

It's a little noisy for me, never realised how many status updates Pyright and lua language server give 😅

For anyone using it currently, I've changed hidden to hide_in_history in the options

@vogelsgesang
Copy link

For clangd it's less noisy. I just never realized how slow clangd actually is, and how long it takes until it finished indexing a file

rcarriga added a commit that referenced this issue Feb 20, 2022
BREAKING CHANGE: Async return value now nests events under `events` key. 

Adds the ability to replace existing notifications and omit
notifications from the history.

See #43
@rcarriga
Copy link
Owner

OK this has now been merged, feel free to show any use cases below that could be added to the wiki 😁

akinsho added a commit to akinsho/dotfiles that referenced this issue Feb 23, 2022
Use the lsp notify function from
rcarriga/nvim-notify#43

to render lsp progress
@vogelsgesang
Copy link

Here is a snippet to display progress notifications from DAP.

For loading a medium-sized binary in lldb, this gives me the following.

Screen.Recording.2022-03-14.at.00.41.11.mov

Given that I also modified the LSP snippet to reuse some of the functionality, I also included my (slightly modified) version of LSP progress reporting

-- Utility functions shared between progress reports for LSP and DAP

local client_notifs = {}

local function get_notif_data(client_id, token)
 if not client_notifs[client_id] then
   client_notifs[client_id] = {}
 end

 if not client_notifs[client_id][token] then
   client_notifs[client_id][token] = {}
 end

 return client_notifs[client_id][token]
end


local spinner_frames = { "⣾", "⣽", "⣻", "⢿", "⡿", "⣟", "⣯", "⣷" }

local function update_spinner(client_id, token)
 local notif_data = get_notif_data(client_id, token)

 if notif_data.spinner then
   local new_spinner = (notif_data.spinner + 1) % #spinner_frames
   notif_data.spinner = new_spinner

   notif_data.notification = vim.notify(nil, nil, {
     hide_from_history = true,
     icon = spinner_frames[new_spinner],
     replace = notif_data.notification,
   })

   vim.defer_fn(function()
     update_spinner(client_id, token)
   end, 100)
 end
end

local function format_title(title, client_name)
 return client_name .. (#title > 0 and ": " .. title or "")
end

local function format_message(message, percentage)
 return (percentage and percentage .. "%\t" or "") .. (message or "")
end


-------------------
-- DAP integration

dap.listeners.before['event_progressStart']['progress-notifications'] = function(session, body)
 local notif_data = get_notif_data("dap", body.progressId)

 local message = format_message(body.message, body.percentage)
 notif_data.notification = vim.notify(message, "info", {
   title = format_title(body.title, session.config.type),
   icon = spinner_frames[1],
   timeout = false,
   hide_form_history = false,
 })

 notif_data.notification.spinner = 1,
 update_spinner("dap", body.progressId)
end

dap.listeners.before['event_progressUpdate']['progress-notifications'] = function(session, body)
 local notif_data = get_notif_data("dap", body.progressId)
 notif_data.notification = vim.notify(format_message(body.message, body.percentage), "info", {
   replace = notif_data.notification,
   hide_form_history = false,
 })
end

dap.listeners.before['event_progressEnd']['progress-notifications'] = function(session, body)
 local notif_data = client_notifs["dap"][body.progressId]
 notif_data.notification = vim.notify(body.message and format_message(body.message) or "Complete", "info", {
    icon = "",
    replace = notif_data.notification,
    timeout = 3000
 })
 notif_data.spinner = nil
end


-------------------
-- LSP integration

vim.lsp.handlers["$/progress"] = function(_, result, ctx)
 local client_id = ctx.client_id

 local val = result.value

 if not val.kind then
   return
 end

 local notif_data = get_notif_data(client_id, result.token)

 if val.kind == "begin" then
   local message = format_message(val.message, val.percentage)

   notif_data.notification = vim.notify(message, "info", {
     title = format_title(val.title, vim.lsp.get_client_by_id(client_id).name),
     icon = spinner_frames[1],
     timeout = false,
     hide_from_history = false,
   })

   notif_data.spinner = 1
   update_spinner(client_id, result.token)
 elseif val.kind == "report" and notif_data then
   notif_data.notification = vim.notify(format_message(val.message, val.percentage), "info", {
     replace = notif_data.notification,
     hide_from_history = false,
   })
 elseif val.kind == "end" and notif_data then
   notif_data.notification =
     vim.notify(val.message and format_message(val.message) or "Complete", "info", {
       icon = "",
       replace = notif_data.notification,
       timeout = 3000,
     })

   notif_data.spinner = nil
 end
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants