# [PATCH] .gitattributes: CR at the end of the line is an error 3 messages from 2009-06-19 to 2009-06-21. Participants: Nanako Shiraishi, Junio C Hamano. Thread: https://gitlist.dev/t/19865 ## Nanako Shiraishi, 2009-06-19 10:42 Subject: [PATCH] .gitattributes: CR at the end of the line is an error Message-ID: <20090619194253.6117@nanako3.lavabit.com> URL: https://gitlist.dev/e/20090619194253.6117%40nanako3.lavabit.com ``` 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 --- 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, 2009-06-21 09:31 Subject: Re: [PATCH] .gitattributes: CR at the end of the line is an error Message-ID: <7vvdmqrl06.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vvdmqrl06.fsf%40alter.siamese.dyndns.org In-Reply-To: <20090619194253.6117@nanako3.lavabit.com> ``` Nanako Shiraishi 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 > --- > > 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, 2009-06-21 09:35 Subject: [PATCH] attribute: whitespace set to true detects all errors known to git Message-ID: <7vr5xerktl.fsf_-_@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vr5xerktl.fsf_-_%40alter.siamese.dyndns.org In-Reply-To: <7vvdmqrl06.fsf@alter.siamese.dyndns.org> ``` 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 --- * 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) */ ```