Re: Git branch outputs usage message on stderr

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Jeff King <peff@xxxxxxxx> 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@xxxxxxxxx>
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@xxxxxxxx>
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
 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





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux