Allow to disable selected capabilities by the client

[przepompownia]

Closes #56

[przepompownia]

I still do not understand why SignatureHelpOptions is set in two places.

[przepompownia]

Is $provideTextEdit left as default false intentionally in CompletionHandler's constructor?

[przepompownia]

testHandleWithDisabledCapability() should not pass in 619ad40 (it currently is almost copy of testHandleACompleteListOfSuggestions but I expect that completion() won't be called (or return an empty result?)). @dantleech do you have idea how to fix it?

[dantleech]

I still do not understand why SignatureHelpOptions is set in two places.

There is no reason. I think the CompletionHandler pre-dated the SignatureHelp handler.

[dantleech]

What else is the purpose of sending client LSP-defined capabilities than making an intersection with the server-provided set in the result?

So that the server knows what the client supports so it can avoid sending messages to the client that would result in an error I think.

In that Reddit post the servers do not support the capabilities, it's a different problem I think and the goal is to stop the client sending messages.

Would it not also make equal sense that the client could be configured to disable certain features and not request them? If you go to the trouble of disabling the Client Capabilty, surely the client can also simply not use that capability (although to me that also makes no sense, as capabilities are not, or do not have to be, user defined)

[dantleech]

I still do not understand if dynamicRegistration can be related to this issue:

I don't think it is. It's a way to say "from now on, I don't want the client to send me these messages" I think. Dynamic registration is sever => client. IIRC

[przepompownia]

So that the server knows what the client supports so it can avoid sending messages to the client that would result in an error I think.

Yes, and if I understand you mean support as the set of possible client capabilities and I mean those chosen from them for a single instance.

For example, nvim built-in client allows to start with vim.lsp.protocol.make_client_capabilities() and change this table before sending to the server. Sometimes we can extend this table but this is not the case.

Do you have concerns that this is LSP incompatible?

In summary we have three possible independent solutions:

• client-side implementation - requires additional implementation from the client app and additional configuration options. If you start using another client then you can have the same problem again (I found an example https://lsp.sublimetext.io/guides/client_configuration/ with phpactor)
• server side non-LSP implementation - requires also specific server-side configuration per project
• server-side LSP implementation (as there) - assuming that is it LSP-compliant, it requires no client or server specific configuration options - only the standard capability set.

[dantleech]

Do you have concerns that this is LSP incompatible?

yes, it's an interpretation of the spec which is not specific on this (or I missed something). I would assume that other LS servers do not disable capabilities based on the client configuration, and then this is not a general solution, but a "hack" for Phpactor

e.g. I would like to disable completion, but there is not a client capabiltiy for that, only a server one.

interesting that the Sublime LSP server has the disable_capabilities feature. I tend to think it should be client side.

[przepompownia]

Neither of us seems to be 100% sure how it should be. I also would not like to propose a hack. The source is my understanding that this completes the missing protocol behavior. Maybe it's good moment to ask someone on https://github.com/microsoft/language-server-protocol before close or merge it.

interesting that the Sublime LSP server has the disable_capabilities feature. I tend to think it should be client side.

I take into account that this client-side solution was implemented to workaround the lack of such feature ;-)

[przepompownia]

Please look at https://github.com/microsoft/language-server-protocol/issues/1403

[dantleech]

Thanks for that. Given that behavior I would simply not register the handler rather than have the handler decide if it exposes that capability. But, at least in my case, it doesn't help with disabling completion or reference etc.

(have to run now, will have a think)

[przepompownia]

Initially I was wondering how to turn off the whole handler but I did not sure if we have one-to-one relation between handler and a corresponding LSP capability.

[dantleech]

Sorry for not getting back on this. I still think that deciding to support/not support a given feature is a client-side decision and should be incorporated there. The client makes the calls. If we were to implement this using the approach here, there would still be no way of f.e. disabling completion, and what's more not all clients will allow the capabilities to be overridden.

Disabling the server side capabilities based on client capabilities in the referenced issue seems more about "not wasting unneeded resources".

I think having Phpactor options to enable/disable a capability would still be beneficial, but I currently don't think that using the client capablities is a satisfactory way of doing that.

[przepompownia]

Sorry for not getting back on this.

No problem. While it works I simply use this branch and can focus on other things in the meantime. Independently of your choice and testing other approaches (including your propositions) there is also no problem for me to keep this branch up to date and, possibly, find some time to check how to free unused resources.

If we were to implement this using the approach here, there would still be no way of f.e. disabling completion, and what's more not all clients will allow the capabilities to be overridden.

Lack of user access to restricting capabilities at a particular client side is an issue of such client.