{"thread":{"id":"46191","subject":"[PATCH] diff-highlight: split code into module","startedAt":"2017-06-15T16:31:02Z","lastAt":"2017-06-16T04:31:44Z","messageCount":5,"participants":["Jeff King","Junio C Hamano","Scott Baker"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"322361","messageId":"20170615163055.b63hcivjogafkerq@sigill.intra.peff.net","threadId":"46191","inReplyTo":null,"subject":"[PATCH] diff-highlight: split code into module","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-06-15T16:30:55Z","receivedAt":"2017-06-15T16:31:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The diff-so-fancy project is also written in perl, and most\nof its users pipe diffs through both diff-highlight and\ndiff-so-fancy. It would be nice if this could be done in a\nsingle script. So let's pull most of diff-highlight's code\ninto its own module which can be used by diff-so-fancy.\n\nIn addition, we'll abstract a few basic items like reading\nfrom stdio so that a script using the module can do more\nprocessing before or after diff-highlight handles the lines.\nSee the README update for more details.\n\nOne small downside is that the diff-highlight script must\nnow be built using the Makefile. There are ways around this,\nbut it quickly gets into perl arcana. Let's go with the\nsimple solution. As a bonus, our Makefile now respects the\nPERL_PATH variable if it is set.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nScott and I discussed this off-list, and this was the least-gross\nsolution I came up with.  The plan would be for diff-so-fancy to pull in\nthis copy of diff-highlight from git.git and have a wrapper script\nsimilar to the diff-highlight.perl found here.\n\n contrib/diff-highlight/.gitignore                  |  2 ++\n .../{diff-highlight => DiffHighlight.pm}           | 40 +++++++++++++---------\n contrib/diff-highlight/Makefile                    | 21 ++++++++++--\n contrib/diff-highlight/README                      | 30 ++++++++++++++++\n contrib/diff-highlight/diff-highlight.perl         |  8 +++++\n 5 files changed, 82 insertions(+), 19 deletions(-)\n create mode 100644 contrib/diff-highlight/.gitignore\n rename contrib/diff-highlight/{diff-highlight => DiffHighlight.pm} (91%)\n mode change 100755 => 100644\n create mode 100644 contrib/diff-highlight/diff-highlight.perl\n\ndiff --git a/contrib/diff-highlight/.gitignore b/contrib/diff-highlight/.gitignore\nnew file mode 100644\nindex 000000000..c07454824\n--- /dev/null\n+++ b/contrib/diff-highlight/.gitignore\n@@ -0,0 +1,2 @@\n+shebang.perl\n+diff-highlight\ndiff --git a/contrib/diff-highlight/diff-highlight b/contrib/diff-highlight/DiffHighlight.pm\nold mode 100755\nnew mode 100644\nsimilarity index 91%\nrename from contrib/diff-highlight/diff-highlight\nrename to contrib/diff-highlight/DiffHighlight.pm\nindex 81bd8040e..663992e53\n--- a/contrib/diff-highlight/diff-highlight\n+++ b/contrib/diff-highlight/DiffHighlight.pm\n@@ -1,4 +1,4 @@\n-#!/usr/bin/perl\n+package DiffHighlight;\n \n use 5.008;\n use warnings FATAL => 'all';\n@@ -29,13 +29,14 @@ my @removed;\n my @added;\n my $in_hunk;\n \n-# Some scripts may not realize that SIGPIPE is being ignored when launching the\n-# pager--for instance scripts written in Python.\n-$SIG{PIPE} = 'DEFAULT';\n+our $line_cb = sub { print @_ };\n+our $flush_cb = sub { local $| = 1 };\n+\n+sub handle_line {\n+\tlocal $_ = shift;\n \n-while (<>) {\n \tif (!$in_hunk) {\n-\t\tprint;\n+\t\t$line_cb->($_);\n \t\t$in_hunk = /^$GRAPH*$COLOR*\\@\\@ /;\n \t}\n \telsif (/^$GRAPH*$COLOR*-/) {\n@@ -49,7 +50,7 @@ while (<>) {\n \t\t@removed = ();\n \t\t@added = ();\n \n-\t\tprint;\n+\t\t$line_cb->($_);\n \t\t$in_hunk = /^$GRAPH*$COLOR*[\\@ ]/;\n \t}\n \n@@ -62,15 +63,22 @@ while (<>) {\n \t# place to flush. Flushing on a blank line is a heuristic that\n \t# happens to match git-log output.\n \tif (!length) {\n-\t\tlocal $| = 1;\n+\t\t$flush_cb->();\n \t}\n }\n \n-# Flush any queued hunk (this can happen when there is no trailing context in\n-# the final diff of the input).\n-show_hunk(\\@removed, \\@added);\n+sub flush {\n+\t# Flush any queued hunk (this can happen when there is no trailing\n+\t# context in the final diff of the input).\n+\tshow_hunk(\\@removed, \\@added);\n+}\n \n-exit 0;\n+sub highlight_stdin {\n+\twhile (<STDIN>) {\n+\t\thandle_line($_);\n+\t}\n+\tflush();\n+}\n \n # Ideally we would feed the default as a human-readable color to\n # git-config as the fallback value. But diff-highlight does\n@@ -88,7 +96,7 @@ sub show_hunk {\n \n \t# If one side is empty, then there is nothing to compare or highlight.\n \tif (!@$a || !@$b) {\n-\t\tprint @$a, @$b;\n+\t\t$line_cb->(@$a, @$b);\n \t\treturn;\n \t}\n \n@@ -97,17 +105,17 @@ sub show_hunk {\n \t# stupid, and only handle multi-line hunks that remove and add the same\n \t# number of lines.\n \tif (@$a != @$b) {\n-\t\tprint @$a, @$b;\n+\t\t$line_cb->(@$a, @$b);\n \t\treturn;\n \t}\n \n \tmy @queue;\n \tfor (my $i = 0; $i < @$a; $i++) {\n \t\tmy ($rm, $add) = highlight_pair($a->[$i], $b->[$i]);\n-\t\tprint $rm;\n+\t\t$line_cb->($rm);\n \t\tpush @queue, $add;\n \t}\n-\tprint @queue;\n+\t$line_cb->(@queue);\n }\n \n sub highlight_pair {\ndiff --git a/contrib/diff-highlight/Makefile b/contrib/diff-highlight/Makefile\nindex 901872452..fbf5c5824 100644\n--- a/contrib/diff-highlight/Makefile\n+++ b/contrib/diff-highlight/Makefile\n@@ -1,5 +1,20 @@\n-# nothing to build\n-all:\n+all: diff-highlight\n \n-test:\n+PERL_PATH = /usr/bin/perl\n+-include ../../config.mak\n+\n+PERL_PATH_SQ = $(subst ','\\'',$(PERL_PATH))\n+\n+diff-highlight: shebang.perl DiffHighlight.pm diff-highlight.perl\n+\tcat $^ >$@+\n+\tchmod +x $@+\n+\tmv $@+ $@\n+\n+shebang.perl: FORCE\n+\t@echo '#!$(PERL_PATH_SQ)' >$@+\n+\t@cmp $@+ $@ >/dev/null 2>/dev/null || mv $@+ $@\n+\n+test: all\n \t$(MAKE) -C t\n+\n+.PHONY: FORCE\ndiff --git a/contrib/diff-highlight/README b/contrib/diff-highlight/README\nindex 836b97a73..d4c234317 100644\n--- a/contrib/diff-highlight/README\n+++ b/contrib/diff-highlight/README\n@@ -99,6 +99,36 @@ newHighlight = \"black #aaffaa\"\n ---------------------------------------------\n \n \n+Using diff-highlight as a module\n+--------------------------------\n+\n+If you want to pre- or post- process the highlighted lines as part of\n+another perl script, you can use the DiffHighlight module. You can\n+either \"require\" it or just cat the module together with your script (to\n+avoid run-time dependencies).\n+\n+Your script may set up one or more of the following variables:\n+\n+  - $DiffHighlight::line_cb - this should point to a function which is\n+    called whenever DiffHighlight has lines (which may contain\n+    highlights) to output. The default function prints each line to\n+    stdout. Note that the function may be called with multiple lines.\n+\n+  - $DiffHighlight::flush_cb - this should point to a function which\n+    flushes the output (because DiffHighlight believes it has completed\n+    processing a logical chunk of input). The default function flushes\n+    stdout.\n+\n+The script may then feed lines, one at a time, to DiffHighlight::handle_line().\n+When lines are done processing, they will be fed to $line_cb. Note that\n+DiffHighlight may queue up many input lines (to analyze a whole hunk)\n+before calling $line_cb. After providing all lines, call\n+DiffHighlight::flush() to flush any unprocessed lines.\n+\n+If you just want to process stdin, DiffHighlight::highlight_stdin()\n+is a convenience helper which will loop and flush for you.\n+\n+\n Bugs\n ----\n \ndiff --git a/contrib/diff-highlight/diff-highlight.perl b/contrib/diff-highlight/diff-highlight.perl\nnew file mode 100644\nindex 000000000..9b3e9c1f4\n--- /dev/null\n+++ b/contrib/diff-highlight/diff-highlight.perl\n@@ -0,0 +1,8 @@\n+package main;\n+\n+# Some scripts may not realize that SIGPIPE is being ignored when launching the\n+# pager--for instance scripts written in Python.\n+$SIG{PIPE} = 'DEFAULT';\n+\n+DiffHighlight::highlight_stdin();\n+exit 0;\n-- \n2.13.1.766.g6bea926c5\n"},{"id":"322376","messageId":"xmqqpoe512rc.fsf@gitster.mtv.corp.google.com","threadId":"46191","inReplyTo":"20170615163055.b63hcivjogafkerq@sigill.intra.peff.net","subject":"Re: [PATCH] diff-highlight: split code into module","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-06-15T19:15:19Z","receivedAt":"2017-06-15T19:15:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> The diff-so-fancy project is also written in perl, and most\n> of its users pipe diffs through both diff-highlight and\n> diff-so-fancy. It would be nice if this could be done in a\n> single script. So let's pull most of diff-highlight's code\n> into its own module which can be used by diff-so-fancy.\n>\n> In addition, we'll abstract a few basic items like reading\n> from stdio so that a script using the module can do more\n> processing before or after diff-highlight handles the lines.\n> See the README update for more details.\n>\n> One small downside is that the diff-highlight script must\n> now be built using the Makefile. There are ways around this,\n> but it quickly gets into perl arcana. Let's go with the\n> simple solution. As a bonus, our Makefile now respects the\n> PERL_PATH variable if it is set.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> Scott and I discussed this off-list, and this was the least-gross\n> solution I came up with.  The plan would be for diff-so-fancy to pull in\n> this copy of diff-highlight from git.git and have a wrapper script\n> similar to the diff-highlight.perl found here.\n>\n>  contrib/diff-highlight/.gitignore                  |  2 ++\n>  .../{diff-highlight => DiffHighlight.pm}           | 40 +++++++++++++---------\n>  contrib/diff-highlight/Makefile                    | 21 ++++++++++--\n>  contrib/diff-highlight/README                      | 30 ++++++++++++++++\n>  contrib/diff-highlight/diff-highlight.perl         |  8 +++++\n>  5 files changed, 82 insertions(+), 19 deletions(-)\n>  create mode 100644 contrib/diff-highlight/.gitignore\n>  rename contrib/diff-highlight/{diff-highlight => DiffHighlight.pm} (91%)\n>  mode change 100755 => 100644\n>  create mode 100644 contrib/diff-highlight/diff-highlight.perl\n\nDo you want +x bit for the last one?  I could throw that bit in\nwhile queuing if you want.\n\nThanks.\n"},{"id":"322377","messageId":"xmqqlgot12o4.fsf@gitster.mtv.corp.google.com","threadId":"46191","inReplyTo":"xmqqpoe512rc.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] diff-highlight: split code into module","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-06-15T19:17:15Z","receivedAt":"2017-06-15T19:17:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>>  contrib/diff-highlight/.gitignore                  |  2 ++\n>>  .../{diff-highlight => DiffHighlight.pm}           | 40 +++++++++++++---------\n>>  contrib/diff-highlight/Makefile                    | 21 ++++++++++--\n>>  contrib/diff-highlight/README                      | 30 ++++++++++++++++\n>>  contrib/diff-highlight/diff-highlight.perl         |  8 +++++\n>>  5 files changed, 82 insertions(+), 19 deletions(-)\n>>  create mode 100644 contrib/diff-highlight/.gitignore\n>>  rename contrib/diff-highlight/{diff-highlight => DiffHighlight.pm} (91%)\n>>  mode change 100755 => 100644\n>>  create mode 100644 contrib/diff-highlight/diff-highlight.perl\n>\n> Do you want +x bit for the last one?  I could throw that bit in\n> while queuing if you want.\n\nAh, I do not think you want it, as this is not something to be\nexecuted as-is (it is a source file, which diff-highlight that is\nexecutable is made out of).\n\n"},{"id":"322383","messageId":"70fc9096-ce97-e781-3cca-bae7c3ce5b20@perturb.org","threadId":"46191","inReplyTo":"xmqqpoe512rc.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] diff-highlight: split code into module","fromName":"Scott Baker","fromEmail":"scott@perturb.org","sentAt":"2017-06-15T20:05:59Z","receivedAt":"2017-06-15T20:16:50Z","isPatch":true,"sender":{"key":"scott@perturb.org","avatar":null},"body":"On 06/15/2017 12:15 PM, Junio C Hamano wrote:\n> Do you want +x bit for the last one?  I could throw that bit in\n> while queuing if you want.\n>\n> Thanks.\n>\nYa probably best.\n\n\n"},{"id":"322437","messageId":"20170616043137.xqgxpnop2svqe6io@sigill.intra.peff.net","threadId":"46191","inReplyTo":"xmqqlgot12o4.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] diff-highlight: split code into module","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-06-16T04:31:37Z","receivedAt":"2017-06-16T04:31:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 15, 2017 at 12:17:15PM -0700, Junio C Hamano wrote:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> >>  contrib/diff-highlight/.gitignore                  |  2 ++\n> >>  .../{diff-highlight => DiffHighlight.pm}           | 40 +++++++++++++---------\n> >>  contrib/diff-highlight/Makefile                    | 21 ++++++++++--\n> >>  contrib/diff-highlight/README                      | 30 ++++++++++++++++\n> >>  contrib/diff-highlight/diff-highlight.perl         |  8 +++++\n> >>  5 files changed, 82 insertions(+), 19 deletions(-)\n> >>  create mode 100644 contrib/diff-highlight/.gitignore\n> >>  rename contrib/diff-highlight/{diff-highlight => DiffHighlight.pm} (91%)\n> >>  mode change 100755 => 100644\n> >>  create mode 100644 contrib/diff-highlight/diff-highlight.perl\n> >\n> > Do you want +x bit for the last one?  I could throw that bit in\n> > while queuing if you want.\n> \n> Ah, I do not think you want it, as this is not something to be\n> executed as-is (it is a source file, which diff-highlight that is\n> executable is made out of).\n\nYes, exactly. Although many of our .sh files, though generally\n\"compiled\" with the Makefile, still have the executable bit. I don't\nmind adding it if we'd rather follow that pattern in general.\n\n-Peff\n"}]}