Bug: git ls-remote -h / --head results differ in output

Bug: git ls-remote -h / --head results differ in output

[Thomas Rikl]

Example:

tom1 ~/emacs/spacemacs/.emacs.d $ export LANG=en_US.utf8

tom1 ~/emacs/spacemacs/.emacs.d $ git ls-remote -h
usage: git ls-remote [--heads] [--tags] [--refs] [--upload-pack=<exec>]
                      [-q | --quiet] [--exit-code] [--get-url]
                      [--symref] [<repository> [<refs>...]]

     -q, --quiet           do not print remote URL
     --upload-pack <exec>  path of git-upload-pack on the remote host
     -t, --tags            limit to tags
     -h, --heads           limit to heads
     --refs                do not show peeled tags
     --get-url             take url.<base>.insteadOf into account
     --exit-code           exit with exit code 2 if no matching refs are 
found
     --symref              show underlying ref in addition to the object 
pointed by it

tom1 ~/emacs/spacemacs/.emacs.d $ git ls-remote --head
 From https://github.com/syl20bnr/spacemacs
07014deead544c51fa6a826e91fe2ef05bf04323 refs/heads/develop
8e1af145480d53e8d32cdff2c83291889903164b refs/heads/master
2450b7e276634ece07b6b7ec6ca6c287af86caf3 refs/heads/release-0.101
8dadfc1494544bb152e80c2a436e43bc3713b389 refs/heads/release-0.102
d993a021847cde2c42865bab6afa8adbb2edda0b refs/heads/release-0.103
44d4525543b1f2a385142721d0cb16cd3b0be580 refs/heads/release-0.104
9f9faa404e3dec3e08cc73cf7b5a0439fc309800 refs/heads/release-0.105
8e1af145480d53e8d32cdff2c83291889903164b refs/heads/release-0.200
tom1 ~/emacs/spacemacs/.emacs.d $ git --version
git version 2.14.2

on archlinux: Linux achse 4.13.5-1-ARCH #1 SMP PREEMPT Fri Oct 6 
09:58:47 CEST 2017 x86_64 GNU/Linux

Re: Bug: git ls-remote -h / --head results differ in output

[Martin Ågren]

On 15 October 2017 at 12:02, Thomas Rikl <trikl@online.de> wrote:
> Example:
>
> tom1 ~/emacs/spacemacs/.emacs.d $ export LANG=en_US.utf8
>
> tom1 ~/emacs/spacemacs/.emacs.d $ git ls-remote -h
> usage: git ls-remote [--heads] [--tags] [--refs] [--upload-pack=<exec>]
>                      [-q | --quiet] [--exit-code] [--get-url]
>                      [--symref] [<repository> [<refs>...]]
>
>     -q, --quiet           do not print remote URL
>     --upload-pack <exec>  path of git-upload-pack on the remote host
>     -t, --tags            limit to tags
>     -h, --heads           limit to heads
>     --refs                do not show peeled tags
>     --get-url             take url.<base>.insteadOf into account
>     --exit-code           exit with exit code 2 if no matching refs are
> found
>     --symref              show underlying ref in addition to the object
> pointed by it
>
> tom1 ~/emacs/spacemacs/.emacs.d $ git ls-remote --head
> From https://github.com/syl20bnr/spacemacs
> 07014deead544c51fa6a826e91fe2ef05bf04323 refs/heads/develop
> 8e1af145480d53e8d32cdff2c83291889903164b refs/heads/master
> 2450b7e276634ece07b6b7ec6ca6c287af86caf3 refs/heads/release-0.101
> 8dadfc1494544bb152e80c2a436e43bc3713b389 refs/heads/release-0.102
> d993a021847cde2c42865bab6afa8adbb2edda0b refs/heads/release-0.103
> 44d4525543b1f2a385142721d0cb16cd3b0be580 refs/heads/release-0.104
> 9f9faa404e3dec3e08cc73cf7b5a0439fc309800 refs/heads/release-0.105
> 8e1af145480d53e8d32cdff2c83291889903164b refs/heads/release-0.200
> tom1 ~/emacs/spacemacs/.emacs.d $ git --version
> git version 2.14.2
>
> on archlinux: Linux achse 4.13.5-1-ARCH #1 SMP PREEMPT Fri Oct 6 09:58:47
> CEST 2017 x86_64 GNU/Linux

The behavior you observed matches with the documentation in gitcli(7)
and is arguably correct. A lone "-h" prints the help/usage. But I would
agree that this can be confusing, especially when considering
git-ls-remote(1) on its own, without any extra knowledge about what a
lone -h should do.

So -h and --heads can only be used interchangeably if you have other
arguments. To see this, you could, e.g., try "git ls-remote -h -h".

Some more details. This looks like ba5f28bf7 (ls-remote: use
parse-options api, 2016-01-19). Before that, "-h" was handled internally
in builtin/ls-files.c. After that it is handled in the general
options-parsing machinery. See for example 5ad0d3d52 (parse-options:
allow -h as a short option, 2015-11-17), which explicitly wants to print
the usage-string if "-h" is given as the lone argument.

I'm not sure which is the best way forward here, or how many other
commands could have similar pitfalls. For example, the "-h"-flag of git
grep shouldn't be able to cause this problem.

Martin

Re: Bug: git ls-remote -h / --head results differ in output

[René Scharfe]

Am 15.10.2017 um 13:08 schrieb Martin Ågren:
> On 15 October 2017 at 12:02, Thomas Rikl <trikl@online.de> wrote:
>> Example:
>>
>> tom1 ~/emacs/spacemacs/.emacs.d $ export LANG=en_US.utf8
>>
>> tom1 ~/emacs/spacemacs/.emacs.d $ git ls-remote -h
>> usage: git ls-remote [--heads] [--tags] [--refs] [--upload-pack=<exec>]
>>                       [-q | --quiet] [--exit-code] [--get-url]
>>                       [--symref] [<repository> [<refs>...]]
>>
>>      -q, --quiet           do not print remote URL
>>      --upload-pack <exec>  path of git-upload-pack on the remote host
>>      -t, --tags            limit to tags
>>      -h, --heads           limit to heads
>>      --refs                do not show peeled tags
>>      --get-url             take url.<base>.insteadOf into account
>>      --exit-code           exit with exit code 2 if no matching refs are
>> found
>>      --symref              show underlying ref in addition to the object
>> pointed by it
>>
>> tom1 ~/emacs/spacemacs/.emacs.d $ git ls-remote --head
>>  From https://github.com/syl20bnr/spacemacs
>> 07014deead544c51fa6a826e91fe2ef05bf04323 refs/heads/develop
>> 8e1af145480d53e8d32cdff2c83291889903164b refs/heads/master
>> 2450b7e276634ece07b6b7ec6ca6c287af86caf3 refs/heads/release-0.101
>> 8dadfc1494544bb152e80c2a436e43bc3713b389 refs/heads/release-0.102
>> d993a021847cde2c42865bab6afa8adbb2edda0b refs/heads/release-0.103
>> 44d4525543b1f2a385142721d0cb16cd3b0be580 refs/heads/release-0.104
>> 9f9faa404e3dec3e08cc73cf7b5a0439fc309800 refs/heads/release-0.105
>> 8e1af145480d53e8d32cdff2c83291889903164b refs/heads/release-0.200
>> tom1 ~/emacs/spacemacs/.emacs.d $ git --version
>> git version 2.14.2
>>
>> on archlinux: Linux achse 4.13.5-1-ARCH #1 SMP PREEMPT Fri Oct 6 09:58:47
>> CEST 2017 x86_64 GNU/Linux
> 
> The behavior you observed matches with the documentation in gitcli(7)
> and is arguably correct. A lone "-h" prints the help/usage. But I would
> agree that this can be confusing, especially when considering
> git-ls-remote(1) on its own, without any extra knowledge about what a
> lone -h should do.
> 
> So -h and --heads can only be used interchangeably if you have other
> arguments. To see this, you could, e.g., try "git ls-remote -h -h".
> 
> Some more details. This looks like ba5f28bf7 (ls-remote: use
> parse-options api, 2016-01-19). Before that, "-h" was handled internally
> in builtin/ls-files.c. After that it is handled in the general
> options-parsing machinery. See for example 5ad0d3d52 (parse-options:
> allow -h as a short option, 2015-11-17), which explicitly wants to print
> the usage-string if "-h" is given as the lone argument.
> 
> I'm not sure which is the best way forward here, or how many other
> commands could have similar pitfalls. For example, the "-h"-flag of git
> grep shouldn't be able to cause this problem.

The flag PARSE_OPT_NO_INTERNAL_HELP should be used to let git ls-remote
fully handle -h.

The same goes for git show-ref, but perhaps it's better to remove its
hidden option -h by now.

But stepping back a bit I wonder if the demure internal -h handler (that
only speaks up when nothing else is said) is a bit too subtle.
Reverting 5ad0d3d52 would make the need for PARSE_OPT_NO_INTERNAL_HELP
more obvious.

René

[PATCH 1/2] git: add NO_INTERNAL_HELP flag for builtins

[René Scharfe]

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@web.de>
---
 git.c           | 6 +++++-
 t/t0012-help.sh | 9 ++++++++-
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/git.c b/git.c
index 9e96dd4090..9d1b8c4716 100644
--- a/git.c
+++ b/git.c
@@ -298,6 +298,7 @@ static int handle_alias(int *argcp, const char ***argv)
 #define NEED_WORK_TREE		(1<<3)
 #define SUPPORT_SUPER_PREFIX	(1<<4)
 #define DELAY_PAGER_CONFIG	(1<<5)
+#define NO_INTERNAL_HELP	(1<<6)
 
 struct cmd_struct {
 	const char *cmd;
@@ -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");
 	if (!help) {
 		if (p->option & RUN_SETUP)
 			prefix = setup_git_directory();
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index 487b92a5de..74eeead168 100755
--- 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
+'
+
 while read builtin
 do
 	test_expect_success "$builtin can handle -h" '
 		test_expect_code 129 git $builtin -h >output 2>&1 &&
 		test_i18ngrep usage output
 	'
-done <builtins
+done <builtins_internal_help
 
 test_done
-- 
2.14.2

[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 <trikl@online.de>
Analyzed-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 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 <<EOF
+ls-remote
 EOF
 
 test_expect_success 'generate builtin list with internal help' '
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 02106c9226..637e9a87f3 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -264,4 +264,10 @@ test_expect_success 'ls-remote works outside repository' '
 	nongit git ls-remote dst.git
 '
 
+test_expect_success 'ls-remote -h == ls-remote --heads' '
+	git ls-remote -h >short &&
+	git ls-remote --heads >long &&
+	test_cmp long short
+'
+
 test_done
-- 
2.14.2

[Alt. PATCH] ls-remote: deprecate -h as short for --heads

[René Scharfe]

Stop advertising -h as the short equivalent of --heads, because it's
used for showing a short help text for almost all other git commands.
Since the ba5f28bf79 (ls-remote: use parse-options api) it has only
been working when used together with other parameters anyway.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
That would be step on the way towards more consistent command line
switches, in the same vein as d69155119 (t0012: test "-h" with
builtins).

 Documentation/git-ls-remote.txt | 1 -
 builtin/ls-remote.c             | 4 +++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 5f2628c8f8..898836a111 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -21,7 +21,6 @@ commit IDs.
 
 OPTIONS
 -------
--h::
 --heads::
 -t::
 --tags::
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index c4be98ab9e..840deedbef 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -56,7 +56,9 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 			   N_("path of git-upload-pack on the remote host"),
 			   PARSE_OPT_HIDDEN },
 		OPT_BIT('t', "tags", &flags, N_("limit to tags"), REF_TAGS),
-		OPT_BIT('h', "heads", &flags, N_("limit to heads"), REF_HEADS),
+		OPT_BIT(0, "heads", &flags, N_("limit to heads"), REF_HEADS),
+		{ OPTION_BIT, 'h', NULL, &flags, NULL, N_("limit to heads"),
+			PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, REF_HEADS },
 		OPT_BIT(0, "refs", &flags, N_("do not show peeled tags"), REF_NORMAL),
 		OPT_BOOL(0, "get-url", &get_url,
 			 N_("take url.<base>.insteadOf into account")),
-- 
2.14.2

Re: [PATCH 1/2] git: add NO_INTERNAL_HELP flag for builtins

[Jonathan Nieder]

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@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 <jrnieder@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

Re: [Alt. PATCH] ls-remote: deprecate -h as short for --heads

[Martin Ågren]

On 17 October 2017 at 17:39, René Scharfe <l.s.r@web.de> wrote:
> Stop advertising -h as the short equivalent of --heads, because it's
> used for showing a short help text for almost all other git commands.
> Since the ba5f28bf79 (ls-remote: use parse-options api) it has only
> been working when used together with other parameters anyway.
>
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
> That would be step on the way towards more consistent command line
> switches, in the same vein as d69155119 (t0012: test "-h" with
> builtins).

FWIW, my first inclination would be to go with a patch like this instead
of the other two (where you introduce an exception so that git ls-remote
-h does not display the usage). ba5f28bf79 was in 2.8.0. That's 18
months ago. Not an eternity, but still some time ago. Not fixing this
regression has an obvious downside, but there's also a downside to
adding the exception.

As long as a lone -h may give the usage or do something else entirely,
then -- put bluntly -- to know whether you can request the usage with
git foo -h without risk of messing up your repo, you'll need to look up
the usage some other way. At which point you've solved your original
problem, without -h.

Of course, we could promise that -h will give the usage or, in case of
historical wart(s), do something harmless. But it seems a bit awkward,
and might limit the perceived usefulness/safeness or -h.

> diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
> index 5f2628c8f8..898836a111 100644
> --- a/Documentation/git-ls-remote.txt
> +++ b/Documentation/git-ls-remote.txt
> @@ -21,7 +21,6 @@ commit IDs.
>
>  OPTIONS
>  -------
> --h::
>  --heads::
>  -t::
>  --tags::

Do we want to document -h as a deprecated alias, so that people have a
slightly larger chance of noticing and adapting?

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 <trikl@online.de>
> Analyzed-by: Martin Ågren <martin.agren@gmail.com>
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
>  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 <jrnieder@gmail.com>

[...]
> --- 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 <l.s.r@web.de> 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 <trikl@online.de>
> Analyzed-by: Martin Ågren <martin.agren@gmail.com>
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
>  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.

Re: [Alt. PATCH] ls-remote: deprecate -h as short for --heads

[Jonathan Nieder]

René Scharfe wrote:

> Stop advertising -h as the short equivalent of --heads, because it's
> used for showing a short help text for almost all other git commands.
> Since the ba5f28bf79 (ls-remote: use parse-options api) it has only
> been working when used together with other parameters anyway.
>
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
>  Documentation/git-ls-remote.txt | 1 -
>  builtin/ls-remote.c             | 4 +++-
>  2 files changed, 3 insertions(+), 2 deletions(-)

Yes, I think this is a good step.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

I would be tempted to go even further and make "git ls-remote -h"
print a warning to stderr to encourage people to update their scripts
to use --heads instead.

Thanks.

Re: [Alt. PATCH] ls-remote: deprecate -h as short for --heads

[Junio C Hamano]

René Scharfe <l.s.r@web.de> writes:

> Stop advertising -h as the short equivalent of --heads, because it's
> used for showing a short help text for almost all other git commands.
> Since the ba5f28bf79 (ls-remote: use parse-options api) it has only
> been working when used together with other parameters anyway.
>
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
> That would be step on the way towards more consistent command line
> switches, in the same vein as d69155119 (t0012: test "-h" with
> builtins).

Sorry, but I am not sure whom this and the other approach are trying
to help.

The rule we have currently seems to be that "git cmd -h" (no other
arguments) consistently gives a short help, and if a subcommand
supports an option "-h" specific to it that is not about giving a
short help, the caller needs to be aware of it to invoke the option,
by making sure that there is some other arguments on the command
line if "-h" form of that subcommand specific option is used, by
doing e.g. "git ls-remote -h origin" or "git ls-remote --head".

I can see that this "alternative" approach makes it less convenent
for folks who have followed that rule by hiding "-h" (with the
intention of deprecating and possibly removing it in the future) and
encouraging (and later foring) "--head" to be used instead.  

The other approach burdens new users by changing the rule to "some
subcommands that have their own '-h' option cannot be asked for a
brief usage with 'git cmd -h'".  But the thing is, these new users
who do not know which subcommands do have their own '-h' and which
ones do not are the ones that benefit most from the consistent "'git
cmd -h' with no other argument gets short help" rule.

When making a backward incompatible change, it is asking us to go to
those who are inconvenienced and say "I am sorry that we are making
things less convenient for you, but by doing so we can gain this
greater good which is ...".  I can explain how this and the other
approach make things less convenient for existing or new users, but
I am having a hard time formulating what the greater good is.

In short, I cannot sell this change to our users.  Please help me do
so if you want this (or the other) change made to our system.

Re: [Alt. PATCH] ls-remote: deprecate -h as short for --heads

[René Scharfe]

Am 18.10.2017 um 01:22 schrieb Junio C Hamano:
> René Scharfe <l.s.r@web.de> writes:
> 
>> Stop advertising -h as the short equivalent of --heads, because it's
>> used for showing a short help text for almost all other git commands.
>> Since the ba5f28bf79 (ls-remote: use parse-options api) it has only
>> been working when used together with other parameters anyway.
>>
>> Signed-off-by: Rene Scharfe <l.s.r@web.de>
>> ---
>> That would be step on the way towards more consistent command line
>> switches, in the same vein as d69155119 (t0012: test "-h" with
>> builtins).
> 
> Sorry, but I am not sure whom this and the other approach are trying
> to help.
> 
> The rule we have currently seems to be that "git cmd -h" (no other
> arguments) consistently gives a short help, and if a subcommand
> supports an option "-h" specific to it that is not about giving a
> short help, the caller needs to be aware of it to invoke the option,
> by making sure that there is some other arguments on the command
> line if "-h" form of that subcommand specific option is used, by
> doing e.g. "git ls-remote -h origin" or "git ls-remote --head".
> 
> I can see that this "alternative" approach makes it less convenent
> for folks who have followed that rule by hiding "-h" (with the
> intention of deprecating and possibly removing it in the future) and
> encouraging (and later foring) "--head" to be used instead.
> 
> The other approach burdens new users by changing the rule to "some
> subcommands that have their own '-h' option cannot be asked for a
> brief usage with 'git cmd -h'".  But the thing is, these new users
> who do not know which subcommands do have their own '-h' and which
> ones do not are the ones that benefit most from the consistent "'git
> cmd -h' with no other argument gets short help" rule.

They would help by aligning documentation and code, at a price.  But
of course there is another way to do that -- I just didn't see it the
other day.  We don't have to take anything away:

-- >8 --
Subject: [PATCH] ls-remote: document behavior of lone parameter -h

Reported-by: Thomas Rikl <trikl@online.de>
Analyzed-by: Martin Ågren <martin.agren@gmail.com>
Analyzed-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 Documentation/git-ls-remote.txt | 10 ++++++++++
 builtin/ls-remote.c             |  5 ++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 5f2628c8f8..82622e7fbc 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -29,6 +29,9 @@ OPTIONS
 	These options are _not_ mutually exclusive; when given
 	both, references stored in refs/heads and refs/tags are
 	displayed.
++
+*NOTE*: -h without any other parameters shows a short help text instead
+of any refs.
 
 --refs::
 	Do not show peeled tags or pseudorefs like HEAD	in the output.
@@ -89,6 +92,13 @@ EXAMPLES
 	f25a265a342aed6041ab0cc484224d9ca54b6f41	refs/tags/v0.99.1
 	c5db5456ae3b0873fc659c19fafdde22313cc441	refs/tags/v0.99.2
 	7ceca275d047c90c0c7d5afb13ab97efdf51bd6e	refs/tags/v0.99.3
+	$ git ls-remote -hh
+	From https://git.kernel.org/pub/scm/git/git.git/
+	4c2224e83951a685185bb8c1f83b28e22fee0e27	refs/heads/maint
+	5fe978a5381f1fbad26a80e682ddd2a401966740	refs/heads/master
+	76aedb4517c834be2dc89efb5f9d15908e324422	refs/heads/next
+	c781a84b5204fb294c9ccc79f8b3baceeb32c061	refs/heads/pu
+	5d38b589ccc7a8355c62f1577865df5b8216c00d	refs/heads/todo
 
 GIT
 ---
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index c4be98ab9e..0438dfec05 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -56,7 +56,10 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 			   N_("path of git-upload-pack on the remote host"),
 			   PARSE_OPT_HIDDEN },
 		OPT_BIT('t', "tags", &flags, N_("limit to tags"), REF_TAGS),
-		OPT_BIT('h', "heads", &flags, N_("limit to heads"), REF_HEADS),
+		OPT_BIT(0, "heads", &flags, N_("limit to heads"), REF_HEADS),
+		OPT_BIT('h', NULL, &flags,
+			N_("if only parameter: show this help text, otherwise limit to heads"),
+			REF_HEADS),
 		OPT_BIT(0, "refs", &flags, N_("do not show peeled tags"), REF_NORMAL),
 		OPT_BOOL(0, "get-url", &get_url,
 			 N_("take url.<base>.insteadOf into account")),
-- 
2.14.2

Re: [Alt. PATCH] ls-remote: deprecate -h as short for --heads

[Jeff King]

On Thu, Oct 19, 2017 at 08:26:47PM +0200, René Scharfe wrote:

> diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
> index 5f2628c8f8..82622e7fbc 100644
> --- a/Documentation/git-ls-remote.txt
> +++ b/Documentation/git-ls-remote.txt
> @@ -29,6 +29,9 @@ OPTIONS
>  	These options are _not_ mutually exclusive; when given
>  	both, references stored in refs/heads and refs/tags are
>  	displayed.
> ++
> +*NOTE*: -h without any other parameters shows a short help text instead
> +of any refs.

Yuck. This "we only treat -h as special in certain cases" rule is
sufficiently magical that I don't think we want to advertise and lock
ourselves into it.

-Peff

Re: [Alt. PATCH] ls-remote: deprecate -h as short for --heads

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Thu, Oct 19, 2017 at 08:26:47PM +0200, René Scharfe wrote:
>
>> diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
>> index 5f2628c8f8..82622e7fbc 100644
>> --- a/Documentation/git-ls-remote.txt
>> +++ b/Documentation/git-ls-remote.txt
>> @@ -29,6 +29,9 @@ OPTIONS
>>  	These options are _not_ mutually exclusive; when given
>>  	both, references stored in refs/heads and refs/tags are
>>  	displayed.
>> ++
>> +*NOTE*: -h without any other parameters shows a short help text instead
>> +of any refs.
>
> Yuck. This "we only treat -h as special in certain cases" rule is
> sufficiently magical that I don't think we want to advertise and lock
> ourselves into it.

Hmph.  I think it is way too late to be worried about "locked into"
the convention---hasn't the "git cmd -h" been with us forever?

Besides, I personally feel that there is nothing magical in the rule
that is "we always treat 'git $cmd -h' as asking for short help, but
individual subcommand may choose to use -h for, perhaps to be
compatible with other tools and conventions, in other situations".

Re: [Alt. PATCH] ls-remote: deprecate -h as short for --heads

[Jeff King]

On Fri, Oct 20, 2017 at 10:04:23AM +0900, Junio C Hamano wrote:

> > Yuck. This "we only treat -h as special in certain cases" rule is
> > sufficiently magical that I don't think we want to advertise and lock
> > ourselves into it.
> 
> Hmph.  I think it is way too late to be worried about "locked into"
> the convention---hasn't the "git cmd -h" been with us forever?

I guess. I still find it pretty nasty (not that "-h" works for help, but
that it can override the normal usage).

> Besides, I personally feel that there is nothing magical in the rule
> that is "we always treat 'git $cmd -h' as asking for short help, but
> individual subcommand may choose to use -h for, perhaps to be
> compatible with other tools and conventions, in other situations".

<shrug> It seems weird and inconsistent to me that the meaning of "-h"
depends on the position and presence of other unrelated options. Maybe
it's just me. I know _why_ it's that way, but this seems like one of
those weird corners of the interface that end up confusing people and
giving Git's interface the reputation of being full of mysterious
inconsistencies. I suspect one of the reasons nobody has complained
about it is that "ls-remote" is not commonly used, and "ls-remote -h"
less so (I didn't even know it was there until this conversation).

-Peff

Re: [Alt. PATCH] ls-remote: deprecate -h as short for --heads

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> <shrug> It seems weird and inconsistent to me that the meaning of "-h"
> depends on the position and presence of other unrelated options. Maybe
> it's just me. I know _why_ it's that way, but this seems like one of
> those weird corners of the interface that end up confusing people and
> giving Git's interface the reputation of being full of mysterious
> inconsistencies. I suspect one of the reasons nobody has complained
> about it is that "ls-remote" is not commonly used, and "ls-remote -h"
> less so (I didn't even know it was there until this conversation).

Technically, yes, it may be weird.

I may be biased as every time I think about this one, the first one
that comes to my mind is how "grep -h" (not "git grep", but GNU
grep) behaves.  Yes, "-h" means something else, but by itself, the
command does not make sense, and "ls-remote -h" is an exception to
the rule: most sane commands do not have a sensible semantics when
they take only "-h" and nothing else.  And even for ls-remote it is
true only when you try to be extra lazy and do not say which remote
you are asking.

And that is why I think "'cmd -h" and nothing else consistently
gives help" may overall be positive in usability, than a rule that
says "'cmd -h' is for short-help unless -h means something else for
the command".

Re: [Alt. PATCH] ls-remote: deprecate -h as short for --heads

[Jeff King]

On Fri, Oct 20, 2017 at 02:35:36PM +0900, Junio C Hamano wrote:

> I may be biased as every time I think about this one, the first one
> that comes to my mind is how "grep -h" (not "git grep", but GNU
> grep) behaves.  Yes, "-h" means something else, but by itself, the
> command does not make sense, and "ls-remote -h" is an exception to
> the rule: most sane commands do not have a sensible semantics when
> they take only "-h" and nothing else.  And even for ls-remote it is
> true only when you try to be extra lazy and do not say which remote
> you are asking.

Yeah, I have to admit "grep -h" is a lot more compelling, since it
matches normal grep. And I've used "grep -h" many times without thinking
about it, simply because the rule just falls out naturally there
(there's nothing possible to do without a regex argument, so we'd show
the usage either way).

> And that is why I think "'cmd -h" and nothing else consistently
> gives help" may overall be positive in usability, than a rule that
> says "'cmd -h' is for short-help unless -h means something else for
> the command".

Yeah, I was shooting more for "let's avoid assigning -h to things". But
that's not really possible if we want to be consistent with upstream
grep, which is probably more important.

I think you've convinced me.

-Peff

Re: [Alt. PATCH] ls-remote: deprecate -h as short for --heads

[René Scharfe]

Am 20.10.2017 um 07:35 schrieb Junio C Hamano:
> Jeff King <peff@peff.net> writes:
> 
>> <shrug> It seems weird and inconsistent to me that the meaning of "-h"
>> depends on the position and presence of other unrelated options.

The position is relevant with parse-options for *each* flag for a
different reason as well: You can't put a flag after a non-flag.  I
find that more annoying, as it slightly but noticeable slows down
adding a flag to a previous command by requiring to navigate to the
middle of the line.

(I guess I should train myself to use Meta-b for going back one word
more often..)

> I may be biased as every time I think about this one, the first one
> that comes to my mind is how "grep -h" (not "git grep", but GNU
> grep) behaves.  Yes, "-h" means something else, but by itself, the
> command does not make sense, and "ls-remote -h" is an exception to
> the rule: most sane commands do not have a sensible semantics when
> they take only "-h" and nothing else.  And even for ls-remote it is
> true only when you try to be extra lazy and do not say which remote
> you are asking.
> 
> And that is why I think "'cmd -h" and nothing else consistently
> gives help" may overall be positive in usability, than a rule that
> says "'cmd -h' is for short-help unless -h means something else for
> the command".

FWIW, I use "-?" for that everywhere.  I have yet to find a command or
environment where it does something dangerous.

René

Re: [Alt. PATCH] ls-remote: deprecate -h as short for --heads

[Junio C Hamano]

René Scharfe <l.s.r@web.de> writes:

> FWIW, I use "-?" for that everywhere.  I have yet to find a command or
> environment where it does something dangerous.

Yeah, it would have made the world a better place if we made that
choice back in 2008.  If we start a transition to make it so right
now, we might be able to make the world a better place by 2022,
perhaps.  I am not sure if the pain during the transition is worth
it, though.

Re: [Alt. PATCH] ls-remote: deprecate -h as short for --heads

[René Scharfe]

Am 21.10.2017 um 14:18 schrieb Junio C Hamano:
> René Scharfe <l.s.r@web.de> writes:
> 
>> FWIW, I use "-?" for that everywhere.  I have yet to find a command or
>> environment where it does something dangerous.
> 
> Yeah, it would have made the world a better place if we made that
> choice back in 2008.  If we start a transition to make it so right
> now, we might be able to make the world a better place by 2022,
> perhaps.  I am not sure if the pain during the transition is worth
> it, though.

"-?" works fine with builtins already -- they complain that the option
is unknown and then show the short help text.

If we removed the special handling for "-h" from parse-options it would
do the same for most commands, i.e. we'd get an additional error
message, followed by the short help text that we're used to see.  We
could check for "git grep -h" without any other arguments and show the
usage text in that case.  We could do the same for "git ls-remote -h"
as well, but I'm not sure we want to.

So the cost would be an extra error message in most cases, and possibly
the inability to show help with "git ls-remote -h".  That doesn't sound
very painful.

René

Re: [Alt. PATCH] ls-remote: deprecate -h as short for --heads

[Junio C Hamano]

René Scharfe <l.s.r@web.de> writes:

> Am 21.10.2017 um 14:18 schrieb Junio C Hamano:
>> René Scharfe <l.s.r@web.de> writes:
>> 
>>> FWIW, I use "-?" for that everywhere.  I have yet to find a command or
>>> environment where it does something dangerous.
>> 
>> Yeah, it would have made the world a better place if we made that
>> choice back in 2008.  If we start a transition to make it so right
>> now, we might be able to make the world a better place by 2022,
>> perhaps.  I am not sure if the pain during the transition is worth
>> it, though.
>
> "-?" works fine with builtins already -- they complain that the option
> is unknown and then show the short help text.

Ah, I misunderstood what you meant, then.  I thought you were
advocating to move the built-in short-help support to know about and
explicitly react to "-?", and somehow forgot that it "works" more or
less already.

The fact that "-?" already works for most things is good, but the
transition pain still remains, as what's costly is to transition
people's expectation (i.e. "'-?' and not '-h' is the way to get
short help from any subcommand"), not the implementation to fill the
gaps for those that do not yet support '-?', I am afraid.

Re: [Alt. PATCH] ls-remote: deprecate -h as short for --heads

[René Scharfe]

Am 24.10.2017 um 02:52 schrieb Junio C Hamano:
> René Scharfe <l.s.r@web.de> writes:
> 
>> Am 21.10.2017 um 14:18 schrieb Junio C Hamano:
>>> René Scharfe <l.s.r@web.de> writes:
>>>
>>>> FWIW, I use "-?" for that everywhere.  I have yet to find a command or
>>>> environment where it does something dangerous.
>>>
>>> Yeah, it would have made the world a better place if we made that
>>> choice back in 2008.  If we start a transition to make it so right
>>> now, we might be able to make the world a better place by 2022,
>>> perhaps.  I am not sure if the pain during the transition is worth
>>> it, though.
>>
>> "-?" works fine with builtins already -- they complain that the option
>> is unknown and then show the short help text.
> 
> Ah, I misunderstood what you meant, then.  I thought you were
> advocating to move the built-in short-help support to know about and
> explicitly react to "-?", and somehow forgot that it "works" more or
> less already.

I don't mean to advocate here -- it's more like trying to get the
accounting right.  A little bit of OCD perhaps?

> The fact that "-?" already works for most things is good, but the
> transition pain still remains, as what's costly is to transition
> people's expectation (i.e. "'-?' and not '-h' is the way to get
> short help from any subcommand"), not the implementation to fill the
> gaps for those that do not yet support '-?', I am afraid.

A minor cost for help-seeking users would be the extra error message at
the top of the short help text.

The main change would cause "git ls-remote -h" to show remote heads and
"git show-ref -h" to show HEAD.  Confused users would have to resort to
e.g. man, -help, --help, their search engine of choice, or -?.  I feel
this could be justified.  It would be different if e.g. reset started
to take -h as shorthand for --hard, as this would cause data loss.

The hard part would probably be allowing the execution of commands with
unknown arguments outside of repositories to show the help text, even
if they'd normally (with correct arguments) require one.  Converting
all commands from RUN_SETUP to RUN_SETUP_GENTLY seems painful.  Showing
help when a required repo is not found might be possible somehow.

With that I'm going to shut up, as I don't even use the affected
commands, nor can I imagine being in the place of someone impacted by
"git <something> -h" not showing a help text.

René