public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
* [BUG] git-blame: --color-lines ignores --ignore-rev
@ 2026-02-01  7:32 Seth McDonald
  2026-02-01 11:47 ` [PATCH] blame: fix coloring for repeated suspects René Scharfe
  0 siblings, 1 reply; 6+ messages in thread
From: Seth McDonald @ 2026-02-01  7:32 UTC (permalink / raw)
  To: git

[-- Attachment #1: Type: text/plain, Size: 4730 bytes --]

G'day all,

I believe I've found a bug regarding how git-blame(1) colours its output
when invoked with both --color-lines and --ignore-rev.

== Summary ==
git-blame(1) has two options of interest.  '--color-lines' colours a
line (in cyan by default) if it has the same blame as the preceding
line.  And '--ignore-rev <rev>' prevents any line from being blamed on
<rev>, instead blaming such lines on the most recent commit prior to
<rev> that modified those lines.

When used in combination, it is possible for consecutive lines to have
the same blame without the latter lines being coloured correctly.  I
first observed this behaviour on Git 2.47.3, but have since compiled
the latest release candidate (Git 2.53.0.rc2) and have observed the same
behaviour.

== Reproducibility ==
I was able to construct a minimal working example as follows.

First create and cd(1) into a temporary directory.  Let's name it
'git-test'.

$ mkdir git-test
$ cd git-test

Make git(1) ignore the system and global config files.  This is mainly
to aid in reproducibility.

$ export GIT_CONFIG_SYSTEM=/dev/null
$ export GIT_CONFIG_GLOBAL=/dev/null

Initialise a Git repository.

$ git init --initial-branch main
Initialized empty Git repository in /.../git-test/.git/

Now set some default values.  In particular, set blame.coloring to
repeatedLines; equivalent to the --color-lines option for git-blame(1).

$ git config set user.name 'John Git'
$ git config set user.email 'johngit@for.real'
$ git config set blame.coloring repeatedLines
$ git config set blame.date short

Create a file (let's name it 'text'), give it some text, and commit it.

$ echo Why hello there how are you | xargs -n1 > text
$ git add text
$ git commit --message 'init'
[main (root-commit) xxxxxxx] init
 1 file changed, 6 insertions(+)
 create mode 100644 text

Then fixup some punctuation in the file and commit it.

$ sed -i -e '3s/$/!/' -e '4s/h/H/' -e '6s/$/?/' text
$ git add text
$ git commit --message 'fix'
[main yyyyyyy] fix
 1 file changed, 3 insertions(+), 3 deletions(-)

If we now blame the file, we get the following.

$ git blame text
^xxxxxxx (John Git YYYY-MM-DD 1) Why
^xxxxxxx (John Git YYYY-MM-DD 2) hello
yyyyyyyy (John Git YYYY-MM-DD 3) there!
yyyyyyyy (John Git YYYY-MM-DD 4) How
^xxxxxxx (John Git YYYY-MM-DD 5) are
yyyyyyyy (John Git YYYY-MM-DD 6) you?

Where lines 2 and 4 are coloured cyan.  This is expected, since their
listed commits are the same as that on lines 1 and 3, respectively.

Now consider the output of git-blame(1) if we ignore the second commit.

$ git blame --ignore-rev HEAD text
^xxxxxxx (John Git YYYY-MM-DD 1) Why
^xxxxxxx (John Git YYYY-MM-DD 2) hello
^xxxxxxx (John Git YYYY-MM-DD 3) there!
^xxxxxxx (John Git YYYY-MM-DD 4) How
^xxxxxxx (John Git YYYY-MM-DD 5) are
^xxxxxxx (John Git YYYY-MM-DD 6) you?

My expected behaviour of git-blame(1) is to colour lines 2-6 cyan.  This
is because lines 1-6 all blame the same commit, meaning lines 2-6 all
have the same blame as their respective preceding lines.  And as the man
page for git-blame(1) states:

$ MANWIDTH=72 man git-blame | sed -n '/--color-lines$/,/^$/p'
       --color-lines
           Color line annotations in the default format differently if
           they come from the same commit as the preceding line. This
           makes it easier to distinguish code blocks introduced by
           different commits. The color defaults to cyan and can be
           adjusted using the color.blame.repeatedLines config option.

However, the actual behaviour of git-blame(1) is to colour only lines 2
and 4 cyan.  That is, it colours the output as if the --ignore-rev
option was not given.

== Environment ==
The following info was added verbatim from git-bugreport(1).

[System Info]
git version:
git version 2.53.0.rc2
cpu: x86_64
built from commit: ab380cb80b0727f7f2d7f6b17592ae6783e9820c
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
rust: disabled
gettext: enabled
libcurl: 8.14.1
OpenSSL: OpenSSL 3.5.4 30 Sep 2025
zlib: 1.3.1
SHA-1: SHA1_DC
SHA-256: SHA256_BLK
default-ref-format: files
default-hash: sha1
uname: Linux 6.12.63+deb13-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.12.63-1 (2025-12-30) x86_64
compiler info: gnuc: 14.2
libc info: glibc: 2.41
$SHELL (typically, interactive shell): /bin/bash


[Enabled Hooks]


And here's some extra info, in case it helps:

OS: Debian GNU/Linux 13 (trixie)
Terminal: GNOME Terminal 3.56.2
bash: (GNU bash) 5.2.37(1)-release (x86_64-pc-linux-gnu)
sed: (GNU sed) 4.9
xargs: (GNU findutils) 4.10.0

I'd be happy to further elaborate or provide help if needed.

-- 
Take care,
	Seth McDonald.

On-list:  2336 E8D2 FEB1 5300 692C  62A9 5839 6AD8 9243 D369
Off-list: 82B9 620E 53D0 A1AE 2D69  6111 C267 B002 0A90 0289

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 322 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] blame: fix coloring for repeated suspects
  2026-02-01  7:32 [BUG] git-blame: --color-lines ignores --ignore-rev Seth McDonald
@ 2026-02-01 11:47 ` René Scharfe
  2026-02-02  2:02   ` Seth McDonald
  2026-02-02 12:42   ` Junio C Hamano
  0 siblings, 2 replies; 6+ messages in thread
From: René Scharfe @ 2026-02-01 11:47 UTC (permalink / raw)
  To: Seth McDonald, git

The option --ignore-rev passes the blame to an older commit.  This can
cause adjacent scoreboard entries to blame the same commit.  Currently
we only look a the present entry when determining whether a line needs
to be colored for --color-lines.  Check the previous entry as well.

Reported-by: Seth McDonald <sethmcmail@pm.me>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
 builtin/blame.c         | 13 +++++++++----
 t/t8012-blame-colors.sh | 14 ++++++++++++++
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 6044973462..bb460346e6 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -454,7 +454,8 @@ static void determine_line_heat(struct commit_info *ci, const char **dest_color)
 	*dest_color = colorfield[i].col;
 }
 
-static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int opt)
+static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent,
+		       int opt, struct blame_entry *prev_ent)
 {
 	int cnt;
 	const char *cp;
@@ -485,7 +486,10 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int
 			the_hash_algo->hexsz : (size_t) abbrev;
 
 		if (opt & OUTPUT_COLOR_LINE) {
-			if (cnt > 0) {
+			if (cnt > 0 ||
+			    (prev_ent &&
+			     oideq(&suspect->commit->object.oid,
+				   &prev_ent->suspect->commit->object.oid))) {
 				color = repeated_meta_color;
 				reset = GIT_COLOR_RESET;
 			} else  {
@@ -571,7 +575,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int
 
 static void output(struct blame_scoreboard *sb, int option)
 {
-	struct blame_entry *ent;
+	struct blame_entry *ent, *prev_ent = NULL;
 
 	if (option & OUTPUT_PORCELAIN) {
 		for (ent = sb->ent; ent; ent = ent->next) {
@@ -593,7 +597,8 @@ static void output(struct blame_scoreboard *sb, int option)
 		if (option & OUTPUT_PORCELAIN)
 			emit_porcelain(sb, ent, option);
 		else {
-			emit_other(sb, ent, option);
+			emit_other(sb, ent, option, prev_ent);
+			prev_ent = ent;
 		}
 	}
 }
diff --git a/t/t8012-blame-colors.sh b/t/t8012-blame-colors.sh
index 3d77352650..5562eba436 100755
--- a/t/t8012-blame-colors.sh
+++ b/t/t8012-blame-colors.sh
@@ -28,6 +28,20 @@ test_expect_success 'colored blame colors contiguous lines' '
 	test_line_count = 3 H.expect
 '
 
+test_expect_success 'color lines becoming contiguous due to --ignore-rev' '
+	mv hello.c hello.orig &&
+	sed "s/	/    /g" <hello.orig >hello.c &&
+	git add hello.c &&
+	git commit -m"tabs to spaces" &&
+	git -c color.blame.repeatedLines=yellow blame --color-lines --ignore-rev=HEAD hello.c >actual.raw &&
+	test_decode_color <actual.raw >actual &&
+	grep "<YELLOW>" <actual >darkened &&
+	grep "(F" darkened > F.expect &&
+	grep "(H" darkened > H.expect &&
+	test_line_count = 2 F.expect &&
+	test_line_count = 3 H.expect
+'
+
 test_expect_success 'color by age consistently colors old code' '
 	git blame --color-by-age hello.c >actual.raw &&
 	git -c blame.coloring=highlightRecent blame hello.c >actual.raw.2 &&
-- 
2.52.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] blame: fix coloring for repeated suspects
  2026-02-01 11:47 ` [PATCH] blame: fix coloring for repeated suspects René Scharfe
@ 2026-02-02  2:02   ` Seth McDonald
  2026-02-02 12:42   ` Junio C Hamano
  1 sibling, 0 replies; 6+ messages in thread
From: Seth McDonald @ 2026-02-02  2:02 UTC (permalink / raw)
  To: René Scharfe; +Cc: git

[-- Attachment #1: Type: text/plain, Size: 494 bytes --]

Hi René,

On Sun, 01 Feb 2026 at 12:47:53 +0100, René Scharfe wrote:
[...]
>  builtin/blame.c         | 13 +++++++++----
>  t/t8012-blame-colors.sh | 14 ++++++++++++++
>  2 files changed, 23 insertions(+), 4 deletions(-)

I've compiled git(1) with your patch and it seems to fix the issue.
Thanks for the fast response!

-- 
Take care,
	Seth McDonald.

On-list:  2336 E8D2 FEB1 5300 692C  62A9 5839 6AD8 9243 D369
Off-list: 82B9 620E 53D0 A1AE 2D69  6111 C267 B002 0A90 0289

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 322 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] blame: fix coloring for repeated suspects
  2026-02-01 11:47 ` [PATCH] blame: fix coloring for repeated suspects René Scharfe
  2026-02-02  2:02   ` Seth McDonald
@ 2026-02-02 12:42   ` Junio C Hamano
  2026-02-02 16:24     ` René Scharfe
  1 sibling, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2026-02-02 12:42 UTC (permalink / raw)
  To: René Scharfe; +Cc: Seth McDonald, git

René Scharfe <l.s.r@web.de> writes:

> The option --ignore-rev passes the blame to an older commit.  This can
> cause adjacent scoreboard entries to blame the same commit.  Currently
> we only look a the present entry when determining whether a line needs

"look at"?

> to be colored for --color-lines.  Check the previous entry as well.

While this should work, I am kind of surprised that this has to done
as a sepecial case.  It often happens that two adjacent blocks may
be originally pass their blames to different parents of a merge, but
then the blame passes down through both branches down to the same
ancestor, at which point these two blocks need to be merged back
into the same source again, and I was hoping that a helper function
for it would be called to take care of this case as well.

In any case, thaks for a fix, and with a test, which is great.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] blame: fix coloring for repeated suspects
  2026-02-02 12:42   ` Junio C Hamano
@ 2026-02-02 16:24     ` René Scharfe
  2026-02-02 17:56       ` Junio C Hamano
  0 siblings, 1 reply; 6+ messages in thread
From: René Scharfe @ 2026-02-02 16:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Seth McDonald, git

On 2/2/26 1:42 PM, Junio C Hamano wrote:
> René Scharfe <l.s.r@web.de> writes:
> 
>> The option --ignore-rev passes the blame to an older commit.  This can
>> cause adjacent scoreboard entries to blame the same commit.  Currently
>> we only look a the present entry when determining whether a line needs
> 
> "look at"?

Yes.

>> to be colored for --color-lines.  Check the previous entry as well.
> 
> While this should work, I am kind of surprised that this has to done
> as a sepecial case.  It often happens that two adjacent blocks may
> be originally pass their blames to different parents of a merge, but
> then the blame passes down through both branches down to the same
> ancestor, at which point these two blocks need to be merged back
> into the same source again, and I was hoping that a helper function
> for it would be called to take care of this case as well.
Do you mean blame_coalesce()?  It is called, but won't merge entries
that are not ignored with those that are.  And we do need to keep them
separate for blame.markignoredlines to work.

René


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] blame: fix coloring for repeated suspects
  2026-02-02 16:24     ` René Scharfe
@ 2026-02-02 17:56       ` Junio C Hamano
  0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2026-02-02 17:56 UTC (permalink / raw)
  To: René Scharfe; +Cc: Seth McDonald, git

René Scharfe <l.s.r@web.de> writes:

>> While this should work, I am kind of surprised that this has to done
>> as a sepecial case.  It often happens that two adjacent blocks may
>> be originally pass their blames to different parents of a merge, but
>> then the blame passes down through both branches down to the same
>> ancestor, at which point these two blocks need to be merged back
>> into the same source again, and I was hoping that a helper function
>> for it would be called to take care of this case as well.
>
> Do you mean blame_coalesce()?  It is called, but won't merge entries
> that are not ignored with those that are.  And we do need to keep them
> separate for blame.markignoredlines to work.

Yes, and sigh.  I know "ignore these commits" came much later than
the main part of blame, and I am not surprised if the way it was
bolted on was not designed to mesh well with existing framework like
the blame_coalesce() helper and what it tried to achieve.

Anyway, thanks for a fix.  Will queue.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-02-02 17:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-01  7:32 [BUG] git-blame: --color-lines ignores --ignore-rev Seth McDonald
2026-02-01 11:47 ` [PATCH] blame: fix coloring for repeated suspects René Scharfe
2026-02-02  2:02   ` Seth McDonald
2026-02-02 12:42   ` Junio C Hamano
2026-02-02 16:24     ` René Scharfe
2026-02-02 17:56       ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox