Add --version CLI argument and return version in InitializeResult #44
Conversation
There was a problem hiding this comment.
Hey @nemethf, thanks for your contribution! Looks good to me, except for the linter issues.
pylsp/__main__.py
Outdated
| @@ -57,13 +58,22 @@ def add_arguments(parser): | |||
| help="Increase verbosity of log output, overrides log config file" | |||
| ) | |||
|
|
|||
| parser.add_argument( | |||
| '-V', '--version', action='store_true', | |||
There was a problem hiding this comment.
Is it common to use the -V option for showing the version? I don't think I've ever seen this used before. And -v is usually used for verbose logging so it seems a little too close to that. Is having --version not enough?
There was a problem hiding this comment.
I copied it from python, but I can remove the short option if needed
usage: python [option] ... [-c cmd | -m mod | file | -] [arg] ...
-v : verbose (trace import statements); also PYTHONVERBOSE=x
can be supplied multiple times to increase verbosity
-V : print the Python version number and exit (also --version)There was a problem hiding this comment.
Also, it looks like argparse have native support for "--version" handling. See:
There was a problem hiding this comment.
Hm. That would simplify the patch a bit.
There was a problem hiding this comment.
If python does it then I guess it's fine.
But could use that "native" way to add the argument.
There was a problem hiding this comment.
OK. I've pushed a new version. Thanks
|
I force-pushed a fix to the linter issue, but now github requires approval once again to run the tests. What's the right way to correct a mistake like this? (Unfortunately, it's too complicated for me to run the tests locally.) |
Unfortunately, for first time contributors all projects are required to approve running tests. |
The extension of InitializeResult is especially useful in tcp mode when users want to check whether the client communicates with pyls or pylsp (or some other python language servers).
Thanks