diff mbox series

[2/3] ls-remote: introduce --branches and deprecate --heads

Message ID 20240603200539.1473345-3-gitster@pobox.com (mailing list archive)
State Superseded
Headers show
Series Branches are branches and not heads | expand

Commit Message

Junio C Hamano June 3, 2024, 8:05 p.m. UTC
We call the tips of branches "heads", but this command calls the
option to show only branches "--heads", which confuses the branches
themselves and the tips of branches.

Straighten the terminology by introducing "--branches" option that
limits the output to branches, and deprecate "--heads" option used
that way.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-ls-remote.txt | 12 +++++++-----
 builtin/ls-remote.c             | 21 ++++++++++++++++++++-
 t/t5512-ls-remote.sh            | 30 +++++++++++++++++++++++++-----
 3 files changed, 52 insertions(+), 11 deletions(-)

Comments

Rubén Justo June 3, 2024, 9:30 p.m. UTC | #1
On Mon, Jun 03, 2024 at 01:05:38PM -0700, Junio C Hamano wrote:
> We call the tips of branches "heads", but this command calls the
> option to show only branches "--heads", which confuses the branches
> themselves and the tips of branches.
> 
> Straighten the terminology by introducing "--branches" option that
> limits the output to branches, and deprecate "--heads" option used
> that way.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/git-ls-remote.txt | 12 +++++++-----
>  builtin/ls-remote.c             | 21 ++++++++++++++++++++-
>  t/t5512-ls-remote.sh            | 30 +++++++++++++++++++++++++-----
>  3 files changed, 52 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
> index 1c4f696ab5..76c86c3ce4 100644
> --- a/Documentation/git-ls-remote.txt
> +++ b/Documentation/git-ls-remote.txt
> @@ -9,7 +9,7 @@ git-ls-remote - List references in a remote repository
>  SYNOPSIS
>  --------
>  [verse]
> -'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=<exec>]
> +'git ls-remote' [--branches] [--tags] [--refs] [--upload-pack=<exec>]
>  	      [-q | --quiet] [--exit-code] [--get-url] [--sort=<key>]
>  	      [--symref] [<repository> [<patterns>...]]
>  
> @@ -21,14 +21,16 @@ commit IDs.
>  
>  OPTIONS
>  -------
> --h::
> ---heads::
> +-b::

Perhaps we can avoid this 'single-letter' option, due to we're no giving
the same abbreviation for '--branches' in other places.

> +--branches::
>  -t::
>  --tags::
> -	Limit to only refs/heads and refs/tags, respectively.
> +	Limit to only local branches and local tags, respectively.
>  	These options are _not_ mutually exclusive; when given
>  	both, references stored in refs/heads and refs/tags are
> -	displayed.  Note that `git ls-remote -h` used without
> +	displayed.  Note that `--heads` and `-h` are deprecated
> +	synonyms for `--branches` and `-b` and may be removed in
> +	the future.  Also note that `git ls-remote -h` used without
>  	anything else on the command line gives help, consistent
>  	with other git subcommands.
>  
> diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
> index 9838de69c0..95fbb8b7b5 100644
> --- a/builtin/ls-remote.c
> +++ b/builtin/ls-remote.c
> @@ -38,6 +38,20 @@ static int tail_match(const char **pattern, const char *path)
>  	return 0;
>  }
>  
> +static int heads_callback(const struct option *opt, const char *arg, int unset)
> +{
> +	unsigned *flags = opt->value;
> +
> +	if (unset) {
> +		warning(_("'--no-heads' is deprecated; use '--no-branches' instead"));

I wonder if this would be better: 

	warning(_("'%s' is deprecated; use '%s' instead"), '--no-heads', '--no-branches');

> +		*flags &= ~REF_BRANCHES;
> +	} else {
> +		warning(_("'--heads' is deprecated; use '--branches' instead"));
> +		*flags |= REF_BRANCHES;
> +	}
> +	return 0;
> +}
> +
>  int cmd_ls_remote(int argc, const char **argv, const char *prefix)
>  {
>  	const char *dest = NULL;
> @@ -68,7 +82,12 @@ 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_BRANCHES),
> +		OPT_BIT('b', "branches", &flags, N_("limit to branches"), REF_BRANCHES),
> +		OPT_CALLBACK_F('h', "heads", &flags,
> +			       NULL,
> +			       N_("deprecated synonym for --branches"),
> +			       PARSE_OPT_NOARG|PARSE_OPT_HIDDEN,
> +			       &heads_callback),
>  		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")),
> diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
> index 5dbe107ce8..b9950ca361 100755
> --- a/t/t5512-ls-remote.sh
> +++ b/t/t5512-ls-remote.sh
> @@ -47,6 +47,7 @@ test_expect_success setup '
>  	git show-ref -d	>refs &&
>  	sed -e "s/ /	/" refs >>expected.all &&
>  
> +	grep refs/heads/ expected.all >expected.branches &&
>  	git remote add self "$(pwd)/.git" &&
>  	git remote add self2 "."
>  '
> @@ -71,6 +72,25 @@ test_expect_success 'ls-remote self' '
>  	test_cmp expected.all actual
>  '
>  
> +test_expect_success 'ls-remote --branches self' '
> +	git ls-remote --branches self >actual &&
> +	test_cmp expected.branches actual &&
> +	git ls-remote -b self >actual &&
> +	test_cmp expected.branches actual
> +'
> +
> +test_expect_success 'ls-remote -h is deprecated' '
> +	git ls-remote -h self >actual 2>warning &&
> +	test_cmp expected.branches actual &&
> +	test_grep deprecated warning
> +'
> +
> +test_expect_success 'ls-remote --heads is deprecated' '
> +	git ls-remote --heads self >actual 2>warning &&
> +	test_cmp expected.branches actual &&
> +	test_grep deprecated warning
> +'
> +
>  test_expect_success 'ls-remote --sort="version:refname" --tags self' '
>  	generate_references \
>  		refs/tags/mark \
> @@ -275,7 +295,7 @@ test_expect_success 'ls-remote with filtered symref (refname)' '
>  	test_cmp expect actual
>  '
>  
> -test_expect_success 'ls-remote with filtered symref (--heads)' '
> +test_expect_success 'ls-remote with filtered symref (--branches)' '
>  	git symbolic-ref refs/heads/foo refs/tags/mark &&
>  	cat >expect.v2 <<-EOF &&
>  	ref: refs/tags/mark	refs/heads/foo
> @@ -283,9 +303,9 @@ test_expect_success 'ls-remote with filtered symref (--heads)' '
>  	$rev	refs/heads/main
>  	EOF
>  	grep -v "^ref: refs/tags/" <expect.v2 >expect.v0 &&
> -	git -c protocol.version=0 ls-remote --symref --heads . >actual.v0 &&
> +	git -c protocol.version=0 ls-remote --symref --branches . >actual.v0 &&
>  	test_cmp expect.v0 actual.v0 &&
> -	git -c protocol.version=2 ls-remote --symref --heads . >actual.v2 &&
> +	git -c protocol.version=2 ls-remote --symref --branches . >actual.v2 &&
>  	test_cmp expect.v2 actual.v2
>  '
>  
> @@ -335,9 +355,9 @@ test_expect_success 'ls-remote patterns work with all protocol versions' '
>  test_expect_success 'ls-remote prefixes work with all protocol versions' '
>  	git for-each-ref --format="%(objectname)	%(refname)" \
>  		refs/heads/ refs/tags/ >expect &&
> -	git -c protocol.version=0 ls-remote --heads --tags . >actual.v0 &&
> +	git -c protocol.version=0 ls-remote --branches --tags . >actual.v0 &&
>  	test_cmp expect actual.v0 &&
> -	git -c protocol.version=2 ls-remote --heads --tags . >actual.v2 &&
> +	git -c protocol.version=2 ls-remote --branches --tags . >actual.v2 &&
>  	test_cmp expect actual.v2
>  '
>  
> -- 
> 2.45.2-404-g9eaef5822c
>
Eric Sunshine June 3, 2024, 9:42 p.m. UTC | #2
On Mon, Jun 3, 2024 at 5:30 PM Rubén Justo <rjusto@gmail.com> wrote:
> On Mon, Jun 03, 2024 at 01:05:38PM -0700, Junio C Hamano wrote:
> > +     if (unset) {
> > +             warning(_("'--no-heads' is deprecated; use '--no-branches' instead"));
>
> I wonder if this would be better:
>
>         warning(_("'%s' is deprecated; use '%s' instead"), '--no-heads', '--no-branches');

Unless there is a concrete plan to free up --heads to mean something
else in the future, I wonder why we need to warn about this at all,
especially since retaining the deprecated --heads alias in perpetuity
is effectively zero-cost for the Git maintainers, whereas adding this
warning potentially punishes users by making them do extra work for no
obvious reason.
Junio C Hamano June 3, 2024, 9:48 p.m. UTC | #3
Eric Sunshine <sunshine@sunshineco.com> writes:

> Unless there is a concrete plan to free up --heads to mean something
> else in the future, I wonder why we need to warn about this at all,

There is no plan to repurpose "--heads", but this is primarily to
make sure "-h" cannot mean anything but "please give me a short
help".
diff mbox series

Patch

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 1c4f696ab5..76c86c3ce4 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -9,7 +9,7 @@  git-ls-remote - List references in a remote repository
 SYNOPSIS
 --------
 [verse]
-'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=<exec>]
+'git ls-remote' [--branches] [--tags] [--refs] [--upload-pack=<exec>]
 	      [-q | --quiet] [--exit-code] [--get-url] [--sort=<key>]
 	      [--symref] [<repository> [<patterns>...]]
 
@@ -21,14 +21,16 @@  commit IDs.
 
 OPTIONS
 -------
--h::
---heads::
+-b::
+--branches::
 -t::
 --tags::
-	Limit to only refs/heads and refs/tags, respectively.
+	Limit to only local branches and local tags, respectively.
 	These options are _not_ mutually exclusive; when given
 	both, references stored in refs/heads and refs/tags are
-	displayed.  Note that `git ls-remote -h` used without
+	displayed.  Note that `--heads` and `-h` are deprecated
+	synonyms for `--branches` and `-b` and may be removed in
+	the future.  Also note that `git ls-remote -h` used without
 	anything else on the command line gives help, consistent
 	with other git subcommands.
 
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 9838de69c0..95fbb8b7b5 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -38,6 +38,20 @@  static int tail_match(const char **pattern, const char *path)
 	return 0;
 }
 
+static int heads_callback(const struct option *opt, const char *arg, int unset)
+{
+	unsigned *flags = opt->value;
+
+	if (unset) {
+		warning(_("'--no-heads' is deprecated; use '--no-branches' instead"));
+		*flags &= ~REF_BRANCHES;
+	} else {
+		warning(_("'--heads' is deprecated; use '--branches' instead"));
+		*flags |= REF_BRANCHES;
+	}
+	return 0;
+}
+
 int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 {
 	const char *dest = NULL;
@@ -68,7 +82,12 @@  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_BRANCHES),
+		OPT_BIT('b', "branches", &flags, N_("limit to branches"), REF_BRANCHES),
+		OPT_CALLBACK_F('h', "heads", &flags,
+			       NULL,
+			       N_("deprecated synonym for --branches"),
+			       PARSE_OPT_NOARG|PARSE_OPT_HIDDEN,
+			       &heads_callback),
 		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")),
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 5dbe107ce8..b9950ca361 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -47,6 +47,7 @@  test_expect_success setup '
 	git show-ref -d	>refs &&
 	sed -e "s/ /	/" refs >>expected.all &&
 
+	grep refs/heads/ expected.all >expected.branches &&
 	git remote add self "$(pwd)/.git" &&
 	git remote add self2 "."
 '
@@ -71,6 +72,25 @@  test_expect_success 'ls-remote self' '
 	test_cmp expected.all actual
 '
 
+test_expect_success 'ls-remote --branches self' '
+	git ls-remote --branches self >actual &&
+	test_cmp expected.branches actual &&
+	git ls-remote -b self >actual &&
+	test_cmp expected.branches actual
+'
+
+test_expect_success 'ls-remote -h is deprecated' '
+	git ls-remote -h self >actual 2>warning &&
+	test_cmp expected.branches actual &&
+	test_grep deprecated warning
+'
+
+test_expect_success 'ls-remote --heads is deprecated' '
+	git ls-remote --heads self >actual 2>warning &&
+	test_cmp expected.branches actual &&
+	test_grep deprecated warning
+'
+
 test_expect_success 'ls-remote --sort="version:refname" --tags self' '
 	generate_references \
 		refs/tags/mark \
@@ -275,7 +295,7 @@  test_expect_success 'ls-remote with filtered symref (refname)' '
 	test_cmp expect actual
 '
 
-test_expect_success 'ls-remote with filtered symref (--heads)' '
+test_expect_success 'ls-remote with filtered symref (--branches)' '
 	git symbolic-ref refs/heads/foo refs/tags/mark &&
 	cat >expect.v2 <<-EOF &&
 	ref: refs/tags/mark	refs/heads/foo
@@ -283,9 +303,9 @@  test_expect_success 'ls-remote with filtered symref (--heads)' '
 	$rev	refs/heads/main
 	EOF
 	grep -v "^ref: refs/tags/" <expect.v2 >expect.v0 &&
-	git -c protocol.version=0 ls-remote --symref --heads . >actual.v0 &&
+	git -c protocol.version=0 ls-remote --symref --branches . >actual.v0 &&
 	test_cmp expect.v0 actual.v0 &&
-	git -c protocol.version=2 ls-remote --symref --heads . >actual.v2 &&
+	git -c protocol.version=2 ls-remote --symref --branches . >actual.v2 &&
 	test_cmp expect.v2 actual.v2
 '
 
@@ -335,9 +355,9 @@  test_expect_success 'ls-remote patterns work with all protocol versions' '
 test_expect_success 'ls-remote prefixes work with all protocol versions' '
 	git for-each-ref --format="%(objectname)	%(refname)" \
 		refs/heads/ refs/tags/ >expect &&
-	git -c protocol.version=0 ls-remote --heads --tags . >actual.v0 &&
+	git -c protocol.version=0 ls-remote --branches --tags . >actual.v0 &&
 	test_cmp expect actual.v0 &&
-	git -c protocol.version=2 ls-remote --heads --tags . >actual.v2 &&
+	git -c protocol.version=2 ls-remote --branches --tags . >actual.v2 &&
 	test_cmp expect actual.v2
 '