neovim
neovim copied to clipboard
feat(lsp): notify on number of changes for `vim.lsp.buf.rename`
This is a small PR modifying vim.lsp.buf.rename(). It adds a notification on how many occurrences in how many files have changed, like inc-rename does.
My first PR to neovim core, so please feel free to correct me if I missed anything.
I wonder if this will make too much unnecessary notification (through vim.notify, and there seems no possible way to turn it off. Isn't customization by overriding the LSP handler enough (in user config, or a plugin provides the functionality)?
vim.lsp.handlers[ms.textDocument_rename] = (function(original_handler)
return function(err, result, ctx, config)
-- TODO: Add Notification here
original_handler(err, result, ctx, config)
end
end)(vim.lsp.handlers[ms.textDocument_rename])
An alternative way one can consider is to make use of config table in the textDocument/rename handler, and allow easier customization via vim.lsp.with. But adding configuration points should be approached carefully.
I wonder if this will make too much unnecessary notification
Imo, the fact that vim.lsp.buf.rename can apply changes to files that you haven't even opened is consequential enough to warrant a notification. Right now, it is too easy to accidentally make a change to several files without noticing it.
and there seems no possible way to turn it off
True. I wanted to be consistent with existing functions, and to my knowledge, none of them offer configurability regarding the display of notifications. But adding an option to opt-out (or even opt-in) of the notification shouldn't be an issue, if that's what everyone else thinks.
Isn't customization by overriding the LSP handler enough
In my view, overriding the LSP handler is not user-friendly or discoverable enough. Reading :h vim.lsp.buf.rename, for instance, there is no indication that it is possible to further customize the behavior via handler. And even with the information that handlers allow for more customization, I'd have a hard time as a new user to figure out how to do that after only reading :h lsp-handler.
I wonder if this will make too much unnecessary notification
Imo, the fact that
vim.lsp.buf.renamecan apply changes to files that you haven't even opened is consequential enough to warrant a notification. Right now, it is too easy to accidentally make a change to several files without noticing it.and there seems no possible way to turn it off
True. I wanted to be consistent with existing functions, and to my knowledge, none of them offer configurability regarding the display of notifications. But adding an option to opt-out (or even opt-in) of the notification shouldn't be an issue, if that's what everyone else thinks.
Isn't customization by overriding the LSP handler enough
In my view, overriding the LSP handler is not user-friendly or discoverable enough. Reading
:h vim.lsp.buf.rename, for instance, there is no indication that it is possible to further customize the behavior via handler. And even with the information that handlers allow for more customization, I'd have a hard time as a new user to figure out how to do that after only reading:h lsp-handler.
I've got my hand-rolled handler that populates QF, and pops up a notification with multiple info points. Would be great not to have to add another override into noice to route these messages out of notifications.
Okay then, I added an option for whether to show the notification.
I think you can make the argument to make it opt-out, since the notification is rather making the function more beginner-friendly, but to be as conservative as possible, I made it opt-in for now.
edit: On the failing tests
- news failing is fine, since this change is too small for news.
- The FreeBSD test failing I don't really understand, tbh.
- docs are failing I can see, but running the recommended
make docgives the following error and I do not know what this is about?
cd .deps && \
cmake -G 'Unix Makefiles' \
/Users/chrisgrieser/repos/neovim//cmake.deps
/bin/sh: line 0: cd: .deps: No such file or directory
make: *** [build/.ran-deps-cmake] Error 1