log --decorate: do not leak "commit" color into the next item

In "git log --decorate", you would see the commit header like this:

commit ... (HEAD, jc/decorate-leaky-separator-color)

where "commit ... (" is painted in color.diff.commit, "HEAD" in
color.decorate.head, ", " in color.diff.commit, the branch name in
color.decorate.branch and then closing ")" in color.diff.commit.

If you wanted to paint the HEAD and local branch name in the same
color as the body text (perhaps because cyan and green are too faint
on a black-on-white terminal to be readable), you would not want to
have to say

[color "decorate"]
head = black
branch = black

because that you would not be able to reuse same configuration on a
white-on-black terminal.  You would naively expect

[color "decorate"]
head = normal
branch = normal

to work, but unfortunately it does not.  It paints the string "HEAD"
and the branch name in the same color as the opening parenthesis or
comma between the decoration elements.  This is because the code
forgets to reset the color after printing the "prefix" in its own
color.

It theoretically is possible that some people were expecting and
relying on that the attribute set as the "diff.commit" color, which
is used to draw these opening parenthesis and inter-item comma, is
inherited by the drawing of branch names, but it is not how the
coloring works everywhere else.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/config.txt b/Documentation/config.txt
index f1cf571fcd561d295e758a0d9fd3f550c22b313b..6d65033e4461703853c6dd4e5925b95719c8fc0b 100644 (file)
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -155,6 +155,13 @@ color::
        `reverse`.  The first color given is the foreground; the
        second is the background.  The position of the attribute, if
        any, doesn't matter.
        `reverse`.  The first color given is the foreground; the
        second is the background.  The position of the attribute, if
        any, doesn't matter.

++
+The attributes are meant to be reset at the beginning of each item
+in the colored output, so setting color.decorate.branch to `black`
+will paint that branch name in a plain `black`, even if the previous
+thing on the same output line (e.g. opening parenthesis before the
+list of branch names in `log --decorate` output) is set to be
+painted with `bold` or some other attribute.

 
 
 Variables
 
 
 Variables

diff --git a/log-tree.c b/log-tree.c
index 1982631ca497002b746873cd62fabbc4b7f9d3ce..11676d523553db7350d015f8b81d46698e43a6f7 100644 (file)
--- a/log-tree.c
+++ b/log-tree.c
@@ -200,6 +200,7 @@ void format_decorations(struct strbuf *sb,
        while (decoration) {
                strbuf_addstr(sb, color_commit);
                strbuf_addstr(sb, prefix);
        while (decoration) {
                strbuf_addstr(sb, color_commit);
                strbuf_addstr(sb, prefix);

+               strbuf_addstr(sb, color_reset);

                strbuf_addstr(sb, decorate_get_color(use_color, decoration->type));
                if (decoration->type == DECORATION_REF_TAG)
                        strbuf_addstr(sb, "tag: ");
                strbuf_addstr(sb, decorate_get_color(use_color, decoration->type));
                if (decoration->type == DECORATION_REF_TAG)
                        strbuf_addstr(sb, "tag: ");

diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
index 925f577a3c4652bed253787d5f540a415b16fccf..6b8ad4f5b0db4ba4e667c091225d35fe486bee5e 100755 (executable)
--- a/t/t4207-log-decoration-colors.sh
+++ b/t/t4207-log-decoration-colors.sh
@@ -44,15 +44,15 @@ test_expect_success setup '
 '
 
 cat >expected <<EOF
 '
 
 cat >expected <<EOF

-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_HEAD}HEAD${c_reset}${c_commit},\
- ${c_tag}tag: v1.0${c_reset}${c_commit},\
- ${c_tag}tag: B${c_reset}${c_commit},\
- ${c_branch}master${c_reset}${c_commit})${c_reset} B
-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_tag}tag: A1${c_reset}${c_commit},\
- ${c_remoteBranch}other/master${c_reset}${c_commit})${c_reset} A1
-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_stash}refs/stash${c_reset}${c_commit})${c_reset}\
+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD${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}${c_branch}master${c_reset}${c_commit})${c_reset} B
+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A1${c_reset}${c_commit},\
+ ${c_reset}${c_remoteBranch}other/master${c_reset}${c_commit})${c_reset} A1
+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_stash}refs/stash${c_reset}${c_commit})${c_reset}\

  On master: Changes to A.t
  On master: Changes to A.t

-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_tag}tag: A${c_reset}${c_commit})${c_reset} A
+${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A${c_reset}${c_commit})${c_reset} A

 EOF
 
 # We want log to show all, but the second parent to refs/stash is irrelevant
 EOF
 
 # We want log to show all, but the second parent to refs/stash is irrelevant