{"thread":{"id":"15202","subject":"question: how to ignore extral CR when diff dos format files with 'color=auto'","startedAt":"2008-08-26T05:32:23Z","lastAt":"2008-08-28T02:47:31Z","messageCount":6,"participants":["goooguo","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"88544","messageId":"1219728743111-783231.post@n2.nabble.com","threadId":"15202","inReplyTo":null,"subject":"question: how to ignore extral CR when diff dos format files with 'color=auto'","fromName":"goooguo","fromEmail":"erwangg@fortemedia.com.cn","sentAt":"2008-08-26T05:32:23Z","receivedAt":"2008-08-26T05:32:23Z","isPatch":false,"sender":{"key":"erwangg@fortemedia.com.cn","avatar":null},"body":"\nhi,\n\ngit-diff prints \"^M\" at each end of line for dos format files when\n'color=auto'.\n\nit's annoying!\n\nI want to know if I can configure git to make git-diff ignore '^M', and I\ndon't want to change file format to unix.\n\nthank you.\n-- \nView this message in context: http://n2.nabble.com/question%3A-how-to-ignore-extral-CR-when-diff-dos-format-files-with-%27color%3Dauto%27-tp783231p783231.html\nSent from the git mailing list archive at Nabble.com.\n"},{"id":"88683","messageId":"1219811272841-786034.post@n2.nabble.com","threadId":"15202","inReplyTo":"1219728743111-783231.post@n2.nabble.com","subject":"Re: question: how to ignore extral CR when diff dos format files with 'color=auto'","fromName":"goooguo","fromEmail":"erwangg@fortemedia.com.cn","sentAt":"2008-08-27T04:27:52Z","receivedAt":"2008-08-27T04:27:52Z","isPatch":false,"sender":{"key":"erwangg@fortemedia.com.cn","avatar":null},"body":"\nhere is my patch\n\nhttp://n2.nabble.com/file/n786034/0001-ingore-cr-at-eol-when-diff-with-color-auto.patch\n0001-ingore-cr-at-eol-when-diff-with-color-auto.patch \n\n-- \nView this message in context: http://n2.nabble.com/question%3A-how-to-ignore-extral-CR-when-diff-dos-format-files-with-%27color%3Dauto%27-tp783231p786034.html\nSent from the git mailing list archive at Nabble.com.\n"},{"id":"88686","messageId":"7v63pnt5us.fsf@gitster.siamese.dyndns.org","threadId":"15202","inReplyTo":"1219728743111-783231.post@n2.nabble.com","subject":"Re: question: how to ignore extral CR when diff dos format files with 'color=auto'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-27T04:54:51Z","receivedAt":"2008-08-27T04:54:51Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"goooguo <erwangg@fortemedia.com.cn> writes:\n\n> git-diff prints \"^M\" at each end of line for dos format files when\n> 'color=auto'.\n>\n> it's annoying!\n\n\"man git-config\" and learn about core.whitespace perhaps?\n"},{"id":"88845","messageId":"1219887555724-788498.post@n2.nabble.com","threadId":"15202","inReplyTo":"1219728743111-783231.post@n2.nabble.com","subject":"Re: question: how to ignore extral CR when diff dos format files with 'color=auto'","fromName":"goooguo","fromEmail":"erwangg@fortemedia.com.cn","sentAt":"2008-08-28T01:39:15Z","receivedAt":"2008-08-28T01:39:15Z","isPatch":false,"sender":{"key":"erwangg@fortemedia.com.cn","avatar":null},"body":"\n\ngit v1.6.0 still didn't work, neither v1.5.6.4. (however,v1.5.5 works on\nmingwin)\n\nI checked the source code of v1.6.0, and I found emit_line didn't check\nwhether there is a cr at eol.\n\nI have fixed it.\n\n>From dae20e25960c73bd7ccc0939fe096bb68a009fb5 Mon Sep 17 00:00:00 2001\nFrom: erwangg <erwangg@fortemedia.com.cn>\nDate: Wed, 27 Aug 2008 12:22:43 +0800\nSubject: [PATCH] ingore cr at eol when diff with color=auto\n\n---\n diff.c |    9 ++++++++-\n 1 files changed, 8 insertions(+), 1 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 18fa7a7..846a9af 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -517,9 +517,16 @@ static void emit_line(FILE *file, const char *set,\nconst char *reset, const char\n \tif (has_trailing_newline)\n \t\tlen--;\n \n+\tint has_trailing_return = (len > 0 && line[len-1] == '\\r');\n+\tif (has_trailing_return)\n+        len--;\n+\n \tfputs(set, file);\n \tfwrite(line, len, 1, file);\n \tfputs(reset, file);\n+\n+    if (has_trailing_return)\n+        fputc('\\r', file);\n \tif (has_trailing_newline)\n \t\tfputc('\\n', file);\n }\n@@ -535,7 +542,7 @@ static void emit_add_line(const char *reset, struct\nemit_callback *ecbdata, cons\n \t\t/* Emit just the prefix, then the rest. */\n \t\temit_line(ecbdata->file, set, reset, line, ecbdata->nparents);\n \t\tws_check_emit(line + ecbdata->nparents,\n-\t\t\t      len - ecbdata->nparents, ecbdata->ws_rule,\n+\t\t\t      len - ecbdata->nparents, ecbdata->ws_rule|WS_CR_AT_EOL,\n \t\t\t      ecbdata->file, set, reset, ws);\n \t}\n }\n-- \n1.6.0.GIT\n\n\n\n\n-- \nView this message in context: http://n2.nabble.com/question%3A-how-to-ignore-extral-CR-when-diff-dos-format-files-with-%27color%3Dauto%27-tp783231p788498.html\nSent from the git mailing list archive at Nabble.com.\n"},{"id":"88852","messageId":"7vbpzdkgy9.fsf@gitster.siamese.dyndns.org","threadId":"15202","inReplyTo":"1219887555724-788498.post@n2.nabble.com","subject":"Re: question: how to ignore extral CR when diff dos format files with 'color=auto'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-28T02:32:14Z","receivedAt":"2008-08-28T02:32:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"> From dae20e25960c73bd7ccc0939fe096bb68a009fb5 Mon Sep 17 00:00:00 2001\n> From: erwangg <erwangg@fortemedia.com.cn>\n> Date: Wed, 27 Aug 2008 12:22:43 +0800\n> Subject: [PATCH] ingore cr at eol when diff with color=auto\n>\n> ---\n\nThanks.  A few comments.\n\n> diff --git a/diff.c b/diff.c\n> index 18fa7a7..846a9af 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -517,9 +517,16 @@ static void emit_line(FILE *file, const char *set,\n> const char *reset, const char\n>  \tif (has_trailing_newline)\n>  \t\tlen--;\n>  \n> +\tint has_trailing_return = (len > 0 && line[len-1] == '\\r');\n\ndecl-after-statement.\n\n> +\tif (has_trailing_return)\n> +        len--;\n> +\n>  \tfputs(set, file);\n>  \tfwrite(line, len, 1, file);\n>  \tfputs(reset, file);\n> +\n> +    if (has_trailing_return)\n> +        fputc('\\r', file);\n>  \tif (has_trailing_newline)\n>  \t\tfputc('\\n', file);\n>  }\n\nWe indent with tab which jumps the cursor to next column that is multiple\nof 8.\n\nOtherwise this hunk is good.\n\n> @@ -535,7 +542,7 @@ static void emit_add_line(const char *reset, struct\n> emit_callback *ecbdata, cons\n>  \t\t/* Emit just the prefix, then the rest. */\n>  \t\temit_line(ecbdata->file, set, reset, line, ecbdata->nparents);\n>  \t\tws_check_emit(line + ecbdata->nparents,\n> -\t\t\t      len - ecbdata->nparents, ecbdata->ws_rule,\n> +\t\t\t      len - ecbdata->nparents, ecbdata->ws_rule|WS_CR_AT_EOL,\n>  \t\t\t      ecbdata->file, set, reset, ws);\n>  \t}\n>  }\n\nI do not think you want to force WS_CR_AT_EOL here.\n\nThat is a policy issue (not \"git policy\", but the policy of the contents\nthat is managed by git, in other words, your repository) that you should\ncontrol with core.whitespace (and if you want finer grain control, then\nwith .gitattributes).\n\nYour patch is whitespace damanged, and has style issues, and lacks any\ncomment to defend the change.  It does not have a Sign-off, either.  Even\nthough the intent is good, I cannot apply this as-is.  Please look at\nDocumentation/SubmittingPatches and emulate patches from other git\nregulars.\n\nYou say \"when color=auto\" in the patch title, but there is nothing that\nrestricts the effect of this change only to that case in your patch.  A\ncomment to defend the change should address things like that, and here is\nhow I would write it.\n\n    When the tracked contents have CRLF line endings, colored diff output\n    shows \"^M\" at the end of output lines, which is distracting, even\n    though the pager we use by default (\"less\") knows to hide them.\n\n    The problem is that \"less\" hides a carriage-return only at the end of\n    the line, immediately before a line feed.  The colored diff output\n    does not take this into account, and emits four element sequence for\n    each line:\n\n       - force this color;\n       - the line up to but not including the terminating line feed;\n       - reset color\n       - line feed.\n\n    By including the carriage return at the end of the line in the second\n    item, we are breaking the smart our pager has in order not to show\n    \"^M\".  This can be fixed by changing the sequence to:\n\n       - force this color;\n       - the line up to but not including the terminating end-of-line;\n       - reset color\n       - end-of-line.\n\n    where end-of-line is either a single linefeed or a CRLF pair.\n    When the output is not colored, \"force this color\" and \"reset color\"\n    sequences are both empty, so we won't have this problem with or\n    without this patch.    \n\nIf we drop the hunk to emit_add_line() that forces WS_CR_AT_EOL, I find\nthat the intent and the approach of the patch is quite good.  I suspect\nthat combined-diff output routines have the same issue that you need to\nfix the same way, but I didn't look.\n"},{"id":"88861","messageId":"7v7ia1kg8s.fsf@gitster.siamese.dyndns.org","threadId":"15202","inReplyTo":"7vbpzdkgy9.fsf@gitster.siamese.dyndns.org","subject":"Re: question: how to ignore extral CR when diff dos format files with 'color=auto'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-28T02:47:31Z","receivedAt":"2008-08-28T02:47:31Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> ...  I suspect\n> that combined-diff output routines have the same issue that you need to\n> fix the same way, but I didn't look.\n\nNow I looked.  The combine-diff part should look like this, I think.\n\ndiff --git i/combine-diff.c w/combine-diff.c\nindex 534be38..de83c69 100644\n--- i/combine-diff.c\n+++ w/combine-diff.c\n@@ -496,6 +496,18 @@ static int hunk_comment_line(const char *bol)\n \treturn (isalpha(ch) || ch == '_' || ch == '$');\n }\n \n+static void show_line_to_eol(const char *line, int len, const char *reset)\n+{\n+\tint saw_cr_at_eol = 0;\n+\tif (len < 0)\n+\t\tlen = strlen(line);\n+\tsaw_cr_at_eol = (len && line[len-1] == '\\r');\n+\n+\tprintf(\"%.*s%s%s\\n\", len - saw_cr_at_eol, line,\n+\t       reset,\n+\t       saw_cr_at_eol ? \"\\r\" : \"\");\n+}\n+\n static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,\n \t\t       int use_color)\n {\n@@ -589,7 +601,7 @@ static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,\n \t\t\t\t\telse\n \t\t\t\t\t\tputchar(' ');\n \t\t\t\t}\n-\t\t\t\tprintf(\"%s%s\\n\", ll->line, c_reset);\n+\t\t\t\tshow_line_to_eol(ll->line, -1, c_reset);\n \t\t\t\tll = ll->next;\n \t\t\t}\n \t\t\tif (cnt < lno)\n@@ -613,7 +625,7 @@ static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,\n \t\t\t\t\tputchar(' ');\n \t\t\t\tp_mask <<= 1;\n \t\t\t}\n-\t\t\tprintf(\"%.*s%s\\n\", sl->len, sl->bol, c_reset);\n+\t\t\tshow_line_to_eol(sl->bol, sl->len, c_reset);\n \t\t}\n \t}\n }\n"}]}