[PATCH] blame: fix unblamable and ignored lines in porcelain mode

[PATCH] blame: fix unblamable and ignored lines in porcelain mode

[Karthik Nayak]

The 'git-blame(1)' command allows users to ignore specific revisions via
the '--ignore-rev <rev>' and '--ignore-revs-file <file>' flags. These
flags are often combined with the 'blame.markIgnoredLines' and
'blame.markUnblamableLines' config options. These config options prefix
ignored and unblamable lines with a '?' and '*', respectively.

However, this option was never extended to the porcelain mode of
'git-blame(1)'. Since the documentation does not indicate this
exclusion, it is a bug.

Fix this by ensuring porcelain mode also prints the markers and add
tests to verify the behavior.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/blame.c              | 10 ++++++++++
 t/t8013-blame-ignore-revs.sh | 30 ++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

Karthik Nayak (1):
      blame: fix unblamable and ignored lines in porcelain mode



base-commit: 683c54c999c301c2cd6f715c411407c413b1d84e
change-id: 20250321-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-4af46f02847e

Thanks
- Karthik
---

---
 builtin/blame.c              | 10 ++++++++++
 t/t8013-blame-ignore-revs.sh | 30 ++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/builtin/blame.c b/builtin/blame.c
index c470654c7e..9a8d7ce7af 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -360,6 +360,11 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
 	struct blame_origin *suspect = ent->suspect;
 	char hex[GIT_MAX_HEXSZ + 1];
 
+	if (mark_unblamable_lines && ent->unblamable)
+		putchar('*');
+	if (mark_ignored_lines && ent->ignored)
+		putchar('?');
+
 	oid_to_hex_r(hex, &suspect->commit->object.oid);
 	printf("%s %d %d %d\n",
 	       hex,
@@ -372,6 +377,11 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
 	for (cnt = 0; cnt < ent->num_lines; cnt++) {
 		char ch;
 		if (cnt) {
+			if (mark_unblamable_lines && ent->unblamable)
+				putchar('*');
+			if (mark_ignored_lines && ent->ignored)
+				putchar('?');
+
 			printf("%s %d %d\n", hex,
 			       ent->s_lno + 1 + cnt,
 			       ent->lno + 1 + cnt);
diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh
index 370b768149..2722eb4598 100755
--- a/t/t8013-blame-ignore-revs.sh
+++ b/t/t8013-blame-ignore-revs.sh
@@ -158,6 +158,16 @@ test_expect_success mark_unblamable_lines '
 	test_cmp expect actual
 '
 
+test_expect_success 'mark_unblamable_lines porcelain' '
+	sha=$(git rev-parse Y) &&
+
+	git -c blame.markUnblamableLines=false blame -p --ignore-rev Y file >blame_raw &&
+	sed "s/^${sha}/*${sha}/g" blame_raw >expect &&
+
+	git -c blame.markUnblamableLines=true blame -p --ignore-rev Y file >actual &&
+	test_cmp expect actual
+'
+
 # Commit Z will touch the first two lines.  Y touched all four.
 # 	A--B--X--Y--Z
 # The blame output when ignoring Z should be:
@@ -191,6 +201,26 @@ test_expect_success mark_ignored_lines '
 	! test_cmp expect actual
 '
 
+test_expect_success 'mark_ignored_lines porcelain' '
+	sha=$(git rev-parse Y) &&
+
+	git -c blame.markIgnoredLines=true blame -p --ignore-rev Z file | grep $sha >blame_raw &&
+
+	echo "?" >expect &&
+
+	sed -n "1p" blame_raw | cut -c1 >actual &&
+	test_cmp expect actual &&
+
+	sed -n "2p" blame_raw | cut -c1 >actual &&
+	test_cmp expect actual &&
+
+	sed -n "3p" blame_raw | cut -c1 >actual &&
+	! test_cmp expect actual &&
+
+	sed -n "4p" blame_raw | cut -c1 >actual &&
+	! test_cmp expect actual
+'
+
 # For ignored revs that added 'unblamable' lines and more recent commits changed
 # the blamable lines, mark the unblamable lines with a
 # '*'

Re: [PATCH] blame: fix unblamable and ignored lines in porcelain mode

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> However, this option was never extended to the porcelain mode of
> 'git-blame(1)'. Since the documentation does not indicate this
> exclusion, it is a bug.

I agree it is a bug when people added ignore or unblamable support
that they did not _consider_ what to do with their new pieces of
information to help porcelain writers.  It is not a bug in the code
per-se, but it is a bug in the brain of these people ;-)

But prefixing random garbage to the commit object name line in the
porcelain mode output does not sound like the right solution to the
bug, either.

When enhancing an existing output format, make sure that your
changes will have minimum empact to existing parsers that do not
know about your extension.  It is reasonably expected that existing
Porcelain scripts reading from --porcelain mode output works by

 - Recognizing a line that match "^[0-9a-f]{40} \d+ \d+ \d+$" and
   take it as the beginning of a new record;

 - Collect all info lines before the payload line.  Lines that
   describe per-commit information are not repeated if it is already
   shown, so remember them when you see the commit for the first
   time, and recall them when you recognize the commit you already
   saw.

 - A payload line is indented with HT and terminates the record.

If you start to add unrecognizable garbage to the line with very
well known fixed format that is used as record delimiter, you would
break the existing parsers, which is not a very nice thing to do.
Are there other and better ways you can think of to add new pieces
of information like this in a way with less severe damage?

Re: [PATCH] blame: fix unblamable and ignored lines in porcelain mode

[Patrick Steinhardt]

On Sun, Mar 23, 2025 at 08:58:03AM -0700, Junio C Hamano wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
> 
> > However, this option was never extended to the porcelain mode of
> > 'git-blame(1)'. Since the documentation does not indicate this
> > exclusion, it is a bug.
> 
> I agree it is a bug when people added ignore or unblamable support
> that they did not _consider_ what to do with their new pieces of
> information to help porcelain writers.  It is not a bug in the code
> per-se, but it is a bug in the brain of these people ;-)
> 
> But prefixing random garbage to the commit object name line in the
> porcelain mode output does not sound like the right solution to the
> bug, either.
> 
> When enhancing an existing output format, make sure that your
> changes will have minimum empact to existing parsers that do not
> know about your extension.  It is reasonably expected that existing
> Porcelain scripts reading from --porcelain mode output works by
> 
>  - Recognizing a line that match "^[0-9a-f]{40} \d+ \d+ \d+$" and
>    take it as the beginning of a new record;
> 
>  - Collect all info lines before the payload line.  Lines that
>    describe per-commit information are not repeated if it is already
>    shown, so remember them when you see the commit for the first
>    time, and recall them when you recognize the commit you already
>    saw.
> 
>  - A payload line is indented with HT and terminates the record.
> 
> If you start to add unrecognizable garbage to the line with very
> well known fixed format that is used as record delimiter, you would
> break the existing parsers, which is not a very nice thing to do.
> Are there other and better ways you can think of to add new pieces
> of information like this in a way with less severe damage?

I think the porcelain mode is already built so that it can be extended
with arbitrary new information, no? In `emit_one_suspect_detail()` we
end up printing one line per info we want to display. I would have
expected that we can extend that function to also print information
around unblamable or ignored commits, like we already do for boundary
commits. E.g. something like the patch further down.

Thanks!

Patrick

diff --git a/builtin/blame.c b/builtin/blame.c
index c470654c7ec..cd8322e2619 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -255,7 +255,8 @@ static void write_filename_info(struct blame_origin *suspect)
  * the first time each commit appears in the output (unless the
  * user has specifically asked for us to repeat).
  */
-static int emit_one_suspect_detail(struct blame_origin *suspect, int repeat)
+static int emit_one_suspect_detail(struct blame_entry *ent,
+				   struct blame_origin *suspect, int repeat)
 {
 	struct commit_info ci = COMMIT_INFO_INIT;
 
@@ -275,6 +276,10 @@ static int emit_one_suspect_detail(struct blame_origin *suspect, int repeat)
 	printf("summary %s\n", ci.summary.buf);
 	if (suspect->commit->object.flags & UNINTERESTING)
 		printf("boundary\n");
+	if (mark_unblamable_lines && ent->unblamable)
+		printf("unblamable\n");
+	if (mark_ignored_lines && ent->ignored)
+		printf("ignored\n");
 
 	commit_info_destroy(&ci);
 
@@ -295,7 +300,7 @@ static void found_guilty_entry(struct blame_entry *ent, void *data)
 		printf("%s %d %d %d\n",
 		       oid_to_hex(&suspect->commit->object.oid),
 		       ent->s_lno + 1, ent->lno + 1, ent->num_lines);
-		emit_one_suspect_detail(suspect, 0);
+		emit_one_suspect_detail(ent, suspect, 0);
 		write_filename_info(suspect);
 		maybe_flush_or_die(stdout, "stdout");
 	}
@@ -344,9 +349,10 @@ static const char *format_time(timestamp_t time, const char *tz_str,
 #define OUTPUT_COLOR_LINE           (1U<<10)
 #define OUTPUT_SHOW_AGE_WITH_COLOR  (1U<<11)
 
-static void emit_porcelain_details(struct blame_origin *suspect, int repeat)
+static void emit_porcelain_details(struct blame_entry *ent,
+				   struct blame_origin *suspect, int repeat)
 {
-	if (emit_one_suspect_detail(suspect, repeat) ||
+	if (emit_one_suspect_detail(ent, suspect, repeat) ||
 	    (suspect->commit->object.flags & MORE_THAN_ONE_PATH))
 		write_filename_info(suspect);
 }
@@ -366,7 +372,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
 	       ent->s_lno + 1,
 	       ent->lno + 1,
 	       ent->num_lines);
-	emit_porcelain_details(suspect, repeat);
+	emit_porcelain_details(ent, suspect, repeat);
 
 	cp = blame_nth_line(sb, ent->lno);
 	for (cnt = 0; cnt < ent->num_lines; cnt++) {
@@ -376,7 +382,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
 			       ent->s_lno + 1 + cnt,
 			       ent->lno + 1 + cnt);
 			if (repeat)
-				emit_porcelain_details(suspect, 1);
+				emit_porcelain_details(ent, suspect, 1);
 		}
 		putchar('\t');
 		do {

Re: [PATCH] blame: fix unblamable and ignored lines in porcelain mode

[Toon Claes]

Patrick Steinhardt <ps@pks.im> writes:

> I think the porcelain mode is already built so that it can be extended
> with arbitrary new information, no? In `emit_one_suspect_detail()` we
> end up printing one line per info we want to display. I would have
> expected that we can extend that function to also print information
> around unblamable or ignored commits, like we already do for boundary
> commits. E.g. something like the patch further down.

Yeah, I think the porcelain format exists to be easy to machine-parse.
Having an optional prefix symbol on the commit OID would complicate
process that.

And I've been thinking about a similar solution as you've been
suggesting below. I was only wondering whether we only do this when
using `--line-porcelain`. When using `--porcelain` the function
`emit_one_suspect_detail()` doesn't print most of the commit info when
it was already printed. But the "unblamable" and "ignored" info might be
different for each line, even if they blame down to the same commit.

-- 
Toon

Re: [PATCH] blame: fix unblamable and ignored lines in porcelain mode

[Karthik Nayak]

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

Toon Claes <toon@iotcl.com> writes:

> Patrick Steinhardt <ps@pks.im> writes:
>
>> I think the porcelain mode is already built so that it can be extended
>> with arbitrary new information, no? In `emit_one_suspect_detail()` we
>> end up printing one line per info we want to display. I would have
>> expected that we can extend that function to also print information
>> around unblamable or ignored commits, like we already do for boundary
>> commits. E.g. something like the patch further down.
>
> Yeah, I think the porcelain format exists to be easy to machine-parse.
> Having an optional prefix symbol on the commit OID would complicate
> process that.
>
> And I've been thinking about a similar solution as you've been
> suggesting below. I was only wondering whether we only do this when
> using `--line-porcelain`. When using `--porcelain` the function
> `emit_one_suspect_detail()` doesn't print most of the commit info when
> it was already printed. But the "unblamable" and "ignored" info might be
> different for each line, even if they blame down to the same commit.
>

I'm curious, how would it be different, if they blame down to the same
commit? My understanding was "unblamable" and "ignored" are tied to
commits.

> --
> Toon

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH] blame: fix unblamable and ignored lines in porcelain mode

[Toon Claes]

Karthik Nayak <karthik.188@gmail.com> writes:

> I'm curious, how would it be different, if they blame down to the same
> commit? My understanding was "unblamable" and "ignored" are tied to
> commits.

Let me include an example, let's blame `varint.h`.

First have a look at the non-porcelain format:

    $ git blame varint.h -l
    d2c1898571a6a2324593e92163e8754880e0c1fb (Junio C Hamano 2012-04-03 15:53:08 -0700 1) #ifndef VARINT_H
    d2c1898571a6a2324593e92163e8754880e0c1fb (Junio C Hamano 2012-04-03 15:53:08 -0700 2) #define VARINT_H
    d2c1898571a6a2324593e92163e8754880e0c1fb (Junio C Hamano 2012-04-03 15:53:08 -0700 3)
    554544276a604c144df45efcb060c80aa322088c (Denton Liu     2019-04-29 04:28:14 -0400 4) int encode_varint(uintmax_t, unsigned char *);
    554544276a604c144df45efcb060c80aa322088c (Denton Liu     2019-04-29 04:28:14 -0400 5) uintmax_t decode_varint(const unsigned char **);
    d2c1898571a6a2324593e92163e8754880e0c1fb (Junio C Hamano 2012-04-03 15:53:08 -0700 6)
    d2c1898571a6a2324593e92163e8754880e0c1fb (Junio C Hamano 2012-04-03 15:53:08 -0700 7) #endif /* VARINT_H */

Now if we put `554544276a604c144df45efcb060c80aa322088c` in `.git-blame-ignore-revs`:

    $ git -c blame.markUnblamableLines=true -c blame.markIgnoredLines=true blame varint.h --ignore-revs-file .git-blame-ignore-revs -l
    d2c1898571a6a2324593e92163e8754880e0c1fb (Junio C Hamano 2012-04-03 15:53:08 -0700 1) #ifndef VARINT_H
    d2c1898571a6a2324593e92163e8754880e0c1fb (Junio C Hamano 2012-04-03 15:53:08 -0700 2) #define VARINT_H
    d2c1898571a6a2324593e92163e8754880e0c1fb (Junio C Hamano 2012-04-03 15:53:08 -0700 3)
    ?d2c1898571a6a2324593e92163e8754880e0c1f (Junio C Hamano 2012-04-03 15:53:08 -0700 4) int encode_varint(uintmax_t, unsigned char *);
    ?d2c1898571a6a2324593e92163e8754880e0c1f (Junio C Hamano 2012-04-03 15:53:08 -0700 5) uintmax_t decode_varint(const unsigned char **);
    d2c1898571a6a2324593e92163e8754880e0c1fb (Junio C Hamano 2012-04-03 15:53:08 -0700 6)
    d2c1898571a6a2324593e92163e8754880e0c1fb (Junio C Hamano 2012-04-03 15:53:08 -0700 7) #endif /* VARINT_H */

If we compare that to the porcelain format:

    $ git blame varint.h -l --porcelain
    d2c1898571a6a2324593e92163e8754880e0c1fb 1 1 3
    author Junio C Hamano
    author-mail <gitster@pobox.com>
    author-time 1333493588
    author-tz -0700
    committer Junio C Hamano
    committer-mail <gitster@pobox.com>
    committer-time 1333495484
    committer-tz -0700
    summary varint: make it available outside the context of pack
    filename varint.h
            #ifndef VARINT_H
    d2c1898571a6a2324593e92163e8754880e0c1fb 2 2
            #define VARINT_H
    d2c1898571a6a2324593e92163e8754880e0c1fb 3 3

    554544276a604c144df45efcb060c80aa322088c 4 4 2
    author Denton Liu
    author-mail <liu.denton@gmail.com>
    author-time 1556526494
    author-tz -0400
    committer Junio C Hamano
    committer-mail <gitster@pobox.com>
    committer-time 1557037206
    committer-tz +0900
    summary *.[ch]: remove extern from function declarations using spatch
    previous ffac537e6cbbf934b08745a378932722df287a53 varint.h
    filename varint.h
            int encode_varint(uintmax_t, unsigned char *);
    554544276a604c144df45efcb060c80aa322088c 5 5
            uintmax_t decode_varint(const unsigned char **);
    d2c1898571a6a2324593e92163e8754880e0c1fb 8 6 2

    d2c1898571a6a2324593e92163e8754880e0c1fb 9 7
            #endif /* VARINT_H */

And now with the `.git-blame-ignore-revs` file:

    $ git -c blame.markUnblamableLines=true -c blame.markIgnoredLines=true blame varint.h --ignore-revs-file .git-blame-ignore-revs -l --porcelain
    d2c1898571a6a2324593e92163e8754880e0c1fb 1 1 3
    author Junio C Hamano
    author-mail <gitster@pobox.com>
    author-time 1333493588
    author-tz -0700
    committer Junio C Hamano
    committer-mail <gitster@pobox.com>
    committer-time 1333495484
    committer-tz -0700
    summary varint: make it available outside the context of pack
    filename varint.h
            #ifndef VARINT_H
    d2c1898571a6a2324593e92163e8754880e0c1fb 2 2
            #define VARINT_H
    d2c1898571a6a2324593e92163e8754880e0c1fb 3 3

    d2c1898571a6a2324593e92163e8754880e0c1fb 6 4 2
            int encode_varint(uintmax_t, unsigned char *);
    d2c1898571a6a2324593e92163e8754880e0c1fb 7 5
            uintmax_t decode_varint(const unsigned char **);
    d2c1898571a6a2324593e92163e8754880e0c1fb 8 6 2

    d2c1898571a6a2324593e92163e8754880e0c1fb 9 7
            #endif /* VARINT_H */

So every line now blames down to commit
d2c1898571a6a2324593e92163e8754880e0c1fb. The lines which used to
blame down to 554544276a604c144df45efcb060c80aa322088c should be marked
as "ignored", but we only emit the details once for each commit. The
commit details (author, committer) are only relevant once, but the
"ignored" info can differ for each line (as you also can see in the
non-porcelain format).

We could make the output look something like:

    $ git -c blame.markUnblamableLines=true -c blame.markIgnoredLines=true blame varint.h --ignore-revs-file .git-blame-ignore-revs -l --porcelain
    d2c1898571a6a2324593e92163e8754880e0c1fb 1 1 3
    author Junio C Hamano
    author-mail <gitster@pobox.com>
    author-time 1333493588
    author-tz -0700
    committer Junio C Hamano
    committer-mail <gitster@pobox.com>
    committer-time 1333495484
    committer-tz -0700
    summary varint: make it available outside the context of pack
    filename varint.h
            #ifndef VARINT_H
    d2c1898571a6a2324593e92163e8754880e0c1fb 2 2
            #define VARINT_H
    d2c1898571a6a2324593e92163e8754880e0c1fb 3 3

    d2c1898571a6a2324593e92163e8754880e0c1fb 6 4 2
    ignored
            int encode_varint(uintmax_t, unsigned char *);
    d2c1898571a6a2324593e92163e8754880e0c1fb 7 5
    ignored
            uintmax_t decode_varint(const unsigned char **);
    d2c1898571a6a2324593e92163e8754880e0c1fb 8 6 2

    d2c1898571a6a2324593e92163e8754880e0c1fb 9 7
            #endif /* VARINT_H */

It feels odd to me only the "ignored" info is emitted and the rest
of the details isn't. But that might be just me...

-- 
Toon

Re: [PATCH] blame: fix unblamable and ignored lines in porcelain mode

[Karthik Nayak]

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

Toon Claes <toon@iotcl.com> writes:

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> I'm curious, how would it be different, if they blame down to the same
>> commit? My understanding was "unblamable" and "ignored" are tied to
>> commits.

[snip]

> And now with the `.git-blame-ignore-revs` file:
>
>     $ git -c blame.markUnblamableLines=true -c blame.markIgnoredLines=true blame varint.h --ignore-revs-file .git-blame-ignore-revs -l --porcelain
>     d2c1898571a6a2324593e92163e8754880e0c1fb 1 1 3
>     author Junio C Hamano
>     author-mail <gitster@pobox.com>
>     author-time 1333493588
>     author-tz -0700
>     committer Junio C Hamano
>     committer-mail <gitster@pobox.com>
>     committer-time 1333495484
>     committer-tz -0700
>     summary varint: make it available outside the context of pack
>     filename varint.h
>             #ifndef VARINT_H
>     d2c1898571a6a2324593e92163e8754880e0c1fb 2 2
>             #define VARINT_H
>     d2c1898571a6a2324593e92163e8754880e0c1fb 3 3
>
>     d2c1898571a6a2324593e92163e8754880e0c1fb 6 4 2
>             int encode_varint(uintmax_t, unsigned char *);
>     d2c1898571a6a2324593e92163e8754880e0c1fb 7 5
>             uintmax_t decode_varint(const unsigned char **);
>     d2c1898571a6a2324593e92163e8754880e0c1fb 8 6 2
>
>     d2c1898571a6a2324593e92163e8754880e0c1fb 9 7
>             #endif /* VARINT_H */
>
> So every line now blames down to commit
> d2c1898571a6a2324593e92163e8754880e0c1fb. The lines which used to
> blame down to 554544276a604c144df45efcb060c80aa322088c should be marked
> as "ignored", but we only emit the details once for each commit. The
> commit details (author, committer) are only relevant once, but the
> "ignored" info can differ for each line (as you also can see in the
> non-porcelain format).
>

Ah! So if a rev is ignored via the `--ignore-rev[s-file]` flag, then the
parent revision is shown in the blame. It could happen that in porcelain
mode the parent revision is clubbed with previous lines if they share
the same revision. This would skip the 'unblamable' or 'ignored'
information.

This would be solved in '--line-porcelain' since details aren't clubbed.

I agree, it makes the most sense to only do this in 'line-porcelain'
mode.

> We could make the output look something like:
>
>     $ git -c blame.markUnblamableLines=true -c blame.markIgnoredLines=true blame varint.h --ignore-revs-file .git-blame-ignore-revs -l --porcelain
>     d2c1898571a6a2324593e92163e8754880e0c1fb 1 1 3
>     author Junio C Hamano
>     author-mail <gitster@pobox.com>
>     author-time 1333493588
>     author-tz -0700
>     committer Junio C Hamano
>     committer-mail <gitster@pobox.com>
>     committer-time 1333495484
>     committer-tz -0700
>     summary varint: make it available outside the context of pack
>     filename varint.h
>             #ifndef VARINT_H
>     d2c1898571a6a2324593e92163e8754880e0c1fb 2 2
>             #define VARINT_H
>     d2c1898571a6a2324593e92163e8754880e0c1fb 3 3
>
>     d2c1898571a6a2324593e92163e8754880e0c1fb 6 4 2
>     ignored
>             int encode_varint(uintmax_t, unsigned char *);
>     d2c1898571a6a2324593e92163e8754880e0c1fb 7 5
>     ignored
>             uintmax_t decode_varint(const unsigned char **);
>     d2c1898571a6a2324593e92163e8754880e0c1fb 8 6 2
>
>     d2c1898571a6a2324593e92163e8754880e0c1fb 9 7
>             #endif /* VARINT_H */
>
> It feels odd to me only the "ignored" info is emitted and the rest
> of the details isn't. But that might be just me...
>

I'm with you on this, this would also require us to explain this odd
exclusion where only for 'ignored' and 'unblamable' lines we output
details on every commit. But the other way is also an exclusion, where
we would say that 'ignored' and 'unblamable' lines are only shown in
'--line-porcelain'.

But the latter can be extended into the former in the future but not the
other way around. So I would say it makes more sense to restrict it to
'--line-porcelain' in that sense.

> --
> Toon

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH] blame: fix unblamable and ignored lines in porcelain mode

[Junio C Hamano]

Toon Claes <toon@iotcl.com> writes:

> It feels odd to me only the "ignored" info is emitted and the rest
> of the details isn't. But that might be just me...

If we have more per-line (and not per-commit/file tuple) pieces of
information, we would have to treat them just like you had "ignored"
in your illustration above.  It is "ignored" that feels "odd", since
it is the only single oddball right now.

Re: [PATCH] blame: fix unblamable and ignored lines in porcelain mode

[Karthik Nayak]

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

Patrick Steinhardt <ps@pks.im> writes:

> On Sun, Mar 23, 2025 at 08:58:03AM -0700, Junio C Hamano wrote:
>> Karthik Nayak <karthik.188@gmail.com> writes:
>>
>> > However, this option was never extended to the porcelain mode of
>> > 'git-blame(1)'. Since the documentation does not indicate this
>> > exclusion, it is a bug.
>>
>> I agree it is a bug when people added ignore or unblamable support
>> that they did not _consider_ what to do with their new pieces of
>> information to help porcelain writers.  It is not a bug in the code
>> per-se, but it is a bug in the brain of these people ;-)
>>
>> But prefixing random garbage to the commit object name line in the
>> porcelain mode output does not sound like the right solution to the
>> bug, either.
>>
>> When enhancing an existing output format, make sure that your
>> changes will have minimum empact to existing parsers that do not
>> know about your extension.  It is reasonably expected that existing
>> Porcelain scripts reading from --porcelain mode output works by
>>
>>  - Recognizing a line that match "^[0-9a-f]{40} \d+ \d+ \d+$" and
>>    take it as the beginning of a new record;
>>
>>  - Collect all info lines before the payload line.  Lines that
>>    describe per-commit information are not repeated if it is already
>>    shown, so remember them when you see the commit for the first
>>    time, and recall them when you recognize the commit you already
>>    saw.
>>
>>  - A payload line is indented with HT and terminates the record.
>>
>> If you start to add unrecognizable garbage to the line with very
>> well known fixed format that is used as record delimiter, you would
>> break the existing parsers, which is not a very nice thing to do.
>> Are there other and better ways you can think of to add new pieces
>> of information like this in a way with less severe damage?
>
> I think the porcelain mode is already built so that it can be extended
> with arbitrary new information, no? In `emit_one_suspect_detail()` we
> end up printing one line per info we want to display. I would have
> expected that we can extend that function to also print information
> around unblamable or ignored commits, like we already do for boundary
> commits. E.g. something like the patch further down.
>

This indeed looks like a much better way of doing this. Let me

> Thanks!
>
> Patrick
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index c470654c7ec..cd8322e2619 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -255,7 +255,8 @@ static void write_filename_info(struct blame_origin *suspect)
>   * the first time each commit appears in the output (unless the
>   * user has specifically asked for us to repeat).
>   */
> -static int emit_one_suspect_detail(struct blame_origin *suspect, int repeat)
> +static int emit_one_suspect_detail(struct blame_entry *ent,
> +				   struct blame_origin *suspect, int repeat)
>  {
>  	struct commit_info ci = COMMIT_INFO_INIT;
>
> @@ -275,6 +276,10 @@ static int emit_one_suspect_detail(struct blame_origin *suspect, int repeat)
>  	printf("summary %s\n", ci.summary.buf);
>  	if (suspect->commit->object.flags & UNINTERESTING)
>  		printf("boundary\n");
> +	if (mark_unblamable_lines && ent->unblamable)
> +		printf("unblamable\n");
> +	if (mark_ignored_lines && ent->ignored)
> +		printf("ignored\n");
>
>  	commit_info_destroy(&ci);
>
> @@ -295,7 +300,7 @@ static void found_guilty_entry(struct blame_entry *ent, void *data)
>  		printf("%s %d %d %d\n",
>  		       oid_to_hex(&suspect->commit->object.oid),
>  		       ent->s_lno + 1, ent->lno + 1, ent->num_lines);
> -		emit_one_suspect_detail(suspect, 0);
> +		emit_one_suspect_detail(ent, suspect, 0);
>  		write_filename_info(suspect);
>  		maybe_flush_or_die(stdout, "stdout");
>  	}
> @@ -344,9 +349,10 @@ static const char *format_time(timestamp_t time, const char *tz_str,
>  #define OUTPUT_COLOR_LINE           (1U<<10)
>  #define OUTPUT_SHOW_AGE_WITH_COLOR  (1U<<11)
>
> -static void emit_porcelain_details(struct blame_origin *suspect, int repeat)
> +static void emit_porcelain_details(struct blame_entry *ent,
> +				   struct blame_origin *suspect, int repeat)
>  {
> -	if (emit_one_suspect_detail(suspect, repeat) ||
> +	if (emit_one_suspect_detail(ent, suspect, repeat) ||
>  	    (suspect->commit->object.flags & MORE_THAN_ONE_PATH))
>  		write_filename_info(suspect);
>  }
> @@ -366,7 +372,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
>  	       ent->s_lno + 1,
>  	       ent->lno + 1,
>  	       ent->num_lines);
> -	emit_porcelain_details(suspect, repeat);
> +	emit_porcelain_details(ent, suspect, repeat);
>
>  	cp = blame_nth_line(sb, ent->lno);
>  	for (cnt = 0; cnt < ent->num_lines; cnt++) {
> @@ -376,7 +382,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
>  			       ent->s_lno + 1 + cnt,
>  			       ent->lno + 1 + cnt);
>  			if (repeat)
> -				emit_porcelain_details(suspect, 1);
> +				emit_porcelain_details(ent, suspect, 1);
>  		}
>  		putchar('\t');
>  		do {

Thanks Patrick, I will send in the new version with this and modified
tests.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH] blame: fix unblamable and ignored lines in porcelain mode

[Karthik Nayak]

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

Junio C Hamano <gitster@pobox.com> writes:

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> However, this option was never extended to the porcelain mode of
>> 'git-blame(1)'. Since the documentation does not indicate this
>> exclusion, it is a bug.
>
> I agree it is a bug when people added ignore or unblamable support
> that they did not _consider_ what to do with their new pieces of
> information to help porcelain writers.  It is not a bug in the code
> per-se, but it is a bug in the brain of these people ;-)
>
> But prefixing random garbage to the commit object name line in the
> porcelain mode output does not sound like the right solution to the
> bug, either.
>
> When enhancing an existing output format, make sure that your
> changes will have minimum empact to existing parsers that do not
> know about your extension.  It is reasonably expected that existing
> Porcelain scripts reading from --porcelain mode output works by
>
>  - Recognizing a line that match "^[0-9a-f]{40} \d+ \d+ \d+$" and
>    take it as the beginning of a new record;
>
>  - Collect all info lines before the payload line.  Lines that
>    describe per-commit information are not repeated if it is already
>    shown, so remember them when you see the commit for the first
>    time, and recall them when you recognize the commit you already
>    saw.
>
>  - A payload line is indented with HT and terminates the record.
>
> If you start to add unrecognizable garbage to the line with very
> well known fixed format that is used as record delimiter, you would
> break the existing parsers, which is not a very nice thing to do.
> Are there other and better ways you can think of to add new pieces
> of information like this in a way with less severe damage?

Fair enough, I was having this argument and convinced myself because the
option is an explicit setting. So users who have
'blame.markIgnoredLines' or/and 'blame.markUnblamableLines' set and are
using one of the 'ignore-rev' options would be expecting either an '?'
or a '*' as mentioned in the documentation.

Let's me figure if there is a more backward-compatible way of doing
this.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH v2] blame: print unblamable and ignored commits in porcelain mode

[Karthik Nayak]

The 'git-blame(1)' command allows users to ignore specific revisions via
the '--ignore-rev <rev>' and '--ignore-revs-file <file>' flags. These
flags are often combined with the 'blame.markIgnoredLines' and
'blame.markUnblamableLines' config options. These config options prefix
ignored and unblamable lines with a '?' and '*', respectively.

However, this option was never extended to the porcelain mode of
'git-blame(1)'. Since the documentation does not indicate this
exclusion, it is a bug.

Fix this by printing 'ignored' and 'unblamable' respectively for the
options when using the porcelain modes.

Helped-by: Patrick Steinhardt <ps@pks.im>
Helped-by: Toon Claes <toon@iotcl.com>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
Changes in v2:
- Instead of printing the markers before the SHA in porcelain
  mode and breaking scripts and backward compatability, let's 
  instead add a newline printing 'unblamable' or 'ignored'.
  This is printed per line in both the porcelain modes. 
- Link to v1: https://lore.kernel.org/r/20250321-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v1-1-44b562d9beb8@gmail.com
---
Range-diff versus v1:

1:  9650db628f < -:  ---------- blame: fix unblamable and ignored lines in porcelain mode
-:  ---------- > 1:  6bbfc0cbd2 blame: print unblamable and ignored commits in porcelain mode
---
 Documentation/blame-options.adoc |  3 ++-
 Documentation/git-blame.adoc     |  9 +++++----
 builtin/blame.c                  | 15 +++++++++++++++
 t/t8013-blame-ignore-revs.sh     | 20 ++++++++++++++++++++
 4 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/Documentation/blame-options.adoc b/Documentation/blame-options.adoc
index aa77406d4e..19ea187238 100644
--- a/Documentation/blame-options.adoc
+++ b/Documentation/blame-options.adoc
@@ -125,7 +125,8 @@ take effect.
 	another commit will be marked with a `?` in the blame output.  If the
 	`blame.markUnblamableLines` config option is set, then those lines touched
 	by an ignored commit that we could not attribute to another revision are
-	marked with a '*'.
+	marked with a '*'. In the porcelain modes, we print 'ignored' and
+	'unblamable' on a newline respectively.
 
 --ignore-revs-file <file>::
 	Ignore revisions listed in `file`, which must be in the same format as an
diff --git a/Documentation/git-blame.adoc b/Documentation/git-blame.adoc
index f75ed44790..e438d28625 100644
--- a/Documentation/git-blame.adoc
+++ b/Documentation/git-blame.adoc
@@ -135,10 +135,11 @@ header elements later.
 The porcelain format generally suppresses commit information that has
 already been seen. For example, two lines that are blamed to the same
 commit will both be shown, but the details for that commit will be shown
-only once. This is more efficient, but may require more state be kept by
-the reader. The `--line-porcelain` option can be used to output full
-commit information for each line, allowing simpler (but less efficient)
-usage like:
+only once. Information which is specific to individual lines will not be
+grouped together, like revs to be marked 'ignored' or 'unblamable'. This
+is more efficient, but may require more state be kept by the reader. The
+`--line-porcelain` option can be used to output full commit information
+for each line, allowing simpler (but less efficient) usage like:
 
 	# count the number of lines attributed to each author
 	git blame --line-porcelain file |
diff --git a/builtin/blame.c b/builtin/blame.c
index c470654c7e..528bfef249 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -351,6 +351,19 @@ static void emit_porcelain_details(struct blame_origin *suspect, int repeat)
 		write_filename_info(suspect);
 }
 
+/*
+ * Information which needs to be printed per-line goes here. Any
+ * information which can be clubbed on a commit/file level, should
+ * be printed via 'emit_one_suspect_detail()'.
+ */
+static void emit_per_line_details(struct blame_entry *ent)
+{
+	if (mark_unblamable_lines && ent->unblamable)
+		printf("unblamable\n");
+	if (mark_ignored_lines && ent->ignored)
+		printf("ignored\n");
+}
+
 static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
 			   int opt)
 {
@@ -367,6 +380,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
 	       ent->lno + 1,
 	       ent->num_lines);
 	emit_porcelain_details(suspect, repeat);
+	emit_per_line_details(ent);
 
 	cp = blame_nth_line(sb, ent->lno);
 	for (cnt = 0; cnt < ent->num_lines; cnt++) {
@@ -377,6 +391,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
 			       ent->lno + 1 + cnt);
 			if (repeat)
 				emit_porcelain_details(suspect, 1);
+			emit_per_line_details(ent);
 		}
 		putchar('\t');
 		do {
diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh
index 370b768149..306fc61057 100755
--- a/t/t8013-blame-ignore-revs.sh
+++ b/t/t8013-blame-ignore-revs.sh
@@ -158,6 +158,16 @@ test_expect_success mark_unblamable_lines '
 	test_cmp expect actual
 '
 
+for opt in --porcelain --line-porcelain
+do
+	test_expect_success 'mark_unblamable_lines with $opt' '
+		sha=$(git rev-parse Y) &&
+
+		git -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&
+		test $(grep ^unblamable actual | wc -l) -eq 2
+	'
+done
+
 # Commit Z will touch the first two lines.  Y touched all four.
 # 	A--B--X--Y--Z
 # The blame output when ignoring Z should be:
@@ -191,6 +201,16 @@ test_expect_success mark_ignored_lines '
 	! test_cmp expect actual
 '
 
+for opt in --porcelain --line-porcelain
+do
+	test_expect_success 'mark_ignored_lines line_porcelain' '
+		sha=$(git rev-parse Y) &&
+
+		git -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&
+		test $(grep ^ignored actual | wc -l) -eq 2
+	'
+done
+
 # For ignored revs that added 'unblamable' lines and more recent commits changed
 # the blamable lines, mark the unblamable lines with a
 # '*'

---

base-commit: 683c54c999c301c2cd6f715c411407c413b1d84e
change-id: 20250321-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-4af46f02847e

Thanks
- Karthik

Re: [PATCH v2] blame: print unblamable and ignored commits in porcelain mode

[Eric Sunshine]

On Wed, Mar 26, 2025 at 5:07 PM Karthik Nayak <karthik.188@gmail.com> wrote:
> The 'git-blame(1)' command allows users to ignore specific revisions via
> the '--ignore-rev <rev>' and '--ignore-revs-file <file>' flags. These
> flags are often combined with the 'blame.markIgnoredLines' and
> 'blame.markUnblamableLines' config options. These config options prefix
> ignored and unblamable lines with a '?' and '*', respectively.
>
> However, this option was never extended to the porcelain mode of
> 'git-blame(1)'. Since the documentation does not indicate this
> exclusion, it is a bug.
>
> Fix this by printing 'ignored' and 'unblamable' respectively for the
> options when using the porcelain modes.
>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
> diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh
> @@ -158,6 +158,16 @@ test_expect_success mark_unblamable_lines '
> +for opt in --porcelain --line-porcelain
> +do
> +       test_expect_success 'mark_unblamable_lines with $opt' '

This test title is going to display literal "$opt" rather than the
intended option. Fix this by replacing the single quotes with double
quotes:

    test_expect_success "mark_unblamable_lines with $opt" '

> +               sha=$(git rev-parse Y) &&
> +
> +               git -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&
> +               test $(grep ^unblamable actual | wc -l) -eq 2
> +       '
> +done
> @@ -191,6 +201,16 @@ test_expect_success mark_ignored_lines '
> +for opt in --porcelain --line-porcelain
> +do
> +       test_expect_success 'mark_ignored_lines line_porcelain' '

Similarly, this is going to display the same title for both cases,
which isn't as helpful as it could be. Presumably, you instead wanted
this (using double quotes):

     test_expect_success "mark_ignored_lines with $opt" '

> +               sha=$(git rev-parse Y) &&
> +
> +               git -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&
> +               test $(grep ^ignored actual | wc -l) -eq 2
> +       '

Re: [PATCH v2] blame: print unblamable and ignored commits in porcelain mode

[Karthik Nayak]

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

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Wed, Mar 26, 2025 at 5:07 PM Karthik Nayak <karthik.188@gmail.com> wrote:
>> The 'git-blame(1)' command allows users to ignore specific revisions via
>> the '--ignore-rev <rev>' and '--ignore-revs-file <file>' flags. These
>> flags are often combined with the 'blame.markIgnoredLines' and
>> 'blame.markUnblamableLines' config options. These config options prefix
>> ignored and unblamable lines with a '?' and '*', respectively.
>>
>> However, this option was never extended to the porcelain mode of
>> 'git-blame(1)'. Since the documentation does not indicate this
>> exclusion, it is a bug.
>>
>> Fix this by printing 'ignored' and 'unblamable' respectively for the
>> options when using the porcelain modes.
>>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>> diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh
>> @@ -158,6 +158,16 @@ test_expect_success mark_unblamable_lines '
>> +for opt in --porcelain --line-porcelain
>> +do
>> +       test_expect_success 'mark_unblamable_lines with $opt' '
>
> This test title is going to display literal "$opt" rather than the
> intended option. Fix this by replacing the single quotes with double
> quotes:
>
>     test_expect_success "mark_unblamable_lines with $opt" '
>

What a silly miss. Thanks for pointing out.

>> +               sha=$(git rev-parse Y) &&
>> +
>> +               git -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&
>> +               test $(grep ^unblamable actual | wc -l) -eq 2
>> +       '
>> +done
>> @@ -191,6 +201,16 @@ test_expect_success mark_ignored_lines '
>> +for opt in --porcelain --line-porcelain
>> +do
>> +       test_expect_success 'mark_ignored_lines line_porcelain' '
>
> Similarly, this is going to display the same title for both cases,
> which isn't as helpful as it could be. Presumably, you instead wanted
> this (using double quotes):
>
>      test_expect_success "mark_ignored_lines with $opt" '
>

Yup, this needs to be fixed too. Thanks again.

>> +               sha=$(git rev-parse Y) &&
>> +
>> +               git -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&
>> +               test $(grep ^ignored actual | wc -l) -eq 2
>> +       '

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v2] blame: print unblamable and ignored commits in porcelain mode

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

>> +       test_expect_success 'mark_unblamable_lines with $opt' '
>
> This test title is going to display literal "$opt" rather than the
> intended option. Fix this by replacing the single quotes with double
> quotes:
>
>     test_expect_success "mark_unblamable_lines with $opt" '

Well spotted.  Sorry for making all developers suffer the
consequence of a mistake I made 20 years ago.  In retrospect, I
really should have anticipated that it would become a common pattern
to have test_expect_success inside a loop, and made this part also
be interpolated, to make it look similar to the next parameter.

Re: [PATCH v2] blame: print unblamable and ignored commits in porcelain mode

[Patrick Steinhardt]

On Wed, Mar 26, 2025 at 10:06:10PM +0100, Karthik Nayak wrote:
> diff --git a/builtin/blame.c b/builtin/blame.c
> index c470654c7e..528bfef249 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -351,6 +351,19 @@ static void emit_porcelain_details(struct blame_origin *suspect, int repeat)
>  		write_filename_info(suspect);
>  }
>  
> +/*
> + * Information which needs to be printed per-line goes here. Any
> + * information which can be clubbed on a commit/file level, should
> + * be printed via 'emit_one_suspect_detail()'.
> + */
> +static void emit_per_line_details(struct blame_entry *ent)

Tiny nit, feel free to ignore: should this something like
`emit_porcelain_per_line_details()` to highlight that this is part of
the porcelain format?

> +{
> +	if (mark_unblamable_lines && ent->unblamable)
> +		printf("unblamable\n");
> +	if (mark_ignored_lines && ent->ignored)
> +		printf("ignored\n");
> +}
> +

Another tiny nit: you may use `puts()` instead of `printf()`. I don't
mind it much though, both versions work equally well.

> diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh
> index 370b768149..306fc61057 100755
> --- a/t/t8013-blame-ignore-revs.sh
> +++ b/t/t8013-blame-ignore-revs.sh
> @@ -158,6 +158,16 @@ test_expect_success mark_unblamable_lines '
>  	test_cmp expect actual
>  '
>  
> +for opt in --porcelain --line-porcelain
> +do
> +	test_expect_success 'mark_unblamable_lines with $opt' '
> +		sha=$(git rev-parse Y) &&
> +
> +		git -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&
> +		test $(grep ^unblamable actual | wc -l) -eq 2
> +	'
> +done
> +

Okay, makes sense. We cannot batch the information on the first time
we've seen the commit here because both the "unblamable" and "ignored"
properties are properties of the line, not of the commit. So we'd expect
to see the information per line in both modes.

Patrick

Re: [PATCH v2] blame: print unblamable and ignored commits in porcelain mode

[Karthik Nayak]

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

Patrick Steinhardt <ps@pks.im> writes:

> On Wed, Mar 26, 2025 at 10:06:10PM +0100, Karthik Nayak wrote:
>> diff --git a/builtin/blame.c b/builtin/blame.c
>> index c470654c7e..528bfef249 100644
>> --- a/builtin/blame.c
>> +++ b/builtin/blame.c
>> @@ -351,6 +351,19 @@ static void emit_porcelain_details(struct blame_origin *suspect, int repeat)
>>  		write_filename_info(suspect);
>>  }
>>
>> +/*
>> + * Information which needs to be printed per-line goes here. Any
>> + * information which can be clubbed on a commit/file level, should
>> + * be printed via 'emit_one_suspect_detail()'.
>> + */
>> +static void emit_per_line_details(struct blame_entry *ent)
>
> Tiny nit, feel free to ignore: should this something like
> `emit_porcelain_per_line_details()` to highlight that this is part of
> the porcelain format?
>

That's a great point, will add that in.

>> +{
>> +	if (mark_unblamable_lines && ent->unblamable)
>> +		printf("unblamable\n");
>> +	if (mark_ignored_lines && ent->ignored)
>> +		printf("ignored\n");
>> +}
>> +
>
> Another tiny nit: you may use `puts()` instead of `printf()`. I don't
> mind it much though, both versions work equally well.
>

Yeah, I think some compilers also do this translation. But I'll change
it as anyways I'm going to push a new version.

>> diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh
>> index 370b768149..306fc61057 100755
>> --- a/t/t8013-blame-ignore-revs.sh
>> +++ b/t/t8013-blame-ignore-revs.sh
>> @@ -158,6 +158,16 @@ test_expect_success mark_unblamable_lines '
>>  	test_cmp expect actual
>>  '
>>
>> +for opt in --porcelain --line-porcelain
>> +do
>> +	test_expect_success 'mark_unblamable_lines with $opt' '
>> +		sha=$(git rev-parse Y) &&
>> +
>> +		git -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&
>> +		test $(grep ^unblamable actual | wc -l) -eq 2
>> +	'
>> +done
>> +
>
> Okay, makes sense. We cannot batch the information on the first time
> we've seen the commit here because both the "unblamable" and "ignored"
> properties are properties of the line, not of the commit. So we'd expect
> to see the information per line in both modes.
>
> Patrick

Thanks for the review.
Karthik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH v3] blame: print unblamable and ignored commits in porcelain mode

[Karthik Nayak]

The 'git-blame(1)' command allows users to ignore specific revisions via
the '--ignore-rev <rev>' and '--ignore-revs-file <file>' flags. These
flags are often combined with the 'blame.markIgnoredLines' and
'blame.markUnblamableLines' config options. These config options prefix
ignored and unblamable lines with a '?' and '*', respectively.

However, this option was never extended to the porcelain mode of
'git-blame(1)'. Since the documentation does not indicate this
exclusion, it is a bug.

Fix this by printing 'ignored' and 'unblamable' respectively for the
options when using the porcelain modes.

Helped-by: Patrick Steinhardt <ps@pks.im>
Helped-by: Toon Claes <toon@iotcl.com>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
Changes in v3:
- Use double-qoutes in the test to ensure correct variable dereference.
- Fix incorrect test name. 
- Rename the function from 'emit_per_line_details()' to
  'emit_porcelain_per_line_details()' to be more descriptive.
- Ues 'puts()' instead of 'printf()'.
- Link to v2: https://lore.kernel.org/r/20250326-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v2-1-79037e17a74b@gmail.com

Changes in v2:
- Instead of printing the markers before the SHA in porcelain
  mode and breaking scripts and backward compatability, let's 
  instead add a newline printing 'unblamable' or 'ignored'.
  This is printed per line in both the porcelain modes. 
- Link to v1: https://lore.kernel.org/r/20250321-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v1-1-44b562d9beb8@gmail.com
---
Range-diff versus v2:

1:  3a4012718b ! 1:  a80353ab00 blame: print unblamable and ignored commits in porcelain mode
    @@ builtin/blame.c: static void emit_porcelain_details(struct blame_origin *suspect
     + * information which can be clubbed on a commit/file level, should
     + * be printed via 'emit_one_suspect_detail()'.
     + */
    -+static void emit_per_line_details(struct blame_entry *ent)
    ++static void emit_porcelain_per_line_details(struct blame_entry *ent)
     +{
     +	if (mark_unblamable_lines && ent->unblamable)
    -+		printf("unblamable\n");
    ++		puts("unblamable\n");
     +	if (mark_ignored_lines && ent->ignored)
    -+		printf("ignored\n");
    ++		puts("ignored\n");
     +}
     +
      static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
    @@ builtin/blame.c: static void emit_porcelain(struct blame_scoreboard *sb, struct
      	       ent->lno + 1,
      	       ent->num_lines);
      	emit_porcelain_details(suspect, repeat);
    -+	emit_per_line_details(ent);
    ++	emit_porcelain_per_line_details(ent);
      
      	cp = blame_nth_line(sb, ent->lno);
      	for (cnt = 0; cnt < ent->num_lines; cnt++) {
    @@ builtin/blame.c: static void emit_porcelain(struct blame_scoreboard *sb, struct
      			       ent->lno + 1 + cnt);
      			if (repeat)
      				emit_porcelain_details(suspect, 1);
    -+			emit_per_line_details(ent);
    ++			emit_porcelain_per_line_details(ent);
      		}
      		putchar('\t');
      		do {
    @@ t/t8013-blame-ignore-revs.sh: test_expect_success mark_unblamable_lines '
      
     +for opt in --porcelain --line-porcelain
     +do
    -+	test_expect_success 'mark_unblamable_lines with $opt' '
    ++	test_expect_success "mark_unblamable_lines with $opt" '
     +		sha=$(git rev-parse Y) &&
     +
     +		git -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&
    @@ t/t8013-blame-ignore-revs.sh: test_expect_success mark_ignored_lines '
      
     +for opt in --porcelain --line-porcelain
     +do
    -+	test_expect_success 'mark_ignored_lines line_porcelain' '
    ++	test_expect_success "mark_ignored_lines with $opt" '
     +		sha=$(git rev-parse Y) &&
     +
     +		git -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&
---
 Documentation/blame-options.adoc |  3 ++-
 Documentation/git-blame.adoc     |  9 +++++----
 builtin/blame.c                  | 15 +++++++++++++++
 t/t8013-blame-ignore-revs.sh     | 20 ++++++++++++++++++++
 4 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/Documentation/blame-options.adoc b/Documentation/blame-options.adoc
index aa77406d4e..19ea187238 100644
--- a/Documentation/blame-options.adoc
+++ b/Documentation/blame-options.adoc
@@ -125,7 +125,8 @@ take effect.
 	another commit will be marked with a `?` in the blame output.  If the
 	`blame.markUnblamableLines` config option is set, then those lines touched
 	by an ignored commit that we could not attribute to another revision are
-	marked with a '*'.
+	marked with a '*'. In the porcelain modes, we print 'ignored' and
+	'unblamable' on a newline respectively.
 
 --ignore-revs-file <file>::
 	Ignore revisions listed in `file`, which must be in the same format as an
diff --git a/Documentation/git-blame.adoc b/Documentation/git-blame.adoc
index f75ed44790..e438d28625 100644
--- a/Documentation/git-blame.adoc
+++ b/Documentation/git-blame.adoc
@@ -135,10 +135,11 @@ header elements later.
 The porcelain format generally suppresses commit information that has
 already been seen. For example, two lines that are blamed to the same
 commit will both be shown, but the details for that commit will be shown
-only once. This is more efficient, but may require more state be kept by
-the reader. The `--line-porcelain` option can be used to output full
-commit information for each line, allowing simpler (but less efficient)
-usage like:
+only once. Information which is specific to individual lines will not be
+grouped together, like revs to be marked 'ignored' or 'unblamable'. This
+is more efficient, but may require more state be kept by the reader. The
+`--line-porcelain` option can be used to output full commit information
+for each line, allowing simpler (but less efficient) usage like:
 
 	# count the number of lines attributed to each author
 	git blame --line-porcelain file |
diff --git a/builtin/blame.c b/builtin/blame.c
index c470654c7e..8a73b7be4a 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -351,6 +351,19 @@ static void emit_porcelain_details(struct blame_origin *suspect, int repeat)
 		write_filename_info(suspect);
 }
 
+/*
+ * Information which needs to be printed per-line goes here. Any
+ * information which can be clubbed on a commit/file level, should
+ * be printed via 'emit_one_suspect_detail()'.
+ */
+static void emit_porcelain_per_line_details(struct blame_entry *ent)
+{
+	if (mark_unblamable_lines && ent->unblamable)
+		puts("unblamable\n");
+	if (mark_ignored_lines && ent->ignored)
+		puts("ignored\n");
+}
+
 static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
 			   int opt)
 {
@@ -367,6 +380,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
 	       ent->lno + 1,
 	       ent->num_lines);
 	emit_porcelain_details(suspect, repeat);
+	emit_porcelain_per_line_details(ent);
 
 	cp = blame_nth_line(sb, ent->lno);
 	for (cnt = 0; cnt < ent->num_lines; cnt++) {
@@ -377,6 +391,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
 			       ent->lno + 1 + cnt);
 			if (repeat)
 				emit_porcelain_details(suspect, 1);
+			emit_porcelain_per_line_details(ent);
 		}
 		putchar('\t');
 		do {
diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh
index 370b768149..c5a7533232 100755
--- a/t/t8013-blame-ignore-revs.sh
+++ b/t/t8013-blame-ignore-revs.sh
@@ -158,6 +158,16 @@ test_expect_success mark_unblamable_lines '
 	test_cmp expect actual
 '
 
+for opt in --porcelain --line-porcelain
+do
+	test_expect_success "mark_unblamable_lines with $opt" '
+		sha=$(git rev-parse Y) &&
+
+		git -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&
+		test $(grep ^unblamable actual | wc -l) -eq 2
+	'
+done
+
 # Commit Z will touch the first two lines.  Y touched all four.
 # 	A--B--X--Y--Z
 # The blame output when ignoring Z should be:
@@ -191,6 +201,16 @@ test_expect_success mark_ignored_lines '
 	! test_cmp expect actual
 '
 
+for opt in --porcelain --line-porcelain
+do
+	test_expect_success "mark_ignored_lines with $opt" '
+		sha=$(git rev-parse Y) &&
+
+		git -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&
+		test $(grep ^ignored actual | wc -l) -eq 2
+	'
+done
+
 # For ignored revs that added 'unblamable' lines and more recent commits changed
 # the blamable lines, mark the unblamable lines with a
 # '*'

---

base-commit: 683c54c999c301c2cd6f715c411407c413b1d84e
change-id: 20250321-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-4af46f02847e

Thanks
- Karthik

Re: [PATCH v3] blame: print unblamable and ignored commits in porcelain mode

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> +static void emit_porcelain_per_line_details(struct blame_entry *ent)
> +{
> +	if (mark_unblamable_lines && ent->unblamable)
> +		puts("unblamable\n");
> +	if (mark_ignored_lines && ent->ignored)
> +		puts("ignored\n");
> +}

Doesn't puts(3), unlike fputs(3), add its own trailing newline to
stdout?

Re: [PATCH v3] blame: print unblamable and ignored commits in porcelain mode

[Phillip Wood]

On 30/03/2025 05:56, Junio C Hamano wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
> 
>> +static void emit_porcelain_per_line_details(struct blame_entry *ent)
>> +{
>> +	if (mark_unblamable_lines && ent->unblamable)
>> +		puts("unblamable\n");
>> +	if (mark_ignored_lines && ent->ignored)
>> +		puts("ignored\n");
>> +}
> 
> Doesn't puts(3), unlike fputs(3), add its own trailing newline to
> stdout?

Indeed. Perhaps the regression test would be more effective if it used 
test_cmp rather than

     test $(grep ^ignored actual | wc -l) -eq 2

which misses this bug and gives no output if it fails.

Best Wishes

Phillip

[PATCH v4] blame: print unblamable and ignored commits in porcelain mode

[Karthik Nayak]

The 'git-blame(1)' command allows users to ignore specific revisions via
the '--ignore-rev <rev>' and '--ignore-revs-file <file>' flags. These
flags are often combined with the 'blame.markIgnoredLines' and
'blame.markUnblamableLines' config options. These config options prefix
ignored and unblamable lines with a '?' and '*', respectively.

However, this option was never extended to the porcelain mode of
'git-blame(1)'. Since the documentation does not indicate this
exclusion, it is a bug.

Fix this by printing 'ignored' and 'unblamable' respectively for the
options when using the porcelain modes.

Helped-by: Patrick Steinhardt <ps@pks.im>
Helped-by: Toon Claes <toon@iotcl.com>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
Changes in v4:
- Remove extra newline in 'puts'. Modify the test to compare the
  entire output, the earlier test missed the extraneous newline.
- Link to v3: https://lore.kernel.org/r/20250329-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v3-1-10f695ae519a@gmail.com

Changes in v3:
- Use double-qoutes in the test to ensure correct variable dereference.
- Fix incorrect test name. 
- Rename the function from 'emit_per_line_details()' to
  'emit_porcelain_per_line_details()' to be more descriptive.
- Ues 'puts()' instead of 'printf()'.
- Link to v2:
https://lore.kernel.org/r/20250326-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v2-1-79037e17a74b@gmail.com

Changes in v2:
- Instead of printing the markers before the SHA in porcelain
  mode and breaking scripts and backward compatability, let's 
  instead add a newline printing 'unblamable' or 'ignored'.
  This is printed per line in both the porcelain modes. 
- Link to v1:
https://lore.kernel.org/r/20250321-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v1-1-44b562d9beb8@gmail.com
---
Range-diff versus v3:

1:  bc359248f6 ! 1:  5250fb436e blame: print unblamable and ignored commits in porcelain mode
    @@ builtin/blame.c: static void emit_porcelain_details(struct blame_origin *suspect
     +static void emit_porcelain_per_line_details(struct blame_entry *ent)
     +{
     +	if (mark_unblamable_lines && ent->unblamable)
    -+		puts("unblamable\n");
    ++		puts("unblamable");
     +	if (mark_ignored_lines && ent->ignored)
    -+		puts("ignored\n");
    ++		puts("ignored");
     +}
     +
      static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
    @@ t/t8013-blame-ignore-revs.sh: test_expect_success mark_unblamable_lines '
     +	test_expect_success "mark_unblamable_lines with $opt" '
     +		sha=$(git rev-parse Y) &&
     +
    ++		git -c blame.markUnblamableLines=false blame $opt --ignore-rev Y file >raw &&
    ++		sed -e "s/^\ty3/unblamable\n&/" raw >expect &&
    ++		cp expect raw &&
    ++		sed -e "s/^\ty4/unblamable\n&/" raw >expect &&
    ++
     +		git -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&
    -+		test $(grep ^unblamable actual | wc -l) -eq 2
    ++		test_cmp expect actual
     +	'
     +done
     +
    @@ t/t8013-blame-ignore-revs.sh: test_expect_success mark_ignored_lines '
     +	test_expect_success "mark_ignored_lines with $opt" '
     +		sha=$(git rev-parse Y) &&
     +
    ++		git -c blame.markIgnoredLines=false blame $opt --ignore-rev Z file >raw &&
    ++		sed -e "s/^\tline-one-Z/ignored\n&/" raw >expect &&
    ++		cp expect raw &&
    ++		sed -e "s/^\tline-two-Z/ignored\n&/" raw >expect &&
    ++
     +		git -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&
    -+		test $(grep ^ignored actual | wc -l) -eq 2
    ++		test_cmp expect actual
     +	'
     +done
     +
---
 Documentation/blame-options.adoc |  3 ++-
 Documentation/git-blame.adoc     |  9 +++++----
 builtin/blame.c                  | 15 +++++++++++++++
 t/t8013-blame-ignore-revs.sh     | 30 ++++++++++++++++++++++++++++++
 4 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/Documentation/blame-options.adoc b/Documentation/blame-options.adoc
index aa77406d4e..19ea187238 100644
--- a/Documentation/blame-options.adoc
+++ b/Documentation/blame-options.adoc
@@ -125,7 +125,8 @@ take effect.
 	another commit will be marked with a `?` in the blame output.  If the
 	`blame.markUnblamableLines` config option is set, then those lines touched
 	by an ignored commit that we could not attribute to another revision are
-	marked with a '*'.
+	marked with a '*'. In the porcelain modes, we print 'ignored' and
+	'unblamable' on a newline respectively.
 
 --ignore-revs-file <file>::
 	Ignore revisions listed in `file`, which must be in the same format as an
diff --git a/Documentation/git-blame.adoc b/Documentation/git-blame.adoc
index f75ed44790..e438d28625 100644
--- a/Documentation/git-blame.adoc
+++ b/Documentation/git-blame.adoc
@@ -135,10 +135,11 @@ header elements later.
 The porcelain format generally suppresses commit information that has
 already been seen. For example, two lines that are blamed to the same
 commit will both be shown, but the details for that commit will be shown
-only once. This is more efficient, but may require more state be kept by
-the reader. The `--line-porcelain` option can be used to output full
-commit information for each line, allowing simpler (but less efficient)
-usage like:
+only once. Information which is specific to individual lines will not be
+grouped together, like revs to be marked 'ignored' or 'unblamable'. This
+is more efficient, but may require more state be kept by the reader. The
+`--line-porcelain` option can be used to output full commit information
+for each line, allowing simpler (but less efficient) usage like:
 
 	# count the number of lines attributed to each author
 	git blame --line-porcelain file |
diff --git a/builtin/blame.c b/builtin/blame.c
index c470654c7e..9436f70aec 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -351,6 +351,19 @@ static void emit_porcelain_details(struct blame_origin *suspect, int repeat)
 		write_filename_info(suspect);
 }
 
+/*
+ * Information which needs to be printed per-line goes here. Any
+ * information which can be clubbed on a commit/file level, should
+ * be printed via 'emit_one_suspect_detail()'.
+ */
+static void emit_porcelain_per_line_details(struct blame_entry *ent)
+{
+	if (mark_unblamable_lines && ent->unblamable)
+		puts("unblamable");
+	if (mark_ignored_lines && ent->ignored)
+		puts("ignored");
+}
+
 static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
 			   int opt)
 {
@@ -367,6 +380,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
 	       ent->lno + 1,
 	       ent->num_lines);
 	emit_porcelain_details(suspect, repeat);
+	emit_porcelain_per_line_details(ent);
 
 	cp = blame_nth_line(sb, ent->lno);
 	for (cnt = 0; cnt < ent->num_lines; cnt++) {
@@ -377,6 +391,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
 			       ent->lno + 1 + cnt);
 			if (repeat)
 				emit_porcelain_details(suspect, 1);
+			emit_porcelain_per_line_details(ent);
 		}
 		putchar('\t');
 		do {
diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh
index 370b768149..50a0a7ca4a 100755
--- a/t/t8013-blame-ignore-revs.sh
+++ b/t/t8013-blame-ignore-revs.sh
@@ -158,6 +158,21 @@ test_expect_success mark_unblamable_lines '
 	test_cmp expect actual
 '
 
+for opt in --porcelain --line-porcelain
+do
+	test_expect_success "mark_unblamable_lines with $opt" '
+		sha=$(git rev-parse Y) &&
+
+		git -c blame.markUnblamableLines=false blame $opt --ignore-rev Y file >raw &&
+		sed -e "s/^\ty3/unblamable\n&/" raw >expect &&
+		cp expect raw &&
+		sed -e "s/^\ty4/unblamable\n&/" raw >expect &&
+
+		git -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&
+		test_cmp expect actual
+	'
+done
+
 # Commit Z will touch the first two lines.  Y touched all four.
 # 	A--B--X--Y--Z
 # The blame output when ignoring Z should be:
@@ -191,6 +206,21 @@ test_expect_success mark_ignored_lines '
 	! test_cmp expect actual
 '
 
+for opt in --porcelain --line-porcelain
+do
+	test_expect_success "mark_ignored_lines with $opt" '
+		sha=$(git rev-parse Y) &&
+
+		git -c blame.markIgnoredLines=false blame $opt --ignore-rev Z file >raw &&
+		sed -e "s/^\tline-one-Z/ignored\n&/" raw >expect &&
+		cp expect raw &&
+		sed -e "s/^\tline-two-Z/ignored\n&/" raw >expect &&
+
+		git -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&
+		test_cmp expect actual
+	'
+done
+
 # For ignored revs that added 'unblamable' lines and more recent commits changed
 # the blamable lines, mark the unblamable lines with a
 # '*'
-- 
2.48.1

Re: [PATCH v4] blame: print unblamable and ignored commits in porcelain mode

[Patrick Steinhardt]

On Sun, Mar 30, 2025 at 10:43:39PM +0200, Karthik Nayak wrote:
> diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh
> index 370b768149..50a0a7ca4a 100755
> --- a/t/t8013-blame-ignore-revs.sh
> +++ b/t/t8013-blame-ignore-revs.sh
> @@ -158,6 +158,21 @@ test_expect_success mark_unblamable_lines '
>  	test_cmp expect actual
>  '
>  
> +for opt in --porcelain --line-porcelain
> +do
> +	test_expect_success "mark_unblamable_lines with $opt" '
> +		sha=$(git rev-parse Y) &&
> +
> +		git -c blame.markUnblamableLines=false blame $opt --ignore-rev Y file >raw &&
> +		sed -e "s/^\ty3/unblamable\n&/" raw >expect &&
> +		cp expect raw &&
> +		sed -e "s/^\ty4/unblamable\n&/" raw >expect &&

The intent here is to do two replacements in "raw", right? You can do
this with a single call to sed(1) by chaining "-e":

	git -c blame.markUnblamableLines=false blame $opt --ignore-rev Y file >raw &&
	sed -e "s/^\ty3/unblamable\n&/" \
        -e "s/^\ty4/unblamable\n&/" raw >expect &&

Patrick

Re: [PATCH v4] blame: print unblamable and ignored commits in porcelain mode

[Karthik Nayak]

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

Patrick Steinhardt <ps@pks.im> writes:

> On Sun, Mar 30, 2025 at 10:43:39PM +0200, Karthik Nayak wrote:
>> diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh
>> index 370b768149..50a0a7ca4a 100755
>> --- a/t/t8013-blame-ignore-revs.sh
>> +++ b/t/t8013-blame-ignore-revs.sh
>> @@ -158,6 +158,21 @@ test_expect_success mark_unblamable_lines '
>>  	test_cmp expect actual
>>  '
>>
>> +for opt in --porcelain --line-porcelain
>> +do
>> +	test_expect_success "mark_unblamable_lines with $opt" '
>> +		sha=$(git rev-parse Y) &&
>> +
>> +		git -c blame.markUnblamableLines=false blame $opt --ignore-rev Y file >raw &&
>> +		sed -e "s/^\ty3/unblamable\n&/" raw >expect &&
>> +		cp expect raw &&
>> +		sed -e "s/^\ty4/unblamable\n&/" raw >expect &&
>
> The intent here is to do two replacements in "raw", right? You can do
> this with a single call to sed(1) by chaining "-e":
>
> 	git -c blame.markUnblamableLines=false blame $opt --ignore-rev Y file >raw &&
> 	sed -e "s/^\ty3/unblamable\n&/" \
>         -e "s/^\ty4/unblamable\n&/" raw >expect &&
>
> Patrick

Nice, I first tried to use `-i`, but seems like that doesn't work with
the 'sed' shipped in OSX. Didn't know I could chain it. This makes it
cleaner.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v4] blame: print unblamable and ignored commits in porcelain mode

[phillip.wood123]

Hi Karthik

On 30/03/2025 21:43, Karthik Nayak wrote:
>   
> +for opt in --porcelain --line-porcelain
> +do
> +	test_expect_success "mark_unblamable_lines with $opt" '
> +		sha=$(git rev-parse Y) &&
> +
> +		git -c blame.markUnblamableLines=false blame $opt --ignore-rev Y file >raw &&
> +		sed -e "s/^\ty3/unblamable\n&/" raw >expect &&
> +		cp expect raw &&
> +		sed -e "s/^\ty4/unblamable\n&/" raw >expect &&

Thanks for improving the test. Unfortunately using '\n' in the 
replacement text is not portable [1] (the normal backslash escapes are 
allowed in the pattern though so the '\t' is fine). One has to write a 
literal newline escaped with a backslash. However here we want to insert 
a whole new line of text into the output without changing the original 
so I would write it as

     sed -e "/^\ty3/a\\" -e unblamable -e "/^\ty4/a\\" -e unblamable \
	raw >expect

Best Wishes

Phillip

[1] <https://pubs.opengroup.org/onlinepubs/9799919799/>

     The relevant section of the text reads

     A line can be split by substituting a <newline> into it. The
     application shall escape the <newline> in the replacement by
     preceding it by a <backslash>.

     The meaning of an unescaped <backslash> immediately followed by any
     character other than '&', <backslash>, a digit, <newline>, or the
     delimiter character used for this command, is unspecified.


> +
> +		git -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&
> +		test_cmp expect actual
> +	'
> +done
> +
>   # Commit Z will touch the first two lines.  Y touched all four.
>   # 	A--B--X--Y--Z
>   # The blame output when ignoring Z should be:
> @@ -191,6 +206,21 @@ test_expect_success mark_ignored_lines '
>   	! test_cmp expect actual
>   '
>   
> +for opt in --porcelain --line-porcelain
> +do
> +	test_expect_success "mark_ignored_lines with $opt" '
> +		sha=$(git rev-parse Y) &&
> +
> +		git -c blame.markIgnoredLines=false blame $opt --ignore-rev Z file >raw &&
> +		sed -e "s/^\tline-one-Z/ignored\n&/" raw >expect &&
> +		cp expect raw &&
> +		sed -e "s/^\tline-two-Z/ignored\n&/" raw >expect &&
> +
> +		git -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&
> +		test_cmp expect actual
> +	'
> +done
> +
>   # For ignored revs that added 'unblamable' lines and more recent commits changed
>   # the blamable lines, mark the unblamable lines with a
>   # '*'

Re: [PATCH v4] blame: print unblamable and ignored commits in porcelain mode

[Phillip Wood]

On 31/03/2025 11:24, phillip.wood123@gmail.com wrote:
 >
 > (the normal backslash escapes are > allowed in the pattern though so 
the '\t' is fine).

Sorry that's wrong, you need to use a literal tab character instead of '\t'

Best Wishes

Phillip

Re: [PATCH v4] blame: print unblamable and ignored commits in porcelain mode

[Phillip Wood]

[Restoring cc to the mailing list that I accidentally dropped in my 
previous message]

On 01/04/2025 17:57, Karthik Nayak wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
>> On 31/03/2025 16:25, Karthik Nayak wrote:
>>> phillip.wood123@gmail.com writes:
>>>> On 30/03/2025 21:43, Karthik Nayak wrote:
>>>>>
>>>>> +for opt in --porcelain --line-porcelain
>>>>> +do
>>>>> +	test_expect_success "mark_unblamable_lines with $opt" '
>>>>> +		sha=$(git rev-parse Y) &&
>>>>> +
>>>>> +		git -c blame.markUnblamableLines=false blame $opt --ignore-rev Y file >raw &&
>>>>> +		sed -e "s/^\ty3/unblamable\n&/" raw >expect &&
>>>>> +		cp expect raw &&
>>>>> +		sed -e "s/^\ty4/unblamable\n&/" raw >expect &&
>>>>
>>>> Thanks for improving the test. Unfortunately using '\n' in the
>>>> replacement text is not portable [1] (the normal backslash escapes are
>>>> allowed in the pattern though so the '\t' is fine). One has to write a
>>>> literal newline escaped with a backslash. However here we want to insert
>>>> a whole new line of text into the output without changing the original
>>>> so I would write it as
>>>
>>> Thanks for bringing this to my notice. I didn't know.
>>>
>>>>        sed -e "/^\ty3/a\\" -e unblamable -e "/^\ty4/a\\" -e unblamable \
>>>> 	raw >expect
>>>
>>> This appends 'unblamable' to the next line, but we want to prepend it.
>>
>> Sorry, I misread the original. If you use 'i' instead of 'a' that will
>> insert a new line before the current one.
> 
> Seems like this won't work either, since MacOS complains [1] about it:
> 
>    expecting success of 8013.16 'mark_ignored_lines with --line-porcelain':
>    		sha=$(git rev-parse Y) &&
>    		git -c blame.markIgnoredLines=false blame $opt --ignore-rev Z file >raw &&
>    		sed -e "/^	line-one-Z/i\\" -e ignored \
>    		    -e "/^	line-two-Z/i\\" -e ignored \
>    			raw >expect &&
>    		git -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&
>    		test_cmp expect actual
>    	
>    +++ git rev-parse Y
>    ++ sha=e0d35d6f2d5fab63267e58d684cea1ECG86f12b1
>    ++ git -c blame.markIgnoredLines=false blame --line-porcelain
> --ignore-rev Z file
>    ++ sed -e '/^	line-one-Z/i\' -e ignored -e '/^	line-two-Z/i\' -e ignored raw
>    sed: 1: "ignored
>    ": command i expects \ followed by text
>    error: last command exited with $?=1

Oh, that's a pain, I thought it was supposed to treat each '-e' as a 
separate line.

> I did have success [2] with using a heredoc instead:
> 
> 	cat > sedscript <<- 'EOF' &&
> 	/^	y3/i\\
> 	unblamable
> 	/^	y4/i\\
> 	unblamable
> 	EOF
> 	sed -f sedscript raw >expect &&
> 
> What do you think about this?

I think that's the best way then. We could pass a multiline string with 
'-e' but then we wouldn't be able to indent the "unblamable" lines.

Best Wishes

Phillip

> [1]: https://gitlab.com/gitlab-org/git/-/jobs/9581456879
> [2]: https://gitlab.com/gitlab-org/git/-/pipelines/1746265204

[PATCH v5] blame: print unblamable and ignored commits in porcelain mode

[Karthik Nayak]

The 'git-blame(1)' command allows users to ignore specific revisions via
the '--ignore-rev <rev>' and '--ignore-revs-file <file>' flags. These
flags are often combined with the 'blame.markIgnoredLines' and
'blame.markUnblamableLines' config options. These config options prefix
ignored and unblamable lines with a '?' and '*', respectively.

However, this option was never extended to the porcelain mode of
'git-blame(1)'. Since the documentation does not indicate this
exclusion, it is a bug.

Fix this by printing 'ignored' and 'unblamable' respectively for the
options when using the porcelain modes.

Helped-by: Patrick Steinhardt <ps@pks.im>
Helped-by: Toon Claes <toon@iotcl.com>
Helped-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
Changes in v5:
- Fix the test to be more portable by not using '\n' in 'sed'. 
- Link to v4: https://lore.kernel.org/all/20250330204339.191382-1-karthik.188@gmail.com/

Changes in v4:
- Remove extra newline in 'puts'. Modify the test to compare the
  entire output, the earlier test missed the extraneous newline.
- Link to v3:
https://lore.kernel.org/r/20250329-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v3-1-10f695ae519a@gmail.com

Changes in v3:
- Use double-qoutes in the test to ensure correct variable dereference.
- Fix incorrect test name. 
- Rename the function from 'emit_per_line_details()' to
  'emit_porcelain_per_line_details()' to be more descriptive.
- Ues 'puts()' instead of 'printf()'.
- Link to v2:
https://lore.kernel.org/r/20250326-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v2-1-79037e17a74b@gmail.com

Changes in v2:
- Instead of printing the markers before the SHA in porcelain
  mode and breaking scripts and backward compatability, let's 
  instead add a newline printing 'unblamable' or 'ignored'.
  This is printed per line in both the porcelain modes. 
- Link to v1:
https://lore.kernel.org/r/20250321-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v1-1-44b562d9beb8@gmail.com
---
Range-diff versus v4:

1:  5250fb436e ! 1:  43bc55bffe blame: print unblamable and ignored commits in porcelain mode
    @@ Commit message
     
         Helped-by: Patrick Steinhardt <ps@pks.im>
         Helped-by: Toon Claes <toon@iotcl.com>
    +    Helped-by: Phillip Wood <phillip.wood123@gmail.com>
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
     
      ## Documentation/blame-options.adoc ##
    @@ t/t8013-blame-ignore-revs.sh: test_expect_success mark_unblamable_lines '
      
     +for opt in --porcelain --line-porcelain
     +do
    -+	test_expect_success "mark_unblamable_lines with $opt" '
    ++	test_expect_success "mark_unblamable_lines with $opt" "
     +		sha=$(git rev-parse Y) &&
     +
     +		git -c blame.markUnblamableLines=false blame $opt --ignore-rev Y file >raw &&
    -+		sed -e "s/^\ty3/unblamable\n&/" raw >expect &&
    -+		cp expect raw &&
    -+		sed -e "s/^\ty4/unblamable\n&/" raw >expect &&
    ++		cat > sedscript <<- 'EOF' &&
    ++		/^	y3/i\\
    ++		unblamable
    ++		/^	y4/i\\
    ++		unblamable
    ++		EOF
    ++		sed -f sedscript raw >expect &&
     +
     +		git -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&
     +		test_cmp expect actual
    -+	'
    ++	"
     +done
     +
      # Commit Z will touch the first two lines.  Y touched all four.
    @@ t/t8013-blame-ignore-revs.sh: test_expect_success mark_ignored_lines '
      
     +for opt in --porcelain --line-porcelain
     +do
    -+	test_expect_success "mark_ignored_lines with $opt" '
    ++	test_expect_success "mark_ignored_lines with $opt" "
     +		sha=$(git rev-parse Y) &&
     +
     +		git -c blame.markIgnoredLines=false blame $opt --ignore-rev Z file >raw &&
    -+		sed -e "s/^\tline-one-Z/ignored\n&/" raw >expect &&
    -+		cp expect raw &&
    -+		sed -e "s/^\tline-two-Z/ignored\n&/" raw >expect &&
    ++		cat > sedscript <<- 'EOF' &&
    ++		/^	line-one-Z/i\\
    ++		ignored
    ++		/^	line-two-Z/i\\
    ++		ignored
    ++		EOF
    ++		sed -f sedscript raw >expect &&
     +
     +		git -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&
     +		test_cmp expect actual
    -+	'
    ++	"
     +done
     +
      # For ignored revs that added 'unblamable' lines and more recent commits changed

---
 Documentation/blame-options.adoc |  3 ++-
 Documentation/git-blame.adoc     |  9 ++++----
 builtin/blame.c                  | 15 +++++++++++++
 t/t8013-blame-ignore-revs.sh     | 38 ++++++++++++++++++++++++++++++++
 4 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/Documentation/blame-options.adoc b/Documentation/blame-options.adoc
index aa77406d4e..19ea187238 100644
--- a/Documentation/blame-options.adoc
+++ b/Documentation/blame-options.adoc
@@ -125,7 +125,8 @@ take effect.
 	another commit will be marked with a `?` in the blame output.  If the
 	`blame.markUnblamableLines` config option is set, then those lines touched
 	by an ignored commit that we could not attribute to another revision are
-	marked with a '*'.
+	marked with a '*'. In the porcelain modes, we print 'ignored' and
+	'unblamable' on a newline respectively.
 
 --ignore-revs-file <file>::
 	Ignore revisions listed in `file`, which must be in the same format as an
diff --git a/Documentation/git-blame.adoc b/Documentation/git-blame.adoc
index f75ed44790..e438d28625 100644
--- a/Documentation/git-blame.adoc
+++ b/Documentation/git-blame.adoc
@@ -135,10 +135,11 @@ header elements later.
 The porcelain format generally suppresses commit information that has
 already been seen. For example, two lines that are blamed to the same
 commit will both be shown, but the details for that commit will be shown
-only once. This is more efficient, but may require more state be kept by
-the reader. The `--line-porcelain` option can be used to output full
-commit information for each line, allowing simpler (but less efficient)
-usage like:
+only once. Information which is specific to individual lines will not be
+grouped together, like revs to be marked 'ignored' or 'unblamable'. This
+is more efficient, but may require more state be kept by the reader. The
+`--line-porcelain` option can be used to output full commit information
+for each line, allowing simpler (but less efficient) usage like:
 
 	# count the number of lines attributed to each author
 	git blame --line-porcelain file |
diff --git a/builtin/blame.c b/builtin/blame.c
index c470654c7e..9436f70aec 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -351,6 +351,19 @@ static void emit_porcelain_details(struct blame_origin *suspect, int repeat)
 		write_filename_info(suspect);
 }
 
+/*
+ * Information which needs to be printed per-line goes here. Any
+ * information which can be clubbed on a commit/file level, should
+ * be printed via 'emit_one_suspect_detail()'.
+ */
+static void emit_porcelain_per_line_details(struct blame_entry *ent)
+{
+	if (mark_unblamable_lines && ent->unblamable)
+		puts("unblamable");
+	if (mark_ignored_lines && ent->ignored)
+		puts("ignored");
+}
+
 static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
 			   int opt)
 {
@@ -367,6 +380,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
 	       ent->lno + 1,
 	       ent->num_lines);
 	emit_porcelain_details(suspect, repeat);
+	emit_porcelain_per_line_details(ent);
 
 	cp = blame_nth_line(sb, ent->lno);
 	for (cnt = 0; cnt < ent->num_lines; cnt++) {
@@ -377,6 +391,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
 			       ent->lno + 1 + cnt);
 			if (repeat)
 				emit_porcelain_details(suspect, 1);
+			emit_porcelain_per_line_details(ent);
 		}
 		putchar('\t');
 		do {
diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh
index 370b768149..cace00ae8d 100755
--- a/t/t8013-blame-ignore-revs.sh
+++ b/t/t8013-blame-ignore-revs.sh
@@ -158,6 +158,25 @@ test_expect_success mark_unblamable_lines '
 	test_cmp expect actual
 '
 
+for opt in --porcelain --line-porcelain
+do
+	test_expect_success "mark_unblamable_lines with $opt" "
+		sha=$(git rev-parse Y) &&
+
+		git -c blame.markUnblamableLines=false blame $opt --ignore-rev Y file >raw &&
+		cat > sedscript <<- 'EOF' &&
+		/^	y3/i\\
+		unblamable
+		/^	y4/i\\
+		unblamable
+		EOF
+		sed -f sedscript raw >expect &&
+
+		git -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&
+		test_cmp expect actual
+	"
+done
+
 # Commit Z will touch the first two lines.  Y touched all four.
 # 	A--B--X--Y--Z
 # The blame output when ignoring Z should be:
@@ -191,6 +210,25 @@ test_expect_success mark_ignored_lines '
 	! test_cmp expect actual
 '
 
+for opt in --porcelain --line-porcelain
+do
+	test_expect_success "mark_ignored_lines with $opt" "
+		sha=$(git rev-parse Y) &&
+
+		git -c blame.markIgnoredLines=false blame $opt --ignore-rev Z file >raw &&
+		cat > sedscript <<- 'EOF' &&
+		/^	line-one-Z/i\\
+		ignored
+		/^	line-two-Z/i\\
+		ignored
+		EOF
+		sed -f sedscript raw >expect &&
+
+		git -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&
+		test_cmp expect actual
+	"
+done
+
 # For ignored revs that added 'unblamable' lines and more recent commits changed
 # the blamable lines, mark the unblamable lines with a
 # '*'
-- 
2.48.1

Re: [PATCH v5] blame: print unblamable and ignored commits in porcelain mode

[Phillip Wood]

Hi Karthik

On 03/04/2025 17:03, Karthik Nayak wrote:
> The 'git-blame(1)' command allows users to ignore specific revisions via
> the '--ignore-rev <rev>' and '--ignore-revs-file <file>' flags. These
> flags are often combined with the 'blame.markIgnoredLines' and
> 'blame.markUnblamableLines' config options. These config options prefix
> ignored and unblamable lines with a '?' and '*', respectively.
> 
> However, this option was never extended to the porcelain mode of
> 'git-blame(1)'. Since the documentation does not indicate this
> exclusion, it is a bug.
> 
> Fix this by printing 'ignored' and 'unblamable' respectively for the
> options when using the porcelain modes.

This looks good to me

Best Wishes

Phillip

> Helped-by: Patrick Steinhardt <ps@pks.im>
> Helped-by: Toon Claes <toon@iotcl.com>
> Helped-by: Phillip Wood <phillip.wood123@gmail.com>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
> Changes in v5:
> - Fix the test to be more portable by not using '\n' in 'sed'.
> - Link to v4: https://lore.kernel.org/all/20250330204339.191382-1-karthik.188@gmail.com/
> 
> Changes in v4:
> - Remove extra newline in 'puts'. Modify the test to compare the
>    entire output, the earlier test missed the extraneous newline.
> - Link to v3:
> https://lore.kernel.org/r/20250329-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v3-1-10f695ae519a@gmail.com
> 
> Changes in v3:
> - Use double-qoutes in the test to ensure correct variable dereference.
> - Fix incorrect test name.
> - Rename the function from 'emit_per_line_details()' to
>    'emit_porcelain_per_line_details()' to be more descriptive.
> - Ues 'puts()' instead of 'printf()'.
> - Link to v2:
> https://lore.kernel.org/r/20250326-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v2-1-79037e17a74b@gmail.com
> 
> Changes in v2:
> - Instead of printing the markers before the SHA in porcelain
>    mode and breaking scripts and backward compatability, let's
>    instead add a newline printing 'unblamable' or 'ignored'.
>    This is printed per line in both the porcelain modes.
> - Link to v1:
> https://lore.kernel.org/r/20250321-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v1-1-44b562d9beb8@gmail.com
> ---
> Range-diff versus v4:
> 
> 1:  5250fb436e ! 1:  43bc55bffe blame: print unblamable and ignored commits in porcelain mode
>      @@ Commit message
>       
>           Helped-by: Patrick Steinhardt <ps@pks.im>
>           Helped-by: Toon Claes <toon@iotcl.com>
>      +    Helped-by: Phillip Wood <phillip.wood123@gmail.com>
>           Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>       
>        ## Documentation/blame-options.adoc ##
>      @@ t/t8013-blame-ignore-revs.sh: test_expect_success mark_unblamable_lines '
>        
>       +for opt in --porcelain --line-porcelain
>       +do
>      -+	test_expect_success "mark_unblamable_lines with $opt" '
>      ++	test_expect_success "mark_unblamable_lines with $opt" "
>       +		sha=$(git rev-parse Y) &&
>       +
>       +		git -c blame.markUnblamableLines=false blame $opt --ignore-rev Y file >raw &&
>      -+		sed -e "s/^\ty3/unblamable\n&/" raw >expect &&
>      -+		cp expect raw &&
>      -+		sed -e "s/^\ty4/unblamable\n&/" raw >expect &&
>      ++		cat > sedscript <<- 'EOF' &&
>      ++		/^	y3/i\\
>      ++		unblamable
>      ++		/^	y4/i\\
>      ++		unblamable
>      ++		EOF
>      ++		sed -f sedscript raw >expect &&
>       +
>       +		git -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&
>       +		test_cmp expect actual
>      -+	'
>      ++	"
>       +done
>       +
>        # Commit Z will touch the first two lines.  Y touched all four.
>      @@ t/t8013-blame-ignore-revs.sh: test_expect_success mark_ignored_lines '
>        
>       +for opt in --porcelain --line-porcelain
>       +do
>      -+	test_expect_success "mark_ignored_lines with $opt" '
>      ++	test_expect_success "mark_ignored_lines with $opt" "
>       +		sha=$(git rev-parse Y) &&
>       +
>       +		git -c blame.markIgnoredLines=false blame $opt --ignore-rev Z file >raw &&
>      -+		sed -e "s/^\tline-one-Z/ignored\n&/" raw >expect &&
>      -+		cp expect raw &&
>      -+		sed -e "s/^\tline-two-Z/ignored\n&/" raw >expect &&
>      ++		cat > sedscript <<- 'EOF' &&
>      ++		/^	line-one-Z/i\\
>      ++		ignored
>      ++		/^	line-two-Z/i\\
>      ++		ignored
>      ++		EOF
>      ++		sed -f sedscript raw >expect &&
>       +
>       +		git -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&
>       +		test_cmp expect actual
>      -+	'
>      ++	"
>       +done
>       +
>        # For ignored revs that added 'unblamable' lines and more recent commits changed
> 
> ---
>   Documentation/blame-options.adoc |  3 ++-
>   Documentation/git-blame.adoc     |  9 ++++----
>   builtin/blame.c                  | 15 +++++++++++++
>   t/t8013-blame-ignore-revs.sh     | 38 ++++++++++++++++++++++++++++++++
>   4 files changed, 60 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/blame-options.adoc b/Documentation/blame-options.adoc
> index aa77406d4e..19ea187238 100644
> --- a/Documentation/blame-options.adoc
> +++ b/Documentation/blame-options.adoc
> @@ -125,7 +125,8 @@ take effect.
>   	another commit will be marked with a `?` in the blame output.  If the
>   	`blame.markUnblamableLines` config option is set, then those lines touched
>   	by an ignored commit that we could not attribute to another revision are
> -	marked with a '*'.
> +	marked with a '*'. In the porcelain modes, we print 'ignored' and
> +	'unblamable' on a newline respectively.
>   
>   --ignore-revs-file <file>::
>   	Ignore revisions listed in `file`, which must be in the same format as an
> diff --git a/Documentation/git-blame.adoc b/Documentation/git-blame.adoc
> index f75ed44790..e438d28625 100644
> --- a/Documentation/git-blame.adoc
> +++ b/Documentation/git-blame.adoc
> @@ -135,10 +135,11 @@ header elements later.
>   The porcelain format generally suppresses commit information that has
>   already been seen. For example, two lines that are blamed to the same
>   commit will both be shown, but the details for that commit will be shown
> -only once. This is more efficient, but may require more state be kept by
> -the reader. The `--line-porcelain` option can be used to output full
> -commit information for each line, allowing simpler (but less efficient)
> -usage like:
> +only once. Information which is specific to individual lines will not be
> +grouped together, like revs to be marked 'ignored' or 'unblamable'. This
> +is more efficient, but may require more state be kept by the reader. The
> +`--line-porcelain` option can be used to output full commit information
> +for each line, allowing simpler (but less efficient) usage like:
>   
>   	# count the number of lines attributed to each author
>   	git blame --line-porcelain file |
> diff --git a/builtin/blame.c b/builtin/blame.c
> index c470654c7e..9436f70aec 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -351,6 +351,19 @@ static void emit_porcelain_details(struct blame_origin *suspect, int repeat)
>   		write_filename_info(suspect);
>   }
>   
> +/*
> + * Information which needs to be printed per-line goes here. Any
> + * information which can be clubbed on a commit/file level, should
> + * be printed via 'emit_one_suspect_detail()'.
> + */
> +static void emit_porcelain_per_line_details(struct blame_entry *ent)
> +{
> +	if (mark_unblamable_lines && ent->unblamable)
> +		puts("unblamable");
> +	if (mark_ignored_lines && ent->ignored)
> +		puts("ignored");
> +}
> +
>   static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
>   			   int opt)
>   {
> @@ -367,6 +380,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
>   	       ent->lno + 1,
>   	       ent->num_lines);
>   	emit_porcelain_details(suspect, repeat);
> +	emit_porcelain_per_line_details(ent);
>   
>   	cp = blame_nth_line(sb, ent->lno);
>   	for (cnt = 0; cnt < ent->num_lines; cnt++) {
> @@ -377,6 +391,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
>   			       ent->lno + 1 + cnt);
>   			if (repeat)
>   				emit_porcelain_details(suspect, 1);
> +			emit_porcelain_per_line_details(ent);
>   		}
>   		putchar('\t');
>   		do {
> diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh
> index 370b768149..cace00ae8d 100755
> --- a/t/t8013-blame-ignore-revs.sh
> +++ b/t/t8013-blame-ignore-revs.sh
> @@ -158,6 +158,25 @@ test_expect_success mark_unblamable_lines '
>   	test_cmp expect actual
>   '
>   
> +for opt in --porcelain --line-porcelain
> +do
> +	test_expect_success "mark_unblamable_lines with $opt" "
> +		sha=$(git rev-parse Y) &&
> +
> +		git -c blame.markUnblamableLines=false blame $opt --ignore-rev Y file >raw &&
> +		cat > sedscript <<- 'EOF' &&
> +		/^	y3/i\\
> +		unblamable
> +		/^	y4/i\\
> +		unblamable
> +		EOF
> +		sed -f sedscript raw >expect &&
> +
> +		git -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&
> +		test_cmp expect actual
> +	"
> +done
> +
>   # Commit Z will touch the first two lines.  Y touched all four.
>   # 	A--B--X--Y--Z
>   # The blame output when ignoring Z should be:
> @@ -191,6 +210,25 @@ test_expect_success mark_ignored_lines '
>   	! test_cmp expect actual
>   '
>   
> +for opt in --porcelain --line-porcelain
> +do
> +	test_expect_success "mark_ignored_lines with $opt" "
> +		sha=$(git rev-parse Y) &&
> +
> +		git -c blame.markIgnoredLines=false blame $opt --ignore-rev Z file >raw &&
> +		cat > sedscript <<- 'EOF' &&
> +		/^	line-one-Z/i\\
> +		ignored
> +		/^	line-two-Z/i\\
> +		ignored
> +		EOF
> +		sed -f sedscript raw >expect &&
> +
> +		git -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&
> +		test_cmp expect actual
> +	"
> +done
> +
>   # For ignored revs that added 'unblamable' lines and more recent commits changed
>   # the blamable lines, mark the unblamable lines with a
>   # '*'

Re: [PATCH v5] blame: print unblamable and ignored commits in porcelain mode

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> Hi Karthik
>
> On 03/04/2025 17:03, Karthik Nayak wrote:
>> The 'git-blame(1)' command allows users to ignore specific revisions via
>> the '--ignore-rev <rev>' and '--ignore-revs-file <file>' flags. These
>> flags are often combined with the 'blame.markIgnoredLines' and
>> 'blame.markUnblamableLines' config options. These config options prefix
>> ignored and unblamable lines with a '?' and '*', respectively.
>> However, this option was never extended to the porcelain mode of
>> 'git-blame(1)'. Since the documentation does not indicate this
>> exclusion, it is a bug.
>> Fix this by printing 'ignored' and 'unblamable' respectively for the
>> options when using the porcelain modes.
>
> This looks good to me

Thanks, both of you.  Will replace and queue.
Let me mark the topic for 'next'.