sansio-lsp-client icon indicating copy to clipboard operation
sansio-lsp-client copied to clipboard

feat: capabilities parameter for Client

Open dsanders11 opened this issue 3 years ago • 8 comments

It's useful to be able to change the capabilities for a client. I'm using clangd and there are capabilities I'd like to enable, such as inline fixes for diagnostics.

dsanders11 avatar Feb 03 '22 02:02 dsanders11

To use codeActions, you need to add a codeActions property to class Diagnostic in structs.py and change the capabilities that are sent. Can you make a new pull request that does that, or do you want me to do it?

I don't think letting users specify the capabilities makes sense. The capabilities represent what sansio-lsp-client is capable of doing, not what the user wants to do.

Akuli avatar Feb 03 '22 18:02 Akuli

I don't think letting users specify the capabilities makes sense.

Perhaps if this was a CLI or other end-user targeting project, but isn't this a library for developers to use? I want to subclass Client and extend it to handle my uses, which in this case involve adding capabilities it doesn't currently handle. I don't expect this project to support all possible capabilities, that's unreasonable, but I do want to have the ability to support them using this library.

Ideally I'd be able to do something like this (this is somewhat pseudocode, since parse_request isn't exposed):

import copy
from typing import List

import sansio_lsp_client as lsp

class ClangdDiagnostic(lsp.Diagnostic):
    codeActions: List

class ClangdPublishDiagnostics(PublishDiagnostics):
    diagnostics: List[ClangdDiagnostic]

class ClangdClient(lsp.Client):
    def __init__(self, *args, **kwargs):
        capabilities = copy.deepcopy(lsp.client.CAPABILITIES)
        capabilities ["textDocument"]["publishDiagnostics"]["codeActionsInline"] = True

        kwargs["capabilities"] = capabilities

        super().__init__(*args, **kwargs)

    def _handle_request(self, request: lsp.Request) -> lsp.Event:
        if request.method == "textDocument/publishDiagnostics":
            return parse_request(ClangdPublishDiagnostics)

        return super()._handle_request(request)

If your concern is making it a keyword argument to Client, perhaps CAPABILITIES could be moved into the Client class so it can be changed by subclasses? As it stands there's no way to change it for a client without monkey patching CAPABILITIES directly, or re-implementing __init__ in a subclass. Neither one is a great option.

dsanders11 avatar Feb 03 '22 19:02 dsanders11

We can make sansio-lsp-client more subclassing friendly. It wasn't designed with subclassing in mind. If moving CAPABILITIES or other things into Client makes subclassing easier for you, let's do it :)

But instead of subclassing, it would be generally better if you just make a pull request here, so that other people can benefit from the new feature too. We can make a release within a few minutes from merging a PR.

Akuli avatar Feb 03 '22 19:02 Akuli

Ok, I'll try to find some time. This was mostly a fire and forget PR so I could implement in the future.

I'm not sure about adding clangd-specific capabilities into the main code though, maybe as a separate subclass like I've illustrated above. Reason being that capabilities not in the main spec could always conflict - another language server might implement also have a capability named "codeActionsInline" but implement it differently. So I wouldn't want to add codeActions to the general Diagnostic class since that's starting to tie too closely to clangd.

dsanders11 avatar Feb 03 '22 19:02 dsanders11

If several langservers add different non-standard ways to do the same thing, it would be IMO best to add the necessary if statements into sansio-lsp-client, rather than into every project that uses sansio-lsp-client.

Akuli avatar Feb 03 '22 20:02 Akuli

Does sansio-lsp-client currently have any support for non-standard, language server specific capabilities? I assumed it's more a generic client, rather than trying to support all possible language servers out there. For my uses I just need a Python LSP client I can build on top of for scripting interaction with clangd, that might be an unusual use case.

dsanders11 avatar Feb 03 '22 22:02 dsanders11

We currently don't support any non-standard langserver specific capabilities, but I'm fine with adding support for them, as long as they don't get in the way when using other langservers. I would like to use clangd's codeActions in my editor too, and I wouldn't mind writing some clangd specific code when using sansio-lsp-client.

Akuli avatar Feb 04 '22 09:02 Akuli

Thanks for the clarification, makes sense. 👍

dsanders11 avatar Feb 04 '22 09:02 dsanders11