threads / discuss / 61562

Commands using -h as an option don't work consistently

Subject: Commands using -h as an option don't work consistently

## tl;dr

5 messages between May 29, 2024 and Jun 3, 2024.

replies: 4people: 2as markdown or json

Kevin Day· May 29, 2024, 22:03 UTC · lore
Because of a bug in parse-options.c, any command that has a '-h' option will sometimes display the usage page instead of executing. For example, ls-remote has two options:
    -t, --[no-]tags       limit to tags
    -h, --[no-]heads      limit to heads

git ls-remote --heads #works git ls-remote --tags #works git ls-remote -t #works git ls-remote -t -h #works git ls-remote -h #shows the help page

This is because of these lines in parse-options.c:

/* lone -h asks for help */ if (internal_help && ctx->total == 1 && !strcmp(arg + 1, "h")) goto show_usage;

This is being executed before it looks to see if there actually is a -h option. So if a program has a -h option, and that's the ONLY parameter you pass, the usage page gets displayed incorrectly. This appears to affect ls-remote, show-ref (it's not documented, but show-ref accepts -h as an alias for --heads) and grep.
I fixed this by moving the lone -h check lower down, which fixed everything. Except now lots and lots of tests are failing because many of them assume you can always pass -h to get the usage page, and now you can't for some commands. I don't think this actually breaks grep because you need to pass at least one more option other than -h to use it, but tests for it are still failing after fixing this bug because it's now showing the man page instead of the expected short usage page because it's erroring out at a different place.
The specific tests that are failing are t1502-rev-parse-parseopt.sh, t0012-help.sh and t0450-txt-doc-vs-help.sh all of which are trying to use -h on commands that have repurposed it.
The options I see:
1) Fix -h handling and add ignores and fixes where possible to the failing tests and try to not use -h as an option for anything new.
2) Change -h to -H or something, but this breaks backwards compatibility
3) Fix it so that -h works if a command uses it, and additionally make a new global option -? or --usage or something that always shows the usage page and change tests to use that, while leaving -h sometimes showing usage and sometimes executing the option to preserve as much backward compatibility as possible.
I'm happy to do the work and submit it, but this is looking more like a policy decision than just a bug now.
-- Kevin
Junio C Hamano· May 29, 2024, 22:22 UTC · re: Kevin Day · lore

Re: Commands using -h as an option don't work consistently

Kevin Day <toasty@dragondata.com> writes:
Show 18 quoted lines
> Because of a bug in parse-options.c, any command that has a '-h' option will sometimes display the usage page instead of executing. For example, ls-remote has two options:
>
>     -t, --[no-]tags       limit to tags
>     -h, --[no-]heads      limit to heads
>
> git ls-remote --heads  #works
> git ls-remote --tags  #works
> git ls-remote -t  #works
> git ls-remote -t -h  #works
> git ls-remote -h  #shows the help page
>
> This is because of these lines in parse-options.c:
>
> /* lone -h asks for help */
> if (internal_help && ctx->total == 1 && !strcmp(arg + 1, "h"))
> goto show_usage;
>
> This is being executed before it looks to see if there actually is a -h option.

This is very much deliberate design. I think in these cases we make an unambiguous longhand (e.g. "--heads" we see above) available, or you can explicitly say the remote, i.e. "git ls-remote -h origin", and that has been the officially accepted "solution".

Let's see what improvements you bring to the table.
> The options I see:
>
> 1) Fix -h handling and add ignores and fixes where possible to the
> failing tests and try to not use -h as an option for anything new.

I do not quite understand the former half, but "try not to use -h for new things" is a very good idea and it is pretty much what we have been doing.

    Note: There however is a scenario where we cannot avoid it---if
    we were trying to match/mimick some established external
    command, we may have to use the same "-h" as they use (e.g.,
    "git grep -h -e ..." tries to mimick "grep -h -e ...").  Even in
    that case, because "git grep -h" alone would not make any sense
    without any pattern, the command behaves very sensibly, by the
    way.
> 2) Change -h to -H or something, but this breaks backwards compatibility
As you said it yourself, this is a non starter.
Show 5 quoted lines
> 3) Fix it so that -h works if a command uses it, and additionally
> make a new global option -? or --usage or something that always
> shows the usage page and change tests to use that, while leaving
> -h sometimes showing usage and sometimes executing the option to
> preserve as much backward compatibility as possible.

Reading this and then going back to the ls-remote examples you gave earlier, I do not think the current behaviour is all that bad, relative to what is being proposed, except for an explicit support for "-?".

We can certainly add support for "-?", but the behaviour when "-?" is not used (and when "-h" is used) would not have to change from the current behaviour, and that would still be serviceable, I presume?

We actually do not have to *add* support for "-?", as it comes with parse-options for free ;-)

    $ git ls-remote -\?
    error: unknown switch '?'
    usage: git ls-remote ...
	... the same "ls-remote -h" output given here ...
So, I dunno.
Kevin Day· May 29, 2024, 22:40 UTC · re: Junio C Hamano · lore

Re: Commands using -h as an option don't work consistently

Show 6 quoted lines
> On May 29, 2024, at 5:22 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 
> This is very much deliberate design.  I think in these cases we make
> an unambiguous longhand (e.g. "--heads" we see above) available, or
> you can explicitly say the remote, i.e. "git ls-remote -h origin",
> and that has been the officially accepted "solution".
Yeah, and that totally makes sense. This bit me because I was programmatically generating git commands that would sometimes call ls-remote with no options, sometimes asking only for heads, sometimes only for tags.... and was baffled for longer than I want to admit why it wasn't working consistently. I thought the usage was being shown because I was doing something wrong. My first thought was it was showing me the help message because if you ask for heads you must also use some other option along with it. It didn't occur to me that -h and --heads would behave differently.
Show 10 quoted lines
> Let's see what improvements you bring to the table.
> 
>> The options I see:
>> 
>> 1) Fix -h handling and add ignores and fixes where possible to the
>> failing tests and try to not use -h as an option for anything new.
> 
> I do not quite understand the former half, but "try not to use -h
> for new things" is a very good idea and it is pretty much what we
> have been doing.
Sorry, let me rephrase that.  1) Apply the fix I was proposing to make it so that the command's -h usage has precedence over showing the usage. Any tests that break because of that which don't have a plausible fix mark as "# TODO known breakage" like several other commands have because they aren't generating usage output correctly for one reason or another.
Show 5 quoted lines
> We can certainly add support for "-?", but the behaviour when "-?"
> is not used (and when "-h" is used) would not have to change from
> the current behaviour, and that would still be serviceable, I
> presume?
> 
That was proposing making -? unconditionally show usage, if a command uses -h give that priority over showing usage even if it's the only option, but leaving -h to still show usage everywhere else, and changing all the tests to use -? Instead of -h so the tests can unambiguously request the usage.
I think that covers every case then. If a command advertises -h it always works, it falls back to showing usage if there is no -h, and tests can use -? which will always show usage and we can even make parse-options reject anyone's attempt to use -? as an option in the future so this doesn't come up again in 10 years.
Show 7 quoted lines
> We actually do not have to *add* support for "-?", as it comes with
> parse-options for free ;-)
> 
>    $ git ls-remote -\?
>    error: unknown switch '?'
>    usage: git ls-remote ...
> ... the same "ls-remote -h" output given here ...
That *almost* works, except some tests would have to be modified because they don't like that error line in there, but that is super clever! It would be pretty trivial to make parse-options deliberately spit the usage out without an error though in the -? case.
Junio C Hamano· May 30, 2024, 03:23 UTC · re: Kevin Day · lore

Re: Commands using -h as an option don't work consistently

Kevin Day <toasty@dragondata.com> writes:
Show 13 quoted lines
>>> 1) Fix -h handling and add ignores and fixes where possible to the
>>> failing tests and try to not use -h as an option for anything new.
>> 
>> I do not quite understand the former half, but "try not to use -h
>> for new things" is a very good idea and it is pretty much what we
>> have been doing.
>
> Sorry, let me rephrase that.  1) Apply the fix I was proposing to make
> it so that the command's -h usage has precedence over showing the
> usage. Any tests that break because of that which don't have a
> plausible fix mark as "# TODO known breakage" like several other
> commands have because they aren't generating usage output correctly
> for one reason or another.
Ah, then no.

I am afraid that it is a non starter. "git ls-remote -h" would then stop breaking existing users' long established expectation, i.e., "for any git subcommand 'cmd', you can say 'git cmd -h' to get the short help". We need to design a good transition plan to wean existing users off of that expectation.

For example, we _could_ introduce "-?" to parse-options _now_, and mark the use of "-h" as deprecated. When the special casing of "-h" parse-options has triggers, the user will be reminded that the "-h" support will have a limited shelf life, and they are urged to retrain their fingers and update their scripts to use "-?", possibly with "advice.shorthelpdeprecationnotice" configuration knob that helps ease the transition. We would update the t/ scripts to use "-?" at the same time, but would probably need to keep tests that use "-h" to ensure that the warning triggers.

We will keep things that way for a while, and once we get the users' expectation updated so that they think 'git cmd -?' (not 'git cmd -h') is _the_ way to get the short help for any git subcommand.

At a major version boundary, e.g., Git 3.0, we could remove the special casing of '-h' from parse-options, and "git ls-remote -h" will start working the same way as "git ls-remote -h origin" all of a sudden, but nobody remembers "git ls-remote -h" used to be the way to get short help for the command any more by that time, so nobody will get hurt by such a flipping of the behaviour.

Junio C Hamano· Jun 3, 2024, 18:33 UTC · re: Junio C Hamano · lore

Re: Commands using -h as an option don't work consistently

Junio C Hamano <gitster@pobox.com> writes:
Show 7 quoted lines
> Ah, then no.
>
> I am afraid that it is a non starter.  "git ls-remote -h" would then
> start breaking existing users' long established expectation, i.e.,
> "for any git subcommand 'cmd', you can say 'git cmd -h' to get the
> short help".  We need to design a good transition plan to wean
> existing users off of that expectation.

Another thing I forgot to mention. For "ls-remote" and "show-ref", there is a much nicer transition path we can take. Back in Linus's days, we called branches "heads" (and we used $GIT_DIR/refs/heads/ to store them as individual files), but if we were starting Git from scratch today, there is no reason to. Call branches branches, not heads, which means we do not need to use "-h" to mean "I want to see only the branches and no other refs" at all. You say "--branches" instead.

Which leads to an obvious transition path, which is
 * Introduce "--branches" that sits next to "--tags" to "ls-remote"
   and "show-ref".  If the command has "-t" as a short-hand for
   "--tags", then add "-b" as a short-hand for "--branches".
   Announce that "--heads" (and "-h" if supported) is deprecated for
   these commands, and when "--heads" (and "-h" if supported) is
   used, give a warning to train finger/muscle memory early.
 * We will keep things that way for a while, and then remove the
   "--heads" (and "-h" if supported) from these two commands.

The approach cannot be used to transition "git grep -h" for obvious reasons, but then it makes no sense to give "-h" alone to tell the command "I do not want to see filenames", so that is not a problem.

I checked output from 
    $ git grep -A2 '^-h' Documentation/

and it seems that ls-remote is the only one that matters, so we do not even have to do the "-?" transition at all.

Thanks.

← back to recent threads