bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.

[Augusto Stoffel <arstoffel@gmail.com>]

On Fri, 25 Mar 2022 at 10:39, Juri Linkov <juri@linkov.net> wrote:

> The name of the function is `query-replace-read-args'.
> And now most of the function is dealing with highlighting:

The good thing is that it was a rather short function before, so now
it's still reasonably sized.  Also, the lazy highlight for query-replace
is essentially taken care of in one single place.

> This highlighting needs to be refactored from `query-replace-read-args'
> into a separate highlighting function.

Okay, you've said something along these lines before but I don't
understand what you mean.

Lazy highlight is controlled by lots of variables.  These variables need
to be set for the duration of the minibuffer session that reads the
replacement strings.  If we move this to a separate function, we can't
use 'let' anymore, so we basically need to reinvent the dynamic scoping
feature of Elisp (i.e., save the current values, set the variables to
new values, then restore the old values in some hook).  And an even ugly
workaround would be needed to replace the necessary condition-case.  Or
do I misunderstand you altogether?

The above accounts for most of the additions to query-replace-read-args.
What could move elsewhere are the 10 lines starting from

     (when query-replace-lazy-highlight

but this will not change the fact that most of query-replace-read-args
now takes care of setting up lazy highlight, so I don't see any real
benefits to this.

Or maybe the issue is with this line:

    (add-hook 'minibuffer-exit-hook cleanup)

which adds a closure rather than a symbol to the hook?  That I could be
changed.  I think it would require to add a defvar just to store
'region-filter', among other things I don't like, but I can do it.