Re: [PATCH 2/2] ls-remote: use PARSE_OPT_NO_INTERNAL_HELP

[Jonathan Nieder]

René Scharfe wrote:

> Since ba5f28bf79 (ls-remote: use parse-options api) git ls-remote -h
> without any other options has shown the short help text of the command
> instead of acting as the short equivalent of --heads.  Fix this
> regression by turning off internal handling of -h for repository setup,
> and option parsing, as well as the corresponding test in t0012.
>
> Reported-by: Thomas Rikl 
> Analyzed-by: Martin Ågren 
> Signed-off-by: Rene Scharfe 
> ---
>  builtin/ls-remote.c  | 1 +
>  git.c| 2 +-
>  t/t0012-help.sh  | 1 +
>  t/t5512-ls-remote.sh | 6 ++
>  4 files changed, 9 insertions(+), 1 deletion(-)

Reviewed-by: Jonathan Nieder 

[...]
> --- a/git.c
> +++ b/git.c
> @@ -421,7 +421,7 @@ static struct cmd_struct commands[] = {
>   { "interpret-trailers", cmd_interpret_trailers, RUN_SETUP_GENTLY },
>   { "log", cmd_log, RUN_SETUP },
>   { "ls-files", cmd_ls_files, RUN_SETUP },
> - { "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY },
> + { "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY | NO_INTERNAL_HELP },

There's only one other command that uses PARSE_OPT_NO_INTERNAL_HELP, and
that's "git archive".  Does it need the same treatment?

More generally, is there a straightforward way for parse-options or
some compile-time analysis to catch if we forget to add
NO_INTERNAL_HELP to a command in the commands[] table?

Thanks,
Jonathan

Re: [PATCH 2/2] ls-remote: use PARSE_OPT_NO_INTERNAL_HELP

[Martin Ågren]

On 17 October 2017 at 17:39, René Scharfe  wrote:
> Since ba5f28bf79 (ls-remote: use parse-options api) git ls-remote -h
> without any other options has shown the short help text of the command
> instead of acting as the short equivalent of --heads.  Fix this
> regression by turning off internal handling of -h for repository setup,
> and option parsing, as well as the corresponding test in t0012.
>
> Reported-by: Thomas Rikl 
> Analyzed-by: Martin Ågren 
> Signed-off-by: Rene Scharfe 
> ---
>  builtin/ls-remote.c  | 1 +
>  git.c| 2 +-
>  t/t0012-help.sh  | 1 +
>  t/t5512-ls-remote.sh | 6 ++
>  4 files changed, 9 insertions(+), 1 deletion(-)

Documentation/gitcli.txt might need updating. It says that "[c]ommands
which have the enhanced option parser activated all understand ... -h".
Of course, it already was in an incorrect state, since it pretends like
no-one uses PARSE_OPT_NO_INTERNAL_HELP. Probably better to leave that
until it's clear in which direction "git ls-remote -h" should go.

[PATCH 2/2] ls-remote: use PARSE_OPT_NO_INTERNAL_HELP

[René Scharfe]

Since ba5f28bf79 (ls-remote: use parse-options api) git ls-remote -h
without any other options has shown the short help text of the command
instead of acting as the short equivalent of --heads.  Fix this
regression by turning off internal handling of -h for repository setup,
and option parsing, as well as the corresponding test in t0012.

Reported-by: Thomas Rikl 
Analyzed-by: Martin Ågren 
Signed-off-by: Rene Scharfe 
---
 builtin/ls-remote.c  | 1 +
 git.c| 2 +-
 t/t0012-help.sh  | 1 +
 t/t5512-ls-remote.sh | 6 ++
 4 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index c4be98ab9e..5f27d252b4 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -68,6 +68,7 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
};
 
argc = parse_options(argc, argv, prefix, options, ls_remote_usage,
+PARSE_OPT_NO_INTERNAL_HELP |
 PARSE_OPT_STOP_AT_NON_OPTION);
dest = argv[0];
 
diff --git a/git.c b/git.c
index 9d1b8c4716..056a123506 100644
--- a/git.c
+++ b/git.c
@@ -421,7 +421,7 @@ static struct cmd_struct commands[] = {
{ "interpret-trailers", cmd_interpret_trailers, RUN_SETUP_GENTLY },
{ "log", cmd_log, RUN_SETUP },
{ "ls-files", cmd_ls_files, RUN_SETUP },
-   { "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY },
+   { "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY | NO_INTERNAL_HELP },
{ "ls-tree", cmd_ls_tree, RUN_SETUP },
{ "mailinfo", cmd_mailinfo, RUN_SETUP_GENTLY },
{ "mailsplit", cmd_mailsplit },
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index 74eeead168..05d8cf9b49 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -54,6 +54,7 @@ test_expect_success 'generate builtin list' '
 '
 
 cat >no_internal_help long &&
+   test_cmp long short
+'
+
 test_done
-- 
2.14.2