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

2016-08-30 Thread Brian Henderson 
Signed-off-by: Brian Henderson <henderson...@gmail.com>
---
 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 54e11fe..e42232d 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
-- 
2.9.3

[PATCH v4 0/3] diff-highlight: add support for --graph option

2016-08-30 Thread Brian Henderson 
On Mon, Aug 29, 2016 at 02:37:46PM -0700, Junio C Hamano wrote:
> Brian Henderson <henderson...@gmail.com> writes:
>
> > How does this look?
> >
> > Drawing the graph helped me a lot in figuring out what I was
> > actually testing. thanks!
>
> Yeah, I also am pleased to see the picture of what is being tested
> in the test script.
>
> With your sign-off, they would have been almost perfect ;-).

doh. fixed.

I left the subject as v4, probably mostly because I have this weird aversion to
increasing version numbers :) but I justified it by thinking that the actual
patch set isn't changing, I just added the sign-off (and updated the commit
messages per Jeff.) Hope that's ok.

thanks everyone for all your help!


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 | 223 +++
 4 files changed, 263 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.3

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

2016-08-30 Thread Brian Henderson 
Signed-off-by: Brian Henderson <henderson...@gmail.com>
---
 contrib/diff-highlight/t/t9400-diff-highlight.sh | 60 
 1 file changed, 60 insertions(+)

diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh 
b/contrib/diff-highlight/t/t9400-diff-highlight.sh
index 7c303f7..54e11fe 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 -am "A" &&
+
+   git checkout master &&
+   cat file2 >file &&
+   git commit -am "E" &&
+
+   cat file3 >file &&
+   git commit -am "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,15 @@ 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
-- 
2.9.3

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

2016-08-30 Thread Brian Henderson 
Signed-off-by: Brian Henderson <henderson...@gmail.com>
---
 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 000..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 000..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 000..7c303f7
--- /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 -am "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 &&
+

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

2016-08-29 Thread Brian Henderson 
---
 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 54e11fe..e42232d 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
-- 
2.9.3

[PATCH v4 0/3] diff-highlight: add support for --graph option

2016-08-29 Thread Brian Henderson 
How does this look?

Drawing the graph helped me a lot in figuring out what I was actually testing. 
thanks!

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 | 223 +++
 4 files changed, 263 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.3

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

2016-08-29 Thread Brian Henderson 
---
 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 000..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 000..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 000..7c303f7
--- /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 -am "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 &&
+

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

2016-08-29 Thread Brian Henderson 
---
 contrib/diff-highlight/t/t9400-diff-highlight.sh | 60 
 1 file changed, 60 insertions(+)

diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh 
b/contrib/diff-highlight/t/t9400-diff-highlight.sh
index 7c303f7..54e11fe 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 -am "A" &&
+
+   git checkout master &&
+   cat file2 >file &&
+   git commit -am "E" &&
+
+   cat file3 >file &&
+   git commit -am "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,15 @@ 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
-- 
2.9.3

[PATCH] diff-highlight: add some tests.

2016-08-22 Thread Brian Henderson 
Jeff, I love your idea. how's this looking?

Junio, I wasn't meaning to be stubborn, although definitely a fault of mine. I
understand a lot better now, thanks for your patience.

---
 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 000..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 000..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 000..7c303f7
--- /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 -am "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
+'
+

[PATCH] diff-highlight: add some tests.

2016-08-19 Thread Brian Henderson 
Junio, how does this look?

Signed-off-by: Brian Henderson <henderson...@gmail.com>
---
 contrib/diff-highlight/Makefile  |   5 +
 contrib/diff-highlight/t/Makefile|  22 
 contrib/diff-highlight/t/t9400-diff-highlight.sh | 122 +++
 3 files changed, 149 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 000..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 000..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 000..3c04116
--- /dev/null
+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
@@ -0,0 +1,122 @@
+#!/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. This parameter is given directly to printf as the format
+# string (in order to properly handle ascii escape codes; CW, CR), so any '%'
+# need to be doubled to protect it.
+# Don't include a 3rd parameter if diff-highlight is supposed to leave the
+# input unmodified.
+# For convienence, the 3rd parameter can begin with a newline which will be
+# stripped.
+dh_test () {
+   a="$1" b="$2" &&
+
+   {
+   printf "$a" >file &&
+   git add file &&
+   git commit -m "Add a file" &&
+
+   printf "$b" >file &&
+   git diff file >diff.raw &&
+   git commit -am "Update a file" &&
+   git show >commit.raw
+   } >/dev/null &&
+
+   if test $# -ge 3
+   then
+   # strip optional beginning newline
+   printf "$3" | perl -pe 's/^\n//'
+   else
+   test_strip_patch_header diff.raw
+   fi >patch.exp &&
+
+   "$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 -e '1,/^@@/d' "$@"
+}
+
+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

--
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

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

2016-08-19 Thread Brian Henderson 
On Fri, Aug 19, 2016 at 11:10:55AM -0700, Junio C Hamano wrote:
> 
> > +# vim: set noet
> 
> We tend to avoid cluttering the source with editor specific insns
> like this.

oops.

Anyone have any suggestions for project level vim settings?
--
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

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

2016-08-19 Thread Brian Henderson 
Signed-off-by: Brian Henderson <henderson...@gmail.com>
---
 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

--
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

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

2016-08-19 Thread Brian Henderson 
Signed-off-by: Brian Henderson <henderson...@gmail.com>
---
 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

--
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

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

2016-08-19 Thread Brian Henderson 
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

--
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

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

2016-08-19 Thread Brian Henderson 
Signed-off-by: Brian Henderson <henderson...@gmail.com>
---
 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 000..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 000..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 000..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.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.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\n

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

2016-08-19 Thread Brian Henderson 
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:
> 
> > > > +# 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
> 
> > > > +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.
> 
> > > > +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!
--
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

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

2016-08-19 Thread Brian Henderson 
On Wed, Aug 17, 2016 at 12:09:25PM -0700, Junio C Hamano wrote:
> Brian Henderson <henderson...@gmail.com> writes:



> 
> > +
> > +# 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?

> 
> > +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.
> 
> > +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?



> 
> > +   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.

> 
> > +   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.

thanks for the feedback.
--
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

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

2016-08-17 Thread Brian Henderson 
Signed-off-by: Brian Henderson <henderson...@gmail.com>
---
 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

diff --git a/contrib/diff-highlight/Makefile b/contrib/diff-highlight/Makefile
new file mode 100644
index 000..b866259
--- /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 000..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 000..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'
+
+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_success 'diff-highlight does not highlight mismatched hunk size' '
+   dh_test \
+   "aaa\nbbb\n" \
+   "aaa\nb0b\nccc\n"
+'
+
+# TODO add multi-byte test
+
+test_done
diff --git a/contrib/diff-highlight/t/test-diff-highlight.sh 
b/contrib/diff-highlight/t/test-diff-highlight.sh
new file mode 100644
index 000..38323e8
--- /dev/null
+++ b/contrib/diff-highlight/t/test-diff-highlight.sh
@@ -0,0 +1,69 @@
+#!/bin/sh
+
+CURR_DIR=$(pwd)
+TEST_OUTPUT_DIRECTORY=$(pwd)
+TEST_DIRECTORY="$CURR_DIR"/../../../t
+DIFF_HIGHLIGHT="$CURR_DIR"/../diff-highlight
+
+CW="\033[7m"
+CR="\033[27m"
+
+export TEST_OUTPUT_DIRECTORY TEST_DIRECTORY CW CR
+
+dh_test() {
+   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
+   printf "$3" >>diff.exp
+   else
+   cat diff.raw >diff.exp
+   fi
+
+   diff.act &&
+   test -s diff.act &&
+   diff diff.exp diff.act
+}
+
+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 "$#" = 3
+   then
+   # remove last newline
+   head -n11 commit.raw | test_chomp_eof >commit.exp
+   printf "$3" >>commit.exp
+   else
+   cat commit.raw >commit.exp
+   fi
+
+   commit.act &&
+   test -s commit.act &&
+   test_cmp commit.exp commit.act
+}
+
+test_chomp_eof() {
+   perl -pe 'chomp if eof'
+}
-- 
2.9.0

--
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

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

2016-08-17 Thread Brian Henderson 
Signed-off-by: Brian Henderson <henderson...@gmail.com>
---
 contrib/diff-highlight/t/t9400-diff-highlight.sh | 13 +++
 contrib/diff-highlight/t/test-diff-highlight.sh  | 43 
 2 files changed, 56 insertions(+)

diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh 
b/contrib/diff-highlight/t/t9400-diff-highlight.sh
index 8eff178..39707c6 100755
--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh
+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
@@ -59,4 +59,17 @@ test_expect_success 'diff-highlight does not highlight 
mismatched hunk size' '
 
 # TODO add multi-byte test
 
+test_expect_success 'diff-highlight highlights the beginning of a line' '
+   dh_graph_test \
+   "aaa\nbbb\nccc\n" \
+   "aaa\n0bb\nccc\n" \
+   "aaa\nb0b\nccc\n" \
+"
+ aaa
+-${CW}b${CR}bb
++${CW}0${CR}bb
+ ccc
+"
+'
+
 test_done
diff --git a/contrib/diff-highlight/t/test-diff-highlight.sh 
b/contrib/diff-highlight/t/test-diff-highlight.sh
index 38323e8..67f742c 100644
--- a/contrib/diff-highlight/t/test-diff-highlight.sh
+++ b/contrib/diff-highlight/t/test-diff-highlight.sh
@@ -64,6 +64,49 @@ dh_commit_test() {
test_cmp commit.exp commit.act
 }
 
+dh_graph_test() {
+   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
+   } >/dev/null 2>&1
+
+   git log -p --graph --no-merges >graph.raw
+
+   # git log --graph orders the commits different than git log so we hack 
it by
+   # using sed to remove the graph part. We know from other tests, that 
DIFF_HIGHLIGHT
+   # works without the graph, so there should be no diff when running it 
with
+   # and without.
+   graph.exp
+   graph.act &&
+
+   test -s graph.act &&
+   # ignore whitespace since we're using a hacky sed command to remove the 
graph
+   # parts.
+   git diff -b --no-index graph.exp graph.act
+}
+
 test_chomp_eof() {
perl -pe 'chomp if eof'
 }
-- 
2.9.0

--
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

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

2016-08-17 Thread Brian Henderson 
Signed-off-by: Brian Henderson <henderson...@gmail.com>
---
 contrib/diff-highlight/diff-highlight | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/contrib/diff-highlight/diff-highlight 
b/contrib/diff-highlight/diff-highlight
index ffefc31..9364423 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,
@@ -211,8 +215,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*$/;
 }
-- 
2.9.0

--
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

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

2016-08-17 Thread Brian Henderson 
Changes made per Eric.

On Wed, Aug 10, 2016 at 08:56:35AM +, Eric Wong wrote:
> Brian Henderson <henderson...@gmail.com> wrote:
> 
> Hi Brian,
> 
> A few minor portability/style nits below, but contrib/ probably
> (still?) has laxer rules than the rest of git...
> 
> I think we still require Signed-off-by lines in contrib,
> though...

added

> 
> > +++ b/contrib/diff-highlight/t/Makefile
> > @@ -0,0 +1,19 @@
> > +-include ../../../config.mak.autogen
> > +-include ../../../config.mak
> > +
> > +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 "*** $@ ***"; sh $@ $(GIT_TEST_OPTS)
> 
> Probably:
> 
>   @echo "*** $@ ***"; '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)
> 
> like we do in t/Makefile in case we need to override 'sh'.
> 

done, although I basically had to copy the SHELL_PATH_SQ logic from t/Makefile

> > +
> > +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 100644
> > index 000..ca7605f
> > --- /dev/null
> > +++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
> > @@ -0,0 +1,63 @@
> > +#!/bin/sh
> > +#
> > +# Copyright (C) 2016
> 
> IANAL, but I think your name (or who you represent) needs to be
> in the copyright.

diff-highlight doesn't have a copyright, so I just removed it. ok?

> 
> > +
> > +test_description='Test diff-highlight'
> > +
> > +. ./test-diff-highlight.sh
> > +. $TEST_DIRECTORY/test-lib.sh
> 
> TEST_DIRECTORY ought to be quoted since it could contain
> shell-unfriendly chars (we intentionally name 'trash directory'
> this way to trigger errors).

done

> 
> > +
> > +# PERL is required, but assumed to be present, although not necessarily 
> > modern
> > +# some tests require 5.8
> > +
> > +test_expect_success 'diff-highlight highlightes the beginning of a line' '
> 
> You can declare a prereq for PERL::
> 
>   test_expect_success PERL 'name' 'true'

done

> 
> And spelling: "highlights" (there's other instances of this, too)

oops, thanks

> 
> > +  dh_test \
> > +"aaa\nbbb\nccc\n" \
> > +"aaa\n0bb\nccc\n" \
> > +"
> 
> We use tabs for shell indentation.

done



> > +
> > +dh_diff_test() {
> > +  local a="$1" b="$2"
> 
> "local" is not a portable construct.  It's common for
> Debian/Ubuntu systems to use dash as /bin/sh instead of bash;
> (dash is faster, and mostly sticks to POSIX)
> 
> The "devscripts" package in Debian/Ubuntu-based systems has a
> handy "checkbashisms" tool for checking portability of shell
> scripts.

checkbashisms didn't output anything, and I found other instances of local in 
some tests. but I removed anyway.

> 
> > +
> > +  printf "$a" > file
> > +  git add file
> > +
> > +  printf "$b" > file
> > +
> > +  git diff file > diff.raw
> 
> Commands should be '&&'-chained here since any of them could fail
> ("git add"/printf/etc could run out of space or fail on disk errors)

I wasn't totally sure what to do here. I added some checks for empty files and
&& chained them with the last command to verify I wasn't diffing 2 empty files.
Hope that is sufficient.

> 
> Also, our redirect style is:  command >file
> without a space between '>' and destination.

done

> 
> See Documentation/CodingGuidelines for more details.
> Unfortunately, the reasoning is not explained for '>NOSPACE'
> and I'm not sure exactly why, either...
> 
> > +  if test "$#" = 3

done

> 
> Better to use -eq for numeric comparisons: test $# -eq 3
> Quoting $# doesn't seem necessary unless your shell is
> hopelessly buggy :)
> 
> > +  then
> > +# remove last newline
> > +head -n5 diff.raw | head -c -1 > diff.act
> 
> "head -c" isn't portable, fortunately Jeff hoisted it out for
> use as test_copy_bytes in commit 48860819e8026
> https://public-inbox.org/git/20160630090753.ga17...@sigill.intra.peff.net/

It didn't look like his version supported a negative number, so I rolled my own.

> 
> > +printf "$3" >> di

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

2016-08-15 Thread Brian Henderson 
On Wed, Aug 10, 2016 at 08:56:35AM +, Eric Wong wrote:



> Typically, we expect a reroll in a few days, and I guess there's
> no rush (so you can squash your comment patch in addressing
> Junio's concern into 3/3).
> 
> Thanks.

thanks, (slowly) working on an update.
--
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

[PATCH] diff-highlight: Add comment for our assumption about --graph output.

2016-08-04 Thread Brian Henderson 
---
 contrib/diff-highlight/diff-highlight | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/contrib/diff-highlight/diff-highlight 
b/contrib/diff-highlight/diff-highlight
index ec31356..9364423 100755
--- a/contrib/diff-highlight/diff-highlight
+++ b/contrib/diff-highlight/diff-highlight
@@ -20,6 +20,9 @@ my @NEW_HIGHLIGHT = (
 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;
-- 
2.9.0

--
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

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

2016-07-30 Thread Brian Henderson 
---
 contrib/diff-highlight/Makefile  |  5 ++
 contrib/diff-highlight/t/Makefile| 19 +++
 contrib/diff-highlight/t/t9400-diff-highlight.sh | 63 ++
 contrib/diff-highlight/t/test-diff-highlight.sh  | 69 
 4 files changed, 156 insertions(+)
 create mode 100644 contrib/diff-highlight/Makefile
 create mode 100644 contrib/diff-highlight/t/Makefile
 create mode 100644 contrib/diff-highlight/t/t9400-diff-highlight.sh
 create mode 100644 contrib/diff-highlight/t/test-diff-highlight.sh

diff --git a/contrib/diff-highlight/Makefile b/contrib/diff-highlight/Makefile
new file mode 100644
index 000..b866259
--- /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 000..ac81fc0
--- /dev/null
+++ b/contrib/diff-highlight/t/Makefile
@@ -0,0 +1,19 @@
+-include ../../../config.mak.autogen
+-include ../../../config.mak
+
+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 "*** $@ ***"; sh $@ $(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 100644
index 000..ca7605f
--- /dev/null
+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
@@ -0,0 +1,63 @@
+#!/bin/sh
+#
+# Copyright (C) 2016
+
+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 'diff-highlight highlightes 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 highlightes 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 highlightes 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_success 'diff-highlight does not highlight mismatched hunk size' '
+  dh_test \
+"aaa\nbbb\n" \
+"aaa\nb0b\nccc\n"
+'
+
+# TODO add multi-byte test
+
+test_done
diff --git a/contrib/diff-highlight/t/test-diff-highlight.sh 
b/contrib/diff-highlight/t/test-diff-highlight.sh
new file mode 100644
index 000..9b26271
--- /dev/null
+++ b/contrib/diff-highlight/t/test-diff-highlight.sh
@@ -0,0 +1,69 @@
+#!/bin/sh
+#
+# Copyright (C) 2016
+
+CURR_DIR=$(pwd)
+TEST_OUTPUT_DIRECTORY=$(pwd)
+TEST_DIRECTORY="$CURR_DIR"/../../../t
+cmd=diff-highlight
+CMD="$CURR_DIR"/../$cmd
+
+CW="\033[7m"
+CR="\033[27m"
+
+export TEST_OUTPUT_DIRECTORY TEST_DIRECTORY CW CR
+
+dh_test() {
+  dh_diff_test "$@"
+  dh_commit_test "$@"
+}
+
+dh_diff_test() {
+  local a="$1" b="$2"
+
+  printf "$a" > file
+  git add file
+
+  printf "$b" > file
+
+  git diff file > diff.raw
+
+  if test "$#" = 3
+  then
+# remove last newline
+head -n5 diff.raw | head -c -1 > diff.act
+printf "$3" >> diff.act
+  else
+cat diff.raw > diff.act
+  fi
+
+  < diff.raw $CMD > diff.exp
+
+  diff diff.exp diff.act
+}
+
+dh_commit_test() {
+  local 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 "$#" = 3
+  then
+# remove last newline
+head -n11 commit.raw | head -c -1 > commit.act
+printf "$3" >> commit.act
+  else
+cat commit.raw > commit.act
+  fi
+
+  < commit.raw $CMD > commit.exp
+
+  diff commit.exp commit.act
+}
-- 
2.9.0

--
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

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

2016-07-30 Thread Brian Henderson 
(resending as thread instead of attachments take 3
I realized that I was adding the To field only to the cover letter and I wasn't
catching that git-send-email wasn't adding it to the other patches. Apologies.)

Hi, I've been working with Jeff King on this patch, but he encouraged me to
email the list.

I recently learned about the diff-highlight tool, and find it very helpful,
however, I frequently use the --graph option to git log which breaks the tool.
This patch looks to fix this.

I wanted to try and add some tests as well, but I was unsure what number to
pick for the second digit. As there were limited tests in the contrib directory
(only t93xx and one t7900) I just chose the next number in the 9xxx range.
Please let me know if I need to change it.

I'm also not super happy about my test case for the graph option. If anyone has
any better ideas, please let me know!

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|  13 +--
 contrib/diff-highlight/t/Makefile|  19 
 contrib/diff-highlight/t/t9400-diff-highlight.sh |  76 
 contrib/diff-highlight/t/test-diff-highlight.sh  | 111 +++
 5 files changed, 218 insertions(+), 6 deletions(-)
 create mode 100644 contrib/diff-highlight/Makefile
 create mode 100644 contrib/diff-highlight/t/Makefile
 create mode 100644 contrib/diff-highlight/t/t9400-diff-highlight.sh
 create mode 100644 contrib/diff-highlight/t/test-diff-highlight.sh

-- 
2.9.0

--
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

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

2016-07-30 Thread Brian Henderson 
---
 contrib/diff-highlight/t/t9400-diff-highlight.sh | 13 
 contrib/diff-highlight/t/test-diff-highlight.sh  | 42 
 2 files changed, 55 insertions(+)

diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh 
b/contrib/diff-highlight/t/t9400-diff-highlight.sh
index ca7605f..2fb4869 100644
--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh
+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
@@ -60,4 +60,17 @@ test_expect_success 'diff-highlight does not highlight 
mismatched hunk size' '
 
 # TODO add multi-byte test
 
+test_expect_success 'diff-highlight highlightes the beginning of a line' '
+  dh_graph_test \
+"aaa\nbbb\nccc\n" \
+"aaa\n0bb\nccc\n" \
+"aaa\nb0b\nccc\n" \
+"
+ aaa
+-${CW}b${CR}bb
++${CW}0${CR}bb
+ ccc
+"
+'
+
 test_done
diff --git a/contrib/diff-highlight/t/test-diff-highlight.sh 
b/contrib/diff-highlight/t/test-diff-highlight.sh
index 9b26271..162a694 100644
--- a/contrib/diff-highlight/t/test-diff-highlight.sh
+++ b/contrib/diff-highlight/t/test-diff-highlight.sh
@@ -67,3 +67,45 @@ dh_commit_test() {
 
   diff commit.exp commit.act
 }
+
+dh_graph_test() {
+  local 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
+  } >/dev/null 2>&1
+
+  git log -p --graph --no-merges > graph.raw
+
+  # git log --graph orders the commits different than git log so we hack it by
+  # using sed to remove the graph part. We know from other tests, that CMD
+  # works without the graph, so there should be no diff when running it with
+  # and without.
+  < graph.raw sed -e 's"^\(*\|| \||/\)\+""' -e 's"^  ""' | $CMD > graph.exp
+  < graph.raw $CMD | sed -e 's"^\(*\|| \||/\)\+""' -e 's"^  ""' > graph.act
+
+  # ignore whitespace since we're using a hacky sed command to remove the graph
+  # parts.
+  diff -b graph.exp graph.act
+}
-- 
2.9.0

--
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

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

2016-07-30 Thread Brian Henderson 
---
 contrib/diff-highlight/diff-highlight | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/contrib/diff-highlight/diff-highlight 
b/contrib/diff-highlight/diff-highlight
index ffefc31..ec31356 100755
--- a/contrib/diff-highlight/diff-highlight
+++ b/contrib/diff-highlight/diff-highlight
@@ -20,6 +20,7 @@ my @NEW_HIGHLIGHT = (
 my $RESET = "\x1b[m";
 my $COLOR = qr/\x1b\[[0-9;]*m/;
 my $BORING = qr/$COLOR|\s/;
+my $GRAPH = qr/$COLOR?\|$COLOR?\s+/;
 
 my @removed;
 my @added;
@@ -32,12 +33,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 +47,7 @@ while (<>) {
@added = ();
 
print;
-   $in_hunk = /^$COLOR*[\@ ]/;
+   $in_hunk = /^$GRAPH*$COLOR*[\@ ]/;
}
 
# Most of the time there is enough output to keep things streaming,
@@ -211,8 +212,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*$/;
 }
-- 
2.9.0

--
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

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

2016-07-29 Thread Brian Henderson 
(resending as thread instead of attachments)

Hi, I've been working with Jeff King on this patch, but he encouraged me to
email the list.

I recently learned about the diff-highlight tool, and find it very helpful,
however, I frequently use the --graph option to git log which breaks the tool.
This patch looks to fix this.

I wanted to try and add some tests as well, but I was unsure what number to
pick for the second digit. As there were limited tests in the contrib directory
(only t93xx and one t7900) I just chose the next number in the 9xxx range.
Please let me know if I need to change it.

I'm also not super happy about my test case for the graph option. If anyone has
any better ideas, please let me know!

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|  13 +--
 contrib/diff-highlight/t/Makefile|  19 
 contrib/diff-highlight/t/t9400-diff-highlight.sh |  76 
 contrib/diff-highlight/t/test-diff-highlight.sh  | 111 +++
 5 files changed, 218 insertions(+), 6 deletions(-)
 create mode 100644 contrib/diff-highlight/Makefile
 create mode 100644 contrib/diff-highlight/t/Makefile
 create mode 100644 contrib/diff-highlight/t/t9400-diff-highlight.sh
 create mode 100644 contrib/diff-highlight/t/test-diff-highlight.sh

-- 
2.9.0

--
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

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

2016-07-28 Thread Brian Henderson 
Hi, I've been working with Jeff King on this patch, but he encouraged me to
email the list.

I recently learned about the diff-highlight tool and find it very helpful,
however, I frequently use the --graph option to git log which breaks the tool.
This patch looks to fix this.

I wanted to try and add some tests as well, but I was unsure what number to
pick for the second digit. As there were limited tests in the contrib directory
(only t93xx and one t7900) I just chose the next number in the 9xxx range.
Please let me know if I need to change it.

I'm also not super happy about my test case for the graph option. If anyone has
any better ideas, please let me know!

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|  13 +--
 contrib/diff-highlight/t/Makefile|  19 
 contrib/diff-highlight/t/t9400-diff-highlight.sh |  76 
 contrib/diff-highlight/t/test-diff-highlight.sh  | 111 +++
 5 files changed, 218 insertions(+), 6 deletions(-)
 create mode 100644 contrib/diff-highlight/Makefile
 create mode 100644 contrib/diff-highlight/t/Makefile
 create mode 100644 contrib/diff-highlight/t/t9400-diff-highlight.sh
 create mode 100644 contrib/diff-highlight/t/test-diff-highlight.sh

-- 
2.9.0

>From f2468205f5f43b17bfd0398959a1ae66588a8f0d Mon Sep 17 00:00:00 2001
From: Brian Henderson <henderson...@gmail.com>
Date: Thu, 21 Jul 2016 09:43:54 -0700
Subject: [PATCH 1/3] diff-highlight: add some tests.

---
 contrib/diff-highlight/Makefile  |  5 ++
 contrib/diff-highlight/t/Makefile| 19 +++
 contrib/diff-highlight/t/t9400-diff-highlight.sh | 63 ++
 contrib/diff-highlight/t/test-diff-highlight.sh  | 69 
 4 files changed, 156 insertions(+)
 create mode 100644 contrib/diff-highlight/Makefile
 create mode 100644 contrib/diff-highlight/t/Makefile
 create mode 100644 contrib/diff-highlight/t/t9400-diff-highlight.sh
 create mode 100644 contrib/diff-highlight/t/test-diff-highlight.sh

diff --git a/contrib/diff-highlight/Makefile b/contrib/diff-highlight/Makefile
new file mode 100644
index 000..b866259
--- /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 000..ac81fc0
--- /dev/null
+++ b/contrib/diff-highlight/t/Makefile
@@ -0,0 +1,19 @@
+-include ../../../config.mak.autogen
+-include ../../../config.mak
+
+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 "*** $@ ***"; sh $@ $(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 100644
index 000..ca7605f
--- /dev/null
+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
@@ -0,0 +1,63 @@
+#!/bin/sh
+#
+# Copyright (C) 2016
+
+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 'diff-highlight highlightes 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 highlightes 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 highlightes 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_success 'diff-highlight does not highlight mismatched hunk size' '
+  dh_test \
+"aaa\nbbb\n" \
+"aaa\nb0b\nccc\n"
+'
+
+# TODO add multi-byte test
+
+test_done
diff --git a/contrib/diff-highlight/t/test-diff-highlight.sh b/contrib/diff-highlight/t/test-diff-highlight.sh
new file mode 100644
index 000..9b26271
--- /dev/null
+++ b/contrib/diff-highlight/t/test-diff-highlight.sh
@@ -0,0 +1,69 @@
+#!/bin/sh
+#
+# Copyright (C) 2016
+
+CURR_DIR=$(pwd)
+TEST_OUTPUT_DIRECTORY=$(pwd)
+TEST_DIRECTORY="$CURR_D