From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com>,
"Matěj Cepl" <mcepl@cepl.eu>,
"Jonas Konrad" <jonas.konrad@uni-muenster.de>,
git@vger.kernel.org
Subject: Re: Git branch outputs usage message on stderr
Date: Wed, 15 Jan 2025 13:56:31 -0800 [thread overview]
Message-ID: <xmqq5xmfyc4w.fsf@gitster.g> (raw)
In-Reply-To: <20250115212952.GA96537@coredump.intra.peff.net> (Jeff King's message of "Wed, 15 Jan 2025 16:29:52 -0500")
Jeff King <peff@peff.net> writes:
> I certainly see an argument for exit(0), but whatever we do should be
> consistent with how parse-options handles it (since whether we use this
> or leave it to parse-options is purely an implementation detail that the
> user should not need to be aware of).
>
> And it uses code 129, even for "-h". I don't see any explicit rationale
> for that in the history; I think it goes back to the beginning of
> parse-options. It happens via the PARSE_OPT_HELP flag, but curiously we
> also trigger that for ambiguous options (which should exit with error).
> That might be a bug-in-waiting if we start handling PARSE_OPT_HELP
> differently.
Here is what I have as v2; there will be patches that touch
builtin/*.c in between and I expect that the last patch to conclude
the series will end with an update to parse-options.c (to exit with
0 when asked to give a help) and t0012 (to stop expecting 129).
--- >8 ---
Subject: [PATCH v2] parse-options: add show_usage_help_and_exit_if_asked()
Many commands call usage_with_options() when they are asked to give
the help message, but it incorrectly sends the help text to the
standard error stream. When the user asked for it with "git cmd -h",
the help message is the primary output from the command, hence we
should send it to the standard output stream.
Introduce a helper function that captures the common pattern
if (argc == 2 && !strcmp(argv[1], "-h"))
usage_with_options(usage, options);
and replaces it with
show_usage_help_and_exit_if_asked(argc, argv, usage, options);
to help correct code paths (there are 40 or so of them).
Note that this helper function still exits with status 129, and
t0012 insists on it. After converting all the mistaken callers of
usage_with_options() to call this new helper, we may want to address
it---the end user is asking us to give the help text, and we are
doing exactly as asked, so there is no reason to exit with non-zero
status.
Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
parse-options.c | 10 ++++++++++
parse-options.h | 4 ++++
2 files changed, 14 insertions(+)
diff --git a/parse-options.c b/parse-options.c
index 33bfba0ed4..8a8b934e67 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -1282,6 +1282,16 @@ void NORETURN usage_with_options(const char * const *usagestr,
exit(129);
}
+void show_usage_help_and_exit_if_asked(int ac, const char **av,
+ const char * const *usagestr,
+ const struct option *opts)
+{
+ if (ac == 2 && !strcmp(av[1], "-h")) {
+ usage_with_options_internal(NULL, usagestr, opts, 0, 0);
+ exit(129);
+ }
+}
+
void NORETURN usage_msg_opt(const char *msg,
const char * const *usagestr,
const struct option *options)
diff --git a/parse-options.h b/parse-options.h
index d01361ca97..6af4ee148a 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -402,6 +402,10 @@ int parse_options(int argc, const char **argv, const char *prefix,
NORETURN void usage_with_options(const char * const *usagestr,
const struct option *options);
+void show_usage_help_and_exit_if_asked(int ac, const char **av,
+ const char * const *usage,
+ const struct option *options);
+
NORETURN void usage_msg_opt(const char *msg,
const char * const *usagestr,
const struct option *options);
--
2.48.1-187-gd93ffc6ef3
next prev parent reply other threads:[~2025-01-15 21:56 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-15 11:21 Git branch outputs usage message on stderr Jonas Konrad
2025-01-15 11:36 ` Matěj Cepl
2025-01-15 14:47 ` Jonas Konrad
2025-01-15 15:28 ` Junio C Hamano
2025-01-15 16:55 ` Kristoffer Haugsbakk
2025-01-15 17:14 ` Jeff King
2025-01-15 17:49 ` Junio C Hamano
2025-01-15 17:56 ` Junio C Hamano
2025-01-15 18:24 ` Jeff King
2025-01-15 21:16 ` Junio C Hamano
2025-01-15 21:29 ` Jeff King
2025-01-15 21:56 ` Junio C Hamano [this message]
2025-01-15 22:27 ` Jeff King
2025-01-15 23:32 ` Junio C Hamano
2025-01-16 1:21 ` Junio C Hamano
2025-01-16 10:24 ` Jeff King
2025-01-15 22:11 ` Junio C Hamano
2025-01-15 22:28 ` Jeff King
2025-01-15 23:35 ` Junio C Hamano
2025-01-15 18:29 ` Junio C Hamano
2025-01-15 18:33 ` Kristoffer Haugsbakk
2025-01-15 21:13 ` Junio C Hamano
2025-01-15 17:14 ` Jonas Konrad
2025-01-15 17:53 ` Kristoffer Haugsbakk
2025-01-15 17:19 ` Junio C Hamano
2025-01-15 17:39 ` Kristoffer Haugsbakk
2025-01-15 17:47 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq5xmfyc4w.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=jonas.konrad@uni-muenster.de \
--cc=kristofferhaugsbakk@fastmail.com \
--cc=mcepl@cepl.eu \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).