From 23b250ab0f041bba7af4ba8eb1a686f6850677a6 Mon Sep 17 00:00:00 2001 From: Brian Henderson Date: Mon, 29 Aug 2016 10:33:45 -0700 Subject: diff-highlight: add some tests Signed-off-by: Brian Henderson Signed-off-by: Junio C Hamano --- contrib/diff-highlight/Makefile | 5 + contrib/diff-highlight/t/Makefile | 22 +++ contrib/diff-highlight/t/t9400-diff-highlight.sh | 163 +++++++++++++++++++++++ 3 files changed, 190 insertions(+) create mode 100644 contrib/diff-highlight/Makefile create mode 100644 contrib/diff-highlight/t/Makefile create mode 100755 contrib/diff-highlight/t/t9400-diff-highlight.sh diff --git a/contrib/diff-highlight/Makefile b/contrib/diff-highlight/Makefile new file mode 100644 index 0000000000..9018724524 --- /dev/null +++ b/contrib/diff-highlight/Makefile @@ -0,0 +1,5 @@ +# nothing to build +all: + +test: + $(MAKE) -C t diff --git a/contrib/diff-highlight/t/Makefile b/contrib/diff-highlight/t/Makefile new file mode 100644 index 0000000000..5ff5275496 --- /dev/null +++ b/contrib/diff-highlight/t/Makefile @@ -0,0 +1,22 @@ +-include ../../../config.mak.autogen +-include ../../../config.mak + +# copied from ../../t/Makefile +SHELL_PATH ?= $(SHELL) +SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH)) +T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh) + +all: test +test: $(T) + +.PHONY: help clean all test $(T) + +help: + @echo 'Run "$(MAKE) test" to launch test scripts' + @echo 'Run "$(MAKE) clean" to remove trash folders' + +$(T): + @echo "*** $@ ***"; '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS) + +clean: + $(RM) -r 'trash directory'.* diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh new file mode 100755 index 0000000000..446b4daed8 --- /dev/null +++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh @@ -0,0 +1,163 @@ +#!/bin/sh + +test_description='Test diff-highlight' + +CURR_DIR=$(pwd) +TEST_OUTPUT_DIRECTORY=$(pwd) +TEST_DIRECTORY="$CURR_DIR"/../../../t +DIFF_HIGHLIGHT="$CURR_DIR"/../diff-highlight + +CW="$(printf "\033[7m")" # white +CR="$(printf "\033[27m")" # reset + +. "$TEST_DIRECTORY"/test-lib.sh + +if ! test_have_prereq PERL +then + skip_all='skipping diff-highlight tests; perl not available' + test_done +fi + +# dh_test is a test helper function which takes 3 file names as parameters. The +# first 2 files are used to generate diff and commit output, which is then +# piped through diff-highlight. The 3rd file should contain the expected output +# of diff-highlight (minus the diff/commit header, ie. everything after and +# including the first @@ line). +dh_test () { + a="$1" b="$2" && + + cat >patch.exp && + + { + cat "$a" >file && + git add file && + git commit -m "Add a file" && + + cat "$b" >file && + git diff file >diff.raw && + git commit -a -m "Update a file" && + git show >commit.raw + } >/dev/null && + + "$DIFF_HIGHLIGHT" diff.act && + "$DIFF_HIGHLIGHT" commit.act && + test_cmp patch.exp diff.act && + test_cmp patch.exp commit.act +} + +test_strip_patch_header () { + sed -n '/^@@/,$p' $* +} + +test_expect_success 'diff-highlight highlights the beginning of a line' ' + cat >a <<-\EOF && + aaa + bbb + ccc + EOF + + cat >b <<-\EOF && + aaa + 0bb + ccc + EOF + + dh_test a b <<-EOF + @@ -1,3 +1,3 @@ + aaa + -${CW}b${CR}bb + +${CW}0${CR}bb + ccc + EOF +' + +test_expect_success 'diff-highlight highlights the end of a line' ' + cat >a <<-\EOF && + aaa + bbb + ccc + EOF + + cat >b <<-\EOF && + aaa + bb0 + ccc + EOF + + dh_test a b <<-EOF + @@ -1,3 +1,3 @@ + aaa + -bb${CW}b${CR} + +bb${CW}0${CR} + ccc + EOF +' + +test_expect_success 'diff-highlight highlights the middle of a line' ' + cat >a <<-\EOF && + aaa + bbb + ccc + EOF + + cat >b <<-\EOF && + aaa + b0b + ccc + EOF + + dh_test a b <<-EOF + @@ -1,3 +1,3 @@ + aaa + -b${CW}b${CR}b + +b${CW}0${CR}b + ccc + EOF +' + +test_expect_success 'diff-highlight does not highlight whole line' ' + cat >a <<-\EOF && + aaa + bbb + ccc + EOF + + cat >b <<-\EOF && + aaa + 000 + ccc + EOF + + dh_test a b <<-EOF + @@ -1,3 +1,3 @@ + aaa + -bbb + +000 + ccc + EOF +' + +test_expect_failure 'diff-highlight highlights mismatched hunk size' ' + cat >a <<-\EOF && + aaa + bbb + EOF + + cat >b <<-\EOF && + aaa + b0b + ccc + EOF + + dh_test a b <<-EOF + @@ -1,3 +1,3 @@ + aaa + -b${CW}b${CR}b + +b${CW}0${CR}b + +ccc + EOF +' + +# TODO add multi-byte test + +test_done -- cgit v1.2.3 From caf5ea707c2a412cdb61ef694678673b4cbec550 Mon Sep 17 00:00:00 2001 From: Brian Henderson Date: Mon, 29 Aug 2016 10:33:46 -0700 Subject: diff-highlight: add failing test for handling --graph output Signed-off-by: Brian Henderson Signed-off-by: Junio C Hamano --- contrib/diff-highlight/t/t9400-diff-highlight.sh | 62 ++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh index 446b4daed8..1537c69d52 100755 --- a/contrib/diff-highlight/t/t9400-diff-highlight.sh +++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh @@ -49,6 +49,55 @@ test_strip_patch_header () { sed -n '/^@@/,$p' $* } +# dh_test_setup_history generates a contrived graph such that we have at least +# 1 nesting (E) and 2 nestings (F). +# +# A branch +# / +# D---E---F master +# +# git log --all --graph +# * commit +# | A +# | * commit +# | | F +# | * commit +# |/ +# | E +# * commit +# D +# +dh_test_setup_history () { + echo "file1" >file1 && + echo "file2" >file2 && + echo "file3" >file3 && + + cat file1 >file && + git add file && + git commit -m "D" && + + git checkout -b branch && + cat file2 >file && + git commit -a -m "A" && + + git checkout master && + cat file2 >file && + git commit -a -m "E" && + + cat file3 >file && + git commit -a -m "F" +} + +left_trim () { + "$PERL_PATH" -pe 's/^\s+//' +} + +trim_graph () { + # graphs start with * or | + # followed by a space or / or \ + "$PERL_PATH" -pe 's@^((\*|\|)( |/|\\))+@@' +} + test_expect_success 'diff-highlight highlights the beginning of a line' ' cat >a <<-\EOF && aaa @@ -160,4 +209,17 @@ test_expect_failure 'diff-highlight highlights mismatched hunk size' ' # TODO add multi-byte test +test_expect_failure 'diff-highlight works with the --graph option' ' + dh_test_setup_history && + + # topo-order so that the order of the commits is the same as with --graph + # trim graph elements so we can do a diff + # trim leading space because our trim_graph is not perfect + git log --branches -p --topo-order | + "$DIFF_HIGHLIGHT" | left_trim >graph.exp && + git log --branches -p --graph | + "$DIFF_HIGHLIGHT" | trim_graph | left_trim >graph.act && + test_cmp graph.exp graph.act +' + test_done -- cgit v1.2.3 From 7e4ffb4c1778c725db3db116f016ef2ad1c6d218 Mon Sep 17 00:00:00 2001 From: Brian Henderson Date: Mon, 29 Aug 2016 10:33:47 -0700 Subject: diff-highlight: add support for --graph output Signed-off-by: Brian Henderson Signed-off-by: Junio C Hamano --- contrib/diff-highlight/diff-highlight | 19 +++++++++++++------ contrib/diff-highlight/t/t9400-diff-highlight.sh | 2 +- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/contrib/diff-highlight/diff-highlight b/contrib/diff-highlight/diff-highlight index ffefc31a98..9280c88800 100755 --- a/contrib/diff-highlight/diff-highlight +++ b/contrib/diff-highlight/diff-highlight @@ -21,6 +21,10 @@ my $RESET = "\x1b[m"; my $COLOR = qr/\x1b\[[0-9;]*m/; my $BORING = qr/$COLOR|\s/; +# The patch portion of git log -p --graph should only ever have preceding | and +# not / or \ as merge history only shows up on the commit line. +my $GRAPH = qr/$COLOR?\|$COLOR?\s+/; + my @removed; my @added; my $in_hunk; @@ -32,12 +36,12 @@ $SIG{PIPE} = 'DEFAULT'; while (<>) { if (!$in_hunk) { print; - $in_hunk = /^$COLOR*\@/; + $in_hunk = /^$GRAPH*$COLOR*\@/; } - elsif (/^$COLOR*-/) { + elsif (/^$GRAPH*$COLOR*-/) { push @removed, $_; } - elsif (/^$COLOR*\+/) { + elsif (/^$GRAPH*$COLOR*\+/) { push @added, $_; } else { @@ -46,7 +50,7 @@ while (<>) { @added = (); print; - $in_hunk = /^$COLOR*[\@ ]/; + $in_hunk = /^$GRAPH*$COLOR*[\@ ]/; } # Most of the time there is enough output to keep things streaming, @@ -163,6 +167,9 @@ sub highlight_pair { } } +# we split either by $COLOR or by character. This has the side effect of +# leaving in graph cruft. It works because the graph cruft does not contain "-" +# or "+" sub split_line { local $_ = shift; return utf8::decode($_) ? @@ -211,8 +218,8 @@ sub is_pair_interesting { my $suffix_a = join('', @$a[($sa+1)..$#$a]); my $suffix_b = join('', @$b[($sb+1)..$#$b]); - return $prefix_a !~ /^$COLOR*-$BORING*$/ || - $prefix_b !~ /^$COLOR*\+$BORING*$/ || + return $prefix_a !~ /^$GRAPH*$COLOR*-$BORING*$/ || + $prefix_b !~ /^$GRAPH*$COLOR*\+$BORING*$/ || $suffix_a !~ /^$BORING*$/ || $suffix_b !~ /^$BORING*$/; } diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh index 1537c69d52..e42ff31f3d 100755 --- a/contrib/diff-highlight/t/t9400-diff-highlight.sh +++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh @@ -209,7 +209,7 @@ test_expect_failure 'diff-highlight highlights mismatched hunk size' ' # TODO add multi-byte test -test_expect_failure 'diff-highlight works with the --graph option' ' +test_expect_success 'diff-highlight works with the --graph option' ' dh_test_setup_history && # topo-order so that the order of the commits is the same as with --graph -- cgit v1.2.3 From 9f76e520027d1bbe675d44a951da6f0b3cc3b51e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 31 Aug 2016 01:02:53 -0400 Subject: diff-highlight: ignore test cruft These are the same as in the normal t/.gitignore, with the exception of ".prove", as our Makefile does not support it. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- contrib/diff-highlight/t/.gitignore | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 contrib/diff-highlight/t/.gitignore diff --git a/contrib/diff-highlight/t/.gitignore b/contrib/diff-highlight/t/.gitignore new file mode 100644 index 0000000000..7dcbb232cd --- /dev/null +++ b/contrib/diff-highlight/t/.gitignore @@ -0,0 +1,2 @@ +/trash directory* +/test-results -- cgit v1.2.3 From 1b5290b125fca91f620aadbc1a5e3c559d4a5313 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 31 Aug 2016 01:03:10 -0400 Subject: diff-highlight: add multi-byte tests Now that we have a test suite for diff highlight, we can show off the improvements from 8d00662 (diff-highlight: do not split multibyte characters, 2015-04-03). While we're at it, we can also add another case that _doesn't_ work: combining code points are treated as their own unit, which means that we may stick colors between them and the character they are modifying (with the result that the color is not shown in an xterm, though it's possible that other terminals err the other way, and show the color but not the accent). There's no fix here, but let's document it as a failure. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- contrib/diff-highlight/t/t9400-diff-highlight.sh | 36 +++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh index e42ff31f3d..b0fe7cc8aa 100755 --- a/contrib/diff-highlight/t/t9400-diff-highlight.sh +++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh @@ -207,7 +207,41 @@ test_expect_failure 'diff-highlight highlights mismatched hunk size' ' EOF ' -# TODO add multi-byte test +# These two code points share the same leading byte in UTF-8 representation; +# a naive byte-wise diff would highlight only the second byte. +# +# - U+00f3 ("o" with acute) +o_accent=$(printf '\303\263') +# - U+00f8 ("o" with stroke) +o_stroke=$(printf '\303\270') + +test_expect_success 'diff-highlight treats multibyte utf-8 as a unit' ' + echo "unic${o_accent}de" >a && + echo "unic${o_stroke}de" >b && + dh_test a b <<-EOF + @@ -1 +1 @@ + -unic${CW}${o_accent}${CR}de + +unic${CW}${o_stroke}${CR}de + EOF +' + +# Unlike the UTF-8 above, these are combining code points which are meant +# to modify the character preceding them: +# +# - U+0301 (combining acute accent) +combine_accent=$(printf '\314\201') +# - U+0302 (combining circumflex) +combine_circum=$(printf '\314\202') + +test_expect_failure 'diff-highlight treats combining code points as a unit' ' + echo "unico${combine_accent}de" >a && + echo "unico${combine_circum}de" >b && + dh_test a b <<-EOF + @@ -1 +1 @@ + -unic${CW}o${combine_accent}${CR}de + +unic${CW}o${combine_circum}${CR}de + EOF +' test_expect_success 'diff-highlight works with the --graph option' ' dh_test_setup_history && -- cgit v1.2.3 From 3dbfe2b8ae94cbdae5f3d32581aedaa5510fdc87 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 31 Aug 2016 01:05:38 -0400 Subject: diff-highlight: avoid highlighting combined diffs The algorithm in diff-highlight only understands how to look at two sides of a diff; it cannot correctly handle combined diffs with multiple preimages. Often highlighting does not trigger at all for these diffs because the line counts do not match up. E.g., if we see: - ours -theirs ++resolved we would not bother highlighting; it naively looks like a single line went away, and then a separate hunk added another single line. But of course there are exceptions. E.g., if the other side deleted the line, we might see: - ours ++resolved which looks like we dropped " ours" and added "+resolved". This is only a small highlighting glitch (we highlight the space and the "+" along with the content), but it's also the tip of the iceberg. Even if we learned to find the true content here (by noticing we are in a 3-way combined diff and marking _two_ characters from the front of the line as uninteresting), there are other more complicated cases where we really do need to handle a 3-way hunk. Let's just punt for now; we can recognize combined diffs by the presence of extra "@" symbols in the hunk header, and treat them as non-diff content. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- contrib/diff-highlight/diff-highlight | 2 +- contrib/diff-highlight/t/t9400-diff-highlight.sh | 37 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/contrib/diff-highlight/diff-highlight b/contrib/diff-highlight/diff-highlight index 9280c88800..81bd8040e3 100755 --- a/contrib/diff-highlight/diff-highlight +++ b/contrib/diff-highlight/diff-highlight @@ -36,7 +36,7 @@ $SIG{PIPE} = 'DEFAULT'; while (<>) { if (!$in_hunk) { print; - $in_hunk = /^$GRAPH*$COLOR*\@/; + $in_hunk = /^$GRAPH*$COLOR*\@\@ /; } elsif (/^$GRAPH*$COLOR*-/) { push @removed, $_; diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh index b0fe7cc8aa..3b43dbed74 100755 --- a/contrib/diff-highlight/t/t9400-diff-highlight.sh +++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh @@ -256,4 +256,41 @@ test_expect_success 'diff-highlight works with the --graph option' ' test_cmp graph.exp graph.act ' +# Most combined diffs won't meet diff-highlight's line-number filter. So we +# create one here where one side drops a line and the other modifies it. That +# should result in a diff like: +# +# - modified content +# ++resolved content +# +# which naively looks like one side added "+resolved". +test_expect_success 'diff-highlight ignores combined diffs' ' + echo "content" >file && + git add file && + git commit -m base && + + >file && + git commit -am master && + + git checkout -b other HEAD^ && + echo "modified content" >file && + git commit -am other && + + test_must_fail git merge master && + echo "resolved content" >file && + git commit -am resolved && + + cat >expect <<-\EOF && + --- a/file + +++ b/file + @@@ -1,1 -1,0 +1,1 @@@ + - modified content + ++resolved content + EOF + + git show -c | "$DIFF_HIGHLIGHT" >actual.raw && + sed -n "/^---/,\$p" actual && + test_cmp expect actual +' + test_done -- cgit v1.2.3