Re: [PATCH] Respect crlf attribute in "git add" even if core.autocrlf has not been set

Hi,

On Wed, 23 Jul 2008, Steffen Prohaska wrote:

On Jul 23, 2008, at 3:31 AM, Johannes Schindelin wrote:

quoted

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!

quoted

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.

quoted

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.

quoted

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