git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] diff-highlight: correctly match blank lines for flush
@ 2020-09-22  5:00 Jeff King
  0 siblings, 0 replies; only message in thread
From: Jeff King @ 2020-09-22  5:00 UTC (permalink / raw)
  To: git

We try to flush the output from diff-highlight whenever we see a blank
line. That lets you see the output for each commit as soon as it is
generated, even if Git is still chugging away at a diff, or traversing
to find the next commit.

However, our "blank line" match checks length($_). That won't ever be
true, because we haven't chomped the line ending. As a result, we never
flush. Instead, let's use a simple regex which handles line endings in
with the end-of-line marker.

This has been broken since the initial version in 927a13fe87 (contrib:
add diff highlight script, 2011-10-18). Probably nobody noticed because:

  - most output is big enough, or comes fast enough, that it flushes
    anyway. And it can be difficult to notice the difference between
    "show a commit, then pause" and "pause, then show two commits". I
    only noticed because I was viewing "git log" output on a repo with a
    very slow textconv filter.

  - if stdout is going to the terminal (and not another pager like
    less), then the flush isn't necessary. So any manual testing would
    show it appearing to work.

You can easily see the difference with something like:

  echo '* diff=slow' >>.gitattributes
  git -c diff.slow.textconv='sleep 1; cat' \
      -c pager.log='diff-highlight | less' \
      log -p

That should generate one commit every second or so (more if it touches
multiple files), but without this patch it waits for many seconds before
generating several pages of output.

Signed-off-by: Jeff King <peff@peff.net>
---
I had this script for a few years before I sent the patch for contrib/.
This feature _did_ work at some point there, but I lost the "chomp" when
cleaning up the code for the list. :)

 contrib/diff-highlight/DiffHighlight.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm
index e2589922a6..376f577737 100644
--- a/contrib/diff-highlight/DiffHighlight.pm
+++ b/contrib/diff-highlight/DiffHighlight.pm
@@ -112,7 +112,7 @@ sub handle_line {
 	# Since we can receive arbitrary input, there's no optimal
 	# place to flush. Flushing on a blank line is a heuristic that
 	# happens to match git-log output.
-	if (!length) {
+	if (/^$/) {
 		$flush_cb->();
 	}
 }
-- 
2.28.0.1034.ge13a789063

^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2020-09-22  5:00 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-22  5:00 [PATCH] diff-highlight: correctly match blank lines for flush Jeff King

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).