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

From: Phillip Wood <[email-masked]>
Date: Wed, 2 Apr 2025 14:07:31 +0100

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

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

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

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

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

Best Wishes

Phillip

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