On Fri, Nov 15, 2024 at 12:11 AM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Phillip Wood <phillip.wood123@xxxxxxxxx> writes: > > > 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]. > > Hmph, it would certainly be a problem, but the right solution is not > to butcher Git, but to make it easier for the participants of such a > project to know what is broken *and* what needs to be updated, to let > them move forward, no? > > > 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 doubt that such a complexity is warranted. > > It depends on the size of diff you are showing, but if it is large, > then giving a small warning that gets buried in the large diff is a > conter-productive way to encourage users to correct such broken > setting. If it is small, then the damage may not be too bad, but > still, we are showing what the user did not really request. > > 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? Hi Junio, Thanks for the review. From what I understand from your comment, we should leave it the way it was which was die right ? Thanks. Usman. > > Thanks.
