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 10:29:59 -0800 [thread overview]
Message-ID: <xmqqikqgx74o.fsf@gitster.g> (raw)
In-Reply-To: <xmqqmsfsx8oo.fsf@gitster.g> (Junio C. Hamano's message of "Wed, 15 Jan 2025 09:56:23 -0800")
Junio C Hamano <gitster@pobox.com> writes:
> Jeff King <peff@peff.net> writes:
>
>> use that everywhere. Possibly it could even do the argc/argv check, too,
>> since every call site is going to be doing that itself.
>
> It would look something like this; I am not sure if I like the "this
> may show help and exit if the user requested, but otherwise it is a
> no-op" semantics, though.
After thinking about it a bit more, I am starting to like it, not
because it reduces a few more lines from the calling site, but
because it makes it almost impossible to use for a careless and
incorrect conversion from usage_with_options(). Any existing
callsite of usage_with_options() MUST be inspected to make sure it
is responding to an end-user request to give "-h"(elp) text before
being replaced with the call to a new helper, and if I made
show_usage_help() without argc/argv, a careless developer can easily
and blindly do string replacement to send a ton of patch that
reviewers need to waste time to do the inspection for them.
But with your "argc and argv checking included" approach, it is
harder to do the blind replacement.
--- >8 ---
From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 15 Jan 2025 09:56:23 -0800
Subject: [PATCH] parse-options: add show_usage_help()
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(argc, argv, usage, options);
to help correct code paths (there are 40 or so of them).
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..4b00065692 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(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(0);
+ }
+}
+
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..f93f13434c 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(int, const char **,
+ const char * const *usagestr,
+ 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 18:30 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
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 [this message]
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=xmqqikqgx74o.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).