threads / patch / 19865

patch.gitattributes: CR at the end of the line is an error

Subject: [PATCH] .gitattributes: CR at the end of the line is an error

## tl;dr

3 messages between Jun 19, 2009 and Jun 21, 2009. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Nanako Shiraishi· Jun 19, 2009, 10:42 UTC · lore

When a CR is accidentally added at the end of a C source file in the git project tree, "git diff --check" doesn't detect it as an error.

    $ echo abQ | tr Q '\015' >>fast-import.c
    $ git diff --check

I think this is because the "whitespace" attribute is set to *.[ch] files without specifying what kind of errors are caught. It makes git "notice all types of errors" (as described in the documentation), but I think it is incorrectly setting cr-at-eol, too, and hides this error.

Signed-off-by: Nanako Shiraishi <nanako3@lavabit.com>
---

Show changes to .gitattributes +1 −1

diff --git a/.gitattributes b/.gitattributes
index 6b9c715..bb03350 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -1,2 +1,2 @@
 * whitespace=!indent,trail,space
-*.[ch] whitespace
+*.[ch] whitespace=indent,trail,space

-- 
1.6.2.GIT

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

Junio C Hamano· Jun 21, 2009, 09:31 UTC · re: Nanako Shiraishi · lore

Re: [PATCH] .gitattributes: CR at the end of the line is an error

Nanako Shiraishi <nanako3@lavabit.com> writes:

Show 23 quoted lines

> When a CR is accidentally added at the end of a C source file in the git
> project tree, "git diff --check" doesn't detect it as an error.
>
>     $ echo abQ | tr Q '\015' >>fast-import.c
>     $ git diff --check
>
> I think this is because the "whitespace" attribute is set to *.[ch] files
> without specifying what kind of errors are caught. It makes git "notice
> all types of errors" (as described in the documentation), but I think it
> is incorrectly setting cr-at-eol, too, and hides this error.
>
> Signed-off-by: Nanako Shiraishi <nanako3@lavabit.com>
> ---
>
> diff --git a/.gitattributes b/.gitattributes
> index 6b9c715..bb03350 100644
> --- a/.gitattributes
> +++ b/.gitattributes
> @@ -1,2 +1,2 @@
>  * whitespace=!indent,trail,space
> -*.[ch] whitespace
> +*.[ch] whitespace=indent,trail,space
>

I like the result of applying this patch to my tree.

A "whitespace" attribute that is Set, which is what the original has, is defined to "notice all types of errors known to git", it is a poor way to define the project policy, which was what 14f9e12 (Define the project whitespace policy, 2008-02-10) tried to do. It means the policy will silently change when newer git learns to detect more types of whitespace errors.

And it never meant to allow trailing carriage-returns. I think the implementation of whitespace attribute handling is broken.

Junio C Hamano· Jun 21, 2009, 09:35 UTC · re: Junio C Hamano · lore

[PATCH] attribute: whitespace set to true detects all errors known to git

That is what the documentation says, but the code pretends as if all the known whitespace error tokens were given.

Among the whitespace error tokens, there is one kind that loosens the rule when set: cr-at-eol. Which means that whitespace error token that is set to true ignores a newly introduced CR at the end, which is inconsistent with the documentation.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * I wish cr-at-eol were no-cr-at-eol instead, so that we didn't have to
   do this, but it is too late for that.  Oh well.

 ws.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

Show changes to ws.c +7 −5

diff --git a/ws.c b/ws.c
index b1efcd9..819c797 100644
--- a/ws.c
+++ b/ws.c
@@ -10,11 +10,12 @@
 static struct whitespace_rule {
 	const char *rule_name;
 	unsigned rule_bits;
+	unsigned loosens_error;
 } whitespace_rule_names[] = {
-	{ "trailing-space", WS_TRAILING_SPACE },
-	{ "space-before-tab", WS_SPACE_BEFORE_TAB },
-	{ "indent-with-non-tab", WS_INDENT_WITH_NON_TAB },
-	{ "cr-at-eol", WS_CR_AT_EOL },
+	{ "trailing-space", WS_TRAILING_SPACE, 0 },
+	{ "space-before-tab", WS_SPACE_BEFORE_TAB, 0 },
+	{ "indent-with-non-tab", WS_INDENT_WITH_NON_TAB, 0 },
+	{ "cr-at-eol", WS_CR_AT_EOL, 1 },
 };
 
 unsigned parse_whitespace_rule(const char *string)
@@ -79,7 +80,8 @@ unsigned whitespace_rule(const char *pathname)
 			unsigned all_rule = 0;
 			int i;
 			for (i = 0; i < ARRAY_SIZE(whitespace_rule_names); i++)
-				all_rule |= whitespace_rule_names[i].rule_bits;
+				if (!whitespace_rule_names[i].loosens_error)
+					all_rule |= whitespace_rule_names[i].rule_bits;
 			return all_rule;
 		} else if (ATTR_FALSE(value)) {
 			/* false (-whitespace) */