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

[Juri Linkov]

>>> 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?
>>
>> Do you think a macro could help to make this function short again?
>
> That could be done, of course, but I'm pretty sure that this macro would
> be used exactly once.  I don't think we should have a general-purpose
> macro to set up the lazy highlighting for arbitrary minibuffer commands,
> because complex cases like query-replace will be rare,

Why do you think query-replace is more complex than other minibuffer commands?
Other minibuffer commands need to set all the same a dozen of variables that
control lazy highlighting.  Otherwise, they will highlight some random matches
that happen to reuse values left from a previous isearch.

> and in any case we can't anticipate all special requirements.

To anticipate all requirements, I recommend to try adding this feature
at least to occur.  Then we will see what code needs to be shared.

> Anyway, should I refactor my patch to use a macro?

As long as experimentation will prove at least occur can use this macro.

> Something else that could make 'query-replace-read-args' slightly
> shorter is to define
>
>     (defun replace-regexp-function (delimited-flag)
>       (or replace-regexp-function
>           delimited-flag
>           (and replace-char-fold
>                (not regexp-flag)
>                #'char-fold-to-regexp)))
>
> This function could be used in 3 different places in replace.el.

This would be nice, maybe to be used as an optional argument of the macro.

>> Then when you would want to add highlighting to the minibuffer
>> reading the TO part of replacement too or to other minibuffers,
>> it would be easy to just add a single line with that macro.
>
> Right, a preview of the replacement text will also need some extra work.
> This will probably involve some extension to
> 'isearch-lazy-highlight-update', making it even more complicated...

Why?  Shouldn't it just highlight the same way the text
entered into the minibuffer that reads the replacement?