git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Subject: [PATCH v2 7/7] log --decorate: do not leak "commit" color into the next item
Date: Wed,  4 Mar 2015 13:33:23 -0800	[thread overview]
Message-ID: <1425504803-16039-8-git-send-email-gitster@pobox.com> (raw)
In-Reply-To: <1425504803-16039-1-git-send-email-gitster@pobox.com>

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>
---
 Documentation/config.txt         |  7 +++++++
 log-tree.c                       |  1 +
 t/t4207-log-decoration-colors.sh | 16 ++++++++--------
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index f1cf571..6d65033 100644
--- 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.
++
+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
diff --git a/log-tree.c b/log-tree.c
index 1982631..11676d5 100644
--- 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);
+		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: ");
diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
index 925f577..6b8ad4f 100755
--- 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
-${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
-${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
-- 
2.3.1-316-g7c93423

      parent reply	other threads:[~2015-03-04 21:33 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-18 21:03 [Q] should "color.*.<slot> = normal" emit nothing? Junio C Hamano
2015-02-18 21:44 ` [PATCH] log --decorate: do not leak "commit" color into the next item Junio C Hamano
2015-02-18 23:07   ` Jeff King
2015-02-19 18:02     ` Junio C Hamano
2015-02-20  1:42       ` Jeff King
2015-02-20 23:06         ` Junio C Hamano
2015-02-21  6:23           ` Jeff King
2015-02-21  7:09             ` Junio C Hamano
2015-03-04 21:33   ` [PATCH v2 0/7] Fix leak of color/attributes in "git log --decorate" Junio C Hamano
2015-03-04 21:33     ` [PATCH v2 1/7] Documentation/config.txt: avoid unnecessary negation Junio C Hamano
2015-03-04 21:33     ` [PATCH v2 2/7] Documentation/config.txt: explain multi-valued variables once Junio C Hamano
2015-03-04 21:33     ` [PATCH v2 3/7] Documentation/config.txt: describe the structure first and then meaning Junio C Hamano
2015-03-04 21:33     ` [PATCH v2 4/7] Documentation/config.txt: have a separate "Values" section Junio C Hamano
2015-03-04 21:33     ` [PATCH v2 5/7] Documentation/config.txt: describe 'color' value type in the " Junio C Hamano
2015-03-04 21:33     ` [PATCH v2 6/7] Documentation/config.txt: simplify boolean description in the syntax section Junio C Hamano
2015-03-04 21:33     ` Junio C Hamano [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1425504803-16039-8-git-send-email-gitster@pobox.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).