ls-remote: Remove conflicting option "-h"
diff mbox series

Message ID 20191030173337.6574-1-mattias.hansson@axis.com
State New
Headers show
Series
  • ls-remote: Remove conflicting option "-h"
Related show

Commit Message

Mattias Hansson Oct. 30, 2019, 5:33 p.m. UTC
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(-)

Comments

SZEDER Gábor Oct. 30, 2019, 6:28 p.m. UTC | #1
On Wed, Oct 30, 2019 at 06:33:37PM +0100, Mattias Hansson wrote:
> 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.

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

Patch
diff mbox series

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")),