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

Should all server capabilities allow the corresponding dynamic registration options? #1908

Open
michaelpj opened this issue Mar 14, 2024 · 2 comments
Labels
info-needed Issue requires more information from poster

Comments

@michaelpj
Copy link
Contributor

Some, but not all, of the server capability fields in the initialize response allow providing the corresponding registration options. It's helpful to be able to do so, since the registration options type usually includes additional fields, such as the documentSelector.

Examples:

  • completionProvider has type CompletionOptions and doesn't allow CompletionRegistrationOptions
  • typeDefinitionProvider has type TypeDefinitionOptions | TypeDefinitionRegistrationOptions

Is this just for backwards compatibility? Is there a way we could migrate the protocol to be more consistent here? It would make things simpler. For example, if you want to make use of the documentSelector capability, you can have many cases:

  • Client supports dynamic registration: dynamically register
  • Client does not support dynamic registration:
    • Options field supports dynamic registration options: statically register
    • Options field does not support dynamic registration options: give up

It's quite fiddly.

@dbaeumer
Copy link
Member

Is this just for backwards compatibility?

Yes, it is only for backwards compatibility. There could be old clients which don't support options. I tired to be consistent for all new requests I introduce.

The only solution that came to my mind when doing this was adding a client capability to inform the server. But it felt a little bit like overhead. If someone has an idea I am happy to look into this again.

@dbaeumer dbaeumer added the info-needed Issue requires more information from poster label Mar 15, 2024
@michaelpj
Copy link
Contributor Author

Yes, I agree that you would probably need a capability (dynamicRegistrationDuringInitialize?). You can't just say the client can ignore the excess fields, since the server might prefer to dynamically register in order to make the settings have effect.

I agree it's some overhead, but it might be worth it. In particular, I don't know whether you have any plan to eventually mark some capability settings as deprecated, so as to push implementors towards the "recommended" way of doing things? If there's a path towards one day making the more sensible behaviour the default then it seems worth taking a step down that path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
info-needed Issue requires more information from poster
Projects
None yet
Development

No branches or pull requests

2 participants