threads / discuss / 46333

git-2.13.2: color.branch.local problem

Subject: git-2.13.2: color.branch.local problem

## tl;dr

6 messages between Jul 8, 2017 and Jul 9, 2017.

replies: 5people: 2as markdown or json

Leo Razoumov· Jul 8, 2017, 19:13 UTC · lore

Hi Everyone,

When I updated from git-2.10.2 to git-2.13.2 my 'color.branch.local' config setting gets ignored. Corresponding 'remote' or 'current' settings are honored and work as expected

Relevant parts of my config file is:
  [color "branch"]
    current=  reverse
    local  = red bold
    remote = green bold

I am on macosx-10.11.6 El Capitan. Git installation courtesy of homebrew.

Using command $ git branch -a --color=always I see current branch in reverse, remote branches in green, but local branches in default color (black)

Any help is greatly appreciated.

Thanks, --Leo

Jeff King· Jul 9, 2017, 09:28 UTC · re: Leo Razoumov · lore

Re: git-2.13.2: color.branch.local problem

On Sat, Jul 08, 2017 at 03:13:04PM -0400, Leo Razoumov wrote:

> When I updated from git-2.10.2 to git-2.13.2 my 'color.branch.local'
> config setting gets ignored. Corresponding 'remote' or 'current'
> settings are honored and work as expected

Looks like this is a regression from the switch to ref-filter in v2.13.0; it bisects to 949af0684 (branch: use ref-filter printing APIs, 2017-01-10). It looks like we don't ever use BRANCH_COLOR_LOCAL. The fix is a little tricky because of another nearby issue. I'll send out a patch in a moment.

-Peff

Jeff King· Jul 9, 2017, 09:57 UTC · re: Jeff King · lore

Re: git-2.13.2: color.branch.local problem

On Sun, Jul 09, 2017 at 05:28:34AM -0400, Jeff King wrote:

Show 11 quoted lines

> On Sat, Jul 08, 2017 at 03:13:04PM -0400, Leo Razoumov wrote:
> 
> > When I updated from git-2.10.2 to git-2.13.2 my 'color.branch.local'
> > config setting gets ignored. Corresponding 'remote' or 'current'
> > settings are honored and work as expected
> 
> Looks like this is a regression from the switch to ref-filter in
> v2.13.0; it bisects to 949af0684 (branch: use ref-filter printing APIs,
> 2017-01-10). It looks like we don't ever use BRANCH_COLOR_LOCAL. The fix
> is a little tricky because of another nearby issue. I'll send out a
> patch in a moment.

Here it is. This is intended for the maint branch.

  [1/3]: branch: only perform HEAD check for local branches
  [2/3]: branch: use BRANCH_COLOR_LOCAL in ref-filter format
  [3/3]: branch: set remote color in ref-filter branch immediately

 builtin/branch.c        | 15 +++++++++------
 t/t3205-branch-color.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 6 deletions(-)
 create mode 100755 t/t3205-branch-color.sh

-Peff

Jeff King· Jul 9, 2017, 09:58 UTC · re: Jeff King · lore

[PATCH 1/3] branch: only perform HEAD check for local branches

When assembling the ref-filter format to show "git branch" output, we put the "%(if)%(HEAD)" conditional at the start of the overall format. But there's no point in checking whether a remote branch matches HEAD, as it never will. The check should go inside the local conditional; we assemble that format inside the "local" strbuf.

By itself, this is just a minor optimization. But in a future patch, we'll need this refactoring to fix local-branch coloring.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/branch.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index c958e9325..a849373b7 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -335,8 +335,9 @@ static char *build_format(struct ref_filter *filter, int maxwidth, const char *r
 	struct strbuf local = STRBUF_INIT;
 	struct strbuf remote = STRBUF_INIT;
 
-	strbuf_addf(&fmt, "%%(if)%%(HEAD)%%(then)* %s%%(else)  %%(end)",
+	strbuf_addf(&local, "%%(if)%%(HEAD)%%(then)* %s%%(else)  %%(end)",
 		    branch_get_color(BRANCH_COLOR_CURRENT));
+	strbuf_addf(&remote, "  ");
 
 	if (filter->verbose) {
 		struct strbuf obname = STRBUF_INIT;

-- 
2.13.2.1066.gabaed60bd

Jeff King· Jul 9, 2017, 09:59 UTC · re: Jeff King · lore

[PATCH 2/3] branch: use BRANCH_COLOR_LOCAL in ref-filter format

Since 949af0684 (branch: use ref-filter printing APIs, 2017-01-10), git-branch's output is generated by passing a custom format to the ref-filter code. This format forgot to pass BRANCH_COLOR_LOCAL, meaning that local branches (besides the current one) were never colored at all.

We can add it in the %(if) block where we decide whether the branch is "current" or merely "local". Note that this means the current/local coloring is either/or. You can't set:

  [color "branch"]
  local = blue
  current = bold

and expect the current branch to be "bold blue". This matches the pre-949af0684 behavior.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/branch.c        |  5 +++--
 t/t3205-branch-color.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 2 deletions(-)
 create mode 100755 t/t3205-branch-color.sh

diff --git a/builtin/branch.c b/builtin/branch.c
index a849373b7..ee4e93c3a 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -335,8 +335,9 @@ static char *build_format(struct ref_filter *filter, int maxwidth, const char *r
 	struct strbuf local = STRBUF_INIT;
 	struct strbuf remote = STRBUF_INIT;
 
-	strbuf_addf(&local, "%%(if)%%(HEAD)%%(then)* %s%%(else)  %%(end)",
-		    branch_get_color(BRANCH_COLOR_CURRENT));
+	strbuf_addf(&local, "%%(if)%%(HEAD)%%(then)* %s%%(else)  %s%%(end)",
+		    branch_get_color(BRANCH_COLOR_CURRENT),
+		    branch_get_color(BRANCH_COLOR_LOCAL));
 	strbuf_addf(&remote, "  ");
 
 	if (filter->verbose) {
diff --git a/t/t3205-branch-color.sh b/t/t3205-branch-color.sh
new file mode 100755
index 000000000..9343550f5
--- /dev/null
+++ b/t/t3205-branch-color.sh
@@ -0,0 +1,44 @@
+#!/bin/sh
+
+test_description='basic branch output coloring'
+. ./test-lib.sh
+
+test_expect_success 'set up some sample branches' '
+	test_commit foo &&
+	git update-ref refs/remotes/origin/master HEAD &&
+	git update-ref refs/heads/other HEAD
+'
+
+# choose non-default colors to make sure config
+# is taking effect
+test_expect_success 'set up some color config' '
+	git config color.branch always &&
+	git config color.branch.local blue &&
+	git config color.branch.remote yellow &&
+	git config color.branch.current cyan
+'
+
+test_expect_success 'regular output shows colors' '
+	cat >expect <<-\EOF &&
+	* <CYAN>master<RESET>
+	  <BLUE>other<RESET>
+	  <YELLOW>remotes/origin/master<RESET>
+	EOF
+	git branch -a >actual.raw &&
+	test_decode_color <actual.raw >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'verbose output shows colors' '
+	oid=$(git rev-parse --short HEAD) &&
+	cat >expect <<-EOF &&
+	* <CYAN>master               <RESET> $oid foo
+	  <BLUE>other                <RESET> $oid foo
+	  <YELLOW>remotes/origin/master<RESET> $oid foo
+	EOF
+	git branch -v -a >actual.raw &&
+	test_decode_color <actual.raw >actual &&
+	test_cmp expect actual
+'
+
+test_done

-- 
2.13.2.1066.gabaed60bd

Jeff King· Jul 9, 2017, 10:00 UTC · re: Jeff King · lore

[PATCH 3/3] branch: set remote color in ref-filter branch immediately

We set the current and local branch colors at the top of the build_format() function. Let's do the same for the remote color. This saves a little bit of repetition, but more importantly it puts all of the color-setting in the same place. That makes it easier to see that we are coloring all possibilities.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/branch.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index ee4e93c3a..8a0595e11 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -338,7 +338,8 @@ static char *build_format(struct ref_filter *filter, int maxwidth, const char *r
 	strbuf_addf(&local, "%%(if)%%(HEAD)%%(then)* %s%%(else)  %s%%(end)",
 		    branch_get_color(BRANCH_COLOR_CURRENT),
 		    branch_get_color(BRANCH_COLOR_LOCAL));
-	strbuf_addf(&remote, "  ");
+	strbuf_addf(&remote, "  %s",
+		    branch_get_color(BRANCH_COLOR_REMOTE));
 
 	if (filter->verbose) {
 		struct strbuf obname = STRBUF_INIT;
@@ -361,17 +362,17 @@ static char *build_format(struct ref_filter *filter, int maxwidth, const char *r
 		else
 			strbuf_addf(&local, "%%(if)%%(upstream:track)%%(then)%%(upstream:track) %%(end)%%(contents:subject)");
 
-		strbuf_addf(&remote, "%s%%(align:%d,left)%s%%(refname:lstrip=2)%%(end)%s"
+		strbuf_addf(&remote, "%%(align:%d,left)%s%%(refname:lstrip=2)%%(end)%s"
 			    "%%(if)%%(symref)%%(then) -> %%(symref:short)"
 			    "%%(else) %s %%(contents:subject)%%(end)",
-			    branch_get_color(BRANCH_COLOR_REMOTE), maxwidth, quote_literal_for_format(remote_prefix),
+			    maxwidth, quote_literal_for_format(remote_prefix),
 			    branch_get_color(BRANCH_COLOR_RESET), obname.buf);
 		strbuf_release(&obname);
 	} else {
 		strbuf_addf(&local, "%%(refname:lstrip=2)%s%%(if)%%(symref)%%(then) -> %%(symref:short)%%(end)",
 			    branch_get_color(BRANCH_COLOR_RESET));
-		strbuf_addf(&remote, "%s%s%%(refname:lstrip=2)%s%%(if)%%(symref)%%(then) -> %%(symref:short)%%(end)",
-			    branch_get_color(BRANCH_COLOR_REMOTE), quote_literal_for_format(remote_prefix),
+		strbuf_addf(&remote, "%s%%(refname:lstrip=2)%s%%(if)%%(symref)%%(then) -> %%(symref:short)%%(end)",
+			    quote_literal_for_format(remote_prefix),
 			    branch_get_color(BRANCH_COLOR_RESET));
 	}

-- 
2.13.2.1066.gabaed60bd