rcarriga / nvim-notify

A fancy, configurable, notification manager for NeoVim
MIT License
3.09k stars 83 forks source link

Feature request: Additional text only to be displayed in notification history #57

Closed vogelsgesang closed 2 years ago

vogelsgesang commented 2 years ago

Some notifications (e.g., clangd's AST dumps; see #56) can be quite large. I would want to truncate the text displayed in the notification itself, and only have the full text visible in the notification history. The notification would then display something like For the full message, look in the notification history.

I could imagine a few alternatives on how to achieve this:

rcarriga commented 2 years ago

a new auto_truncate option which, when set, automatically truncates the on-screen notification but still adds the full notification text to the notification history

Something along these lines is definitely the route I prefer, my thought would be a simple max_height/max_width config option or something similar that'd limit the length of the windows when animated. I think keeping it as a config option makes sense as currently the notification options don't have anything to do with window placement or sizing. Don't think I can see a use case for a single notification determining its dimensions outside of the user config

vogelsgesang commented 2 years ago

I agree, having a max_height/max_width config option set through the notify.setup option would also be my preferred solution. It's certainly most user-friendly/least effort to use. I also don't see an use case for changing max_width/other truncation options for individual messages

rcarriga commented 2 years ago

Try with the latest commit, you can now supply max_height/max_width settings

vogelsgesang commented 2 years ago

Works beautifully! Thank you!

The notification is now truncated:

Screenshot 2022-02-01 at 12 36 22

In the history it is still complete:

Screenshot 2022-02-01 at 12 36 46
fitrh commented 2 years ago

Is it possible to pass max_width/height on every call ? I have configuration like this:

max_width = math.floor(vim.api.nvim_win_get_width(0) / 3)

So I can have notification with width of 1/3 of the window width, but it seems the option only passed on setup ?

rcarriga commented 2 years ago

I don't really want to add window placement/sizing logic to the notifications themselves as it couples them to the implementation.

I'm a bit confused as to why you'd want to use the current window's width in the first place. What happens if you have a few splits open and the notification window ends up tiny? Or do you mean the NeoVim window as a whole?

vogelsgesang commented 2 years ago

interesting idea, @fitrh. I am also switching between different screens, and a fixed max-widt of 120 characters is too low on the bigger screen and too high on the smaller screen :/

@rcarriga I agree that this should not be an adhoc per notification, but be part of the global configuration. I could imagine something like

require("notify").setup({
  max_width = function() begin
    return math.floor(vim.api.nvim_get_screen_size() / 3)
  end
})

(no idea how to get the size of the complete neovim window/screen. So I just made up the nvim_get_screen_size call.)

fitrh commented 2 years ago

I'm a bit confused as to why you'd want to use the current window's width in the first place. What happens if you have a few splits open and the notification window ends up tiny? Or do you mean the NeoVim window as a whole?

Sorry, yes, I mean the whole window

rcarriga commented 2 years ago

OK that makes more sense to me then yes as @vogelsgesang said, this is more of a requirement for dynamically setting the value rather than setting it per notification :smile:

rcarriga commented 2 years ago

Please try with the latest commit

    max_width = function ()
      return math.ceil(math.max(vim.opt.columns:get() / 3, 10))
    end,
    max_height = function ()
      return math.ceil(math.max(vim.opt.lines:get() / 3, 4))
    end,
fitrh commented 2 years ago

https://user-images.githubusercontent.com/55179750/152957414-1d0037dc-b97a-4a03-83b6-f603d17e89e8.mp4

Thanks @rcarriga, it works as expected

rcarriga commented 2 years ago

Just noticed I pushed that commit to the wrong branch :facepalm: It's now in master