* Bug: Problem with CRLF line ending in git-diff with coloring
@ 2014-02-09 11:01 Stefan-W. Hahn
2014-02-09 18:30 ` Johannes Sixt
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Stefan-W. Hahn @ 2014-02-09 11:01 UTC (permalink / raw)
To: git
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.
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Bug: Problem with CRLF line ending in git-diff with coloring
2014-02-09 11:01 Bug: Problem with CRLF line ending in git-diff with coloring Stefan-W. Hahn
@ 2014-02-09 18:30 ` Johannes Sixt
2014-02-14 16:47 ` Stefan-W. Hahn
2014-02-14 21:17 ` Stefan-W. Hahn
2 siblings, 0 replies; 6+ messages in thread
From: Johannes Sixt @ 2014-02-09 18:30 UTC (permalink / raw)
To: Stefan-W. Hahn, git
Am 09.02.2014 12:01, schrieb Stefan-W. Hahn:
> 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.
...
> If WS_CR_AT_EOL is set in ecbdata->ws_rule, it works correctly, but this seems
> not the right solutions.
It's the right solution. IOW, you should place something like this in
your .gitattributes:
*.html whitespace=cr-at-eol
-- Hannes
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug: Problem with CRLF line ending in git-diff with coloring
2014-02-09 11:01 Bug: Problem with CRLF line ending in git-diff with coloring Stefan-W. Hahn
2014-02-09 18:30 ` Johannes Sixt
@ 2014-02-14 16:47 ` Stefan-W. Hahn
2014-02-14 21:19 ` Johannes Sixt
2014-02-14 21:17 ` Stefan-W. Hahn
2 siblings, 1 reply; 6+ messages in thread
From: Stefan-W. Hahn @ 2014-02-14 16:47 UTC (permalink / raw)
To: git; +Cc: Johannes Sixt
[-- Attachment #1: Type: text/plain, Size: 836 bytes --]
Mail von Stefan-W. Hahn, Sun, 09 Feb 2014 at 12:01:55 +0100:
Good afternoon,
I updated the test a little bit. Test 3 and 7 are going wrong.
Both tests have a CRLF line ending in the changed line.
I you redirect the output of the test to a file you see the main
problem:
,----
| -^[[32m+^[[m^[[32mZeile 22^[[m^[[32m\r^[[m
| +^[[32m+^[[m^[[32mZeile 22^[[m^[[41m\r^[[m
`----
> It's the right solution. IOW, you should place something like this in
> your .gitattributes:
> *.html whitespace=cr-at-eol
Sorry, but this is not possible, because I have files of both sorts (mainly
C/C++) files in my repository and cannot change the files as I wish.
With kind regards,
Stefan
--
Stefan-W. Hahn It is easy to make things.
It is hard to make things simple.
[-- Attachment #2: t4060-diff-eol.sh --]
[-- Type: application/x-sh, Size: 4199 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug: Problem with CRLF line ending in git-diff with coloring
2014-02-09 11:01 Bug: Problem with CRLF line ending in git-diff with coloring Stefan-W. Hahn
2014-02-09 18:30 ` Johannes Sixt
2014-02-14 16:47 ` Stefan-W. Hahn
@ 2014-02-14 21:17 ` Stefan-W. Hahn
2 siblings, 0 replies; 6+ messages in thread
From: Stefan-W. Hahn @ 2014-02-14 21:17 UTC (permalink / raw)
To: git; +Cc: Johannes Sixt
Mail von Stefan-W. Hahn, Sun, 09 Feb 2014 at 12:01:55 +0100:
Good afternoon,
I updated the test a little bit. Test 3 and 7 are going wrong.
Both tests have a CRLF line ending in the changed line.
I you redirect the output of the test to a file you see the main
problem:
,----
| -^[[32m+^[[m^[[32mZeile 22^[[m^[[32m
^[[m
| +^[[32m+^[[m^[[32mZeile 22^[[m^[[41m
^[[m
`----
> It's the right solution. IOW, you should place something like this in
> your .gitattributes:
> *.html whitespace=cr-at-eol
Sorry, but this is not possible, because I have files of both sorts (mainly
C/C++) files in my repository and cannot change the files as I wish.
Second try, the mail was blocked because of the attachment blocked the mail,
so not as attachment.
,----
| Date: Fri, 14 Feb 2014 11:50:37 -0500
| From: Administrator <scanmail@arrisi.com>
| To: stefan.hahn@s-hahn.de
| Subject: [MailServer Notification]Attachment Blocking Notification
| From prvs=81222a4311=scanmail@arrisi.com Fri Feb 14 22:03:58 2014
| X-Mailer: Microsoft CDO for Windows 2000
|
| The t4060-diff-eol.sh has been blocked since it violated the Microsoft
| Exchange attachment policy and Replace with text/file has been taken on
| 2/14/2014 11:49:56 AM.
| Please zip the attachment and send it again. If you have any questions,
| please contact IT. Thank you
|
| Message details:
| Server: ATLOWA1
| Sender: stefan.hahn@s-hahn.de;
| Recipient: git@vger.kernel.org;
| Subject: Re: Bug: Problem with CRLF line ending in git-diff with coloring
| Attachment name: t4060-diff-eol.sh
| ~
`----
With kind regards,
Stefan
#!/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"
}
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)
'
# Test cases
# - DOS line ending
# - change one line
# - change one line ending to UNIX
# - UNIX line ending
# - change one line (trivial not tested here)
# - change one line ending to DOS
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
tr 'Q' '\015' << EOF > expect_color
${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}${c_new}Q${c_reset}
${c_plain} Zeile 3${c_reset}Q
EOF
git -c color.diff=false diff > out
test_expect_success "diff files: change line in DOS file without color" '
test_cmp expect out'
git -c color.diff=always diff > out
test_expect_success "diff files: change line in DOS file with color" '
test_cmp expect_color out'
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 2
Zeile 3Q
EOF
tr 'Q' '\015' << EOF > expect
diff --git a/x b/x
index 3411cc1..c040c67 100644
--- a/x
+++ b/x
@@ -1,3 +1,3 @@
Zeile 1Q
-Zeile 2Q
+Zeile 2
Zeile 3Q
EOF
tr 'Q' '\015' << EOF > expect_color
${c_meta}diff --git a/x b/x${c_reset}
${c_meta}index 3411cc1..c040c67 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 2${c_reset}
${c_plain} Zeile 3${c_reset}Q
EOF
git -c color.diff=false diff > out
test_expect_success "diff files: change line ending in DOS file to LF ending without color" '
test_cmp expect out'
git -c color.diff=always diff > out
test_expect_success "diff files: change line ending in DOS file to LF ending with color" '
test_cmp expect_color out'
tr 'Q' '\015' << EOF > x
Zeile 1
Zeile 2
Zeile 3
EOF
git update-index --add x
tr 'Q' '\015' << EOF > x
Zeile 1
Zeile 2Q
Zeile 3
EOF
tr 'Q' '\015' << EOF > expect
diff --git a/x b/x
index a385875..63416d7 100644
--- a/x
+++ b/x
@@ -1,3 +1,3 @@
Zeile 1
-Zeile 2
+Zeile 2Q
Zeile 3
EOF
tr 'Q' '\015' << EOF > expect_color
${c_meta}diff --git a/x b/x${c_reset}
${c_meta}index a385875..63416d7 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}
${c_old}-Zeile 2${c_reset}
${c_new}+${c_reset}${c_new}Zeile 2${c_reset}${c_new}Q${c_reset}
${c_plain} Zeile 3${c_reset}
EOF
git -c color.diff=false diff > out
test_expect_success "diff files: change line ending in UNIX file to CRLF ending without color" '
test_cmp expect out'
git -c color.diff=always diff > out
test_expect_success "diff files: change line ending in UNIX file to CRLF ending with color" '
test_cmp expect_color out'
test_done
--
Stefan-W. Hahn It is easy to make things.
It is hard to make things simple.
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Bug: Problem with CRLF line ending in git-diff with coloring
2014-02-14 16:47 ` Stefan-W. Hahn
@ 2014-02-14 21:19 ` Johannes Sixt
2014-02-15 7:21 ` Stefan-W. Hahn
0 siblings, 1 reply; 6+ messages in thread
From: Johannes Sixt @ 2014-02-14 21:19 UTC (permalink / raw)
To: Stefan-W. Hahn, git
Am 14.02.2014 17:47, schrieb Stefan-W. Hahn:
>> It's the right solution. IOW, you should place something like this in
>> your .gitattributes:
>> *.html whitespace=cr-at-eol
>
> Sorry, but this is not possible, because I have files of both sorts (mainly
> C/C++) files in my repository and cannot change the files as I wish.
I'm confused. This setting does not change your files, but instructs git
diff and git apply to not report the trailing CR as white-space error.
Didn't you try it?
-- Hannes
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug: Problem with CRLF line ending in git-diff with coloring
2014-02-14 21:19 ` Johannes Sixt
@ 2014-02-15 7:21 ` Stefan-W. Hahn
0 siblings, 0 replies; 6+ messages in thread
From: Stefan-W. Hahn @ 2014-02-15 7:21 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git
Mail von Johannes Sixt, Fri, 14 Feb 2014 at 22:19:03 +0100:
Good morning,
> Am 14.02.2014 17:47, schrieb Stefan-W. Hahn:
> >> It's the right solution. IOW, you should place something like this in
> >> your .gitattributes:
> >> *.html whitespace=cr-at-eol
> >
> > Sorry, but this is not possible, because I have files of both sorts (mainly
> > C/C++) files in my repository and cannot change the files as I wish.
>
> I'm confused. This setting does not change your files, but instructs git
> diff and git apply to not report the trailing CR as white-space error.
> Didn't you try it?
You are right, if I configure
git config core.whitespace cr-at-eol
then the CR is not highlighted.
I try to work with it; I hope there are no other traps with it.
I changed the test to regard this, here it is.
With kind regards,
Stefan
#!/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"
}
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)
'
# Test cases
# - DOS line ending
# - change one line
# - change one line ending to UNIX
# - UNIX line ending
# - change one line (trivial not tested here)
# - change one line ending to DOS
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
tr 'Q' '\015' << EOF > expect_color
${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=false diff > out
test_expect_success "diff files: change line in DOS file without color" '
test_cmp expect out'
git -c color.diff=always -c core.whitespace=cr-at-eol diff > out
test_expect_success "diff files: change line in DOS file with color" '
test_cmp expect_color out'
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 2
Zeile 3Q
EOF
tr 'Q' '\015' << EOF > expect
diff --git a/x b/x
index 3411cc1..c040c67 100644
--- a/x
+++ b/x
@@ -1,3 +1,3 @@
Zeile 1Q
-Zeile 2Q
+Zeile 2
Zeile 3Q
EOF
tr 'Q' '\015' << EOF > expect_color
${c_meta}diff --git a/x b/x${c_reset}
${c_meta}index 3411cc1..c040c67 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 2${c_reset}
${c_plain} Zeile 3${c_reset}Q
EOF
git -c color.diff=false diff > out
test_expect_success "diff files: change line ending in DOS file to LF ending without color" '
test_cmp expect out'
git -c color.diff=always diff > out
test_expect_success "diff files: change line ending in DOS file to LF ending with color" '
test_cmp expect_color out'
tr 'Q' '\015' << EOF > x
Zeile 1
Zeile 2
Zeile 3
EOF
git update-index --add x
tr 'Q' '\015' << EOF > x
Zeile 1
Zeile 2Q
Zeile 3
EOF
tr 'Q' '\015' << EOF > expect
diff --git a/x b/x
index a385875..63416d7 100644
--- a/x
+++ b/x
@@ -1,3 +1,3 @@
Zeile 1
-Zeile 2
+Zeile 2Q
Zeile 3
EOF
tr 'Q' '\015' << EOF > expect_color
${c_meta}diff --git a/x b/x${c_reset}
${c_meta}index a385875..63416d7 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}
${c_old}-Zeile 2${c_reset}
${c_new}+${c_reset}${c_new}Zeile 2${c_reset}Q
${c_plain} Zeile 3${c_reset}
EOF
git -c color.diff=false diff > out
test_expect_success "diff files: change line ending in UNIX file to CRLF ending without color" '
test_cmp expect out'
git -c color.diff=always -c core.whitespace=cr-at-eol diff > out
test_expect_success "diff files: change line ending in UNIX file to CRLF ending with color" '
test_cmp expect_color out'
test_done
--
Stefan-W. Hahn It is easy to make things.
It is hard to make things simple.
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-02-15 7:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-09 11:01 Bug: Problem with CRLF line ending in git-diff with coloring Stefan-W. Hahn
2014-02-09 18:30 ` Johannes Sixt
2014-02-14 16:47 ` Stefan-W. Hahn
2014-02-14 21:19 ` Johannes Sixt
2014-02-15 7:21 ` Stefan-W. Hahn
2014-02-14 21:17 ` Stefan-W. Hahn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).