[PATCH v3 0/6] Replacement for rr/for-each-ref-decoration

[PATCH v3 0/6] Replacement for rr/for-each-ref-decoration

[Ramkumar Ramachandra]

Hi Junio et al,

v3 contains the following changes:

- [1/6] is totally unrelated; it's a "while we're there" patch.
- [2/6] is required for the test modification in [4/6].
- [3/6], [4/6], [5/6] now come with tests.
- [6/6] is new: implements Junio's suggestion to auto-reset color.

I haven't included Junio's 78465bb (fixup! for-each-ref: introduce
%(upstream:track[short]), 2013-10-31), because my compiler doesn't
complain, and because it's ugly.

Thanks.

Ramkumar Ramachandra (6):
  t6300 (for-each-ref): clearly demarcate setup
  t6300 (for-each-ref): don't hardcode SHA-1 hexes
  for-each-ref: introduce %(HEAD) asterisk marker
  for-each-ref: introduce %(upstream:track[short])
  for-each-ref: introduce %(color:...) for color
  for-each-ref: auto reset color after each atom

 Documentation/git-for-each-ref.txt | 14 ++++++-
 builtin/for-each-ref.c             | 86 +++++++++++++++++++++++++++++++++-----
 t/t6300-for-each-ref.sh            | 59 ++++++++++++++++++++------
 3 files changed, 135 insertions(+), 24 deletions(-)

-- 
1.8.5.rc0.6.gfd75b41

[PATCH v3 1/6] t6300 (for-each-ref): clearly demarcate setup

[Ramkumar Ramachandra]

Condense the two-step setup into one step, and give it an appropriate
name.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 t/t6300-for-each-ref.sh | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 752f5cb..72d282f 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -18,16 +18,13 @@ setdate_and_increment () {
     export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
 }
 
-test_expect_success 'Create sample commit with known timestamp' '
+test_expect_success setup '
 	setdate_and_increment &&
 	echo "Using $datestamp" > one &&
 	git add one &&
 	git commit -m "Initial" &&
 	setdate_and_increment &&
 	git tag -a -m "Tagging at $datestamp" testtag
-'
-
-test_expect_success 'Create upstream config' '
 	git update-ref refs/remotes/origin/master master &&
 	git remote add origin nowhere &&
 	git config branch.master.remote origin &&
-- 
1.8.5.rc0.6.gfd75b41

[PATCH v3 2/6] t6300 (for-each-ref): don't hardcode SHA-1 hexes

[Ramkumar Ramachandra]

Use rev-parse in its place, making it easier for future patches to
modify the test script.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 t/t6300-for-each-ref.sh | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 72d282f..e1f71ff 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -49,8 +49,8 @@ test_atom head refname refs/heads/master
 test_atom head upstream refs/remotes/origin/master
 test_atom head objecttype commit
 test_atom head objectsize 171
-test_atom head objectname 67a36f10722846e891fbada1ba48ed035de75581
-test_atom head tree 0e51c00fcb93dffc755546f27593d511e1bdb46f
+test_atom head objectname $(git rev-parse refs/heads/master)
+test_atom head tree $(git rev-parse refs/heads/master^{tree})
 test_atom head parent ''
 test_atom head numparent 0
 test_atom head object ''
@@ -82,11 +82,11 @@ test_atom tag refname refs/tags/testtag
 test_atom tag upstream ''
 test_atom tag objecttype tag
 test_atom tag objectsize 154
-test_atom tag objectname 98b46b1d36e5b07909de1b3886224e3e81e87322
+test_atom tag objectname $(git rev-parse refs/tags/testtag)
 test_atom tag tree ''
 test_atom tag parent ''
 test_atom tag numparent ''
-test_atom tag object '67a36f10722846e891fbada1ba48ed035de75581'
+test_atom tag object $(git rev-parse refs/tags/testtag^0)
 test_atom tag type 'commit'
 test_atom tag author ''
 test_atom tag authorname ''
@@ -302,7 +302,7 @@ test_expect_success 'Check short upstream format' '
 '
 
 cat >expected <<EOF
-67a36f1
+$(git rev-parse --short HEAD)
 EOF
 
 test_expect_success 'Check short objectname format' '
@@ -453,9 +453,9 @@ test_atom refs/tags/signed-long contents "subject line
 body contents
 $sig"
 
-cat >expected <<\EOF
-408fe76d02a785a006c2e9c669b7be5589ede96d <committer@example.com> refs/tags/master
-90b5ebede4899eda64893bc2a4c8f1d6fb6dfc40 <committer@example.com> refs/tags/bogo
+cat >expected <<EOF
+$(git rev-parse refs/tags/master) <committer@example.com> refs/tags/master
+$(git rev-parse refs/tags/bogo) <committer@example.com> refs/tags/bogo
 EOF
 
 test_expect_success 'Verify sort with multiple keys' '
-- 
1.8.5.rc0.6.gfd75b41

[PATCH v3 3/6] for-each-ref: introduce %(HEAD) asterisk marker

[Ramkumar Ramachandra]

'git branch' shows which branch you are currently on with an '*', but
'git for-each-ref' misses this feature.  So, extend its format with
%(HEAD) for the same effect.

Now you can use the following format in for-each-ref:

  %(HEAD) %(refname:short)

to display an asterisk next to the current ref.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Documentation/git-for-each-ref.txt |  4 ++++
 builtin/for-each-ref.c             | 13 +++++++++++--
 t/t6300-for-each-ref.sh            |  2 ++
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index f2e08d1..ab3da0e 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -93,6 +93,10 @@ upstream::
 	from the displayed ref. Respects `:short` in the same way as
 	`refname` above.
 
+HEAD::
+	Used to indicate the currently checked out branch.  Is '*' if
+	HEAD points to the current ref, and ' ' otherwise.
+
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
 be used to specify the value in the header field.
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 1d4083c..5f1842f 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -75,6 +75,7 @@ static struct {
 	{ "upstream" },
 	{ "symref" },
 	{ "flag" },
+	{ "HEAD" },
 };
 
 /*
@@ -675,8 +676,16 @@ static void populate_value(struct refinfo *ref)
 				v->s = xstrdup(buf + 1);
 			}
 			continue;
-		}
-		else
+		} else if (!strcmp(name, "HEAD")) {
+			const char *head;
+			unsigned char sha1[20];
+			head = resolve_ref_unsafe("HEAD", sha1, 1, NULL);
+			if (!strcmp(ref->refname, head))
+				v->s = "*";
+			else
+				v->s = " ";
+			continue;
+		} else
 			continue;
 
 		formatp = strchr(name, ':');
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index e1f71ff..5e29ffc 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -77,6 +77,7 @@ test_atom head contents:body ''
 test_atom head contents:signature ''
 test_atom head contents 'Initial
 '
+test_atom head HEAD '*'
 
 test_atom tag refname refs/tags/testtag
 test_atom tag upstream ''
@@ -110,6 +111,7 @@ test_atom tag contents:body ''
 test_atom tag contents:signature ''
 test_atom tag contents 'Tagging at 1151939927
 '
+test_atom tag HEAD ' '
 
 test_expect_success 'Check invalid atoms names are errors' '
 	test_must_fail git for-each-ref --format="%(INVALID)" refs/heads
-- 
1.8.5.rc0.6.gfd75b41

[PATCH v3 4/6] for-each-ref: introduce %(upstream:track[short])

[Ramkumar Ramachandra]

Introduce %(upstream:track) to display "[ahead M, behind N]" and
%(upstream:trackshort) to display "=", ">", "<", or "<>"
appropriately (inspired by contrib/completion/git-prompt.sh).

Now you can use the following format in for-each-ref:

  %(refname:short)%(upstream:trackshort)

to display refs with terse tracking information.

Note that :track and :trackshort only work with "upstream", and error
out when used with anything else.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Documentation/git-for-each-ref.txt |  6 +++++-
 builtin/for-each-ref.c             | 40 +++++++++++++++++++++++++++++++++++---
 t/t6300-for-each-ref.sh            | 22 +++++++++++++++++++++
 3 files changed, 64 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index ab3da0e..c9b192e 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -91,7 +91,11 @@ objectname::
 upstream::
 	The name of a local ref which can be considered ``upstream''
 	from the displayed ref. Respects `:short` in the same way as
-	`refname` above.
+	`refname` above.  Additionally respects `:track` to show
+	"[ahead N, behind M]" and `:trackshort` to show the terse
+	version (like the prompt) ">", "<", "<>", or "=".  Has no
+	effect if the ref does not have tracking information
+	associated with it.
 
 HEAD::
 	Used to indicate the currently checked out branch.  Is '*' if
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 5f1842f..ed81407 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -641,6 +641,7 @@ static void populate_value(struct refinfo *ref)
 		int deref = 0;
 		const char *refname;
 		const char *formatp;
+		struct branch *branch;
 
 		if (*name == '*') {
 			deref = 1;
@@ -652,7 +653,6 @@ static void populate_value(struct refinfo *ref)
 		else if (!prefixcmp(name, "symref"))
 			refname = ref->symref ? ref->symref : "";
 		else if (!prefixcmp(name, "upstream")) {
-			struct branch *branch;
 			/* only local branches may have an upstream */
 			if (prefixcmp(ref->refname, "refs/heads/"))
 				continue;
@@ -679,6 +679,7 @@ static void populate_value(struct refinfo *ref)
 		} else if (!strcmp(name, "HEAD")) {
 			const char *head;
 			unsigned char sha1[20];
+
 			head = resolve_ref_unsafe("HEAD", sha1, 1, NULL);
 			if (!strcmp(ref->refname, head))
 				v->s = "*";
@@ -689,13 +690,46 @@ static void populate_value(struct refinfo *ref)
 			continue;
 
 		formatp = strchr(name, ':');
-		/* look for "short" refname format */
 		if (formatp) {
+			int num_ours, num_theirs;
+
 			formatp++;
 			if (!strcmp(formatp, "short"))
 				refname = shorten_unambiguous_ref(refname,
 						      warn_ambiguous_refs);
-			else
+			else if (!strcmp(formatp, "track") &&
+				!prefixcmp(name, "upstream")) {
+				char buf[40];
+
+				stat_tracking_info(branch, &num_ours, &num_theirs);
+				if (!num_ours && !num_theirs)
+					v->s = "";
+				else if (!num_ours) {
+					sprintf(buf, "[behind %d]", num_theirs);
+					v->s = xstrdup(buf);
+				} else if (!num_theirs) {
+					sprintf(buf, "[ahead %d]", num_ours);
+					v->s = xstrdup(buf);
+				} else {
+					sprintf(buf, "[ahead %d, behind %d]",
+						num_ours, num_theirs);
+					v->s = xstrdup(buf);
+				}
+				continue;
+			} else if (!strcmp(formatp, "trackshort") &&
+				!prefixcmp(name, "upstream")) {
+
+				stat_tracking_info(branch, &num_ours, &num_theirs);
+				if (!num_ours && !num_theirs)
+					v->s = "=";
+				else if (!num_ours)
+					v->s = "<";
+				else if (!num_theirs)
+					v->s = ">";
+				else
+					v->s = "<>";
+				continue;
+			} else
 				die("unknown %.*s format %s",
 				    (int)(formatp - name), name, formatp);
 		}
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 5e29ffc..9d874fd 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -303,6 +303,28 @@ test_expect_success 'Check short upstream format' '
 	test_cmp expected actual
 '
 
+test_expect_success 'setup for upstream:track[short]' '
+	test_commit two
+'
+
+cat >expected <<EOF
+[ahead 1]
+EOF
+
+test_expect_success 'Check upstream:track format' '
+	git for-each-ref --format="%(upstream:track)" refs/heads >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<EOF
+>
+EOF
+
+test_expect_success 'Check upstream:trackshort format' '
+	git for-each-ref --format="%(upstream:trackshort)" refs/heads >actual &&
+	test_cmp expected actual
+'
+
 cat >expected <<EOF
 $(git rev-parse --short HEAD)
 EOF
-- 
1.8.5.rc0.6.gfd75b41

[PATCH v3 5/6] for-each-ref: introduce %(color:...) for color

[Ramkumar Ramachandra]

Enhance 'git for-each-ref' with color formatting options.  You can now
use the following format in for-each-ref:

  %(color:green)%(refname:short)%(color:reset)

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Documentation/git-for-each-ref.txt |  4 ++++
 builtin/for-each-ref.c             | 11 +++++++++--
 t/t6300-for-each-ref.sh            | 14 ++++++++++++++
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index c9b192e..2f3ac22 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -101,6 +101,10 @@ HEAD::
 	Used to indicate the currently checked out branch.  Is '*' if
 	HEAD points to the current ref, and ' ' otherwise.
 
+color::
+	Used to change color of output.  Followed by `:<colorname>`,
+	where names are described in `color.branch.*`.
+
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
 be used to specify the value in the header field.
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index ed81407..2ff4e54 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -9,6 +9,7 @@
 #include "quote.h"
 #include "parse-options.h"
 #include "remote.h"
+#include "color.h"
 
 /* Quoting styles */
 #define QUOTE_NONE 0
@@ -76,6 +77,7 @@ static struct {
 	{ "symref" },
 	{ "flag" },
 	{ "HEAD" },
+	{ "color" },
 };
 
 /*
@@ -662,8 +664,13 @@ static void populate_value(struct refinfo *ref)
 			    !branch->merge[0]->dst)
 				continue;
 			refname = branch->merge[0]->dst;
-		}
-		else if (!strcmp(name, "flag")) {
+		} else if (!prefixcmp(name, "color:")) {
+			char color[COLOR_MAXLEN] = "";
+
+			color_parse(name + 6, "--format", color);
+			v->s = xstrdup(color);
+			continue;
+		} else if (!strcmp(name, "flag")) {
 			char buf[256], *cp = buf;
 			if (ref->flag & REF_ISSYMREF)
 				cp = copy_advance(cp, ",symref");
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 9d874fd..35ca991 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -338,6 +338,20 @@ test_expect_success 'Check for invalid refname format' '
 	test_must_fail git for-each-ref --format="%(refname:INVALID)"
 '
 
+get_color ()
+{
+	git config --get-color no.such.slot "$1"
+}
+
+cat >expected <<EOF
+$(get_color red)$(git rev-parse --short HEAD)$(get_color reset) origin/master
+EOF
+
+test_expect_success 'Check %(color:...) ' '
+	git for-each-ref --format="%(color:red)%(objectname:short)%(color:reset) %(upstream:short)" refs/heads >actual &&
+	test_cmp expected actual
+'
+
 cat >expected <<\EOF
 heads/master
 tags/master
-- 
1.8.5.rc0.6.gfd75b41

[PATCH v3 6/6] for-each-ref: auto reset color after each atom

[Ramkumar Ramachandra]

It makes it easier to write a sensible format string, since you don't
have to %(color:reset) after each atom. Additionally, to make sure that
an invocation like the following doesn't leak color,

  $ git for-each-ref --format='%(subject)%(color:green)'

auto-reset at the end of the format string as well.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 builtin/for-each-ref.c  | 22 ++++++++++++++++++----
 t/t6300-for-each-ref.sh |  2 +-
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 2ff4e54..1050aea 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -23,6 +23,7 @@ typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 struct atom_value {
 	const char *s;
 	unsigned long ul; /* used for sorting when not FIELD_STR */
+	int color : 1;
 };
 
 struct ref_sort {
@@ -669,6 +670,7 @@ static void populate_value(struct refinfo *ref)
 
 			color_parse(name + 6, "--format", color);
 			v->s = xstrdup(color);
+			v->color = 1;
 			continue;
 		} else if (!strcmp(name, "flag")) {
 			char buf[256], *cp = buf;
@@ -914,11 +916,9 @@ static void sort_refs(struct ref_sort *sort, struct refinfo **refs, int num_refs
 	qsort(refs, num_refs, sizeof(struct refinfo *), compare_refs);
 }
 
-static void print_value(struct refinfo *ref, int atom, int quote_style)
+static void print_value(struct atom_value *v, int quote_style)
 {
-	struct atom_value *v;
 	struct strbuf sb = STRBUF_INIT;
-	get_value(ref, atom, &v);
 	switch (quote_style) {
 	case QUOTE_NONE:
 		fputs(v->s, stdout);
@@ -983,17 +983,31 @@ static void emit(const char *cp, const char *ep)
 static void show_ref(struct refinfo *info, const char *format, int quote_style)
 {
 	const char *cp, *sp, *ep;
+	struct atom_value *atomv, resetv;
+	int reset_color = 0;
+	char color[COLOR_MAXLEN] = "";
 
+	color_parse("reset", "--format", color);
+	resetv.s = color;
 	for (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {
 		ep = strchr(sp, ')');
 		if (cp < sp)
 			emit(cp, sp);
-		print_value(info, parse_atom(sp + 2, ep), quote_style);
+		get_value(info, parse_atom(sp + 2, ep), &atomv);
+		print_value(atomv, quote_style);
+		if (reset_color) {
+			print_value(&resetv, quote_style);
+			reset_color = 0;
+		}
+		if (atomv->color)
+			reset_color = 1;
 	}
 	if (*cp) {
 		sp = cp + strlen(cp);
 		emit(cp, sp);
 	}
+	if (reset_color)
+		print_value(&resetv, quote_style);
 	putchar('\n');
 }
 
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 35ca991..2bf2bea 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -348,7 +348,7 @@ $(get_color red)$(git rev-parse --short HEAD)$(get_color reset) origin/master
 EOF
 
 test_expect_success 'Check %(color:...) ' '
-	git for-each-ref --format="%(color:red)%(objectname:short)%(color:reset) %(upstream:short)" refs/heads >actual &&
+	git for-each-ref --format="%(color:red)%(objectname:short) %(upstream:short)" refs/heads >actual &&
 	test_cmp expected actual
 '
 
-- 
1.8.5.rc0.6.gfd75b41

Re: [PATCH v3 1/6] t6300 (for-each-ref): clearly demarcate setup

[Eric Sunshine]

On Fri, Nov 15, 2013 at 5:59 AM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> Condense the two-step setup into one step, and give it an appropriate
> name.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  t/t6300-for-each-ref.sh | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index 752f5cb..72d282f 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh
> @@ -18,16 +18,13 @@ setdate_and_increment () {
>      export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
>  }
>
> -test_expect_success 'Create sample commit with known timestamp' '
> +test_expect_success setup '
>         setdate_and_increment &&
>         echo "Using $datestamp" > one &&
>         git add one &&
>         git commit -m "Initial" &&
>         setdate_and_increment &&
>         git tag -a -m "Tagging at $datestamp" testtag

Broken &&-chain here after these functions are combined.

> -'
> -
> -test_expect_success 'Create upstream config' '
>         git update-ref refs/remotes/origin/master master &&
>         git remote add origin nowhere &&
>         git config branch.master.remote origin &&
> --
> 1.8.5.rc0.6.gfd75b41

Re: [PATCH v3 3/6] for-each-ref: introduce %(HEAD) asterisk marker

[Eric Sunshine]

On Fri, Nov 15, 2013 at 5:59 AM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> 'git branch' shows which branch you are currently on with an '*', but
> 'git for-each-ref' misses this feature.  So, extend its format with
> %(HEAD) for the same effect.
>
> Now you can use the following format in for-each-ref:
>
>   %(HEAD) %(refname:short)
>
> to display an asterisk next to the current ref.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  Documentation/git-for-each-ref.txt |  4 ++++
>  builtin/for-each-ref.c             | 13 +++++++++++--
>  t/t6300-for-each-ref.sh            |  2 ++
>  3 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index f2e08d1..ab3da0e 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -93,6 +93,10 @@ upstream::
>         from the displayed ref. Respects `:short` in the same way as
>         `refname` above.
>
> +HEAD::
> +       Used to indicate the currently checked out branch.  Is '*' if
> +       HEAD points to the current ref, and ' ' otherwise.

"Used to" sounds like it is explaining something it did in the past.
None of the other field names are described in this fashion. Perhaps
something simpler along the lines of:

    Asterisk "*" if HEAD matches the current ref (the checked out branch),
    otherwise space " ".

> +
>  In addition to the above, for commit and tag objects, the header
>  field names (`tree`, `parent`, `object`, `type`, and `tag`) can
>  be used to specify the value in the header field.

Re: [PATCH v3 4/6] for-each-ref: introduce %(upstream:track[short])

[Eric Sunshine]

On Fri, Nov 15, 2013 at 5:59 AM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> Introduce %(upstream:track) to display "[ahead M, behind N]" and
> %(upstream:trackshort) to display "=", ">", "<", or "<>"
> appropriately (inspired by contrib/completion/git-prompt.sh).
>
> Now you can use the following format in for-each-ref:
>
>   %(refname:short)%(upstream:trackshort)
>
> to display refs with terse tracking information.
>
> Note that :track and :trackshort only work with "upstream", and error
> out when used with anything else.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  Documentation/git-for-each-ref.txt |  6 +++++-
>  builtin/for-each-ref.c             | 40 +++++++++++++++++++++++++++++++++++---
>  t/t6300-for-each-ref.sh            | 22 +++++++++++++++++++++
>  3 files changed, 64 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index ab3da0e..c9b192e 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -91,7 +91,11 @@ objectname::
>  upstream::
>         The name of a local ref which can be considered ``upstream''
>         from the displayed ref. Respects `:short` in the same way as
> -       `refname` above.
> +       `refname` above.  Additionally respects `:track` to show
> +       "[ahead N, behind M]" and `:trackshort` to show the terse
> +       version (like the prompt) ">", "<", "<>", or "=".  Has no

The "prompt" is not mentioned elsewhere in for-each-ref documentation,
and a person not familiar with contrib/completion/ may be confused by
this reference. It might make sense instead to explain the meanings of
">", "<", "<>", and "=" directly since they are not necessarily
obvious to the casual reader.

> +       effect if the ref does not have tracking information
> +       associated with it.
>
>  HEAD::
>         Used to indicate the currently checked out branch.  Is '*' if
> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
> index 5f1842f..ed81407 100644
> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c
> @@ -689,13 +690,46 @@ static void populate_value(struct refinfo *ref)
>                         continue;
>
>                 formatp = strchr(name, ':');
> -               /* look for "short" refname format */
>                 if (formatp) {
> +                       int num_ours, num_theirs;
> +
>                         formatp++;
>                         if (!strcmp(formatp, "short"))
>                                 refname = shorten_unambiguous_ref(refname,
>                                                       warn_ambiguous_refs);
> -                       else
> +                       else if (!strcmp(formatp, "track") &&
> +                               !prefixcmp(name, "upstream")) {
> +                               char buf[40];
> +
> +                               stat_tracking_info(branch, &num_ours, &num_theirs);
> +                               if (!num_ours && !num_theirs)
> +                                       v->s = "";
> +                               else if (!num_ours) {
> +                                       sprintf(buf, "[behind %d]", num_theirs);
> +                                       v->s = xstrdup(buf);
> +                               } else if (!num_theirs) {
> +                                       sprintf(buf, "[ahead %d]", num_ours);
> +                                       v->s = xstrdup(buf);
> +                               } else {
> +                                       sprintf(buf, "[ahead %d, behind %d]",

Is the intention that these strings ("[ahead %d]", etc.) will be
internationalized in the future? If so, the allocated 40-character
buffer may be insufficient.

> +                                               num_ours, num_theirs);
> +                                       v->s = xstrdup(buf);
> +                               }
> +                               continue;
> +                       } else if (!strcmp(formatp, "trackshort") &&
> +                               !prefixcmp(name, "upstream")) {
> +
> +                               stat_tracking_info(branch, &num_ours, &num_theirs);
> +                               if (!num_ours && !num_theirs)
> +                                       v->s = "=";
> +                               else if (!num_ours)
> +                                       v->s = "<";
> +                               else if (!num_theirs)
> +                                       v->s = ">";
> +                               else
> +                                       v->s = "<>";
> +                               continue;
> +                       } else
>                                 die("unknown %.*s format %s",
>                                     (int)(formatp - name), name, formatp);

Is it still accurate to call this a "format" in the error message?
'track' and 'trackshort' seem more like decorations.

>                 }
> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index 5e29ffc..9d874fd 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh
> @@ -303,6 +303,28 @@ test_expect_success 'Check short upstream format' '
>         test_cmp expected actual
>  '
>
> +test_expect_success 'setup for upstream:track[short]' '
> +       test_commit two
> +'
> +
> +cat >expected <<EOF
> +[ahead 1]
> +EOF
> +
> +test_expect_success 'Check upstream:track format' '
> +       git for-each-ref --format="%(upstream:track)" refs/heads >actual &&
> +       test_cmp expected actual
> +'
> +
> +cat >expected <<EOF
> +>
> +EOF
> +
> +test_expect_success 'Check upstream:trackshort format' '
> +       git for-each-ref --format="%(upstream:trackshort)" refs/heads >actual &&
> +       test_cmp expected actual
> +'
> +
>  cat >expected <<EOF
>  $(git rev-parse --short HEAD)
>  EOF

Would it make sense also to add tests verifying that :track and
:trackshort correctly fail when applied to a key other than
"upstream"?

Re: [PATCH v3 5/6] for-each-ref: introduce %(color:...) for color

[Eric Sunshine]

On Fri, Nov 15, 2013 at 5:59 AM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> Enhance 'git for-each-ref' with color formatting options.  You can now
> use the following format in for-each-ref:
>
>   %(color:green)%(refname:short)%(color:reset)
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  Documentation/git-for-each-ref.txt |  4 ++++
>  builtin/for-each-ref.c             | 11 +++++++++--
>  t/t6300-for-each-ref.sh            | 14 ++++++++++++++
>  3 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index c9b192e..2f3ac22 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -101,6 +101,10 @@ HEAD::
>         Used to indicate the currently checked out branch.  Is '*' if
>         HEAD points to the current ref, and ' ' otherwise.
>
> +color::
> +       Used to change color of output.  Followed by `:<colorname>`,
> +       where names are described in `color.branch.*`.

"Used to" sounds past tense. Perhaps:

    color:<colorname>::
      Change output color to `<colorname>`, where...

>  In addition to the above, for commit and tag objects, the header
>  field names (`tree`, `parent`, `object`, `type`, and `tag`) can
>  be used to specify the value in the header field.

Re: [PATCH v3 4/6] for-each-ref: introduce %(upstream:track[short])

[Ramkumar Ramachandra]

Eric Sunshine wrote:
> The "prompt" is not mentioned elsewhere in for-each-ref documentation,
> and a person not familiar with contrib/completion/ may be confused by
> this reference. It might make sense instead to explain the meanings of
> ">", "<", "<>", and "=" directly since they are not necessarily
> obvious to the casual reader.

Someone else raised the same point earlier. Okay, I'll elaborate.

>> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
>> index 5f1842f..ed81407 100644
>> --- a/builtin/for-each-ref.c
>> +++ b/builtin/for-each-ref.c
>> @@ -689,13 +690,46 @@ static void populate_value(struct refinfo *ref)
>>                         continue;
>>
>>                 formatp = strchr(name, ':');
>> -               /* look for "short" refname format */
>>                 if (formatp) {
>> +                       int num_ours, num_theirs;
>> +
>>                         formatp++;
>>                         if (!strcmp(formatp, "short"))
>>                                 refname = shorten_unambiguous_ref(refname,
>>                                                       warn_ambiguous_refs);
>> -                       else
>> +                       else if (!strcmp(formatp, "track") &&
>> +                               !prefixcmp(name, "upstream")) {
>> +                               char buf[40];
>> +
>> +                               stat_tracking_info(branch, &num_ours, &num_theirs);
>> +                               if (!num_ours && !num_theirs)
>> +                                       v->s = "";
>> +                               else if (!num_ours) {
>> +                                       sprintf(buf, "[behind %d]", num_theirs);
>> +                                       v->s = xstrdup(buf);
>> +                               } else if (!num_theirs) {
>> +                                       sprintf(buf, "[ahead %d]", num_ours);
>> +                                       v->s = xstrdup(buf);
>> +                               } else {
>> +                                       sprintf(buf, "[ahead %d, behind %d]",
>
> Is the intention that these strings ("[ahead %d]", etc.) will be
> internationalized in the future? If so, the allocated 40-character
> buffer may be insufficient.

Similar strings in wt-status.c aren't ready for internationalization
either. Besides, these xstrdup() calls leak memory and are quite
suboptimal: if I allocate more memory, I'm going to be leaking more;
so, I'd defer the internationalization discussion until we restructure
this.

>> +                                               num_ours, num_theirs);
>> +                                       v->s = xstrdup(buf);
>> +                               }
>> +                               continue;
>> +                       } else if (!strcmp(formatp, "trackshort") &&
>> +                               !prefixcmp(name, "upstream")) {
>> +
>> +                               stat_tracking_info(branch, &num_ours, &num_theirs);
>> +                               if (!num_ours && !num_theirs)
>> +                                       v->s = "=";
>> +                               else if (!num_ours)
>> +                                       v->s = "<";
>> +                               else if (!num_theirs)
>> +                                       v->s = ">";
>> +                               else
>> +                                       v->s = "<>";
>> +                               continue;
>> +                       } else
>>                                 die("unknown %.*s format %s",
>>                                     (int)(formatp - name), name, formatp);
>
> Is it still accurate to call this a "format" in the error message?
> 'track' and 'trackshort' seem more like decorations.

By convention, all f-e-r formatting errors are reported as fatal
errors (see color errors too, for instance), unlike pretty-formats. We
might want to change this in a later series.

>> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
>> index 5e29ffc..9d874fd 100755
>> --- a/t/t6300-for-each-ref.sh
>> +++ b/t/t6300-for-each-ref.sh
>> @@ -303,6 +303,28 @@ test_expect_success 'Check short upstream format' '
>>         test_cmp expected actual
>>  '
>>
>> +test_expect_success 'setup for upstream:track[short]' '
>> +       test_commit two
>> +'
>> +
>> +cat >expected <<EOF
>> +[ahead 1]
>> +EOF
>> +
>> +test_expect_success 'Check upstream:track format' '
>> +       git for-each-ref --format="%(upstream:track)" refs/heads >actual &&
>> +       test_cmp expected actual
>> +'
>> +
>> +cat >expected <<EOF
>> +>
>> +EOF
>> +
>> +test_expect_success 'Check upstream:trackshort format' '
>> +       git for-each-ref --format="%(upstream:trackshort)" refs/heads >actual &&
>> +       test_cmp expected actual
>> +'
>> +
>>  cat >expected <<EOF
>>  $(git rev-parse --short HEAD)
>>  EOF
>
> Would it make sense also to add tests verifying that :track and
> :trackshort correctly fail when applied to a key other than
> "upstream"?

Sure.

And thanks for the detailed review.