Skip to content

Add generated Requests/Responses/Notifications to protocol#2781

Merged
rchl merged 5 commits intomainfrom
fix/protocol-new-tyeps
Feb 27, 2026
Merged

Add generated Requests/Responses/Notifications to protocol#2781
rchl merged 5 commits intomainfrom
fix/protocol-new-tyeps

Conversation

@rchl
Copy link
Copy Markdown
Member

@rchl rchl commented Feb 26, 2026

New types to be used in the new LspPlugin API. More details in sublimelsp/lsp-python-types#31 (comment)

@jwortmann
Copy link
Copy Markdown
Member

jwortmann commented Feb 27, 2026

So in a plugin would you now do

    def on_server_response_async(self, response: ServerResponse) -> None:
        if response.method == "textDocument/hover":
            hover = response.result  # automatically inferred as Hover | None

Is that correct?

How are the params for custom requests/notifications handled? I guess they are Any by default and you still need cast for them?

Edit: Actually I guess that's not too important, because for custom requests/notifications, the handler functions and params (if sent from client to server) need to be defined manually in the plugin anyways. So I guess there is no point in using the LspPlugin.on_... methods for those.

@rchl
Copy link
Copy Markdown
Member Author

rchl commented Feb 27, 2026

Yes, as you said.

EDIT: One correction - the response would be a dict so we'd need to access using square brackets.

@jwortmann
Copy link
Copy Markdown
Member

Okay, I have one suggestion:
Could we generate a separate file for these (or alternatively put them into plugin/core/protocol.py)? Because they are not part of the official specs, and I would like to keep the file under LSP/protocol clean from any custom definitions.

@rchl
Copy link
Copy Markdown
Member Author

rchl commented Feb 27, 2026

Okay, I have one suggestion: Could we generate a separate file for these (or alternatively put them into plugin/core/protocol.py)? Because they are not part of the official specs, and I would like to keep the file under LSP/protocol clean from any custom definitions.

That's a bit debatable. The spec schema has definitions for all the requests, responses and notifications. The types here are not 1:1 reflection of those, but shaped and filtered a bit but still very close to protocol.

I'm not against the idea of separating it but implementation-wise it would be messy because then we'd need to import hundreds of types from .protocol which I wouldn't love. (I know about star imports but those are discouraged from typing point of view)

@jwortmann
Copy link
Copy Markdown
Member

If you put it into plugin/core/protocol.py you don't even need any new import, because we already have everything from official protocol types available there:

from ...protocol import * # For backward compatibility with LSP packages.

I think that is the best place where these types should belong to. Things like

class RequestMessage(TypedDict):
jsonrpc: str
id: str | int
method: str
params: NotRequired[Any]
class ResponseMessage(TypedDict):
jsonrpc: str
id: str | int
result: NotRequired[Any]
error: NotRequired[ResponseError]
class NotificationMessage(TypedDict):
jsonrpc: str
method: str
params: NotRequired[Any]
JSONRPCMessage = Union[RequestMessage, ResponseMessage, NotificationMessage]

are also very related to the protocol, but still not in the official types.

The main point of #2660 was to separate official protocol types from our own helpers and type definitions, and I would prefer to keep it that way (otherwise we could just revert #2660 and put everything back into the plugin/core/protocol.py file).

@rchl
Copy link
Copy Markdown
Member Author

rchl commented Feb 27, 2026

If you put it into plugin/core/protocol.py you don't even need any new import, because we already have everything from official protocol types available there:

Did not remember that. Then it won't be fault using it :)

It will make it harder to make all of those "public" then since I don't want to re-import all of those through LSP.plugin. I will re-import the Union types but someone might still want to use individual types.

EDIT: Actually, shouldn't be needed to re-export all. People will only need the actual request/result params from protocol.

@rchl rchl merged commit 4c9c17f into main Feb 27, 2026
8 checks passed
@rchl rchl deleted the fix/protocol-new-tyeps branch February 27, 2026 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants