[PATCH 0/4] ls-remote: introduce symref argument

[PATCH 0/4] ls-remote: introduce symref argument

[Thomas Gummerer]

> I thought it might be nice for any porcelain which tries to wrap
> `ls-remote`, make some decision based on the capabilities, and then
> invoke another plumbing command. But I guess that is probably slightly
> crazy, and nobody is doing it.
> 
> Something like `ls-remote --symrefs` probably would be a better place to
> start.

Turns out adding this is pretty simple.

The first two patches are documentation, which I noticed when reading
up about the command.  Patch three is a cleanup patch, which makes
ls-remote use the parse-options api instead of the hand-rolled option
parser.  Patch four is actually adding the option.

Thomas Gummerer (4):
  ls-remote: document --quiet option
  ls-remote: fix synopsis
  ls-remote: use parse-options api
  ls-remote: add support for showing symrefs

 Documentation/git-ls-remote.txt | 12 +++++-
 builtin/ls-remote.c             | 90 +++++++++++++++++------------------------
 t/t5512-ls-remote.sh            | 20 +++++++++
 3 files changed, 68 insertions(+), 54 deletions(-)

-- 
2.7.0.14.g2b6d3d6

[PATCH 1/4] ls-remote: document --quiet option

[Thomas Gummerer]

cefb2a5e3 ("ls-remote: print URL when no repo is specified") added a
quiet option to ls-remote, but didn't add it to the documentation.  Add
it.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git-ls-remote.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index d510c05..27380de 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git ls-remote' [--heads] [--tags]  [--upload-pack=<exec>]
-	      [--exit-code] <repository> [<refs>...]
+	      [-q | --quiet] [--exit-code] <repository> [<refs>...]
 
 DESCRIPTION
 -----------
@@ -29,6 +29,10 @@ OPTIONS
 	both, references stored in refs/heads and refs/tags are
 	displayed.
 
+-q::
+--quiet::
+	Do not print remote URL to stderr.
+
 --upload-pack=<exec>::
 	Specify the full path of 'git-upload-pack' on the remote
 	host. This allows listing references from repositories accessed via
-- 
2.7.0.14.g2b6d3d6

[PATCH 2/4] ls-remote: fix synopsis

[Thomas Gummerer]

git ls-remote takes an optional get-url argument, and specifying the
repository is optional.  Fix the synopsis in the documentation to
reflect this.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git-ls-remote.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 27380de..31c1427 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git ls-remote' [--heads] [--tags]  [--upload-pack=<exec>]
-	      [-q | --quiet] [--exit-code] <repository> [<refs>...]
+	      [-q | --quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]
 
 DESCRIPTION
 -----------
-- 
2.7.0.14.g2b6d3d6

[PATCH 3/4] ls-remote: use parse-options api

[Thomas Gummerer]

Currently ls-remote uses a hand rolled parser for the its command line
arguments.  Use the parse-options api instead of the hand rolled parser
to simplify the code and make it easier to add new arguments.  In
addition this improves the help message.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 builtin/ls-remote.c | 83 +++++++++++++++++++----------------------------------
 1 file changed, 30 insertions(+), 53 deletions(-)

diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index fa65a84..6ee7b0e 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -3,9 +3,11 @@
 #include "transport.h"
 #include "remote.h"
 
-static const char ls_remote_usage[] =
-"git ls-remote [--heads] [--tags]  [--upload-pack=<exec>]\n"
-"                     [-q | --quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]";
+static const char * const ls_remote_usage[] = {
+	N_("git ls-remote [--heads] [--tags]  [--upload-pack=<exec>]\n"
+	   "                     [-q | --quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]"),
+	NULL
+};
 
 /*
  * Is there one among the list of patterns that match the tail part
@@ -30,12 +32,12 @@ static int tail_match(const char **pattern, const char *path)
 
 int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 {
-	int i;
 	const char *dest = NULL;
 	unsigned flags = 0;
 	int get_url = 0;
 	int quiet = 0;
 	int status = 0;
+	int tags = 0, heads = 0, refs = 0;
 	const char *uploadpack = NULL;
 	const char **pattern = NULL;
 
@@ -43,59 +45,34 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	struct transport *transport;
 	const struct ref *ref;
 
-	if (argc == 2 && !strcmp("-h", argv[1]))
-		usage(ls_remote_usage);
+	struct option options[] = {
+		OPT__QUIET(&quiet, N_("do not print remote URL")),
+		OPT_STRING(0, "upload-pack", &uploadpack, N_("exec"),
+			   N_("path of git-upload-pack on the remote host")),
+		OPT_STRING(0, "exec", &uploadpack, N_("exec"),
+			   N_("path of git-upload-pack on the remote host")),
+		OPT_SET_INT('t', "tags", &tags, N_("limit to tags"), REF_TAGS),
+		OPT_SET_INT('h', "heads", &heads, N_("limit to heads"), REF_HEADS),
+		OPT_SET_INT(0, "refs", &refs, N_("no magic fake tag refs"), REF_NORMAL),
+		OPT_SET_INT(0, "get-url", &get_url,
+			    N_("take url.<base>.insteadOf into account"), 1),
+		OPT_SET_INT(0, "exit-code", &status,
+			    N_("exit with exit code 2 if no matching refs are found"), 2),
+		OPT_END()
+	};
 
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
+	argc = parse_options(argc, argv, prefix, options, ls_remote_usage,
+			     PARSE_OPT_STOP_AT_NON_OPTION);
+	flags = tags | heads | refs;
+	dest = argv[0];
 
-		if (*arg == '-') {
-			if (starts_with(arg, "--upload-pack=")) {
-				uploadpack = arg + 14;
-				continue;
-			}
-			if (starts_with(arg, "--exec=")) {
-				uploadpack = arg + 7;
-				continue;
-			}
-			if (!strcmp("--tags", arg) || !strcmp("-t", arg)) {
-				flags |= REF_TAGS;
-				continue;
-			}
-			if (!strcmp("--heads", arg) || !strcmp("-h", arg)) {
-				flags |= REF_HEADS;
-				continue;
-			}
-			if (!strcmp("--refs", arg)) {
-				flags |= REF_NORMAL;
-				continue;
-			}
-			if (!strcmp("--quiet", arg) || !strcmp("-q", arg)) {
-				quiet = 1;
-				continue;
-			}
-			if (!strcmp("--get-url", arg)) {
-				get_url = 1;
-				continue;
-			}
-			if (!strcmp("--exit-code", arg)) {
-				/* return this code if no refs are reported */
-				status = 2;
-				continue;
-			}
-			usage(ls_remote_usage);
-		}
-		dest = arg;
-		i++;
-		break;
+	if (argc > 1) {
+		int i;
+		pattern = xcalloc(argc, sizeof(const char *));
+		for (i = 1; i < argc; i++)
+			pattern[i - 1] = xstrfmt("*/%s", argv[i]);
 	}
 
-	if (argv[i]) {
-		int j;
-		pattern = xcalloc(argc - i + 1, sizeof(const char *));
-		for (j = i; j < argc; j++)
-			pattern[j - i] = xstrfmt("*/%s", argv[j]);
-	}
 	remote = remote_get(dest);
 	if (!remote) {
 		if (dest)
-- 
2.7.0.14.g2b6d3d6

[PATCH 4/4] builtin/ls-remote: add support for showing symrefs

[Thomas Gummerer]

Sometimes it's useful to know the main branch of a git repository
without actually downloading the repository.  This can be done by
looking at the symrefs stored in the remote repository.  Currently git
doesn't provide a simple way to show the symrefs stored on the remote
repository, even though the information is available.  Add a --symrefs
command line argument to the ls-remote command, which shows the symrefs
on the remote repository.

Suggested-by: pedro rijo <pedrorijo91@gmail.com>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git-ls-remote.txt |  8 +++++++-
 builtin/ls-remote.c             |  9 ++++++++-
 t/t5512-ls-remote.sh            | 20 ++++++++++++++++++++
 3 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 31c1427..5efef9e 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -10,7 +10,8 @@ SYNOPSIS
 --------
 [verse]
 'git ls-remote' [--heads] [--tags]  [--upload-pack=<exec>]
-	      [-q | --quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]
+	      [-q | --quiet] [--exit-code]
+	      [--symrefs] [--get-url] [<repository> [<refs>...]]
 
 DESCRIPTION
 -----------
@@ -50,6 +51,11 @@ OPTIONS
 	"url.<base>.insteadOf" config setting (See linkgit:git-config[1]) and
 	exit without talking to the remote.
 
+--symrefs::
+	Show the symrefs on the server.  Shows only the symrefs by
+	default, and can be combined with --tags and --heads to show
+	refs/heads and refs/tags as well.
+
 <repository>::
 	The "remote" repository to query.  This parameter can be
 	either a URL or the name of a remote (see the GIT URLS and
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 6ee7b0e..f33ada9 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -5,7 +5,8 @@
 
 static const char * const ls_remote_usage[] = {
 	N_("git ls-remote [--heads] [--tags]  [--upload-pack=<exec>]\n"
-	   "                     [-q | --quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]"),
+	   "                     [-q | --quiet] [--exit-code] [--get-url]\n"
+	   "                     [--symrefs] [<repository> [<refs>...]]"),
 	NULL
 };
 
@@ -38,6 +39,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	int quiet = 0;
 	int status = 0;
 	int tags = 0, heads = 0, refs = 0;
+	int symrefs = 0;
 	const char *uploadpack = NULL;
 	const char **pattern = NULL;
 
@@ -58,6 +60,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 			    N_("take url.<base>.insteadOf into account"), 1),
 		OPT_SET_INT(0, "exit-code", &status,
 			    N_("exit with exit code 2 if no matching refs are found"), 2),
+		OPT_BOOL(0, "symrefs", &symrefs, N_("show symrefs")),
 		OPT_END()
 	};
 
@@ -98,6 +101,10 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	if (!dest && !quiet)
 		fprintf(stderr, "From %s\n", *remote->url);
 	for ( ; ref; ref = ref->next) {
+		if (symrefs && ref->symref)
+			printf("symref: %s	%s\n", ref->symref, ref->name);
+		if (symrefs && !flags)
+			continue;
 		if (!check_ref_type(ref, flags))
 			continue;
 		if (!tail_match(pattern, ref->name))
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index aadaac5..68a1429 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -163,4 +163,24 @@ test_expect_success 'overrides work between mixed transfer/upload-pack hideRefs'
 	grep refs/tags/magic actual
 '
 
+test_expect_success 'ls-remote --symrefs' '
+	cat >expect <<-EOF &&
+	symref: refs/heads/master	HEAD
+	EOF
+	git ls-remote --symrefs >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'ls-remote with symrefs and refs combined' '
+	cat >expect <<-EOF &&
+	symref: refs/heads/master	HEAD
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/HEAD
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/master
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark
+	EOF
+	git ls-remote --symrefs --refs >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.7.0.14.g2b6d3d6

[PATCH 4/4] ls-remote: add support for showing symrefs

[Thomas Gummerer]

Sometimes it's useful to know the main branch of a git repository
without actually downloading the repository.  This can be done by
looking at the symrefs stored in the remote repository.  Currently git
doesn't provide a simple way to show the symrefs stored on the remote
repository, even though the information is available.  Add a --symrefs
command line argument to the ls-remote command, which shows the symrefs
on the remote repository.

The new argument works similar to the --heads and --tags arguments.
When only --symrefs is given, only the symrefs are shown.  It can
however be combined with the --refs, --heads or --tags arguments, to
show all refs, heads, or tags as well.

Suggested-by: pedro rijo <pedrorijo91@gmail.com>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git-ls-remote.txt |  8 +++++++-
 builtin/ls-remote.c             |  9 ++++++++-
 t/t5512-ls-remote.sh            | 20 ++++++++++++++++++++
 3 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 31c1427..5b606dd 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -10,7 +10,8 @@ SYNOPSIS
 --------
 [verse]
 'git ls-remote' [--heads] [--tags]  [--upload-pack=<exec>]
-	      [-q | --quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]
+	      [-q | --quiet] [--exit-code] [--get-url]
+	      [--symrefs] [<repository> [<refs>...]]
 
 DESCRIPTION
 -----------
@@ -50,6 +51,11 @@ OPTIONS
 	"url.<base>.insteadOf" config setting (See linkgit:git-config[1]) and
 	exit without talking to the remote.
 
+--symrefs::
+	Show the symrefs on the server.  Shows only the symrefs by
+	default, and can be combined with --tags and --heads to show
+	refs/heads and refs/tags as well.
+
 <repository>::
 	The "remote" repository to query.  This parameter can be
 	either a URL or the name of a remote (see the GIT URLS and
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 6ee7b0e..f33ada9 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -5,7 +5,8 @@
 
 static const char * const ls_remote_usage[] = {
 	N_("git ls-remote [--heads] [--tags]  [--upload-pack=<exec>]\n"
-	   "                     [-q | --quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]"),
+	   "                     [-q | --quiet] [--exit-code] [--get-url]\n"
+	   "                     [--symrefs] [<repository> [<refs>...]]"),
 	NULL
 };
 
@@ -38,6 +39,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	int quiet = 0;
 	int status = 0;
 	int tags = 0, heads = 0, refs = 0;
+	int symrefs = 0;
 	const char *uploadpack = NULL;
 	const char **pattern = NULL;
 
@@ -58,6 +60,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 			    N_("take url.<base>.insteadOf into account"), 1),
 		OPT_SET_INT(0, "exit-code", &status,
 			    N_("exit with exit code 2 if no matching refs are found"), 2),
+		OPT_BOOL(0, "symrefs", &symrefs, N_("show symrefs")),
 		OPT_END()
 	};
 
@@ -98,6 +101,10 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	if (!dest && !quiet)
 		fprintf(stderr, "From %s\n", *remote->url);
 	for ( ; ref; ref = ref->next) {
+		if (symrefs && ref->symref)
+			printf("symref: %s	%s\n", ref->symref, ref->name);
+		if (symrefs && !flags)
+			continue;
 		if (!check_ref_type(ref, flags))
 			continue;
 		if (!tail_match(pattern, ref->name))
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index aadaac5..68a1429 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -163,4 +163,24 @@ test_expect_success 'overrides work between mixed transfer/upload-pack hideRefs'
 	grep refs/tags/magic actual
 '
 
+test_expect_success 'ls-remote --symrefs' '
+	cat >expect <<-EOF &&
+	symref: refs/heads/master	HEAD
+	EOF
+	git ls-remote --symrefs >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'ls-remote with symrefs and refs combined' '
+	cat >expect <<-EOF &&
+	symref: refs/heads/master	HEAD
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/HEAD
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/master
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark
+	EOF
+	git ls-remote --symrefs --refs >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.7.0.14.g2b6d3d6

Re: [PATCH 0/4] ls-remote: introduce symref argument

[Thomas Gummerer]

On 01/17, Thomas Gummerer wrote:
> > I thought it might be nice for any porcelain which tries to wrap
> > `ls-remote`, make some decision based on the capabilities, and then
> > invoke another plumbing command. But I guess that is probably slightly
> > crazy, and nobody is doing it.
> >
> > Something like `ls-remote --symrefs` probably would be a better place to
> > start.
>
> Turns out adding this is pretty simple.
>
> The first two patches are documentation, which I noticed when reading
> up about the command.  Patch three is a cleanup patch, which makes
> ls-remote use the parse-options api instead of the hand-rolled option
> parser.  Patch four is actually adding the option.

Sorry, I forgot to add --in-reply-to to git-send email.  For
reference, the thread that got me to do this patch series is at
http://thread.gmane.org/gmane.comp.version-control.git/284075.

> Thomas Gummerer (4):
>   ls-remote: document --quiet option
>   ls-remote: fix synopsis
>   ls-remote: use parse-options api
>   ls-remote: add support for showing symrefs
>
>  Documentation/git-ls-remote.txt | 12 +++++-
>  builtin/ls-remote.c             | 90 +++++++++++++++++------------------------
>  t/t5512-ls-remote.sh            | 20 +++++++++
>  3 files changed, 68 insertions(+), 54 deletions(-)
>
> --
> 2.7.0.14.g2b6d3d6
>

Re: [PATCH 4/4] builtin/ls-remote: add support for showing symrefs

[Thomas Gummerer]

On 01/17, Thomas Gummerer wrote:
> Sometimes it's useful to know the main branch of a git repository
> without actually downloading the repository.  This can be done by
> looking at the symrefs stored in the remote repository.  Currently git
> doesn't provide a simple way to show the symrefs stored on the remote
> repository, even though the information is available.  Add a --symrefs
> command line argument to the ls-remote command, which shows the symrefs
> on the remote repository.
>
> Suggested-by: pedro rijo <pedrorijo91@gmail.com>
> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
> ---

I meant to delete this, the other 4/4 has the re-worded commit
message.  Please ignore this one.

>  Documentation/git-ls-remote.txt |  8 +++++++-
>  builtin/ls-remote.c             |  9 ++++++++-
>  t/t5512-ls-remote.sh            | 20 ++++++++++++++++++++
>  3 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
> index 31c1427..5efef9e 100644
> --- a/Documentation/git-ls-remote.txt
> +++ b/Documentation/git-ls-remote.txt
> @@ -10,7 +10,8 @@ SYNOPSIS
>  --------
>  [verse]
>  'git ls-remote' [--heads] [--tags]  [--upload-pack=<exec>]
> -	      [-q | --quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]
> +	      [-q | --quiet] [--exit-code]
> +	      [--symrefs] [--get-url] [<repository> [<refs>...]]
>
>  DESCRIPTION
>  -----------
> @@ -50,6 +51,11 @@ OPTIONS
>  	"url.<base>.insteadOf" config setting (See linkgit:git-config[1]) and
>  	exit without talking to the remote.
>
> +--symrefs::
> +	Show the symrefs on the server.  Shows only the symrefs by
> +	default, and can be combined with --tags and --heads to show
> +	refs/heads and refs/tags as well.
> +
>  <repository>::
>  	The "remote" repository to query.  This parameter can be
>  	either a URL or the name of a remote (see the GIT URLS and
> diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
> index 6ee7b0e..f33ada9 100644
> --- a/builtin/ls-remote.c
> +++ b/builtin/ls-remote.c
> @@ -5,7 +5,8 @@
>
>  static const char * const ls_remote_usage[] = {
>  	N_("git ls-remote [--heads] [--tags]  [--upload-pack=<exec>]\n"
> -	   "                     [-q | --quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]"),
> +	   "                     [-q | --quiet] [--exit-code] [--get-url]\n"
> +	   "                     [--symrefs] [<repository> [<refs>...]]"),
>  	NULL
>  };
>
> @@ -38,6 +39,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
>  	int quiet = 0;
>  	int status = 0;
>  	int tags = 0, heads = 0, refs = 0;
> +	int symrefs = 0;
>  	const char *uploadpack = NULL;
>  	const char **pattern = NULL;
>
> @@ -58,6 +60,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
>  			    N_("take url.<base>.insteadOf into account"), 1),
>  		OPT_SET_INT(0, "exit-code", &status,
>  			    N_("exit with exit code 2 if no matching refs are found"), 2),
> +		OPT_BOOL(0, "symrefs", &symrefs, N_("show symrefs")),
>  		OPT_END()
>  	};
>
> @@ -98,6 +101,10 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
>  	if (!dest && !quiet)
>  		fprintf(stderr, "From %s\n", *remote->url);
>  	for ( ; ref; ref = ref->next) {
> +		if (symrefs && ref->symref)
> +			printf("symref: %s	%s\n", ref->symref, ref->name);
> +		if (symrefs && !flags)
> +			continue;
>  		if (!check_ref_type(ref, flags))
>  			continue;
>  		if (!tail_match(pattern, ref->name))
> diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
> index aadaac5..68a1429 100755
> --- a/t/t5512-ls-remote.sh
> +++ b/t/t5512-ls-remote.sh
> @@ -163,4 +163,24 @@ test_expect_success 'overrides work between mixed transfer/upload-pack hideRefs'
>  	grep refs/tags/magic actual
>  '
>
> +test_expect_success 'ls-remote --symrefs' '
> +	cat >expect <<-EOF &&
> +	symref: refs/heads/master	HEAD
> +	EOF
> +	git ls-remote --symrefs >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'ls-remote with symrefs and refs combined' '
> +	cat >expect <<-EOF &&
> +	symref: refs/heads/master	HEAD
> +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
> +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/HEAD
> +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/master
> +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark
> +	EOF
> +	git ls-remote --symrefs --refs >actual &&
> +	test_cmp expect actual
> +'
> +
>  test_done
> --
> 2.7.0.14.g2b6d3d6
>

--
Thomas Gummerer

Re: [PATCH 3/4] ls-remote: use parse-options api

[Jeff King]

On Sun, Jan 17, 2016 at 12:04:01PM +0100, Thomas Gummerer wrote:

> Currently ls-remote uses a hand rolled parser for the its command line
> arguments.  Use the parse-options api instead of the hand rolled parser
> to simplify the code and make it easier to add new arguments.  In
> addition this improves the help message.

Sounds like a good idea.

> +	int tags = 0, heads = 0, refs = 0;
> [...]
> +		OPT_SET_INT('t', "tags", &tags, N_("limit to tags"), REF_TAGS),
> +		OPT_SET_INT('h', "heads", &heads, N_("limit to heads"), REF_HEADS),
> +		OPT_SET_INT(0, "refs", &refs, N_("no magic fake tag refs"), REF_NORMAL),
> [...]
> +	flags = tags | heads | refs;

Is there any reason these can't be:

  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, "refs", &flags, N_("no magic fake tag refs"), REF_NORMAL),

to make their interaction more obvious? I wondered if there was
anything tricky going on (like some of the bits for each option
overlapping), but I didn't see anything.

> +		OPT_SET_INT(0, "refs", &refs, N_("no magic fake tag refs"), REF_NORMAL),

I imagine you took the help string from the comment in check_ref. We can
probably come up with something more descriptive for the user-facing
string. :) How about "do not show peeled tags"?

> +		OPT_STRING(0, "upload-pack", &uploadpack, N_("exec"),
> +			   N_("path of git-upload-pack on the remote host")),
> +		OPT_STRING(0, "exec", &uploadpack, N_("exec"),
> +			   N_("path of git-upload-pack on the remote host")),

Since these are redundant with each other, should we declare one
"hidden" to not appear in "-h" output?

> +		OPT_SET_INT(0, "get-url", &get_url,
> +			    N_("take url.<base>.insteadOf into account"), 1),

Should this one be OPT_BOOL? I think "--no-get-url" works either way (it
resets the variable to 0), but OPT_BOOL communicates the intent more
clearly, I think.

> +		OPT_SET_INT(0, "exit-code", &status,
> +			    N_("exit with exit code 2 if no matching refs are found"), 2),

This one can't be OPT_BOOL, obviously. What happens with
"--no-exit-code"? We'll set it back to "0", which I think is the right
thing to do. Good.

The rest of the patch looked good to me.

-Peff

Re: [PATCH 1/4] ls-remote: document --quiet option

[Jeff King]

On Sun, Jan 17, 2016 at 12:03:59PM +0100, Thomas Gummerer wrote:

> cefb2a5e3 ("ls-remote: print URL when no repo is specified") added a
> quiet option to ls-remote, but didn't add it to the documentation.  Add
> it.

Great. I love it when a patch series starts by tidying up the area.

The patch looks obviously correct. Should we do the same for "--refs",
which looks like the only other undocumented option (aside from --exec,
but I think that's just for historical compatibility).

-Peff

Re: [PATCH 4/4] ls-remote: add support for showing symrefs

[Jeff King]

On Sun, Jan 17, 2016 at 12:04:03PM +0100, Thomas Gummerer wrote:

> Sometimes it's useful to know the main branch of a git repository
> without actually downloading the repository.  This can be done by
> looking at the symrefs stored in the remote repository.  Currently git
> doesn't provide a simple way to show the symrefs stored on the remote
> repository, even though the information is available.  Add a --symrefs
> command line argument to the ls-remote command, which shows the symrefs
> on the remote repository.
> 
> The new argument works similar to the --heads and --tags arguments.
> When only --symrefs is given, only the symrefs are shown.  It can
> however be combined with the --refs, --heads or --tags arguments, to
> show all refs, heads, or tags as well.

I would have expected --symrefs to be "also show symref destinations for
refs we are showing" and not otherwise affect the set of refs we pick.
That would make:

  git ls-remote --symrefs

show everything, including symrefs and peeled tags (which I think is not
possible with your patch). It would also make:

  git ls-remote --symrefs --heads

show symrefs and refs in refs/heads, but _not_ show symrefs outside
of refs/heads.

On the flip side, though, it does not provide a way to just get the
symrefs without any other output. But I think if you just want a specific
symref (say, HEAD), you can ask for it:

  git ls-remote --symrefs $remote HEAD

This is all somewhat moot, perhaps, as the server side currently only
shares symref information for HEAD. But that may change in the future
(it's a limitation of the current protocol).

I'm also somewhat doubtful that people regularly use ls-remote much at
all these days, let alone with "--heads" or "--tags". So it's hard to
come up with concrete use cases for any of this. The above is just what
I would expect for general flexibility and consistency with other
commands.

> ---
>  Documentation/git-ls-remote.txt |  8 +++++++-
>  builtin/ls-remote.c             |  9 ++++++++-
>  t/t5512-ls-remote.sh            | 20 ++++++++++++++++++++

Documentation _and_ tests. Yay. :)

> @@ -98,6 +101,10 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
>  	if (!dest && !quiet)
>  		fprintf(stderr, "From %s\n", *remote->url);
>  	for ( ; ref; ref = ref->next) {
> +		if (symrefs && ref->symref)
> +			printf("symref: %s	%s\n", ref->symref, ref->name);

I assume that's a raw tab in the string. Please use "\t", which makes it
more obvious to readers what is going on.

Since this output is only triggered by a new option, we're not
constrained by compatibility in the output. But I think it's still a
good idea to keep the general "<content>\t<refname>" pattern set by the
other lines, as you did.

Here's my obligatory bikeshedding for the format. Feel free to ignore.

I wondered if just:

  refs/heads/master       HEAD
  1bd44cb9d13204b0fe1958db0082f5028a16eb3a       refs/heads/master

would look nicer. It is technically ambiguous if a symref can point to a
40-hex refname. They generally will start with refs/, but I'm not sure
that is strict requirement. It also makes it harder to add other output
later if we choose to. So some kind of keyword like "symref:" is a good
idea.

We could also do it as:

  ref: refs/heads/master     HEAD

which matches the symref format itself. I guess that doesn't really
matter here, but somehow it seems more aesthetically pleasing to me.

The output would look a lot nicer for humans if we right-padded the
symref destination to match the 40-hex that is on all the other lines
(so all of the refnames line up).  But that makes machine-parsing a lot
harder. We could do something clever with isatty(1), but I don't think
it's worth the effort.

> +test_expect_success 'ls-remote with symrefs and refs combined' '
> +	cat >expect <<-EOF &&
> +	symref: refs/heads/master	HEAD
> +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
> +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/HEAD
> +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/master
> +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark
> +	EOF

I expected there to be a:

  1bd44cb9d13204b0fe1958db0082f5028a16eb3a	HEAD

line. It's technically redundant, since the caller can dereference
refs/heads/master themselves, but it potentially makes things easier for
a caller. I realized why it isn't here, though. We print all symrefs,
regardless of whether they match the flags, but "HEAD" doesn't match
"--refs", so we don't show its value.  Under the semantics I proposed
above, "ls-remote --symrefs" would show both.

-Peff

Re: [PATCH 0/4] ls-remote: introduce symref argument

[Jeff King]

On Sun, Jan 17, 2016 at 12:03:58PM +0100, Thomas Gummerer wrote:

> > I thought it might be nice for any porcelain which tries to wrap
> > `ls-remote`, make some decision based on the capabilities, and then
> > invoke another plumbing command. But I guess that is probably slightly
> > crazy, and nobody is doing it.
> > 
> > Something like `ls-remote --symrefs` probably would be a better place to
> > start.
> 
> Turns out adding this is pretty simple.
> 
> The first two patches are documentation, which I noticed when reading
> up about the command.  Patch three is a cleanup patch, which makes
> ls-remote use the parse-options api instead of the hand-rolled option
> parser.  Patch four is actually adding the option.
> 
> Thomas Gummerer (4):
>   ls-remote: document --quiet option
>   ls-remote: fix synopsis
>   ls-remote: use parse-options api
>   ls-remote: add support for showing symrefs
> 
>  Documentation/git-ls-remote.txt | 12 +++++-
>  builtin/ls-remote.c             | 90 +++++++++++++++++------------------------
>  t/t5512-ls-remote.sh            | 20 +++++++++
>  3 files changed, 68 insertions(+), 54 deletions(-)

Thanks for working on this. One of my favorite things about open source
is when I realize I'm too lazy/busy to work on something, and then it
magically appears in my inbox. :)

This looks like a good start. I left a few comments on the specific
patches.

-Peff

Re: [PATCH 1/4] ls-remote: document --quiet option

[Thomas Gummerer]

On 01/17, Jeff King wrote:
> On Sun, Jan 17, 2016 at 12:03:59PM +0100, Thomas Gummerer wrote:
>
> > cefb2a5e3 ("ls-remote: print URL when no repo is specified") added a
> > quiet option to ls-remote, but didn't add it to the documentation.  Add
> > it.
>
> Great. I love it when a patch series starts by tidying up the area.
>
> The patch looks obviously correct. Should we do the same for "--refs",
> which looks like the only other undocumented option (aside from --exec,
> but I think that's just for historical compatibility).

Sounds like a good idea.  I will add a patch for that.

>
> -Peff

--
Thomas

Re: [PATCH 3/4] ls-remote: use parse-options api

[Thomas Gummerer]

On 01/17, Jeff King wrote:
> On Sun, Jan 17, 2016 at 12:04:01PM +0100, Thomas Gummerer wrote:
>
> > Currently ls-remote uses a hand rolled parser for the its command line
> > arguments.  Use the parse-options api instead of the hand rolled parser
> > to simplify the code and make it easier to add new arguments.  In
> > addition this improves the help message.
>
> Sounds like a good idea.
>
> > +	int tags = 0, heads = 0, refs = 0;
> > [...]
> > +		OPT_SET_INT('t', "tags", &tags, N_("limit to tags"), REF_TAGS),
> > +		OPT_SET_INT('h', "heads", &heads, N_("limit to heads"), REF_HEADS),
> > +		OPT_SET_INT(0, "refs", &refs, N_("no magic fake tag refs"), REF_NORMAL),
> > [...]
> > +	flags = tags | heads | refs;
>
> Is there any reason these can't be:
>
>   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, "refs", &flags, N_("no magic fake tag refs"), REF_NORMAL),
>
> to make their interaction more obvious? I wondered if there was
> anything tricky going on (like some of the bits for each option
> overlapping), but I didn't see anything.

I was looking for something like this, but totally overlooked it when
going through the docs.  Thanks, will change.

> > +		OPT_SET_INT(0, "refs", &refs, N_("no magic fake tag refs"), REF_NORMAL),
>
> I imagine you took the help string from the comment in check_ref. We can
> probably come up with something more descriptive for the user-facing
> string. :) How about "do not show peeled tags"?

Indeed, I wasn't really happy about it, but couldn't come up with
anything better.  Your version sounds much better, will fix.

> > +		OPT_STRING(0, "upload-pack", &uploadpack, N_("exec"),
> > +			   N_("path of git-upload-pack on the remote host")),
> > +		OPT_STRING(0, "exec", &uploadpack, N_("exec"),
> > +			   N_("path of git-upload-pack on the remote host")),
>
> Since these are redundant with each other, should we declare one
> "hidden" to not appear in "-h" output?

Makes sense, I'll declare the exec option as hidden, as that's the one
that's not documented anywhere else either.

> > +		OPT_SET_INT(0, "get-url", &get_url,
> > +			    N_("take url.<base>.insteadOf into account"), 1),
>
> Should this one be OPT_BOOL? I think "--no-get-url" works either way (it
> resets the variable to 0), but OPT_BOOL communicates the intent more
> clearly, I think.

Makes sense, will change in the re-roll.

>
> > +		OPT_SET_INT(0, "exit-code", &status,
> > +			    N_("exit with exit code 2 if no matching refs are found"), 2),
>
> This one can't be OPT_BOOL, obviously. What happens with
> "--no-exit-code"? We'll set it back to "0", which I think is the right
> thing to do. Good.
>
> The rest of the patch looked good to me.

Thanks for the review!

>
> -Peff

--
Thomas

Re: [PATCH 4/4] ls-remote: add support for showing symrefs

[Thomas Gummerer]

On 01/17, Jeff King wrote:
> On Sun, Jan 17, 2016 at 12:04:03PM +0100, Thomas Gummerer wrote:
>
> > Sometimes it's useful to know the main branch of a git repository
> > without actually downloading the repository.  This can be done by
> > looking at the symrefs stored in the remote repository.  Currently git
> > doesn't provide a simple way to show the symrefs stored on the remote
> > repository, even though the information is available.  Add a --symrefs
> > command line argument to the ls-remote command, which shows the symrefs
> > on the remote repository.
> >
> > The new argument works similar to the --heads and --tags arguments.
> > When only --symrefs is given, only the symrefs are shown.  It can
> > however be combined with the --refs, --heads or --tags arguments, to
> > show all refs, heads, or tags as well.
>
> I would have expected --symrefs to be "also show symref destinations for
> refs we are showing" and not otherwise affect the set of refs we pick.
> That would make:
>
>   git ls-remote --symrefs
>
> show everything, including symrefs and peeled tags (which I think is not
> possible with your patch). It would also make:
>
>   git ls-remote --symrefs --heads
>
> show symrefs and refs in refs/heads, but _not_ show symrefs outside
> of refs/heads.
>
> On the flip side, though, it does not provide a way to just get the
> symrefs without any other output.

That's why I decided to implement it this way.  However I think the
below makes sense, so I'll change the behavior in the re-roll.  In
case we really want to have only symrefs we could still introduce
something like --symrefs-only, though I'm doubtful we'll need that.

> But I think if you just want a specific
> symref (say, HEAD), you can ask for it:
>
>   git ls-remote --symrefs $remote HEAD
>
> This is all somewhat moot, perhaps, as the server side currently only
> shares symref information for HEAD. But that may change in the future
> (it's a limitation of the current protocol).
>
> I'm also somewhat doubtful that people regularly use ls-remote much at
> all these days, let alone with "--heads" or "--tags". So it's hard to
> come up with concrete use cases for any of this. The above is just what
> I would expect for general flexibility and consistency with other
> commands.
>
> > ---
> >  Documentation/git-ls-remote.txt |  8 +++++++-
> >  builtin/ls-remote.c             |  9 ++++++++-
> >  t/t5512-ls-remote.sh            | 20 ++++++++++++++++++++
>
> Documentation _and_ tests. Yay. :)
>
> > @@ -98,6 +101,10 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
> >  	if (!dest && !quiet)
> >  		fprintf(stderr, "From %s\n", *remote->url);
> >  	for ( ; ref; ref = ref->next) {
> > +		if (symrefs && ref->symref)
> > +			printf("symref: %s	%s\n", ref->symref, ref->name);
>
> I assume that's a raw tab in the string. Please use "\t", which makes it
> more obvious to readers what is going on.

Thanks, will change.

> Since this output is only triggered by a new option, we're not
> constrained by compatibility in the output. But I think it's still a
> good idea to keep the general "<content>\t<refname>" pattern set by the
> other lines, as you did.
>
> Here's my obligatory bikeshedding for the format. Feel free to ignore.
>
> I wondered if just:
>
>   refs/heads/master       HEAD
>   1bd44cb9d13204b0fe1958db0082f5028a16eb3a       refs/heads/master
>
> would look nicer. It is technically ambiguous if a symref can point to a
> 40-hex refname. They generally will start with refs/, but I'm not sure
> that is strict requirement. It also makes it harder to add other output
> later if we choose to. So some kind of keyword like "symref:" is a good
> idea.
>
> We could also do it as:
>
>   ref: refs/heads/master     HEAD
>
> which matches the symref format itself. I guess that doesn't really
> matter here, but somehow it seems more aesthetically pleasing to me.

I like this format the most so far, so unless I hear any objections
I'll do this.

> The output would look a lot nicer for humans if we right-padded the
> symref destination to match the 40-hex that is on all the other lines
> (so all of the refnames line up).  But that makes machine-parsing a lot
> harder. We could do something clever with isatty(1), but I don't think
> it's worth the effort.
>
> > +test_expect_success 'ls-remote with symrefs and refs combined' '
> > +	cat >expect <<-EOF &&
> > +	symref: refs/heads/master	HEAD
> > +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
> > +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/HEAD
> > +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/master
> > +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark
> > +	EOF
>
> I expected there to be a:
>
>   1bd44cb9d13204b0fe1958db0082f5028a16eb3a	HEAD
>
> line. It's technically redundant, since the caller can dereference
> refs/heads/master themselves, but it potentially makes things easier for
> a caller. I realized why it isn't here, though. We print all symrefs,
> regardless of whether they match the flags, but "HEAD" doesn't match
> "--refs", so we don't show its value.  Under the semantics I proposed
> above, "ls-remote --symrefs" would show both.
>
> -Peff

--
Thomas

Re: [PATCH 0/4] ls-remote: introduce symref argument

[Thomas Gummerer]

On 01/17, Jeff King wrote:
> On Sun, Jan 17, 2016 at 12:03:58PM +0100, Thomas Gummerer wrote:
>
> > > I thought it might be nice for any porcelain which tries to wrap
> > > `ls-remote`, make some decision based on the capabilities, and then
> > > invoke another plumbing command. But I guess that is probably slightly
> > > crazy, and nobody is doing it.
> > >
> > > Something like `ls-remote --symrefs` probably would be a better place to
> > > start.
> >
> > Turns out adding this is pretty simple.
> >
> > The first two patches are documentation, which I noticed when reading
> > up about the command.  Patch three is a cleanup patch, which makes
> > ls-remote use the parse-options api instead of the hand-rolled option
> > parser.  Patch four is actually adding the option.
> >
> > Thomas Gummerer (4):
> >   ls-remote: document --quiet option
> >   ls-remote: fix synopsis
> >   ls-remote: use parse-options api
> >   ls-remote: add support for showing symrefs
> >
> >  Documentation/git-ls-remote.txt | 12 +++++-
> >  builtin/ls-remote.c             | 90 +++++++++++++++++------------------------
> >  t/t5512-ls-remote.sh            | 20 +++++++++
> >  3 files changed, 68 insertions(+), 54 deletions(-)
>
> Thanks for working on this. One of my favorite things about open source
> is when I realize I'm too lazy/busy to work on something, and then it
> magically appears in my inbox. :)

:) thanks for suggesting it, and thanks for the review!

> This looks like a good start. I left a few comments on the specific
> patches.
>
> -Peff

--
Thomas

Re: [PATCH 4/4] ls-remote: add support for showing symrefs

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> We could also do it as:
>
>   ref: refs/heads/master     HEAD
>
> which matches the symref format itself. I guess that doesn't really
> matter here, but somehow it seems more aesthetically pleasing to me.

Yes, I think this should be the way to go.

> The output would look a lot nicer for humans if we right-padded the
> symref destination to match the 40-hex that is on all the other lines
> (so all of the refnames line up).  But that makes machine-parsing a lot
> harder. We could do something clever with isatty(1), but I don't think
> it's worth the effort.

And the target can be longer than 40-hex.  Don't play games with
padding.

>> +test_expect_success 'ls-remote with symrefs and refs combined' '
>> +	cat >expect <<-EOF &&
>> +	symref: refs/heads/master	HEAD
>> +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
>> +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/HEAD
>> +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/master
>> +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark
>> +	EOF
>
> I expected there to be a:
>
>   1bd44cb9d13204b0fe1958db0082f5028a16eb3a	HEAD
>
> line.

Yes, there should be, as I would imagine the most natural
interpretation of "--symrefs" by end users would be "show me ALSO
the symref information.", not "show me ONLY the symref information"
(if it were the latter we woudln't be seeing refs/tags/mark in the
above output).

I also suspect that this part of the patch is wrong (or at least
misleading):

@@ -98,6 +101,10 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	if (!dest && !quiet)
 		fprintf(stderr, "From %s\n", *remote->url);
 	for ( ; ref; ref = ref->next) {
+		if (symrefs && ref->symref)
+			printf("symref: %s	%s\n", ref->symref, ref->name);
+		if (symrefs && !flags)
+			continue;
 		if (!check_ref_type(ref, flags))
 			continue;
 		if (!tail_match(pattern, ref->name))

It looks wrong that the usual filtering with check_ref_type() and
tail_match() are bypassed for symbolic refs.  Even though the server
side currently feeds reflog information for only "HEAD", the code
should be prepared to do the sane thing in a future in which that
server-side limitation is corrected, and allow the user to ask

    ls-remote $there --symref refs/remotes/origin/HEAD

to learn the information on one specific ref, without having to see
the primary branch of $there repository, for example.

Thanks.

Re: [PATCH 0/4] ls-remote: introduce symref argument

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Thanks for working on this. One of my favorite things about open source
> is when I realize I'm too lazy/busy to work on something, and then it
> magically appears in my inbox. :)

;-) Once you suggest guess_remote_head(), 50% of the necessary
brainwork for this topic is already done, so you should take as much
credit, too.

Thanks, both.  Lookikng forward to a reroll.

[PATCH v2 0/5] ls-remote: introduce symrefs argument

[Thomas Gummerer]

The previous round is at $gmane/284248.  Thanks to Peff an Junio for
comments on the previous round.

Changes from the previous round:
 - added patch documenting the --refs option
 - addressed peffs comments on the parse-option patch
 - the symrefs format now uses only ref: as an indicator for a symref
 - symrefs are now shown in addition to the other refs, instead of
   replacing them in the output.
 - symrefs are now filtered by the same rules as other refs.

Interdiff below:

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 5b606dd..9356df2 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]  [--upload-pack=<exec>]
+'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=<exec>]
 	      [-q | --quiet] [--exit-code] [--get-url]
 	      [--symrefs] [<repository> [<refs>...]]
 
@@ -30,6 +30,10 @@ OPTIONS
 	both, references stored in refs/heads and refs/tags are
 	displayed.
 
+--refs::
+	Do not show peeled tags or pseudo-refs like HEAD or MERGE_HEAD
+	in the output.
+
 -q::
 --quiet::
 	Do not print remote URL to stderr.
@@ -52,9 +56,7 @@ OPTIONS
 	exit without talking to the remote.
 
 --symrefs::
-	Show the symrefs on the server.  Shows only the symrefs by
-	default, and can be combined with --tags and --heads to show
-	refs/heads and refs/tags as well.
+	Show the symrefs in addition to the other refs.
 
 <repository>::
 	The "remote" repository to query.  This parameter can be
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index f33ada9..ea73d53 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -4,7 +4,7 @@
 #include "remote.h"
 
 static const char * const ls_remote_usage[] = {
-	N_("git ls-remote [--heads] [--tags]  [--upload-pack=<exec>]\n"
+	N_("git ls-remote [--heads] [--tags] [--refs] [--upload-pack=<exec>]\n"
 	   "                     [-q | --quiet] [--exit-code] [--get-url]\n"
 	   "                     [--symrefs] [<repository> [<refs>...]]"),
 	NULL
@@ -38,7 +38,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	int get_url = 0;
 	int quiet = 0;
 	int status = 0;
-	int tags = 0, heads = 0, refs = 0;
 	int symrefs = 0;
 	const char *uploadpack = NULL;
 	const char **pattern = NULL;
@@ -51,13 +50,14 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 		OPT__QUIET(&quiet, N_("do not print remote URL")),
 		OPT_STRING(0, "upload-pack", &uploadpack, N_("exec"),
 			   N_("path of git-upload-pack on the remote host")),
-		OPT_STRING(0, "exec", &uploadpack, N_("exec"),
-			   N_("path of git-upload-pack on the remote host")),
-		OPT_SET_INT('t', "tags", &tags, N_("limit to tags"), REF_TAGS),
-		OPT_SET_INT('h', "heads", &heads, N_("limit to heads"), REF_HEADS),
-		OPT_SET_INT(0, "refs", &refs, N_("no magic fake tag refs"), REF_NORMAL),
-		OPT_SET_INT(0, "get-url", &get_url,
-			    N_("take url.<base>.insteadOf into account"), 1),
+		{ OPTION_STRING, 0, "exec", &uploadpack, N_("exec"),
+			   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, "refs", &flags, N_("do not show peeled tags"), REF_NORMAL),
+		OPT_BOOL(0, "get-url", &get_url,
+			 N_("take url.<base>.insteadOf into account")),
 		OPT_SET_INT(0, "exit-code", &status,
 			    N_("exit with exit code 2 if no matching refs are found"), 2),
 		OPT_BOOL(0, "symrefs", &symrefs, N_("show symrefs")),
@@ -66,7 +66,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, prefix, options, ls_remote_usage,
 			     PARSE_OPT_STOP_AT_NON_OPTION);
-	flags = tags | heads | refs;
 	dest = argv[0];
 
 	if (argc > 1) {
@@ -101,15 +100,13 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	if (!dest && !quiet)
 		fprintf(stderr, "From %s\n", *remote->url);
 	for ( ; ref; ref = ref->next) {
-		if (symrefs && ref->symref)
-			printf("symref: %s	%s\n", ref->symref, ref->name);
-		if (symrefs && !flags)
-			continue;
 		if (!check_ref_type(ref, flags))
 			continue;
 		if (!tail_match(pattern, ref->name))
 			continue;
-		printf("%s	%s\n", oid_to_hex(&ref->old_oid), ref->name);
+		if (symrefs && ref->symref)
+			printf("ref: %s\t%s\n", ref->symref, ref->name);
+		printf("%s\t%s\n", oid_to_hex(&ref->old_oid), ref->name);
 		status = 0; /* we found something */
 	}
 	return status;
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 68a1429..3edbc9e 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -165,21 +165,23 @@ test_expect_success 'overrides work between mixed transfer/upload-pack hideRefs'
 
 test_expect_success 'ls-remote --symrefs' '
 	cat >expect <<-EOF &&
-	symref: refs/heads/master	HEAD
+	ref: refs/heads/master	HEAD
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	HEAD
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/HEAD
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/master
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark
 	EOF
 	git ls-remote --symrefs >actual &&
 	test_cmp expect actual
 '
 
-test_expect_success 'ls-remote with symrefs and refs combined' '
+test_expect_success 'ls-remote with filtered symrefs' '
 	cat >expect <<-EOF &&
-	symref: refs/heads/master	HEAD
-	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
-	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/HEAD
-	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/master
-	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark
+	ref: refs/heads/master	HEAD
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	HEAD
 	EOF
-	git ls-remote --symrefs --refs >actual &&
+	git ls-remote --symrefs . HEAD >actual &&
 	test_cmp expect actual
 '
 
Thomas Gummerer (5):
  ls-remote: document --quiet option
  ls-remote: document --refs option
  ls-remote: fix synopsis
  ls-remote: use parse-options api
  ls-remote: add support for showing symrefs

 Documentation/git-ls-remote.txt | 16 +++++++-
 builtin/ls-remote.c             | 89 ++++++++++++++++-------------------------
 t/t5512-ls-remote.sh            | 22 ++++++++++
 3 files changed, 71 insertions(+), 56 deletions(-)

-- 
2.7.0.30.gd0a78e9.dirty

[PATCH v2 1/5] ls-remote: document --quiet option

[Thomas Gummerer]

cefb2a5e3 ("ls-remote: print URL when no repo is specified") added a
quiet option to ls-remote, but didn't add it to the documentation.  Add
it.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git-ls-remote.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index d510c05..27380de 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git ls-remote' [--heads] [--tags]  [--upload-pack=<exec>]
-	      [--exit-code] <repository> [<refs>...]
+	      [-q | --quiet] [--exit-code] <repository> [<refs>...]
 
 DESCRIPTION
 -----------
@@ -29,6 +29,10 @@ OPTIONS
 	both, references stored in refs/heads and refs/tags are
 	displayed.
 
+-q::
+--quiet::
+	Do not print remote URL to stderr.
+
 --upload-pack=<exec>::
 	Specify the full path of 'git-upload-pack' on the remote
 	host. This allows listing references from repositories accessed via
-- 
2.7.0.30.gd0a78e9.dirty

[PATCH v2 2/5] ls-remote: document --refs option

[Thomas Gummerer]

The --refs option was originally introduced in 2718ff0 ("Improve
git-peek-remote").  The ls-remote command was first documented in
972b6fe ("ls-remote: drop storing operation and add documentation."),
but the --refs option was never documented.  Fix this.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git-ls-remote.txt | 6 +++++-
 builtin/ls-remote.c             | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 27380de..5cd47c0 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]  [--upload-pack=<exec>]
+'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=<exec>]
 	      [-q | --quiet] [--exit-code] <repository> [<refs>...]
 
 DESCRIPTION
@@ -29,6 +29,10 @@ OPTIONS
 	both, references stored in refs/heads and refs/tags are
 	displayed.
 
+--refs::
+	Do not show peeled tags or pseudo-refs like HEAD or MERGE_HEAD
+	in the output.
+
 -q::
 --quiet::
 	Do not print remote URL to stderr.
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index fa65a84..db21e52 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -4,7 +4,7 @@
 #include "remote.h"
 
 static const char ls_remote_usage[] =
-"git ls-remote [--heads] [--tags]  [--upload-pack=<exec>]\n"
+"git ls-remote [--heads] [--tags] [--refs] [--upload-pack=<exec>]\n"
 "                     [-q | --quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]";
 
 /*
-- 
2.7.0.30.gd0a78e9.dirty

[PATCH v2 3/5] ls-remote: fix synopsis

[Thomas Gummerer]

git ls-remote takes an optional get-url argument, and specifying the
repository is optional.  Fix the synopsis in the documentation to
reflect this.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git-ls-remote.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 5cd47c0..f7d1091 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=<exec>]
-	      [-q | --quiet] [--exit-code] <repository> [<refs>...]
+	      [-q | --quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]
 
 DESCRIPTION
 -----------
-- 
2.7.0.30.gd0a78e9.dirty

[PATCH v2 4/5] ls-remote: use parse-options api

[Thomas Gummerer]

Currently ls-remote uses a hand rolled parser for the its command line
arguments.  Use the parse-options api instead of the hand rolled parser
to simplify the code and make it easier to add new arguments.  In
addition this improves the help message.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 builtin/ls-remote.c | 82 +++++++++++++++++++----------------------------------
 1 file changed, 29 insertions(+), 53 deletions(-)

diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index db21e52..3a20378 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -3,9 +3,11 @@
 #include "transport.h"
 #include "remote.h"
 
-static const char ls_remote_usage[] =
-"git ls-remote [--heads] [--tags] [--refs] [--upload-pack=<exec>]\n"
-"                     [-q | --quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]";
+static const char * const ls_remote_usage[] = {
+	N_("git ls-remote [--heads] [--tags] [--refs] [--upload-pack=<exec>]\n"
+	   "                     [-q | --quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]"),
+	NULL
+};
 
 /*
  * Is there one among the list of patterns that match the tail part
@@ -30,7 +32,6 @@ static int tail_match(const char **pattern, const char *path)
 
 int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 {
-	int i;
 	const char *dest = NULL;
 	unsigned flags = 0;
 	int get_url = 0;
@@ -43,59 +44,34 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	struct transport *transport;
 	const struct ref *ref;
 
-	if (argc == 2 && !strcmp("-h", argv[1]))
-		usage(ls_remote_usage);
+	struct option options[] = {
+		OPT__QUIET(&quiet, N_("do not print remote URL")),
+		OPT_STRING(0, "upload-pack", &uploadpack, N_("exec"),
+			   N_("path of git-upload-pack on the remote host")),
+		{ OPTION_STRING, 0, "exec", &uploadpack, N_("exec"),
+			   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, "refs", &flags, N_("do not show peeled tags"), REF_NORMAL),
+		OPT_BOOL(0, "get-url", &get_url,
+			 N_("take url.<base>.insteadOf into account")),
+		OPT_SET_INT(0, "exit-code", &status,
+			    N_("exit with exit code 2 if no matching refs are found"), 2),
+		OPT_END()
+	};
 
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
+	argc = parse_options(argc, argv, prefix, options, ls_remote_usage,
+			     PARSE_OPT_STOP_AT_NON_OPTION);
+	dest = argv[0];
 
-		if (*arg == '-') {
-			if (starts_with(arg, "--upload-pack=")) {
-				uploadpack = arg + 14;
-				continue;
-			}
-			if (starts_with(arg, "--exec=")) {
-				uploadpack = arg + 7;
-				continue;
-			}
-			if (!strcmp("--tags", arg) || !strcmp("-t", arg)) {
-				flags |= REF_TAGS;
-				continue;
-			}
-			if (!strcmp("--heads", arg) || !strcmp("-h", arg)) {
-				flags |= REF_HEADS;
-				continue;
-			}
-			if (!strcmp("--refs", arg)) {
-				flags |= REF_NORMAL;
-				continue;
-			}
-			if (!strcmp("--quiet", arg) || !strcmp("-q", arg)) {
-				quiet = 1;
-				continue;
-			}
-			if (!strcmp("--get-url", arg)) {
-				get_url = 1;
-				continue;
-			}
-			if (!strcmp("--exit-code", arg)) {
-				/* return this code if no refs are reported */
-				status = 2;
-				continue;
-			}
-			usage(ls_remote_usage);
-		}
-		dest = arg;
-		i++;
-		break;
+	if (argc > 1) {
+		int i;
+		pattern = xcalloc(argc, sizeof(const char *));
+		for (i = 1; i < argc; i++)
+			pattern[i - 1] = xstrfmt("*/%s", argv[i]);
 	}
 
-	if (argv[i]) {
-		int j;
-		pattern = xcalloc(argc - i + 1, sizeof(const char *));
-		for (j = i; j < argc; j++)
-			pattern[j - i] = xstrfmt("*/%s", argv[j]);
-	}
 	remote = remote_get(dest);
 	if (!remote) {
 		if (dest)
-- 
2.7.0.30.gd0a78e9.dirty

[PATCH v2 5/5] ls-remote: add support for showing symrefs

[Thomas Gummerer]

Sometimes it's useful to know the main branch of a git repository
without actually downloading the repository.  This can be done by
looking at the symrefs stored in the remote repository.  Currently git
doesn't provide a simple way to show the symrefs stored on the remote
repository, even though the information is available.  Add a --symrefs
command line argument to the ls-remote command, which shows the symrefs
on the remote repository.

The new argument works similar to the --heads and --tags arguments.
When only --symrefs is given, only the symrefs are shown.  It can
however be combined with the --refs, --heads or --tags arguments, to
show all refs, heads, or tags as well.

While there, replace a literal tab in the format string with \t to make
it more obvious to the reader.

Suggested-by: pedro rijo <pedrorijo91@gmail.com>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git-ls-remote.txt |  6 +++++-
 builtin/ls-remote.c             |  9 +++++++--
 t/t5512-ls-remote.sh            | 22 ++++++++++++++++++++++
 3 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index f7d1091..9356df2 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -10,7 +10,8 @@ SYNOPSIS
 --------
 [verse]
 'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=<exec>]
-	      [-q | --quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]
+	      [-q | --quiet] [--exit-code] [--get-url]
+	      [--symrefs] [<repository> [<refs>...]]
 
 DESCRIPTION
 -----------
@@ -54,6 +55,9 @@ OPTIONS
 	"url.<base>.insteadOf" config setting (See linkgit:git-config[1]) and
 	exit without talking to the remote.
 
+--symrefs::
+	Show the symrefs in addition to the other refs.
+
 <repository>::
 	The "remote" repository to query.  This parameter can be
 	either a URL or the name of a remote (see the GIT URLS and
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 3a20378..ea73d53 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -5,7 +5,8 @@
 
 static const char * const ls_remote_usage[] = {
 	N_("git ls-remote [--heads] [--tags] [--refs] [--upload-pack=<exec>]\n"
-	   "                     [-q | --quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]"),
+	   "                     [-q | --quiet] [--exit-code] [--get-url]\n"
+	   "                     [--symrefs] [<repository> [<refs>...]]"),
 	NULL
 };
 
@@ -37,6 +38,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	int get_url = 0;
 	int quiet = 0;
 	int status = 0;
+	int symrefs = 0;
 	const char *uploadpack = NULL;
 	const char **pattern = NULL;
 
@@ -58,6 +60,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 			 N_("take url.<base>.insteadOf into account")),
 		OPT_SET_INT(0, "exit-code", &status,
 			    N_("exit with exit code 2 if no matching refs are found"), 2),
+		OPT_BOOL(0, "symrefs", &symrefs, N_("show symrefs")),
 		OPT_END()
 	};
 
@@ -101,7 +104,9 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 			continue;
 		if (!tail_match(pattern, ref->name))
 			continue;
-		printf("%s	%s\n", oid_to_hex(&ref->old_oid), ref->name);
+		if (symrefs && ref->symref)
+			printf("ref: %s\t%s\n", ref->symref, ref->name);
+		printf("%s\t%s\n", oid_to_hex(&ref->old_oid), ref->name);
 		status = 0; /* we found something */
 	}
 	return status;
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index aadaac5..3edbc9e 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -163,4 +163,26 @@ test_expect_success 'overrides work between mixed transfer/upload-pack hideRefs'
 	grep refs/tags/magic actual
 '
 
+test_expect_success 'ls-remote --symrefs' '
+	cat >expect <<-EOF &&
+	ref: refs/heads/master	HEAD
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	HEAD
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/HEAD
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/master
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark
+	EOF
+	git ls-remote --symrefs >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'ls-remote with filtered symrefs' '
+	cat >expect <<-EOF &&
+	ref: refs/heads/master	HEAD
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	HEAD
+	EOF
+	git ls-remote --symrefs . HEAD >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.7.0.30.gd0a78e9.dirty

Re: [PATCH v2 2/5] ls-remote: document --refs option

[Jeff King]

On Mon, Jan 18, 2016 at 05:57:15PM +0100, Thomas Gummerer wrote:

> +--refs::
> +	Do not show peeled tags or pseudo-refs like HEAD or MERGE_HEAD
> +	in the output.
> +

Minor nit: we show whatever the other side sends us, which is the refs,
HEAD, and peeled tags. So mentioning MERGE_HEAD isn't wrong (if the
server _did_ send it to us, we would omit it), but it is a bit
misleading.

I think saying "pseudo-refs like HEAD" is OK; even though we know it is
only HEAD in the current server implementation, it better describes what
the client side is doing.

-Peff

Re: [PATCH v2 4/5] ls-remote: use parse-options api

[Jeff King]

On Mon, Jan 18, 2016 at 05:57:17PM +0100, Thomas Gummerer wrote:

> Currently ls-remote uses a hand rolled parser for the its command line

s/the its/its/

> arguments.  Use the parse-options api instead of the hand rolled parser
> to simplify the code and make it easier to add new arguments.  In
> addition this improves the help message.
> 
> Helped-by: Jeff King <peff@peff.net>
> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
> ---
>  builtin/ls-remote.c | 82 +++++++++++++++++++----------------------------------
>  1 file changed, 29 insertions(+), 53 deletions(-)

Patch itself looks good to me.

-Peff

Re: [PATCH v2 5/5] ls-remote: add support for showing symrefs

[Jeff King]

On Mon, Jan 18, 2016 at 05:57:18PM +0100, Thomas Gummerer wrote:

> While there, replace a literal tab in the format string with \t to make
> it more obvious to the reader.

Heh, I didn't notice in the first review that you actually inherited
that from the original. Definitely worth doing, IMHO.

> @@ -101,7 +104,9 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
>  			continue;
>  		if (!tail_match(pattern, ref->name))
>  			continue;
> -		printf("%s	%s\n", oid_to_hex(&ref->old_oid), ref->name);
> +		if (symrefs && ref->symref)
> +			printf("ref: %s\t%s\n", ref->symref, ref->name);
> +		printf("%s\t%s\n", oid_to_hex(&ref->old_oid), ref->name);
>  		status = 0; /* we found something */

Yeah, this looks like the right logic to me.

> +test_expect_success 'ls-remote --symrefs' '
> +	cat >expect <<-EOF &&

Please use "<<-\EOF" here (and in the test below) to prevent
interpolation. It's not wrong in your case, but it's easier for a reader
(or somebody who later modifies the test) to not have to wonder what you
were expecting to be expanded. So as a general style, we quote our
here-doc markers.

> +	ref: refs/heads/master	HEAD
> +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	HEAD
> +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
> +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/HEAD
> +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/master
> +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark
> +	EOF
> +	git ls-remote --symrefs >actual &&
> +	test_cmp expect actual
> +'

This test covers "symrefs, along with everything". And this one:

> +test_expect_success 'ls-remote with filtered symrefs' '
> +	cat >expect <<-EOF &&
> +	ref: refs/heads/master	HEAD
> +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	HEAD
> +	EOF
> +	git ls-remote --symrefs . HEAD >actual &&
> +	test_cmp expect actual
> +'

covers symrefs plus a refname filter. It would be nice to also test that
"git ls-remote --symrefs --heads" shows "refs/heads/foo" as a symref.
But that cannot work with the current code, because upload-pack only
tells us about the symref HEAD, and not any others.

This may change in the future, though. I'm not sure if it's worth
squashing in the expect_failure test below. The "negative" one below
that does tell us something, though it is somewhat redundant (it does
catch the "always show symrefs" logic from your original version, but
it seems unlikely that would pop up as a regression).

---
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 3edbc9e..92fc7e9 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -176,7 +176,7 @@ test_expect_success 'ls-remote --symrefs' '
 	test_cmp expect actual
 '
 
-test_expect_success 'ls-remote with filtered symrefs' '
+test_expect_success 'ls-remote with filtered symrefs (refname)' '
 	cat >expect <<-EOF &&
 	ref: refs/heads/master	HEAD
 	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	HEAD
@@ -185,4 +185,27 @@ test_expect_success 'ls-remote with filtered symrefs' '
 	test_cmp expect actual
 '
 
+test_expect_failure 'ls-remote with filtered symrefs (--heads)' '
+	git symbolic-ref refs/heads/foo refs/tags/mark &&
+	cat >expect <<-\EOF &&
+	ref: refs/heads/bar	refs/tags/mark
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/foo
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
+	EOF
+	git ls-remote --symrefs --heads . >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'ls-remote --symrefs omits filtered-out matches' '
+	cat >expect <<-\EOF &&
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/foo
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
+	EOF
+	git ls-remote --symrefs --heads . >actual &&
+	test_cmp expect actual &&
+	git ls-remote --symrefs . "refs/heads/*" >actual &&
+	test_cmp expect actual
+'
+
+
 test_done

Re: [PATCH v2 5/5] ls-remote: add support for showing symrefs

[Jeff King]

On Mon, Jan 18, 2016 at 02:51:59PM -0500, Jeff King wrote:

> It would be nice to also test that
> "git ls-remote --symrefs --heads" shows "refs/heads/foo" as a symref.
> But that cannot work with the current code, because upload-pack only
> tells us about the symref HEAD, and not any others.

Actually, I wonder if it is worth making a note of that in the new
"--symref" documentation, so people do not report it is a bug that
"ls-remote" does not show it. :)

-Peff

Re: [PATCH v2 0/5] ls-remote: introduce symrefs argument

[Jeff King]

On Mon, Jan 18, 2016 at 05:57:13PM +0100, Thomas Gummerer wrote:

> The previous round is at $gmane/284248.  Thanks to Peff an Junio for
> comments on the previous round.

Thanks. Aside from a few minor nits, this whole thing looks good to me.

-Peff

Re: [PATCH v2 2/5] ls-remote: document --refs option

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Mon, Jan 18, 2016 at 05:57:15PM +0100, Thomas Gummerer wrote:
>
>> +--refs::
>> +	Do not show peeled tags or pseudo-refs like HEAD or MERGE_HEAD
>> +	in the output.
>> +
>
> Minor nit: we show whatever the other side sends us, which is the refs,
> HEAD, and peeled tags. So mentioning MERGE_HEAD isn't wrong (if the
> server _did_ send it to us, we would omit it), but it is a bit
> misleading.
>
> I think saying "pseudo-refs like HEAD" is OK; even though we know it is
> only HEAD in the current server implementation, it better describes what
> the client side is doing.

Good point, I think.  We probably would want to fix the glossary
entry of the word that is not spelled with a dash in between the
two word components (or update the above to spell it as a single
word).

Re: [PATCH v2 5/5] ls-remote: add support for showing symrefs

[Junio C Hamano]

Thomas Gummerer <t.gummerer@gmail.com> writes:

> +--symrefs::
> +	Show the symrefs in addition to the other refs.
> +

Micronit.  The above sounds as if the output would not talk about
HEAD at all unless this option is given, which is not what you meant
here.  "In addition to the object pointed by it, show the underlying
ref pointed by it when showing a symbolic ref" or something like that,
perhaps?

> @@ -58,6 +60,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
>  			 N_("take url.<base>.insteadOf into account")),
>  		OPT_SET_INT(0, "exit-code", &status,
>  			    N_("exit with exit code 2 if no matching refs are found"), 2),
> +		OPT_BOOL(0, "symrefs", &symrefs, N_("show symrefs")),

Likewise.

By the way, unlike "--heads" and "--tags", which is to choose a
group of refs to be shown, this controls one specific aspect,
i.e. target of symbolic ref, of each thing that is being shown,
without affecting the set of refs that the command talks about.

Shouldn't this option be "--symref" (and variable named as
show_symref_target or something more explicit)?

Other than these nits, the code itself looks good.

Thanks.

Re: [PATCH v2 2/5] ls-remote: document --refs option

[Thomas Gummerer]

On 01/18, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > On Mon, Jan 18, 2016 at 05:57:15PM +0100, Thomas Gummerer wrote:
> >
> >> +--refs::
> >> +	Do not show peeled tags or pseudo-refs like HEAD or MERGE_HEAD
> >> +	in the output.
> >> +
> >
> > Minor nit: we show whatever the other side sends us, which is the refs,
> > HEAD, and peeled tags. So mentioning MERGE_HEAD isn't wrong (if the
> > server _did_ send it to us, we would omit it), but it is a bit
> > misleading.
> >
> > I think saying "pseudo-refs like HEAD" is OK; even though we know it is
> > only HEAD in the current server implementation, it better describes what
> > the client side is doing.

Makes sense, will change.

>
> Good point, I think.  We probably would want to fix the glossary
> entry of the word that is not spelled with a dash in between the
> two word components (or update the above to spell it as a single
> word).
>

I'll update to spell it as a single word, as that seems to be more in
line with how it was used throughout the history.

$ git log | grep pseudo-ref | wc -l
2
$ git log | grep pseudoref | wc -l
18

Thanks, both.

--
Thomas

Re: [PATCH v2 5/5] ls-remote: add support for showing symrefs

[Thomas Gummerer]

On 01/18, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
>
> > +--symrefs::
> > +	Show the symrefs in addition to the other refs.
> > +
>
> Micronit.  The above sounds as if the output would not talk about
> HEAD at all unless this option is given, which is not what you meant
> here.  "In addition to the object pointed by it, show the underlying
> ref pointed by it when showing a symbolic ref" or something like that,
> perhaps?

Thanks, that sounds better, will change.

> > @@ -58,6 +60,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
> >  			 N_("take url.<base>.insteadOf into account")),
> >  		OPT_SET_INT(0, "exit-code", &status,
> >  			    N_("exit with exit code 2 if no matching refs are found"), 2),
> > +		OPT_BOOL(0, "symrefs", &symrefs, N_("show symrefs")),
>
> Likewise.
>
> By the way, unlike "--heads" and "--tags", which is to choose a
> group of refs to be shown, this controls one specific aspect,
> i.e. target of symbolic ref, of each thing that is being shown,
> without affecting the set of refs that the command talks about.
>
> Shouldn't this option be "--symref" (and variable named as
> show_symref_target or something more explicit)?

The change of the variable name definitely makes sense.  My thoughts
were that potentially multiple symrefs are shown, so the plural would
make sense, but after reading your explanation I think I agree with
using --symref as the name.  Thanks.

> Other than these nits, the code itself looks good.
>
> Thanks.
>

--
Thomas

Re: [PATCH v2 5/5] ls-remote: add support for showing symrefs

[Thomas Gummerer]

On 01/18, Jeff King wrote:
> On Mon, Jan 18, 2016 at 05:57:18PM +0100, Thomas Gummerer wrote:
>
> > While there, replace a literal tab in the format string with \t to make
> > it more obvious to the reader.
>
> Heh, I didn't notice in the first review that you actually inherited
> that from the original. Definitely worth doing, IMHO.
>
> > @@ -101,7 +104,9 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
> >  			continue;
> >  		if (!tail_match(pattern, ref->name))
> >  			continue;
> > -		printf("%s	%s\n", oid_to_hex(&ref->old_oid), ref->name);
> > +		if (symrefs && ref->symref)
> > +			printf("ref: %s\t%s\n", ref->symref, ref->name);
> > +		printf("%s\t%s\n", oid_to_hex(&ref->old_oid), ref->name);
> >  		status = 0; /* we found something */
>
> Yeah, this looks like the right logic to me.
>
> > +test_expect_success 'ls-remote --symrefs' '
> > +	cat >expect <<-EOF &&
>
> Please use "<<-\EOF" here (and in the test below) to prevent
> interpolation. It's not wrong in your case, but it's easier for a reader
> (or somebody who later modifies the test) to not have to wonder what you
> were expecting to be expanded. So as a general style, we quote our
> here-doc markers.

Will do, thanks!

> > +	ref: refs/heads/master	HEAD
> > +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	HEAD
> > +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
> > +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/HEAD
> > +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/master
> > +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark
> > +	EOF
> > +	git ls-remote --symrefs >actual &&
> > +	test_cmp expect actual
> > +'
>
> This test covers "symrefs, along with everything". And this one:
>
> > +test_expect_success 'ls-remote with filtered symrefs' '
> > +	cat >expect <<-EOF &&
> > +	ref: refs/heads/master	HEAD
> > +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	HEAD
> > +	EOF
> > +	git ls-remote --symrefs . HEAD >actual &&
> > +	test_cmp expect actual
> > +'
>
> covers symrefs plus a refname filter. It would be nice to also test that
> "git ls-remote --symrefs --heads" shows "refs/heads/foo" as a symref.
> But that cannot work with the current code, because upload-pack only
> tells us about the symref HEAD, and not any others.
>
> This may change in the future, though. I'm not sure if it's worth
> squashing in the expect_failure test below. The "negative" one below
> that does tell us something, though it is somewhat redundant (it does
> catch the "always show symrefs" logic from your original version, but
> it seems unlikely that would pop up as a regression).
>
> ---
> diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
> index 3edbc9e..92fc7e9 100755
> --- a/t/t5512-ls-remote.sh
> +++ b/t/t5512-ls-remote.sh
> @@ -176,7 +176,7 @@ test_expect_success 'ls-remote --symrefs' '
>  	test_cmp expect actual
>  '
>
> -test_expect_success 'ls-remote with filtered symrefs' '
> +test_expect_success 'ls-remote with filtered symrefs (refname)' '
>  	cat >expect <<-EOF &&
>  	ref: refs/heads/master	HEAD
>  	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	HEAD
> @@ -185,4 +185,27 @@ test_expect_success 'ls-remote with filtered symrefs' '
>  	test_cmp expect actual
>  '
>
> +test_expect_failure 'ls-remote with filtered symrefs (--heads)' '
> +	git symbolic-ref refs/heads/foo refs/tags/mark &&
> +	cat >expect <<-\EOF &&
> +	ref: refs/heads/bar	refs/tags/mark
> +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/foo
> +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
> +	EOF
> +	git ls-remote --symrefs --heads . >actual &&
> +	test_cmp expect actual
> +'

I'm a bit confused by this.  Shouldn't the "ref: refs/heads/bar
refs/tags/mark" line only show up when we use --tags, not --heads?
Also should refs/heads/bar be refs/heads/foo?

> +
> +test_expect_success 'ls-remote --symrefs omits filtered-out matches' '
> +	cat >expect <<-\EOF &&
> +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/foo
> +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
> +	EOF
> +	git ls-remote --symrefs --heads . >actual &&
> +	test_cmp expect actual &&
> +	git ls-remote --symrefs . "refs/heads/*" >actual &&
> +	test_cmp expect actual
> +'
> +
> +
>  test_done
>

--
Thomas

Re: [PATCH v2 5/5] ls-remote: add support for showing symrefs

[Thomas Gummerer]

On 01/18, Jeff King wrote:
> On Mon, Jan 18, 2016 at 02:51:59PM -0500, Jeff King wrote:
>
> > It would be nice to also test that
> > "git ls-remote --symrefs --heads" shows "refs/heads/foo" as a symref.
> > But that cannot work with the current code, because upload-pack only
> > tells us about the symref HEAD, and not any others.
>
> Actually, I wonder if it is worth making a note of that in the new
> "--symref" documentation, so people do not report it is a bug that
> "ls-remote" does not show it. :)

I agree, I'll add that.  Thanks.

> -Peff

--
Thomas

Re: [PATCH v2 5/5] ls-remote: add support for showing symrefs

[Jeff King]

On Mon, Jan 18, 2016 at 11:09:13PM +0100, Thomas Gummerer wrote:

> > +test_expect_failure 'ls-remote with filtered symrefs (--heads)' '
> > +	git symbolic-ref refs/heads/foo refs/tags/mark &&
> > +	cat >expect <<-\EOF &&
> > +	ref: refs/heads/bar	refs/tags/mark
> > +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/foo
> > +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
> > +	EOF
> > +	git ls-remote --symrefs --heads . >actual &&
> > +	test_cmp expect actual
> > +'
> 
> I'm a bit confused by this.  Shouldn't the "ref: refs/heads/bar
> refs/tags/mark" line only show up when we use --tags, not --heads?
> Also should refs/heads/bar be refs/heads/foo?

Yes, sorry, I bungled this. It should expect:

  ref: refs/tags/mark\trefs/heads/foo

I changed my mind about which refs to use halfway through writing, and
of course because it is marked to expect failure, running the test
didn't clue me in. :)

-Peff

Re: [PATCH v2 5/5] ls-remote: add support for showing symrefs

[Thomas Gummerer]

On 01/18, Jeff King wrote:
> On Mon, Jan 18, 2016 at 11:09:13PM +0100, Thomas Gummerer wrote:
>
> > > +test_expect_failure 'ls-remote with filtered symrefs (--heads)' '
> > > +	git symbolic-ref refs/heads/foo refs/tags/mark &&
> > > +	cat >expect <<-\EOF &&
> > > +	ref: refs/heads/bar	refs/tags/mark
> > > +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/foo
> > > +	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
> > > +	EOF
> > > +	git ls-remote --symrefs --heads . >actual &&
> > > +	test_cmp expect actual
> > > +'
> >
> > I'm a bit confused by this.  Shouldn't the "ref: refs/heads/bar
> > refs/tags/mark" line only show up when we use --tags, not --heads?
> > Also should refs/heads/bar be refs/heads/foo?
>
> Yes, sorry, I bungled this. It should expect:
>
>   ref: refs/tags/mark\trefs/heads/foo
>
> I changed my mind about which refs to use halfway through writing, and
> of course because it is marked to expect failure, running the test
> didn't clue me in. :)

Heh, thanks for clearing this up :-)

> -Peff

--
Thomas

[PATCH v3 0/5] ls-remote: introduce symref argument

[Thomas Gummerer]

The previous rounds are at $gmane/284248.  Thanks to Peff and Junio
for reviewing and suggestions on the previous round.

Changes from the previous round:
 - Slightly reworded the documentation for the --refs option
 - Small fix in the commit message of patch 4.
 - use <<-\EOF instead of <<-EOF in the tests
 - added a note about upload-pack only showing the HEAD symref
 - squashed in tests by peff
 - changed --symrefs option to --symref
 - reworded description and documentation of the option according to
   comments from junio.

Interdiff below:

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 9356df2..5f2628c 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 [verse]
 'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=<exec>]
 	      [-q | --quiet] [--exit-code] [--get-url]
-	      [--symrefs] [<repository> [<refs>...]]
+	      [--symref] [<repository> [<refs>...]]
 
 DESCRIPTION
 -----------
@@ -31,8 +31,7 @@ OPTIONS
 	displayed.
 
 --refs::
-	Do not show peeled tags or pseudo-refs like HEAD or MERGE_HEAD
-	in the output.
+	Do not show peeled tags or pseudorefs like HEAD	in the output.
 
 -q::
 --quiet::
@@ -55,8 +54,11 @@ OPTIONS
 	"url.<base>.insteadOf" config setting (See linkgit:git-config[1]) and
 	exit without talking to the remote.
 
---symrefs::
-	Show the symrefs in addition to the other refs.
+--symref::
+	In addition to the object pointed by it, show the underlying
+	ref pointed by it when showing a symbolic ref.  Currently,
+	upload-pack only shows the symref HEAD, so it will be the only
+	one shown by ls-remote.
 
 <repository>::
 	The "remote" repository to query.  This parameter can be
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index ea73d53..66cdd45 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -6,7 +6,7 @@
 static const char * const ls_remote_usage[] = {
 	N_("git ls-remote [--heads] [--tags] [--refs] [--upload-pack=<exec>]\n"
 	   "                     [-q | --quiet] [--exit-code] [--get-url]\n"
-	   "                     [--symrefs] [<repository> [<refs>...]]"),
+	   "                     [--symref] [<repository> [<refs>...]]"),
 	NULL
 };
 
@@ -38,7 +38,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	int get_url = 0;
 	int quiet = 0;
 	int status = 0;
-	int symrefs = 0;
+	int show_symref_target = 0;
 	const char *uploadpack = NULL;
 	const char **pattern = NULL;
 
@@ -60,7 +60,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 			 N_("take url.<base>.insteadOf into account")),
 		OPT_SET_INT(0, "exit-code", &status,
 			    N_("exit with exit code 2 if no matching refs are found"), 2),
-		OPT_BOOL(0, "symrefs", &symrefs, N_("show symrefs")),
+		OPT_BOOL(0, "symref", &show_symref_target,
+			 N_("show underlying ref in addition to the object pointed by it")),
 		OPT_END()
 	};
 
@@ -104,7 +105,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 			continue;
 		if (!tail_match(pattern, ref->name))
 			continue;
-		if (symrefs && ref->symref)
+		if (show_symref_target && ref->symref)
 			printf("ref: %s\t%s\n", ref->symref, ref->name);
 		printf("%s\t%s\n", oid_to_hex(&ref->old_oid), ref->name);
 		status = 0; /* we found something */
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 3edbc9e..819b9dd 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -163,8 +163,8 @@ test_expect_success 'overrides work between mixed transfer/upload-pack hideRefs'
 	grep refs/tags/magic actual
 '
 
-test_expect_success 'ls-remote --symrefs' '
-	cat >expect <<-EOF &&
+test_expect_success 'ls-remote --symref' '
+	cat >expect <<-\EOF &&
 	ref: refs/heads/master	HEAD
 	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	HEAD
 	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
@@ -172,17 +172,40 @@ test_expect_success 'ls-remote --symrefs' '
 	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/master
 	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark
 	EOF
-	git ls-remote --symrefs >actual &&
+	git ls-remote --symref >actual &&
 	test_cmp expect actual
 '
 
-test_expect_success 'ls-remote with filtered symrefs' '
-	cat >expect <<-EOF &&
+test_expect_success 'ls-remote with filtered symref (refname)' '
+	cat >expect <<-\EOF &&
 	ref: refs/heads/master	HEAD
 	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	HEAD
 	EOF
-	git ls-remote --symrefs . HEAD >actual &&
+	git ls-remote --symref . HEAD >actual &&
 	test_cmp expect actual
 '
 
+test_expect_failure 'ls-remote with filtered symref (--heads)' '
+	git symbolic-ref refs/heads/foo refs/tags/mark &&
+	cat >expect <<-\EOF &&
+	ref: refs/tags/mark	refs/heads/foo
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/foo
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
+	EOF
+	git ls-remote --symref --heads . >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'ls-remote --symref omits filtered-out matches' '
+	cat >expect <<-\EOF &&
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/foo
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
+	EOF
+	git ls-remote --symref --heads . >actual &&
+	test_cmp expect actual &&
+	git ls-remote --symref . "refs/heads/*" >actual &&
+	test_cmp expect actual
+'
+
+
 test_done

Thomas Gummerer (5):
  ls-remote: document --quiet option
  ls-remote: document --refs option
  ls-remote: fix synopsis
  ls-remote: use parse-options api
  ls-remote: add support for showing symrefs

 Documentation/git-ls-remote.txt | 16 +++++++-
 builtin/ls-remote.c             | 90 +++++++++++++++++------------------------
 t/t5512-ls-remote.sh            | 45 +++++++++++++++++++++
 3 files changed, 95 insertions(+), 56 deletions(-)

-- 
2.7.0.30.g56a8654.dirty

[PATCH v3 1/5] ls-remote: document --quiet option

[Thomas Gummerer]

cefb2a5e3 ("ls-remote: print URL when no repo is specified") added a
quiet option to ls-remote, but didn't add it to the documentation.  Add
it.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git-ls-remote.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index d510c05..27380de 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git ls-remote' [--heads] [--tags]  [--upload-pack=<exec>]
-	      [--exit-code] <repository> [<refs>...]
+	      [-q | --quiet] [--exit-code] <repository> [<refs>...]
 
 DESCRIPTION
 -----------
@@ -29,6 +29,10 @@ OPTIONS
 	both, references stored in refs/heads and refs/tags are
 	displayed.
 
+-q::
+--quiet::
+	Do not print remote URL to stderr.
+
 --upload-pack=<exec>::
 	Specify the full path of 'git-upload-pack' on the remote
 	host. This allows listing references from repositories accessed via
-- 
2.7.0.30.g56a8654.dirty

[PATCH v3 2/5] ls-remote: document --refs option

[Thomas Gummerer]

The --refs option was originally introduced in 2718ff0 ("Improve
git-peek-remote").  The ls-remote command was first documented in
972b6fe ("ls-remote: drop storing operation and add documentation."),
but the --refs option was never documented.  Fix this.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git-ls-remote.txt | 5 ++++-
 builtin/ls-remote.c             | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 27380de..7467162 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]  [--upload-pack=<exec>]
+'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=<exec>]
 	      [-q | --quiet] [--exit-code] <repository> [<refs>...]
 
 DESCRIPTION
@@ -29,6 +29,9 @@ OPTIONS
 	both, references stored in refs/heads and refs/tags are
 	displayed.
 
+--refs::
+	Do not show peeled tags or pseudorefs like HEAD	in the output.
+
 -q::
 --quiet::
 	Do not print remote URL to stderr.
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index fa65a84..db21e52 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -4,7 +4,7 @@
 #include "remote.h"
 
 static const char ls_remote_usage[] =
-"git ls-remote [--heads] [--tags]  [--upload-pack=<exec>]\n"
+"git ls-remote [--heads] [--tags] [--refs] [--upload-pack=<exec>]\n"
 "                     [-q | --quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]";
 
 /*
-- 
2.7.0.30.g56a8654.dirty

[PATCH v3 3/5] ls-remote: fix synopsis

[Thomas Gummerer]

git ls-remote takes an optional get-url argument, and specifying the
repository is optional.  Fix the synopsis in the documentation to
reflect this.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git-ls-remote.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 7467162..453e93c 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=<exec>]
-	      [-q | --quiet] [--exit-code] <repository> [<refs>...]
+	      [-q | --quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]
 
 DESCRIPTION
 -----------
-- 
2.7.0.30.g56a8654.dirty

[PATCH v3 4/5] ls-remote: use parse-options api

[Thomas Gummerer]

Currently ls-remote uses a hand rolled parser for its command line
arguments.  Use the parse-options api instead of the hand rolled parser
to simplify the code and make it easier to add new arguments.  In
addition this improves the help message.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 builtin/ls-remote.c | 82 +++++++++++++++++++----------------------------------
 1 file changed, 29 insertions(+), 53 deletions(-)

diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index db21e52..3a20378 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -3,9 +3,11 @@
 #include "transport.h"
 #include "remote.h"
 
-static const char ls_remote_usage[] =
-"git ls-remote [--heads] [--tags] [--refs] [--upload-pack=<exec>]\n"
-"                     [-q | --quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]";
+static const char * const ls_remote_usage[] = {
+	N_("git ls-remote [--heads] [--tags] [--refs] [--upload-pack=<exec>]\n"
+	   "                     [-q | --quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]"),
+	NULL
+};
 
 /*
  * Is there one among the list of patterns that match the tail part
@@ -30,7 +32,6 @@ static int tail_match(const char **pattern, const char *path)
 
 int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 {
-	int i;
 	const char *dest = NULL;
 	unsigned flags = 0;
 	int get_url = 0;
@@ -43,59 +44,34 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	struct transport *transport;
 	const struct ref *ref;
 
-	if (argc == 2 && !strcmp("-h", argv[1]))
-		usage(ls_remote_usage);
+	struct option options[] = {
+		OPT__QUIET(&quiet, N_("do not print remote URL")),
+		OPT_STRING(0, "upload-pack", &uploadpack, N_("exec"),
+			   N_("path of git-upload-pack on the remote host")),
+		{ OPTION_STRING, 0, "exec", &uploadpack, N_("exec"),
+			   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, "refs", &flags, N_("do not show peeled tags"), REF_NORMAL),
+		OPT_BOOL(0, "get-url", &get_url,
+			 N_("take url.<base>.insteadOf into account")),
+		OPT_SET_INT(0, "exit-code", &status,
+			    N_("exit with exit code 2 if no matching refs are found"), 2),
+		OPT_END()
+	};
 
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
+	argc = parse_options(argc, argv, prefix, options, ls_remote_usage,
+			     PARSE_OPT_STOP_AT_NON_OPTION);
+	dest = argv[0];
 
-		if (*arg == '-') {
-			if (starts_with(arg, "--upload-pack=")) {
-				uploadpack = arg + 14;
-				continue;
-			}
-			if (starts_with(arg, "--exec=")) {
-				uploadpack = arg + 7;
-				continue;
-			}
-			if (!strcmp("--tags", arg) || !strcmp("-t", arg)) {
-				flags |= REF_TAGS;
-				continue;
-			}
-			if (!strcmp("--heads", arg) || !strcmp("-h", arg)) {
-				flags |= REF_HEADS;
-				continue;
-			}
-			if (!strcmp("--refs", arg)) {
-				flags |= REF_NORMAL;
-				continue;
-			}
-			if (!strcmp("--quiet", arg) || !strcmp("-q", arg)) {
-				quiet = 1;
-				continue;
-			}
-			if (!strcmp("--get-url", arg)) {
-				get_url = 1;
-				continue;
-			}
-			if (!strcmp("--exit-code", arg)) {
-				/* return this code if no refs are reported */
-				status = 2;
-				continue;
-			}
-			usage(ls_remote_usage);
-		}
-		dest = arg;
-		i++;
-		break;
+	if (argc > 1) {
+		int i;
+		pattern = xcalloc(argc, sizeof(const char *));
+		for (i = 1; i < argc; i++)
+			pattern[i - 1] = xstrfmt("*/%s", argv[i]);
 	}
 
-	if (argv[i]) {
-		int j;
-		pattern = xcalloc(argc - i + 1, sizeof(const char *));
-		for (j = i; j < argc; j++)
-			pattern[j - i] = xstrfmt("*/%s", argv[j]);
-	}
 	remote = remote_get(dest);
 	if (!remote) {
 		if (dest)
-- 
2.7.0.30.g56a8654.dirty

[PATCH v3 5/5] ls-remote: add support for showing symrefs

[Thomas Gummerer]

Sometimes it's useful to know the main branch of a git repository
without actually downloading the repository.  This can be done by
looking at the symrefs stored in the remote repository.  Currently git
doesn't provide a simple way to show the symrefs stored on the remote
repository, even though the information is available.  Add a --symref
command line argument to the ls-remote command, which shows the symrefs
in the remote repository.

While there, replace a literal tab in the format string with \t to make
it more obvious to the reader.

Suggested-by: pedro rijo <pedrorijo91@gmail.com>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git-ls-remote.txt |  9 ++++++++-
 builtin/ls-remote.c             | 10 +++++++--
 t/t5512-ls-remote.sh            | 45 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 453e93c..5f2628c 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -10,7 +10,8 @@ SYNOPSIS
 --------
 [verse]
 'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=<exec>]
-	      [-q | --quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]
+	      [-q | --quiet] [--exit-code] [--get-url]
+	      [--symref] [<repository> [<refs>...]]
 
 DESCRIPTION
 -----------
@@ -53,6 +54,12 @@ OPTIONS
 	"url.<base>.insteadOf" config setting (See linkgit:git-config[1]) and
 	exit without talking to the remote.
 
+--symref::
+	In addition to the object pointed by it, show the underlying
+	ref pointed by it when showing a symbolic ref.  Currently,
+	upload-pack only shows the symref HEAD, so it will be the only
+	one shown by ls-remote.
+
 <repository>::
 	The "remote" repository to query.  This parameter can be
 	either a URL or the name of a remote (see the GIT URLS and
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 3a20378..66cdd45 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -5,7 +5,8 @@
 
 static const char * const ls_remote_usage[] = {
 	N_("git ls-remote [--heads] [--tags] [--refs] [--upload-pack=<exec>]\n"
-	   "                     [-q | --quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]"),
+	   "                     [-q | --quiet] [--exit-code] [--get-url]\n"
+	   "                     [--symref] [<repository> [<refs>...]]"),
 	NULL
 };
 
@@ -37,6 +38,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	int get_url = 0;
 	int quiet = 0;
 	int status = 0;
+	int show_symref_target = 0;
 	const char *uploadpack = NULL;
 	const char **pattern = NULL;
 
@@ -58,6 +60,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 			 N_("take url.<base>.insteadOf into account")),
 		OPT_SET_INT(0, "exit-code", &status,
 			    N_("exit with exit code 2 if no matching refs are found"), 2),
+		OPT_BOOL(0, "symref", &show_symref_target,
+			 N_("show underlying ref in addition to the object pointed by it")),
 		OPT_END()
 	};
 
@@ -101,7 +105,9 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 			continue;
 		if (!tail_match(pattern, ref->name))
 			continue;
-		printf("%s	%s\n", oid_to_hex(&ref->old_oid), ref->name);
+		if (show_symref_target && ref->symref)
+			printf("ref: %s\t%s\n", ref->symref, ref->name);
+		printf("%s\t%s\n", oid_to_hex(&ref->old_oid), ref->name);
 		status = 0; /* we found something */
 	}
 	return status;
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index aadaac5..819b9dd 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -163,4 +163,49 @@ test_expect_success 'overrides work between mixed transfer/upload-pack hideRefs'
 	grep refs/tags/magic actual
 '
 
+test_expect_success 'ls-remote --symref' '
+	cat >expect <<-\EOF &&
+	ref: refs/heads/master	HEAD
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	HEAD
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/HEAD
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/remotes/origin/master
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/tags/mark
+	EOF
+	git ls-remote --symref >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'ls-remote with filtered symref (refname)' '
+	cat >expect <<-\EOF &&
+	ref: refs/heads/master	HEAD
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	HEAD
+	EOF
+	git ls-remote --symref . HEAD >actual &&
+	test_cmp expect actual
+'
+
+test_expect_failure 'ls-remote with filtered symref (--heads)' '
+	git symbolic-ref refs/heads/foo refs/tags/mark &&
+	cat >expect <<-\EOF &&
+	ref: refs/tags/mark	refs/heads/foo
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/foo
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
+	EOF
+	git ls-remote --symref --heads . >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'ls-remote --symref omits filtered-out matches' '
+	cat >expect <<-\EOF &&
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/foo
+	1bd44cb9d13204b0fe1958db0082f5028a16eb3a	refs/heads/master
+	EOF
+	git ls-remote --symref --heads . >actual &&
+	test_cmp expect actual &&
+	git ls-remote --symref . "refs/heads/*" >actual &&
+	test_cmp expect actual
+'
+
+
 test_done
-- 
2.7.0.30.g56a8654.dirty

Re: [PATCH v3 0/5] ls-remote: introduce symref argument

[Junio C Hamano]

Looks good to me; will queue.

Thanks.