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....@web.de>
> ---
>  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 <jrnie...@gmail.com>

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

Reply via email to