[PATCH v2 0/3] Towards a useable git-branch

So, while investigating alignment operators in pretty-formats, I found
out that it's way too much effort and totally not worth it (atleast
not immediately; we can add it later if we want).  What I want now is
a useable git-branch output.  And I think I can say that I've achieved
it.

I currently have hot aliased to

for-each-ref --format='%C(red)%(HEAD)%C(reset) %C(green)%(refname:short)%C(reset)%(upstream:trackshort)' --count 10 --sort='-committerdate' refs/heads

and it works beautifully for me.  Sample output:

% git hot
* hot-branch<>
  pickaxe-doc>
  publish-rev=
  publish-rev-test
  upstream-error=
  push-current-head=
  master=
  prompt=
  autostash-stash=
  rebase.autostash=

The asterisk is red, the branch names are in green, and the tracking
marker is white.

I'm very happy with the implementation too:

1. color only kicks in at the parsing layer.
2. HEAD is a new atom.
3. :track[short] is a formatp like :short.

There is no need to use a hammer and coerce everything into an atom,
or throw everything out the window and start from scratch to conform
to pretty-formats perfectly.  Let's extend the existing format to be
_useful_ sensibly.

Thanks.

Ramkumar Ramachandra (3):
  for-each-ref: introduce %C(...) for color
  for-each-ref: introduce %(HEAD) marker
  for-each-ref: introduce %(upstream:track[short])

 builtin/for-each-ref.c | 81 +++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 73 insertions(+), 8 deletions(-)

-- 
1.8.3.rc3.2.g99b8f3f.dirty

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

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

  %C(red)%(HEAD)%C(reset) %C(green)%(refname:short)%C(reset)%(upstream:trackshort)

to display refs with terse tracking information.

Note that :track and :trackshort only works with upstream, and errors
out when used with anything else.

Signed-off-by: Ramkumar Ramachandra <redacted>
---
 builtin/for-each-ref.c | 42 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 63d3a85..838b125 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c

@@ -632,6 +632,7 @@ static void populate_value(struct refinfo *ref)
 	int eaten, i;
 	unsigned long size;
 	const unsigned char *tagged;
+	int upstream_present = 0;
 
 	ref->value = xcalloc(sizeof(struct atom_value), used_atom_cnt);
 

@@ -649,6 +650,7 @@ static void populate_value(struct refinfo *ref)
 		int deref = 0;
 		const char *refname;
 		const char *formatp;
+		struct branch *branch;
 
 		if (*name == '*') {
 			deref = 1;

@@ -660,7 +662,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;

@@ -670,6 +671,7 @@ static void populate_value(struct refinfo *ref)
 			    !branch->merge[0]->dst)
 				continue;
 			refname = branch->merge[0]->dst;
+			upstream_present = 1;
 		}
 		else if (!strcmp(name, "flag")) {
 			char buf[256], *cp = buf;

@@ -687,6 +689,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 = "*";

@@ -699,11 +702,46 @@ static void populate_value(struct refinfo *ref)
 		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];
+
+				if (!upstream_present)
+					continue;
+				if (!stat_tracking_info(branch, &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")) {
+				if (!upstream_present)
+					continue;
+				if (!stat_tracking_info(branch, &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);
 		}

-- 
1.8.3.rc3.2.g99b8f3f.dirty

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

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

  %C(red)%(HEAD)%C(reset) %C(green)%(refname:short)%C(reset)

to display a red asterisk next to the current ref.

Signed-off-by: Ramkumar Ramachandra <redacted>
---
 builtin/for-each-ref.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 1563b25..63d3a85 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c

@@ -76,6 +76,7 @@ static struct {
 	{ "upstream" },
 	{ "symref" },
 	{ "flag" },
+	{ "HEAD" },
 };
 
 /*

@@ -683,8 +684,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, ':');

-- 
1.8.3.rc3.2.g99b8f3f.dirty

Since 'git branch' misses important options like --sort, --count, and
--format that are present in 'git for-each-ref'.  Until we are in a
position to fix 'git branch', let us enhance the 'git for-each-ref'
format so it can atleast colorize output.

You can use the following format in for-each-ref:

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

To display refs in green.  Future patches will attempt to extend the
format even more to get useful output.

Signed-off-by: Ramkumar Ramachandra <redacted>
---
 builtin/for-each-ref.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 7f059c3..1563b25 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

@@ -155,10 +156,13 @@ static const char *find_next(const char *cp)
 	while (*cp) {
 		if (*cp == '%') {
 			/*
+			 * %C( is the start of a color;
 			 * %( is the start of an atom;
 			 * %% is a quoted per-cent.
 			 */
-			if (cp[1] == '(')
+			if (cp[1] == 'C' && cp[2] == '(')
+				return cp;
+			else if (cp[1] == '(')
 				return cp;
 			else if (cp[1] == '%')
 				cp++; /* skip over two % */

@@ -180,8 +184,12 @@ static int verify_format(const char *format)
 		const char *ep = strchr(sp, ')');
 		if (!ep)
 			return error("malformed format string %s", sp);
-		/* sp points at "%(" and ep points at the closing ")" */
-		parse_atom(sp + 2, ep);
+		/*
+		 * Ignore color specifications: %C(
+		 * sp points at "%(" and ep points at the closing ")"
+		 */
+		if (prefixcmp(sp, "%C("))
+			parse_atom(sp + 2, ep);
 		cp = ep + 1;
 	}
 	return 0;

@@ -928,12 +936,22 @@ 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;
+	char color[COLOR_MAXLEN];
 
 	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);
+
+		/* Do we have a color specification? */
+		if (!prefixcmp(sp, "%C("))
+			color_parse_mem(sp + 3,
+					ep - sp - 3,
+					"--format ", color);
+		else {
+			printf("%s", color);
+			print_value(info, parse_atom(sp + 2, ep), quote_style);
+		}
 	}
 	if (*cp) {
 		sp = cp + strlen(cp);

-- 
1.8.3.rc3.2.g99b8f3f.dirty

On Fri, May 24, 2013 at 9:19 PM, Ramkumar Ramachandra
[off-list ref] wrote:

There is no need to use a hammer and coerce everything into an atom,
or throw everything out the window and start from scratch to conform
to pretty-formats perfectly.  Let's extend the existing format to be
_useful_ sensibly.

Usefulness is one thing. Another is maintenance and in that regard I
still think we should be able to remove -v and -vv code (not the
functionality) with this topic.
--
Duy

Duy Nguyen wrote:

Usefulness is one thing. Another is maintenance and in that regard I
still think we should be able to remove -v and -vv code (not the
functionality) with this topic.

Why?  The topic adds good functionality, doesn't break anything,
doesn't paint us into any corner, and has users.  Replacing -v and -vv
can be done eventually, after we get alignment.

From: "Ramkumar Ramachandra" <redacted>
Sent: Friday, May 24, 2013 3:19 PM

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

Maybe 'isHEAD'  as a better name, or 'ifHEAD', or something to indicate 
its boolean nature.

On Fri, May 24, 2013 at 4:19 PM, Ramkumar Ramachandra
[off-list ref] wrote:

Is it not possible for "color" to be used uninitialized here ?

+                       print_value(info, parse_atom(sp + 2, ep), quote_style);
+               }
        }
        if (*cp) {
                sp = cp + strlen(cp);

On Fri, May 24, 2013 at 9:19 PM, Ramkumar Ramachandra
[off-list ref] wrote:

So, while investigating alignment operators in pretty-formats, I found
out that it's way too much effort and totally not worth it (atleast
not immediately; we can add it later if we want).

I just had an idea that might bring pretty stuff to for-each-ref with
probably reasonable effort, making for-each-ref format a superset of
pretty. But I need to clean up my backlog first. Give me a few days, I
will show you something (or give up ;)
--
Duy

On Fri, May 24, 2013 at 7:19 AM, Ramkumar Ramachandra
[off-list ref] wrote:

Since 'git branch' misses important options like --sort, --count, and
--format that are present in 'git for-each-ref'.  Until we are in a
position to fix 'git branch', let us enhance the 'git for-each-ref'
format so it can atleast colorize output.

You can use the following format in for-each-ref:

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

To display refs in green.  Future patches will attempt to extend the
format even more to get useful output.

Signed-off-by: Ramkumar Ramachandra <redacted>
---
 builtin/for-each-ref.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

Can you please also update Documentation/?
--
David

On Sat, May 25, 2013 at 5:51 AM, Duy Nguyen [off-list ref] wrote:

I just had an idea that might bring pretty stuff to for-each-ref with
probably reasonable effort, making for-each-ref format a superset of
pretty. But I need to clean up my backlog first. Give me a few days, I
will show you something (or give up ;)

And I can't get it out of my head. Might as well write it down. Check out

https://github.com/pclouds/git.git for-each-ref-pretty

It opens a hook in format_commit_message() to plug f-e-r's atom syntax
in. I didn't do extensive test or anything, just t6300. The %xx syntax
in for-each-ref might override some placeholders in pretty, I did not
check. You can add extra %(xx) on top as you have done. I still need
one more hook to support %>(*) with automatic width detection. After
that I'm quite confident we can kill -v/-vv code.
--
Duy

On Fri, May 24, 2013 at 10:19 AM, Ramkumar Ramachandra
[off-list ref] wrote:

Since 'git branch' misses important options like --sort, --count, and
--format that are present in 'git for-each-ref'.  Until we are in a
position to fix 'git branch', let us enhance the 'git for-each-ref'
format so it can atleast colorize output.

s/atleast/at least/

You can use the following format in for-each-ref:

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

To display refs in green.  Future patches will attempt to extend the
format even more to get useful output.

Signed-off-by: Ramkumar Ramachandra <redacted>

On Sat, May 25, 2013 at 1:26 PM, Duy Nguyen [off-list ref] wrote:

On Sat, May 25, 2013 at 5:51 AM, Duy Nguyen [off-list ref] wrote:

quoted

I just had an idea that might bring pretty stuff to for-each-ref with
probably reasonable effort, making for-each-ref format a superset of
pretty. But I need to clean up my backlog first. Give me a few days, I
will show you something (or give up ;)

And I can't get it out of my head. Might as well write it down. Check out

https://github.com/pclouds/git.git for-each-ref-pretty

Ram, fetch the url above again. Its tip now is 5b4aa27 (for-each-ref:
introduce format specifier %>(*) and %<(*) - 2013-05-25). Those
changes make for-each-ref --format a superset of pretty. You can add
new %(xxx) on top and resend the whole thing to the list for review.
You can remove "branch -v/-vv" code as well or I'll add some patches
on top to do that later. I have some compatibility concerns about the
"superset" thing. But let's wait until the series hits git@vger.
--
Duy

Duy Nguyen wrote:

Ram, fetch the url above again. Its tip now is 5b4aa27 (for-each-ref:
introduce format specifier %>(*) and %<(*) - 2013-05-25). Those
changes make for-each-ref --format a superset of pretty. You can add
new %(xxx) on top and resend the whole thing to the list for review.
You can remove "branch -v/-vv" code as well or I'll add some patches
on top to do that later. I have some compatibility concerns about the
"superset" thing. But let's wait until the series hits git@vger.

Great work Duy!  I see that you've used format_commit_message(), but
there are some concerns about pretty-formats conflicting with f-e-r's
format.  We'll iron it out slowly, but I like the overall approach.

Thanks.

(Very sleep deprived at the moment; will review and collaborate after I wake up)

Antoine Pelisse wrote:

Is it not possible for "color" to be used uninitialized here ?

My compiler didn't complain; what am I missing?  Doesn't the
declaration char color[COLOR_MAXLEN]; initialize an empty string?
More importantly, aren't there numerous instances of this in the
codebase?

David Aguilar wrote:

Can you please also update Documentation/?

Yeah, will do in the re-roll.  Duy is bringing in pretty-formats.
We'll probably need a separate document called pretty-ref-formats or
some such thing.

On Sat, May 25, 2013 at 05:20:29PM +0530, Ramkumar Ramachandra wrote:

Antoine Pelisse wrote:

quoted

Is it not possible for "color" to be used uninitialized here ?

My compiler didn't complain; what am I missing?  Doesn't the
declaration char color[COLOR_MAXLEN]; initialize an empty string?

Why would it?  The variable's begin allocated on the stack and the C
standard only zero-initializes variables with static storage duration;
Section 6.7.9 of the C11 standard says:

    If an object that has automatic storage duration is not initialized
    explicitly, its value is indeterminate.


I suspect the compiler doesn't complain because there is a path through
the function that initializes color before reading it (if we hit the
"if" branch in the loop before the "else" branch) and the compile
assumes that there is something in the function's contract that
guarantees that we follow this path.  But I don't think that's correct
so you do need to initialize color to the empty string.

More importantly, aren't there numerous instances of this in the
codebase?

Care to point at one?  I had a quick look and all places I inspected are
either static or write to the array before reading it.

On Sat, May 25, 2013 at 1:50 PM, Ramkumar Ramachandra
[off-list ref] wrote:

Antoine Pelisse wrote:

quoted

Is it not possible for "color" to be used uninitialized here ?

My compiler didn't complain; what am I missing?  Doesn't the
declaration char color[COLOR_MAXLEN]; initialize an empty string?

As John said, this is allocated on the stack.
What do you want it to be initialized to ?
Filled with zeros ? That's overkill because only the first char needs
to be zeroed.
The compiler can't know what to do and it lets you to take the best action.

More importantly, aren't there numerous instances of this in the
codebase?

I think Valgrind would be mad at us if there was many instances of
this. By the way Ramkumar, could you check if Valgrind complains with
your patch ?

John Keeping wrote:

Section 6.7.9 of the C11 standard says:

    If an object that has automatic storage duration is not initialized
    explicitly, its value is indeterminate.

Ah, thanks.  I'll initialize it to an empty string.

quoted

More importantly, aren't there numerous instances of this in the
codebase?

Care to point at one?  I had a quick look and all places I inspected are
either static or write to the array before reading it.

I'll run some Valgrind tests to see what it yields.

Hi Duy,

I just woke up and started looking at the series: it's rather well
done, and I'm confident that this is the way forward.  To reciprocate,
I've done some work at gh:artagnon/git for-each-ref-pretty.  See:

https://github.com/artagnon/git/commits/for-each-ref-pretty

There is one major problem though:

%>(N) doesn't work properly with f-e-r, and I'm not sure why.  I'm not
talking about your last patch where you compute * -- that works fine;
it's just that %>(N) doesn't when N is a concrete number.

Also, a couple of minor annoyances:

1. When f-e-r is invoked with refs/tags, we get stray output.  Atleast
it doesn't segfault, thanks to your ignore-commit patch.  Maybe
printing stray output is the right thing to do, as opposed to erroring
out.

2. %>(*) only works with f-e-r atoms, not with pretty-format atoms.
This is ofcourse obvious from the implementation, but isn't it a
little consistent?

Should we start off a new pretty-ref-formats document, where we
explicitly exclude things like %ae (because of the hex overriding
thing)?  I don't think it's a problem if documented properly.

Oh, and by the way:

We're pretty close we are to replacing branch -v and branch -vv.

brv = for-each-ref --format='%(HEAD)
%C(green)%<(*)%(refname:short)%C(reset) %<(*)%(objectname:short)
%(subject)' refs/heads

brvv = for-each-ref --format='%(HEAD)
%C(green)%<(*)%(refname:short)%C(reset) %<(*)%(objectname:short)
%C(blue)%(upstream:short)%C(reset) %(subject)' refs/heads

There are small differences:

1. In branch -v, the green-color of the branch name is dependent on
%(HEAD).  Not worth ironing out, in my opinion.

2. In branch -vv, there are dependent square braces that come on when
%(refname:short) is set.  We might want to introduce an undocumented
%(refname:branchvv) for internal use by branch -vv, for backward
compatibility.

What do you think?

Ramkumar Ramachandra wrote:

%>(N) doesn't work properly with f-e-r, and I'm not sure why.  I'm not
talking about your last patch where you compute * -- that works fine;
it's just that %>(N) doesn't when N is a concrete number.

Try this:

%(refname:short)%>(30)%(upstream:short)

(assuming that you have lots of branches).  I'm noticing random
alignment problems.

However, %<(N) doesn't seem to have this problem:

%<(30)%(refname:short)%(upstream:short)

I'm not able to figure this out.

On Tue, May 28, 2013 at 9:28 PM, Ramkumar Ramachandra
[off-list ref] wrote:

Ramkumar Ramachandra wrote:

quoted

%>(N) doesn't work properly with f-e-r, and I'm not sure why.  I'm not
talking about your last patch where you compute * -- that works fine;
it's just that %>(N) doesn't when N is a concrete number.

Try this:

%(refname:short)%>(30)%(upstream:short)

(assuming that you have lots of branches).  I'm noticing random
alignment problems.

It's because you don't pad enough spaces after %(refname:short) so the
starting point of %(upstream:short) on each line is already unaligned,
I think. Try this:

%<(*)%(refname:short)%>(30)%(upstream:short)

or if you prefer at specific column (e.g. align upstream close to the
60th column, regardless of refname's length):

%(refname:short)%>|(60)%(upstream:short)
--
Duy

On Tue, May 28, 2013 at 9:01 PM, Ramkumar Ramachandra
[off-list ref] wrote:

Also, a couple of minor annoyances:

1. When f-e-r is invoked with refs/tags, we get stray output.  Atleast
it doesn't segfault, thanks to your ignore-commit patch.  Maybe
printing stray output is the right thing to do, as opposed to erroring
out.

What format did you use?

2. %>(*) only works with f-e-r atoms, not with pretty-format atoms.
This is ofcourse obvious from the implementation, but isn't it a
little consistent?

It is not. I think it's documented as well as a known implementation
limitation. It's not hard to be fixed (we could call
format_commit_message again for all entries, this time with a single
placeholder, to collect the best width). If anybody does need %>(*)%H
to work, we could do it. BTW, the way I implement get_atom_width() is
not really optimal. For n lines, we call print_value() in
get_atom_width n^2 times. We could cache the calculated width instead
and reduce that to n times.

Should we start off a new pretty-ref-formats document, where we
explicitly exclude things like %ae (because of the hex overriding
thing)?  I don't think it's a problem if documented properly.

And this one is doucmented as well, I think. I'd really like to
introduce a new --pretty option (or something) that does not accept
%xx as a hex notion, so %ae and friends won't be hidden. It's also a
good thing for compatibility because currently %H in --format produces
%H. After this, %H produces something else. It's unlikely that anybody
has done that. But it's even better if we avoid that possibility
entirely with a new option.
--
Duy

Duy Nguyen wrote:

It's because you don't pad enough spaces after %(refname:short) so the
starting point of %(upstream:short) on each line is already unaligned,
I think.

Yeah, my stupidity.  I really meant %>|(30), and that works fine.

On Tue, May 28, 2013 at 9:24 PM, Ramkumar Ramachandra
[off-list ref] wrote:

Oh, and by the way:

We're pretty close we are to replacing branch -v and branch -vv.

brv = for-each-ref --format='%(HEAD)
%C(green)%<(*)%(refname:short)%C(reset) %<(*)%(objectname:short)
%(subject)' refs/heads

brvv = for-each-ref --format='%(HEAD)
%C(green)%<(*)%(refname:short)%C(reset) %<(*)%(objectname:short)
%C(blue)%(upstream:short)%C(reset) %(subject)' refs/heads

There are small differences:

1. In branch -v, the green-color of the branch name is dependent on
%(HEAD).  Not worth ironing out, in my opinion.

2. In branch -vv, there are dependent square braces that come on when
%(refname:short) is set.  We might want to introduce an undocumented
%(refname:branchvv) for internal use by branch -vv, for backward
compatibility.

What do you think?

I think we can change -v and -vv format slightly as long as the
information remains the same. Nobody should ever parse these output
with scripts. The color can be generated from color.branch.*. A bigger
problem is show_detached(), --[no-]merged, --with and --contains. We
need to move some of those code over to for-each-ref. Another problem
is the new "branch -v" seems to less responsive than old "branch -v"
because (I think) of sorting, even if we don't need it. I checked out
your branch, made some more updates and pushed out to my
for-each-ref-pretty again. Changes are:

 - fix segfault with "for-each-ref --format=%something refs/tags/'
 - add --pretty for new format syntax and keep --format unchanged
 - add --[no-]column to for-each-ref (for "git branch" with no arguments)
 - remove branch listing code and use for-each-ref instead (41
inserts, 378 deletes, beautiful).
 - add --sort and --count to git-branch
--
Duy

Duy Nguyen wrote:

Nobody should ever parse these output
with scripts. The color can be generated from color.branch.*.

How do we implement color.branch.(current|local|remote|plain)?  In the
current code, we take a crude approach by hand-constructing argc, argv
strings and passing it to cmd_for_each_ref().  There are no
conditionals in the format syntax (and introducing one is probably not
a good idea either): so, I'm guessing these configuration variables
have to be read by for-each-ref?

A bigger
problem is show_detached(), --[no-]merged, --with and --contains. We
need to move some of those code over to for-each-ref.

I saw that you fixed this.

Another problem
is the new "branch -v" seems to less responsive than old "branch -v"
because (I think) of sorting, even if we don't need it.

Does your track-responsiveness patch fix this?

I checked out
your branch, made some more updates and pushed out to my
for-each-ref-pretty again. Changes are:

*pants* Sorry, I'm finding it hard to keep up.

On Tue, Jun 4, 2013 at 7:52 PM, Ramkumar Ramachandra [off-list ref] wrote:

Duy Nguyen wrote:

quoted

Nobody should ever parse these output
with scripts. The color can be generated from color.branch.*.

How do we implement color.branch.(current|local|remote|plain)?  In the
current code, we take a crude approach by hand-constructing argc, argv
strings and passing it to cmd_for_each_ref().  There are no
conditionals in the format syntax (and introducing one is probably not
a good idea either): so, I'm guessing these configuration variables
have to be read by for-each-ref?

Maybe. I don't really like the idea that for-each-ref reads _branch_
config. We could introduce the same set of keys but in
color.for-each-ref namespace. %C(auto) will take care of the logic and
choose the correct color key. When we replace branch's listing code
with for-each-ref, I think we could somehow override for-each-ref keys
with branch ones in-core. Too hacky?

quoted

A bigger
problem is show_detached(), --[no-]merged, --with and --contains. We
need to move some of those code over to for-each-ref.

I saw that you fixed this.

Nope. --[no-]merged and --contains seem easy. show_detached is still
WIP, mostly because detached HEAD may or may not show when you provide
a pattern to git-branch (e.g. git branch --list 'foo*') and because
HEAD is always the first item, regardless of sorting order.
get_head_description also seems more porcelain-ish than a plumbing
feature..

quoted

Another problem
is the new "branch -v" seems to less responsive than old "branch -v"
because (I think) of sorting, even if we don't need it.

Does your track-responsiveness patch fix this?

Yes.

quoted

I checked out
your branch, made some more updates and pushed out to my
for-each-ref-pretty again. Changes are:

*pants* Sorry, I'm finding it hard to keep up.

Sorry that branch was in a better shape the day I sent my previous
email. Then I kind of used it as a playground with --[no-]merged,
--contains and stuff :-P
--
Duy