-
Notifications
You must be signed in to change notification settings - Fork 200
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
Add more interfaces to lsp-interface-alist #805
base: master
Are you sure you want to change the base?
Conversation
What are the benefits with listing them there besides documentation? If the only benefit is documentation, how about adding the ones we don't support commented out? |
abec8df
to
6d0bc9b
Compare
Well, the benefit is hypothetical. If a user decides to use the eglot-dbind and friends using one of them and trying to use incorrect keys, they'll get a warning. So they are in a sense supported, but not used. This is used mostly(only?) for the compilation of the spec, and only used when strict mode is set, I think. |
Sounds good to me. Are there any downsides to adding them that we should consider? Also, do we really want to do that reformatting? I don't have an opinion either way, but maybe @joaotavora does. |
No need to do the formatting, but it is a huge wall of text without, hehe :) |
eglot.el
Outdated
(:arguments)) | ||
(CompletionItem | ||
(:label) | ||
(:kind :detail :documentation :deprecated :preselect |
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.
BTW, if you type ( :kind
you should get the data indentation in a recent Emacs. I believe it might be more natural here.
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.
TIL, thanks :)
* eglot.el (eglot--lsp-interface-alist): Add most of, if not all relevant lsp client interfaces.
a418dfd
to
74ffece
Compare
I've rebased my stuff and made some changes to the formatting. It has a little smaller footprint now. I think it's good to go, unless @joaotavora thinks this is too much :) |
Sorry I'm late. I don't have a very strong opinion here. I lean towards adding these things as we go, so as to generate as little noise as possible, like searching for keywords inthe file and getting the (wrong) impression that Eglot somehow implements the associated features. But If @skangas thinks it's useful, then ok. The above problem could be fixed by having the variable declaration in a separate file, or, better yet, generating it compile-time from the actual LSP spec. Yeah, that would be the Lisp thing to do. |
There are many missing interfaces in eglot, and I added most, if not all of the relevant ones to the alist. This should help us move further along into lsp compliance.
What do you think? Is it useful adding them in bulk like this, or should we wait until there's need for a new one?