On 15/11/2024 00:11, Junio C Hamano 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?
Arguably yes, but that's not the approach we take when the attributes
file is too large, a line in the file is is too long or the file
contains a negative filename pattern. For those cases we print a warning
and continue. The recently merged e36f009e69b (merge: replace atoi()
with strtol_i() for marker size validation, 2024-10-24) followed suit
and warns rather than dies for an invalid marker size. It would be nice
to be consistent in the way we treat invalid attributes. Consistently
dying and telling the user how to fix the problem would be a reasonable
approach on the client side but I wonder if it could cause problems for
forges running "git diff" and "git merge-tree" on a server though.
[...]
If we were to fix anything, it is to make sure that we die() before
producing a single line of output.
That would certainly be a good idea
Best Wishes
Phillip
