threads / patch / 52160

patchls-remote: Remove conflicting option "-h"

Subject: [PATCH] ls-remote: Remove conflicting option "-h"

## tl;dr

2 messages between Oct 30, 2019 and Oct 30, 2019. Diffs are folded; open one to read it.

replies: 1people: 2as markdown or json

Mattias Hansson· Oct 30, 2019, 17:33 UTC · lore
From: Mattias Hansson <hansson.mattias@gmail.com>

The "-h" option is documented to limit the result to heads, which is a bit nonintuitive since "-h" is commonly used as the short option for "--help". However, "-h" currently displays the usage information which proves that there's a double reference to the option.

This patch will remove "-h" as a short option for "--heads". This will maintain the current behaviour and correct the misleading documentation.

Signed-off-by: Mattias Hansson <hansson.mattias@gmail.com>
---
 builtin/ls-remote.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to builtin/ls-remote.c +1 −1
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 6ef519514b..85ce336fc3 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -63,7 +63,7 @@ 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(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.22.0
SZEDER Gábor· Oct 30, 2019, 18:28 UTC · re: Mattias Hansson · lore

Re: [PATCH] ls-remote: Remove conflicting option "-h"

On Wed, Oct 30, 2019 at 06:33:37PM +0100, Mattias Hansson wrote:
Show 6 quoted lines
> From: Mattias Hansson <hansson.mattias@gmail.com>
> 
> The "-h" option is documented to limit the result to heads, which is a
> bit nonintuitive since "-h" is commonly used as the short option for
> "--help". However, "-h" currently displays the usage information
> which proves that there's a double reference to the option.
Interesting.

While 'git ls-remote -h' shows the usage information, 'git ls-remote -h origin' does list branches from the given remote. This is intentional: 'git ls-remote -h' without a remote listed branches from the default remote for years, but, as you pointed out, it's nonintuitive, so it's behavior was changed to what we have today in commit 91a640ffb6 (ls-remote: a lone "-h" is asking for help, 2011-09-16).

I think the short option should stay, because removing it will break the case when the remote is specified on the command line.

However, its description in the usage string and in the man page could be improved to point out this caveat. Doing so in the man page is surely not too difficult, because we can dedicate a whole sentence or even two to this corner case, but I'm not sure how it could be sensibly squeezed into the usage string.

> This patch will remove "-h" as a short option for "--heads". This
> will maintain the current behaviour and correct the misleading
> documentation.

Note that '-h' is documentet in the man page as well, so if we decide to go for removal, then it should be removed from the man page as well.

Show 21 quoted lines
> Signed-off-by: Mattias Hansson <hansson.mattias@gmail.com>
> ---
>  builtin/ls-remote.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
> index 6ef519514b..85ce336fc3 100644
> --- a/builtin/ls-remote.c
> +++ b/builtin/ls-remote.c
> @@ -63,7 +63,7 @@ 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(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.22.0
> 

← back to recent threads