openlawlibrary / pygls

A pythonic generic language server
https://pygls.readthedocs.io/en/latest/
Apache License 2.0
552 stars 101 forks source link

Explore parity between `server.LanguageServer` and `protocol.LanguageServerProtocol` methods #306

Open tombh opened 1 year ago

tombh commented 1 year ago

In server.LanguageServer we have a lot of methods that look like:

def apply_edit(
        self, edit: WorkspaceEdit, label: Optional[str] = None
    ) -> WorkspaceApplyEditResponse:
        """Sends apply edit request to the client."""
        return self.lsp.apply_edit(edit, label)

Therefore, they do nothing but call out to the underlying protocol.LanguageServerProtocol method. There may be a good reason for this, but it isn't documented. There are some cases such as with publish_diagnostics, where there is an advantage because the server.LanguageServer method signature is simpler.

Let's either document the reasoning for the apparent repetition, or deprecate the duplicated server versions in favour of the protocol versions.

alcarney commented 1 year ago

or deprecate the duplicated server versions in favour of the protocol versions.

Do you have any thoughts on the opposite?

Would it make sense to try and simplify the protocol classes (possibly even try to remove LanguageServerProtocol completely??) and instead keep the methods on the LanguageServer class and have them call the base notify, send_request, send_response methods on the JsonRPCProtocol class?

tombh commented 1 year ago

Sorry for the late reply. The short answer is I don't know enough to have an opinion. I would very much trust your intuition on this. Will be interesting to see what #328 and its successors bring to the table.