{"thread":{"id":"42949","subject":"[PATCH 0/3] diff-highlight: add support for git log --graph output.","startedAt":"2016-07-28T16:27:20Z","lastAt":"2016-07-29T07:47:03Z","messageCount":2,"participants":["Brian Henderson","Jakub Narębski"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"292426","messageId":"20160728162712.GA29220@tci.corp.yp.com","threadId":"42949","inReplyTo":null,"subject":"[PATCH 0/3] diff-highlight: add support for git log --graph output.","fromName":"Brian Henderson","fromEmail":"henderson.bj@gmail.com","sentAt":"2016-07-28T16:27:12Z","receivedAt":"2016-07-28T16:27:20Z","isPatch":true,"sender":{"key":"henderson.bj@gmail.com","avatar":"https://avatars.githubusercontent.com/u/26167?v=4"},"body":"Hi, I've been working with Jeff King on this patch, but he encouraged me to\nemail the list.\n\nI recently learned about the diff-highlight tool and find it very helpful,\nhowever, I frequently use the --graph option to git log which breaks the tool.\nThis patch looks to fix this.\n\nI wanted to try and add some tests as well, but I was unsure what number to\npick for the second digit. As there were limited tests in the contrib directory\n(only t93xx and one t7900) I just chose the next number in the 9xxx range.\nPlease let me know if I need to change it.\n\nI'm also not super happy about my test case for the graph option. If anyone has\nany better ideas, please let me know!\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            |  13 +--\n contrib/diff-highlight/t/Makefile                |  19 ++++\n contrib/diff-highlight/t/t9400-diff-highlight.sh |  76 ++++++++++++++++\n contrib/diff-highlight/t/test-diff-highlight.sh  | 111 +++++++++++++++++++++++\n 5 files changed, 218 insertions(+), 6 deletions(-)\n create mode 100644 contrib/diff-highlight/Makefile\n create mode 100644 contrib/diff-highlight/t/Makefile\n create mode 100644 contrib/diff-highlight/t/t9400-diff-highlight.sh\n create mode 100644 contrib/diff-highlight/t/test-diff-highlight.sh\n\n-- \n2.9.0\n\n\n\nFrom f2468205f5f43b17bfd0398959a1ae66588a8f0d Mon Sep 17 00:00:00 2001\nFrom: Brian Henderson <henderson.bj@gmail.com>\nDate: Thu, 21 Jul 2016 09:43:54 -0700\nSubject: [PATCH 1/3] diff-highlight: add some tests.\n\n---\n contrib/diff-highlight/Makefile                  |  5 ++\n contrib/diff-highlight/t/Makefile                | 19 +++++++\n contrib/diff-highlight/t/t9400-diff-highlight.sh | 63 ++++++++++++++++++++++\n contrib/diff-highlight/t/test-diff-highlight.sh  | 69 ++++++++++++++++++++++++\n 4 files changed, 156 insertions(+)\n create mode 100644 contrib/diff-highlight/Makefile\n create mode 100644 contrib/diff-highlight/t/Makefile\n create mode 100644 contrib/diff-highlight/t/t9400-diff-highlight.sh\n create mode 100644 contrib/diff-highlight/t/test-diff-highlight.sh\n\ndiff --git a/contrib/diff-highlight/Makefile b/contrib/diff-highlight/Makefile\nnew file mode 100644\nindex 0000000..b866259\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..ac81fc0\n--- /dev/null\n+++ b/contrib/diff-highlight/t/Makefile\n@@ -0,0 +1,19 @@\n+-include ../../../config.mak.autogen\n+-include ../../../config.mak\n+\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 \"*** $@ ***\"; sh $@ $(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 100644\nindex 0000000..ca7605f\n--- /dev/null\n+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n@@ -0,0 +1,63 @@\n+#!/bin/sh\n+#\n+# Copyright (C) 2016\n+\n+test_description='Test diff-highlight'\n+\n+. ./test-diff-highlight.sh\n+. $TEST_DIRECTORY/test-lib.sh\n+\n+# PERL is required, but assumed to be present, although not necessarily modern\n+# some tests require 5.8\n+\n+test_expect_success 'diff-highlight highlightes the beginning of a line' '\n+  dh_test \\\n+    \"aaa\\nbbb\\nccc\\n\" \\\n+    \"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 highlightes the end of a line' '\n+  dh_test \\\n+    \"aaa\\nbbb\\nccc\\n\" \\\n+    \"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 highlightes the middle of a line' '\n+  dh_test \\\n+    \"aaa\\nbbb\\nccc\\n\" \\\n+    \"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+  dh_test \\\n+    \"aaa\\nbbb\\nccc\\n\" \\\n+    \"aaa\\n000\\nccc\\n\"\n+'\n+\n+test_expect_success 'diff-highlight does not highlight mismatched hunk size' '\n+  dh_test \\\n+    \"aaa\\nbbb\\n\" \\\n+    \"aaa\\nb0b\\nccc\\n\"\n+'\n+\n+# TODO add multi-byte test\n+\n+test_done\ndiff --git a/contrib/diff-highlight/t/test-diff-highlight.sh b/contrib/diff-highlight/t/test-diff-highlight.sh\nnew file mode 100644\nindex 0000000..9b26271\n--- /dev/null\n+++ b/contrib/diff-highlight/t/test-diff-highlight.sh\n@@ -0,0 +1,69 @@\n+#!/bin/sh\n+#\n+# Copyright (C) 2016\n+\n+CURR_DIR=$(pwd)\n+TEST_OUTPUT_DIRECTORY=$(pwd)\n+TEST_DIRECTORY=\"$CURR_DIR\"/../../../t\n+cmd=diff-highlight\n+CMD=\"$CURR_DIR\"/../$cmd\n+\n+CW=\"\\033[7m\"\n+CR=\"\\033[27m\"\n+\n+export TEST_OUTPUT_DIRECTORY TEST_DIRECTORY CW CR\n+\n+dh_test() {\n+  dh_diff_test \"$@\"\n+  dh_commit_test \"$@\"\n+}\n+\n+dh_diff_test() {\n+  local a=\"$1\" b=\"$2\"\n+\n+  printf \"$a\" > file\n+  git add file\n+\n+  printf \"$b\" > file\n+\n+  git diff file > diff.raw\n+\n+  if test \"$#\" = 3\n+  then\n+    # remove last newline\n+    head -n5 diff.raw | head -c -1 > diff.act\n+    printf \"$3\" >> diff.act\n+  else\n+    cat diff.raw > diff.act\n+  fi\n+\n+  < diff.raw $CMD > diff.exp\n+\n+  diff diff.exp diff.act\n+}\n+\n+dh_commit_test() {\n+  local a=\"$1\" b=\"$2\"\n+\n+  printf \"$a\" > file\n+  git add file\n+  git commit -m\"Add a file\" >/dev/null\n+\n+  printf \"$b\" > file\n+  git commit -am\"Update a file\" >/dev/null\n+\n+  git show > commit.raw\n+\n+  if test \"$#\" = 3\n+  then\n+    # remove last newline\n+    head -n11 commit.raw | head -c -1 > commit.act\n+    printf \"$3\" >> commit.act\n+  else\n+    cat commit.raw > commit.act\n+  fi\n+\n+  < commit.raw $CMD > commit.exp\n+\n+  diff commit.exp commit.act\n+}\n-- \n2.9.0\n\n\n\nFrom 78cca284fb5d0c094dc47947cb4cc54c5a8c0f78 Mon Sep 17 00:00:00 2001\nFrom: Brian Henderson <henderson.bj@gmail.com>\nDate: Tue, 26 Jul 2016 07:49:45 -0700\nSubject: [PATCH 2/3] diff-highlight: add failing test for handling --graph\n output.\n\n---\n contrib/diff-highlight/t/t9400-diff-highlight.sh | 13 ++++++++\n contrib/diff-highlight/t/test-diff-highlight.sh  | 42 ++++++++++++++++++++++++\n 2 files changed, 55 insertions(+)\n\ndiff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh\nindex ca7605f..2fb4869 100644\n--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh\n+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n@@ -60,4 +60,17 @@ test_expect_success 'diff-highlight does not highlight mismatched hunk size' '\n \n # TODO add multi-byte test\n \n+test_expect_success 'diff-highlight highlightes the beginning of a line' '\n+  dh_graph_test \\\n+    \"aaa\\nbbb\\nccc\\n\" \\\n+    \"aaa\\n0bb\\nccc\\n\" \\\n+    \"aaa\\nb0b\\nccc\\n\" \\\n+\"\n+ aaa\n+-${CW}b${CR}bb\n++${CW}0${CR}bb\n+ ccc\n+\"\n+'\n+\n test_done\ndiff --git a/contrib/diff-highlight/t/test-diff-highlight.sh b/contrib/diff-highlight/t/test-diff-highlight.sh\nindex 9b26271..162a694 100644\n--- a/contrib/diff-highlight/t/test-diff-highlight.sh\n+++ b/contrib/diff-highlight/t/test-diff-highlight.sh\n@@ -67,3 +67,45 @@ dh_commit_test() {\n \n   diff commit.exp commit.act\n }\n+\n+dh_graph_test() {\n+  local a=\"$1\" b=\"$2\" c=\"$3\"\n+\n+  {\n+    printf \"$a\" > file\n+    git add file\n+    git commit -m\"Add a file\"\n+\n+    printf \"$b\" > file\n+    git commit -am\"Update a file\"\n+\n+    git checkout -b branch\n+    printf \"$c\" > file\n+    git commit -am\"Update a file on branch\"\n+\n+    git checkout master\n+    printf \"$a\" > file\n+    git commit -am\"Update a file again\"\n+\n+    git checkout branch\n+    printf \"$b\" > file\n+    git commit -am\"Update a file similar to master\"\n+\n+    git merge master\n+    git checkout master\n+    git merge branch --no-ff\n+  } >/dev/null 2>&1\n+\n+  git log -p --graph --no-merges > graph.raw\n+\n+  # git log --graph orders the commits different than git log so we hack it by\n+  # using sed to remove the graph part. We know from other tests, that CMD\n+  # works without the graph, so there should be no diff when running it with\n+  # and without.\n+  < graph.raw sed -e 's\"^\\(*\\|| \\||/\\)\\+\"\"' -e 's\"^  \"\"' | $CMD > graph.exp\n+  < graph.raw $CMD | sed -e 's\"^\\(*\\|| \\||/\\)\\+\"\"' -e 's\"^  \"\"' > graph.act\n+\n+  # ignore whitespace since we're using a hacky sed command to remove the graph\n+  # parts.\n+  diff -b graph.exp graph.act\n+}\n-- \n2.9.0\n\n\n\nFrom 615451541643a07486e209f9f17edae9436b882c Mon Sep 17 00:00:00 2001\nFrom: Brian Henderson <henderson.bj@gmail.com>\nDate: Tue, 26 Jul 2016 07:51:07 -0700\nSubject: [PATCH 3/3] diff-highlight: add support for --graph output.\n\n---\n contrib/diff-highlight/diff-highlight | 13 +++++++------\n 1 file changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/contrib/diff-highlight/diff-highlight b/contrib/diff-highlight/diff-highlight\nindex ffefc31..ec31356 100755\n--- a/contrib/diff-highlight/diff-highlight\n+++ b/contrib/diff-highlight/diff-highlight\n@@ -20,6 +20,7 @@ my @NEW_HIGHLIGHT = (\n my $RESET = \"\\x1b[m\";\n my $COLOR = qr/\\x1b\\[[0-9;]*m/;\n my $BORING = qr/$COLOR|\\s/;\n+my $GRAPH = qr/$COLOR?\\|$COLOR?\\s+/;\n \n my @removed;\n my @added;\n@@ -32,12 +33,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 +47,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@@ -211,8 +212,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 }\n-- \n2.9.0\n\n"},{"id":"292497","messageId":"579B09DD.9000603@gmail.com","threadId":"42949","inReplyTo":"20160728162712.GA29220@tci.corp.yp.com","subject":"Re: [PATCH 0/3] diff-highlight: add support for git log --graph output.","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2016-07-29T07:46:37Z","receivedAt":"2016-07-29T07:47:03Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 2016-07-28 o 18:27, Brian Henderson pisze:\n> Hi, I've been working with Jeff King on this patch, but he encouraged me to\n> email the list.\n> \n> I recently learned about the diff-highlight tool and find it very helpful,\n> however, I frequently use the --graph option to git log which breaks the tool.\n> This patch looks to fix this.\n> \n> I wanted to try and add some tests as well, but I was unsure what number to\n> pick for the second digit. As there were limited tests in the contrib directory\n> (only t93xx and one t7900) I just chose the next number in the 9xxx range.\n> Please let me know if I need to change it.\n> \n> I'm also not super happy about my test case for the graph option. If anyone has\n> any better ideas, please let me know!\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\nCould you please follow SubmittingPatches, and send patches in separate emails,\ninline, with those emails being replies to cover letter (like `git format-patch`\nand `git send-email` would do), and not as attachments to cover letter?\n\nWith email as it stands it is really difficult to review individual\npatches.\n\n> \n>  contrib/diff-highlight/Makefile                  |   5 +\n>  contrib/diff-highlight/diff-highlight            |  13 +--\n>  contrib/diff-highlight/t/Makefile                |  19 ++++\n>  contrib/diff-highlight/t/t9400-diff-highlight.sh |  76 ++++++++++++++++\n>  contrib/diff-highlight/t/test-diff-highlight.sh  | 111 +++++++++++++++++++++++\n>  5 files changed, 218 insertions(+), 6 deletions(-)\n>  create mode 100644 contrib/diff-highlight/Makefile\n>  create mode 100644 contrib/diff-highlight/t/Makefile\n>  create mode 100644 contrib/diff-highlight/t/t9400-diff-highlight.sh\n>  create mode 100644 contrib/diff-highlight/t/test-diff-highlight.sh\n> \n\n"}]}