benma / visual-regexp.el

A regexp/replace command for Emacs with interactive visual feedback
392 stars 28 forks source link

Idle delay before highlight update #68

Closed codecoll closed 2 years ago

codecoll commented 2 years ago

Could you add a config option to delay the update of the highlight according to regexp changes if the user keeps typing? Sometimes with big matched regions in big files, typing is hindered by updates.

The delay can be a simple sit-for to make sure the user is not typing for the configured delay time.

So, something like

   (if (sit-for vr/update-idle-delay)
      ... do update

where vr/update-idle-delay is the number of seconds to wait. I usually set it for 0.3 or 0.5 in other packages, but the default can be 0 for people who don't want a delay.

benma commented 2 years ago

The visual updates are limited to 50 matches. Try reducing these to a smaller number (vr/default-feedback-limit) and see if this actually speeds things up. Chances are the visual updating is not the bottleneck, as 50 matches shown are not that many.

Even if the bottleneck is elsewhere, delaying update would probably still help.

Can you make a PR? I don't have the time to look into it.

codecoll commented 2 years ago

. Chances are the visual updating is not the bottleneck, as 50 matches shown are not that many.

Yes, it looks like something else. I'll try to profile it. Thanks

codecoll commented 2 years ago

I tried profiling it and I may have found a bug.

In my use case I advised vr--get-regexp-string to modify the regex syntax of the returned pattern and it was slow when I tried it on a large buffer. Turns out, sometimes vr--get-regexp-string does not return the current pattern, but the whole target buffer contents instead.

It is because vr--get-replacement calls vr--get-regexp-string with vr--target-buffer as the current buffer:

(defun vr--get-replacement (replacement match-data i)
  (with-current-buffer vr--target-buffer
    (let*
        ;; emulate case-conversion of (perform-replace)
        ((case-fold-search (if (and case-fold-search search-upper-case)
                               (ignore-errors (isearch-no-upper-case-p (vr--get-regexp-string) t))
                             case-fold-search))

vr--get-regexp-string calls vr--get-regexp-string-full and that calls (minibuffer-contents).

And (minibuffer-contents) returns the whole buffer contents if the current buffer is not a minibuffer, which is the case here.

benma commented 2 years ago

@codecoll in which case(s) exactly does it return the contents of the current buffer and not the minibuffer? how to reproduce? does it also happen if you don't advise any functions?

codecoll commented 2 years ago

First try running (minibuffer-contents) from the scratch buffer. You'll see it returns the contents of the scratch buffer, because if the minibuffer is not active then it returns the contents of the current buffer.

Next, change the code of vr--get-regexp-string-full, so it prints the returned regexp, before returning it, so you can see what it returns:

(defun vr--get-regexp-string-full ()
  (if (equal vr--in-minibuffer 'vr--minibuffer-regexp)
      (progn (print (minibuffer-contents)) (minibuffer-contents))
    vr--regexp-string))

Find a buffer to run vr/query-replace on, e.g. a lisp a file. Split the window, so you can see the *Messages*buffer in an other window, to see what vr--get-regexp-string-full returns.

Start vr/query-replace and start some trivial replacing, but make sure there are matches. E.g. change defun to func Start the replace, but cancel it at the first match. You can see in the Messages buffer that the pattern is returned correctly.

Now start vr/query-replace again and press up arrow or M-p to get the last history element which we created in the previous step. You can see in the Messages buffer that the pattern is returned correctly first, and then the current buffer text is returned instead.