[PATCH] for-each-ref: `:short` format for `refname`

Subsystems: documentation, the rest

DORMANTno replies

6 messages, 3 authors, 2016-06-15 · open the first message on its own page

[PATCH] for-each-ref: `:short` format for `refname`

From: Bert Wesarg <hidden>
Date: 2016-06-15 22:45:16

This strips from the refname the common directory prefix with the
matched pattern.

This is particular usefull for bash completion, to get refs without
`refs/heads` or `refs/tags`.

Signed-off-by: Bert Wesarg <redacted>

---
 Documentation/git-for-each-ref.txt |    5 ++
 builtin-for-each-ref.c             |   74 +++++++++++++++++++++++++++++++----
 t/t6300-for-each-ref.sh            |   61 +++++++++++++++++++++++++++++
 3 files changed, 131 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index eae6c0e..deeae79 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -74,6 +74,11 @@ For all objects, the following names can be used:
 
 refname::
 	The name of the ref (the part after $GIT_DIR/).
+	For a short name of the ref append `:short`. This will strip
+	the common directory prefix with the pattern which matches this ref.
+	I.e. for a the pattern `refs/heads` you get `master`, or for
+	`refs/tags/v1.5.[01].*` you get `v1.5.[01].*`.
+	This is particular usefull for bash completion.
 
 objecttype::
 	The type of the object (`blob`, `tree`, `commit`, `tag`).
diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index 21e92bb..946f79b 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -31,6 +31,7 @@ struct ref_sort {
 
 struct refinfo {
 	char *refname;
+	const char *pattern; /* the pattern which matched this ref */
 	unsigned char objectname[20];
 	struct atom_value *value;
 };
@@ -546,6 +547,40 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, v
 }
 
 /*
+ * Use the matched pattern from ref to shorten the refname
+ */
+static char *get_short_ref(struct refinfo *ref)
+{
+	int rlen, plen, common = 0;
+
+	if (!ref->pattern)
+		return ref->refname;
+
+	rlen = strlen(ref->refname);
+	plen = strlen(ref->pattern);
+
+	if ((plen <= rlen) &&
+	    !strncmp(ref->refname, ref->pattern, plen) &&
+	    (ref->refname[plen] == '\0' ||
+	     ref->refname[plen] == '/' ||
+	     ref->pattern[plen - 1] == '/')) {
+		common = plen + (ref->refname[plen] == '/');
+		/* prevent stripping the whole refname */
+		if (common == rlen)
+			common = 0;
+	} else {
+		/* find the first wildcard and go back to the previous '/' */
+		common = strcspn(ref->pattern, "*?[");
+		while (common >= 0 && ref->pattern[common] != '/')
+			--common;
+		common++;
+	}
+
+	return ref->refname + common;
+}
+
+
+/*
  * Parse the object referred by ref, and grab needed value.
  */
 static void populate_value(struct refinfo *ref)
@@ -570,13 +605,33 @@ static void populate_value(struct refinfo *ref)
 	for (i = 0; i < used_atom_cnt; i++) {
 		const char *name = used_atom[i];
 		struct atom_value *v = &ref->value[i];
-		if (!strcmp(name, "refname"))
-			v->s = ref->refname;
-		else if (!strcmp(name, "*refname")) {
-			int len = strlen(ref->refname);
-			char *s = xmalloc(len + 4);
-			sprintf(s, "%s^{}", ref->refname);
-			v->s = s;
+		int deref = 0;
+		if (*name == '*') {
+			deref = 1;
+			name++;
+		}
+		if (!prefixcmp(name, "refname")) {
+			const char *formatp = strchr(name, ':');
+			const char *refname = ref->refname;
+
+			/* look for "short" refname format */
+			if (formatp) {
+				formatp++;
+				if (!strcmp(formatp, "short"))
+					refname = get_short_ref(ref);
+				else
+					die("unknown refname format %s",
+					    formatp);
+			}
+
+			if (!deref)
+				v->s = refname;
+			else {
+				int len = strlen(refname);
+				char *s = xmalloc(len + 4);
+				sprintf(s, "%s^{}", refname);
+				v->s = s;
+			}
 		}
 	}
 
@@ -641,9 +696,9 @@ static int grab_single_ref(const char *refname, const unsigned char *sha1, int f
 	struct grab_ref_cbdata *cb = cb_data;
 	struct refinfo *ref;
 	int cnt;
+	const char **pattern = cb->grab_pattern;
 
-	if (*cb->grab_pattern) {
-		const char **pattern;
+	if (*pattern) {
 		int namelen = strlen(refname);
 		for (pattern = cb->grab_pattern; *pattern; pattern++) {
 			const char *p = *pattern;
@@ -668,6 +723,7 @@ static int grab_single_ref(const char *refname, const unsigned char *sha1, int f
 	 */
 	ref = xcalloc(1, sizeof(*ref));
 	ref->refname = xstrdup(refname);
+	ref->pattern = *pattern;
 	hashcpy(ref->objectname, sha1);
 
 	cnt = cb->grab_cnt;
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 8ced593..a4a2fd3 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -262,6 +262,67 @@ for i in "--perl --shell" "-s --python" "--python --tcl" "--tcl --perl"; do
 	"
 done
 
+cat >expected <<\EOF
+master
+testtag
+master
+testtag
+EOF
+
+test_expect_success 'Check short refname format' '
+	(git for-each-ref --format="%(refname:short)" refs/heads &&
+	git for-each-ref --format="%(refname:short)" refs/tags &&
+	git for-each-ref --format="%(refname:short)" refs/heads/ &&
+	git for-each-ref --format="%(refname:short)" refs/tags/) >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<\EOF
+master
+testtag
+EOF
+
+test_expect_success 'Check short refname format with multiple patterns' '
+	(git for-each-ref --format="%(refname:short)" refs/heads refs/tags) >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<\EOF
+refs/heads/master
+refs/tags/testtag
+EOF
+
+test_expect_success 'Check short refname format without patterns' '
+	git for-each-ref --format="%(refname:short)" >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'Check for invalid refname format' '
+	test_must_fail git for-each-ref --format="%(refname:INVALID)"
+'
+
+cat >expected <<\EOF
+heads/master
+master
+testtag
+EOF
+
+test_expect_success 'Check short refname format with wildcard pattern' '
+	(git for-each-ref --format="%(refname:short)" refs/*/m* &&
+	git for-each-ref --format="%(refname:short)" refs/heads/?aster &&
+	git for-each-ref --format="%(refname:short)" refs/tags/t[aeiou]sttag) >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<\EOF
+refs/heads/master
+EOF
+
+test_expect_success 'Check disabled short refname format with exact pattern' '
+	(git for-each-ref --format="%(refname:short)" refs/heads/master) >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'an unusual tag with an incomplete line' '
 
 	git tag -m "bogo" bogo &&
-- 
tg: (445cac1..) t/for-each-ref-refshort (depends on: master)

Re: [PATCH] for-each-ref: `:short` format for `refname`

From: SZEDER Gábor <hidden>
Date: 2016-06-15 22:45:16

Hi,

On Sun, Aug 31, 2008 at 02:41:07PM +0200, Bert Wesarg wrote:
This strips from the refname the common directory prefix with the
matched pattern.

This is particular usefull for bash completion, to get refs without
`refs/heads` or `refs/tags`.
 refname::
 	The name of the ref (the part after $GIT_DIR/).
+	For a short name of the ref append `:short`. This will strip
+	the common directory prefix with the pattern which matches this ref.
+	I.e. for a the pattern `refs/heads` you get `master`, or for
+	`refs/tags/v1.5.[01].*` you get `v1.5.[01].*`.
+	This is particular usefull for bash completion.
Should this last sentence really belong to the documentation?

Furthermore, I think ':strip' better describes what this format
actually does.  Even you have used the word 'strip' in the commit
message and in the documentation as well.


As far as bash completion is concerned, I'm for it, as it does exactly
what the completion script needs to perform better, it doesn't have
those conceptual issues 'refbasename' has, and it's only a tad slower
than 'refbasename'.

However, if we consider possible use cases other than bash completion,
I don't know which one is more useful.  For example, if you have two
branches 'foo/bar' and 'foo/baz', then 'git merge $(git for-each-ref
--format=%(refbasename) refs/heads/foo)' will work as expected, but
'refname:short' not, as it will output only 'bar' and 'baz' which 'git
merge' can not find.


Best,
Gábor

Re: [PATCH] for-each-ref: `:short` format for `refname`

From: Bert Wesarg <hidden>
Date: 2016-06-15 22:45:16

On Mon, Sep 1, 2008 at 15:15, SZEDER Gábor [off-list ref] wrote:
Hi,

On Sun, Aug 31, 2008 at 02:41:07PM +0200, Bert Wesarg wrote:
quoted
This strips from the refname the common directory prefix with the
matched pattern.

This is particular usefull for bash completion, to get refs without
`refs/heads` or `refs/tags`.
quoted
 refname::
      The name of the ref (the part after $GIT_DIR/).
+     For a short name of the ref append `:short`. This will strip
+     the common directory prefix with the pattern which matches this ref.
+     I.e. for a the pattern `refs/heads` you get `master`, or for
+     `refs/tags/v1.5.[01].*` you get `v1.5.[01].*`.
+     This is particular usefull for bash completion.
Should this last sentence really belong to the documentation?
At least it is not the only example in the documentation.
Furthermore, I think ':strip' better describes what this format
actually does.  Even you have used the word 'strip' in the commit
message and in the documentation as well.
True, I'm ok with this proposal.

As far as bash completion is concerned, I'm for it, as it does exactly
what the completion script needs to perform better, it doesn't have
those conceptual issues 'refbasename' has, and it's only a tad slower
than 'refbasename'.

However, if we consider possible use cases other than bash completion,
I don't know which one is more useful.  For example, if you have two
branches 'foo/bar' and 'foo/baz', then 'git merge $(git for-each-ref
--format=%(refbasename) refs/heads/foo)' will work as expected, but
'refname:short' not, as it will output only 'bar' and 'baz' which 'git
merge' can not find.
Yeah, thats an disadvantage and I thought about this, too. But I have
no particular opinion about it.

Regards,
Bert
Best,
Gábor

Re: [PATCH] for-each-ref: `:short` format for `refname`

From: Bert Wesarg <hidden>
Date: 2016-06-15 22:45:16

On Mon, Sep 1, 2008 at 16:13, Bert Wesarg [off-list ref] wrote:
On Mon, Sep 1, 2008 at 15:15, SZEDER Gábor [off-list ref] wrote:
quoted
However, if we consider possible use cases other than bash completion,
I don't know which one is more useful.  For example, if you have two
branches 'foo/bar' and 'foo/baz', then 'git merge $(git for-each-ref
--format=%(refbasename) refs/heads/foo)' will work as expected, but
'refname:short' not, as it will output only 'bar' and 'baz' which 'git
merge' can not find.
Yeah, thats an disadvantage and I thought about this, too. But I have
no particular opinion about it.
Ok, I have a new idea, which could be made all happy:

IMHO the goal of this new format for refname should be, that it can be
used as an ref on the command line. This isn't given with my current
:short proposal (which I call :strip as of now), as Gábor showed. What
we need is the reverse of what happened with refnames given on the
command line to commands like checkout/merge/... The only thing that
comes near to this is this from refs.c:

    const char *ref_rev_parse_rules[] = {
            "%.*s",
            "refs/%.*s",
            "refs/tags/%.*s",
            "refs/heads/%.*s",
            "refs/remotes/%.*s",
            "refs/remotes/%.*s/HEAD",
            NULL
    };

Which doesn't look very useful, because every ref from for_each_ref
would match rule one. So we probably need to try the reverse of this
list. Now my knowledge from git internals is really low, I don't know
if this is sane. I know that this can't be bijective but at least the
bash completion would be happy with this idea.

Comments, thoughts, brown paper bags...

Thanks,
Bert

Re: [PATCH] for-each-ref: `:short` format for `refname`

From: Shawn O. Pearce <hidden>
Date: 2016-06-15 22:45:16

Bert Wesarg [off-list ref] wrote:
IMHO the goal of this new format for refname should be, that it can be
used as an ref on the command line. This isn't given with my current
:short proposal (which I call :strip as of now), as Gábor showed. What
we need is the reverse of what happened with refnames given on the
command line to commands like checkout/merge/... The only thing that
comes near to this is this from refs.c:

    const char *ref_rev_parse_rules[] = {
            "%.*s",
            "refs/%.*s",
            "refs/tags/%.*s",
            "refs/heads/%.*s",
            "refs/remotes/%.*s",
            "refs/remotes/%.*s/HEAD",
            NULL
    };

Which doesn't look very useful, because every ref from for_each_ref
would match rule one. So we probably need to try the reverse of this
list.
Yup.  If you search the list backwards and extract the part of the
ref that matches %.*s you'll get a name that other tools can find,
and which is the shortest name possible.

You can still get ambiguous names.  Avoiding them requires going
through all refs and building their short forms, then using the
full ref name for any ref which had more than one name shorten to
the same string.  Ugly, but implementable, and probably something
that should be considered.

-- 
Shawn.

Re: [PATCH] for-each-ref: `:short` format for `refname`

From: Bert Wesarg <hidden>
Date: 2016-06-15 22:45:16

On Mon, Sep 1, 2008 at 21:10, Shawn O. Pearce [off-list ref] wrote:
Bert Wesarg [off-list ref] wrote:
quoted
IMHO the goal of this new format for refname should be, that it can be
used as an ref on the command line. This isn't given with my current
:short proposal (which I call :strip as of now), as Gábor showed. What
we need is the reverse of what happened with refnames given on the
command line to commands like checkout/merge/... The only thing that
comes near to this is this from refs.c:

    const char *ref_rev_parse_rules[] = {
            "%.*s",
            "refs/%.*s",
            "refs/tags/%.*s",
            "refs/heads/%.*s",
            "refs/remotes/%.*s",
            "refs/remotes/%.*s/HEAD",
            NULL
    };

Which doesn't look very useful, because every ref from for_each_ref
would match rule one. So we probably need to try the reverse of this
list.
Yup.  If you search the list backwards and extract the part of the
ref that matches %.*s you'll get a name that other tools can find,
and which is the shortest name possible.

You can still get ambiguous names.  Avoiding them requires going
through all refs and building their short forms, then using the
full ref name for any ref which had more than one name shorten to
the same string.  Ugly, but implementable, and probably something
that should be considered.
What about: try the list backwards until the first match, than try the
matched part (this what %.*s matched) with the forward list, if both
give the same pattern, its not disambiguous. If not try the next
pattern backwards.

Its quadratic in the number of patterns, but this is maybe smaller
than scanning all refs (which may include a sort phase).

Bert
--
Shawn.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help