Good morning,
when diffing output where files have CRLF line ending, the coloring
seems wrong, because in changed lines the CR (^M) is highlighted,
even if the line ending has not changed.
The diff engine itself is correct.
I added a test case to show this behaviour.
The problem seems to come from emit_add_line() where ws_check_emit() is
called. The parameter ecbdata->ws_rule has not set WS_CR_AT_EOL. In this
case ws_check_emit() handles the CR at eol as whitespace character and
therfore highlights it. This seems wrong for files with CRLF lineending.
,----
| static void emit_add_line(const char *reset,
| struct emit_callback *ecbdata,
| const char *line, int len)
| {
| const char *ws = diff_get_color(ecbdata->color_diff, DIFF_WHITESPACE);
| ...
|
| if (!*ws)
| ...
| else {
| /* Emit just the prefix, then the rest. */
| emit_line_0(ecbdata->opt, set, reset, '+', "", 0);
| ws_check_emit(line, len, ecbdata->ws_rule,
| ecbdata->opt->file, set, reset, ws);
| }
| }
`----
If WS_CR_AT_EOL is set in ecbdata->ws_rule, it works correctly, but this seems
not the right solutions. (Sorry, but I'm not deep enough in the code to
propose a solution.)
Another nitpick: While writing the test it was unclear for me where the color
start and end sequences will be put. Here is a difference between old lines and
new lines, because old lines will be printed with emit_line_0() and new lines
with emit_line_0() + ws_check_emit(). So in case of new lines the "+" itself
is enclosed by the color sequences, where in case of the old lines the whole
line is enclosed by the color sequences.
I tested this with 6a7071958620dad (Git 1.9.0-rc3), but this is also wrong
in older versions.
With kind regards,
Stefan
---
t/t4060-diff-eol.sh | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 81 insertions(+), 0 deletion(-)
create mode 100755 t/t4060-diff-eol.sh
diff --git a/t/t4060-diff-eol.sh b/t/t4060-diff-eol.sh
new file mode 100755
index 0000000..8cf9a69
--- /dev/null
+++ b/t/t4060-diff-eol.sh
@@ -0,0 +1,81 @@
+#!/bin/sh
+#
+# Copyright (c) 2014 Stefan-W. Hahn
+#
+
+test_description='Test coloring of diff with CRLF line ending.
+
+'
+. ./test-lib.sh
+
+get_color ()
+{
+ git config --get-color "$1"
+}
+
+tr 'Q' '\015' << EOF > x
+Zeile 1Q
+Zeile 2Q
+Zeile 3Q
+EOF
+
+git update-index --add x
+
+tr 'Q' '\015' << EOF > x
+Zeile 1Q
+Zeile 22Q
+Zeile 3Q
+EOF
+
+tr 'Q' '\015' << EOF > expect
+diff --git a/x b/x
+index 3411cc1..68a4b2c 100644
+--- a/x
++++ b/x
+@@ -1,3 +1,3 @@
+ Zeile 1Q
+-Zeile 2Q
++Zeile 22Q
+ Zeile 3Q
+EOF
+
+
+git -c color.diff=false diff > out
+test_expect_success "diff files ending with CRLF without color" '
+ test_cmp expect out'
+
+test_expect_success setup '
+ git config color.diff.plain black &&
+ git config color.diff.meta blue &&
+ git config color.diff.frag yellow &&
+ git config color.diff.func normal &&
+ git config color.diff.old red &&
+ git config color.diff.new green &&
+ git config color.diff.commit normal &&
+ c_reset=$(git config --get-color no.such.color reset) &&
+ c_plain=$(get_color color.diff.plain) &&
+ c_meta=$(get_color color.diff.meta) &&
+ c_frag=$(get_color color.diff.frag) &&
+ c_func=$(get_color color.diff.func) &&
+ c_old=$(get_color color.diff.old) &&
+ c_new=$(get_color color.diff.new) &&
+ c_commit=$(get_color color.diff.commit) &&
+ c_whitespace=$(get_color color.diff.whitespace)
+'
+
+tr 'Q' '\015' << EOF > expect
+${c_meta}diff --git a/x b/x${c_reset}
+${c_meta}index 3411cc1..68a4b2c 100644${c_reset}
+${c_meta}--- a/x${c_reset}
+${c_meta}+++ b/x${c_reset}
+${c_frag}@@ -1,3 +1,3 @@${c_reset}
+${c_plain} Zeile 1${c_reset}Q
+${c_old}-Zeile 2${c_reset}Q
+${c_new}+${c_reset}${c_new}Zeile 22${c_reset}Q
+${c_plain} Zeile 3${c_reset}Q
+EOF
+
+git -c color.diff=always diff > out
+test_expect_success "diff files ending with CRLF with color coding" 'test_cmp
expect out'
+
+test_done
--
1.8.3.2.733.gf8abaeb
--
Stefan-W. Hahn It is easy to make things.
It is hard to make things simple.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
