Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.3k
gh-91818: Add executable detection for CLIs#91819
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
The help message for things like json.tool and ast should match theexecutable that was run, for example python.exe on windows and python3 on*nix OSes. This will facilitate users being able to copy and paste fromthe help text displayed by argparse, and clarify which version of pythonis being run on some OSes.The previous display for json.tool of```$ python3 -m json.tool -husage: python -m json.tool [-h] [--sort-keys] [--json-lines] [infile] [outfile]```becomes```$ python3 -m json.tool -husage: python3 -m json.tool [-h] [--sort-keys] [--json-lines] [infile] [outfile]```Pip has a something like PrettyExecutableName in a funciton calledget_prog found inhttps://github.com/pypa/pip/blob/main/src/pip/_internal/utils/misc.py.The pip version has a lot of defensive coding practices around catchingvarious exception related to sys.argv. But since this version does notparse argv, those exception can be dropped.As a side effect the help usage of venv has been updated to use thepython -m <module> syntax as it was the only command not using thatstyle, 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. |
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.
Please see my suggestions.
| 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.
| self.exit(2,_('%(prog)s: error: %(message)s\n')%args) | ||
| classPrettyExecutableName(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:
defadjust_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]ifexe!='python':returncmdifexe=='python':sysexe=sys.executableifnotsysexe:returncmdexe=os.path.basename(sysexe)ifos.name=='nt':exe=os.path.splitext(exe)[0]returnexeiflen(parts)==1elsef'{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 interactivelyhere.
| 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 thepython -m in the function, as you've done in your implementation.
bedevere-bot commentedApr 26, 2022
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 |
See also#66436. This is more complex issue, because it should work for running files, modules, directories, ZIP files. |
Thank you for your PR,@scrummyin, but this is now the default behavior of |
Uh oh!
There was an error while loading.Please reload this page.
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.