swyddfa / lsp-devtools

Tooling for working with language servers and clients.
https://lsp-devtools.readthedocs.io/en/latest/
64 stars 10 forks source link

Add flexibility for extending the LanguageClient #195

Open eliericha opened 4 days ago

eliericha commented 4 days ago

The API is made so that many aspects are customizable, and that's great! However I hit the following limitation.

ClientServerConfig can be given a client_factory to replace make_test_lsp_client. That allows me to inject my own class that extends LanguageClient.

However make_test_lsp_client does the registration of the handlers for capturing diagnostics, progress reporting, etc... I still want to benefit from that and currently I cannot reuse that.

So would it be possible to move the registration of handlers to a reusable method that takes the client as a parameter?

Alternatively, could make_test_lsp_client be given a factory function to instantiate the LanguageClient object? That would allow me to pass my own class.

Thanks!

eliericha commented 3 days ago

Actually I tried copying make_test_lsp_client to use my own LanguageClient and hit an issue:

def my_client_factory() -> MyLanguageClient:
    client = MyLanguageClient(
        converter_factory=default_converter,
    )

    @client.feature(types.WORKSPACE_CONFIGURATION)
    def configuration(client: LanguageClient, params: types.ConfigurationParams):
        return [
            client.get_configuration(section=item.section, scope_uri=item.scope_uri)
            for item in params.items
        ]

    ...

This works initially, but fails later when the feature handler is called. The first argument of the handler has type LanguageClient while the client type is MyLanguageClient. The generic handler infrastructure is unable to determine that it should pass the client as the first parameter because the types don't match exactly, and thus it makes a call to the handler with one parameter missing --> exception.

This means that it will be difficult/impossible to extract the feature registration code to a function that accepts any class extending LanguageClient.

alcarney commented 3 days ago

This works initially, but fails later when the feature handler is called. The first argument of the handler has type LanguageClient while the client type is MyLanguageClient. The generic handler infrastructure is unable to determine that it should pass the client as the first parameter because the types don't match exactly, and thus it makes a call to the handler with one parameter missing --> exception.

That definitely sounds like a bug report for pygls (which provides the feature registration system).

However, I'm fairly sure you can work around the issue for now by renaming your client: LanguageClient parameter to ls: LanguageClient. (See upstream docs)

alcarney commented 3 days ago

But yes, I agree there should be a mechanism to make it easy to use a custom client class.

eliericha commented 2 days ago

I see. Thanks for the doc pointer!

Indeed renaming the parameter to ls makes it work. And a quick debug session allowed me to understand the source of the bug :) I'll report the findings on the pygls issue.

So now there is nothing blocking an API change in pytest-lsp to support extending LanguageClient.

I suggest simply moving the feature registration code into a register_test_client_lsp_features(ls : LanguageClient).

That would allow me to provide a client_factory that instantiates my class, and defers to register_test_client_lsp_features for reuse.