question: how to ignore extral CR when diff dos format files with 'color=auto'

question: how to ignore extral CR when diff dos format files with 'color=auto'

[goooguo]

hi,

git-diff prints "^M" at each end of line for dos format files when
'color=auto'.

it's annoying!

I want to know if I can configure git to make git-diff ignore '^M', and I
don't want to change file format to unix.

thank you.
-- 
View 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
Sent from the git mailing list archive at Nabble.com.

Re: question: how to ignore extral CR when diff dos format files with 'color=auto'

[goooguo]

here is my patch

http://n2.nabble.com/file/n786034/0001-ingore-cr-at-eol-when-diff-with-color-auto.patch
0001-ingore-cr-at-eol-when-diff-with-color-auto.patch 

-- 
View 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
Sent from the git mailing list archive at Nabble.com.

Re: question: how to ignore extral CR when diff dos format files with 'color=auto'

[Junio C Hamano]

goooguo <erwangg@fortemedia.com.cn> writes:

> git-diff prints "^M" at each end of line for dos format files when
> 'color=auto'.
>
> it's annoying!

"man git-config" and learn about core.whitespace perhaps?

Re: question: how to ignore extral CR when diff dos format files with 'color=auto'

[goooguo]

git v1.6.0 still didn't work, neither v1.5.6.4. (however,v1.5.5 works on
mingwin)

I checked the source code of v1.6.0, and I found emit_line didn't check
whether there is a cr at eol.

I have fixed it.

>From dae20e25960c73bd7ccc0939fe096bb68a009fb5 Mon Sep 17 00:00:00 2001
From: erwangg <erwangg@fortemedia.com.cn>
Date: Wed, 27 Aug 2008 12:22:43 +0800
Subject: [PATCH] ingore cr at eol when diff with color=auto

---
 diff.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/diff.c b/diff.c
index 18fa7a7..846a9af 100644
--- a/diff.c
+++ b/diff.c
@@ -517,9 +517,16 @@ static void emit_line(FILE *file, const char *set,
const char *reset, const char
 	if (has_trailing_newline)
 		len--;
 
+	int has_trailing_return = (len > 0 && line[len-1] == '\r');
+	if (has_trailing_return)
+        len--;
+
 	fputs(set, file);
 	fwrite(line, len, 1, file);
 	fputs(reset, file);
+
+    if (has_trailing_return)
+        fputc('\r', file);
 	if (has_trailing_newline)
 		fputc('\n', file);
 }
@@ -535,7 +542,7 @@ static void emit_add_line(const char *reset, struct
emit_callback *ecbdata, cons
 		/* Emit just the prefix, then the rest. */
 		emit_line(ecbdata->file, set, reset, line, ecbdata->nparents);
 		ws_check_emit(line + ecbdata->nparents,
-			      len - ecbdata->nparents, ecbdata->ws_rule,
+			      len - ecbdata->nparents, ecbdata->ws_rule|WS_CR_AT_EOL,
 			      ecbdata->file, set, reset, ws);
 	}
 }
-- 
1.6.0.GIT




-- 
View 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
Sent from the git mailing list archive at Nabble.com.

Re: question: how to ignore extral CR when diff dos format files with 'color=auto'

[Junio C Hamano]

> From dae20e25960c73bd7ccc0939fe096bb68a009fb5 Mon Sep 17 00:00:00 2001
> From: erwangg <erwangg@fortemedia.com.cn>
> Date: Wed, 27 Aug 2008 12:22:43 +0800
> Subject: [PATCH] ingore cr at eol when diff with color=auto
>
> ---

Thanks.  A few comments.

> diff --git a/diff.c b/diff.c
> index 18fa7a7..846a9af 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -517,9 +517,16 @@ static void emit_line(FILE *file, const char *set,
> const char *reset, const char
>  	if (has_trailing_newline)
>  		len--;
>  
> +	int has_trailing_return = (len > 0 && line[len-1] == '\r');

decl-after-statement.

> +	if (has_trailing_return)
> +        len--;
> +
>  	fputs(set, file);
>  	fwrite(line, len, 1, file);
>  	fputs(reset, file);
> +
> +    if (has_trailing_return)
> +        fputc('\r', file);
>  	if (has_trailing_newline)
>  		fputc('\n', file);
>  }

We indent with tab which jumps the cursor to next column that is multiple
of 8.

Otherwise this hunk is good.

> @@ -535,7 +542,7 @@ static void emit_add_line(const char *reset, struct
> emit_callback *ecbdata, cons
>  		/* Emit just the prefix, then the rest. */
>  		emit_line(ecbdata->file, set, reset, line, ecbdata->nparents);
>  		ws_check_emit(line + ecbdata->nparents,
> -			      len - ecbdata->nparents, ecbdata->ws_rule,
> +			      len - ecbdata->nparents, ecbdata->ws_rule|WS_CR_AT_EOL,
>  			      ecbdata->file, set, reset, ws);
>  	}
>  }

I do not think you want to force WS_CR_AT_EOL here.

That is a policy issue (not "git policy", but the policy of the contents
that is managed by git, in other words, your repository) that you should
control with core.whitespace (and if you want finer grain control, then
with .gitattributes).

Your patch is whitespace damanged, and has style issues, and lacks any
comment to defend the change.  It does not have a Sign-off, either.  Even
though the intent is good, I cannot apply this as-is.  Please look at
Documentation/SubmittingPatches and emulate patches from other git
regulars.

You say "when color=auto" in the patch title, but there is nothing that
restricts the effect of this change only to that case in your patch.  A
comment to defend the change should address things like that, and here is
how I would write it.

    When the tracked contents have CRLF line endings, colored diff output
    shows "^M" at the end of output lines, which is distracting, even
    though the pager we use by default ("less") knows to hide them.

    The problem is that "less" hides a carriage-return only at the end of
    the line, immediately before a line feed.  The colored diff output
    does not take this into account, and emits four element sequence for
    each line:

       - force this color;
       - the line up to but not including the terminating line feed;
       - reset color
       - line feed.

    By including the carriage return at the end of the line in the second
    item, we are breaking the smart our pager has in order not to show
    "^M".  This can be fixed by changing the sequence to:

       - force this color;
       - the line up to but not including the terminating end-of-line;
       - reset color
       - end-of-line.

    where end-of-line is either a single linefeed or a CRLF pair.
    When the output is not colored, "force this color" and "reset color"
    sequences are both empty, so we won't have this problem with or
    without this patch.    

If we drop the hunk to emit_add_line() that forces WS_CR_AT_EOL, I find
that the intent and the approach of the patch is quite good.  I suspect
that combined-diff output routines have the same issue that you need to
fix the same way, but I didn't look.

Re: question: how to ignore extral CR when diff dos format files with 'color=auto'

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> ...  I suspect
> that combined-diff output routines have the same issue that you need to
> fix the same way, but I didn't look.

Now I looked.  The combine-diff part should look like this, I think.

diff --git i/combine-diff.c w/combine-diff.c
index 534be38..de83c69 100644
--- i/combine-diff.c
+++ w/combine-diff.c
@@ -496,6 +496,18 @@ static int hunk_comment_line(const char *bol)
 	return (isalpha(ch) || ch == '_' || ch == '$');
 }
 
+static void show_line_to_eol(const char *line, int len, const char *reset)
+{
+	int saw_cr_at_eol = 0;
+	if (len < 0)
+		len = strlen(line);
+	saw_cr_at_eol = (len && line[len-1] == '\r');
+
+	printf("%.*s%s%s\n", len - saw_cr_at_eol, line,
+	       reset,
+	       saw_cr_at_eol ? "\r" : "");
+}
+
 static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,
 		       int use_color)
 {
@@ -589,7 +601,7 @@ static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,
 					else
 						putchar(' ');
 				}
-				printf("%s%s\n", ll->line, c_reset);
+				show_line_to_eol(ll->line, -1, c_reset);
 				ll = ll->next;
 			}
 			if (cnt < lno)
@@ -613,7 +625,7 @@ static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,
 					putchar(' ');
 				p_mask <<= 1;
 			}
-			printf("%.*s%s\n", sl->len, sl->bol, c_reset);
+			show_line_to_eol(sl->bol, sl->len, c_reset);
 		}
 	}
 }