Re: Improving CRLF error message; also, enabling autocrlf and safecrlf by default
On Mon, Feb 16, 2009 at 02:45:43AM +0000, Jason Spiro wrote:
Show 5 quoted lines
> One of the pre-commit hooks detects trailing whitespace:
>
> if (/\s$/) {
> bad_line("trailing whitespace", $_);
> }Not since 03e2b63 (Update sample pre-commit hook to use "diff --check", 2008-06-26), when that line was removed.
I'm happy you want to improve git; but please, if you want to report problems, check what the status is in a more recent version (or at least tell us your version, which can help).
> Unfortunately, when I try to check in a file with DOS (CR+LF) line
> endings, this hook triggers on every line. This happens on Cygwin. I
> haven't checked, but I bet it happens on other platforms as well, as
> long as this hook runs.
Yes, I believe carriage returns are considered trailing whitespace. I think (and I am not 100% sure here, because I have the good fortune not to have to deal with line-ending conversions on any of my platforms) that the general philosophy is that the "canonical" form in the repository should be LF-only, and that conversions can optionally make the worktree version CRLF (or whatever your platform desires it). But it's important that the canonical version be the same across platforms so that the blob sha-1's (and therefore the tree and commit sha-1's) all match.
IOW, setting up core.autocrlf properly should make this go away.
> But the error message "trailing whitespace" doesn't clearly tell me
> what's wrong.
Modern versions use "diff --check", which should look like this (on my LF-only box, at least):
$ mkdir repo && cd repo && git init
$ touch file && git add file && git commit -m one
$ printf 'foo\r\n' >file
$ git diff --check
file:1: trailing whitespace.
+foo^M
and if you use "git diff --color --check", the problem is highlighted.
> 1. Could you please modify Git so that, when such a problem happens,
> it instead prints an message saying that the file has CR+LF line
> endings, and that Git does not allow this?
It might be worth splitting the trailing whitespace detection into "spaces and tabs at the end" and "CRLF", and providing different messages (though it is hopefully also obvious with the new output that it is a CRLF issue).
> 2. In addition, could you please enable the core.autocrlf and core.safecrlf
> options by default in the next version of Git?
I think that is up to your platform packaging, I think. I think msysgit is shipping with core.autocrlf on by default these days. But again, I don't know very much about that area.
-Peff
