Subject: Re: bug#25751: Query replace lazy highlighting

From: Juri Linkov <[email-masked]>
Date: Fri, 17 Feb 2017 00:45:05 +0200

>> When query-replacing and confirming, matches get unhighlighted, and then
>> highlighted again, which is very distracting. E.g. open a file, M-% a ->
>> a, y, other matches of a get unhighlighted then highlighted again.
>>
>> (setq lazy-highlight-initial-delay 0) (shouldn't it be default by the
>> way, at least on graphical displays?) reduces the problem but does not
>> eliminate it (it produces small flickers). There's
>> lazy-highlight-cleanup, but that disables cleanup completely, which I
>> don't want.
>>
>> Can't this be eliminated?
>
> The reason why it works this way is because lazy-highlight is not yet
> optimized to handle changes: it needs to dehighlight the replaced text,
> and to add highlighting in the new replacing text after every replacement.
> I see no simpler solution than to write a new function with a name like
> ‘isearch-lazy-highlight-update-in-region’ that given the region boundaries
> of the last replacement will rehighlight matches in that region.

Actually, there are too many special casing to optimize that makes this
solution too brittle: for example, replacing a string with shorter text
requires highlighting new matches at the window-end minus the length of the
deleted text, i.e. deletion shifts previously invisible text into the view
where matches should be highlighted.

But I realized that fortunately there is a better and simpler solution.
The problem is not specific to replacement, the same flicker exists in isearch,
e.g. while scrolling by one line where all overlays are deleted, and
created again in the same places.  To void flickering the best solution is
to delete old overlays AFTER adding new ones, not BEFORE as it is now -
this works pleasingly well:

diff --git a/lisp/isearch.el b/lisp/isearch.el
index 5262435..9cb5399 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -3101,6 +3101,7 @@ (defun isearch-dehighlight ()
 ;;    only if `isearch-string' is an invalid regexp.
 
 (defvar isearch-lazy-highlight-overlays nil)
+(defvar isearch-lazy-highlight-overlays-old nil)
 (defvar isearch-lazy-highlight-wrapped nil)
 (defvar isearch-lazy-highlight-start-limit nil)
 (defvar isearch-lazy-highlight-end-limit nil)
@@ -3173,7 +3174,7 @@ (defun isearch-lazy-highlight-new-loop (&optional beg end)
 		 (not (equal isearch-error
 			     isearch-lazy-highlight-error))))
     ;; something important did indeed change
-    (lazy-highlight-cleanup t)        ;kill old loop & remove overlays
+    (setq isearch-lazy-highlight-overlays-old isearch-lazy-highlight-overlays)
     (setq isearch-lazy-highlight-error isearch-error)
     ;; It used to check for `(not isearch-error)' here, but actually
     ;; lazy-highlighting might find matches to highlight even when
@@ -3315,6 +3316,11 @@ (defun isearch-lazy-highlight-update ()
 			(setq isearch-lazy-highlight-start (window-group-end))
 			(goto-char (min (or isearch-lazy-highlight-end-limit (point-max))
 					(window-group-end))))))))
+            (when nomore
+              ;; Remove old overlays
+              (let ((isearch-lazy-highlight-overlays isearch-lazy-highlight-overlays-old)
+                    (isearch-lazy-highlight-timer nil))
+                (lazy-highlight-cleanup t)))
 	    (unless nomore
 	      (setq isearch-lazy-highlight-timer
 		    (run-at-time lazy-highlight-interval nil