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

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

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

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

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

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 <redacted>
Signed-off-by: Thomas Gummerer <redacted>
---
 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

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 <redacted>
Signed-off-by: Thomas Gummerer <redacted>
---
 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

On 01/17, Thomas Gummerer wrote:

quoted

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

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 <redacted>
Signed-off-by: Thomas Gummerer <redacted>
---

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

--
Thomas Gummerer

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

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

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. :)

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

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

quoted

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

On 01/17, Jeff King wrote:

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

quoted

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

On 01/17, Jeff King wrote:

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

quoted

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.

quoted

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

quoted

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

quoted

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

quoted

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

quoted

+		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

On 01/17, Jeff King wrote:

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

quoted

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.

quoted

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

Documentation _and_ tests. Yay. :)

quoted

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

quoted

+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

On 01/17, Jeff King wrote:

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

quoted

quoted

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

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

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

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

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

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 <redacted>
Signed-off-by: Thomas Gummerer <redacted>
---
 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

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 [off-list ref]
Signed-off-by: Thomas Gummerer <redacted>
---
 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

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

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 [off-list ref]
Signed-off-by: Thomas Gummerer <redacted>
---
 builtin/ls-remote.c | 82 +++++++++++++++++++----------------------------------
 1 file changed, 29 insertions(+), 53 deletions(-)

Patch itself looks good to me.

-Peff

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.

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

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

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

On 01/18, Jeff King wrote:

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

quoted

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.

quoted

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

quoted

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

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

On 01/18, Jeff King wrote:

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

quoted

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

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

quoted

+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

On 01/18, Jeff King wrote:

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

quoted

quoted

+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

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

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

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

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 [off-list ref]
Signed-off-by: Thomas Gummerer <redacted>
---
 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)