Re: [PATCH 2/2] Define a few built-in attribute rules.

Junio C Hamano [off-list ref] writes:

Linus Torvalds [off-list ref] writes:

quoted

Why would 

	* diff crlf

be "obviously sane"?

In fact, I'd call it obviously insane.

We do *not* want to default crlf to all files. We want the default to be 
"automatic crlf depending on content". 

You do not have to worry.

That's how "crlf" is defined.  Paths you explicitly say !crlf
will _not_ go through the existing core.autocrlf mechanism.

"* crlf" just says, by default everybody is subject to core.autocrlf,
and on sane platforms, core.autocrlf is by default off, hence you will
not get LF <-> CRLF applied.

Having said that, if we really wanted to, we could introduce a
way to explicitly say "Even if the contents do not look like
text, apply line ending conversion, always", by redefining the
meaning of 'crlf' attribute.

But I do not know if that makes much sense.  Being able to turn
_off_ would be a good thing because a particular file that looks
like CRLF terminated text might not be text.  But the other way
around?  IOW, I do not think of a case where a file that does
not even look like a text wants CRLF conversion.

---

diff --git a/convert.c b/convert.c
index 20c744a..f9e5d63 100644
--- a/convert.c
+++ b/convert.c

@@ -191,7 +191,7 @@ static void setup_crlf_check(struct git_attr_check *check)
 	check->attr = attr_crlf;
 }
 
-static int git_path_is_binary(const char *path)
+static int git_path_check_crlf(const char *path)
 {
 	struct git_attr_check attr_crlf_check;
 

@@ -202,20 +202,31 @@ static int git_path_is_binary(const char *path)
 	 * disable autocrlf only when crlf attribute is explicitly
 	 * unset.
 	 */
-	return (!git_checkattr(path, 1, &attr_crlf_check) &&
-		(0 == attr_crlf_check.isset));
+	if (!git_checkattr(path, 1, &attr_crlf_check))
+		return -1;
+	return attr_crlf_check.isset;
 }
 
 int convert_to_git(const char *path, char **bufp, unsigned long *sizep)
 {
-	if (git_path_is_binary(path))
+	switch (git_path_check_crlf(path)) {
+	case 0:
 		return 0;
-	return autocrlf_to_git(path, bufp, sizep);
+	case 1:
+		return forcecrlf_to_git(path, bufp, sizep);
+	default:
+		return autocrlf_to_git(path, bufp, sizep);
+	}
 }
 
 int convert_to_working_tree(const char *path, char **bufp, unsigned long *sizep)
 {
-	if (git_path_is_binary(path))
+	switch (git_path_check_crlf(path)) {
+	case 0:
 		return 0;
-	return autocrlf_to_working_tree(path, bufp, sizep);
+	case 1:
+		return forcecrlf_to_working_tree(path, bufp, sizep);
+	default:
+		return autocrlf_to_working_tree(path, bufp, sizep);
+	}
 }

Hi,

On Sat, 14 Apr 2007, Junio C Hamano wrote:

[...] if we really wanted to, we could introduce a way to explicitly say 
"Even if the contents do not look like text, apply line ending 
conversion, always", by redefining the meaning of 'crlf' attribute.

I think it might make more sense to introduce a way to say "do not even 
check; I _know_ that I want crlf on these".

It might show performance improvements on large repos, for example.

Ciao,
Dscho

Johannes Schindelin [off-list ref] writes:

On Sat, 14 Apr 2007, Junio C Hamano wrote:

quoted

[...] if we really wanted to, we could introduce a way to explicitly say 
"Even if the contents do not look like text, apply line ending 
conversion, always", by redefining the meaning of 'crlf' attribute.

I think it might make more sense to introduce a way to say "do not even 
check; I _know_ that I want crlf on these".

You said I wanted to say in the message in clearer words.  We
are in agreement.

Also the comments to the two patches in this series you sent are
valid; I reworked them before merging to 'next' and pushing them
out yesterday, but I think the Makefile thing still remains.
Will fix up.