Re: [PATCH v2] merge-ll: expose revision names to custom drivers

From: Antonin Delpeuch <antonin@delpeuch.eu>
Date: Jan 18, 2024, 20:56 UTC
Message-ID: <00fd20e2-73f5-42ca-b9a8-1ee227150eff@delpeuch.eu>
In-Reply-To: <xmqq1qaeqtw7.fsf@gitster.g>

Hi Junio,

Thanks a lot for your review! (and many apologies for the double sending as HTML…)

On 18/01/2024 21:16, Junio C Hamano wrote:

> Or you could fix %O's description "while at it" and use the right
> term from the get-go for %S.

Agreed, I'll do that, it also feels more fitting to me.

> I see some funny indentation for "S" here.

Oops, sorry about that.

Show 13 quoted lines

>> @@ -173,7 +179,7 @@ test_expect_success !WINDOWS 'custom merge driver that is killed with a signal'
>>   
>>   	git reset --hard anchor &&
>>   	git config --replace-all \
>> -	merge.custom.driver "./custom-merge %O %A %B 0 %P" &&
>> +	merge.custom.driver "./custom-merge %O %A %B 0 %P %S %X %Y" &&
>>   	git config --replace-all \
>>   	merge.custom.name "custom merge driver for testing" &&
> ;-)
>
> This one is expected to die and not produce meaningful output;
> I was wondering why this does not need to make corresponding changes
> to the expected output pattern like the earlier tests.

As far as I can tell, this test does not compare the result of the merge to the expected merge driver output, because that output is expected to be disregarded by git given that the merge driver died (see the last two lines). So it seems normal to me that we don't need to adapt the expected output: the repository files are still left unchanged.

I'll submit a new version of the patch with the two changes above.

Antonin