diff mbox series

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

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

Commit Message

Junio C Hamano June 4, 2024, 10:01 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.

We do not plan to remove "--heads" or "-h" yet; we may want to do so
at Git 3.0, in which case, we may need to start advertising upcoming
removal with an extra warning when they are used.

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

Comments

Patrick Steinhardt June 6, 2024, 9:39 a.m. UTC | #1 
On Tue, Jun 04, 2024 at 03:01:44PM -0700, Junio C Hamano wrote:
> diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
> index 65fb22a8a2..69841ed49f 100644
> --- a/builtin/ls-remote.c
> +++ b/builtin/ls-remote.c
> @@ -9,7 +9,7 @@
>  #include "wildmatch.h"
>  
>  static const char * const ls_remote_usage[] = {
> -	N_("git ls-remote [--heads] [--tags] [--refs] [--upload-pack=<exec>]\n"
> +	N_("git ls-remote [--branches] [--tags] [--refs] [--upload-pack=<exec>]\n"
>  	   "              [-q | --quiet] [--exit-code] [--get-url] [--sort=<key>]\n"
>  	   "              [--symref] [<repository> [<patterns>...]]"),
>  	NULL
> @@ -68,7 +68,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_BRANCHES),
> +		OPT_BIT('b', "branches", &flags, N_("limit to branches"), REF_BRANCHES),
> +		OPT_BIT_F('h', "heads", &flags,
> +			  N_("deprecated synonym for --branches"), REF_BRANCHES,
> +			  PARSE_OPT_HIDDEN),
>  		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..42e77eb5a9 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,27 @@ 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 w/o warning' '
> +	git ls-remote -h self >actual 2>warning &&
> +	test_cmp expected.branches actual &&
> +	test_grep ! deprecated warning
> +'

It is a bit funny to grep for something that wasn't ever there. But I
don't mind it much as we may eventually want to introduce such a
deprecation warning if we ever decide to go through with the
deprecation.

Patrick
Junio C Hamano June 6, 2024, 3:18 p.m. UTC | #2 
Patrick Steinhardt <ps@pks.im> writes:

>> +test_expect_success 'ls-remote -h is deprecated w/o warning' '
>> +	git ls-remote -h self >actual 2>warning &&
>> +	test_cmp expected.branches actual &&
>> +	test_grep ! deprecated warning
>> +'
>
> It is a bit funny to grep for something that wasn't ever there. But I
> don't mind it much as we may eventually want to introduce such a
> deprecation warning if we ever decide to go through with the
> deprecation.

It is indeed funny.  We thought about adding and then made a
conscious decision not to add a deprecation warning, so this
testpiece serves as a documentation of the status quo, and a
back-pointer for readers to blame back to the message of the
commit that introduced this one, which clearly says the lack
of warning is something we may want to reconsider later.

So, this should be fine (the same for 3/3).

Thanks.
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 65fb22a8a2..69841ed49f 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -9,7 +9,7 @@ 
 #include "wildmatch.h"
 
 static const char * const ls_remote_usage[] = {
-	N_("git ls-remote [--heads] [--tags] [--refs] [--upload-pack=<exec>]\n"
+	N_("git ls-remote [--branches] [--tags] [--refs] [--upload-pack=<exec>]\n"
 	   "              [-q | --quiet] [--exit-code] [--get-url] [--sort=<key>]\n"
 	   "              [--symref] [<repository> [<patterns>...]]"),
 	NULL
@@ -68,7 +68,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_BRANCHES),
+		OPT_BIT('b', "branches", &flags, N_("limit to branches"), REF_BRANCHES),
+		OPT_BIT_F('h', "heads", &flags,
+			  N_("deprecated synonym for --branches"), REF_BRANCHES,
+			  PARSE_OPT_HIDDEN),
 		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..42e77eb5a9 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,27 @@  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 w/o warning' '
+	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 and hidden w/o warning' '
+	test_expect_code 129 git ls-remote -h >short-help &&
+	test_grep ! -e --head short-help &&
+	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 +297,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 +305,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 +357,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
 '