New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gh-91818: Add executable detection for clis #91819
base: main
Are you sure you want to change the base?
gh-91818: Add executable detection for clis #91819
Conversation
The help message for things like json.tool and ast should match the executable that was run, for example python.exe on windows and python3 on *nix OSes. This will facilitate users being able to copy and paste from the help text displayed by argparse, and clarify which version of python is being run on some OSes. The previous display for json.tool of ``` $ python3 -m json.tool -h usage: python -m json.tool [-h] [--sort-keys] [--json-lines] [infile] [outfile] ``` becomes ``` $ python3 -m json.tool -h usage: python3 -m json.tool [-h] [--sort-keys] [--json-lines] [infile] [outfile] ``` Pip has a something like PrettyExecutableName in a funciton called get_prog found in https://github.com/pypa/pip/blob/main/src/pip/_internal/utils/misc.py. The pip version has a lot of defensive coding practices around catching various exception related to sys.argv. But since this version does not parse argv, those exception can be dropped. As a side effect the help usage of venv has been updated to use the python -m <module> syntax as it was the only command not using that style, and docs show that style of invocation.
I'm happy to update the news if people think this is a good idea. Also as a side note, the Ubuntu tests failed on me before for no reason, and that is odd since this was written and originally tested on Ubuntu. So any other OS would not have been a surprise, but the only one I know the tests run on failing is. |
As a required test is still failing, you could try rebasing so that the latest changes in main are reflected in this PR. |
@@ -56,6 +56,9 @@ | |||
ArgumentDefaultsHelpFormatter adds information about argument defaults | |||
to the help. | |||
- PrettyExecutableName -- A class to create strings for use as the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having a class seems overkill, see below.
@@ -2597,3 +2601,24 @@ def error(self, message): | |||
self.print_usage(_sys.stderr) | |||
args = {'prog': self.prog, 'message': message} | |||
self.exit(2, _('%(prog)s: error: %(message)s\n') % args) | |||
|
|||
|
|||
class PrettyExecutableName(object): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why a class when a function will do? For example, this function:
def adjust_command(cmd):
"""
Adjust a command line which uses 'python' to substitute the name
of the actual running interpreter, e.g. 'python3' or 'python3.10'.
"""
parts = cmd.split(' ', 1)
exe = parts[0]
if exe != 'python':
return cmd
if exe == 'python':
sysexe = sys.executable
if not sysexe:
return cmd
exe = os.path.basename(sysexe)
if os.name == 'nt':
exe = os.path.splitext(exe)[0]
return exe if len(parts) == 1 else f'{exe} {parts[1]}'
would appear to do the trick and not be restricted to e.g. -m
commands. You should be able to try it interactively here.
@@ -1699,7 +1699,8 @@ def unparse(ast_obj): | |||
def main(): | |||
import argparse | |||
|
|||
parser = argparse.ArgumentParser(prog='python -m ast') | |||
program_name = argparse.PrettyExecutableName("ast") | |||
parser = argparse.ArgumentParser(prog=f"{program_name}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With my suggestion this would become simpler. e.g.
parser = argparse.ArgumentParser(prog=argparse.adjust_command('python -m ast'))
which is also clearer than hiding the python -m
in the function, as you've done in your implementation.
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
The help message for things like json.tool and ast should match the
executable that was run, for example python.exe on windows and python3 on
*nix OSes. This will facilitate users being able to copy and paste from
the help text displayed by argparse, and clarify which version of python
is being run on some OSes.
The previous display for json.tool of
becomes
Pip has a something like PrettyExecutableName in a funciton called
get_prog found in
https://github.com/pypa/pip/blob/main/src/pip/_internal/utils/misc.py.
The pip version has a lot of defensive coding practices around catching
various exception related to sys.argv. But since this version does not
parse argv, those exception can be dropped.
As a side effect the help usage of venv has been updated to use the
python -m syntax as it was the only command not using that
style, and docs show that style of invocation.