{"thread":{"id":"54270","subject":"[PATCH] diff-highlight: correctly match blank lines for flush","startedAt":"2020-09-22T05:00:36Z","lastAt":"2020-09-22T05:00:36Z","messageCount":1,"participants":["Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"406108","messageId":"20200922050033.GA528837@coredump.intra.peff.net","threadId":"54270","inReplyTo":null,"subject":"[PATCH] diff-highlight: correctly match blank lines for flush","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-22T05:00:33Z","receivedAt":"2020-09-22T05:00:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We try to flush the output from diff-highlight whenever we see a blank\nline. That lets you see the output for each commit as soon as it is\ngenerated, even if Git is still chugging away at a diff, or traversing\nto find the next commit.\n\nHowever, our \"blank line\" match checks length($_). That won't ever be\ntrue, because we haven't chomped the line ending. As a result, we never\nflush. Instead, let's use a simple regex which handles line endings in\nwith the end-of-line marker.\n\nThis has been broken since the initial version in 927a13fe87 (contrib:\nadd diff highlight script, 2011-10-18). Probably nobody noticed because:\n\n  - most output is big enough, or comes fast enough, that it flushes\n    anyway. And it can be difficult to notice the difference between\n    \"show a commit, then pause\" and \"pause, then show two commits\". I\n    only noticed because I was viewing \"git log\" output on a repo with a\n    very slow textconv filter.\n\n  - if stdout is going to the terminal (and not another pager like\n    less), then the flush isn't necessary. So any manual testing would\n    show it appearing to work.\n\nYou can easily see the difference with something like:\n\n  echo '* diff=slow' >>.gitattributes\n  git -c diff.slow.textconv='sleep 1; cat' \\\n      -c pager.log='diff-highlight | less' \\\n      log -p\n\nThat should generate one commit every second or so (more if it touches\nmultiple files), but without this patch it waits for many seconds before\ngenerating several pages of output.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI had this script for a few years before I sent the patch for contrib/.\nThis feature _did_ work at some point there, but I lost the \"chomp\" when\ncleaning up the code for the list. :)\n\n contrib/diff-highlight/DiffHighlight.pm | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm\nindex e2589922a6..376f577737 100644\n--- a/contrib/diff-highlight/DiffHighlight.pm\n+++ b/contrib/diff-highlight/DiffHighlight.pm\n@@ -112,7 +112,7 @@ sub handle_line {\n \t# Since we can receive arbitrary input, there's no optimal\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+\tif (/^$/) {\n \t\t$flush_cb->();\n \t}\n }\n-- \n2.28.0.1034.ge13a789063\n"}]}