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

Previous Next

Full log

Message #139 received at 53126 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Augusto Stoffel <arstoffel <at> gmail.com>
Cc: 53126 <at> debbugs.gnu.org
Subject: Re: bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading
 query-replace string, etc.
Date: Fri, 01 Apr 2022 19:35:38 +0300

>>> 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?

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.