Skip to content

Conversation

@antoinemadec
Copy link
Contributor

Implement vim.notify to use simple pre-existing ui.nvim messages.

It calls msg_show() with replace_last = false, I believe this is expected implementation.
That way we don't have to set respect_replace_last = false globally. See #28

@OXY2DEV
Copy link
Owner

OXY2DEV commented May 18, 2025

I don't think it's a good idea to wrap any of the message functions as it won't be visible in :messages after disabling the plugin.

Maybe it would be better to do a feature request in Neovim to allow manually disabling replace_last for vim.notify, vim.notify_once & nvim_echo via the {opts} parameter.

@antoinemadec
Copy link
Contributor Author

antoinemadec commented May 18, 2025

I mean vim.notify is meant to be overridden by plugins, it's in the help.

People started to rely on the opts.title as well.

We could save the original implementation of vim.notify and fallback to it when ui.nvim is disabled.

What do you think?

To me it's good that this plugin handles all messages, including the ones coming from vim.notify.

I could install another plugin on the side or put that bit of code in my init.lua, but it just makes it simpler this way IMO.

Just let me know 🙂

@OXY2DEV OXY2DEV closed this in cc9a233 May 19, 2025
@OXY2DEV
Copy link
Owner

OXY2DEV commented May 19, 2025

For some reason GitHub wouldn't let me push changes to the PR(even though it says otherwise on the site). So, I am doing a normal commit.

The main difference between my commit and this one is,

  • Parity with the original functions(see runtime/lua/vim/_editor.lua). OFF level messages are no longer discarded. WARNING & ERROR level messages get their respective highlight groups applied to the message.
  • Added support for notify_once().
  • Added support for :UI toggle.
  • Added support for title in notify messages.
  • Changed option name to wrap_notify as it wraps the actual function.

@antoinemadec
Copy link
Contributor Author

Thanks a lot!

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

Successfully merging this pull request may close these issues.

2 participants