Hi,
René Scharfe wrote:
> Builtin commands have skipped repo setup when called with just a single
> option -h and thus shown their short help text even outside of
> repositories since 99caeed05d3 (Let 'git <command> -h' show usage
> without a git dir).
>
> Add the flag NO_INTERNAL_HELP for builtins to opt out of this special
> treatment and provide a list of commands that are exempt from the
> corresponding test in t0012. That allows builtins to handle -h
> themselves.
>
> Signed-off-by: Rene Scharfe <l.s.r@xxxxxx>
> ---
> git.c | 6 +++++-
> t/t0012-help.sh | 9 ++++++++-
> 2 files changed, 13 insertions(+), 2 deletions(-)
Makes sense.
[...]
> +++ b/git.c
[...]
> @@ -312,7 +313,10 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
> const char *prefix;
>
> prefix = NULL;
> - help = argc == 2 && !strcmp(argv[1], "-h");
> + if (p->option & NO_INTERNAL_HELP)
> + help = 0;
> + else
> + help = argc == 2 && !strcmp(argv[1], "-h");
optional: this could be part of the same expression:
help = !(p->option & NO_INTERNAL_HELP) && argc == 2 && ...;
[...]
> --- a/t/t0012-help.sh
> +++ b/t/t0012-help.sh
> @@ -53,12 +53,19 @@ test_expect_success 'generate builtin list' '
> git --list-builtins >builtins
> '
>
> +cat >no_internal_help <<EOF
> +EOF
> +
> +test_expect_success 'generate list of builtins with internal help' '
> + grep -w -v -f no_internal_help <builtins >builtins_internal_help
> +'
Hm, I don't see any instances of "grep -f" in the testsuite. Are
there portability pitfalls in it I don't know about? It's in POSIX,
so it looks pretty safe.
I would have been tempted to use comm, which is already used in
t6500-gc.sh:
comm -1 -3 no_internal_help builtins >builtins_internal_help
Other nits:
- preparatory 'cat' commands like the above tend to go inside
test_expect_success these days
- test that set up for later tests get marked as 'setup' or 'set up'
With whatever subset of the changes below looks good squashed in,
Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
Thanks.
diff --git i/git.c w/git.c
index 9d1b8c4716..e4b340df7d 100644
--- i/git.c
+++ w/git.c
@@ -313,10 +313,8 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
const char *prefix;
prefix = NULL;
- if (p->option & NO_INTERNAL_HELP)
- help = 0;
- else
- help = argc == 2 && !strcmp(argv[1], "-h");
+ help = !(p->option & NO_INTERNAL_HELP) &&
+ argc == 2 && !strcmp(argv[1], "-h");
if (!help) {
if (p->option & RUN_SETUP)
prefix = setup_git_directory();
diff --git i/t/t0012-help.sh w/t/t0012-help.sh
index c5a748837c..73fdfd99ab 100755
--- i/t/t0012-help.sh
+++ w/t/t0012-help.sh
@@ -49,15 +49,11 @@ test_expect_success "--help does not work for guides" "
test_i18ncmp expect actual
"
-test_expect_success 'generate builtin list' '
- git --list-builtins >builtins
-'
-
-cat >no_internal_help <<EOF
-EOF
-
-test_expect_success 'generate list of builtins with internal help' '
- grep -w -v -f no_internal_help <builtins >builtins_internal_help
+test_expect_success 'set up list of builtins with internal help' '
+ cat >no_internal_help <<-\EOF &&
+ EOF
+ git --list-builtins >builtins &&
+ comm -1 -3 no_internal_help builtins >builtins_internal_help
'
while read builtin
