On 14/11/2024 02:15, Junio C Hamano wrote:
"Usman Akinyemi via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
[jc: As Phillip is blamed for suggesting this addition, I added him
to the recipient of this message.]
Thanks
From: Usman Akinyemi <usmanakinyemi202@xxxxxxxxx>
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().
Best Wishes
Phillip
[1]
https://lore.kernel.org/git/e4a70501-af2d-450a-a232-4c7952196a74@xxxxxxxxx
[2]
https://lore.kernel.org/git/3c081d3c-3f6f-45ff-b254-09f1cd6b7de5@xxxxxxxxx
Suggested-by: Phillip Wood <phillip.wood123@xxxxxxxxx>
Signed-off-by: Usman Akinyemi <usmanakinyemi202@xxxxxxxxx>
---
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.
