-
-
Notifications
You must be signed in to change notification settings - Fork 335
modules/lsp: add onAttach
option
#3280
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation looks good to me.
However, I agree about assessing the relevance of such a feature first.
We could maybe poll our users in the Matrix chat?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would make my work in #3289 simpler!
ae8fd97
to
665586f
Compare
Similar to `plugins.lsp.onAttach`, implement a "global" equivalent to the per-server `on_attach` callback. This is implemented using a `LspAttach` autoCmd.
665586f
to
2b3daaf
Compare
I've added a second commit which refactors #3289 to make use of the new option. Hopefully this demonstrates the usefulness of it? |
2b3daaf
to
1ed7754
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
1ed7754
to
2ea0131
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Similar to
plugins.lsp.onAttach
, implement a "global" equivalent to the per-serveron_attach
callback.The implementation is simpler than
plugins.lsp.onAttach
, by using aLspAttach
autocmd. The old impl was to propagate the global onAttach lines into each server'son_attach
callback.The old impl should be able to migrate to using the new option under the hood, or even become an alias of it.
Alternatively, we might decide we don't want this at all? Although it seems convenient.
Perhaps in future it'd be nice to have a similar per-server
onAttach
option? When the per-serveronAttach
option is defined,settings.on_attach
would be derived from it, using it as the function body.