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

[Usman Akinyemi <usmanakinyemi202@gmail.com>]

On Thu, Nov 14, 2024 at 5:06 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> On 14/11/2024 02:15, Junio C Hamano wrote:
> > "Usman Akinyemi via GitGitGadget" <gitgitgadget@gmail.com> writes:
> >
> > [jc: As Phillip is blamed for suggesting this addition, I added him
> > to the recipient of this message.]
>
> Thanks
Hi Philip and Junio,

>
> >> From: Usman Akinyemi <usmanakinyemi202@gmail.com>
> >>
> >> Modify the conflict resolution between tab-in-indent and
> >> indent-with-non-tab to issue a warning instead of terminating
> >> the operation with `die()`. Update the `git diff --check` test to
> >> capture and verify the warning message output.
>
> Usman - when you're writing a commit message it is important to explain
> the reason for making the changes contained in the patch so others can
> understand why it is a good idea. In this case the idea is to avoid
> breaking "git diff" for everyone who clones a repository containing a
> .gitattributes file with bad whitespace attributes [1]. As I mentioned
> in [2] I think we only want to change the behavior when parsing
> whitespace attributes - we still want the other callers of
> parse_whitespace_rule() to die() so the user can fix their config or
> commandline. We can do that by adding a boolean parameter called
> "gentle" that determines whether we call warning() or die().

I am very sorry for the confusion. I will take this into consideration
next time and always give more explanation
in commit messages.

I will make the necessary changes.

Thank you very much.
Usman.

>
> Best Wishes
>
> Phillip
>
> [1]
> https://lore.kernel.org/git/e4a70501-af2d-450a-a232-4c7952196a74@gmail.com
> [2]
> https://lore.kernel.org/git/3c081d3c-3f6f-45ff-b254-09f1cd6b7de5@gmail.com
>
> >> Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
> >> Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
> >> ---
> >
> > If the settings requires an impossible way to use whitespaces, the
> > settings is buggy, and it generally would be better to correct the
> > setting before moving on.
> >
> > I am curious to know in what situations this new behaviour can be
> > seen as an improvement.  It may allow you to go on _without_ fixing
> > such a broken setting, but how would it help the end user?  If the
> > user set both of these mutually-incompatible options A and B by
> > mistake, but what the user really wanted to check for was A, picking
> > just one of A or B arbitrarily and disabling it would not help, and
> > disabling both would not help, either.  But wouldn't the real source
> > of the problem be that we are trying to demote die() to force the
> > user to correct contradictiong setting into warning()?
> >
> > Thanks.
>