threads / patch / 42758

patchlog: decorate HEAD -> branch with the same color for branch and HEAD

Subject: [PATCH] log: decorate HEAD -> branch with the same color for branch and HEAD

## tl;dr

6 messages between Jun 30, 2016 and Jul 9, 2016. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Nguyễn Thái Ngọc Duy· Jun 30, 2016, 16:39 UTC · lore

Commit 76c61fb (log: decorate HEAD with branch name under --decorate=full, too - 2015-05-13) adds "HEAD -> branch" decoration to show current branch vs detached HEAD. The sign of whether HEAD is detached or not is "->" (vs ", "). It's too subtle for my poor eyes. If color is used, we can make the branch name's color the same as HEAD to visually emphasize that it's the current branch.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 For the original discussion of 76c61fb see
 http://thread.gmane.org/gmane.comp.version-control.git/263922

 log-tree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to log-tree.c +1 −1

diff --git a/log-tree.c b/log-tree.c
index 78a5381..440e7cc 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -267,7 +267,7 @@ void format_decorations_extended(struct strbuf *sb,
 				strbuf_addstr(sb, color_commit);
 				strbuf_addstr(sb, " -> ");
 				strbuf_addstr(sb, color_reset);
-				strbuf_addstr(sb, decorate_get_color(use_color, current_and_HEAD->type));
+				strbuf_addstr(sb, decorate_get_color(use_color, DECORATION_REF_HEAD));
 				show_name(sb, current_and_HEAD);
 			}
 			strbuf_addstr(sb, color_reset);

-- 
2.8.2.531.gd073806

Duy Nguyen· Jun 30, 2016, 16:43 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH] log: decorate HEAD -> branch with the same color for branch and HEAD

On Thu, Jun 30, 2016 at 6:39 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:

> Commit 76c61fb (log: decorate HEAD with branch name under
> --decorate=full, too - 2015-05-13)

.. and I got the commit wrong. It should be 51ff0f2 (log: decorate HEAD with branch name - 2015-03-10)

adds "HEAD -> branch" decoration to

> show current branch vs detached HEAD. The sign of whether HEAD is
> detached or not is "->" (vs ", "). It's too subtle for my poor
> eyes. If color is used, we can make the branch name's color the same
> as HEAD to visually emphasize that it's the current branch.

-- 
Duy

Jeff King· Jul 1, 2016, 06:31 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH] log: decorate HEAD -> branch with the same color for branch and HEAD

On Thu, Jun 30, 2016 at 06:39:42PM +0200, Nguyễn Thái Ngọc Duy wrote:

Show 6 quoted lines

> Commit 76c61fb (log: decorate HEAD with branch name under
> --decorate=full, too - 2015-05-13) adds "HEAD -> branch" decoration to
> show current branch vs detached HEAD. The sign of whether HEAD is
> detached or not is "->" (vs ", "). It's too subtle for my poor
> eyes. If color is used, we can make the branch name's color the same
> as HEAD to visually emphasize that it's the current branch.

Hmm. I think I like this, as it uses color to make the grouping between HEAD and its referent more clear.

We do already use colors to indicate "type", though. Which means:

  1. The branch now uses the "symref" color. Probably OK, as that is the
     point of the grouping (I wonder if the "->" in the middle should
     match in color, too).

  2. We used to color based on current_and_HEAD->type, but now that
     information isn't conveyed. However, can this really ever have been
     anything _except_ a branch?

-Peff

Michael J Gruber· Jul 1, 2016, 14:20 UTC · re: Jeff King · lore

Re: [PATCH] log: decorate HEAD -> branch with the same color for branch and HEAD

Jeff King venit, vidit, dixit 01.07.2016 08:31:

Show 24 quoted lines

> On Thu, Jun 30, 2016 at 06:39:42PM +0200, Nguyễn Thái Ngọc Duy wrote:
> 
>> Commit 76c61fb (log: decorate HEAD with branch name under
>> --decorate=full, too - 2015-05-13) adds "HEAD -> branch" decoration to
>> show current branch vs detached HEAD. The sign of whether HEAD is
>> detached or not is "->" (vs ", "). It's too subtle for my poor
>> eyes. If color is used, we can make the branch name's color the same
>> as HEAD to visually emphasize that it's the current branch.
> 
> Hmm. I think I like this, as it uses color to make the grouping between
> HEAD and its referent more clear.
> 
> We do already use colors to indicate "type", though. Which means:
> 
>   1. The branch now uses the "symref" color. Probably OK, as that is the
>      point of the grouping (I wonder if the "->" in the middle should
>      match in color, too).
> 
>   2. We used to color based on current_and_HEAD->type, but now that
>      information isn't conveyed. However, can this really ever have been
>      anything _except_ a branch?
> 
> -Peff
>

So, I rechecked, and I'm afraid I don't like the proposal.

Currently, we have a clear color code that colors refs according their type (local branch, remote branch, tag, symref). This also corresponds to status colors, for example.

Under the proposal, a checked out branch would not be colored like a local branch - note that back then, the result of the discussion was:

- If a branch foo is checked out, do not decorate by "foo", but amend
the HEAD decoration with "foo" and a symbol that denotes the connection.

So, "HEAD -> foo" is an intentional union of two decorations.

I wouldn't mind changing the color of "- > foo" if it were just an addition to "HEAD", but it is not - it's the decoration by the local branch foo.

With git branch, we use "*" as a symbol for the checked out branch...

Michael

Duy Nguyen· Jul 1, 2016, 15:13 UTC · re: Michael J Gruber · lore

Re: [PATCH] log: decorate HEAD -> branch with the same color for branch and HEAD

On Fri, Jul 1, 2016 at 4:20 PM, Michael J Gruber <git@drmicha.warpmail.net> wrote:

Show 27 quoted lines

> Jeff King venit, vidit, dixit 01.07.2016 08:31:
>> On Thu, Jun 30, 2016 at 06:39:42PM +0200, Nguyễn Thái Ngọc Duy wrote:
>>
>>> Commit 76c61fb (log: decorate HEAD with branch name under
>>> --decorate=full, too - 2015-05-13) adds "HEAD -> branch" decoration to
>>> show current branch vs detached HEAD. The sign of whether HEAD is
>>> detached or not is "->" (vs ", "). It's too subtle for my poor
>>> eyes. If color is used, we can make the branch name's color the same
>>> as HEAD to visually emphasize that it's the current branch.
>>
>> Hmm. I think I like this, as it uses color to make the grouping between
>> HEAD and its referent more clear.
>>
>> We do already use colors to indicate "type", though. Which means:
>>
>>   1. The branch now uses the "symref" color. Probably OK, as that is the
>>      point of the grouping (I wonder if the "->" in the middle should
>>      match in color, too).
>>
>>   2. We used to color based on current_and_HEAD->type, but now that
>>      information isn't conveyed. However, can this really ever have been
>>      anything _except_ a branch?
>>
>> -Peff
>>
>
> So, I rechecked, and I'm afraid I don't like the proposal.

How about a middle ground: change color of the arrow (easier to catch eyes) and leave the associated branch alone.

Show 20 quoted lines

>
> Currently, we have a clear color code that colors refs according their
> type (local branch, remote branch, tag, symref). This also corresponds
> to status colors, for example.
>
> Under the proposal, a checked out branch would not be colored like a
> local branch - note that back then, the result of the discussion was:
>
> - If a branch foo is checked out, do not decorate by "foo", but amend
> the HEAD decoration with "foo" and a symbol that denotes the connection.
>
> So, "HEAD -> foo" is an intentional union of two decorations.
>
> I wouldn't mind changing the color of "- > foo" if it were just an
> addition to "HEAD", but it is not - it's the decoration by the local
> branch foo.
>
> With git branch, we use "*" as a symbol for the checked out branch...
>
> Michael

-- 
Duy

Nguyễn Thái Ngọc Duy· Jul 9, 2016, 12:49 UTC · re: Duy Nguyen · lore

[PATCH v2] log: decorate HEAD -> branch with the same color for arrow and HEAD

Commit 76c61fb (log: decorate HEAD with branch name under --decorate=full, too - 2015-05-13) adds "HEAD -> branch" decoration to show current branch vs detached HEAD. The sign of whether HEAD is detached or not is "->" (vs ",") because the branch is always colored by type. Color the arrow the same as HEAD to visually emphasize that the following branch is HEAD, without paying too much attention to the actual separators "->" or ","

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 I'm going to take Michael's silence as no objection to the recoloring
 the arrow. So v2 changes the arrow's color instead of the branch's.

 log-tree.c                       | 2 --
 t/t4207-log-decoration-colors.sh | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

Show changes to 2 files +1 −3

log-tree.c, t/t4207-log-decoration-colors.sh

diff --git a/log-tree.c b/log-tree.c
index 78a5381..e647b08 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -263,8 +263,6 @@ void format_decorations_extended(struct strbuf *sb,
 
 			if (current_and_HEAD &&
 			    decoration->type == DECORATION_REF_HEAD) {
-				strbuf_addstr(sb, color_reset);
-				strbuf_addstr(sb, color_commit);
 				strbuf_addstr(sb, " -> ");
 				strbuf_addstr(sb, color_reset);
 				strbuf_addstr(sb, decorate_get_color(use_color, current_and_HEAD->type));
diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
index f8008b6..b972296 100755
--- a/t/t4207-log-decoration-colors.sh
+++ b/t/t4207-log-decoration-colors.sh
@@ -44,7 +44,7 @@ test_expect_success setup '
 '
 
 cat >expected <<EOF
-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD${c_reset}${c_commit} ->\
+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD ->\
  ${c_reset}${c_branch}master${c_reset}${c_commit},\
  ${c_reset}${c_tag}tag: v1.0${c_reset}${c_commit},\
  ${c_reset}${c_tag}tag: B${c_reset}${c_commit})${c_reset} B

-- 
2.8.2.537.g0965dd9