Re: [PATCH v2 1/3] diff-highlight: add some tests.

8 messages, 3 authors, 2016-08-19 · open the first message on its own page

Re: [PATCH v2 1/3] diff-highlight: add some tests.

From: Junio C Hamano <hidden>
Date: 2016-08-17 19:09:34

Brian Henderson [off-list ref] writes:
Signed-off-by: Brian Henderson <redacted>
---
 contrib/diff-highlight/Makefile                  |  5 ++
 contrib/diff-highlight/t/Makefile                | 22 ++++++++
 contrib/diff-highlight/t/t9400-diff-highlight.sh | 62 +++++++++++++++++++++
 contrib/diff-highlight/t/test-diff-highlight.sh  | 69 ++++++++++++++++++++++++
 4 files changed, 158 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
 create mode 100644 contrib/diff-highlight/t/test-diff-highlight.sh
I am not sure test-diff-highlight.sh should be there; the function
definitions would still be useful but move them to the beginning of
t9400-diff-highlight.sh perhaps?
quoted hunk
diff --git a/contrib/diff-highlight/Makefile b/contrib/diff-highlight/Makefile
new file mode 100644
index 0000000..b866259
--- /dev/null
+++ b/contrib/diff-highlight/Makefile
@@ -0,0 +1,5 @@
+# nothing to build
+all:;
Drop ';'.
quoted hunk
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 0000000..8eff178
--- /dev/null
+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
@@ -0,0 +1,62 @@
+#!/bin/sh
+
+test_description='Test diff-highlight'
+
+. ./test-diff-highlight.sh
+. "$TEST_DIRECTORY"/test-lib.sh
+
+# PERL is required, but assumed to be present, although not necessarily modern
+# some tests require 5.8
+test_expect_success PERL 'name' 'true'
If the platform lacks PERL prerequisite, this will simply be
skipped, and if the platform has it, it will always succeed.

I am not sure what you are trying to achieve by having this line
here.
+test_expect_success 'diff-highlight does not highlight whole line' '
+	dh_test \
+		"aaa\nbbb\nccc\n" \
+		"aaa\n000\nccc\n"
+'
Hmm, does this express the desired outcome, or just document the
current (possibly broken--I dunno) behaviour?  The same question for
the next one.
+test_expect_success 'diff-highlight does not highlight mismatched hunk size' '
+	dh_test \
+		"aaa\nbbb\n" \
+		"aaa\nb0b\nccc\n"
+'
+dh_test() {
Style: "dh_test () {"

The other functions in this file share the same.
+	dh_diff_test "$@" &&
+	dh_commit_test "$@"
+}
+
+dh_diff_test() {
+	a="$1" b="$2"
+
+	printf "$a" >file
+	git add file
+
+	printf "$b" >file
+	git diff file >diff.raw
+
+	if test $# -eq 3
+	then
+		# remove last newline
+		head -n5 diff.raw | test_chomp_eof >diff.exp
A reader can see "remove last newline" by seeing test_chomp_eof and
what it does without a comment, but it is totally unclear why you
need to remove.  The comment that says what it does without saying
why it does it is useless.
+		printf "$3" >>diff.exp
+	else
+		cat diff.raw >diff.exp
+	fi
+
+	<diff.raw "$DIFF_HIGHLIGHT" >diff.act &&
Even though this is technically kosher, I do not see a merit of
deviating from the common practice of starting a line with the
command, i.e.

	"$DIFF_HIGHLIGHT" <diff.raw >diff.actual

would be much easier to read.
+	test -s diff.act &&
Why?  If you always have the expected output that you are going to
compare with, wouldn't that sufficient to do that test without this?
Besides, having "test -s" means that you can never make sure that a
certain pair of input does not show any changes.  Perhaps drop it?
+	diff diff.exp diff.act
Use test_cmp unless there is a strong reason why you shouldn't?
+}
+
+dh_commit_test() {
+	a="$1" b="$2"
+
+	printf "$a" >file
+	git add file
+	git commit -m"Add a file" >/dev/null
Avoid sticking a short-option to its argument, i.e.

    git commit -m "Add a file"
+
+	printf "$b" >file
+	git commit -am"Update a file" >/dev/null
Likewise.

    git commit -a -m "Update a file"

The remainder of the file invites the same set of questions and
comments you see for dh_diff_test() above, so I won't repeat them.

Thanks.

Re: [PATCH v2 1/3] diff-highlight: add some tests.

From: Brian Henderson <hidden>
Date: 2016-08-19 14:42:53

On Wed, Aug 17, 2016 at 12:09:25PM -0700, Junio C Hamano wrote:
Brian Henderson [off-list ref] writes:
<snip>
quoted
+
+# PERL is required, but assumed to be present, although not necessarily modern
+# some tests require 5.8
+test_expect_success PERL 'name' 'true'
If the platform lacks PERL prerequisite, this will simply be
skipped, and if the platform has it, it will always succeed.

I am not sure what you are trying to achieve by having this line
here.
I originally didn't have this line, and my comment was referring to the
t/README which says

   Even without the PERL prerequisite, tests can assume there is a
   usable perl interpreter at $PERL_PATH, though it need not be
   particularly modern.

There is current functionality in diff-highlight which requires at least
perl 5.8 (the utf8 functions). I was going to add a test for this as
well, but I'm not super comfy with multibyte chars.

Eric recommended adding this line, what do you think?

would `test_set_prereq PERL` be better?
quoted
+test_expect_success 'diff-highlight does not highlight whole line' '
+	dh_test \
+		"aaa\nbbb\nccc\n" \
+		"aaa\n000\nccc\n"
+'
This (at least to me) is desired. See comment for `sub
is_pair_interesting`
Hmm, does this express the desired outcome, or just document the
current (possibly broken--I dunno) behaviour?  The same question for
the next one.
quoted
+test_expect_success 'diff-highlight does not highlight mismatched hunk size' '
+	dh_test \
+		"aaa\nbbb\n" \
+		"aaa\nb0b\nccc\n"
+'
This is undesired behavior, but currently implemented for simplicity,
see `sub show_hunk`

Do they need comments or something?

<snip>
quoted
+	test -s diff.act &&
Why?  If you always have the expected output that you are going to
compare with, wouldn't that sufficient to do that test without this?
Besides, having "test -s" means that you can never make sure that a
certain pair of input does not show any changes.  Perhaps drop it?
I was trying to address Eric's concern for `printf` or `git commit` et
al failing. Also, this file will always be a diff, it just might not
having any highlighting (so not empty?).

I'll take another stab.
quoted
+	diff diff.exp diff.act
Use test_cmp unless there is a strong reason why you shouldn't?
quoted
+}
+
+dh_commit_test() {
+	a="$1" b="$2"
+
+	printf "$a" >file
+	git add file
+	git commit -m"Add a file" >/dev/null
Avoid sticking a short-option to its argument, i.e.

    git commit -m "Add a file"
quoted
+
+	printf "$b" >file
+	git commit -am"Update a file" >/dev/null
Likewise.

    git commit -a -m "Update a file"

The remainder of the file invites the same set of questions and
comments you see for dh_diff_test() above, so I won't repeat them.

Thanks.
thanks for the feedback.

Re: [PATCH v2 1/3] diff-highlight: add some tests.

From: Jeff King <hidden>
Date: 2016-08-19 14:52:17

On Fri, Aug 19, 2016 at 07:42:35AM -0700, Brian Henderson wrote:
quoted
quoted
+# PERL is required, but assumed to be present, although not necessarily modern
+# some tests require 5.8
+test_expect_success PERL 'name' 'true'
If the platform lacks PERL prerequisite, this will simply be
skipped, and if the platform has it, it will always succeed.

I am not sure what you are trying to achieve by having this line
here.
I originally didn't have this line, and my comment was referring to the
t/README which says

   Even without the PERL prerequisite, tests can assume there is a
   usable perl interpreter at $PERL_PATH, though it need not be
   particularly modern.

There is current functionality in diff-highlight which requires at least
perl 5.8 (the utf8 functions). I was going to add a test for this as
well, but I'm not super comfy with multibyte chars.
Yeah, I'd agree this test would want the PERL prereq. It is not just
using perl for one-liners in support of the script; it is testing major
perl functionality that should be skipped if we do not have a modern
perl available.
Eric recommended adding this line, what do you think?

would `test_set_prereq PERL` be better?
test_set_prereq is for telling the test scripts that we _have_ perl, but
what I think this script wants to do is test "do we have perl?" and
abort otherwise. The way to do that is:

  if ! test_have_prereq PERL
  then
	skip_all='skipping diff-highlight tests; perl not available'
	test_done
  fi
quoted
quoted
+test_expect_success 'diff-highlight does not highlight whole line' '
+	dh_test \
+		"aaa\nbbb\nccc\n" \
+		"aaa\n000\nccc\n"
+'
This (at least to me) is desired. See comment for `sub
is_pair_interesting`
Yeah, that is an intentional behavior, and makes sense to test.
quoted
quoted
+test_expect_success 'diff-highlight does not highlight mismatched hunk size' '
+	dh_test \
+		"aaa\nbbb\n" \
+		"aaa\nb0b\nccc\n"
+'
This is undesired behavior, but currently implemented for simplicity,
see `sub show_hunk`

Do they need comments or something?
Undesired behavior should generally not be tested for. It just makes
life harder for somebody when they make a change that violates it, and
they have to figure out "oh, but it's _good_ that I changed that, the
tests were wrong" (or more likely "I didn't fix it, but it's just broken
in a different way, and neither is preferable").

If you want to document known shortcomings, the best thing to do is show
what you'd _like_ to have happen, and mark it as test_expect_failure;
the test scripts show this as a known-breakage, and somebody later who
fixes it can flip the "failure" to "success".

-Peff

Re: [PATCH v2 1/3] diff-highlight: add some tests.

From: Brian Henderson <hidden>
Date: 2016-08-19 15:15:03

On Fri, Aug 19, 2016 at 10:51:23AM -0400, Jeff King wrote:
On Fri, Aug 19, 2016 at 07:42:35AM -0700, Brian Henderson wrote:
quoted
quoted
quoted
+# PERL is required, but assumed to be present, although not necessarily modern
+# some tests require 5.8
+test_expect_success PERL 'name' 'true'
If the platform lacks PERL prerequisite, this will simply be
skipped, and if the platform has it, it will always succeed.

I am not sure what you are trying to achieve by having this line
here.
I originally didn't have this line, and my comment was referring to the
t/README which says

   Even without the PERL prerequisite, tests can assume there is a
   usable perl interpreter at $PERL_PATH, though it need not be
   particularly modern.

There is current functionality in diff-highlight which requires at least
perl 5.8 (the utf8 functions). I was going to add a test for this as
well, but I'm not super comfy with multibyte chars.
Yeah, I'd agree this test would want the PERL prereq. It is not just
using perl for one-liners in support of the script; it is testing major
perl functionality that should be skipped if we do not have a modern
perl available.
quoted
Eric recommended adding this line, what do you think?

would `test_set_prereq PERL` be better?
test_set_prereq is for telling the test scripts that we _have_ perl, but
what I think this script wants to do is test "do we have perl?" and
abort otherwise. The way to do that is:

  if ! test_have_prereq PERL
  then
	skip_all='skipping diff-highlight tests; perl not available'
	test_done
  fi
quoted
quoted
quoted
+test_expect_success 'diff-highlight does not highlight whole line' '
+	dh_test \
+		"aaa\nbbb\nccc\n" \
+		"aaa\n000\nccc\n"
+'
This (at least to me) is desired. See comment for `sub
is_pair_interesting`
Yeah, that is an intentional behavior, and makes sense to test.
quoted
quoted
quoted
+test_expect_success 'diff-highlight does not highlight mismatched hunk size' '
+	dh_test \
+		"aaa\nbbb\n" \
+		"aaa\nb0b\nccc\n"
+'
This is undesired behavior, but currently implemented for simplicity,
see `sub show_hunk`

Do they need comments or something?
Undesired behavior should generally not be tested for. It just makes
life harder for somebody when they make a change that violates it, and
they have to figure out "oh, but it's _good_ that I changed that, the
tests were wrong" (or more likely "I didn't fix it, but it's just broken
in a different way, and neither is preferable").

If you want to document known shortcomings, the best thing to do is show
what you'd _like_ to have happen, and mark it as test_expect_failure;
the test scripts show this as a known-breakage, and somebody later who
fixes it can flip the "failure" to "success".

-Peff
all very helpful, thanks!

[PATCH v3 2/3] diff-highlight: add failing test for handling --graph output.

From: Brian Henderson <hidden>
Date: 2016-08-19 17:08:54

Signed-off-by: Brian Henderson <redacted>
---
 contrib/diff-highlight/t/t9400-diff-highlight.sh | 54 ++++++++++++++++++++++++
 1 file changed, 54 insertions(+)
diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh
index 6b8a461..3b3c831 100755
--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh
+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
@@ -85,10 +85,50 @@ dh_commit_test () {
 	test_cmp commit.exp commit.act
 }
 
+# dh_test_setup_history takes a series (3) of changes and generates a contrived graph
+# of commits and merges
+dh_test_setup_history () {
+	a="$1" b="$2" c="$3"
+
+	printf "$a" >file &&
+	git add file &&
+	git commit -m"Add a file" &&
+
+	printf "$b" >file &&
+	git commit -am"Update a file" &&
+
+	git checkout -b branch &&
+	printf "$c" >file &&
+	git commit -am"Update a file on branch" &&
+
+	git checkout master &&
+	printf "$a" >file &&
+	git commit -am"Update a file again" &&
+
+	git checkout branch &&
+	printf "$b" >file &&
+	git commit -am"Update a file similar to master" &&
+
+	git merge master &&
+	git checkout master &&
+	git merge branch --no-ff
+}
+
+
 test_chomp_eof () {
 	"$PERL_PATH" -pe 'chomp if eof'
 }
 
+left_trim () {
+	"$PERL_PATH" -pe 's/^\s+//'
+}
+
+trim_graph_el () {
+	# 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' '
 	dh_test \
 		"aaa\nbbb\nccc\n" \
@@ -145,6 +185,20 @@ 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 \
+		"aaa\nbbb\nccc\n" \
+		"aaa\n0bb\nccc\n" \
+		"aaa\nb0b\nccc\n" &&
+
+	# 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_el is not perfect
+	git log -p --topo-order | "$DIFF_HIGHLIGHT" | left_trim >graph.exp &&
+	git log -p --graph | "$DIFF_HIGHLIGHT" | trim_graph_el | left_trim >graph.act &&
+	test_cmp graph.exp graph.act
+'
+
 test_done
 
 # vim: set noet
-- 
2.9.0

[PATCH v3 0/3] diff-highlight: add support for git log --graph output.

From: Brian Henderson <hidden>
Date: 2016-08-19 17:08:59

I cleaned up the graph test, hopefully it's better.

Brian Henderson (3):
  diff-highlight: add some tests.
  diff-highlight: add failing test for handling --graph output.
  diff-highlight: add support for --graph output.

 contrib/diff-highlight/Makefile                  |   5 +
 contrib/diff-highlight/diff-highlight            |  19 ++-
 contrib/diff-highlight/t/Makefile                |  22 +++
 contrib/diff-highlight/t/t9400-diff-highlight.sh | 204 +++++++++++++++++++++++
 4 files changed, 244 insertions(+), 6 deletions(-)
 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

-- 
2.9.0

[PATCH v3 1/3] diff-highlight: add some tests.

From: Brian Henderson <hidden>
Date: 2016-08-19 17:09:02

Signed-off-by: Brian Henderson <redacted>
---
 contrib/diff-highlight/Makefile                  |   5 +
 contrib/diff-highlight/t/Makefile                |  22 ++++
 contrib/diff-highlight/t/t9400-diff-highlight.sh | 150 +++++++++++++++++++++++
 3 files changed, 177 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 0000000..9018724
--- /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 0000000..5ff5275
--- /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 0000000..6b8a461
--- /dev/null
+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
@@ -0,0 +1,150 @@
+#!/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="\033[7m"	# white
+CR="\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 1) some file data, 2) some
+# change of the file data, creates a diff and commit of the changes and passes
+# that through diff-highlight. The optional 3rd parameter is the expected
+# output of diff-highlight minus the diff/commit header. Don't include a 3rd
+# parameter if diff-highlight is stupposed to leave the input unmodified.
+dh_test () {
+	dh_diff_test "$@" &&
+	dh_commit_test "$@"
+}
+
+# see dh_test for usage
+dh_diff_test () {
+	a="$1" b="$2"
+
+	printf "$a" >file
+	git add file
+
+	printf "$b" >file
+	git diff file >diff.raw
+
+	if test $# -ge 3
+	then
+		# Add the diff header to the expected file
+		# we remove the trailing newline to make the test a little more readable
+		# this means $3 should start with a newline
+		head -n5 diff.raw | test_chomp_eof >diff.exp
+		printf "$3" >>diff.exp
+	else
+		cat diff.raw >diff.exp
+	fi
+
+	"$DIFF_HIGHLIGHT" <diff.raw >diff.act &&
+	# check that at least one of the files is not empty (any of the above
+	# commands could have failed resulting in an empty file)
+	test -s diff.act &&
+	test_cmp diff.exp diff.act
+}
+
+# see dh_test for usage
+dh_commit_test () {
+	a="$1" b="$2"
+
+	printf "$a" >file
+	git add file
+	git commit -m "Add a file" >/dev/null
+
+	printf "$b" >file
+	git commit -am "Update a file" >/dev/null
+
+	git show >commit.raw
+
+	if test $# -ge 3
+	then
+		# same as dh_diff_test
+		head -n11 commit.raw | test_chomp_eof >commit.exp
+		printf "$3" >>commit.exp
+	else
+		cat commit.raw >commit.exp
+	fi
+
+	"$DIFF_HIGHLIGHT" <commit.raw >commit.act &&
+	# check that at least one of the files is not empty (any of the above
+	# commands could have failed resulting in an empty file)
+	test -s commit.act &&
+	test_cmp commit.exp commit.act
+}
+
+test_chomp_eof () {
+	"$PERL_PATH" -pe 'chomp if eof'
+}
+
+test_expect_success 'diff-highlight highlights the beginning of a line' '
+	dh_test \
+		"aaa\nbbb\nccc\n" \
+		"aaa\n0bb\nccc\n" \
+"
+ aaa
+-${CW}b${CR}bb
++${CW}0${CR}bb
+ ccc
+"
+'
+
+test_expect_success 'diff-highlight highlights the end of a line' '
+	dh_test \
+		"aaa\nbbb\nccc\n" \
+		"aaa\nbb0\nccc\n" \
+"
+ aaa
+-bb${CW}b${CR}
++bb${CW}0${CR}
+ ccc
+"
+'
+
+test_expect_success 'diff-highlight highlights the middle of a line' '
+	dh_test \
+		"aaa\nbbb\nccc\n" \
+		"aaa\nb0b\nccc\n" \
+"
+ aaa
+-b${CW}b${CR}b
++b${CW}0${CR}b
+ ccc
+"
+'
+
+test_expect_success 'diff-highlight does not highlight whole line' '
+	dh_test \
+		"aaa\nbbb\nccc\n" \
+		"aaa\n000\nccc\n"
+'
+
+test_expect_failure 'diff-highlight highlights mismatched hunk size' '
+	dh_test \
+		"aaa\nbbb\n" \
+		"aaa\nb0b\nccc\n" \
+"
+ aaa
+-b${CW}b${CR}b
++b${CW}0${CR}b
++ccc
+"
+'
+
+# TODO add multi-byte test
+
+test_done
+
+# vim: set noet
-- 
2.9.0

[PATCH v3 3/3] diff-highlight: add support for --graph output.

From: Brian Henderson <hidden>
Date: 2016-08-19 17:09:26

Signed-off-by: Brian Henderson <redacted>
---
 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 ffefc31..9280c88 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 3b3c831..b88174e 100755
--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh
+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
@@ -185,7 +185,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 \
 		"aaa\nbbb\nccc\n" \
 		"aaa\n0bb\nccc\n" \
-- 
2.9.0
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help