Hi,
Thanks for writing git. It's a darn useful tool. But one thing:
One of the pre-commit hooks detects trailing whitespace:
if (/\s$/) {
bad_line("trailing whitespace", $_);
}
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.
But the error message "trailing whitespace" doesn't clearly tell me what's
wrong.
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?
2. In addition, could you please enable the core.autocrlf and core.safecrlf
options by default in the next version of Git?
P.S. I hereby release the contents of this e-mail message to the public
domain.
Thanks in advance,
--
Jason Spiro: software/web developer, packager, trainer, IT consultant.
I support Linux, UNIX, Windows, and more. Contact me to discuss your needs.
+1 (416) 992-3445 / www.jspiro.com
On Mon, Feb 16, 2009 at 02:45:43AM +0000, Jason Spiro wrote:
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
Jeff King <peff <at> peff.net> writes:
On Mon, Feb 16, 2009 at 02:45:43AM +0000, Jason Spiro wrote:
quoted
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).
Sorry. Will do.
...
quoted
2. In addition, could you please enable the core.autocrlf and
core.safecrlf
quoted
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.
Are you saying that only my platform's packager can decide what options are
enabled by default, and that you upstream folks have no influence at all? :)
On Mon, Feb 16, 2009 at 03:29:23AM +0000, Jason Spiro wrote:
quoted
quoted
2. In addition, could you please enable the core.autocrlf and
core.safecrlf
quoted
quoted
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.
Are you saying that only my platform's packager can decide what options are
enabled by default, and that you upstream folks have no influence at all? :)
Not necessarily. But I don't think we want core.autocrlf on by default
for all platforms. So the decision needs to be made on a platform by
platform basis. The cleanest way to do that (in my opinion) is through a
system-level configuration file. But git built from src does not
distribute such a configuration file at all; that seems to be in the
scope of package distributors.
-Peff