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

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

[Nguyễn Thái Ngọc Duy]

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(-)

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

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

[Duy Nguyen]

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

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

[Jeff King]

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

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

[Michael J Gruber]

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.

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

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

[Duy Nguyen]

On Fri, Jul 1, 2016 at 4:20 PM, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> 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.

>
> 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

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

[Nguyễn Thái Ngọc Duy]

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(-)

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