git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] show-index: the short help should say the command reads from its input

From
Patrick Steinhardt <ps@pks.im>
Date
Dec 27, 2024, 14:06 UTC
Message-ID
<Z260VVuIH_-0Ylis@pks.im>
In-Reply-To
<xmqqfrmidyhk.fsf@gitster.g>
On Fri, Dec 20, 2024 at 10:02:15AM -0800, Junio C Hamano wrote:
Show 15 quoted lines
> 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.
Show 17 quoted lines
>  * 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?

Show 15 quoted lines
>  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
Previous: Junio C HamanoNext: Junio C Hamano
Message 2 of 3 in “show-index: the short help should say the command reads from its input”
  1. show-index: the short help should say the command reads from its inputJunio C Hamano, Dec 20, 2024
  2. Patrick SteinhardtDec 27, 2024
  3. Junio C HamanoDec 27, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.