threads / patch / 29765

patch, 2 partsbuiltin/symbolic-ref.c: add option to output shortened ref

Subject: [PATCH 2/2] builtin/symbolic-ref.c: add option to output shortened ref

## tl;dr

5 messages between Feb 27, 2012 and Feb 27, 2012. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Jan Krüger· Feb 27, 2012, 22:10 UTC · lore

In scripts meant to generate user-consumable output, it can be helpful to resolve a symbolic ref and output the result in a shortened form, such as for use in shell prompts. Add a new -s option to allow this.

Signed-off-by: Jan Krüger <jk@jk.gs>
---
 Documentation/git-symbolic-ref.txt |    6 +++++-
 builtin/symbolic-ref.c             |    5 +++++
 2 files changed, 10 insertions(+), 1 deletion(-)
Show changes to 2 files +10 −1

Documentation/git-symbolic-ref.txt, builtin/symbolic-ref.c

diff --git a/Documentation/git-symbolic-ref.txt b/Documentation/git-symbolic-ref.txt
index a05819b..7f108ce 100644
--- a/Documentation/git-symbolic-ref.txt
+++ b/Documentation/git-symbolic-ref.txt
@@ -8,7 +8,7 @@ git-symbolic-ref - Read and modify symbolic refs
 SYNOPSIS
 --------
 [verse]
-'git symbolic-ref' [-q] [-f] [-m <reason>] <name> [<ref>]
+'git symbolic-ref' [-q] [-f] [-s] [-m <reason>] <name> [<ref>]
 
 DESCRIPTION
 -----------
@@ -38,6 +38,10 @@ OPTIONS
 	not a symbolic ref; instead output the SHA1 value referenced by
 	<name>.
 
+-s::
+	When showing the value of <name> as a symbolic ref, try to shorten the
+	value, e.g. from `refs/heads/master` to `master`.
+
 -m::
 	Update the reflog for <name> with <reason>.  This is valid only
 	when creating or updating a symbolic ref.
diff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c
index 2e0a86f..df8da11 100644
--- a/builtin/symbolic-ref.c
+++ b/builtin/symbolic-ref.c
@@ -9,6 +9,7 @@ static const char * const git_symbolic_ref_usage[] = {
 };
 
 static int fallback_regular_ref;
+static int shorten;
 
 static void check_symref(const char *HEAD, int quiet)
 {
@@ -32,6 +33,9 @@ static void check_symref(const char *HEAD, int quiet)
 			exit(1);
 		}
 	}
+	if (shorten)
+		refs_heads_master = shorten_unambiguous_ref(
+			refs_heads_master, 0);
 	puts(refs_heads_master);
 }
 
@@ -44,6 +48,7 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)
 			"suppress error message for non-symbolic (detached) refs"),
 		OPT_BOOLEAN('f', NULL, &fallback_regular_ref,
 					"fall back to showing as a regular ref"),
+		OPT_BOOLEAN('s', NULL, &shorten, "shorten ref output"),
 		OPT_STRING('m', NULL, &msg, "reason", "reason of the update"),
 		OPT_END(),
 	};

-- 
1.7.9.2.302.g3724c.dirty
Junio C Hamano· Feb 27, 2012, 22:28 UTC · re: Jan Krüger · lore

Re: [PATCH 2/2] builtin/symbolic-ref.c: add option to output shortened ref

Jan Krüger <jk@jk.gs> writes:
> In scripts meant to generate user-consumable output, it can be helpful
> to resolve a symbolic ref and output the result in a shortened form,
> such as for use in shell prompts. Add a new -s option to allow this.

I think this one (unlike 1/2) makes sense, but a single letter -s feels a bit too vague. Always spelling in long option "--short" so that it matches "%(refname:short)" in for-each-ref might be better, I would think.

Especially given that the expected use case is primarily in scripts not from the command line, being more explicit and easier to read has value over being short and easier to (mis)type.

Jan Krüger· Feb 27, 2012, 22:46 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/2] builtin/symbolic-ref.c: add option to output shortened ref

On 02/27/2012 11:28 PM, Junio C Hamano wrote:
Show 7 quoted lines
> I think this one (unlike 1/2) makes sense, but a single letter -s feels a
> bit too vague.  Always spelling in long option "--short" so that it
> matches "%(refname:short)" in for-each-ref might be better, I would think.
> 
> Especially given that the expected use case is primarily in scripts not
> from the command line, being more explicit and easier to read has value
> over being short and easier to (mis)type.

Good point. If we can agree on what to do with the first patch, I'll change that in the next iteration.

-Jan
Junio C Hamano· Feb 27, 2012, 23:04 UTC · re: Jan Krüger · lore

Re: [PATCH 2/2] builtin/symbolic-ref.c: add option to output shortened ref

Jan Krüger <jk@jk.gs> writes:
Show 11 quoted lines
> On 02/27/2012 11:28 PM, Junio C Hamano wrote:
>> I think this one (unlike 1/2) makes sense, but a single letter -s feels a
>> bit too vague.  Always spelling in long option "--short" so that it
>> matches "%(refname:short)" in for-each-ref might be better, I would think.
>> 
>> Especially given that the expected use case is primarily in scripts not
>> from the command line, being more explicit and easier to read has value
>> over being short and easier to (mis)type.
>
> Good point. If we can agree on what to do with the first patch, I'll
> change that in the next iteration.

Dropping the first patch would be my preference. I do not think this change deserves to be taken hostage to it.

Junio C Hamano· Feb 27, 2012, 23:54 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/2] builtin/symbolic-ref.c: add option to output shortened ref

Junio C Hamano <gitster@pobox.com> writes:
> ... I do not think this change deserves to be taken hostage to it.
So here is a trivial rebase of the patch.

The --short option should apply only when querying (-q also shares this, I think), so I split the synopsis into two.

I was the guity one who named the variable refs_heads_master, hoping that it would be easier to name it after representative value it would contain (so that it would be clear for other code that may want to strip the leading component what to expect), but now it has happened with your patch, the variable name looks silly.

OPT_BOOLEAN() is deprecated; OPT_BOOL() has a much saner semantics for most callers' needs and we should consider using it when adding new options.

Perhaps we would also want some tests?
 Documentation/git-symbolic-ref.txt |    7 ++++++-
 builtin/symbolic-ref.c             |   11 ++++++++---
 2 files changed, 14 insertions(+), 4 deletions(-)
Show changes to 2 files +14 −4

Documentation/git-symbolic-ref.txt, builtin/symbolic-ref.c

diff --git a/Documentation/git-symbolic-ref.txt b/Documentation/git-symbolic-ref.txt
index a45d4c4..89e7707 100644
--- a/Documentation/git-symbolic-ref.txt
+++ b/Documentation/git-symbolic-ref.txt
@@ -8,7 +8,8 @@ git-symbolic-ref - Read and modify symbolic refs
 SYNOPSIS
 --------
 [verse]
-'git symbolic-ref' [-q] [-m <reason>] <name> [<ref>]
+'git symbolic-ref' [-m <reason>] <name> <ref>
+'git symbolic-ref' [-q] [--short] <name>
 
 DESCRIPTION
 -----------
@@ -33,6 +34,10 @@ OPTIONS
 	symbolic ref but a detached HEAD; instead exit with
 	non-zero status silently.
 
+--short::
+	When showing the value of <name> as a symbolic ref, try to shorten the
+	value, e.g. from `refs/heads/master` to `master`.
+
 -m::
 	Update the reflog for <name> with <reason>.  This is valid only
 	when creating or updating a symbolic ref.
diff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c
index 2ef5962..801d62e 100644
--- a/builtin/symbolic-ref.c
+++ b/builtin/symbolic-ref.c
@@ -8,13 +8,15 @@ static const char * const git_symbolic_ref_usage[] = {
 	NULL
 };
 
+static int shorten;
+
 static void check_symref(const char *HEAD, int quiet)
 {
 	unsigned char sha1[20];
 	int flag;
-	const char *refs_heads_master = resolve_ref_unsafe(HEAD, sha1, 0, &flag);
+	const char *refname = resolve_ref_unsafe(HEAD, sha1, 0, &flag);
 
-	if (!refs_heads_master)
+	if (!refname)
 		die("No such ref: %s", HEAD);
 	else if (!(flag & REF_ISSYMREF)) {
 		if (!quiet)
@@ -22,7 +24,9 @@ static void check_symref(const char *HEAD, int quiet)
 		else
 			exit(1);
 	}
-	puts(refs_heads_master);
+	if (shorten)
+		refname = shorten_unambiguous_ref(refname, 0);
+	puts(refname);
 }
 
 int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)
@@ -32,6 +36,7 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)
 	struct option options[] = {
 		OPT__QUIET(&quiet,
 			"suppress error message for non-symbolic (detached) refs"),
+		OPT_BOOL(0, "short", &shorten, "shorten ref output"),
 		OPT_STRING('m', NULL, &msg, "reason", "reason of the update"),
 		OPT_END(),
 	};

← back to recent threads