Skip to content
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

Make getopt error message style more consistent #128129

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

9cel
Copy link

@9cel 9cel commented Dec 20, 2024

Fixes this minor inconsistency:

➜  cpython git:(consistent-getopt-errors) python3 -a
Unknown option: -a

vs.

➜  cpython git:(consistent-getopt-errors) python3 --a
unknown option --a

Copy link

cpython-cla-bot bot commented Dec 20, 2024

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app
Copy link

bedevere-app bot commented Dec 20, 2024

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@StanFromIreland
Copy link
Contributor

image

Consistent with the launcher.

@bedevere-app
Copy link

bedevere-app bot commented Dec 20, 2024

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@picnixz
Copy link
Contributor

picnixz commented Dec 21, 2024

I can agree with making the error message consistent but I wonder if there's any chance we break CI parsing the output of the module in case of an error. While getopt is currently marked as deprecated, there are talks about considering it non-deprecated #126180 so the module could become active again. In addition, for users coming from C, getopt is usually easier to use since it's almost identitcal to getopt.h, so the module might be still in use in the wild.

If we accept changing the error message, we should probably make a NEWS entry (and perhaps a What's New entry) just so that users are notified, even if there's only one user (and in this case, this requires an issue). cc @serhiy-storchaka

@serhiy-storchaka
Copy link
Member

This is not related to the getopt module. This is emitted from a local copy of getopt.c. Error messages are different from error messages in GNU getopt.c and getopt.py:

$ LC_ALL=C ls -z
ls: invalid option -- 'z'
Try 'ls --help' for more information.
$ LC_ALL=C ls --y
ls: unrecognized option '--y'
Try 'ls --help' for more information.
$ ./python -m quopri -z
option -z not recognized
usage: quopri [-t | -d] [file] ...
-t: quote tabs
-d: decode; default encode
$ ./python -m quopri --z
option --z not recognized
usage: quopri [-t | -d] [file] ...
-t: quote tabs
-d: decode; default encode

So there is no single correct variant.

But error messages starting with a lower-case were introduced not so long ago in 42aa93b (bpo-31650). Error messages starting with an upper-case were here from the beginning. So we can assume that it was an error in 42aa93b.

cc @benjaminp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants