Skip to content
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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

theothornhill
Copy link
Collaborator

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?

@skangas
Copy link
Collaborator

skangas commented Jan 14, 2022

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?

@theothornhill
Copy link
Collaborator Author

theothornhill commented Jan 14, 2022

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.

@skangas
Copy link
Collaborator

skangas commented Jan 15, 2022

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.

@theothornhill
Copy link
Collaborator Author

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
Copy link
Collaborator

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.

Copy link
Collaborator Author

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.
@theothornhill
Copy link
Collaborator Author

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 :)

@joaotavora
Copy link
Owner

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.

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.

3 participants