Open
Conversation
✅ Deploy Preview for sublime-lsp ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Add a test that checks that "tcp server mode" works. Server meaning that this plugin acts as the TCP server and the langserver connects as TCP client.
rchl
reviewed
Apr 14, 2026
|
|
||
|
|
||
| async def main(tcp_port: int | None = None) -> bool: | ||
| async def main(tcp_port: int | None = None, mode: str | None = None) -> bool: |
Member
There was a problem hiding this comment.
Can you set type for mode to Literal['client', 'server'] | None so that it self-documents allowed values?
rchl
reviewed
Apr 14, 2026
| await server.serve_forever() | ||
| return callback.received_shutdown | ||
|
|
||
| if mode is not None and mode == "client": |
Member
There was a problem hiding this comment.
Suggested change
| if mode is not None and mode == "client": | |
| if mode == "client": |
| parser = ArgumentParser(prog=__package__, description=__doc__) | ||
| parser.add_argument("-v", "--version", action="store_true", help="print version and exit") | ||
| parser.add_argument("-p", "--tcp-port", type=int) | ||
| parser.add_argument("--mode", help="one of 'client' or 'server'", default="server") |
Member
There was a problem hiding this comment.
Can validate the argument early and then also doesn't really need a redundant help:
Suggested change
| parser.add_argument("--mode", help="one of 'client' or 'server'", default="server") | |
| parser.add_argument("--mode", help="one of 'client' or 'server'", default="server", choices=["client", "server"]) |
Comment on lines
+46
to
+53
| config = ClientConfig( | ||
| name=name, | ||
| command=["python3", join("$packages", "LSP", "tests", "server.py")], | ||
| selector="text.plain", | ||
| enabled=True, | ||
| ) | ||
| config.initialization_options.assign(init_options) | ||
| return config |
Member
There was a problem hiding this comment.
Why not pass init_options to the constructor and save lines?
Suggested change
| config = ClientConfig( | |
| name=name, | |
| command=["python3", join("$packages", "LSP", "tests", "server.py")], | |
| selector="text.plain", | |
| enabled=True, | |
| ) | |
| config.initialization_options.assign(init_options) | |
| return config | |
| return ClientConfig( | |
| name=name, | |
| command=["python3", join("$packages", "LSP", "tests", "server.py")], | |
| selector="text.plain", | |
| initialization_options=DottedDict(init_options), | |
| enabled=True, | |
| ) |
Same below.
| def make_stdio_test_config() -> ClientConfig: | ||
| return ClientConfig( | ||
| name="TEST", | ||
| def make_stdio_test_config(name: str, init_options: dict[str, Any]) -> ClientConfig: |
Member
There was a problem hiding this comment.
Maybe make init_options optional since it's not always needed?
Suggested change
| def make_stdio_test_config(name: str, init_options: dict[str, Any]) -> ClientConfig: | |
| def make_stdio_test_config(name: str, init_options: dict[str, Any] | None = None) -> ClientConfig: |
| return ClientConfig( | ||
| name="TEST", | ||
| def make_stdio_test_config(name: str, init_options: dict[str, Any]) -> ClientConfig: | ||
| """Start the fake language server in STDIO mode.""" |
Member
There was a problem hiding this comment.
Nitpicking but the function doesn't really start the server. It just creates a config for starting a server.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a test that checks that "tcp server mode" works. Server meaning that this plugin acts as the TCP server and the langserver connects as TCP client.