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

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

[Nanako Shiraishi]

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

-- 
1.6.2.GIT

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

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

[Junio C Hamano]

Nanako Shiraishi <nanako3@lavabit.com> writes:

> 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.

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

[Junio C Hamano]

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(-)

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) */