-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
all: Undo custom exit codes #6162
all: Undo custom exit codes #6162
Conversation
GNU testsuite comparison:
|
Hooray! It works. It also reveals yet another inconsistency in GNU ls: $ ls --invalid
ls: unrecognized option '--invalid'
Try 'ls --help' for more information.
[$? = 2]
$ ls --classify=invalid
ls: invalid argument 'invalid' for '--classify'
Valid arguments are:
- 'always', 'yes', 'force'
- 'never', 'no', 'none'
- 'auto', 'tty', 'if-tty'
Try 'ls --help' for more information.
[$? = 1] |
9360ec6
to
503da34
Compare
Changes since last push:
|
GNU testsuite comparison:
|
503da34
to
e4903b7
Compare
Changes since last push:
Locally this passes |
GNU testsuite comparison:
|
e4903b7
to
32962b8
Compare
Changes since last push:
|
@@ -89,6 +89,16 @@ pub fn uu_app() -> Command { | |||
.override_usage(format_usage(USAGE)) | |||
.after_help(AFTER_HELP) | |||
.infer_long_args(true) | |||
// Since we use value-specific help texts for "--output-error", clap's "short help" and "long help" differ. |
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.
I'm not sure about this change. It's more compatible, sure, but is it better? I want to limit all sorts of edge cases in the code and might prefer disabling that test here. Or can we force the bit for --output-error
to always be shown?
Note that this will be fixed by uutils-args too by the way, so maybe it just doesn't matter much either way.
(Don't consider this a blocking review)
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.
I don't quite understand? All that this piece of code does is force the bit for --output-error
to always be shown, just like you suggest.
Fancy new flake discovered in Windows CI:
I guess we're (I am) making too many calls and we're getting rate-limited? |
GNU testsuite comparison:
|
32962b8
to
286dc1c
Compare
Changes since last push: Nothing, just a rebase to show that it still works. @tertsdiepraam Since the code already does what you suggest (I probably misunderstand you, please clarify), I didn't change anything :) |
I wanted it by including the same text in the short help (if possible), instead of forcing the long help for the whole util. Again though, not important enough to block on. Edit: whoops that message was borderline incomprehensible. Fixed it now. |
GNU testsuite comparison:
|
286dc1c
to
120f031
Compare
Changes since last push:
Is there anything else needed? |
GNU testsuite comparison:
|
well done |
This PR:
tee -h
andtee --help
misc/usage_vs_getopt
All in all, we should now pass
misc/usage_vs_getopt
. Let's see whether CI agrees!Ping @tertsdiepraam, you touched on this topic a long time ago. What do you think? :)