From: Patrick Steinhardt <ps@pks•im>
To: Junio C Hamano <gitster@pobox•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] show-index: the short help should say the command reads from its input
Date: Fri, 27 Dec 2024 15:06:13 +0100 [thread overview]
Message-ID: <Z260VVuIH_-0Ylis@pks.im> (raw)
In-Reply-To: <xmqqfrmidyhk.fsf@gitster.g>
On Fri, Dec 20, 2024 at 10:02:15AM -0800, Junio C Hamano wrote:
> The short help text given by "git show-index -h" says
>
> $ git show-index -h
> usage: git show-index [--object-format=<hash-algorithm>]
>
> --[no-]object-format <hash-algorithm>
> specify the hash algorithm to use
>
>
> The command takes a pack .idx file from its standard input. The
> user has to _know_ this, as there is no indication from this output.
>
> Give a hint that the data to work on is fed from its standard input.
>
> Signed-off-by: Junio C Hamano <gitster@pobox•com>
Makes sense.
> * I also found the option description somewhat funny in that
>
> (1) it makes it look like "--no-object-format sha256" is
> accepted, which is not a case, and
>
> (2) "git show-index --no-object-format" already is a curious
> thing to say; the command certainly needs to work in _some_
> format.
>
> But (2) is common to all the usual command line options to allow
> defeating another instance of the same option that is given
> positively previously on the command line (i.e. "git show-index
> --object-format=sha256 --no-object-format" should behave as if no
> object-format option was given), and (1) is shared by all the
> other options that allow such override. So I'll let it pass, but
> if we really wanted to improve it, the fix should go into how the
> parse-options subsystem works.
Can't we already fix this via OPT_NONEG? Or is your point rather that it
is awkward in general and choices like this should never have a negated
variant by default?
> builtin/show-index.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git c/builtin/show-index.c w/builtin/show-index.c
> index f164c01bbe..8678b741a4 100644
> --- c/builtin/show-index.c
> +++ w/builtin/show-index.c
> @@ -7,7 +7,7 @@
> #include "parse-options.h"
>
> static const char *const show_index_usage[] = {
> - "git show-index [--object-format=<hash-algorithm>]",
> + "git show-index [--object-format=<hash-algorithm>] < <pack-idx-file>",
> NULL
> };
I was wondering whether we have any other usage strings that show an
expected stdin like this, and indeed we do. The usage string in
"builtin/mailinfo.c" uses different syntax though without the angular
brackets, but "builtin/pack-objects.c" does use them. I think with the
angular brackets is more idiomatic in our codebase though, so the
addition looks good to me.
Patrick
next prev parent reply other threads:[~2024-12-27 14:06 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-20 18:02 [PATCH] show-index: the short help should say the command reads from its input Junio C Hamano
2024-12-27 14:06 ` Patrick Steinhardt [this message]
2024-12-27 15:07 ` 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=Z260VVuIH_-0Ylis@pks.im \
--to=ps@pks$(echo .)im \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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