Subject: Re: [PATCH v2] diff: update conflict handling for whitespace to issue a warning

From: Junio C Hamano <[email-masked]>
Date: Tue, 19 Nov 2024 09:36:34 +0900

Usman Akinyemi <[email-masked]> writes:

> On Fri, Nov 15, 2024 at 12:11 AM Junio C Hamano <[email-masked]> wrote:
>> ...
>> If we were to fix anything, it is to make sure that we die() before
>> producing a single line of output.  If you have a change to a path
>> whose "type" is without such a misconfigured attribute, that sorts
>> lexicographically earlier than another path with a change, with a
>> conflicting whitespace attribute, I suspect that with the way the
>> code is structured currently, we show the diff for the first path,
>> before realizing that the second path has an issue and then die.
>>
>> If we fix it, and then make sure that the die() message shows
>> clearly what attribute setting we did not like, that would be
>> sufficient to help users to locate the problem, fix it, and quickly
>> move on, no?
>
> Thanks for the review. From what I understand from your comment,
> we should leave it the way it was which was die right ?

Correct.  I do not think replacing die() with warning() without
doing anything else makes sense.  Making sure that we detect the
breakage before going half-way while producing a patch that touches
many paths may improve the end-user experience, though.

Thanks.