Hi, On Wed, 23 Jul 2008, Steffen Prohaska wrote: > On Jul 23, 2008, at 3:31 AM, Johannes Schindelin wrote: > > >When a file's crlf attribute is explicitely set, it does not make sense > >to ignore it when adding the file, just because the config variable > >core.autocrlf has not been set. > > Your patch is not about core.autocrlf unset, but about > core.autocrlf=false. No, no, no! It is about autocrlf _unset_, i.e. when the user was _not_ explicit! > >The alternative would be risking to get CR/LF files into the repository > >just because one user forgot to set core.autocrlf. > > Git could help the user *and* the maintainer of the repository if we > chose core.autocrlf=input as the default on Unix. NO! The machinery behind core.autocrlf _does_ slow down Git, even if only by a few percent. But with patch queues as maintainers of busy projects encounter frequently, this makes a difference. So no, I am _totally_ opposed to punishing Unix users for Windows' faults. Totally. > >diff --git a/convert.c b/convert.c > >index 78efed8..d038d2f 100644 > >--- a/convert.c > >+++ b/convert.c > >@@ -126,7 +126,7 @@ static int crlf_to_git(const char *path, const char *src, > >size_t len, > > struct text_stat stats; > > char *dst; > > > >- if ((action == CRLF_BINARY) || !auto_crlf || !len) > >+ if ((action == CRLF_BINARY) || (!auto_crlf && action < 0) || !len) > > I think we should strictly follow the documentation, so this should read > > + if ((action == CRLF_BINARY) || (!auto_crlf && action != CRLF_INPUT) || > !len) NO! Your version would break action == CRLF_TEXT. > >diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh > >index 1be7446..0bb3e6f 100755 > >--- a/t/t0020-crlf.sh > >+++ b/t/t0020-crlf.sh > >@@ -436,4 +436,14 @@ test_expect_success 'invalid .gitattributes (must not > >crash)' ' > > > >' > > > >+test_expect_success 'attribute crlf is heeded even without core.autocrlf' ' > >+ > >+ echo "allcrlf crlf=input" > .gitattributes && > >+ git config --unset core.autocrlf && > > You should set core.autocrlf explicitly to false: > > git config core.autocrlf false But this is _not_ what I want! I want it to be _unset_. Yes, at the moment, auto_crlf is initialized to 0. But I do not _care_ what it is set. I want this test to succeed when the user did _not_ set autocrlf. > Otherwise the test would pick up the user's global default. It will not. We went to great pains not to pick up global defaults in the test suite. > ... and we should verify that we only treat crlf=input specially, but not > "crlf". Umm... Why? Why should a file marked "crlf=true" be allowed to be checked in _with_ CRs? Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
