{"thread":{"id":"43885","subject":"[PATCH v3 2/3] diff-highlight: add failing test for handling --graph output.","startedAt":"2016-08-19T17:08:54Z","lastAt":"2016-08-31T05:05:47Z","messageCount":30,"participants":["Brian Henderson","Junio C Hamano","Eric Wong","Jeff King"],"isPatch":true,"patchVersion":3,"patchTotal":3},"messages":[{"id":"299720","messageId":"20160819170812.1676-3-henderson.bj@gmail.com","threadId":"43885","inReplyTo":"20160819170812.1676-1-henderson.bj@gmail.com","subject":"[PATCH v3 2/3] diff-highlight: add failing test for handling --graph output.","fromName":"Brian Henderson","fromEmail":"henderson.bj@gmail.com","sentAt":"2016-08-19T17:08:11Z","receivedAt":"2016-08-19T17:08:54Z","isPatch":true,"sender":{"key":"henderson.bj@gmail.com","avatar":"https://avatars.githubusercontent.com/u/26167?v=4"},"body":"Signed-off-by: Brian Henderson \n---\n contrib/diff-highlight/t/t9400-diff-highlight.sh | 54 ++++++++++++++++++++++++\n 1 file changed, 54 insertions(+)\n\ndiff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh\nindex 6b8a461..3b3c831 100755\n--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh\n+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n@@ -85,10 +85,50 @@ dh_commit_test () {\n \ttest_cmp commit.exp commit.act\n }\n \n+# dh_test_setup_history takes a series (3) of changes and generates a contrived graph\n+# of commits and merges\n+dh_test_setup_history () {\n+\ta=\"$1\" b=\"$2\" c=\"$3\"\n+\n+\tprintf \"$a\" >file &&\n+\tgit add file &&\n+\tgit commit -m\"Add a file\" &&\n+\n+\tprintf \"$b\" >file &&\n+\tgit commit -am\"Update a file\" &&\n+\n+\tgit checkout -b branch &&\n+\tprintf \"$c\" >file &&\n+\tgit commit -am\"Update a file on branch\" &&\n+\n+\tgit checkout master &&\n+\tprintf \"$a\" >file &&\n+\tgit commit -am\"Update a file again\" &&\n+\n+\tgit checkout branch &&\n+\tprintf \"$b\" >file &&\n+\tgit commit -am\"Update a file similar to master\" &&\n+\n+\tgit merge master &&\n+\tgit checkout master &&\n+\tgit merge branch --no-ff\n+}\n+\n+\n test_chomp_eof () {\n \t\"$PERL_PATH\" -pe 'chomp if eof'\n }\n \n+left_trim () {\n+\t\"$PERL_PATH\" -pe 's/^\\s+//'\n+}\n+\n+trim_graph_el () {\n+\t# graphs start with * or |\n+\t# followed by a space or / or \\\n+\t\"$PERL_PATH\" -pe 's@^((\\*|\\|)( |/|\\\\))+@@'\n+}\n+\n test_expect_success 'diff-highlight highlights the beginning of a line' '\n \tdh_test \\\n \t\t\"aaa\\nbbb\\nccc\\n\" \\\n@@ -145,6 +185,20 @@ test_expect_failure 'diff-highlight highlights mismatched hunk size' '\n \n # TODO add multi-byte test\n \n+test_expect_failure 'diff-highlight works with the --graph option' '\n+\tdh_test_setup_history \\\n+\t\t\"aaa\\nbbb\\nccc\\n\" \\\n+\t\t\"aaa\\n0bb\\nccc\\n\" \\\n+\t\t\"aaa\\nb0b\\nccc\\n\" &&\n+\n+\t# topo-order so that the order of the commits is the same as with --graph\n+\t# trim graph elements so we can do a diff\n+\t# trim leading space because our trim_graph_el is not perfect\n+\tgit log -p --topo-order | \"$DIFF_HIGHLIGHT\" | left_trim >graph.exp &&\n+\tgit log -p --graph | \"$DIFF_HIGHLIGHT\" | trim_graph_el | left_trim >graph.act &&\n+\ttest_cmp graph.exp graph.act\n+'\n+\n test_done\n \n # vim: set noet\n-- \n2.9.0\n\n"},{"id":"299721","messageId":"20160819170812.1676-1-henderson.bj@gmail.com","threadId":"43885","inReplyTo":"20160819145123.73hf7ffysy53l3kz@sigill.intra.peff.net","subject":"[PATCH v3 0/3] diff-highlight: add support for git log --graph output.","fromName":"Brian Henderson","fromEmail":"henderson.bj@gmail.com","sentAt":"2016-08-19T17:08:09Z","receivedAt":"2016-08-19T17:08:59Z","isPatch":true,"sender":{"key":"henderson.bj@gmail.com","avatar":"https://avatars.githubusercontent.com/u/26167?v=4"},"body":"I cleaned up the graph test, hopefully it's better.\n\nBrian Henderson (3):\n diff-highlight: add some tests.\n diff-highlight: add failing test for handling --graph output.\n diff-highlight: add support for --graph output.\n\n contrib/diff-highlight/Makefile | 5 +\n contrib/diff-highlight/diff-highlight | 19 ++-\n contrib/diff-highlight/t/Makefile | 22 +++\n contrib/diff-highlight/t/t9400-diff-highlight.sh | 204 +++++++++++++++++++++++\n 4 files changed, 244 insertions(+), 6 deletions(-)\n create mode 100644 contrib/diff-highlight/Makefile\n create mode 100644 contrib/diff-highlight/t/Makefile\n create mode 100755 contrib/diff-highlight/t/t9400-diff-highlight.sh\n\n-- \n2.9.0\n\n"},{"id":"299722","messageId":"20160819170812.1676-2-henderson.bj@gmail.com","threadId":"43885","inReplyTo":"20160819170812.1676-1-henderson.bj@gmail.com","subject":"[PATCH v3 1/3] diff-highlight: add some tests.","fromName":"Brian Henderson","fromEmail":"henderson.bj@gmail.com","sentAt":"2016-08-19T17:08:10Z","receivedAt":"2016-08-19T17:09:02Z","isPatch":true,"sender":{"key":"henderson.bj@gmail.com","avatar":"https://avatars.githubusercontent.com/u/26167?v=4"},"body":"Signed-off-by: Brian Henderson \n---\n contrib/diff-highlight/Makefile | 5 +\n contrib/diff-highlight/t/Makefile | 22 ++++\n contrib/diff-highlight/t/t9400-diff-highlight.sh | 150 +++++++++++++++++++++++\n 3 files changed, 177 insertions(+)\n create mode 100644 contrib/diff-highlight/Makefile\n create mode 100644 contrib/diff-highlight/t/Makefile\n create mode 100755 contrib/diff-highlight/t/t9400-diff-highlight.sh\n\ndiff --git a/contrib/diff-highlight/Makefile b/contrib/diff-highlight/Makefile\nnew file mode 100644\nindex 0000000..9018724\n--- /dev/null\n+++ b/contrib/diff-highlight/Makefile\n@@ -0,0 +1,5 @@\n+# nothing to build\n+all:\n+\n+test:\n+\t$(MAKE) -C t\ndiff --git a/contrib/diff-highlight/t/Makefile b/contrib/diff-highlight/t/Makefile\nnew file mode 100644\nindex 0000000..5ff5275\n--- /dev/null\n+++ b/contrib/diff-highlight/t/Makefile\n@@ -0,0 +1,22 @@\n+-include ../../../config.mak.autogen\n+-include ../../../config.mak\n+\n+# copied from ../../t/Makefile\n+SHELL_PATH ?= $(SHELL)\n+SHELL_PATH_SQ = $(subst ','\\'',$(SHELL_PATH))\n+T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)\n+\n+all: test\n+test: $(T)\n+\n+.PHONY: help clean all test $(T)\n+\n+help:\n+\t@echo 'Run \"$(MAKE) test\" to launch test scripts'\n+\t@echo 'Run \"$(MAKE) clean\" to remove trash folders'\n+\n+$(T):\n+\t@echo \"*** $@ ***\"; '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)\n+\n+clean:\n+\t$(RM) -r 'trash directory'.*\ndiff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh\nnew file mode 100755\nindex 0000000..6b8a461\n--- /dev/null\n+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n@@ -0,0 +1,150 @@\n+#!/bin/sh\n+\n+test_description='Test diff-highlight'\n+\n+CURR_DIR=$(pwd)\n+TEST_OUTPUT_DIRECTORY=$(pwd)\n+TEST_DIRECTORY=\"$CURR_DIR\"/../../../t\n+DIFF_HIGHLIGHT=\"$CURR_DIR\"/../diff-highlight\n+\n+CW=\"\\033[7m\"\t# white\n+CR=\"\\033[27m\" # reset\n+\n+. \"$TEST_DIRECTORY\"/test-lib.sh\n+\n+if ! test_have_prereq PERL\n+then\n+\tskip_all='skipping diff-highlight tests; perl not available'\n+\ttest_done\n+fi\n+\n+# dh_test is a test helper function which takes 1) some file data, 2) some\n+# change of the file data, creates a diff and commit of the changes and passes\n+# that through diff-highlight. The optional 3rd parameter is the expected\n+# output of diff-highlight minus the diff/commit header. Don't include a 3rd\n+# parameter if diff-highlight is stupposed to leave the input unmodified.\n+dh_test () {\n+\tdh_diff_test \"$@\" &&\n+\tdh_commit_test \"$@\"\n+}\n+\n+# see dh_test for usage\n+dh_diff_test () {\n+\ta=\"$1\" b=\"$2\"\n+\n+\tprintf \"$a\" >file\n+\tgit add file\n+\n+\tprintf \"$b\" >file\n+\tgit diff file >diff.raw\n+\n+\tif test $# -ge 3\n+\tthen\n+\t\t# Add the diff header to the expected file\n+\t\t# we remove the trailing newline to make the test a little more readable\n+\t\t# this means $3 should start with a newline\n+\t\thead -n5 diff.raw | test_chomp_eof >diff.exp\n+\t\tprintf \"$3\" >>diff.exp\n+\telse\n+\t\tcat diff.raw >diff.exp\n+\tfi\n+\n+\t\"$DIFF_HIGHLIGHT\" diff.act &&\n+\t# check that at least one of the files is not empty (any of the above\n+\t# commands could have failed resulting in an empty file)\n+\ttest -s diff.act &&\n+\ttest_cmp diff.exp diff.act\n+}\n+\n+# see dh_test for usage\n+dh_commit_test () {\n+\ta=\"$1\" b=\"$2\"\n+\n+\tprintf \"$a\" >file\n+\tgit add file\n+\tgit commit -m \"Add a file\" >/dev/null\n+\n+\tprintf \"$b\" >file\n+\tgit commit -am \"Update a file\" >/dev/null\n+\n+\tgit show >commit.raw\n+\n+\tif test $# -ge 3\n+\tthen\n+\t\t# same as dh_diff_test\n+\t\thead -n11 commit.raw | test_chomp_eof >commit.exp\n+\t\tprintf \"$3\" >>commit.exp\n+\telse\n+\t\tcat commit.raw >commit.exp\n+\tfi\n+\n+\t\"$DIFF_HIGHLIGHT\" commit.act &&\n+\t# check that at least one of the files is not empty (any of the above\n+\t# commands could have failed resulting in an empty file)\n+\ttest -s commit.act &&\n+\ttest_cmp commit.exp commit.act\n+}\n+\n+test_chomp_eof () {\n+\t\"$PERL_PATH\" -pe 'chomp if eof'\n+}\n+\n+test_expect_success 'diff-highlight highlights the beginning of a line' '\n+\tdh_test \\\n+\t\t\"aaa\\nbbb\\nccc\\n\" \\\n+\t\t\"aaa\\n0bb\\nccc\\n\" \\\n+\"\n+ aaa\n+-${CW}b${CR}bb\n++${CW}0${CR}bb\n+ ccc\n+\"\n+'\n+\n+test_expect_success 'diff-highlight highlights the end of a line' '\n+\tdh_test \\\n+\t\t\"aaa\\nbbb\\nccc\\n\" \\\n+\t\t\"aaa\\nbb0\\nccc\\n\" \\\n+\"\n+ aaa\n+-bb${CW}b${CR}\n++bb${CW}0${CR}\n+ ccc\n+\"\n+'\n+\n+test_expect_success 'diff-highlight highlights the middle of a line' '\n+\tdh_test \\\n+\t\t\"aaa\\nbbb\\nccc\\n\" \\\n+\t\t\"aaa\\nb0b\\nccc\\n\" \\\n+\"\n+ aaa\n+-b${CW}b${CR}b\n++b${CW}0${CR}b\n+ ccc\n+\"\n+'\n+\n+test_expect_success 'diff-highlight does not highlight whole line' '\n+\tdh_test \\\n+\t\t\"aaa\\nbbb\\nccc\\n\" \\\n+\t\t\"aaa\\n000\\nccc\\n\"\n+'\n+\n+test_expect_failure 'diff-highlight highlights mismatched hunk size' '\n+\tdh_test \\\n+\t\t\"aaa\\nbbb\\n\" \\\n+\t\t\"aaa\\nb0b\\nccc\\n\" \\\n+\"\n+ aaa\n+-b${CW}b${CR}b\n++b${CW}0${CR}b\n++ccc\n+\"\n+'\n+\n+# TODO add multi-byte test\n+\n+test_done\n+\n+# vim: set noet\n-- \n2.9.0\n\n"},{"id":"299723","messageId":"20160819170812.1676-4-henderson.bj@gmail.com","threadId":"43885","inReplyTo":"20160819170812.1676-1-henderson.bj@gmail.com","subject":"[PATCH v3 3/3] diff-highlight: add support for --graph output.","fromName":"Brian Henderson","fromEmail":"henderson.bj@gmail.com","sentAt":"2016-08-19T17:08:12Z","receivedAt":"2016-08-19T17:09:26Z","isPatch":true,"sender":{"key":"henderson.bj@gmail.com","avatar":"https://avatars.githubusercontent.com/u/26167?v=4"},"body":"Signed-off-by: Brian Henderson \n---\n contrib/diff-highlight/diff-highlight | 19 +++++++++++++------\n contrib/diff-highlight/t/t9400-diff-highlight.sh | 2 +-\n 2 files changed, 14 insertions(+), 7 deletions(-)\n\ndiff --git a/contrib/diff-highlight/diff-highlight b/contrib/diff-highlight/diff-highlight\nindex ffefc31..9280c88 100755\n--- a/contrib/diff-highlight/diff-highlight\n+++ b/contrib/diff-highlight/diff-highlight\n@@ -21,6 +21,10 @@ my $RESET = \"\\x1b[m\";\n my $COLOR = qr/\\x1b\\[[0-9;]*m/;\n my $BORING = qr/$COLOR|\\s/;\n \n+# The patch portion of git log -p --graph should only ever have preceding | and\n+# not / or \\ as merge history only shows up on the commit line.\n+my $GRAPH = qr/$COLOR?\\|$COLOR?\\s+/;\n+\n my @removed;\n my @added;\n my $in_hunk;\n@@ -32,12 +36,12 @@ $SIG{PIPE} = 'DEFAULT';\n while (<>) {\n \tif (!$in_hunk) {\n \t\tprint;\n-\t\t$in_hunk = /^$COLOR*\\@/;\n+\t\t$in_hunk = /^$GRAPH*$COLOR*\\@/;\n \t}\n-\telsif (/^$COLOR*-/) {\n+\telsif (/^$GRAPH*$COLOR*-/) {\n \t\tpush @removed, $_;\n \t}\n-\telsif (/^$COLOR*\\+/) {\n+\telsif (/^$GRAPH*$COLOR*\\+/) {\n \t\tpush @added, $_;\n \t}\n \telse {\n@@ -46,7 +50,7 @@ while (<>) {\n \t\t@added = ();\n \n \t\tprint;\n-\t\t$in_hunk = /^$COLOR*[\\@ ]/;\n+\t\t$in_hunk = /^$GRAPH*$COLOR*[\\@ ]/;\n \t}\n \n \t# Most of the time there is enough output to keep things streaming,\n@@ -163,6 +167,9 @@ sub highlight_pair {\n \t}\n }\n \n+# we split either by $COLOR or by character. This has the side effect of\n+# leaving in graph cruft. It works because the graph cruft does not contain \"-\"\n+# or \"+\"\n sub split_line {\n \tlocal $_ = shift;\n \treturn utf8::decode($_) ?\n@@ -211,8 +218,8 @@ sub is_pair_interesting {\n \tmy $suffix_a = join('', @$a[($sa+1)..$#$a]);\n \tmy $suffix_b = join('', @$b[($sb+1)..$#$b]);\n \n-\treturn $prefix_a !~ /^$COLOR*-$BORING*$/ ||\n-\t $prefix_b !~ /^$COLOR*\\+$BORING*$/ ||\n+\treturn $prefix_a !~ /^$GRAPH*$COLOR*-$BORING*$/ ||\n+\t $prefix_b !~ /^$GRAPH*$COLOR*\\+$BORING*$/ ||\n \t $suffix_a !~ /^$BORING*$/ ||\n \t $suffix_b !~ /^$BORING*$/;\n }\ndiff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh\nindex 3b3c831..b88174e 100755\n--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh\n+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n@@ -185,7 +185,7 @@ test_expect_failure 'diff-highlight highlights mismatched hunk size' '\n \n # TODO add multi-byte test\n \n-test_expect_failure 'diff-highlight works with the --graph option' '\n+test_expect_success 'diff-highlight works with the --graph option' '\n \tdh_test_setup_history \\\n \t\t\"aaa\\nbbb\\nccc\\n\" \\\n \t\t\"aaa\\n0bb\\nccc\\n\" \\\n-- \n2.9.0\n\n"},{"id":"299725","messageId":"xmqqh9ag39zk.fsf@gitster.mtv.corp.google.com","threadId":"43885","inReplyTo":"20160819170812.1676-2-henderson.bj@gmail.com","subject":"Re: [PATCH v3 1/3] diff-highlight: add some tests.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-19T18:10:55Z","receivedAt":"2016-08-19T18:11:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\n> +# dh_test is a test helper function which takes 1) some file data, 2) some\n> +# change of the file data, creates a diff and commit of the changes and passes\n> +# that through diff-highlight. The optional 3rd parameter is the expected\n> +# output of diff-highlight minus the diff/commit header. Don't include a 3rd\n> +# parameter if diff-highlight is stupposed to leave the input unmodified.\n\nGood explanation, but it fails to say one crucial thing. The third\nparameter is directly fed to printf and not to \"printf '%s' $3\",\nhence any '%' you expect to see in diff output need to be doubled to\nprotect it.\n\n> +# see dh_test for usage\n> +dh_diff_test () {\n> +\ta=\"$1\" b=\"$2\"\n> +\n> +\tprintf \"$a\" >file\n> +\tgit add file\n> +\n> +\tprintf \"$b\" >file\n> +\tgit diff file >diff.raw\n> +\n> +\tif test $# -ge 3\n> +\tthen\n> +\t\t# Add the diff header to the expected file\n> +\t\t# we remove the trailing newline to make the test a little more readable\n> +\t\t# this means $3 should start with a newline\n> +\t\thead -n5 diff.raw | test_chomp_eof >diff.exp\n> +\t\tprintf \"$3\" >>diff.exp\n> +\telse\n> +\t\tcat diff.raw >diff.exp\n> +\tfi\n\nSorry, but I do not understand why you hardcode -n5 or why you need\nchomp-eof.\n\nI _think_ you are expecting \"git diff file\" to start with\n\n diff --git a/file b/file\n index fffffff..fffffff 100644\n --- a/file\n +++ b/file\n @@ -l,k +m,n @@ comments\n\nand want to grab everything before and including this first hunk\nheader. A more future-proof way to do the \"stop at the first\noccurrence of this\" (this comment applies to -n11 we see below) is\n\n\tsed -e '/^@@/q' diff.raw\n\nAs to chomp-eof, I still do not see the point, especially with the\nnew comment you added: \"this means $3 should start with newline\".\n\nYou are forcing the caller to have an extra empty line at the\nbeginnig ONLY because you do this \"chomp\" thing. Otherwise the\ncallers do not need to.\n\nUnless you actually mean something else by the new comment, e.g. \"I\nthink the callers look prettier if it begins with a newline, so we\ncompensate for that by removing the end of line from what comes\nbefore it\", that is. If \"this means $3 should\" is what it sounds\nlike, i.e. imposing an unnatural constraint on the callers of this\nhelper function, then the helper can do this\n\n\tprintf \"\\n$3\" >>diff.exp\n\nso that the callers do not have to worry about it.\n\nIf \"I think the caller looks prettier if it begins with a newline\"\nis the true motivation, then \"this means $3 should start...\" needs\nto be rephrased. And as to the implementation of the helper, I\nthink\n\n\t{\n\t\tsed -e '/^@@/q' diff.raw\n\t\tprintf \"$3\" | sed -e 1d\n\t} >diff.expected\n\nmay be easier to see what is going on.\n\n> +\n> +\t\"$DIFF_HIGHLIGHT\" diff.act &&\n\n> +\t# check that at least one of the files is not empty (any of the above\n> +\t# commands could have failed resulting in an empty file)\n> +\ttest -s diff.act &&\n\nA more established way in our test suite to ensure that \"any of the\nabove commands could have failed\" is caught is to &&-chain all the\ncommands, like this:\n\n\ta=$1 b=$2 &&\n\tprintf \"$a\" >file && git add file &&\n printf \"$b\" >file && git diff file >diff.raw &&\n if test $# = 3\n then\n \t...\n\telse\n \t...\n\tfi &&\n\n> +test_done\n> +\n> +# vim: set noet\n\nWe tend to avoid cluttering the source with editor specific insns\nlike this.\n\nStepping back a bit. Because you are only interested in making sure\nthe body of the diff match what is expected, it may be a better\nalternative approach to make the comparison _after_ stripping the\nheaders from the actual output with the expected (which you do not\nhave headers for), rather than comparing the expected (with fake\nheaders added in as necessary) and the actual output with headers,\ni.e.\n\n\tgit diff file >diff.raw &&\n\tif test $# = 3\n\tthen\n\t\tprintf \"$3\"\n\telse\n\t sed -e '1,/^@@/d' diff.expected &&\n\t\"$DIFF_HIGHLIGHT\" diff.highlight &&\n\tsed -e '1,/^@@/d' diff.actual &&\n\ttest_cmp diff.expected diff.actual\n\nor something like that.\n\nThat does not address \"is it a good idea to require an empty line at\nthe beginning of $3\"? at all, though. If you want to require a\nblank in front of \"$3\", the \"printf\" needs to be tweaked to somehow\nstrip it.\n"},{"id":"299726","messageId":"xmqqd1l439am.fsf@gitster.mtv.corp.google.com","threadId":"43885","inReplyTo":"20160819170812.1676-3-henderson.bj@gmail.com","subject":"Re: [PATCH v3 2/3] diff-highlight: add failing test for handling --graph output.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-19T18:25:53Z","receivedAt":"2016-08-19T18:26:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brian Henderson writes:\n\n> Signed-off-by: Brian Henderson \n> ---\n> contrib/diff-highlight/t/t9400-diff-highlight.sh | 54 ++++++++++++++++++++++++\n> 1 file changed, 54 insertions(+)\n>\n> diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n> index 6b8a461..3b3c831 100755\n> --- a/contrib/diff-highlight/t/t9400-diff-highlight.sh\n> +++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n> @@ -85,10 +85,50 @@ dh_commit_test () {\n> \ttest_cmp commit.exp commit.act\n> }\n> \n> +# dh_test_setup_history takes a series (3) of changes and generates a contrived graph\n> +# of commits and merges\n\nThe first line look a bit overlong.\n\nIf the graph is truly \"contrived\" as the comment at the beginning\nsays (I haven't formed an opinion), it probably is a good idea to\ndraw a picture upfront.\n\n> +dh_test_setup_history () {\n> +\ta=\"$1\" b=\"$2\" c=\"$3\"\n\nEven though assignment would not fail, it probably is a good idea to\n&&-chain this line, too, for consistency. I.e.\n\n\ta=$1 b=$2 c=$3 &&\n\nNote that RHS of an assignment do not need dq; it does not hurt to\nquote them, though.\n\n> +\tprintf \"$a\" >file &&\n> +\tgit add file &&\n> +\tgit commit -m\"Add a file\" &&\n\nAvoid sticking a short-option to its argument, i.e.\n\n git commit -m \"Add a file\"\n\n> +\tprintf \"$b\" >file &&\n> +\tgit commit -am\"Update a file\" &&\n\nLikewise.\n\n git commit -a -m \"Update a file\".\n\n> +left_trim () {\n> +\t\"$PERL_PATH\" -pe 's/^\\s+//'\n> +}\n> +\n> +trim_graph_el () {\n\nWhat does \"el\" stand for? Not E-lisp ;-)\n\nIf you meant \"elements\" or something, probably that is unnecessary\nand \"trim_graph\" would be sufficient, but you may have meant\nsomething else.\n\n> +\t# graphs start with * or |\n> +\t# followed by a space or / or \\\n> +\t\"$PERL_PATH\" -pe 's@^((\\*|\\|)( |/|\\\\))+@@'\n> +}\n> +\n> test_expect_success 'diff-highlight highlights the beginning of a line' '\n> \tdh_test \\\n> \t\t\"aaa\\nbbb\\nccc\\n\" \\\n> @@ -145,6 +185,20 @@ test_expect_failure 'diff-highlight highlights mismatched hunk size' '\n> \n> # TODO add multi-byte test\n> \n> +test_expect_failure 'diff-highlight works with the --graph option' '\n> +\tdh_test_setup_history \\\n> +\t\t\"aaa\\nbbb\\nccc\\n\" \\\n> +\t\t\"aaa\\n0bb\\nccc\\n\" \\\n> +\t\t\"aaa\\nb0b\\nccc\\n\" &&\n> +\n> +\t# topo-order so that the order of the commits is the same as with --graph\n> +\t# trim graph elements so we can do a diff\n> +\t# trim leading space because our trim_graph_el is not perfect\n> +\tgit log -p --topo-order | \"$DIFF_HIGHLIGHT\" | left_trim >graph.exp &&\n> +\tgit log -p --graph | \"$DIFF_HIGHLIGHT\" | trim_graph_el | left_trim >graph.act &&\n> +\ttest_cmp graph.exp graph.act\n> +'\n> +\n> test_done\n> \n> # vim: set noet\n"},{"id":"299727","messageId":"xmqq8tvs397z.fsf@gitster.mtv.corp.google.com","threadId":"43885","inReplyTo":"20160819170812.1676-4-henderson.bj@gmail.com","subject":"Re: [PATCH v3 3/3] diff-highlight: add support for --graph output.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-19T18:27:28Z","receivedAt":"2016-08-19T18:27:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brian Henderson writes:\n\n> diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n> index 3b3c831..b88174e 100755\n> --- a/contrib/diff-highlight/t/t9400-diff-highlight.sh\n> +++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n> @@ -185,7 +185,7 @@ test_expect_failure 'diff-highlight highlights mismatched hunk size' '\n> \n> # TODO add multi-byte test\n> \n> -test_expect_failure 'diff-highlight works with the --graph option' '\n> +test_expect_success 'diff-highlight works with the --graph option' '\n> \tdh_test_setup_history \\\n> \t\t\"aaa\\nbbb\\nccc\\n\" \\\n> \t\t\"aaa\\n0bb\\nccc\\n\" \\\n\nYup, the standard \"describe what we want and document a bug to be\nfixed with _failure, and flip it to _success when you fix the bug\"\npattern makes it very easy to understand what is going on.\n\n"},{"id":"299729","messageId":"20160819193045.GA9262@tci.corp.yp.com","threadId":"43885","inReplyTo":"xmqqh9ag39zk.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 1/3] diff-highlight: add some tests.","fromName":"Brian Henderson","fromEmail":"henderson.bj@gmail.com","sentAt":"2016-08-19T19:30:45Z","receivedAt":"2016-08-19T19:30:52Z","isPatch":true,"sender":{"key":"henderson.bj@gmail.com","avatar":"https://avatars.githubusercontent.com/u/26167?v=4"},"body":"On Fri, Aug 19, 2016 at 11:10:55AM -0700, Junio C Hamano wrote:\n> \n> > +# vim: set noet\n> \n> We tend to avoid cluttering the source with editor specific insns\n> like this.\n\noops.\n\nAnyone have any suggestions for project level vim settings?\n"},{"id":"299733","messageId":"20160819201837.GA18918@tci.corp.yp.com","threadId":"43885","inReplyTo":"xmqqh9ag39zk.fsf@gitster.mtv.corp.google.com","subject":"[PATCH] diff-highlight: add some tests.","fromName":"Brian Henderson","fromEmail":"henderson.bj@gmail.com","sentAt":"2016-08-19T20:18:37Z","receivedAt":"2016-08-19T20:19:28Z","isPatch":true,"sender":{"key":"henderson.bj@gmail.com","avatar":"https://avatars.githubusercontent.com/u/26167?v=4"},"body":"Junio, how does this look?\n\nSigned-off-by: Brian Henderson \n---\n contrib/diff-highlight/Makefile | 5 +\n contrib/diff-highlight/t/Makefile | 22 ++++\n contrib/diff-highlight/t/t9400-diff-highlight.sh | 122 +++++++++++++++++++++++\n 3 files changed, 149 insertions(+)\n create mode 100644 contrib/diff-highlight/Makefile\n create mode 100644 contrib/diff-highlight/t/Makefile\n create mode 100755 contrib/diff-highlight/t/t9400-diff-highlight.sh\n\ndiff --git a/contrib/diff-highlight/Makefile b/contrib/diff-highlight/Makefile\nnew file mode 100644\nindex 0000000..9018724\n--- /dev/null\n+++ b/contrib/diff-highlight/Makefile\n@@ -0,0 +1,5 @@\n+# nothing to build\n+all:\n+\n+test:\n+\t$(MAKE) -C t\ndiff --git a/contrib/diff-highlight/t/Makefile b/contrib/diff-highlight/t/Makefile\nnew file mode 100644\nindex 0000000..5ff5275\n--- /dev/null\n+++ b/contrib/diff-highlight/t/Makefile\n@@ -0,0 +1,22 @@\n+-include ../../../config.mak.autogen\n+-include ../../../config.mak\n+\n+# copied from ../../t/Makefile\n+SHELL_PATH ?= $(SHELL)\n+SHELL_PATH_SQ = $(subst ','\\'',$(SHELL_PATH))\n+T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)\n+\n+all: test\n+test: $(T)\n+\n+.PHONY: help clean all test $(T)\n+\n+help:\n+\t@echo 'Run \"$(MAKE) test\" to launch test scripts'\n+\t@echo 'Run \"$(MAKE) clean\" to remove trash folders'\n+\n+$(T):\n+\t@echo \"*** $@ ***\"; '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)\n+\n+clean:\n+\t$(RM) -r 'trash directory'.*\ndiff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh\nnew file mode 100755\nindex 0000000..3c04116\n--- /dev/null\n+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n@@ -0,0 +1,122 @@\n+#!/bin/sh\n+\n+test_description='Test diff-highlight'\n+\n+CURR_DIR=$(pwd)\n+TEST_OUTPUT_DIRECTORY=$(pwd)\n+TEST_DIRECTORY=\"$CURR_DIR\"/../../../t\n+DIFF_HIGHLIGHT=\"$CURR_DIR\"/../diff-highlight\n+\n+CW=\"\\033[7m\"\t# white\n+CR=\"\\033[27m\" # reset\n+\n+. \"$TEST_DIRECTORY\"/test-lib.sh\n+\n+if ! test_have_prereq PERL\n+then\n+\tskip_all='skipping diff-highlight tests; perl not available'\n+\ttest_done\n+fi\n+\n+# dh_test is a test helper function which takes 1) some file data, 2) some\n+# change of the file data, creates a diff and commit of the changes and passes\n+# that through diff-highlight.\n+# The optional 3rd parameter is the expected output of diff-highlight minus the\n+# diff/commit header. This parameter is given directly to printf as the format\n+# string (in order to properly handle ascii escape codes; CW, CR), so any '%'\n+# need to be doubled to protect it.\n+# Don't include a 3rd parameter if diff-highlight is supposed to leave the\n+# input unmodified.\n+# For convienence, the 3rd parameter can begin with a newline which will be\n+# stripped.\n+dh_test () {\n+\ta=\"$1\" b=\"$2\" &&\n+\n+\t{\n+\t\tprintf \"$a\" >file &&\n+\t\tgit add file &&\n+\t\tgit commit -m \"Add a file\" &&\n+\n+\t\tprintf \"$b\" >file &&\n+\t\tgit diff file >diff.raw &&\n+\t\tgit commit -am \"Update a file\" &&\n+\t\tgit show >commit.raw\n+\t} >/dev/null &&\n+\n+\tif test $# -ge 3\n+\tthen\n+\t\t# strip optional beginning newline\n+\t\tprintf \"$3\" | perl -pe 's/^\\n//'\n+\telse\n+\t\ttest_strip_patch_header diff.raw\n+\tfi >patch.exp &&\n+\n+\t\"$DIFF_HIGHLIGHT\" diff.act &&\n+\t\"$DIFF_HIGHLIGHT\" commit.act &&\n+\ttest_cmp patch.exp diff.act &&\n+\ttest_cmp patch.exp commit.act\n+}\n+\n+test_strip_patch_header () {\n+\tsed -e '1,/^@@/d' \"$@\"\n+}\n+\n+test_expect_success 'diff-highlight highlights the beginning of a line' '\n+\tdh_test \\\n+\t\t\"aaa\\nbbb\\nccc\\n\" \\\n+\t\t\"aaa\\n0bb\\nccc\\n\" \\\n+\"\n+ aaa\n+-${CW}b${CR}bb\n++${CW}0${CR}bb\n+ ccc\n+\"\n+'\n+\n+test_expect_success 'diff-highlight highlights the end of a line' '\n+\tdh_test \\\n+\t\t\"aaa\\nbbb\\nccc\\n\" \\\n+\t\t\"aaa\\nbb0\\nccc\\n\" \\\n+\"\n+ aaa\n+-bb${CW}b${CR}\n++bb${CW}0${CR}\n+ ccc\n+\"\n+'\n+\n+test_expect_success 'diff-highlight highlights the middle of a line' '\n+\tdh_test \\\n+\t\t\"aaa\\nbbb\\nccc\\n\" \\\n+\t\t\"aaa\\nb0b\\nccc\\n\" \\\n+\"\n+ aaa\n+-b${CW}b${CR}b\n++b${CW}0${CR}b\n+ ccc\n+\"\n+'\n+\n+test_expect_success 'diff-highlight does not highlight whole line' '\n+\tdh_test \\\n+\t\t\"aaa\\nbbb\\nccc\\n\" \\\n+\t\t\"aaa\\n000\\nccc\\n\"\n+'\n+\n+test_expect_failure 'diff-highlight highlights mismatched hunk size' '\n+\tdh_test \\\n+\t\t\"aaa\\nbbb\\n\" \\\n+\t\t\"aaa\\nb0b\\nccc\\n\" \\\n+\"\n+ aaa\n+-b${CW}b${CR}b\n++b${CW}0${CR}b\n++ccc\n+\"\n+'\n+\n+# TODO add multi-byte test\n+\n+test_done\n+\n+# vim: set noet\n-- \n2.9.0\n\n"},{"id":"299740","messageId":"xmqqbn0o1ob7.fsf@gitster.mtv.corp.google.com","threadId":"43885","inReplyTo":"20160819201837.GA18918@tci.corp.yp.com","subject":"Re: [PATCH] diff-highlight: add some tests.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-19T20:44:28Z","receivedAt":"2016-08-19T20:50:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brian Henderson writes:\n\n> Junio, how does this look?\n> ...\n> +# dh_test is a test helper function which takes 1) some file data, 2) some\n> +# change of the file data, creates a diff and commit of the changes and passes\n> +# that through diff-highlight.\n> +# The optional 3rd parameter is the expected output of diff-highlight minus the\n> +# diff/commit header. This parameter is given directly to printf as the format\n> +# string (in order to properly handle ascii escape codes; CW, CR), so any '%'\n> +# need to be doubled to protect it.\n> +# Don't include a 3rd parameter if diff-highlight is supposed to leave the\n> +# input unmodified.\n> +# For convienence, the 3rd parameter can begin with a newline which will be\n> +# stripped.\n\nYou seem to be stripping any and all empty lines with \"perl -pe\"; I\nam not sure if that is sensible.\n\nI really do not see the point of being able to spell\n\n\"\naaa\nbbb\n\"\n\nwhen you can perfectly well read\n\n\"aaa\nbbb\"\n\nor even \"aaa\\nbbb\\n\" for that matter. I personally do not think the\ndifference is worth the cost of an extra invocation of Perl, but we\nalready saw how stubborn you are, so there is no point spending my\ntime on trying to convince you further. Assuming that it is so\nprecious that the input can start with an extra blank line, what you\nwrote is a sensible implementation.\n\n> +dh_test () {\n> +\ta=\"$1\" b=\"$2\" &&\n> +\n> +\t{\n> +\t\tprintf \"$a\" >file &&\n> +\t\tgit add file &&\n> +\t\tgit commit -m \"Add a file\" &&\n> +\n> +\t\tprintf \"$b\" >file &&\n> +\t\tgit diff file >diff.raw &&\n> +\t\tgit commit -am \"Update a file\" &&\n> +\t\tgit show >commit.raw\n> +\t} >/dev/null &&\n> +\n> +\tif test $# -ge 3\n\nI think\n\n\tif test $# = 3\n\nis much better; you would want to catch bugs in a caller that gave\nyou fourth parameter by mistake, instead of silently ignoring it.\n\n> +test_strip_patch_header () {\n> +\tsed -e '1,/^@@/d' \"$@\"\n> +}\n\nWhile I think it is a great idea to hide the sed command behind a\nhelper function for readability, I am not sure \"strip patch header\"\nis a great name that describes what it does. A natural expectation\nfor an operation with that name done to this input:\n\n diff --git a/file b/file\n index fffffff..fffffff 100644\n --- a/file\n +++ b/file\n @@ -l,k +m,n @@ comments\n common\n -old\n +new\n common\n @@ -l,k +m,n @@ more comments\n common\n +added more\n +added even more\n\nwould be to remove the first four lines, not five.\n\n"},{"id":"299749","messageId":"20160819205641.GA31393@dcvr","threadId":"43885","inReplyTo":"20160819193045.GA9262@tci.corp.yp.com","subject":"Re: [PATCH v3 1/3] diff-highlight: add some tests.","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-08-19T20:56:41Z","receivedAt":"2016-08-19T20:57:01Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Brian Henderson wrote:\n> On Fri, Aug 19, 2016 at 11:10:55AM -0700, Junio C Hamano wrote:\n> > \n> > > +# vim: set noet\n> > \n> > We tend to avoid cluttering the source with editor specific insns\n> > like this.\n> \n> oops.\n> \n> Anyone have any suggestions for project level vim settings?\n\nvim defaults work fine for me on FreeBSD and Debian:\n\n\ttabstop=8 shiftwidth=8 softtabstop=0 noexpandtab\n\nGenerally, we take after the Linux kernel:\n\n\thttps://kernel.org/doc/Documentation/CodingStyle\n\nWhich I'm happy about and largely agree with.\n"},{"id":"299751","messageId":"20160819210431.usy6x3x6sdhe3vbp@sigill.intra.peff.net","threadId":"43885","inReplyTo":"xmqqbn0o1ob7.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] diff-highlight: add some tests.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-08-19T21:04:31Z","receivedAt":"2016-08-19T21:04:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Aug 19, 2016 at 01:44:28PM -0700, Junio C Hamano wrote:\n\n> Brian Henderson writes:\n> \n> > Junio, how does this look?\n> > ...\n> > +# dh_test is a test helper function which takes 1) some file data, 2) some\n> > +# change of the file data, creates a diff and commit of the changes and passes\n> > +# that through diff-highlight.\n> > +# The optional 3rd parameter is the expected output of diff-highlight minus the\n> > +# diff/commit header. This parameter is given directly to printf as the format\n> > +# string (in order to properly handle ascii escape codes; CW, CR), so any '%'\n> > +# need to be doubled to protect it.\n> > +# Don't include a 3rd parameter if diff-highlight is supposed to leave the\n> > +# input unmodified.\n> > +# For convienence, the 3rd parameter can begin with a newline which will be\n> > +# stripped.\n> \n> You seem to be stripping any and all empty lines with \"perl -pe\"; I\n> am not sure if that is sensible.\n> \n> I really do not see the point of being able to spell\n> \n> \"\n> aaa\n> bbb\n> \"\n> \n> when you can perfectly well read\n> \n> \"aaa\n> bbb\"\n> \n> or even \"aaa\\nbbb\\n\" for that matter. I personally do not think the\n> difference is worth the cost of an extra invocation of Perl, but we\n> already saw how stubborn you are, so there is no point spending my\n> time on trying to convince you further. Assuming that it is so\n> precious that the input can start with an extra blank line, what you\n> wrote is a sensible implementation.\n\nI didn't want to bikeshed, so I resisted saying so up until now, but I\nactually think:\n\n dh_test \\\n \"aaa\\nbbb\\nccc\\n\" \\\n \"aaa\\n0bb\\nccc\\n\" \\\n <<-EOF\n aaa\n -${CW}b${CR}bb\n +${CW}0${CR}bb\n EOF\n\nmight before readable, if only because it lets you indent the content to\nmatch the rest of the test content. For that matter, I'm not sure that:\n\n cat >a <<-\\EOF &&\n aaa\n bbb\n ccc\n EOF\n\n cat >b <<-\\EOF &&\n aaa\n 0bb\n ccc\n EOF\n\n dh_test a b <<\\EOF\n aaa\n -${CW}b${CR}bb\n +${CW}0${CR}bb\n EOF\n\nisn't more readable, too. It's more lines, certainly, but it makes it\nvery easy to see what the input files look like, rather than cramming\n\"\\n\" into the middle of a string (the existing code does make the diff\neasy to see for _this_ case, because the pre- and post-image line up\nvertically, but that is only the case for pure transliterations like\nthis).\n\nJust my two cents.\n\n-Peff\n"},{"id":"299752","messageId":"xmqq37m01nci.fsf@gitster.mtv.corp.google.com","threadId":"43885","inReplyTo":"20160819201837.GA18918@tci.corp.yp.com","subject":"Re: [PATCH] diff-highlight: add some tests.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-19T21:05:17Z","receivedAt":"2016-08-19T21:05:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brian Henderson writes:\n\n> +# dh_test is a test helper function which takes 1) some file data, 2) some\n> +# change of the file data, creates a diff and commit of the changes and passes\n> +# that through diff-highlight.\n> +# The optional 3rd parameter is the expected output of diff-highlight minus the\n> +# diff/commit header. This parameter is given directly to printf as the format\n> +# string (in order to properly handle ascii escape codes; CW, CR), so any '%'\n> +# need to be doubled to protect it.\n\nMy mistake. All three are given directly to printf as the format\nstring, so it is misleading to single out $3 when warning the\ncallers about '%'.\n\nSorry about that.\n"},{"id":"299768","messageId":"xmqqeg5kz8zq.fsf@gitster.mtv.corp.google.com","threadId":"43885","inReplyTo":"20160819210431.usy6x3x6sdhe3vbp@sigill.intra.peff.net","subject":"Re: [PATCH] diff-highlight: add some tests.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-19T22:31:21Z","receivedAt":"2016-08-19T22:31:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> For that matter, I'm not sure that:\n>\n> cat >a <<-\\EOF &&\n> aaa\n> bbb\n> ccc\n> EOF\n>\n> cat >b <<-\\EOF &&\n> aaa\n> 0bb\n> ccc\n> EOF\n>\n> dh_test a b <<\\EOF\n> aaa\n> -${CW}b${CR}bb\n> +${CW}0${CR}bb\n> EOF\n>\n> isn't more readable, too. It's more lines, certainly, but it makes it\n> very easy to see what the input files look like, rather than cramming\n> \"\\n\" into the middle of a string (the existing code does make the diff\n> easy to see for _this_ case, because the pre- and post-image line up\n> vertically, but that is only the case for pure transliterations like\n> this).\n\nYeah, the simplicity and obviousness certainly is very tempting.\n\nWith something like\n\nCW=$(printf \"\\033[7m\")\t# white\nCR=$(printf \"\\033[27m\") # reset\n\nupfront, the test helper does not even need to worry about feeding a\nrandom string through printf as if it is a format string.\n\n"},{"id":"299852","messageId":"20160822155539.25350-1-henderson.bj@gmail.com","threadId":"43885","inReplyTo":"xmqqeg5kz8zq.fsf@gitster.mtv.corp.google.com","subject":"[PATCH] diff-highlight: add some tests.","fromName":"Brian Henderson","fromEmail":"henderson.bj@gmail.com","sentAt":"2016-08-22T15:55:39Z","receivedAt":"2016-08-22T16:13:26Z","isPatch":true,"sender":{"key":"henderson.bj@gmail.com","avatar":"https://avatars.githubusercontent.com/u/26167?v=4"},"body":"Jeff, I love your idea. how's this looking?\n\nJunio, I wasn't meaning to be stubborn, although definitely a fault of mine. I\nunderstand a lot better now, thanks for your patience.\n\n---\n contrib/diff-highlight/Makefile | 5 +\n contrib/diff-highlight/t/Makefile | 22 +++\n contrib/diff-highlight/t/t9400-diff-highlight.sh | 163 +++++++++++++++++++++++\n 3 files changed, 190 insertions(+)\n create mode 100644 contrib/diff-highlight/Makefile\n create mode 100644 contrib/diff-highlight/t/Makefile\n create mode 100755 contrib/diff-highlight/t/t9400-diff-highlight.sh\n\ndiff --git a/contrib/diff-highlight/Makefile b/contrib/diff-highlight/Makefile\nnew file mode 100644\nindex 0000000..9018724\n--- /dev/null\n+++ b/contrib/diff-highlight/Makefile\n@@ -0,0 +1,5 @@\n+# nothing to build\n+all:\n+\n+test:\n+\t$(MAKE) -C t\ndiff --git a/contrib/diff-highlight/t/Makefile b/contrib/diff-highlight/t/Makefile\nnew file mode 100644\nindex 0000000..5ff5275\n--- /dev/null\n+++ b/contrib/diff-highlight/t/Makefile\n@@ -0,0 +1,22 @@\n+-include ../../../config.mak.autogen\n+-include ../../../config.mak\n+\n+# copied from ../../t/Makefile\n+SHELL_PATH ?= $(SHELL)\n+SHELL_PATH_SQ = $(subst ','\\'',$(SHELL_PATH))\n+T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)\n+\n+all: test\n+test: $(T)\n+\n+.PHONY: help clean all test $(T)\n+\n+help:\n+\t@echo 'Run \"$(MAKE) test\" to launch test scripts'\n+\t@echo 'Run \"$(MAKE) clean\" to remove trash folders'\n+\n+$(T):\n+\t@echo \"*** $@ ***\"; '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)\n+\n+clean:\n+\t$(RM) -r 'trash directory'.*\ndiff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh\nnew file mode 100755\nindex 0000000..7c303f7\n--- /dev/null\n+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n@@ -0,0 +1,163 @@\n+#!/bin/sh\n+\n+test_description='Test diff-highlight'\n+\n+CURR_DIR=$(pwd)\n+TEST_OUTPUT_DIRECTORY=$(pwd)\n+TEST_DIRECTORY=\"$CURR_DIR\"/../../../t\n+DIFF_HIGHLIGHT=\"$CURR_DIR\"/../diff-highlight\n+\n+CW=\"$(printf \"\\033[7m\")\"\t# white\n+CR=\"$(printf \"\\033[27m\")\"\t# reset\n+\n+. \"$TEST_DIRECTORY\"/test-lib.sh\n+\n+if ! test_have_prereq PERL\n+then\n+\tskip_all='skipping diff-highlight tests; perl not available'\n+\ttest_done\n+fi\n+\n+# dh_test is a test helper function which takes 3 file names as parameters. The\n+# first 2 files are used to generate diff and commit output, which is then\n+# piped through diff-highlight. The 3rd file should contain the expected output\n+# of diff-highlight (minus the diff/commit header, ie. everything after and\n+# including the first @@ line).\n+dh_test () {\n+\ta=\"$1\" b=\"$2\" &&\n+\n+\tcat >patch.exp &&\n+\n+\t{\n+\t\tcat \"$a\" >file &&\n+\t\tgit add file &&\n+\t\tgit commit -m \"Add a file\" &&\n+\n+\t\tcat \"$b\" >file &&\n+\t\tgit diff file >diff.raw &&\n+\t\tgit commit -am \"Update a file\" &&\n+\t\tgit show >commit.raw\n+\t} >/dev/null &&\n+\n+\t\"$DIFF_HIGHLIGHT\" diff.act &&\n+\t\"$DIFF_HIGHLIGHT\" commit.act &&\n+\ttest_cmp patch.exp diff.act &&\n+\ttest_cmp patch.exp commit.act\n+}\n+\n+test_strip_patch_header () {\n+\tsed -n '/^@@/,$p' $*\n+}\n+\n+test_expect_success 'diff-highlight highlights the beginning of a line' '\n+\tcat >a <<-\\EOF &&\n+\t\taaa\n+\t\tbbb\n+\t\tccc\n+\tEOF\n+\n+\tcat >b <<-\\EOF &&\n+\t\taaa\n+\t\t0bb\n+\t\tccc\n+\tEOF\n+\n+\tdh_test a b <<-EOF\n+\t\t@@ -1,3 +1,3 @@\n+\t\t aaa\n+\t\t-${CW}b${CR}bb\n+\t\t+${CW}0${CR}bb\n+\t\t ccc\n+\tEOF\n+'\n+\n+test_expect_success 'diff-highlight highlights the end of a line' '\n+\tcat >a <<-\\EOF &&\n+\t\taaa\n+\t\tbbb\n+\t\tccc\n+\tEOF\n+\n+\tcat >b <<-\\EOF &&\n+\t\taaa\n+\t\tbb0\n+\t\tccc\n+\tEOF\n+\n+\tdh_test a b <<-EOF\n+\t\t@@ -1,3 +1,3 @@\n+\t\t aaa\n+\t\t-bb${CW}b${CR}\n+\t\t+bb${CW}0${CR}\n+\t\t ccc\n+\tEOF\n+'\n+\n+test_expect_success 'diff-highlight highlights the middle of a line' '\n+\tcat >a <<-\\EOF &&\n+\t\taaa\n+\t\tbbb\n+\t\tccc\n+\tEOF\n+\n+\tcat >b <<-\\EOF &&\n+\t\taaa\n+\t\tb0b\n+\t\tccc\n+\tEOF\n+\n+\tdh_test a b <<-EOF\n+\t\t@@ -1,3 +1,3 @@\n+\t\t aaa\n+\t\t-b${CW}b${CR}b\n+\t\t+b${CW}0${CR}b\n+\t\t ccc\n+\tEOF\n+'\n+\n+test_expect_success 'diff-highlight does not highlight whole line' '\n+\tcat >a <<-\\EOF &&\n+\t\taaa\n+\t\tbbb\n+\t\tccc\n+\tEOF\n+\n+\tcat >b <<-\\EOF &&\n+\t\taaa\n+\t\t000\n+\t\tccc\n+\tEOF\n+\n+\tdh_test a b <<-EOF\n+\t\t@@ -1,3 +1,3 @@\n+\t\t aaa\n+\t\t-bbb\n+\t\t+000\n+\t\t ccc\n+\tEOF\n+'\n+\n+test_expect_failure 'diff-highlight highlights mismatched hunk size' '\n+\tcat >a <<-\\EOF &&\n+\t\taaa\n+\t\tbbb\n+\tEOF\n+\n+\tcat >b <<-\\EOF &&\n+\t\taaa\n+\t\tb0b\n+\t\tccc\n+\tEOF\n+\n+\tdh_test a b <<-EOF\n+\t\t@@ -1,3 +1,3 @@\n+\t\t aaa\n+\t\t-b${CW}b${CR}b\n+\t\t+b${CW}0${CR}b\n+\t\t+ccc\n+\tEOF\n+'\n+\n+# TODO add multi-byte test\n+\n+test_done\n-- \n2.9.0\n\n"},{"id":"299893","messageId":"20160823041252.53ldwacgdey2euxt@sigill.intra.peff.net","threadId":"43885","inReplyTo":"20160822155539.25350-1-henderson.bj@gmail.com","subject":"Re: [PATCH] diff-highlight: add some tests.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-08-23T04:12:52Z","receivedAt":"2016-08-23T04:13:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 22, 2016 at 08:55:39AM -0700, Brian Henderson wrote:\n\n> Jeff, I love your idea. how's this looking?\n\nMuch more readable, IMHO.\n\n-Peff\n"},{"id":"300468","messageId":"20160829173347.454-1-henderson.bj@gmail.com","threadId":"43885","inReplyTo":"20160823041252.53ldwacgdey2euxt@sigill.intra.peff.net","subject":"[PATCH v4 0/3] diff-highlight: add support for --graph option","fromName":"Brian Henderson","fromEmail":"henderson.bj@gmail.com","sentAt":"2016-08-29T17:33:44Z","receivedAt":"2016-08-29T17:34:07Z","isPatch":true,"sender":{"key":"henderson.bj@gmail.com","avatar":"https://avatars.githubusercontent.com/u/26167?v=4"},"body":"How does this look?\n\nDrawing the graph helped me a lot in figuring out what I was actually testing. thanks!\n\nBrian Henderson (3):\n diff-highlight: add some tests.\n diff-highlight: add failing test for handling --graph output.\n diff-highlight: add support for --graph output.\n\n contrib/diff-highlight/Makefile | 5 +\n contrib/diff-highlight/diff-highlight | 19 +-\n contrib/diff-highlight/t/Makefile | 22 +++\n contrib/diff-highlight/t/t9400-diff-highlight.sh | 223 +++++++++++++++++++++++\n 4 files changed, 263 insertions(+), 6 deletions(-)\n create mode 100644 contrib/diff-highlight/Makefile\n create mode 100644 contrib/diff-highlight/t/Makefile\n create mode 100755 contrib/diff-highlight/t/t9400-diff-highlight.sh\n\n-- \n2.9.3\n\n"},{"id":"300469","messageId":"20160829173347.454-3-henderson.bj@gmail.com","threadId":"43885","inReplyTo":"20160829173347.454-1-henderson.bj@gmail.com","subject":"[PATCH v4 2/3] diff-highlight: add failing test for handling --graph output.","fromName":"Brian Henderson","fromEmail":"henderson.bj@gmail.com","sentAt":"2016-08-29T17:33:46Z","receivedAt":"2016-08-29T17:34:14Z","isPatch":true,"sender":{"key":"henderson.bj@gmail.com","avatar":"https://avatars.githubusercontent.com/u/26167?v=4"},"body":"---\n contrib/diff-highlight/t/t9400-diff-highlight.sh | 60 ++++++++++++++++++++++++\n 1 file changed, 60 insertions(+)\n\ndiff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh\nindex 7c303f7..54e11fe 100755\n--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh\n+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n@@ -49,6 +49,55 @@ test_strip_patch_header () {\n \tsed -n '/^@@/,$p' $*\n }\n \n+# dh_test_setup_history generates a contrived graph such that we have at least\n+# 1 nesting (E) and 2 nestings (F).\n+#\n+#\t A branch\n+#\t /\n+#\tD---E---F master\n+#\n+#\tgit log --all --graph\n+#\t* commit\n+#\t| A\n+#\t| * commit\n+#\t| | F\n+#\t| * commit\n+#\t|/\n+#\t| E\n+#\t* commit\n+#\t D\n+#\n+dh_test_setup_history () {\n+\techo \"file1\" >file1 &&\n+\techo \"file2\" >file2 &&\n+\techo \"file3\" >file3 &&\n+\n+\tcat file1 >file &&\n+\tgit add file &&\n+\tgit commit -m \"D\" &&\n+\n+\tgit checkout -b branch &&\n+\tcat file2 >file &&\n+\tgit commit -am \"A\" &&\n+\n+\tgit checkout master &&\n+\tcat file2 >file &&\n+\tgit commit -am \"E\" &&\n+\n+\tcat file3 >file &&\n+\tgit commit -am \"F\"\n+}\n+\n+left_trim () {\n+\t\"$PERL_PATH\" -pe 's/^\\s+//'\n+}\n+\n+trim_graph () {\n+\t# graphs start with * or |\n+\t# followed by a space or / or \\\n+\t\"$PERL_PATH\" -pe 's@^((\\*|\\|)( |/|\\\\))+@@'\n+}\n+\n test_expect_success 'diff-highlight highlights the beginning of a line' '\n \tcat >a <<-\\EOF &&\n \t\taaa\n@@ -160,4 +209,15 @@ test_expect_failure 'diff-highlight highlights mismatched hunk size' '\n \n # TODO add multi-byte test\n \n+test_expect_failure 'diff-highlight works with the --graph option' '\n+\tdh_test_setup_history &&\n+\n+\t# topo-order so that the order of the commits is the same as with --graph\n+\t# trim graph elements so we can do a diff\n+\t# trim leading space because our trim_graph is not perfect\n+\tgit log --branches -p --topo-order | \"$DIFF_HIGHLIGHT\" | left_trim >graph.exp &&\n+\tgit log --branches -p --graph | \"$DIFF_HIGHLIGHT\" | trim_graph | left_trim >graph.act &&\n+\ttest_cmp graph.exp graph.act\n+'\n+\n test_done\n-- \n2.9.3\n\n"},{"id":"300470","messageId":"20160829173347.454-2-henderson.bj@gmail.com","threadId":"43885","inReplyTo":"20160829173347.454-1-henderson.bj@gmail.com","subject":"[PATCH v4 1/3] diff-highlight: add some tests.","fromName":"Brian Henderson","fromEmail":"henderson.bj@gmail.com","sentAt":"2016-08-29T17:33:45Z","receivedAt":"2016-08-29T17:34:18Z","isPatch":true,"sender":{"key":"henderson.bj@gmail.com","avatar":"https://avatars.githubusercontent.com/u/26167?v=4"},"body":"---\n contrib/diff-highlight/Makefile | 5 +\n contrib/diff-highlight/t/Makefile | 22 +++\n contrib/diff-highlight/t/t9400-diff-highlight.sh | 163 +++++++++++++++++++++++\n 3 files changed, 190 insertions(+)\n create mode 100644 contrib/diff-highlight/Makefile\n create mode 100644 contrib/diff-highlight/t/Makefile\n create mode 100755 contrib/diff-highlight/t/t9400-diff-highlight.sh\n\ndiff --git a/contrib/diff-highlight/Makefile b/contrib/diff-highlight/Makefile\nnew file mode 100644\nindex 0000000..9018724\n--- /dev/null\n+++ b/contrib/diff-highlight/Makefile\n@@ -0,0 +1,5 @@\n+# nothing to build\n+all:\n+\n+test:\n+\t$(MAKE) -C t\ndiff --git a/contrib/diff-highlight/t/Makefile b/contrib/diff-highlight/t/Makefile\nnew file mode 100644\nindex 0000000..5ff5275\n--- /dev/null\n+++ b/contrib/diff-highlight/t/Makefile\n@@ -0,0 +1,22 @@\n+-include ../../../config.mak.autogen\n+-include ../../../config.mak\n+\n+# copied from ../../t/Makefile\n+SHELL_PATH ?= $(SHELL)\n+SHELL_PATH_SQ = $(subst ','\\'',$(SHELL_PATH))\n+T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)\n+\n+all: test\n+test: $(T)\n+\n+.PHONY: help clean all test $(T)\n+\n+help:\n+\t@echo 'Run \"$(MAKE) test\" to launch test scripts'\n+\t@echo 'Run \"$(MAKE) clean\" to remove trash folders'\n+\n+$(T):\n+\t@echo \"*** $@ ***\"; '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)\n+\n+clean:\n+\t$(RM) -r 'trash directory'.*\ndiff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh\nnew file mode 100755\nindex 0000000..7c303f7\n--- /dev/null\n+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n@@ -0,0 +1,163 @@\n+#!/bin/sh\n+\n+test_description='Test diff-highlight'\n+\n+CURR_DIR=$(pwd)\n+TEST_OUTPUT_DIRECTORY=$(pwd)\n+TEST_DIRECTORY=\"$CURR_DIR\"/../../../t\n+DIFF_HIGHLIGHT=\"$CURR_DIR\"/../diff-highlight\n+\n+CW=\"$(printf \"\\033[7m\")\"\t# white\n+CR=\"$(printf \"\\033[27m\")\"\t# reset\n+\n+. \"$TEST_DIRECTORY\"/test-lib.sh\n+\n+if ! test_have_prereq PERL\n+then\n+\tskip_all='skipping diff-highlight tests; perl not available'\n+\ttest_done\n+fi\n+\n+# dh_test is a test helper function which takes 3 file names as parameters. The\n+# first 2 files are used to generate diff and commit output, which is then\n+# piped through diff-highlight. The 3rd file should contain the expected output\n+# of diff-highlight (minus the diff/commit header, ie. everything after and\n+# including the first @@ line).\n+dh_test () {\n+\ta=\"$1\" b=\"$2\" &&\n+\n+\tcat >patch.exp &&\n+\n+\t{\n+\t\tcat \"$a\" >file &&\n+\t\tgit add file &&\n+\t\tgit commit -m \"Add a file\" &&\n+\n+\t\tcat \"$b\" >file &&\n+\t\tgit diff file >diff.raw &&\n+\t\tgit commit -am \"Update a file\" &&\n+\t\tgit show >commit.raw\n+\t} >/dev/null &&\n+\n+\t\"$DIFF_HIGHLIGHT\" diff.act &&\n+\t\"$DIFF_HIGHLIGHT\" commit.act &&\n+\ttest_cmp patch.exp diff.act &&\n+\ttest_cmp patch.exp commit.act\n+}\n+\n+test_strip_patch_header () {\n+\tsed -n '/^@@/,$p' $*\n+}\n+\n+test_expect_success 'diff-highlight highlights the beginning of a line' '\n+\tcat >a <<-\\EOF &&\n+\t\taaa\n+\t\tbbb\n+\t\tccc\n+\tEOF\n+\n+\tcat >b <<-\\EOF &&\n+\t\taaa\n+\t\t0bb\n+\t\tccc\n+\tEOF\n+\n+\tdh_test a b <<-EOF\n+\t\t@@ -1,3 +1,3 @@\n+\t\t aaa\n+\t\t-${CW}b${CR}bb\n+\t\t+${CW}0${CR}bb\n+\t\t ccc\n+\tEOF\n+'\n+\n+test_expect_success 'diff-highlight highlights the end of a line' '\n+\tcat >a <<-\\EOF &&\n+\t\taaa\n+\t\tbbb\n+\t\tccc\n+\tEOF\n+\n+\tcat >b <<-\\EOF &&\n+\t\taaa\n+\t\tbb0\n+\t\tccc\n+\tEOF\n+\n+\tdh_test a b <<-EOF\n+\t\t@@ -1,3 +1,3 @@\n+\t\t aaa\n+\t\t-bb${CW}b${CR}\n+\t\t+bb${CW}0${CR}\n+\t\t ccc\n+\tEOF\n+'\n+\n+test_expect_success 'diff-highlight highlights the middle of a line' '\n+\tcat >a <<-\\EOF &&\n+\t\taaa\n+\t\tbbb\n+\t\tccc\n+\tEOF\n+\n+\tcat >b <<-\\EOF &&\n+\t\taaa\n+\t\tb0b\n+\t\tccc\n+\tEOF\n+\n+\tdh_test a b <<-EOF\n+\t\t@@ -1,3 +1,3 @@\n+\t\t aaa\n+\t\t-b${CW}b${CR}b\n+\t\t+b${CW}0${CR}b\n+\t\t ccc\n+\tEOF\n+'\n+\n+test_expect_success 'diff-highlight does not highlight whole line' '\n+\tcat >a <<-\\EOF &&\n+\t\taaa\n+\t\tbbb\n+\t\tccc\n+\tEOF\n+\n+\tcat >b <<-\\EOF &&\n+\t\taaa\n+\t\t000\n+\t\tccc\n+\tEOF\n+\n+\tdh_test a b <<-EOF\n+\t\t@@ -1,3 +1,3 @@\n+\t\t aaa\n+\t\t-bbb\n+\t\t+000\n+\t\t ccc\n+\tEOF\n+'\n+\n+test_expect_failure 'diff-highlight highlights mismatched hunk size' '\n+\tcat >a <<-\\EOF &&\n+\t\taaa\n+\t\tbbb\n+\tEOF\n+\n+\tcat >b <<-\\EOF &&\n+\t\taaa\n+\t\tb0b\n+\t\tccc\n+\tEOF\n+\n+\tdh_test a b <<-EOF\n+\t\t@@ -1,3 +1,3 @@\n+\t\t aaa\n+\t\t-b${CW}b${CR}b\n+\t\t+b${CW}0${CR}b\n+\t\t+ccc\n+\tEOF\n+'\n+\n+# TODO add multi-byte test\n+\n+test_done\n-- \n2.9.3\n\n"},{"id":"300471","messageId":"20160829173347.454-4-henderson.bj@gmail.com","threadId":"43885","inReplyTo":"20160829173347.454-1-henderson.bj@gmail.com","subject":"[PATCH v4 3/3] diff-highlight: add support for --graph output.","fromName":"Brian Henderson","fromEmail":"henderson.bj@gmail.com","sentAt":"2016-08-29T17:33:47Z","receivedAt":"2016-08-29T17:34:21Z","isPatch":true,"sender":{"key":"henderson.bj@gmail.com","avatar":"https://avatars.githubusercontent.com/u/26167?v=4"},"body":"---\n contrib/diff-highlight/diff-highlight | 19 +++++++++++++------\n contrib/diff-highlight/t/t9400-diff-highlight.sh | 2 +-\n 2 files changed, 14 insertions(+), 7 deletions(-)\n\ndiff --git a/contrib/diff-highlight/diff-highlight b/contrib/diff-highlight/diff-highlight\nindex ffefc31..9280c88 100755\n--- a/contrib/diff-highlight/diff-highlight\n+++ b/contrib/diff-highlight/diff-highlight\n@@ -21,6 +21,10 @@ my $RESET = \"\\x1b[m\";\n my $COLOR = qr/\\x1b\\[[0-9;]*m/;\n my $BORING = qr/$COLOR|\\s/;\n \n+# The patch portion of git log -p --graph should only ever have preceding | and\n+# not / or \\ as merge history only shows up on the commit line.\n+my $GRAPH = qr/$COLOR?\\|$COLOR?\\s+/;\n+\n my @removed;\n my @added;\n my $in_hunk;\n@@ -32,12 +36,12 @@ $SIG{PIPE} = 'DEFAULT';\n while (<>) {\n \tif (!$in_hunk) {\n \t\tprint;\n-\t\t$in_hunk = /^$COLOR*\\@/;\n+\t\t$in_hunk = /^$GRAPH*$COLOR*\\@/;\n \t}\n-\telsif (/^$COLOR*-/) {\n+\telsif (/^$GRAPH*$COLOR*-/) {\n \t\tpush @removed, $_;\n \t}\n-\telsif (/^$COLOR*\\+/) {\n+\telsif (/^$GRAPH*$COLOR*\\+/) {\n \t\tpush @added, $_;\n \t}\n \telse {\n@@ -46,7 +50,7 @@ while (<>) {\n \t\t@added = ();\n \n \t\tprint;\n-\t\t$in_hunk = /^$COLOR*[\\@ ]/;\n+\t\t$in_hunk = /^$GRAPH*$COLOR*[\\@ ]/;\n \t}\n \n \t# Most of the time there is enough output to keep things streaming,\n@@ -163,6 +167,9 @@ sub highlight_pair {\n \t}\n }\n \n+# we split either by $COLOR or by character. This has the side effect of\n+# leaving in graph cruft. It works because the graph cruft does not contain \"-\"\n+# or \"+\"\n sub split_line {\n \tlocal $_ = shift;\n \treturn utf8::decode($_) ?\n@@ -211,8 +218,8 @@ sub is_pair_interesting {\n \tmy $suffix_a = join('', @$a[($sa+1)..$#$a]);\n \tmy $suffix_b = join('', @$b[($sb+1)..$#$b]);\n \n-\treturn $prefix_a !~ /^$COLOR*-$BORING*$/ ||\n-\t $prefix_b !~ /^$COLOR*\\+$BORING*$/ ||\n+\treturn $prefix_a !~ /^$GRAPH*$COLOR*-$BORING*$/ ||\n+\t $prefix_b !~ /^$GRAPH*$COLOR*\\+$BORING*$/ ||\n \t $suffix_a !~ /^$BORING*$/ ||\n \t $suffix_b !~ /^$BORING*$/;\n }\ndiff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh\nindex 54e11fe..e42232d 100755\n--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh\n+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n@@ -209,7 +209,7 @@ test_expect_failure 'diff-highlight highlights mismatched hunk size' '\n \n # TODO add multi-byte test\n \n-test_expect_failure 'diff-highlight works with the --graph option' '\n+test_expect_success 'diff-highlight works with the --graph option' '\n \tdh_test_setup_history &&\n \n \t# topo-order so that the order of the commits is the same as with --graph\n-- \n2.9.3\n\n"},{"id":"300514","messageId":"xmqq4m638ded.fsf@gitster.mtv.corp.google.com","threadId":"43885","inReplyTo":"20160829173347.454-1-henderson.bj@gmail.com","subject":"Re: [PATCH v4 0/3] diff-highlight: add support for --graph option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-29T21:37:46Z","receivedAt":"2016-08-29T21:37:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brian Henderson writes:\n\n> How does this look?\n>\n> Drawing the graph helped me a lot in figuring out what I was\n> actually testing. thanks!\n\nYeah, I also am pleased to see the picture of what is being tested\nin the test script.\n\nWith your sign-off, they would have been almost perfect ;-).\n\n> Brian Henderson (3):\n> diff-highlight: add some tests.\n> diff-highlight: add failing test for handling --graph output.\n> diff-highlight: add support for --graph output.\n>\n> contrib/diff-highlight/Makefile | 5 +\n> contrib/diff-highlight/diff-highlight | 19 +-\n> contrib/diff-highlight/t/Makefile | 22 +++\n> contrib/diff-highlight/t/t9400-diff-highlight.sh | 223 +++++++++++++++++++++++\n> 4 files changed, 263 insertions(+), 6 deletions(-)\n> create mode 100644 contrib/diff-highlight/Makefile\n> create mode 100644 contrib/diff-highlight/t/Makefile\n> create mode 100755 contrib/diff-highlight/t/t9400-diff-highlight.sh\n"},{"id":"300543","messageId":"20160830081152.rbvwexqqe6jwkue2@sigill.intra.peff.net","threadId":"43885","inReplyTo":"20160829173347.454-1-henderson.bj@gmail.com","subject":"Re: [PATCH v4 0/3] diff-highlight: add support for --graph option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-08-30T08:11:52Z","receivedAt":"2016-08-30T08:12:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 29, 2016 at 10:33:44AM -0700, Brian Henderson wrote:\n\n> How does this look?\n> \n> Drawing the graph helped me a lot in figuring out what I was actually testing. thanks!\n> \n> Brian Henderson (3):\n> diff-highlight: add some tests.\n> diff-highlight: add failing test for handling --graph output.\n> diff-highlight: add support for --graph output.\n\nThese all look good to me. As Junio mentioned, you need to add a\nsignoff (see the section \"Sign your work\" in Documentation/SubmittingPatches).\n\nAlso, a minor nit, but we don't usually put a \".\" at the end of our\ncommit message subjects (not worth a re-roll on its own, but if you are\nsending them again anyway...).\n\n-Peff\n"},{"id":"300559","messageId":"20160830140714.19048-1-henderson.bj@gmail.com","threadId":"43885","inReplyTo":"xmqq4m638ded.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v4 0/3] diff-highlight: add support for --graph option","fromName":"Brian Henderson","fromEmail":"henderson.bj@gmail.com","sentAt":"2016-08-30T14:07:11Z","receivedAt":"2016-08-30T14:07:44Z","isPatch":true,"sender":{"key":"henderson.bj@gmail.com","avatar":"https://avatars.githubusercontent.com/u/26167?v=4"},"body":"On Mon, Aug 29, 2016 at 02:37:46PM -0700, Junio C Hamano wrote:\n> Brian Henderson writes:\n>\n> > How does this look?\n> >\n> > Drawing the graph helped me a lot in figuring out what I was\n> > actually testing. thanks!\n>\n> Yeah, I also am pleased to see the picture of what is being tested\n> in the test script.\n>\n> With your sign-off, they would have been almost perfect ;-).\n\ndoh. fixed.\n\nI left the subject as v4, probably mostly because I have this weird aversion to\nincreasing version numbers :) but I justified it by thinking that the actual\npatch set isn't changing, I just added the sign-off (and updated the commit\nmessages per Jeff.) Hope that's ok.\n\nthanks everyone for all your help!\n\n\nBrian Henderson (3):\n diff-highlight: add some tests\n diff-highlight: add failing test for handling --graph output\n diff-highlight: add support for --graph output\n\n contrib/diff-highlight/Makefile | 5 +\n contrib/diff-highlight/diff-highlight | 19 +-\n contrib/diff-highlight/t/Makefile | 22 +++\n contrib/diff-highlight/t/t9400-diff-highlight.sh | 223 +++++++++++++++++++++++\n 4 files changed, 263 insertions(+), 6 deletions(-)\n create mode 100644 contrib/diff-highlight/Makefile\n create mode 100644 contrib/diff-highlight/t/Makefile\n create mode 100755 contrib/diff-highlight/t/t9400-diff-highlight.sh\n\n-- \n2.9.3\n\n"},{"id":"300560","messageId":"20160830140714.19048-2-henderson.bj@gmail.com","threadId":"43885","inReplyTo":"20160830140714.19048-1-henderson.bj@gmail.com","subject":"[PATCH v4 1/3] diff-highlight: add some tests","fromName":"Brian Henderson","fromEmail":"henderson.bj@gmail.com","sentAt":"2016-08-30T14:07:12Z","receivedAt":"2016-08-30T14:07:52Z","isPatch":true,"sender":{"key":"henderson.bj@gmail.com","avatar":"https://avatars.githubusercontent.com/u/26167?v=4"},"body":"Signed-off-by: Brian Henderson \n---\n contrib/diff-highlight/Makefile | 5 +\n contrib/diff-highlight/t/Makefile | 22 +++\n contrib/diff-highlight/t/t9400-diff-highlight.sh | 163 +++++++++++++++++++++++\n 3 files changed, 190 insertions(+)\n create mode 100644 contrib/diff-highlight/Makefile\n create mode 100644 contrib/diff-highlight/t/Makefile\n create mode 100755 contrib/diff-highlight/t/t9400-diff-highlight.sh\n\ndiff --git a/contrib/diff-highlight/Makefile b/contrib/diff-highlight/Makefile\nnew file mode 100644\nindex 0000000..9018724\n--- /dev/null\n+++ b/contrib/diff-highlight/Makefile\n@@ -0,0 +1,5 @@\n+# nothing to build\n+all:\n+\n+test:\n+\t$(MAKE) -C t\ndiff --git a/contrib/diff-highlight/t/Makefile b/contrib/diff-highlight/t/Makefile\nnew file mode 100644\nindex 0000000..5ff5275\n--- /dev/null\n+++ b/contrib/diff-highlight/t/Makefile\n@@ -0,0 +1,22 @@\n+-include ../../../config.mak.autogen\n+-include ../../../config.mak\n+\n+# copied from ../../t/Makefile\n+SHELL_PATH ?= $(SHELL)\n+SHELL_PATH_SQ = $(subst ','\\'',$(SHELL_PATH))\n+T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)\n+\n+all: test\n+test: $(T)\n+\n+.PHONY: help clean all test $(T)\n+\n+help:\n+\t@echo 'Run \"$(MAKE) test\" to launch test scripts'\n+\t@echo 'Run \"$(MAKE) clean\" to remove trash folders'\n+\n+$(T):\n+\t@echo \"*** $@ ***\"; '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)\n+\n+clean:\n+\t$(RM) -r 'trash directory'.*\ndiff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh\nnew file mode 100755\nindex 0000000..7c303f7\n--- /dev/null\n+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n@@ -0,0 +1,163 @@\n+#!/bin/sh\n+\n+test_description='Test diff-highlight'\n+\n+CURR_DIR=$(pwd)\n+TEST_OUTPUT_DIRECTORY=$(pwd)\n+TEST_DIRECTORY=\"$CURR_DIR\"/../../../t\n+DIFF_HIGHLIGHT=\"$CURR_DIR\"/../diff-highlight\n+\n+CW=\"$(printf \"\\033[7m\")\"\t# white\n+CR=\"$(printf \"\\033[27m\")\"\t# reset\n+\n+. \"$TEST_DIRECTORY\"/test-lib.sh\n+\n+if ! test_have_prereq PERL\n+then\n+\tskip_all='skipping diff-highlight tests; perl not available'\n+\ttest_done\n+fi\n+\n+# dh_test is a test helper function which takes 3 file names as parameters. The\n+# first 2 files are used to generate diff and commit output, which is then\n+# piped through diff-highlight. The 3rd file should contain the expected output\n+# of diff-highlight (minus the diff/commit header, ie. everything after and\n+# including the first @@ line).\n+dh_test () {\n+\ta=\"$1\" b=\"$2\" &&\n+\n+\tcat >patch.exp &&\n+\n+\t{\n+\t\tcat \"$a\" >file &&\n+\t\tgit add file &&\n+\t\tgit commit -m \"Add a file\" &&\n+\n+\t\tcat \"$b\" >file &&\n+\t\tgit diff file >diff.raw &&\n+\t\tgit commit -am \"Update a file\" &&\n+\t\tgit show >commit.raw\n+\t} >/dev/null &&\n+\n+\t\"$DIFF_HIGHLIGHT\" diff.act &&\n+\t\"$DIFF_HIGHLIGHT\" commit.act &&\n+\ttest_cmp patch.exp diff.act &&\n+\ttest_cmp patch.exp commit.act\n+}\n+\n+test_strip_patch_header () {\n+\tsed -n '/^@@/,$p' $*\n+}\n+\n+test_expect_success 'diff-highlight highlights the beginning of a line' '\n+\tcat >a <<-\\EOF &&\n+\t\taaa\n+\t\tbbb\n+\t\tccc\n+\tEOF\n+\n+\tcat >b <<-\\EOF &&\n+\t\taaa\n+\t\t0bb\n+\t\tccc\n+\tEOF\n+\n+\tdh_test a b <<-EOF\n+\t\t@@ -1,3 +1,3 @@\n+\t\t aaa\n+\t\t-${CW}b${CR}bb\n+\t\t+${CW}0${CR}bb\n+\t\t ccc\n+\tEOF\n+'\n+\n+test_expect_success 'diff-highlight highlights the end of a line' '\n+\tcat >a <<-\\EOF &&\n+\t\taaa\n+\t\tbbb\n+\t\tccc\n+\tEOF\n+\n+\tcat >b <<-\\EOF &&\n+\t\taaa\n+\t\tbb0\n+\t\tccc\n+\tEOF\n+\n+\tdh_test a b <<-EOF\n+\t\t@@ -1,3 +1,3 @@\n+\t\t aaa\n+\t\t-bb${CW}b${CR}\n+\t\t+bb${CW}0${CR}\n+\t\t ccc\n+\tEOF\n+'\n+\n+test_expect_success 'diff-highlight highlights the middle of a line' '\n+\tcat >a <<-\\EOF &&\n+\t\taaa\n+\t\tbbb\n+\t\tccc\n+\tEOF\n+\n+\tcat >b <<-\\EOF &&\n+\t\taaa\n+\t\tb0b\n+\t\tccc\n+\tEOF\n+\n+\tdh_test a b <<-EOF\n+\t\t@@ -1,3 +1,3 @@\n+\t\t aaa\n+\t\t-b${CW}b${CR}b\n+\t\t+b${CW}0${CR}b\n+\t\t ccc\n+\tEOF\n+'\n+\n+test_expect_success 'diff-highlight does not highlight whole line' '\n+\tcat >a <<-\\EOF &&\n+\t\taaa\n+\t\tbbb\n+\t\tccc\n+\tEOF\n+\n+\tcat >b <<-\\EOF &&\n+\t\taaa\n+\t\t000\n+\t\tccc\n+\tEOF\n+\n+\tdh_test a b <<-EOF\n+\t\t@@ -1,3 +1,3 @@\n+\t\t aaa\n+\t\t-bbb\n+\t\t+000\n+\t\t ccc\n+\tEOF\n+'\n+\n+test_expect_failure 'diff-highlight highlights mismatched hunk size' '\n+\tcat >a <<-\\EOF &&\n+\t\taaa\n+\t\tbbb\n+\tEOF\n+\n+\tcat >b <<-\\EOF &&\n+\t\taaa\n+\t\tb0b\n+\t\tccc\n+\tEOF\n+\n+\tdh_test a b <<-EOF\n+\t\t@@ -1,3 +1,3 @@\n+\t\t aaa\n+\t\t-b${CW}b${CR}b\n+\t\t+b${CW}0${CR}b\n+\t\t+ccc\n+\tEOF\n+'\n+\n+# TODO add multi-byte test\n+\n+test_done\n-- \n2.9.3\n\n"},{"id":"300561","messageId":"20160830140714.19048-4-henderson.bj@gmail.com","threadId":"43885","inReplyTo":"20160830140714.19048-1-henderson.bj@gmail.com","subject":"[PATCH v4 3/3] diff-highlight: add support for --graph output","fromName":"Brian Henderson","fromEmail":"henderson.bj@gmail.com","sentAt":"2016-08-30T14:07:14Z","receivedAt":"2016-08-30T14:07:57Z","isPatch":true,"sender":{"key":"henderson.bj@gmail.com","avatar":"https://avatars.githubusercontent.com/u/26167?v=4"},"body":"Signed-off-by: Brian Henderson \n---\n contrib/diff-highlight/diff-highlight | 19 +++++++++++++------\n contrib/diff-highlight/t/t9400-diff-highlight.sh | 2 +-\n 2 files changed, 14 insertions(+), 7 deletions(-)\n\ndiff --git a/contrib/diff-highlight/diff-highlight b/contrib/diff-highlight/diff-highlight\nindex ffefc31..9280c88 100755\n--- a/contrib/diff-highlight/diff-highlight\n+++ b/contrib/diff-highlight/diff-highlight\n@@ -21,6 +21,10 @@ my $RESET = \"\\x1b[m\";\n my $COLOR = qr/\\x1b\\[[0-9;]*m/;\n my $BORING = qr/$COLOR|\\s/;\n \n+# The patch portion of git log -p --graph should only ever have preceding | and\n+# not / or \\ as merge history only shows up on the commit line.\n+my $GRAPH = qr/$COLOR?\\|$COLOR?\\s+/;\n+\n my @removed;\n my @added;\n my $in_hunk;\n@@ -32,12 +36,12 @@ $SIG{PIPE} = 'DEFAULT';\n while (<>) {\n \tif (!$in_hunk) {\n \t\tprint;\n-\t\t$in_hunk = /^$COLOR*\\@/;\n+\t\t$in_hunk = /^$GRAPH*$COLOR*\\@/;\n \t}\n-\telsif (/^$COLOR*-/) {\n+\telsif (/^$GRAPH*$COLOR*-/) {\n \t\tpush @removed, $_;\n \t}\n-\telsif (/^$COLOR*\\+/) {\n+\telsif (/^$GRAPH*$COLOR*\\+/) {\n \t\tpush @added, $_;\n \t}\n \telse {\n@@ -46,7 +50,7 @@ while (<>) {\n \t\t@added = ();\n \n \t\tprint;\n-\t\t$in_hunk = /^$COLOR*[\\@ ]/;\n+\t\t$in_hunk = /^$GRAPH*$COLOR*[\\@ ]/;\n \t}\n \n \t# Most of the time there is enough output to keep things streaming,\n@@ -163,6 +167,9 @@ sub highlight_pair {\n \t}\n }\n \n+# we split either by $COLOR or by character. This has the side effect of\n+# leaving in graph cruft. It works because the graph cruft does not contain \"-\"\n+# or \"+\"\n sub split_line {\n \tlocal $_ = shift;\n \treturn utf8::decode($_) ?\n@@ -211,8 +218,8 @@ sub is_pair_interesting {\n \tmy $suffix_a = join('', @$a[($sa+1)..$#$a]);\n \tmy $suffix_b = join('', @$b[($sb+1)..$#$b]);\n \n-\treturn $prefix_a !~ /^$COLOR*-$BORING*$/ ||\n-\t $prefix_b !~ /^$COLOR*\\+$BORING*$/ ||\n+\treturn $prefix_a !~ /^$GRAPH*$COLOR*-$BORING*$/ ||\n+\t $prefix_b !~ /^$GRAPH*$COLOR*\\+$BORING*$/ ||\n \t $suffix_a !~ /^$BORING*$/ ||\n \t $suffix_b !~ /^$BORING*$/;\n }\ndiff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh\nindex 54e11fe..e42232d 100755\n--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh\n+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n@@ -209,7 +209,7 @@ test_expect_failure 'diff-highlight highlights mismatched hunk size' '\n \n # TODO add multi-byte test\n \n-test_expect_failure 'diff-highlight works with the --graph option' '\n+test_expect_success 'diff-highlight works with the --graph option' '\n \tdh_test_setup_history &&\n \n \t# topo-order so that the order of the commits is the same as with --graph\n-- \n2.9.3\n\n"},{"id":"300562","messageId":"20160830140714.19048-3-henderson.bj@gmail.com","threadId":"43885","inReplyTo":"20160830140714.19048-1-henderson.bj@gmail.com","subject":"[PATCH v4 2/3] diff-highlight: add failing test for handling --graph output","fromName":"Brian Henderson","fromEmail":"henderson.bj@gmail.com","sentAt":"2016-08-30T14:07:13Z","receivedAt":"2016-08-30T14:07:59Z","isPatch":true,"sender":{"key":"henderson.bj@gmail.com","avatar":"https://avatars.githubusercontent.com/u/26167?v=4"},"body":"Signed-off-by: Brian Henderson \n---\n contrib/diff-highlight/t/t9400-diff-highlight.sh | 60 ++++++++++++++++++++++++\n 1 file changed, 60 insertions(+)\n\ndiff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh\nindex 7c303f7..54e11fe 100755\n--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh\n+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n@@ -49,6 +49,55 @@ test_strip_patch_header () {\n \tsed -n '/^@@/,$p' $*\n }\n \n+# dh_test_setup_history generates a contrived graph such that we have at least\n+# 1 nesting (E) and 2 nestings (F).\n+#\n+#\t A branch\n+#\t /\n+#\tD---E---F master\n+#\n+#\tgit log --all --graph\n+#\t* commit\n+#\t| A\n+#\t| * commit\n+#\t| | F\n+#\t| * commit\n+#\t|/\n+#\t| E\n+#\t* commit\n+#\t D\n+#\n+dh_test_setup_history () {\n+\techo \"file1\" >file1 &&\n+\techo \"file2\" >file2 &&\n+\techo \"file3\" >file3 &&\n+\n+\tcat file1 >file &&\n+\tgit add file &&\n+\tgit commit -m \"D\" &&\n+\n+\tgit checkout -b branch &&\n+\tcat file2 >file &&\n+\tgit commit -am \"A\" &&\n+\n+\tgit checkout master &&\n+\tcat file2 >file &&\n+\tgit commit -am \"E\" &&\n+\n+\tcat file3 >file &&\n+\tgit commit -am \"F\"\n+}\n+\n+left_trim () {\n+\t\"$PERL_PATH\" -pe 's/^\\s+//'\n+}\n+\n+trim_graph () {\n+\t# graphs start with * or |\n+\t# followed by a space or / or \\\n+\t\"$PERL_PATH\" -pe 's@^((\\*|\\|)( |/|\\\\))+@@'\n+}\n+\n test_expect_success 'diff-highlight highlights the beginning of a line' '\n \tcat >a <<-\\EOF &&\n \t\taaa\n@@ -160,4 +209,15 @@ test_expect_failure 'diff-highlight highlights mismatched hunk size' '\n \n # TODO add multi-byte test\n \n+test_expect_failure 'diff-highlight works with the --graph option' '\n+\tdh_test_setup_history &&\n+\n+\t# topo-order so that the order of the commits is the same as with --graph\n+\t# trim graph elements so we can do a diff\n+\t# trim leading space because our trim_graph is not perfect\n+\tgit log --branches -p --topo-order | \"$DIFF_HIGHLIGHT\" | left_trim >graph.exp &&\n+\tgit log --branches -p --graph | \"$DIFF_HIGHLIGHT\" | trim_graph | left_trim >graph.act &&\n+\ttest_cmp graph.exp graph.act\n+'\n+\n test_done\n-- \n2.9.3\n\n"},{"id":"300619","messageId":"20160831050229.cabhfzqcpcpvkugl@sigill.intra.peff.net","threadId":"43885","inReplyTo":"20160830140714.19048-1-henderson.bj@gmail.com","subject":"Re: [PATCH v4 0/3] diff-highlight: add support for --graph option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-08-31T05:02:30Z","receivedAt":"2016-08-31T05:02:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 30, 2016 at 07:07:11AM -0700, Brian Henderson wrote:\n\n> On Mon, Aug 29, 2016 at 02:37:46PM -0700, Junio C Hamano wrote:\n> > Brian Henderson writes:\n> >\n> > > How does this look?\n> > >\n> > > Drawing the graph helped me a lot in figuring out what I was\n> > > actually testing. thanks!\n> >\n> > Yeah, I also am pleased to see the picture of what is being tested\n> > in the test script.\n> >\n> > With your sign-off, they would have been almost perfect ;-).\n> \n> doh. fixed.\n> \n> I left the subject as v4, probably mostly because I have this weird aversion to\n> increasing version numbers :) but I justified it by thinking that the actual\n> patch set isn't changing, I just added the sign-off (and updated the commit\n> messages per Jeff.) Hope that's ok.\n\nThanks. Here are a few patches to go on top. The first one could\narguably be squashed into your first patch (and I don't mind if Junio\nwants to do so while applying, but I don't think it's worth you\nre-sending).\n\nThe second one fleshes out the test scripts a bit, now that we have them\n(yay!).\n\nAnd the third fixes a bug that was reported to me off-list. I held back\nbecause it touches the same lines as your topic (and as a bonus, I was\nnow able to write a test for it). It could be its own topic branch that\ngraduates separately, but seeing as it's contrib, I don't mind one big\ndiff-highlight potpourri topic if it makes things simpler.\n\n [1/3]: diff-highlight: ignore test cruft\n [2/3]: diff-highlight: add multi-byte tests\n [3/3]: diff-highlight: avoid highlighting combined diffs\n\n-Peff\n"},{"id":"300620","messageId":"20160831050253.5diq5coyedgpaiil@sigill.intra.peff.net","threadId":"43885","inReplyTo":"20160831050229.cabhfzqcpcpvkugl@sigill.intra.peff.net","subject":"[PATCH 1/3] diff-highlight: ignore test cruft","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-08-31T05:02:53Z","receivedAt":"2016-08-31T05:03:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"These are the same as in the normal t/.gitignore, with the\nexception of \".prove\", as our Makefile does not support it.\n\nSigned-off-by: Jeff King \n---\n contrib/diff-highlight/t/.gitignore | 2 ++\n 1 file changed, 2 insertions(+)\n create mode 100644 contrib/diff-highlight/t/.gitignore\n\ndiff --git a/contrib/diff-highlight/t/.gitignore b/contrib/diff-highlight/t/.gitignore\nnew file mode 100644\nindex 0000000..7dcbb23\n--- /dev/null\n+++ b/contrib/diff-highlight/t/.gitignore\n@@ -0,0 +1,2 @@\n+/trash directory*\n+/test-results\n-- \n2.10.0.rc2.125.gcfb3d08\n\n"},{"id":"300621","messageId":"20160831050309.3k43nn737ztcajiz@sigill.intra.peff.net","threadId":"43885","inReplyTo":"20160831050229.cabhfzqcpcpvkugl@sigill.intra.peff.net","subject":"[PATCH 2/3] diff-highlight: add multi-byte tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-08-31T05:03:10Z","receivedAt":"2016-08-31T05:03:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Now that we have a test suite for diff highlight, we can\nshow off the improvements from 8d00662 (diff-highlight: do\nnot split multibyte characters, 2015-04-03).\n\nWhile we're at it, we can also add another case that\n_doesn't_ work: combining code points are treated as their\nown unit, which means that we may stick colors between them\nand the character they are modifying (with the result that\nthe color is not shown in an xterm, though it's possible\nthat other terminals err the other way, and show the color\nbut not the accent). There's no fix here, but let's\ndocument it as a failure.\n\nSigned-off-by: Jeff King \n---\n contrib/diff-highlight/t/t9400-diff-highlight.sh | 36 +++++++++++++++++++++++-\n 1 file changed, 35 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh\nindex e42232d..7d034aa 100755\n--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh\n+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n@@ -207,7 +207,41 @@ test_expect_failure 'diff-highlight highlights mismatched hunk size' '\n \tEOF\n '\n \n-# TODO add multi-byte test\n+# These two code points share the same leading byte in UTF-8 representation;\n+# a naive byte-wise diff would highlight only the second byte.\n+#\n+# - U+00f3 (\"o\" with acute)\n+o_accent=$(printf '\\303\\263')\n+# - U+00f8 (\"o\" with stroke)\n+o_stroke=$(printf '\\303\\270')\n+\n+test_expect_success 'diff-highlight treats multibyte utf-8 as a unit' '\n+\techo \"unic${o_accent}de\" >a &&\n+\techo \"unic${o_stroke}de\" >b &&\n+\tdh_test a b <<-EOF\n+\t\t@@ -1 +1 @@\n+\t\t-unic${CW}${o_accent}${CR}de\n+\t\t+unic${CW}${o_stroke}${CR}de\n+\tEOF\n+'\n+\n+# Unlike the UTF-8 above, these are combining code points which are meant\n+# to modify the character preceding them:\n+#\n+# - U+0301 (combining acute accent)\n+combine_accent=$(printf '\\314\\201')\n+# - U+0302 (combining circumflex)\n+combine_circum=$(printf '\\314\\202')\n+\n+test_expect_failure 'diff-highlight treats combining code points as a unit' '\n+\techo \"unico${combine_accent}de\" >a &&\n+\techo \"unico${combine_circum}de\" >b &&\n+\tdh_test a b <<-EOF\n+\t\t@@ -1 +1 @@\n+\t\t-unic${CW}o${combine_accent}${CR}de\n+\t\t+unic${CW}o${combine_circum}${CR}de\n+\tEOF\n+'\n \n test_expect_success 'diff-highlight works with the --graph option' '\n \tdh_test_setup_history &&\n-- \n2.10.0.rc2.125.gcfb3d08\n\n"},{"id":"300623","messageId":"20160831050538.53fki52cvwlmkd6y@sigill.intra.peff.net","threadId":"43885","inReplyTo":"20160831050229.cabhfzqcpcpvkugl@sigill.intra.peff.net","subject":"[PATCH 3/3] diff-highlight: avoid highlighting combined diffs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-08-31T05:05:38Z","receivedAt":"2016-08-31T05:05:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The algorithm in diff-highlight only understands how to look\nat two sides of a diff; it cannot correctly handle combined\ndiffs with multiple preimages. Often highlighting does not\ntrigger at all for these diffs because the line counts do\nnot match up. E.g., if we see:\n\n - ours\n -theirs\n ++resolved\n\nwe would not bother highlighting; it naively looks like a\nsingle line went away, and then a separate hunk added\nanother single line.\n\nBut of course there are exceptions. E.g., if the other side\ndeleted the line, we might see:\n\n - ours\n ++resolved\n\nwhich looks like we dropped \" ours\" and added \"+resolved\".\nThis is only a small highlighting glitch (we highlight the\nspace and the \"+\" along with the content), but it's also the\ntip of the iceberg. Even if we learned to find the true\ncontent here (by noticing we are in a 3-way combined diff\nand marking _two_ characters from the front of the line as\nuninteresting), there are other more complicated cases where\nwe really do need to handle a 3-way hunk.\n\nLet's just punt for now; we can recognize combined diffs by\nthe presence of extra \"@\" symbols in the hunk header, and\ntreat them as non-diff content.\n\nSigned-off-by: Jeff King \n---\n contrib/diff-highlight/diff-highlight | 2 +-\n contrib/diff-highlight/t/t9400-diff-highlight.sh | 37 ++++++++++++++++++++++++\n 2 files changed, 38 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/diff-highlight/diff-highlight b/contrib/diff-highlight/diff-highlight\nindex 9280c88..81bd804 100755\n--- a/contrib/diff-highlight/diff-highlight\n+++ b/contrib/diff-highlight/diff-highlight\n@@ -36,7 +36,7 @@ $SIG{PIPE} = 'DEFAULT';\n while (<>) {\n \tif (!$in_hunk) {\n \t\tprint;\n-\t\t$in_hunk = /^$GRAPH*$COLOR*\\@/;\n+\t\t$in_hunk = /^$GRAPH*$COLOR*\\@\\@ /;\n \t}\n \telsif (/^$GRAPH*$COLOR*-/) {\n \t\tpush @removed, $_;\ndiff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh\nindex 7d034aa..64dd9f7 100755\n--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh\n+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n@@ -254,4 +254,41 @@ test_expect_success 'diff-highlight works with the --graph option' '\n \ttest_cmp graph.exp graph.act\n '\n \n+# Most combined diffs won't meet diff-highlight's line-number filter. So we\n+# create one here where one side drops a line and the other modifies it. That\n+# should result in a diff like:\n+#\n+# - modified content\n+# ++resolved content\n+#\n+# which naively looks like one side added \"+resolved\".\n+test_expect_success 'diff-highlight ignores combined diffs' '\n+\techo \"content\" >file &&\n+\tgit add file &&\n+\tgit commit -m base &&\n+\n+\t>file &&\n+\tgit commit -am master &&\n+\n+\tgit checkout -b other HEAD^ &&\n+\techo \"modified content\" >file &&\n+\tgit commit -am other &&\n+\n+\ttest_must_fail git merge master &&\n+\techo \"resolved content\" >file &&\n+\tgit commit -am resolved &&\n+\n+\tcat >expect <<-\\EOF &&\n+\t--- a/file\n+\t+++ b/file\n+\t@@@ -1,1 -1,0 +1,1 @@@\n+\t- modified content\n+\t++resolved content\n+\tEOF\n+\n+\tgit show -c | \"$DIFF_HIGHLIGHT\" >actual.raw &&\n+\tsed -n \"/^---/,\\$p\" actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.10.0.rc2.125.gcfb3d08\n"}]}