{"thread":{"id":"19865","subject":"[PATCH] .gitattributes: CR at the end of the line is an error","startedAt":"2009-06-19T10:42:53Z","lastAt":"2009-06-21T09:35:18Z","messageCount":3,"participants":["Nanako Shiraishi","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"116623","messageId":"20090619194253.6117@nanako3.lavabit.com","threadId":"19865","inReplyTo":null,"subject":"[PATCH] .gitattributes: CR at the end of the line is an error","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-06-19T10:42:53Z","receivedAt":"2009-06-19T10:42:53Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"When a CR is accidentally added at the end of a C source file in the git\nproject tree, \"git diff --check\" doesn't detect it as an error.\n\n $ echo abQ | tr Q '\\015' >>fast-import.c\n $ git diff --check\n\nI think this is because the \"whitespace\" attribute is set to *.[ch] files\nwithout specifying what kind of errors are caught. It makes git \"notice\nall types of errors\" (as described in the documentation), but I think it\nis incorrectly setting cr-at-eol, too, and hides this error.\n\nSigned-off-by: Nanako Shiraishi \n---\n\ndiff --git a/.gitattributes b/.gitattributes\nindex 6b9c715..bb03350 100644\n--- a/.gitattributes\n+++ b/.gitattributes\n@@ -1,2 +1,2 @@\n * whitespace=!indent,trail,space\n-*.[ch] whitespace\n+*.[ch] whitespace=indent,trail,space\n\n-- \n1.6.2.GIT\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"116704","messageId":"7vvdmqrl06.fsf@alter.siamese.dyndns.org","threadId":"19865","inReplyTo":"20090619194253.6117@nanako3.lavabit.com","subject":"Re: [PATCH] .gitattributes: CR at the end of the line is an error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-21T09:31:21Z","receivedAt":"2009-06-21T09:31:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nanako Shiraishi writes:\n\n> When a CR is accidentally added at the end of a C source file in the git\n> project tree, \"git diff --check\" doesn't detect it as an error.\n>\n> $ echo abQ | tr Q '\\015' >>fast-import.c\n> $ git diff --check\n>\n> I think this is because the \"whitespace\" attribute is set to *.[ch] files\n> without specifying what kind of errors are caught. It makes git \"notice\n> all types of errors\" (as described in the documentation), but I think it\n> is incorrectly setting cr-at-eol, too, and hides this error.\n>\n> Signed-off-by: Nanako Shiraishi \n> ---\n>\n> diff --git a/.gitattributes b/.gitattributes\n> index 6b9c715..bb03350 100644\n> --- a/.gitattributes\n> +++ b/.gitattributes\n> @@ -1,2 +1,2 @@\n> * whitespace=!indent,trail,space\n> -*.[ch] whitespace\n> +*.[ch] whitespace=indent,trail,space\n>\n\nI like the result of applying this patch to my tree.\n\nA \"whitespace\" attribute that is Set, which is what the original has, is\ndefined to \"notice all types of errors known to git\", it is a poor way to\ndefine the project policy, which was what 14f9e12 (Define the project\nwhitespace policy, 2008-02-10) tried to do. It means the policy will\nsilently change when newer git learns to detect more types of whitespace\nerrors.\n\nAnd it never meant to allow trailing carriage-returns. I think the\nimplementation of whitespace attribute handling is broken.\n"},{"id":"116706","messageId":"7vr5xerktl.fsf_-_@alter.siamese.dyndns.org","threadId":"19865","inReplyTo":"7vvdmqrl06.fsf@alter.siamese.dyndns.org","subject":"[PATCH] attribute: whitespace set to true detects all errors known to git","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-21T09:35:18Z","receivedAt":"2009-06-21T09:35:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"That is what the documentation says, but the code pretends as if all the\nknown whitespace error tokens were given.\n\nAmong the whitespace error tokens, there is one kind that loosens the rule\nwhen set: cr-at-eol. Which means that whitespace error token that is set\nto true ignores a newly introduced CR at the end, which is inconsistent\nwith the documentation.\n\nSigned-off-by: Junio C Hamano \n---\n\n * I wish cr-at-eol were no-cr-at-eol instead, so that we didn't have to\n do this, but it is too late for that. Oh well.\n\n ws.c | 12 +++++++-----\n 1 files changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/ws.c b/ws.c\nindex b1efcd9..819c797 100644\n--- a/ws.c\n+++ b/ws.c\n@@ -10,11 +10,12 @@\n static struct whitespace_rule {\n \tconst char *rule_name;\n \tunsigned rule_bits;\n+\tunsigned loosens_error;\n } whitespace_rule_names[] = {\n-\t{ \"trailing-space\", WS_TRAILING_SPACE },\n-\t{ \"space-before-tab\", WS_SPACE_BEFORE_TAB },\n-\t{ \"indent-with-non-tab\", WS_INDENT_WITH_NON_TAB },\n-\t{ \"cr-at-eol\", WS_CR_AT_EOL },\n+\t{ \"trailing-space\", WS_TRAILING_SPACE, 0 },\n+\t{ \"space-before-tab\", WS_SPACE_BEFORE_TAB, 0 },\n+\t{ \"indent-with-non-tab\", WS_INDENT_WITH_NON_TAB, 0 },\n+\t{ \"cr-at-eol\", WS_CR_AT_EOL, 1 },\n };\n \n unsigned parse_whitespace_rule(const char *string)\n@@ -79,7 +80,8 @@ unsigned whitespace_rule(const char *pathname)\n \t\t\tunsigned all_rule = 0;\n \t\t\tint i;\n \t\t\tfor (i = 0; i < ARRAY_SIZE(whitespace_rule_names); i++)\n-\t\t\t\tall_rule |= whitespace_rule_names[i].rule_bits;\n+\t\t\t\tif (!whitespace_rule_names[i].loosens_error)\n+\t\t\t\t\tall_rule |= whitespace_rule_names[i].rule_bits;\n \t\t\treturn all_rule;\n \t\t} else if (ATTR_FALSE(value)) {\n \t\t\t/* false (-whitespace) */\n"}]}