{"thread":{"id":"63172","subject":"[PATCH] blame: fix unblamable and ignored lines in porcelain mode","startedAt":"2025-03-21T16:39:19Z","lastAt":"2025-04-08T00:32:55Z","messageCount":28,"participants":["Karthik Nayak","Junio C Hamano","Patrick Steinhardt","Toon Claes","Eric Sunshine","Phillip Wood","phillip.wood123@gmail.com"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"514836","messageId":"20250321-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v1-1-44b562d9beb8@gmail.com","threadId":"63172","inReplyTo":null,"subject":"[PATCH] blame: fix unblamable and ignored lines in porcelain mode","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-03-21T16:39:14Z","receivedAt":"2025-03-21T16:39:19Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The 'git-blame(1)' command allows users to ignore specific revisions via\nthe '--ignore-rev ' and '--ignore-revs-file ' flags. These\nflags are often combined with the 'blame.markIgnoredLines' and\n'blame.markUnblamableLines' config options. These config options prefix\nignored and unblamable lines with a '?' and '*', respectively.\n\nHowever, this option was never extended to the porcelain mode of\n'git-blame(1)'. Since the documentation does not indicate this\nexclusion, it is a bug.\n\nFix this by ensuring porcelain mode also prints the markers and add\ntests to verify the behavior.\n\nSigned-off-by: Karthik Nayak \n---\n builtin/blame.c | 10 ++++++++++\n t/t8013-blame-ignore-revs.sh | 30 ++++++++++++++++++++++++++++++\n 2 files changed, 40 insertions(+)\n\nKarthik Nayak (1):\n blame: fix unblamable and ignored lines in porcelain mode\n\n\n\nbase-commit: 683c54c999c301c2cd6f715c411407c413b1d84e\nchange-id: 20250321-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-4af46f02847e\n\nThanks\n- Karthik\n---\n\n---\n builtin/blame.c | 10 ++++++++++\n t/t8013-blame-ignore-revs.sh | 30 ++++++++++++++++++++++++++++++\n 2 files changed, 40 insertions(+)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex c470654c7e..9a8d7ce7af 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -360,6 +360,11 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,\n \tstruct blame_origin *suspect = ent->suspect;\n \tchar hex[GIT_MAX_HEXSZ + 1];\n \n+\tif (mark_unblamable_lines && ent->unblamable)\n+\t\tputchar('*');\n+\tif (mark_ignored_lines && ent->ignored)\n+\t\tputchar('?');\n+\n \toid_to_hex_r(hex, &suspect->commit->object.oid);\n \tprintf(\"%s %d %d %d\\n\",\n \t hex,\n@@ -372,6 +377,11 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,\n \tfor (cnt = 0; cnt < ent->num_lines; cnt++) {\n \t\tchar ch;\n \t\tif (cnt) {\n+\t\t\tif (mark_unblamable_lines && ent->unblamable)\n+\t\t\t\tputchar('*');\n+\t\t\tif (mark_ignored_lines && ent->ignored)\n+\t\t\t\tputchar('?');\n+\n \t\t\tprintf(\"%s %d %d\\n\", hex,\n \t\t\t ent->s_lno + 1 + cnt,\n \t\t\t ent->lno + 1 + cnt);\ndiff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh\nindex 370b768149..2722eb4598 100755\n--- a/t/t8013-blame-ignore-revs.sh\n+++ b/t/t8013-blame-ignore-revs.sh\n@@ -158,6 +158,16 @@ test_expect_success mark_unblamable_lines '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'mark_unblamable_lines porcelain' '\n+\tsha=$(git rev-parse Y) &&\n+\n+\tgit -c blame.markUnblamableLines=false blame -p --ignore-rev Y file >blame_raw &&\n+\tsed \"s/^${sha}/*${sha}/g\" blame_raw >expect &&\n+\n+\tgit -c blame.markUnblamableLines=true blame -p --ignore-rev Y file >actual &&\n+\ttest_cmp expect actual\n+'\n+\n # Commit Z will touch the first two lines. Y touched all four.\n # \tA--B--X--Y--Z\n # The blame output when ignoring Z should be:\n@@ -191,6 +201,26 @@ test_expect_success mark_ignored_lines '\n \t! test_cmp expect actual\n '\n \n+test_expect_success 'mark_ignored_lines porcelain' '\n+\tsha=$(git rev-parse Y) &&\n+\n+\tgit -c blame.markIgnoredLines=true blame -p --ignore-rev Z file | grep $sha >blame_raw &&\n+\n+\techo \"?\" >expect &&\n+\n+\tsed -n \"1p\" blame_raw | cut -c1 >actual &&\n+\ttest_cmp expect actual &&\n+\n+\tsed -n \"2p\" blame_raw | cut -c1 >actual &&\n+\ttest_cmp expect actual &&\n+\n+\tsed -n \"3p\" blame_raw | cut -c1 >actual &&\n+\t! test_cmp expect actual &&\n+\n+\tsed -n \"4p\" blame_raw | cut -c1 >actual &&\n+\t! test_cmp expect actual\n+'\n+\n # For ignored revs that added 'unblamable' lines and more recent commits changed\n # the blamable lines, mark the unblamable lines with a\n # '*'\n\n\n\n"},{"id":"514880","messageId":"xmqqzfhbrb78.fsf@gitster.g","threadId":"63172","inReplyTo":"20250321-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v1-1-44b562d9beb8@gmail.com","subject":"Re: [PATCH] blame: fix unblamable and ignored lines in porcelain mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-23T15:58:03Z","receivedAt":"2025-03-23T15:58:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> However, this option was never extended to the porcelain mode of\n> 'git-blame(1)'. Since the documentation does not indicate this\n> exclusion, it is a bug.\n\nI agree it is a bug when people added ignore or unblamable support\nthat they did not _consider_ what to do with their new pieces of\ninformation to help porcelain writers. It is not a bug in the code\nper-se, but it is a bug in the brain of these people ;-)\n\nBut prefixing random garbage to the commit object name line in the\nporcelain mode output does not sound like the right solution to the\nbug, either.\n\nWhen enhancing an existing output format, make sure that your\nchanges will have minimum empact to existing parsers that do not\nknow about your extension. It is reasonably expected that existing\nPorcelain scripts reading from --porcelain mode output works by\n\n - Recognizing a line that match \"^[0-9a-f]{40} \\d+ \\d+ \\d+$\" and\n take it as the beginning of a new record;\n\n - Collect all info lines before the payload line. Lines that\n describe per-commit information are not repeated if it is already\n shown, so remember them when you see the commit for the first\n time, and recall them when you recognize the commit you already\n saw.\n\n - A payload line is indented with HT and terminates the record.\n\nIf you start to add unrecognizable garbage to the line with very\nwell known fixed format that is used as record delimiter, you would\nbreak the existing parsers, which is not a very nice thing to do.\nAre there other and better ways you can think of to add new pieces\nof information like this in a way with less severe damage?\n"},{"id":"514911","messageId":"Z-Ew_12NWX_5qATN@pks.im","threadId":"63172","inReplyTo":"xmqqzfhbrb78.fsf@gitster.g","subject":"Re: [PATCH] blame: fix unblamable and ignored lines in porcelain mode","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-24T10:16:31Z","receivedAt":"2025-03-24T10:16:36Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sun, Mar 23, 2025 at 08:58:03AM -0700, Junio C Hamano wrote:\n> Karthik Nayak writes:\n> \n> > However, this option was never extended to the porcelain mode of\n> > 'git-blame(1)'. Since the documentation does not indicate this\n> > exclusion, it is a bug.\n> \n> I agree it is a bug when people added ignore or unblamable support\n> that they did not _consider_ what to do with their new pieces of\n> information to help porcelain writers. It is not a bug in the code\n> per-se, but it is a bug in the brain of these people ;-)\n> \n> But prefixing random garbage to the commit object name line in the\n> porcelain mode output does not sound like the right solution to the\n> bug, either.\n> \n> When enhancing an existing output format, make sure that your\n> changes will have minimum empact to existing parsers that do not\n> know about your extension. It is reasonably expected that existing\n> Porcelain scripts reading from --porcelain mode output works by\n> \n> - Recognizing a line that match \"^[0-9a-f]{40} \\d+ \\d+ \\d+$\" and\n> take it as the beginning of a new record;\n> \n> - Collect all info lines before the payload line. Lines that\n> describe per-commit information are not repeated if it is already\n> shown, so remember them when you see the commit for the first\n> time, and recall them when you recognize the commit you already\n> saw.\n> \n> - A payload line is indented with HT and terminates the record.\n> \n> If you start to add unrecognizable garbage to the line with very\n> well known fixed format that is used as record delimiter, you would\n> break the existing parsers, which is not a very nice thing to do.\n> Are there other and better ways you can think of to add new pieces\n> of information like this in a way with less severe damage?\n\nI think the porcelain mode is already built so that it can be extended\nwith arbitrary new information, no? In `emit_one_suspect_detail()` we\nend up printing one line per info we want to display. I would have\nexpected that we can extend that function to also print information\naround unblamable or ignored commits, like we already do for boundary\ncommits. E.g. something like the patch further down.\n\nThanks!\n\nPatrick\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex c470654c7ec..cd8322e2619 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -255,7 +255,8 @@ static void write_filename_info(struct blame_origin *suspect)\n * the first time each commit appears in the output (unless the\n * user has specifically asked for us to repeat).\n */\n-static int emit_one_suspect_detail(struct blame_origin *suspect, int repeat)\n+static int emit_one_suspect_detail(struct blame_entry *ent,\n+\t\t\t\t struct blame_origin *suspect, int repeat)\n {\n \tstruct commit_info ci = COMMIT_INFO_INIT;\n \n@@ -275,6 +276,10 @@ static int emit_one_suspect_detail(struct blame_origin *suspect, int repeat)\n \tprintf(\"summary %s\\n\", ci.summary.buf);\n \tif (suspect->commit->object.flags & UNINTERESTING)\n \t\tprintf(\"boundary\\n\");\n+\tif (mark_unblamable_lines && ent->unblamable)\n+\t\tprintf(\"unblamable\\n\");\n+\tif (mark_ignored_lines && ent->ignored)\n+\t\tprintf(\"ignored\\n\");\n \n \tcommit_info_destroy(&ci);\n \n@@ -295,7 +300,7 @@ static void found_guilty_entry(struct blame_entry *ent, void *data)\n \t\tprintf(\"%s %d %d %d\\n\",\n \t\t oid_to_hex(&suspect->commit->object.oid),\n \t\t ent->s_lno + 1, ent->lno + 1, ent->num_lines);\n-\t\temit_one_suspect_detail(suspect, 0);\n+\t\temit_one_suspect_detail(ent, suspect, 0);\n \t\twrite_filename_info(suspect);\n \t\tmaybe_flush_or_die(stdout, \"stdout\");\n \t}\n@@ -344,9 +349,10 @@ static const char *format_time(timestamp_t time, const char *tz_str,\n #define OUTPUT_COLOR_LINE (1U<<10)\n #define OUTPUT_SHOW_AGE_WITH_COLOR (1U<<11)\n \n-static void emit_porcelain_details(struct blame_origin *suspect, int repeat)\n+static void emit_porcelain_details(struct blame_entry *ent,\n+\t\t\t\t struct blame_origin *suspect, int repeat)\n {\n-\tif (emit_one_suspect_detail(suspect, repeat) ||\n+\tif (emit_one_suspect_detail(ent, suspect, repeat) ||\n \t (suspect->commit->object.flags & MORE_THAN_ONE_PATH))\n \t\twrite_filename_info(suspect);\n }\n@@ -366,7 +372,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,\n \t ent->s_lno + 1,\n \t ent->lno + 1,\n \t ent->num_lines);\n-\temit_porcelain_details(suspect, repeat);\n+\temit_porcelain_details(ent, suspect, repeat);\n \n \tcp = blame_nth_line(sb, ent->lno);\n \tfor (cnt = 0; cnt < ent->num_lines; cnt++) {\n@@ -376,7 +382,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,\n \t\t\t ent->s_lno + 1 + cnt,\n \t\t\t ent->lno + 1 + cnt);\n \t\t\tif (repeat)\n-\t\t\t\temit_porcelain_details(suspect, 1);\n+\t\t\t\temit_porcelain_details(ent, suspect, 1);\n \t\t}\n \t\tputchar('\\t');\n \t\tdo {\n"},{"id":"514913","messageId":"87a59aww8j.fsf@iotcl.com","threadId":"63172","inReplyTo":"Z-Ew_12NWX_5qATN@pks.im","subject":"Re: [PATCH] blame: fix unblamable and ignored lines in porcelain mode","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-03-24T10:37:00Z","receivedAt":"2025-03-24T10:37:18Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Patrick Steinhardt writes:\n\n> I think the porcelain mode is already built so that it can be extended\n> with arbitrary new information, no? In `emit_one_suspect_detail()` we\n> end up printing one line per info we want to display. I would have\n> expected that we can extend that function to also print information\n> around unblamable or ignored commits, like we already do for boundary\n> commits. E.g. something like the patch further down.\n\nYeah, I think the porcelain format exists to be easy to machine-parse.\nHaving an optional prefix symbol on the commit OID would complicate\nprocess that.\n\nAnd I've been thinking about a similar solution as you've been\nsuggesting below. I was only wondering whether we only do this when\nusing `--line-porcelain`. When using `--porcelain` the function\n`emit_one_suspect_detail()` doesn't print most of the commit info when\nit was already printed. But the \"unblamable\" and \"ignored\" info might be\ndifferent for each line, even if they blame down to the same commit.\n\n-- \nToon\n"},{"id":"514963","messageId":"CAOLa=ZR9f=TiZ7ezNWOMwTqbkjiBYa_4B_zjiA8jBQ4nkEgqqQ@mail.gmail.com","threadId":"63172","inReplyTo":"xmqqzfhbrb78.fsf@gitster.g","subject":"Re: [PATCH] blame: fix unblamable and ignored lines in porcelain mode","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-03-24T19:56:19Z","receivedAt":"2025-03-24T19:56:21Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Junio C Hamano writes:\n\n> Karthik Nayak writes:\n>\n>> However, this option was never extended to the porcelain mode of\n>> 'git-blame(1)'. Since the documentation does not indicate this\n>> exclusion, it is a bug.\n>\n> I agree it is a bug when people added ignore or unblamable support\n> that they did not _consider_ what to do with their new pieces of\n> information to help porcelain writers. It is not a bug in the code\n> per-se, but it is a bug in the brain of these people ;-)\n>\n> But prefixing random garbage to the commit object name line in the\n> porcelain mode output does not sound like the right solution to the\n> bug, either.\n>\n> When enhancing an existing output format, make sure that your\n> changes will have minimum empact to existing parsers that do not\n> know about your extension. It is reasonably expected that existing\n> Porcelain scripts reading from --porcelain mode output works by\n>\n> - Recognizing a line that match \"^[0-9a-f]{40} \\d+ \\d+ \\d+$\" and\n> take it as the beginning of a new record;\n>\n> - Collect all info lines before the payload line. Lines that\n> describe per-commit information are not repeated if it is already\n> shown, so remember them when you see the commit for the first\n> time, and recall them when you recognize the commit you already\n> saw.\n>\n> - A payload line is indented with HT and terminates the record.\n>\n> If you start to add unrecognizable garbage to the line with very\n> well known fixed format that is used as record delimiter, you would\n> break the existing parsers, which is not a very nice thing to do.\n> Are there other and better ways you can think of to add new pieces\n> of information like this in a way with less severe damage?\n\nFair enough, I was having this argument and convinced myself because the\noption is an explicit setting. So users who have\n'blame.markIgnoredLines' or/and 'blame.markUnblamableLines' set and are\nusing one of the 'ignore-rev' options would be expecting either an '?'\nor a '*' as mentioned in the documentation.\n\nLet's me figure if there is a more backward-compatible way of doing\nthis.\n"},{"id":"514964","messageId":"CAOLa=ZS5vwVDFNTV3_bHcg7_AjimevPj8u_rFBhotu--y1ukGQ@mail.gmail.com","threadId":"63172","inReplyTo":"Z-Ew_12NWX_5qATN@pks.im","subject":"Re: [PATCH] blame: fix unblamable and ignored lines in porcelain mode","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-03-24T20:00:19Z","receivedAt":"2025-03-24T20:00:21Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Sun, Mar 23, 2025 at 08:58:03AM -0700, Junio C Hamano wrote:\n>> Karthik Nayak writes:\n>>\n>> > However, this option was never extended to the porcelain mode of\n>> > 'git-blame(1)'. Since the documentation does not indicate this\n>> > exclusion, it is a bug.\n>>\n>> I agree it is a bug when people added ignore or unblamable support\n>> that they did not _consider_ what to do with their new pieces of\n>> information to help porcelain writers. It is not a bug in the code\n>> per-se, but it is a bug in the brain of these people ;-)\n>>\n>> But prefixing random garbage to the commit object name line in the\n>> porcelain mode output does not sound like the right solution to the\n>> bug, either.\n>>\n>> When enhancing an existing output format, make sure that your\n>> changes will have minimum empact to existing parsers that do not\n>> know about your extension. It is reasonably expected that existing\n>> Porcelain scripts reading from --porcelain mode output works by\n>>\n>> - Recognizing a line that match \"^[0-9a-f]{40} \\d+ \\d+ \\d+$\" and\n>> take it as the beginning of a new record;\n>>\n>> - Collect all info lines before the payload line. Lines that\n>> describe per-commit information are not repeated if it is already\n>> shown, so remember them when you see the commit for the first\n>> time, and recall them when you recognize the commit you already\n>> saw.\n>>\n>> - A payload line is indented with HT and terminates the record.\n>>\n>> If you start to add unrecognizable garbage to the line with very\n>> well known fixed format that is used as record delimiter, you would\n>> break the existing parsers, which is not a very nice thing to do.\n>> Are there other and better ways you can think of to add new pieces\n>> of information like this in a way with less severe damage?\n>\n> I think the porcelain mode is already built so that it can be extended\n> with arbitrary new information, no? In `emit_one_suspect_detail()` we\n> end up printing one line per info we want to display. I would have\n> expected that we can extend that function to also print information\n> around unblamable or ignored commits, like we already do for boundary\n> commits. E.g. something like the patch further down.\n>\n\nThis indeed looks like a much better way of doing this. Let me\n\n> Thanks!\n>\n> Patrick\n>\n> diff --git a/builtin/blame.c b/builtin/blame.c\n> index c470654c7ec..cd8322e2619 100644\n> --- a/builtin/blame.c\n> +++ b/builtin/blame.c\n> @@ -255,7 +255,8 @@ static void write_filename_info(struct blame_origin *suspect)\n> * the first time each commit appears in the output (unless the\n> * user has specifically asked for us to repeat).\n> */\n> -static int emit_one_suspect_detail(struct blame_origin *suspect, int repeat)\n> +static int emit_one_suspect_detail(struct blame_entry *ent,\n> +\t\t\t\t struct blame_origin *suspect, int repeat)\n> {\n> \tstruct commit_info ci = COMMIT_INFO_INIT;\n>\n> @@ -275,6 +276,10 @@ static int emit_one_suspect_detail(struct blame_origin *suspect, int repeat)\n> \tprintf(\"summary %s\\n\", ci.summary.buf);\n> \tif (suspect->commit->object.flags & UNINTERESTING)\n> \t\tprintf(\"boundary\\n\");\n> +\tif (mark_unblamable_lines && ent->unblamable)\n> +\t\tprintf(\"unblamable\\n\");\n> +\tif (mark_ignored_lines && ent->ignored)\n> +\t\tprintf(\"ignored\\n\");\n>\n> \tcommit_info_destroy(&ci);\n>\n> @@ -295,7 +300,7 @@ static void found_guilty_entry(struct blame_entry *ent, void *data)\n> \t\tprintf(\"%s %d %d %d\\n\",\n> \t\t oid_to_hex(&suspect->commit->object.oid),\n> \t\t ent->s_lno + 1, ent->lno + 1, ent->num_lines);\n> -\t\temit_one_suspect_detail(suspect, 0);\n> +\t\temit_one_suspect_detail(ent, suspect, 0);\n> \t\twrite_filename_info(suspect);\n> \t\tmaybe_flush_or_die(stdout, \"stdout\");\n> \t}\n> @@ -344,9 +349,10 @@ static const char *format_time(timestamp_t time, const char *tz_str,\n> #define OUTPUT_COLOR_LINE (1U<<10)\n> #define OUTPUT_SHOW_AGE_WITH_COLOR (1U<<11)\n>\n> -static void emit_porcelain_details(struct blame_origin *suspect, int repeat)\n> +static void emit_porcelain_details(struct blame_entry *ent,\n> +\t\t\t\t struct blame_origin *suspect, int repeat)\n> {\n> -\tif (emit_one_suspect_detail(suspect, repeat) ||\n> +\tif (emit_one_suspect_detail(ent, suspect, repeat) ||\n> \t (suspect->commit->object.flags & MORE_THAN_ONE_PATH))\n> \t\twrite_filename_info(suspect);\n> }\n> @@ -366,7 +372,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,\n> \t ent->s_lno + 1,\n> \t ent->lno + 1,\n> \t ent->num_lines);\n> -\temit_porcelain_details(suspect, repeat);\n> +\temit_porcelain_details(ent, suspect, repeat);\n>\n> \tcp = blame_nth_line(sb, ent->lno);\n> \tfor (cnt = 0; cnt < ent->num_lines; cnt++) {\n> @@ -376,7 +382,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,\n> \t\t\t ent->s_lno + 1 + cnt,\n> \t\t\t ent->lno + 1 + cnt);\n> \t\t\tif (repeat)\n> -\t\t\t\temit_porcelain_details(suspect, 1);\n> +\t\t\t\temit_porcelain_details(ent, suspect, 1);\n> \t\t}\n> \t\tputchar('\\t');\n> \t\tdo {\n\nThanks Patrick, I will send in the new version with this and modified\ntests.\n"},{"id":"514965","messageId":"CAOLa=ZTHb6ZFmpDpSwzUTiDs44gY-W3aqBooAMyORK8Xs3YQxQ@mail.gmail.com","threadId":"63172","inReplyTo":"87a59aww8j.fsf@iotcl.com","subject":"Re: [PATCH] blame: fix unblamable and ignored lines in porcelain mode","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-03-24T20:04:03Z","receivedAt":"2025-03-24T20:04:06Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Toon Claes writes:\n\n> Patrick Steinhardt writes:\n>\n>> I think the porcelain mode is already built so that it can be extended\n>> with arbitrary new information, no? In `emit_one_suspect_detail()` we\n>> end up printing one line per info we want to display. I would have\n>> expected that we can extend that function to also print information\n>> around unblamable or ignored commits, like we already do for boundary\n>> commits. E.g. something like the patch further down.\n>\n> Yeah, I think the porcelain format exists to be easy to machine-parse.\n> Having an optional prefix symbol on the commit OID would complicate\n> process that.\n>\n> And I've been thinking about a similar solution as you've been\n> suggesting below. I was only wondering whether we only do this when\n> using `--line-porcelain`. When using `--porcelain` the function\n> `emit_one_suspect_detail()` doesn't print most of the commit info when\n> it was already printed. But the \"unblamable\" and \"ignored\" info might be\n> different for each line, even if they blame down to the same commit.\n>\n\nI'm curious, how would it be different, if they blame down to the same\ncommit? My understanding was \"unblamable\" and \"ignored\" are tied to\ncommits.\n\n> --\n> Toon\n"},{"id":"514985","messageId":"875xjxwlb6.fsf@iotcl.com","threadId":"63172","inReplyTo":"CAOLa=ZTHb6ZFmpDpSwzUTiDs44gY-W3aqBooAMyORK8Xs3YQxQ@mail.gmail.com","subject":"Re: [PATCH] blame: fix unblamable and ignored lines in porcelain mode","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-03-25T08:45:17Z","receivedAt":"2025-03-25T08:45:34Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Karthik Nayak writes:\n\n> I'm curious, how would it be different, if they blame down to the same\n> commit? My understanding was \"unblamable\" and \"ignored\" are tied to\n> commits.\n\nLet me include an example, let's blame `varint.h`.\n\nFirst have a look at the non-porcelain format:\n\n $ git blame varint.h -l\n d2c1898571a6a2324593e92163e8754880e0c1fb (Junio C Hamano 2012-04-03 15:53:08 -0700 1) #ifndef VARINT_H\n d2c1898571a6a2324593e92163e8754880e0c1fb (Junio C Hamano 2012-04-03 15:53:08 -0700 2) #define VARINT_H\n d2c1898571a6a2324593e92163e8754880e0c1fb (Junio C Hamano 2012-04-03 15:53:08 -0700 3)\n 554544276a604c144df45efcb060c80aa322088c (Denton Liu 2019-04-29 04:28:14 -0400 4) int encode_varint(uintmax_t, unsigned char *);\n 554544276a604c144df45efcb060c80aa322088c (Denton Liu 2019-04-29 04:28:14 -0400 5) uintmax_t decode_varint(const unsigned char **);\n d2c1898571a6a2324593e92163e8754880e0c1fb (Junio C Hamano 2012-04-03 15:53:08 -0700 6)\n d2c1898571a6a2324593e92163e8754880e0c1fb (Junio C Hamano 2012-04-03 15:53:08 -0700 7) #endif /* VARINT_H */\n\nNow if we put `554544276a604c144df45efcb060c80aa322088c` in `.git-blame-ignore-revs`:\n\n $ git -c blame.markUnblamableLines=true -c blame.markIgnoredLines=true blame varint.h --ignore-revs-file .git-blame-ignore-revs -l\n d2c1898571a6a2324593e92163e8754880e0c1fb (Junio C Hamano 2012-04-03 15:53:08 -0700 1) #ifndef VARINT_H\n d2c1898571a6a2324593e92163e8754880e0c1fb (Junio C Hamano 2012-04-03 15:53:08 -0700 2) #define VARINT_H\n d2c1898571a6a2324593e92163e8754880e0c1fb (Junio C Hamano 2012-04-03 15:53:08 -0700 3)\n ?d2c1898571a6a2324593e92163e8754880e0c1f (Junio C Hamano 2012-04-03 15:53:08 -0700 4) int encode_varint(uintmax_t, unsigned char *);\n ?d2c1898571a6a2324593e92163e8754880e0c1f (Junio C Hamano 2012-04-03 15:53:08 -0700 5) uintmax_t decode_varint(const unsigned char **);\n d2c1898571a6a2324593e92163e8754880e0c1fb (Junio C Hamano 2012-04-03 15:53:08 -0700 6)\n d2c1898571a6a2324593e92163e8754880e0c1fb (Junio C Hamano 2012-04-03 15:53:08 -0700 7) #endif /* VARINT_H */\n\nIf we compare that to the porcelain format:\n\n $ git blame varint.h -l --porcelain\n d2c1898571a6a2324593e92163e8754880e0c1fb 1 1 3\n author Junio C Hamano\n author-mail \n author-time 1333493588\n author-tz -0700\n committer Junio C Hamano\n committer-mail \n committer-time 1333495484\n committer-tz -0700\n summary varint: make it available outside the context of pack\n filename varint.h\n #ifndef VARINT_H\n d2c1898571a6a2324593e92163e8754880e0c1fb 2 2\n #define VARINT_H\n d2c1898571a6a2324593e92163e8754880e0c1fb 3 3\n\n 554544276a604c144df45efcb060c80aa322088c 4 4 2\n author Denton Liu\n author-mail \n author-time 1556526494\n author-tz -0400\n committer Junio C Hamano\n committer-mail \n committer-time 1557037206\n committer-tz +0900\n summary *.[ch]: remove extern from function declarations using spatch\n previous ffac537e6cbbf934b08745a378932722df287a53 varint.h\n filename varint.h\n int encode_varint(uintmax_t, unsigned char *);\n 554544276a604c144df45efcb060c80aa322088c 5 5\n uintmax_t decode_varint(const unsigned char **);\n d2c1898571a6a2324593e92163e8754880e0c1fb 8 6 2\n\n d2c1898571a6a2324593e92163e8754880e0c1fb 9 7\n #endif /* VARINT_H */\n\nAnd now with the `.git-blame-ignore-revs` file:\n\n $ git -c blame.markUnblamableLines=true -c blame.markIgnoredLines=true blame varint.h --ignore-revs-file .git-blame-ignore-revs -l --porcelain\n d2c1898571a6a2324593e92163e8754880e0c1fb 1 1 3\n author Junio C Hamano\n author-mail \n author-time 1333493588\n author-tz -0700\n committer Junio C Hamano\n committer-mail \n committer-time 1333495484\n committer-tz -0700\n summary varint: make it available outside the context of pack\n filename varint.h\n #ifndef VARINT_H\n d2c1898571a6a2324593e92163e8754880e0c1fb 2 2\n #define VARINT_H\n d2c1898571a6a2324593e92163e8754880e0c1fb 3 3\n\n d2c1898571a6a2324593e92163e8754880e0c1fb 6 4 2\n int encode_varint(uintmax_t, unsigned char *);\n d2c1898571a6a2324593e92163e8754880e0c1fb 7 5\n uintmax_t decode_varint(const unsigned char **);\n d2c1898571a6a2324593e92163e8754880e0c1fb 8 6 2\n\n d2c1898571a6a2324593e92163e8754880e0c1fb 9 7\n #endif /* VARINT_H */\n\nSo every line now blames down to commit\nd2c1898571a6a2324593e92163e8754880e0c1fb. The lines which used to\nblame down to 554544276a604c144df45efcb060c80aa322088c should be marked\nas \"ignored\", but we only emit the details once for each commit. The\ncommit details (author, committer) are only relevant once, but the\n\"ignored\" info can differ for each line (as you also can see in the\nnon-porcelain format).\n\nWe could make the output look something like:\n\n $ git -c blame.markUnblamableLines=true -c blame.markIgnoredLines=true blame varint.h --ignore-revs-file .git-blame-ignore-revs -l --porcelain\n d2c1898571a6a2324593e92163e8754880e0c1fb 1 1 3\n author Junio C Hamano\n author-mail \n author-time 1333493588\n author-tz -0700\n committer Junio C Hamano\n committer-mail \n committer-time 1333495484\n committer-tz -0700\n summary varint: make it available outside the context of pack\n filename varint.h\n #ifndef VARINT_H\n d2c1898571a6a2324593e92163e8754880e0c1fb 2 2\n #define VARINT_H\n d2c1898571a6a2324593e92163e8754880e0c1fb 3 3\n\n d2c1898571a6a2324593e92163e8754880e0c1fb 6 4 2\n ignored\n int encode_varint(uintmax_t, unsigned char *);\n d2c1898571a6a2324593e92163e8754880e0c1fb 7 5\n ignored\n uintmax_t decode_varint(const unsigned char **);\n d2c1898571a6a2324593e92163e8754880e0c1fb 8 6 2\n\n d2c1898571a6a2324593e92163e8754880e0c1fb 9 7\n #endif /* VARINT_H */\n\nIt feels odd to me only the \"ignored\" info is emitted and the rest\nof the details isn't. But that might be just me...\n\n-- \nToon\n"},{"id":"514986","messageId":"CAOLa=ZRZB0hidGGvepOQFaMaHbBqnGO_Qh38vqBZFvScbs4xzw@mail.gmail.com","threadId":"63172","inReplyTo":"875xjxwlb6.fsf@iotcl.com","subject":"Re: [PATCH] blame: fix unblamable and ignored lines in porcelain mode","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-03-25T10:31:28Z","receivedAt":"2025-03-25T10:31:30Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Toon Claes writes:\n\n> Karthik Nayak writes:\n>\n>> I'm curious, how would it be different, if they blame down to the same\n>> commit? My understanding was \"unblamable\" and \"ignored\" are tied to\n>> commits.\n\n[snip]\n\n> And now with the `.git-blame-ignore-revs` file:\n>\n> $ git -c blame.markUnblamableLines=true -c blame.markIgnoredLines=true blame varint.h --ignore-revs-file .git-blame-ignore-revs -l --porcelain\n> d2c1898571a6a2324593e92163e8754880e0c1fb 1 1 3\n> author Junio C Hamano\n> author-mail \n> author-time 1333493588\n> author-tz -0700\n> committer Junio C Hamano\n> committer-mail \n> committer-time 1333495484\n> committer-tz -0700\n> summary varint: make it available outside the context of pack\n> filename varint.h\n> #ifndef VARINT_H\n> d2c1898571a6a2324593e92163e8754880e0c1fb 2 2\n> #define VARINT_H\n> d2c1898571a6a2324593e92163e8754880e0c1fb 3 3\n>\n> d2c1898571a6a2324593e92163e8754880e0c1fb 6 4 2\n> int encode_varint(uintmax_t, unsigned char *);\n> d2c1898571a6a2324593e92163e8754880e0c1fb 7 5\n> uintmax_t decode_varint(const unsigned char **);\n> d2c1898571a6a2324593e92163e8754880e0c1fb 8 6 2\n>\n> d2c1898571a6a2324593e92163e8754880e0c1fb 9 7\n> #endif /* VARINT_H */\n>\n> So every line now blames down to commit\n> d2c1898571a6a2324593e92163e8754880e0c1fb. The lines which used to\n> blame down to 554544276a604c144df45efcb060c80aa322088c should be marked\n> as \"ignored\", but we only emit the details once for each commit. The\n> commit details (author, committer) are only relevant once, but the\n> \"ignored\" info can differ for each line (as you also can see in the\n> non-porcelain format).\n>\n\nAh! So if a rev is ignored via the `--ignore-rev[s-file]` flag, then the\nparent revision is shown in the blame. It could happen that in porcelain\nmode the parent revision is clubbed with previous lines if they share\nthe same revision. This would skip the 'unblamable' or 'ignored'\ninformation.\n\nThis would be solved in '--line-porcelain' since details aren't clubbed.\n\nI agree, it makes the most sense to only do this in 'line-porcelain'\nmode.\n\n> We could make the output look something like:\n>\n> $ git -c blame.markUnblamableLines=true -c blame.markIgnoredLines=true blame varint.h --ignore-revs-file .git-blame-ignore-revs -l --porcelain\n> d2c1898571a6a2324593e92163e8754880e0c1fb 1 1 3\n> author Junio C Hamano\n> author-mail \n> author-time 1333493588\n> author-tz -0700\n> committer Junio C Hamano\n> committer-mail \n> committer-time 1333495484\n> committer-tz -0700\n> summary varint: make it available outside the context of pack\n> filename varint.h\n> #ifndef VARINT_H\n> d2c1898571a6a2324593e92163e8754880e0c1fb 2 2\n> #define VARINT_H\n> d2c1898571a6a2324593e92163e8754880e0c1fb 3 3\n>\n> d2c1898571a6a2324593e92163e8754880e0c1fb 6 4 2\n> ignored\n> int encode_varint(uintmax_t, unsigned char *);\n> d2c1898571a6a2324593e92163e8754880e0c1fb 7 5\n> ignored\n> uintmax_t decode_varint(const unsigned char **);\n> d2c1898571a6a2324593e92163e8754880e0c1fb 8 6 2\n>\n> d2c1898571a6a2324593e92163e8754880e0c1fb 9 7\n> #endif /* VARINT_H */\n>\n> It feels odd to me only the \"ignored\" info is emitted and the rest\n> of the details isn't. But that might be just me...\n>\n\nI'm with you on this, this would also require us to explain this odd\nexclusion where only for 'ignored' and 'unblamable' lines we output\ndetails on every commit. But the other way is also an exclusion, where\nwe would say that 'ignored' and 'unblamable' lines are only shown in\n'--line-porcelain'.\n\nBut the latter can be extended into the former in the future but not the\nother way around. So I would say it makes more sense to restrict it to\n'--line-porcelain' in that sense.\n\n> --\n> Toon\n"},{"id":"515036","messageId":"xmqqbjtonbdy.fsf@gitster.g","threadId":"63172","inReplyTo":"875xjxwlb6.fsf@iotcl.com","subject":"Re: [PATCH] blame: fix unblamable and ignored lines in porcelain mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-25T19:44:25Z","receivedAt":"2025-03-25T19:44:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Toon Claes writes:\n\n> It feels odd to me only the \"ignored\" info is emitted and the rest\n> of the details isn't. But that might be just me...\n\nIf we have more per-line (and not per-commit/file tuple) pieces of\ninformation, we would have to treat them just like you had \"ignored\"\nin your illustration above. It is \"ignored\" that feels \"odd\", since\nit is the only single oddball right now.\n"},{"id":"515124","messageId":"20250326-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v2-1-79037e17a74b@gmail.com","threadId":"63172","inReplyTo":"20250321-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v1-1-44b562d9beb8@gmail.com","subject":"[PATCH v2] blame: print unblamable and ignored commits in porcelain mode","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-03-26T21:06:10Z","receivedAt":"2025-03-26T21:06:21Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The 'git-blame(1)' command allows users to ignore specific revisions via\nthe '--ignore-rev ' and '--ignore-revs-file ' flags. These\nflags are often combined with the 'blame.markIgnoredLines' and\n'blame.markUnblamableLines' config options. These config options prefix\nignored and unblamable lines with a '?' and '*', respectively.\n\nHowever, this option was never extended to the porcelain mode of\n'git-blame(1)'. Since the documentation does not indicate this\nexclusion, it is a bug.\n\nFix this by printing 'ignored' and 'unblamable' respectively for the\noptions when using the porcelain modes.\n\nHelped-by: Patrick Steinhardt \nHelped-by: Toon Claes \nSigned-off-by: Karthik Nayak \n---\nChanges in v2:\n- Instead of printing the markers before the SHA in porcelain\n mode and breaking scripts and backward compatability, let's \n instead add a newline printing 'unblamable' or 'ignored'.\n This is printed per line in both the porcelain modes. \n- 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\n---\nRange-diff versus v1:\n\n1: 9650db628f < -: ---------- blame: fix unblamable and ignored lines in porcelain mode\n-: ---------- > 1: 6bbfc0cbd2 blame: print unblamable and ignored commits in porcelain mode\n---\n Documentation/blame-options.adoc | 3 ++-\n Documentation/git-blame.adoc | 9 +++++----\n builtin/blame.c | 15 +++++++++++++++\n t/t8013-blame-ignore-revs.sh | 20 ++++++++++++++++++++\n 4 files changed, 42 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/blame-options.adoc b/Documentation/blame-options.adoc\nindex aa77406d4e..19ea187238 100644\n--- a/Documentation/blame-options.adoc\n+++ b/Documentation/blame-options.adoc\n@@ -125,7 +125,8 @@ take effect.\n \tanother commit will be marked with a `?` in the blame output. If the\n \t`blame.markUnblamableLines` config option is set, then those lines touched\n \tby an ignored commit that we could not attribute to another revision are\n-\tmarked with a '*'.\n+\tmarked with a '*'. In the porcelain modes, we print 'ignored' and\n+\t'unblamable' on a newline respectively.\n \n --ignore-revs-file ::\n \tIgnore revisions listed in `file`, which must be in the same format as an\ndiff --git a/Documentation/git-blame.adoc b/Documentation/git-blame.adoc\nindex f75ed44790..e438d28625 100644\n--- a/Documentation/git-blame.adoc\n+++ b/Documentation/git-blame.adoc\n@@ -135,10 +135,11 @@ header elements later.\n The porcelain format generally suppresses commit information that has\n already been seen. For example, two lines that are blamed to the same\n commit will both be shown, but the details for that commit will be shown\n-only once. This is more efficient, but may require more state be kept by\n-the reader. The `--line-porcelain` option can be used to output full\n-commit information for each line, allowing simpler (but less efficient)\n-usage like:\n+only once. Information which is specific to individual lines will not be\n+grouped together, like revs to be marked 'ignored' or 'unblamable'. This\n+is more efficient, but may require more state be kept by the reader. The\n+`--line-porcelain` option can be used to output full commit information\n+for each line, allowing simpler (but less efficient) usage like:\n \n \t# count the number of lines attributed to each author\n \tgit blame --line-porcelain file |\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex c470654c7e..528bfef249 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -351,6 +351,19 @@ static void emit_porcelain_details(struct blame_origin *suspect, int repeat)\n \t\twrite_filename_info(suspect);\n }\n \n+/*\n+ * Information which needs to be printed per-line goes here. Any\n+ * information which can be clubbed on a commit/file level, should\n+ * be printed via 'emit_one_suspect_detail()'.\n+ */\n+static void emit_per_line_details(struct blame_entry *ent)\n+{\n+\tif (mark_unblamable_lines && ent->unblamable)\n+\t\tprintf(\"unblamable\\n\");\n+\tif (mark_ignored_lines && ent->ignored)\n+\t\tprintf(\"ignored\\n\");\n+}\n+\n static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,\n \t\t\t int opt)\n {\n@@ -367,6 +380,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,\n \t ent->lno + 1,\n \t ent->num_lines);\n \temit_porcelain_details(suspect, repeat);\n+\temit_per_line_details(ent);\n \n \tcp = blame_nth_line(sb, ent->lno);\n \tfor (cnt = 0; cnt < ent->num_lines; cnt++) {\n@@ -377,6 +391,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,\n \t\t\t ent->lno + 1 + cnt);\n \t\t\tif (repeat)\n \t\t\t\temit_porcelain_details(suspect, 1);\n+\t\t\temit_per_line_details(ent);\n \t\t}\n \t\tputchar('\\t');\n \t\tdo {\ndiff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh\nindex 370b768149..306fc61057 100755\n--- a/t/t8013-blame-ignore-revs.sh\n+++ b/t/t8013-blame-ignore-revs.sh\n@@ -158,6 +158,16 @@ test_expect_success mark_unblamable_lines '\n \ttest_cmp expect actual\n '\n \n+for opt in --porcelain --line-porcelain\n+do\n+\ttest_expect_success 'mark_unblamable_lines with $opt' '\n+\t\tsha=$(git rev-parse Y) &&\n+\n+\t\tgit -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&\n+\t\ttest $(grep ^unblamable actual | wc -l) -eq 2\n+\t'\n+done\n+\n # Commit Z will touch the first two lines. Y touched all four.\n # \tA--B--X--Y--Z\n # The blame output when ignoring Z should be:\n@@ -191,6 +201,16 @@ test_expect_success mark_ignored_lines '\n \t! test_cmp expect actual\n '\n \n+for opt in --porcelain --line-porcelain\n+do\n+\ttest_expect_success 'mark_ignored_lines line_porcelain' '\n+\t\tsha=$(git rev-parse Y) &&\n+\n+\t\tgit -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&\n+\t\ttest $(grep ^ignored actual | wc -l) -eq 2\n+\t'\n+done\n+\n # For ignored revs that added 'unblamable' lines and more recent commits changed\n # the blamable lines, mark the unblamable lines with a\n # '*'\n\n---\n\nbase-commit: 683c54c999c301c2cd6f715c411407c413b1d84e\nchange-id: 20250321-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-4af46f02847e\n\nThanks\n- Karthik\n\n"},{"id":"515126","messageId":"CAPig+cRLXDohGG0RLpC41hy=rTmbmNQ_qScFWMv2LP8pdM8_hg@mail.gmail.com","threadId":"63172","inReplyTo":"20250326-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v2-1-79037e17a74b@gmail.com","subject":"Re: [PATCH v2] blame: print unblamable and ignored commits in porcelain mode","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2025-03-26T22:49:32Z","receivedAt":"2025-03-26T22:49:44Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Mar 26, 2025 at 5:07 PM Karthik Nayak wrote:\n> The 'git-blame(1)' command allows users to ignore specific revisions via\n> the '--ignore-rev ' and '--ignore-revs-file ' flags. These\n> flags are often combined with the 'blame.markIgnoredLines' and\n> 'blame.markUnblamableLines' config options. These config options prefix\n> ignored and unblamable lines with a '?' and '*', respectively.\n>\n> However, this option was never extended to the porcelain mode of\n> 'git-blame(1)'. Since the documentation does not indicate this\n> exclusion, it is a bug.\n>\n> Fix this by printing 'ignored' and 'unblamable' respectively for the\n> options when using the porcelain modes.\n>\n> Signed-off-by: Karthik Nayak \n> ---\n> diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh\n> @@ -158,6 +158,16 @@ test_expect_success mark_unblamable_lines '\n> +for opt in --porcelain --line-porcelain\n> +do\n> + test_expect_success 'mark_unblamable_lines with $opt' '\n\nThis test title is going to display literal \"$opt\" rather than the\nintended option. Fix this by replacing the single quotes with double\nquotes:\n\n test_expect_success \"mark_unblamable_lines with $opt\" '\n\n> + sha=$(git rev-parse Y) &&\n> +\n> + git -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&\n> + test $(grep ^unblamable actual | wc -l) -eq 2\n> + '\n> +done\n> @@ -191,6 +201,16 @@ test_expect_success mark_ignored_lines '\n> +for opt in --porcelain --line-porcelain\n> +do\n> + test_expect_success 'mark_ignored_lines line_porcelain' '\n\nSimilarly, this is going to display the same title for both cases,\nwhich isn't as helpful as it could be. Presumably, you instead wanted\nthis (using double quotes):\n\n test_expect_success \"mark_ignored_lines with $opt\" '\n\n> + sha=$(git rev-parse Y) &&\n> +\n> + git -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&\n> + test $(grep ^ignored actual | wc -l) -eq 2\n> + '\n"},{"id":"515174","messageId":"CAOLa=ZT8VjTrJcVMYtCK3+8Rt1h79GPKdNr4OyKAJ+57a=EHDw@mail.gmail.com","threadId":"63172","inReplyTo":"CAPig+cRLXDohGG0RLpC41hy=rTmbmNQ_qScFWMv2LP8pdM8_hg@mail.gmail.com","subject":"Re: [PATCH v2] blame: print unblamable and ignored commits in porcelain mode","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-03-27T11:07:01Z","receivedAt":"2025-03-27T11:07:03Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Eric Sunshine writes:\n\n> On Wed, Mar 26, 2025 at 5:07 PM Karthik Nayak wrote:\n>> The 'git-blame(1)' command allows users to ignore specific revisions via\n>> the '--ignore-rev ' and '--ignore-revs-file ' flags. These\n>> flags are often combined with the 'blame.markIgnoredLines' and\n>> 'blame.markUnblamableLines' config options. These config options prefix\n>> ignored and unblamable lines with a '?' and '*', respectively.\n>>\n>> However, this option was never extended to the porcelain mode of\n>> 'git-blame(1)'. Since the documentation does not indicate this\n>> exclusion, it is a bug.\n>>\n>> Fix this by printing 'ignored' and 'unblamable' respectively for the\n>> options when using the porcelain modes.\n>>\n>> Signed-off-by: Karthik Nayak \n>> ---\n>> diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh\n>> @@ -158,6 +158,16 @@ test_expect_success mark_unblamable_lines '\n>> +for opt in --porcelain --line-porcelain\n>> +do\n>> + test_expect_success 'mark_unblamable_lines with $opt' '\n>\n> This test title is going to display literal \"$opt\" rather than the\n> intended option. Fix this by replacing the single quotes with double\n> quotes:\n>\n> test_expect_success \"mark_unblamable_lines with $opt\" '\n>\n\nWhat a silly miss. Thanks for pointing out.\n\n>> + sha=$(git rev-parse Y) &&\n>> +\n>> + git -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&\n>> + test $(grep ^unblamable actual | wc -l) -eq 2\n>> + '\n>> +done\n>> @@ -191,6 +201,16 @@ test_expect_success mark_ignored_lines '\n>> +for opt in --porcelain --line-porcelain\n>> +do\n>> + test_expect_success 'mark_ignored_lines line_porcelain' '\n>\n> Similarly, this is going to display the same title for both cases,\n> which isn't as helpful as it could be. Presumably, you instead wanted\n> this (using double quotes):\n>\n> test_expect_success \"mark_ignored_lines with $opt\" '\n>\n\nYup, this needs to be fixed too. Thanks again.\n\n>> + sha=$(git rev-parse Y) &&\n>> +\n>> + git -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&\n>> + test $(grep ^ignored actual | wc -l) -eq 2\n>> + '\n"},{"id":"515216","messageId":"Z-ZI-M2jBf5pqMoA@pks.im","threadId":"63172","inReplyTo":"20250326-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v2-1-79037e17a74b@gmail.com","subject":"Re: [PATCH v2] blame: print unblamable and ignored commits in porcelain mode","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-28T07:00:08Z","receivedAt":"2025-03-28T07:00:15Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Mar 26, 2025 at 10:06:10PM +0100, Karthik Nayak wrote:\n> diff --git a/builtin/blame.c b/builtin/blame.c\n> index c470654c7e..528bfef249 100644\n> --- a/builtin/blame.c\n> +++ b/builtin/blame.c\n> @@ -351,6 +351,19 @@ static void emit_porcelain_details(struct blame_origin *suspect, int repeat)\n> \t\twrite_filename_info(suspect);\n> }\n> \n> +/*\n> + * Information which needs to be printed per-line goes here. Any\n> + * information which can be clubbed on a commit/file level, should\n> + * be printed via 'emit_one_suspect_detail()'.\n> + */\n> +static void emit_per_line_details(struct blame_entry *ent)\n\nTiny nit, feel free to ignore: should this something like\n`emit_porcelain_per_line_details()` to highlight that this is part of\nthe porcelain format?\n\n> +{\n> +\tif (mark_unblamable_lines && ent->unblamable)\n> +\t\tprintf(\"unblamable\\n\");\n> +\tif (mark_ignored_lines && ent->ignored)\n> +\t\tprintf(\"ignored\\n\");\n> +}\n> +\n\nAnother tiny nit: you may use `puts()` instead of `printf()`. I don't\nmind it much though, both versions work equally well.\n\n> diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh\n> index 370b768149..306fc61057 100755\n> --- a/t/t8013-blame-ignore-revs.sh\n> +++ b/t/t8013-blame-ignore-revs.sh\n> @@ -158,6 +158,16 @@ test_expect_success mark_unblamable_lines '\n> \ttest_cmp expect actual\n> '\n> \n> +for opt in --porcelain --line-porcelain\n> +do\n> +\ttest_expect_success 'mark_unblamable_lines with $opt' '\n> +\t\tsha=$(git rev-parse Y) &&\n> +\n> +\t\tgit -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&\n> +\t\ttest $(grep ^unblamable actual | wc -l) -eq 2\n> +\t'\n> +done\n> +\n\nOkay, makes sense. We cannot batch the information on the first time\nwe've seen the commit here because both the \"unblamable\" and \"ignored\"\nproperties are properties of the line, not of the commit. So we'd expect\nto see the information per line in both modes.\n\nPatrick\n"},{"id":"515268","messageId":"CAOLa=ZRgAQ+FKznxOaOe0ou_6vRPOA7SorhxwvGeBZ07SasgqA@mail.gmail.com","threadId":"63172","inReplyTo":"Z-ZI-M2jBf5pqMoA@pks.im","subject":"Re: [PATCH v2] blame: print unblamable and ignored commits in porcelain mode","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-03-29T10:26:14Z","receivedAt":"2025-03-29T10:26:17Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Wed, Mar 26, 2025 at 10:06:10PM +0100, Karthik Nayak wrote:\n>> diff --git a/builtin/blame.c b/builtin/blame.c\n>> index c470654c7e..528bfef249 100644\n>> --- a/builtin/blame.c\n>> +++ b/builtin/blame.c\n>> @@ -351,6 +351,19 @@ static void emit_porcelain_details(struct blame_origin *suspect, int repeat)\n>> \t\twrite_filename_info(suspect);\n>> }\n>>\n>> +/*\n>> + * Information which needs to be printed per-line goes here. Any\n>> + * information which can be clubbed on a commit/file level, should\n>> + * be printed via 'emit_one_suspect_detail()'.\n>> + */\n>> +static void emit_per_line_details(struct blame_entry *ent)\n>\n> Tiny nit, feel free to ignore: should this something like\n> `emit_porcelain_per_line_details()` to highlight that this is part of\n> the porcelain format?\n>\n\nThat's a great point, will add that in.\n\n>> +{\n>> +\tif (mark_unblamable_lines && ent->unblamable)\n>> +\t\tprintf(\"unblamable\\n\");\n>> +\tif (mark_ignored_lines && ent->ignored)\n>> +\t\tprintf(\"ignored\\n\");\n>> +}\n>> +\n>\n> Another tiny nit: you may use `puts()` instead of `printf()`. I don't\n> mind it much though, both versions work equally well.\n>\n\nYeah, I think some compilers also do this translation. But I'll change\nit as anyways I'm going to push a new version.\n\n>> diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh\n>> index 370b768149..306fc61057 100755\n>> --- a/t/t8013-blame-ignore-revs.sh\n>> +++ b/t/t8013-blame-ignore-revs.sh\n>> @@ -158,6 +158,16 @@ test_expect_success mark_unblamable_lines '\n>> \ttest_cmp expect actual\n>> '\n>>\n>> +for opt in --porcelain --line-porcelain\n>> +do\n>> +\ttest_expect_success 'mark_unblamable_lines with $opt' '\n>> +\t\tsha=$(git rev-parse Y) &&\n>> +\n>> +\t\tgit -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&\n>> +\t\ttest $(grep ^unblamable actual | wc -l) -eq 2\n>> +\t'\n>> +done\n>> +\n>\n> Okay, makes sense. We cannot batch the information on the first time\n> we've seen the commit here because both the \"unblamable\" and \"ignored\"\n> properties are properties of the line, not of the commit. So we'd expect\n> to see the information per line in both modes.\n>\n> Patrick\n\nThanks for the review.\nKarthik\n"},{"id":"515281","messageId":"20250329-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v3-1-10f695ae519a@gmail.com","threadId":"63172","inReplyTo":"20250321-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v1-1-44b562d9beb8@gmail.com","subject":"[PATCH v3] blame: print unblamable and ignored commits in porcelain mode","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-03-29T18:21:16Z","receivedAt":"2025-03-29T18:21:22Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The 'git-blame(1)' command allows users to ignore specific revisions via\nthe '--ignore-rev ' and '--ignore-revs-file ' flags. These\nflags are often combined with the 'blame.markIgnoredLines' and\n'blame.markUnblamableLines' config options. These config options prefix\nignored and unblamable lines with a '?' and '*', respectively.\n\nHowever, this option was never extended to the porcelain mode of\n'git-blame(1)'. Since the documentation does not indicate this\nexclusion, it is a bug.\n\nFix this by printing 'ignored' and 'unblamable' respectively for the\noptions when using the porcelain modes.\n\nHelped-by: Patrick Steinhardt \nHelped-by: Toon Claes \nSigned-off-by: Karthik Nayak \n---\nChanges in v3:\n- Use double-qoutes in the test to ensure correct variable dereference.\n- Fix incorrect test name. \n- Rename the function from 'emit_per_line_details()' to\n 'emit_porcelain_per_line_details()' to be more descriptive.\n- Ues 'puts()' instead of 'printf()'.\n- 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\n\nChanges in v2:\n- Instead of printing the markers before the SHA in porcelain\n mode and breaking scripts and backward compatability, let's \n instead add a newline printing 'unblamable' or 'ignored'.\n This is printed per line in both the porcelain modes. \n- 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\n---\nRange-diff versus v2:\n\n1: 3a4012718b ! 1: a80353ab00 blame: print unblamable and ignored commits in porcelain mode\n @@ builtin/blame.c: static void emit_porcelain_details(struct blame_origin *suspect\n + * information which can be clubbed on a commit/file level, should\n + * be printed via 'emit_one_suspect_detail()'.\n + */\n -+static void emit_per_line_details(struct blame_entry *ent)\n ++static void emit_porcelain_per_line_details(struct blame_entry *ent)\n +{\n +\tif (mark_unblamable_lines && ent->unblamable)\n -+\t\tprintf(\"unblamable\\n\");\n ++\t\tputs(\"unblamable\\n\");\n +\tif (mark_ignored_lines && ent->ignored)\n -+\t\tprintf(\"ignored\\n\");\n ++\t\tputs(\"ignored\\n\");\n +}\n +\n static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,\n @@ builtin/blame.c: static void emit_porcelain(struct blame_scoreboard *sb, struct\n \t ent->lno + 1,\n \t ent->num_lines);\n \temit_porcelain_details(suspect, repeat);\n -+\temit_per_line_details(ent);\n ++\temit_porcelain_per_line_details(ent);\n \n \tcp = blame_nth_line(sb, ent->lno);\n \tfor (cnt = 0; cnt < ent->num_lines; cnt++) {\n @@ builtin/blame.c: static void emit_porcelain(struct blame_scoreboard *sb, struct\n \t\t\t ent->lno + 1 + cnt);\n \t\t\tif (repeat)\n \t\t\t\temit_porcelain_details(suspect, 1);\n -+\t\t\temit_per_line_details(ent);\n ++\t\t\temit_porcelain_per_line_details(ent);\n \t\t}\n \t\tputchar('\\t');\n \t\tdo {\n @@ t/t8013-blame-ignore-revs.sh: test_expect_success mark_unblamable_lines '\n \n +for opt in --porcelain --line-porcelain\n +do\n -+\ttest_expect_success 'mark_unblamable_lines with $opt' '\n ++\ttest_expect_success \"mark_unblamable_lines with $opt\" '\n +\t\tsha=$(git rev-parse Y) &&\n +\n +\t\tgit -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&\n @@ t/t8013-blame-ignore-revs.sh: test_expect_success mark_ignored_lines '\n \n +for opt in --porcelain --line-porcelain\n +do\n -+\ttest_expect_success 'mark_ignored_lines line_porcelain' '\n ++\ttest_expect_success \"mark_ignored_lines with $opt\" '\n +\t\tsha=$(git rev-parse Y) &&\n +\n +\t\tgit -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&\n---\n Documentation/blame-options.adoc | 3 ++-\n Documentation/git-blame.adoc | 9 +++++----\n builtin/blame.c | 15 +++++++++++++++\n t/t8013-blame-ignore-revs.sh | 20 ++++++++++++++++++++\n 4 files changed, 42 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/blame-options.adoc b/Documentation/blame-options.adoc\nindex aa77406d4e..19ea187238 100644\n--- a/Documentation/blame-options.adoc\n+++ b/Documentation/blame-options.adoc\n@@ -125,7 +125,8 @@ take effect.\n \tanother commit will be marked with a `?` in the blame output. If the\n \t`blame.markUnblamableLines` config option is set, then those lines touched\n \tby an ignored commit that we could not attribute to another revision are\n-\tmarked with a '*'.\n+\tmarked with a '*'. In the porcelain modes, we print 'ignored' and\n+\t'unblamable' on a newline respectively.\n \n --ignore-revs-file ::\n \tIgnore revisions listed in `file`, which must be in the same format as an\ndiff --git a/Documentation/git-blame.adoc b/Documentation/git-blame.adoc\nindex f75ed44790..e438d28625 100644\n--- a/Documentation/git-blame.adoc\n+++ b/Documentation/git-blame.adoc\n@@ -135,10 +135,11 @@ header elements later.\n The porcelain format generally suppresses commit information that has\n already been seen. For example, two lines that are blamed to the same\n commit will both be shown, but the details for that commit will be shown\n-only once. This is more efficient, but may require more state be kept by\n-the reader. The `--line-porcelain` option can be used to output full\n-commit information for each line, allowing simpler (but less efficient)\n-usage like:\n+only once. Information which is specific to individual lines will not be\n+grouped together, like revs to be marked 'ignored' or 'unblamable'. This\n+is more efficient, but may require more state be kept by the reader. The\n+`--line-porcelain` option can be used to output full commit information\n+for each line, allowing simpler (but less efficient) usage like:\n \n \t# count the number of lines attributed to each author\n \tgit blame --line-porcelain file |\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex c470654c7e..8a73b7be4a 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -351,6 +351,19 @@ static void emit_porcelain_details(struct blame_origin *suspect, int repeat)\n \t\twrite_filename_info(suspect);\n }\n \n+/*\n+ * Information which needs to be printed per-line goes here. Any\n+ * information which can be clubbed on a commit/file level, should\n+ * be printed via 'emit_one_suspect_detail()'.\n+ */\n+static void emit_porcelain_per_line_details(struct blame_entry *ent)\n+{\n+\tif (mark_unblamable_lines && ent->unblamable)\n+\t\tputs(\"unblamable\\n\");\n+\tif (mark_ignored_lines && ent->ignored)\n+\t\tputs(\"ignored\\n\");\n+}\n+\n static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,\n \t\t\t int opt)\n {\n@@ -367,6 +380,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,\n \t ent->lno + 1,\n \t ent->num_lines);\n \temit_porcelain_details(suspect, repeat);\n+\temit_porcelain_per_line_details(ent);\n \n \tcp = blame_nth_line(sb, ent->lno);\n \tfor (cnt = 0; cnt < ent->num_lines; cnt++) {\n@@ -377,6 +391,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,\n \t\t\t ent->lno + 1 + cnt);\n \t\t\tif (repeat)\n \t\t\t\temit_porcelain_details(suspect, 1);\n+\t\t\temit_porcelain_per_line_details(ent);\n \t\t}\n \t\tputchar('\\t');\n \t\tdo {\ndiff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh\nindex 370b768149..c5a7533232 100755\n--- a/t/t8013-blame-ignore-revs.sh\n+++ b/t/t8013-blame-ignore-revs.sh\n@@ -158,6 +158,16 @@ test_expect_success mark_unblamable_lines '\n \ttest_cmp expect actual\n '\n \n+for opt in --porcelain --line-porcelain\n+do\n+\ttest_expect_success \"mark_unblamable_lines with $opt\" '\n+\t\tsha=$(git rev-parse Y) &&\n+\n+\t\tgit -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&\n+\t\ttest $(grep ^unblamable actual | wc -l) -eq 2\n+\t'\n+done\n+\n # Commit Z will touch the first two lines. Y touched all four.\n # \tA--B--X--Y--Z\n # The blame output when ignoring Z should be:\n@@ -191,6 +201,16 @@ test_expect_success mark_ignored_lines '\n \t! test_cmp expect actual\n '\n \n+for opt in --porcelain --line-porcelain\n+do\n+\ttest_expect_success \"mark_ignored_lines with $opt\" '\n+\t\tsha=$(git rev-parse Y) &&\n+\n+\t\tgit -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&\n+\t\ttest $(grep ^ignored actual | wc -l) -eq 2\n+\t'\n+done\n+\n # For ignored revs that added 'unblamable' lines and more recent commits changed\n # the blamable lines, mark the unblamable lines with a\n # '*'\n\n---\n\nbase-commit: 683c54c999c301c2cd6f715c411407c413b1d84e\nchange-id: 20250321-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-4af46f02847e\n\nThanks\n- Karthik\n\n"},{"id":"515283","messageId":"xmqqldsnd5bu.fsf@gitster.g","threadId":"63172","inReplyTo":"CAPig+cRLXDohGG0RLpC41hy=rTmbmNQ_qScFWMv2LP8pdM8_hg@mail.gmail.com","subject":"Re: [PATCH v2] blame: print unblamable and ignored commits in porcelain mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-29T19:06:45Z","receivedAt":"2025-03-29T19:06:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n>> + test_expect_success 'mark_unblamable_lines with $opt' '\n>\n> This test title is going to display literal \"$opt\" rather than the\n> intended option. Fix this by replacing the single quotes with double\n> quotes:\n>\n> test_expect_success \"mark_unblamable_lines with $opt\" '\n\nWell spotted. Sorry for making all developers suffer the\nconsequence of a mistake I made 20 years ago. In retrospect, I\nreally should have anticipated that it would become a common pattern\nto have test_expect_success inside a loop, and made this part also\nbe interpolated, to make it look similar to the next parameter.\n"},{"id":"515290","messageId":"xmqqecyfazg2.fsf@gitster.g","threadId":"63172","inReplyTo":"20250329-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v3-1-10f695ae519a@gmail.com","subject":"Re: [PATCH v3] blame: print unblamable and ignored commits in porcelain mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-30T04:56:45Z","receivedAt":"2025-03-30T04:56:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> +static void emit_porcelain_per_line_details(struct blame_entry *ent)\n> +{\n> +\tif (mark_unblamable_lines && ent->unblamable)\n> +\t\tputs(\"unblamable\\n\");\n> +\tif (mark_ignored_lines && ent->ignored)\n> +\t\tputs(\"ignored\\n\");\n> +}\n\nDoesn't puts(3), unlike fputs(3), add its own trailing newline to\nstdout?\n"},{"id":"515294","messageId":"be0ca59b-2daa-4089-8948-d04fcdc3bfa7@gmail.com","threadId":"63172","inReplyTo":"xmqqecyfazg2.fsf@gitster.g","subject":"Re: [PATCH v3] blame: print unblamable and ignored commits in porcelain mode","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-03-30T09:28:00Z","receivedAt":"2025-03-30T09:28:04Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 30/03/2025 05:56, Junio C Hamano wrote:\n> Karthik Nayak writes:\n> \n>> +static void emit_porcelain_per_line_details(struct blame_entry *ent)\n>> +{\n>> +\tif (mark_unblamable_lines && ent->unblamable)\n>> +\t\tputs(\"unblamable\\n\");\n>> +\tif (mark_ignored_lines && ent->ignored)\n>> +\t\tputs(\"ignored\\n\");\n>> +}\n> \n> Doesn't puts(3), unlike fputs(3), add its own trailing newline to\n> stdout?\n\nIndeed. Perhaps the regression test would be more effective if it used \ntest_cmp rather than\n\n test $(grep ^ignored actual | wc -l) -eq 2\n\nwhich misses this bug and gives no output if it fails.\n\nBest Wishes\n\nPhillip\n\n"},{"id":"515310","messageId":"20250330204339.191382-1-karthik.188@gmail.com","threadId":"63172","inReplyTo":"20250321-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v1-1-44b562d9beb8@gmail.com","subject":"[PATCH v4] blame: print unblamable and ignored commits in porcelain mode","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-03-30T20:43:39Z","receivedAt":"2025-03-30T20:43:46Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The 'git-blame(1)' command allows users to ignore specific revisions via\nthe '--ignore-rev ' and '--ignore-revs-file ' flags. These\nflags are often combined with the 'blame.markIgnoredLines' and\n'blame.markUnblamableLines' config options. These config options prefix\nignored and unblamable lines with a '?' and '*', respectively.\n\nHowever, this option was never extended to the porcelain mode of\n'git-blame(1)'. Since the documentation does not indicate this\nexclusion, it is a bug.\n\nFix this by printing 'ignored' and 'unblamable' respectively for the\noptions when using the porcelain modes.\n\nHelped-by: Patrick Steinhardt \nHelped-by: Toon Claes \nSigned-off-by: Karthik Nayak \n---\nChanges in v4:\n- Remove extra newline in 'puts'. Modify the test to compare the\n entire output, the earlier test missed the extraneous newline.\n- 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\n\nChanges in v3:\n- Use double-qoutes in the test to ensure correct variable dereference.\n- Fix incorrect test name. \n- Rename the function from 'emit_per_line_details()' to\n 'emit_porcelain_per_line_details()' to be more descriptive.\n- Ues 'puts()' instead of 'printf()'.\n- Link to v2:\nhttps://lore.kernel.org/r/20250326-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v2-1-79037e17a74b@gmail.com\n\nChanges in v2:\n- Instead of printing the markers before the SHA in porcelain\n mode and breaking scripts and backward compatability, let's \n instead add a newline printing 'unblamable' or 'ignored'.\n This is printed per line in both the porcelain modes. \n- Link to v1:\nhttps://lore.kernel.org/r/20250321-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v1-1-44b562d9beb8@gmail.com\n---\nRange-diff versus v3:\n\n1: bc359248f6 ! 1: 5250fb436e blame: print unblamable and ignored commits in porcelain mode\n @@ builtin/blame.c: static void emit_porcelain_details(struct blame_origin *suspect\n +static void emit_porcelain_per_line_details(struct blame_entry *ent)\n +{\n +\tif (mark_unblamable_lines && ent->unblamable)\n -+\t\tputs(\"unblamable\\n\");\n ++\t\tputs(\"unblamable\");\n +\tif (mark_ignored_lines && ent->ignored)\n -+\t\tputs(\"ignored\\n\");\n ++\t\tputs(\"ignored\");\n +}\n +\n static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,\n @@ t/t8013-blame-ignore-revs.sh: test_expect_success mark_unblamable_lines '\n +\ttest_expect_success \"mark_unblamable_lines with $opt\" '\n +\t\tsha=$(git rev-parse Y) &&\n +\n ++\t\tgit -c blame.markUnblamableLines=false blame $opt --ignore-rev Y file >raw &&\n ++\t\tsed -e \"s/^\\ty3/unblamable\\n&/\" raw >expect &&\n ++\t\tcp expect raw &&\n ++\t\tsed -e \"s/^\\ty4/unblamable\\n&/\" raw >expect &&\n ++\n +\t\tgit -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&\n -+\t\ttest $(grep ^unblamable actual | wc -l) -eq 2\n ++\t\ttest_cmp expect actual\n +\t'\n +done\n +\n @@ t/t8013-blame-ignore-revs.sh: test_expect_success mark_ignored_lines '\n +\ttest_expect_success \"mark_ignored_lines with $opt\" '\n +\t\tsha=$(git rev-parse Y) &&\n +\n ++\t\tgit -c blame.markIgnoredLines=false blame $opt --ignore-rev Z file >raw &&\n ++\t\tsed -e \"s/^\\tline-one-Z/ignored\\n&/\" raw >expect &&\n ++\t\tcp expect raw &&\n ++\t\tsed -e \"s/^\\tline-two-Z/ignored\\n&/\" raw >expect &&\n ++\n +\t\tgit -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&\n -+\t\ttest $(grep ^ignored actual | wc -l) -eq 2\n ++\t\ttest_cmp expect actual\n +\t'\n +done\n +\n---\n Documentation/blame-options.adoc | 3 ++-\n Documentation/git-blame.adoc | 9 +++++----\n builtin/blame.c | 15 +++++++++++++++\n t/t8013-blame-ignore-revs.sh | 30 ++++++++++++++++++++++++++++++\n 4 files changed, 52 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/blame-options.adoc b/Documentation/blame-options.adoc\nindex aa77406d4e..19ea187238 100644\n--- a/Documentation/blame-options.adoc\n+++ b/Documentation/blame-options.adoc\n@@ -125,7 +125,8 @@ take effect.\n \tanother commit will be marked with a `?` in the blame output. If the\n \t`blame.markUnblamableLines` config option is set, then those lines touched\n \tby an ignored commit that we could not attribute to another revision are\n-\tmarked with a '*'.\n+\tmarked with a '*'. In the porcelain modes, we print 'ignored' and\n+\t'unblamable' on a newline respectively.\n \n --ignore-revs-file ::\n \tIgnore revisions listed in `file`, which must be in the same format as an\ndiff --git a/Documentation/git-blame.adoc b/Documentation/git-blame.adoc\nindex f75ed44790..e438d28625 100644\n--- a/Documentation/git-blame.adoc\n+++ b/Documentation/git-blame.adoc\n@@ -135,10 +135,11 @@ header elements later.\n The porcelain format generally suppresses commit information that has\n already been seen. For example, two lines that are blamed to the same\n commit will both be shown, but the details for that commit will be shown\n-only once. This is more efficient, but may require more state be kept by\n-the reader. The `--line-porcelain` option can be used to output full\n-commit information for each line, allowing simpler (but less efficient)\n-usage like:\n+only once. Information which is specific to individual lines will not be\n+grouped together, like revs to be marked 'ignored' or 'unblamable'. This\n+is more efficient, but may require more state be kept by the reader. The\n+`--line-porcelain` option can be used to output full commit information\n+for each line, allowing simpler (but less efficient) usage like:\n \n \t# count the number of lines attributed to each author\n \tgit blame --line-porcelain file |\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex c470654c7e..9436f70aec 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -351,6 +351,19 @@ static void emit_porcelain_details(struct blame_origin *suspect, int repeat)\n \t\twrite_filename_info(suspect);\n }\n \n+/*\n+ * Information which needs to be printed per-line goes here. Any\n+ * information which can be clubbed on a commit/file level, should\n+ * be printed via 'emit_one_suspect_detail()'.\n+ */\n+static void emit_porcelain_per_line_details(struct blame_entry *ent)\n+{\n+\tif (mark_unblamable_lines && ent->unblamable)\n+\t\tputs(\"unblamable\");\n+\tif (mark_ignored_lines && ent->ignored)\n+\t\tputs(\"ignored\");\n+}\n+\n static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,\n \t\t\t int opt)\n {\n@@ -367,6 +380,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,\n \t ent->lno + 1,\n \t ent->num_lines);\n \temit_porcelain_details(suspect, repeat);\n+\temit_porcelain_per_line_details(ent);\n \n \tcp = blame_nth_line(sb, ent->lno);\n \tfor (cnt = 0; cnt < ent->num_lines; cnt++) {\n@@ -377,6 +391,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,\n \t\t\t ent->lno + 1 + cnt);\n \t\t\tif (repeat)\n \t\t\t\temit_porcelain_details(suspect, 1);\n+\t\t\temit_porcelain_per_line_details(ent);\n \t\t}\n \t\tputchar('\\t');\n \t\tdo {\ndiff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh\nindex 370b768149..50a0a7ca4a 100755\n--- a/t/t8013-blame-ignore-revs.sh\n+++ b/t/t8013-blame-ignore-revs.sh\n@@ -158,6 +158,21 @@ test_expect_success mark_unblamable_lines '\n \ttest_cmp expect actual\n '\n \n+for opt in --porcelain --line-porcelain\n+do\n+\ttest_expect_success \"mark_unblamable_lines with $opt\" '\n+\t\tsha=$(git rev-parse Y) &&\n+\n+\t\tgit -c blame.markUnblamableLines=false blame $opt --ignore-rev Y file >raw &&\n+\t\tsed -e \"s/^\\ty3/unblamable\\n&/\" raw >expect &&\n+\t\tcp expect raw &&\n+\t\tsed -e \"s/^\\ty4/unblamable\\n&/\" raw >expect &&\n+\n+\t\tgit -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+done\n+\n # Commit Z will touch the first two lines. Y touched all four.\n # \tA--B--X--Y--Z\n # The blame output when ignoring Z should be:\n@@ -191,6 +206,21 @@ test_expect_success mark_ignored_lines '\n \t! test_cmp expect actual\n '\n \n+for opt in --porcelain --line-porcelain\n+do\n+\ttest_expect_success \"mark_ignored_lines with $opt\" '\n+\t\tsha=$(git rev-parse Y) &&\n+\n+\t\tgit -c blame.markIgnoredLines=false blame $opt --ignore-rev Z file >raw &&\n+\t\tsed -e \"s/^\\tline-one-Z/ignored\\n&/\" raw >expect &&\n+\t\tcp expect raw &&\n+\t\tsed -e \"s/^\\tline-two-Z/ignored\\n&/\" raw >expect &&\n+\n+\t\tgit -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+done\n+\n # For ignored revs that added 'unblamable' lines and more recent commits changed\n # the blamable lines, mark the unblamable lines with a\n # '*'\n-- \n2.48.1\n\n"},{"id":"515324","messageId":"Z-o-suJAfu5yc2AW@pks.im","threadId":"63172","inReplyTo":"20250330204339.191382-1-karthik.188@gmail.com","subject":"Re: [PATCH v4] blame: print unblamable and ignored commits in porcelain mode","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-31T07:05:22Z","receivedAt":"2025-03-31T07:05:28Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sun, Mar 30, 2025 at 10:43:39PM +0200, Karthik Nayak wrote:\n> diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh\n> index 370b768149..50a0a7ca4a 100755\n> --- a/t/t8013-blame-ignore-revs.sh\n> +++ b/t/t8013-blame-ignore-revs.sh\n> @@ -158,6 +158,21 @@ test_expect_success mark_unblamable_lines '\n> \ttest_cmp expect actual\n> '\n> \n> +for opt in --porcelain --line-porcelain\n> +do\n> +\ttest_expect_success \"mark_unblamable_lines with $opt\" '\n> +\t\tsha=$(git rev-parse Y) &&\n> +\n> +\t\tgit -c blame.markUnblamableLines=false blame $opt --ignore-rev Y file >raw &&\n> +\t\tsed -e \"s/^\\ty3/unblamable\\n&/\" raw >expect &&\n> +\t\tcp expect raw &&\n> +\t\tsed -e \"s/^\\ty4/unblamable\\n&/\" raw >expect &&\n\nThe intent here is to do two replacements in \"raw\", right? You can do\nthis with a single call to sed(1) by chaining \"-e\":\n\n\tgit -c blame.markUnblamableLines=false blame $opt --ignore-rev Y file >raw &&\n\tsed -e \"s/^\\ty3/unblamable\\n&/\" \\\n -e \"s/^\\ty4/unblamable\\n&/\" raw >expect &&\n\nPatrick\n"},{"id":"515329","messageId":"CAOLa=ZTxjgdndbFHOC9dwxbjo6hUyrmyddGH1uhS0WYOSpY=fA@mail.gmail.com","threadId":"63172","inReplyTo":"Z-o-suJAfu5yc2AW@pks.im","subject":"Re: [PATCH v4] blame: print unblamable and ignored commits in porcelain mode","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-03-31T07:34:54Z","receivedAt":"2025-03-31T07:34:56Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Sun, Mar 30, 2025 at 10:43:39PM +0200, Karthik Nayak wrote:\n>> diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh\n>> index 370b768149..50a0a7ca4a 100755\n>> --- a/t/t8013-blame-ignore-revs.sh\n>> +++ b/t/t8013-blame-ignore-revs.sh\n>> @@ -158,6 +158,21 @@ test_expect_success mark_unblamable_lines '\n>> \ttest_cmp expect actual\n>> '\n>>\n>> +for opt in --porcelain --line-porcelain\n>> +do\n>> +\ttest_expect_success \"mark_unblamable_lines with $opt\" '\n>> +\t\tsha=$(git rev-parse Y) &&\n>> +\n>> +\t\tgit -c blame.markUnblamableLines=false blame $opt --ignore-rev Y file >raw &&\n>> +\t\tsed -e \"s/^\\ty3/unblamable\\n&/\" raw >expect &&\n>> +\t\tcp expect raw &&\n>> +\t\tsed -e \"s/^\\ty4/unblamable\\n&/\" raw >expect &&\n>\n> The intent here is to do two replacements in \"raw\", right? You can do\n> this with a single call to sed(1) by chaining \"-e\":\n>\n> \tgit -c blame.markUnblamableLines=false blame $opt --ignore-rev Y file >raw &&\n> \tsed -e \"s/^\\ty3/unblamable\\n&/\" \\\n> -e \"s/^\\ty4/unblamable\\n&/\" raw >expect &&\n>\n> Patrick\n\nNice, I first tried to use `-i`, but seems like that doesn't work with\nthe 'sed' shipped in OSX. Didn't know I could chain it. This makes it\ncleaner.\n"},{"id":"515358","messageId":"72b19ccb-a6a8-4aca-b70e-b879ec15cff1@gmail.com","threadId":"63172","inReplyTo":"20250330204339.191382-1-karthik.188@gmail.com","subject":"Re: [PATCH v4] blame: print unblamable and ignored commits in porcelain mode","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-03-31T10:24:53Z","receivedAt":"2025-03-31T10:24:58Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Karthik\n\nOn 30/03/2025 21:43, Karthik Nayak wrote:\n> \n> +for opt in --porcelain --line-porcelain\n> +do\n> +\ttest_expect_success \"mark_unblamable_lines with $opt\" '\n> +\t\tsha=$(git rev-parse Y) &&\n> +\n> +\t\tgit -c blame.markUnblamableLines=false blame $opt --ignore-rev Y file >raw &&\n> +\t\tsed -e \"s/^\\ty3/unblamable\\n&/\" raw >expect &&\n> +\t\tcp expect raw &&\n> +\t\tsed -e \"s/^\\ty4/unblamable\\n&/\" raw >expect &&\n\nThanks for improving the test. Unfortunately using '\\n' in the \nreplacement text is not portable [1] (the normal backslash escapes are \nallowed in the pattern though so the '\\t' is fine). One has to write a \nliteral newline escaped with a backslash. However here we want to insert \na whole new line of text into the output without changing the original \nso I would write it as\n\n sed -e \"/^\\ty3/a\\\\\" -e unblamable -e \"/^\\ty4/a\\\\\" -e unblamable \\\n\traw >expect\n\nBest Wishes\n\nPhillip\n\n[1] \n\n The relevant section of the text reads\n\n A line can be split by substituting a into it. The\n application shall escape the in the replacement by\n preceding it by a .\n\n The meaning of an unescaped immediately followed by any\n character other than '&', , a digit, , or the\n delimiter character used for this command, is unspecified.\n\n\n> +\n> +\t\tgit -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&\n> +\t\ttest_cmp expect actual\n> +\t'\n> +done\n> +\n> # Commit Z will touch the first two lines. Y touched all four.\n> # \tA--B--X--Y--Z\n> # The blame output when ignoring Z should be:\n> @@ -191,6 +206,21 @@ test_expect_success mark_ignored_lines '\n> \t! test_cmp expect actual\n> '\n> \n> +for opt in --porcelain --line-porcelain\n> +do\n> +\ttest_expect_success \"mark_ignored_lines with $opt\" '\n> +\t\tsha=$(git rev-parse Y) &&\n> +\n> +\t\tgit -c blame.markIgnoredLines=false blame $opt --ignore-rev Z file >raw &&\n> +\t\tsed -e \"s/^\\tline-one-Z/ignored\\n&/\" raw >expect &&\n> +\t\tcp expect raw &&\n> +\t\tsed -e \"s/^\\tline-two-Z/ignored\\n&/\" raw >expect &&\n> +\n> +\t\tgit -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&\n> +\t\ttest_cmp expect actual\n> +\t'\n> +done\n> +\n> # For ignored revs that added 'unblamable' lines and more recent commits changed\n> # the blamable lines, mark the unblamable lines with a\n> # '*'\n\n"},{"id":"515359","messageId":"acac3dc0-deaf-44f5-a568-2de3060fecf2@gmail.com","threadId":"63172","inReplyTo":"72b19ccb-a6a8-4aca-b70e-b879ec15cff1@gmail.com","subject":"Re: [PATCH v4] blame: print unblamable and ignored commits in porcelain mode","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-03-31T10:47:01Z","receivedAt":"2025-03-31T10:47:05Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 31/03/2025 11:24, phillip.wood123@gmail.com wrote:\n >\n > (the normal backslash escapes are > allowed in the pattern though so \nthe '\\t' is fine).\n\nSorry that's wrong, you need to use a literal tab character instead of '\\t'\n\nBest Wishes\n\nPhillip\n\n"},{"id":"515517","messageId":"494c9e59-b7b2-4a1c-ad41-3a7fc72503c9@gmail.com","threadId":"63172","inReplyTo":"CAOLa=ZQMYn2eYndX0saTKnuzAacjtNZeTb9PCrcNC50nneAq5g@mail.gmail.com","subject":"Re: [PATCH v4] blame: print unblamable and ignored commits in porcelain mode","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-04-02T13:07:31Z","receivedAt":"2025-04-02T13:07:39Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"[Restoring cc to the mailing list that I accidentally dropped in my \nprevious message]\n\nOn 01/04/2025 17:57, Karthik Nayak wrote:\n> Phillip Wood writes:\n>> On 31/03/2025 16:25, Karthik Nayak wrote:\n>>> phillip.wood123@gmail.com writes:\n>>>> On 30/03/2025 21:43, Karthik Nayak wrote:\n>>>>>\n>>>>> +for opt in --porcelain --line-porcelain\n>>>>> +do\n>>>>> +\ttest_expect_success \"mark_unblamable_lines with $opt\" '\n>>>>> +\t\tsha=$(git rev-parse Y) &&\n>>>>> +\n>>>>> +\t\tgit -c blame.markUnblamableLines=false blame $opt --ignore-rev Y file >raw &&\n>>>>> +\t\tsed -e \"s/^\\ty3/unblamable\\n&/\" raw >expect &&\n>>>>> +\t\tcp expect raw &&\n>>>>> +\t\tsed -e \"s/^\\ty4/unblamable\\n&/\" raw >expect &&\n>>>>\n>>>> Thanks for improving the test. Unfortunately using '\\n' in the\n>>>> replacement text is not portable [1] (the normal backslash escapes are\n>>>> allowed in the pattern though so the '\\t' is fine). One has to write a\n>>>> literal newline escaped with a backslash. However here we want to insert\n>>>> a whole new line of text into the output without changing the original\n>>>> so I would write it as\n>>>\n>>> Thanks for bringing this to my notice. I didn't know.\n>>>\n>>>> sed -e \"/^\\ty3/a\\\\\" -e unblamable -e \"/^\\ty4/a\\\\\" -e unblamable \\\n>>>> \traw >expect\n>>>\n>>> This appends 'unblamable' to the next line, but we want to prepend it.\n>>\n>> Sorry, I misread the original. If you use 'i' instead of 'a' that will\n>> insert a new line before the current one.\n> \n> Seems like this won't work either, since MacOS complains [1] about it:\n> \n> expecting success of 8013.16 'mark_ignored_lines with --line-porcelain':\n> \t\tsha=$(git rev-parse Y) &&\n> \t\tgit -c blame.markIgnoredLines=false blame $opt --ignore-rev Z file >raw &&\n> \t\tsed -e \"/^\tline-one-Z/i\\\\\" -e ignored \\\n> \t\t -e \"/^\tline-two-Z/i\\\\\" -e ignored \\\n> \t\t\traw >expect &&\n> \t\tgit -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&\n> \t\ttest_cmp expect actual\n> \t\n> +++ git rev-parse Y\n> ++ sha=e0d35d6f2d5fab63267e58d684cea1ECG86f12b1\n> ++ git -c blame.markIgnoredLines=false blame --line-porcelain\n> --ignore-rev Z file\n> ++ sed -e '/^\tline-one-Z/i\\' -e ignored -e '/^\tline-two-Z/i\\' -e ignored raw\n> sed: 1: \"ignored\n> \": command i expects \\ followed by text\n> error: last command exited with $?=1\n\nOh, that's a pain, I thought it was supposed to treat each '-e' as a \nseparate line.\n\n> I did have success [2] with using a heredoc instead:\n> \n> \tcat > sedscript <<- 'EOF' &&\n> \t/^\ty3/i\\\\\n> \tunblamable\n> \t/^\ty4/i\\\\\n> \tunblamable\n> \tEOF\n> \tsed -f sedscript raw >expect &&\n> \n> What do you think about this?\n\nI think that's the best way then. We could pass a multiline string with \n'-e' but then we wouldn't be able to indent the \"unblamable\" lines.\n\nBest Wishes\n\nPhillip\n\n> [1]: https://gitlab.com/gitlab-org/git/-/jobs/9581456879\n> [2]: https://gitlab.com/gitlab-org/git/-/pipelines/1746265204\n\n"},{"id":"515595","messageId":"20250403160326.120124-1-karthik.188@gmail.com","threadId":"63172","inReplyTo":"20250321-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v1-1-44b562d9beb8@gmail.com","subject":"[PATCH v5] blame: print unblamable and ignored commits in porcelain mode","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-04-03T16:03:26Z","receivedAt":"2025-04-03T16:03:32Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The 'git-blame(1)' command allows users to ignore specific revisions via\nthe '--ignore-rev ' and '--ignore-revs-file ' flags. These\nflags are often combined with the 'blame.markIgnoredLines' and\n'blame.markUnblamableLines' config options. These config options prefix\nignored and unblamable lines with a '?' and '*', respectively.\n\nHowever, this option was never extended to the porcelain mode of\n'git-blame(1)'. Since the documentation does not indicate this\nexclusion, it is a bug.\n\nFix this by printing 'ignored' and 'unblamable' respectively for the\noptions when using the porcelain modes.\n\nHelped-by: Patrick Steinhardt \nHelped-by: Toon Claes \nHelped-by: Phillip Wood \nSigned-off-by: Karthik Nayak \n---\nChanges in v5:\n- Fix the test to be more portable by not using '\\n' in 'sed'. \n- Link to v4: https://lore.kernel.org/all/20250330204339.191382-1-karthik.188@gmail.com/\n\nChanges in v4:\n- Remove extra newline in 'puts'. Modify the test to compare the\n entire output, the earlier test missed the extraneous newline.\n- Link to v3:\nhttps://lore.kernel.org/r/20250329-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v3-1-10f695ae519a@gmail.com\n\nChanges in v3:\n- Use double-qoutes in the test to ensure correct variable dereference.\n- Fix incorrect test name. \n- Rename the function from 'emit_per_line_details()' to\n 'emit_porcelain_per_line_details()' to be more descriptive.\n- Ues 'puts()' instead of 'printf()'.\n- Link to v2:\nhttps://lore.kernel.org/r/20250326-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v2-1-79037e17a74b@gmail.com\n\nChanges in v2:\n- Instead of printing the markers before the SHA in porcelain\n mode and breaking scripts and backward compatability, let's \n instead add a newline printing 'unblamable' or 'ignored'.\n This is printed per line in both the porcelain modes. \n- Link to v1:\nhttps://lore.kernel.org/r/20250321-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v1-1-44b562d9beb8@gmail.com\n---\nRange-diff versus v4:\n\n1: 5250fb436e ! 1: 43bc55bffe blame: print unblamable and ignored commits in porcelain mode\n @@ Commit message\n \n Helped-by: Patrick Steinhardt \n Helped-by: Toon Claes \n + Helped-by: Phillip Wood \n Signed-off-by: Karthik Nayak \n \n ## Documentation/blame-options.adoc ##\n @@ t/t8013-blame-ignore-revs.sh: test_expect_success mark_unblamable_lines '\n \n +for opt in --porcelain --line-porcelain\n +do\n -+\ttest_expect_success \"mark_unblamable_lines with $opt\" '\n ++\ttest_expect_success \"mark_unblamable_lines with $opt\" \"\n +\t\tsha=$(git rev-parse Y) &&\n +\n +\t\tgit -c blame.markUnblamableLines=false blame $opt --ignore-rev Y file >raw &&\n -+\t\tsed -e \"s/^\\ty3/unblamable\\n&/\" raw >expect &&\n -+\t\tcp expect raw &&\n -+\t\tsed -e \"s/^\\ty4/unblamable\\n&/\" raw >expect &&\n ++\t\tcat > sedscript <<- 'EOF' &&\n ++\t\t/^\ty3/i\\\\\n ++\t\tunblamable\n ++\t\t/^\ty4/i\\\\\n ++\t\tunblamable\n ++\t\tEOF\n ++\t\tsed -f sedscript raw >expect &&\n +\n +\t\tgit -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&\n +\t\ttest_cmp expect actual\n -+\t'\n ++\t\"\n +done\n +\n # Commit Z will touch the first two lines. Y touched all four.\n @@ t/t8013-blame-ignore-revs.sh: test_expect_success mark_ignored_lines '\n \n +for opt in --porcelain --line-porcelain\n +do\n -+\ttest_expect_success \"mark_ignored_lines with $opt\" '\n ++\ttest_expect_success \"mark_ignored_lines with $opt\" \"\n +\t\tsha=$(git rev-parse Y) &&\n +\n +\t\tgit -c blame.markIgnoredLines=false blame $opt --ignore-rev Z file >raw &&\n -+\t\tsed -e \"s/^\\tline-one-Z/ignored\\n&/\" raw >expect &&\n -+\t\tcp expect raw &&\n -+\t\tsed -e \"s/^\\tline-two-Z/ignored\\n&/\" raw >expect &&\n ++\t\tcat > sedscript <<- 'EOF' &&\n ++\t\t/^\tline-one-Z/i\\\\\n ++\t\tignored\n ++\t\t/^\tline-two-Z/i\\\\\n ++\t\tignored\n ++\t\tEOF\n ++\t\tsed -f sedscript raw >expect &&\n +\n +\t\tgit -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&\n +\t\ttest_cmp expect actual\n -+\t'\n ++\t\"\n +done\n +\n # For ignored revs that added 'unblamable' lines and more recent commits changed\n\n---\n Documentation/blame-options.adoc | 3 ++-\n Documentation/git-blame.adoc | 9 ++++----\n builtin/blame.c | 15 +++++++++++++\n t/t8013-blame-ignore-revs.sh | 38 ++++++++++++++++++++++++++++++++\n 4 files changed, 60 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/blame-options.adoc b/Documentation/blame-options.adoc\nindex aa77406d4e..19ea187238 100644\n--- a/Documentation/blame-options.adoc\n+++ b/Documentation/blame-options.adoc\n@@ -125,7 +125,8 @@ take effect.\n \tanother commit will be marked with a `?` in the blame output. If the\n \t`blame.markUnblamableLines` config option is set, then those lines touched\n \tby an ignored commit that we could not attribute to another revision are\n-\tmarked with a '*'.\n+\tmarked with a '*'. In the porcelain modes, we print 'ignored' and\n+\t'unblamable' on a newline respectively.\n \n --ignore-revs-file ::\n \tIgnore revisions listed in `file`, which must be in the same format as an\ndiff --git a/Documentation/git-blame.adoc b/Documentation/git-blame.adoc\nindex f75ed44790..e438d28625 100644\n--- a/Documentation/git-blame.adoc\n+++ b/Documentation/git-blame.adoc\n@@ -135,10 +135,11 @@ header elements later.\n The porcelain format generally suppresses commit information that has\n already been seen. For example, two lines that are blamed to the same\n commit will both be shown, but the details for that commit will be shown\n-only once. This is more efficient, but may require more state be kept by\n-the reader. The `--line-porcelain` option can be used to output full\n-commit information for each line, allowing simpler (but less efficient)\n-usage like:\n+only once. Information which is specific to individual lines will not be\n+grouped together, like revs to be marked 'ignored' or 'unblamable'. This\n+is more efficient, but may require more state be kept by the reader. The\n+`--line-porcelain` option can be used to output full commit information\n+for each line, allowing simpler (but less efficient) usage like:\n \n \t# count the number of lines attributed to each author\n \tgit blame --line-porcelain file |\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex c470654c7e..9436f70aec 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -351,6 +351,19 @@ static void emit_porcelain_details(struct blame_origin *suspect, int repeat)\n \t\twrite_filename_info(suspect);\n }\n \n+/*\n+ * Information which needs to be printed per-line goes here. Any\n+ * information which can be clubbed on a commit/file level, should\n+ * be printed via 'emit_one_suspect_detail()'.\n+ */\n+static void emit_porcelain_per_line_details(struct blame_entry *ent)\n+{\n+\tif (mark_unblamable_lines && ent->unblamable)\n+\t\tputs(\"unblamable\");\n+\tif (mark_ignored_lines && ent->ignored)\n+\t\tputs(\"ignored\");\n+}\n+\n static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,\n \t\t\t int opt)\n {\n@@ -367,6 +380,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,\n \t ent->lno + 1,\n \t ent->num_lines);\n \temit_porcelain_details(suspect, repeat);\n+\temit_porcelain_per_line_details(ent);\n \n \tcp = blame_nth_line(sb, ent->lno);\n \tfor (cnt = 0; cnt < ent->num_lines; cnt++) {\n@@ -377,6 +391,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,\n \t\t\t ent->lno + 1 + cnt);\n \t\t\tif (repeat)\n \t\t\t\temit_porcelain_details(suspect, 1);\n+\t\t\temit_porcelain_per_line_details(ent);\n \t\t}\n \t\tputchar('\\t');\n \t\tdo {\ndiff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh\nindex 370b768149..cace00ae8d 100755\n--- a/t/t8013-blame-ignore-revs.sh\n+++ b/t/t8013-blame-ignore-revs.sh\n@@ -158,6 +158,25 @@ test_expect_success mark_unblamable_lines '\n \ttest_cmp expect actual\n '\n \n+for opt in --porcelain --line-porcelain\n+do\n+\ttest_expect_success \"mark_unblamable_lines with $opt\" \"\n+\t\tsha=$(git rev-parse Y) &&\n+\n+\t\tgit -c blame.markUnblamableLines=false blame $opt --ignore-rev Y file >raw &&\n+\t\tcat > sedscript <<- 'EOF' &&\n+\t\t/^\ty3/i\\\\\n+\t\tunblamable\n+\t\t/^\ty4/i\\\\\n+\t\tunblamable\n+\t\tEOF\n+\t\tsed -f sedscript raw >expect &&\n+\n+\t\tgit -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&\n+\t\ttest_cmp expect actual\n+\t\"\n+done\n+\n # Commit Z will touch the first two lines. Y touched all four.\n # \tA--B--X--Y--Z\n # The blame output when ignoring Z should be:\n@@ -191,6 +210,25 @@ test_expect_success mark_ignored_lines '\n \t! test_cmp expect actual\n '\n \n+for opt in --porcelain --line-porcelain\n+do\n+\ttest_expect_success \"mark_ignored_lines with $opt\" \"\n+\t\tsha=$(git rev-parse Y) &&\n+\n+\t\tgit -c blame.markIgnoredLines=false blame $opt --ignore-rev Z file >raw &&\n+\t\tcat > sedscript <<- 'EOF' &&\n+\t\t/^\tline-one-Z/i\\\\\n+\t\tignored\n+\t\t/^\tline-two-Z/i\\\\\n+\t\tignored\n+\t\tEOF\n+\t\tsed -f sedscript raw >expect &&\n+\n+\t\tgit -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&\n+\t\ttest_cmp expect actual\n+\t\"\n+done\n+\n # For ignored revs that added 'unblamable' lines and more recent commits changed\n # the blamable lines, mark the unblamable lines with a\n # '*'\n-- \n2.48.1\n\n"},{"id":"515678","messageId":"2fbecc76-c3d9-4a8e-9326-5b83db1ceb26@gmail.com","threadId":"63172","inReplyTo":"20250403160326.120124-1-karthik.188@gmail.com","subject":"Re: [PATCH v5] blame: print unblamable and ignored commits in porcelain mode","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-04-04T15:58:44Z","receivedAt":"2025-04-04T15:58:49Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Karthik\n\nOn 03/04/2025 17:03, Karthik Nayak wrote:\n> The 'git-blame(1)' command allows users to ignore specific revisions via\n> the '--ignore-rev ' and '--ignore-revs-file ' flags. These\n> flags are often combined with the 'blame.markIgnoredLines' and\n> 'blame.markUnblamableLines' config options. These config options prefix\n> ignored and unblamable lines with a '?' and '*', respectively.\n> \n> However, this option was never extended to the porcelain mode of\n> 'git-blame(1)'. Since the documentation does not indicate this\n> exclusion, it is a bug.\n> \n> Fix this by printing 'ignored' and 'unblamable' respectively for the\n> options when using the porcelain modes.\n\nThis looks good to me\n\nBest Wishes\n\nPhillip\n\n> Helped-by: Patrick Steinhardt \n> Helped-by: Toon Claes \n> Helped-by: Phillip Wood \n> Signed-off-by: Karthik Nayak \n> ---\n> Changes in v5:\n> - Fix the test to be more portable by not using '\\n' in 'sed'.\n> - Link to v4: https://lore.kernel.org/all/20250330204339.191382-1-karthik.188@gmail.com/\n> \n> Changes in v4:\n> - Remove extra newline in 'puts'. Modify the test to compare the\n> entire output, the earlier test missed the extraneous newline.\n> - Link to v3:\n> 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\n> \n> Changes in v3:\n> - Use double-qoutes in the test to ensure correct variable dereference.\n> - Fix incorrect test name.\n> - Rename the function from 'emit_per_line_details()' to\n> 'emit_porcelain_per_line_details()' to be more descriptive.\n> - Ues 'puts()' instead of 'printf()'.\n> - Link to v2:\n> 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\n> \n> Changes in v2:\n> - Instead of printing the markers before the SHA in porcelain\n> mode and breaking scripts and backward compatability, let's\n> instead add a newline printing 'unblamable' or 'ignored'.\n> This is printed per line in both the porcelain modes.\n> - Link to v1:\n> 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\n> ---\n> Range-diff versus v4:\n> \n> 1: 5250fb436e ! 1: 43bc55bffe blame: print unblamable and ignored commits in porcelain mode\n> @@ Commit message\n> \n> Helped-by: Patrick Steinhardt \n> Helped-by: Toon Claes \n> + Helped-by: Phillip Wood \n> Signed-off-by: Karthik Nayak \n> \n> ## Documentation/blame-options.adoc ##\n> @@ t/t8013-blame-ignore-revs.sh: test_expect_success mark_unblamable_lines '\n> \n> +for opt in --porcelain --line-porcelain\n> +do\n> -+\ttest_expect_success \"mark_unblamable_lines with $opt\" '\n> ++\ttest_expect_success \"mark_unblamable_lines with $opt\" \"\n> +\t\tsha=$(git rev-parse Y) &&\n> +\n> +\t\tgit -c blame.markUnblamableLines=false blame $opt --ignore-rev Y file >raw &&\n> -+\t\tsed -e \"s/^\\ty3/unblamable\\n&/\" raw >expect &&\n> -+\t\tcp expect raw &&\n> -+\t\tsed -e \"s/^\\ty4/unblamable\\n&/\" raw >expect &&\n> ++\t\tcat > sedscript <<- 'EOF' &&\n> ++\t\t/^\ty3/i\\\\\n> ++\t\tunblamable\n> ++\t\t/^\ty4/i\\\\\n> ++\t\tunblamable\n> ++\t\tEOF\n> ++\t\tsed -f sedscript raw >expect &&\n> +\n> +\t\tgit -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&\n> +\t\ttest_cmp expect actual\n> -+\t'\n> ++\t\"\n> +done\n> +\n> # Commit Z will touch the first two lines. Y touched all four.\n> @@ t/t8013-blame-ignore-revs.sh: test_expect_success mark_ignored_lines '\n> \n> +for opt in --porcelain --line-porcelain\n> +do\n> -+\ttest_expect_success \"mark_ignored_lines with $opt\" '\n> ++\ttest_expect_success \"mark_ignored_lines with $opt\" \"\n> +\t\tsha=$(git rev-parse Y) &&\n> +\n> +\t\tgit -c blame.markIgnoredLines=false blame $opt --ignore-rev Z file >raw &&\n> -+\t\tsed -e \"s/^\\tline-one-Z/ignored\\n&/\" raw >expect &&\n> -+\t\tcp expect raw &&\n> -+\t\tsed -e \"s/^\\tline-two-Z/ignored\\n&/\" raw >expect &&\n> ++\t\tcat > sedscript <<- 'EOF' &&\n> ++\t\t/^\tline-one-Z/i\\\\\n> ++\t\tignored\n> ++\t\t/^\tline-two-Z/i\\\\\n> ++\t\tignored\n> ++\t\tEOF\n> ++\t\tsed -f sedscript raw >expect &&\n> +\n> +\t\tgit -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&\n> +\t\ttest_cmp expect actual\n> -+\t'\n> ++\t\"\n> +done\n> +\n> # For ignored revs that added 'unblamable' lines and more recent commits changed\n> \n> ---\n> Documentation/blame-options.adoc | 3 ++-\n> Documentation/git-blame.adoc | 9 ++++----\n> builtin/blame.c | 15 +++++++++++++\n> t/t8013-blame-ignore-revs.sh | 38 ++++++++++++++++++++++++++++++++\n> 4 files changed, 60 insertions(+), 5 deletions(-)\n> \n> diff --git a/Documentation/blame-options.adoc b/Documentation/blame-options.adoc\n> index aa77406d4e..19ea187238 100644\n> --- a/Documentation/blame-options.adoc\n> +++ b/Documentation/blame-options.adoc\n> @@ -125,7 +125,8 @@ take effect.\n> \tanother commit will be marked with a `?` in the blame output. If the\n> \t`blame.markUnblamableLines` config option is set, then those lines touched\n> \tby an ignored commit that we could not attribute to another revision are\n> -\tmarked with a '*'.\n> +\tmarked with a '*'. In the porcelain modes, we print 'ignored' and\n> +\t'unblamable' on a newline respectively.\n> \n> --ignore-revs-file ::\n> \tIgnore revisions listed in `file`, which must be in the same format as an\n> diff --git a/Documentation/git-blame.adoc b/Documentation/git-blame.adoc\n> index f75ed44790..e438d28625 100644\n> --- a/Documentation/git-blame.adoc\n> +++ b/Documentation/git-blame.adoc\n> @@ -135,10 +135,11 @@ header elements later.\n> The porcelain format generally suppresses commit information that has\n> already been seen. For example, two lines that are blamed to the same\n> commit will both be shown, but the details for that commit will be shown\n> -only once. This is more efficient, but may require more state be kept by\n> -the reader. The `--line-porcelain` option can be used to output full\n> -commit information for each line, allowing simpler (but less efficient)\n> -usage like:\n> +only once. Information which is specific to individual lines will not be\n> +grouped together, like revs to be marked 'ignored' or 'unblamable'. This\n> +is more efficient, but may require more state be kept by the reader. The\n> +`--line-porcelain` option can be used to output full commit information\n> +for each line, allowing simpler (but less efficient) usage like:\n> \n> \t# count the number of lines attributed to each author\n> \tgit blame --line-porcelain file |\n> diff --git a/builtin/blame.c b/builtin/blame.c\n> index c470654c7e..9436f70aec 100644\n> --- a/builtin/blame.c\n> +++ b/builtin/blame.c\n> @@ -351,6 +351,19 @@ static void emit_porcelain_details(struct blame_origin *suspect, int repeat)\n> \t\twrite_filename_info(suspect);\n> }\n> \n> +/*\n> + * Information which needs to be printed per-line goes here. Any\n> + * information which can be clubbed on a commit/file level, should\n> + * be printed via 'emit_one_suspect_detail()'.\n> + */\n> +static void emit_porcelain_per_line_details(struct blame_entry *ent)\n> +{\n> +\tif (mark_unblamable_lines && ent->unblamable)\n> +\t\tputs(\"unblamable\");\n> +\tif (mark_ignored_lines && ent->ignored)\n> +\t\tputs(\"ignored\");\n> +}\n> +\n> static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,\n> \t\t\t int opt)\n> {\n> @@ -367,6 +380,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,\n> \t ent->lno + 1,\n> \t ent->num_lines);\n> \temit_porcelain_details(suspect, repeat);\n> +\temit_porcelain_per_line_details(ent);\n> \n> \tcp = blame_nth_line(sb, ent->lno);\n> \tfor (cnt = 0; cnt < ent->num_lines; cnt++) {\n> @@ -377,6 +391,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,\n> \t\t\t ent->lno + 1 + cnt);\n> \t\t\tif (repeat)\n> \t\t\t\temit_porcelain_details(suspect, 1);\n> +\t\t\temit_porcelain_per_line_details(ent);\n> \t\t}\n> \t\tputchar('\\t');\n> \t\tdo {\n> diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh\n> index 370b768149..cace00ae8d 100755\n> --- a/t/t8013-blame-ignore-revs.sh\n> +++ b/t/t8013-blame-ignore-revs.sh\n> @@ -158,6 +158,25 @@ test_expect_success mark_unblamable_lines '\n> \ttest_cmp expect actual\n> '\n> \n> +for opt in --porcelain --line-porcelain\n> +do\n> +\ttest_expect_success \"mark_unblamable_lines with $opt\" \"\n> +\t\tsha=$(git rev-parse Y) &&\n> +\n> +\t\tgit -c blame.markUnblamableLines=false blame $opt --ignore-rev Y file >raw &&\n> +\t\tcat > sedscript <<- 'EOF' &&\n> +\t\t/^\ty3/i\\\\\n> +\t\tunblamable\n> +\t\t/^\ty4/i\\\\\n> +\t\tunblamable\n> +\t\tEOF\n> +\t\tsed -f sedscript raw >expect &&\n> +\n> +\t\tgit -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&\n> +\t\ttest_cmp expect actual\n> +\t\"\n> +done\n> +\n> # Commit Z will touch the first two lines. Y touched all four.\n> # \tA--B--X--Y--Z\n> # The blame output when ignoring Z should be:\n> @@ -191,6 +210,25 @@ test_expect_success mark_ignored_lines '\n> \t! test_cmp expect actual\n> '\n> \n> +for opt in --porcelain --line-porcelain\n> +do\n> +\ttest_expect_success \"mark_ignored_lines with $opt\" \"\n> +\t\tsha=$(git rev-parse Y) &&\n> +\n> +\t\tgit -c blame.markIgnoredLines=false blame $opt --ignore-rev Z file >raw &&\n> +\t\tcat > sedscript <<- 'EOF' &&\n> +\t\t/^\tline-one-Z/i\\\\\n> +\t\tignored\n> +\t\t/^\tline-two-Z/i\\\\\n> +\t\tignored\n> +\t\tEOF\n> +\t\tsed -f sedscript raw >expect &&\n> +\n> +\t\tgit -c blame.markIgnoredLines=true blame $opt --ignore-rev Z file >actual &&\n> +\t\ttest_cmp expect actual\n> +\t\"\n> +done\n> +\n> # For ignored revs that added 'unblamable' lines and more recent commits changed\n> # the blamable lines, mark the unblamable lines with a\n> # '*'\n\n"},{"id":"515814","messageId":"xmqqv7rfze4r.fsf@gitster.g","threadId":"63172","inReplyTo":"2fbecc76-c3d9-4a8e-9326-5b83db1ceb26@gmail.com","subject":"Re: [PATCH v5] blame: print unblamable and ignored commits in porcelain mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-04-08T00:32:52Z","receivedAt":"2025-04-08T00:32:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> Hi Karthik\n>\n> On 03/04/2025 17:03, Karthik Nayak wrote:\n>> The 'git-blame(1)' command allows users to ignore specific revisions via\n>> the '--ignore-rev ' and '--ignore-revs-file ' flags. These\n>> flags are often combined with the 'blame.markIgnoredLines' and\n>> 'blame.markUnblamableLines' config options. These config options prefix\n>> ignored and unblamable lines with a '?' and '*', respectively.\n>> However, this option was never extended to the porcelain mode of\n>> 'git-blame(1)'. Since the documentation does not indicate this\n>> exclusion, it is a bug.\n>> Fix this by printing 'ignored' and 'unblamable' respectively for the\n>> options when using the porcelain modes.\n>\n> This looks good to me\n\nThanks, both of you. Will replace and queue.\nLet me mark the topic for 'next'.\n"}]}