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? Thanks.
