unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Juri Linkov <juri@linkov.net>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 25751@debbugs.gnu.org, antoine.levitt@gmail.com
Subject: bug#25751: Query replace lazy highlighting
Date: Sat, 18 Feb 2017 00:52:47 +0200	[thread overview]
Message-ID: <87poig2zwg.fsf@localhost> (raw)
In-Reply-To: <83d1ehxqzd.fsf@gnu.org> (Eli Zaretskii's message of "Fri, 17 Feb 2017 08:37:26 +0200")

>> > (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.
>
> I think the problem is not that we remove the highlight and add it
> anew, the problem is that there's a redisplay cycle in between the
> removal and the following addition.  The fact that setting
> lazy-highlight-initial-delay alleviates the problem to some extent,
> but still leaves the flicker tells me that there's a call to sit-for
> or some such somewhere in the code that processes replacements, and
> the removal and addition of the highlight are on two different sides
> of that sit-for call.  One possible solution would be to remove the
> highlight and add it without triggering redisplay, then I'd expect the
> flicker to go away.
>
> Does this make sense?

This feature is called _lazy_ highlighting where _lazy_ implies that it's
intended to highlight matches much later after a lot of redisplay cycles.
Currently its steps are the following:

1. Delete old overlays
2. Wait for 0.25 seconds of idle time
3. Highlight first 20 matches
4. Wait 0.0625 seconds
5. Highlight next 20 matches
6. Repeat 4 until all matches are highlighted

It makes a big improvement to avoid flicker by moving ‘Delete old overlays’
to the end when all new matches are highlighted, essentially extending
lazy-highlight with ‘lazy-cleanup’, i.e. making its cleanup _lazy_ as well.

Here is a more tested patch that works especially well for the most
frequently used isearch operation of adding characters into the
search string - there is no more flicker while typing the search string.

diff --git a/lisp/isearch.el b/lisp/isearch.el
index 5262435..f8796dd 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)
@@ -3122,13 +3123,22 @@ (define-obsolete-variable-alias 'isearch-lazy-highlight-word
 (defvar isearch-lazy-highlight-forward nil)
 (defvar isearch-lazy-highlight-error nil)
 
-(defun lazy-highlight-cleanup (&optional force)
+(defun lazy-highlight-cleanup (&optional force lazy)
   "Stop lazy highlighting and remove extra highlighting from current buffer.
 FORCE non-nil means do it whether or not `lazy-highlight-cleanup'
 is nil.  This function is called when exiting an incremental search if
 `lazy-highlight-cleanup' is non-nil."
   (interactive '(t))
-  (if (or force lazy-highlight-cleanup)
+  (if (eq lazy t)
+      (setq isearch-lazy-highlight-overlays-old
+            (append isearch-lazy-highlight-overlays-old
+                    isearch-lazy-highlight-overlays))
+    (while isearch-lazy-highlight-overlays-old
+      (delete-overlay (car isearch-lazy-highlight-overlays-old))
+      (setq isearch-lazy-highlight-overlays-old
+            (cdr isearch-lazy-highlight-overlays-old))))
+  (when (and (or force lazy-highlight-cleanup)
+             (not lazy))
       (while isearch-lazy-highlight-overlays
         (delete-overlay (car isearch-lazy-highlight-overlays))
         (setq isearch-lazy-highlight-overlays
@@ -3173,7 +3183,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
+    (lazy-highlight-cleanup t (not (equal isearch-string "")))
     (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,7 +3325,8 @@ (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))))))))
-	    (unless nomore
+            (if nomore
+                (lazy-highlight-cleanup t 'delete-old)
 	      (setq isearch-lazy-highlight-timer
 		    (run-at-time lazy-highlight-interval nil
 				 'isearch-lazy-highlight-update)))))))))






  reply	other threads:[~2017-02-17 22:52 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-16 13:18 bug#25751: Query replace lazy highlighting Antoine Levitt
2017-02-16 21:01 ` Juri Linkov
2017-02-16 22:45   ` Juri Linkov
2017-02-17  6:37   ` Eli Zaretskii
2017-02-17 22:52     ` Juri Linkov [this message]
2017-02-18  8:13       ` Eli Zaretskii
2017-02-18 23:17         ` Juri Linkov
2017-02-18 23:51           ` Drew Adams
2017-02-20  0:30             ` Juri Linkov
2017-02-21 23:22           ` Juri Linkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87poig2zwg.fsf@localhost \
    --to=juri@linkov.net \
    --cc=25751@debbugs.gnu.org \
    --cc=antoine.levitt@gmail.com \
    --cc=eliz@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).