Jonathan Nieder <jrnieder@xxxxxxxxx> 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 from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html