Jonathan Nieder <jrnie...@gmail.com> writes:

> diff --git a/git.c b/git.c
> index bd2c5fe..bfa9518 100644
> --- a/git.c
> +++ b/git.c
> @@ -220,6 +220,11 @@ const char git_version_string[] = GIT_VERSION;
>   * RUN_SETUP for reading from the configuration file.
>   */
>  #define NEED_WORK_TREE       (1<<2)
> +/*
> + * Let RUN_SETUP, USE_PAGER, and NEED_WORK_TREE take effect even if
> + * passed the -h option.
> + */
> +#define H_IS_NOT_HELP        (1<<3)

Yuck.  Let's think of a way to avoid this ugliness.

> @@ -278,7 +287,8 @@ static void handle_internal_command(int argc, const char 
> **argv)
>               { "annotate", cmd_annotate, RUN_SETUP },
>               { "apply", cmd_apply },
>               { "archive", cmd_archive },
> -             { "bisect--helper", cmd_bisect__helper, RUN_SETUP | 
> NEED_WORK_TREE },
> +             { "bisect--helper", cmd_bisect__helper,
> +                     RUN_SETUP | NEED_WORK_TREE },

Besides, this hunk is totally unwarranted.

Here are the relevant parts (some of your H_IS_NOT_HELP are not visible
because you needlessly wrapped the lines):

> +             { "cherry", cmd_cherry, RUN_SETUP | H_IS_NOT_HELP },
> +             { "commit-tree", cmd_commit_tree, RUN_SETUP | H_IS_NOT_HELP },
> +             { "fetch--tool", cmd_fetch__tool, RUN_SETUP | H_IS_NOT_HELP },
> +             { "grep", cmd_grep, RUN_SETUP | USE_PAGER | H_IS_NOT_HELP },
> +             { "merge-ours", cmd_merge_ours, RUN_SETUP | H_IS_NOT_HELP },
> +             { "merge-recursive", cmd_merge_recursive,
> +             { "merge-subtree", cmd_merge_recursive,
> +             { "show-ref", cmd_show_ref, RUN_SETUP | H_IS_NOT_HELP },

Except for "grep" and "show-ref", none of these have a valid -h option
that means something else.

Considering that this niggle is strictly about "git cmd -h", and not about
"git cmd --otheropt -h somearg", we can even say that "git grep -h" is
asking for help, and not "do not show filenames from match", as there is
no pattern specified.

So I think the right approach is something like how you handled http-push;
namely, check if the sole argument is "-h", and if so show help and exit.

        Clarification. the following description only talks about "cmd -h"
        without any other options and arguments.

Such a change cannot be breaking backward compatibility for...

 * "cherry -h" could be asking to compare histories that leads to our HEAD
   and a commit that can be named as "-h".  Strictly speaking, that may be
   a valid refname, but the user would have to say something like
   "tags/-h" to name such a pathological ref already, so I do not think it
   is such a big deal.

 * "commit-tree -h" is to make a root commit that records a tree-ish
   pointed by a tag whose name is "-h".  Same as above.

 * The first word to "fetch--tool" is a subcommand name, so "fetch--tool -h"
   is an error and there cannot be any existing callers.  Besides, is it
   still being used?

 * "grep -h" cannot be asking for suppressing filenames as there is no
   match pattern specified.

 * "merge-*" strategy backends take the merge base (or "--") as the first
   parameter; it cannot sanely be "-h". The callers are supposed to run
   rev-parse to make it 40-hexdigit and the command won't see a refname
   anyway.

That leaves "show-ref -h".  It shows all the refs/* and HEAD, as opposed
to "show-ref" that shows all the refs/* and not HEAD.

Does anybody use "show-ref -h"?  It was in Linus's original, and I suspect
it was done only because he thought "it might be handy", not because "the
command should not show the HEAD by default for such and such reasons".
So I think it actually is Ok if "show-ref -h" (but not "show-ref --head")
gave help and exit.



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Reply via email to