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

Previous Next

Package: emacs;

Reported by: Augusto Stoffel <arstoffel <at> gmail.com>

Date: Sat, 8 Jan 2022 13:25:01 UTC

Severity: normal

Tags: patch

Merged with 53341

Fixed in version 29.0.50

Done: Juri Linkov <juri <at> linkov.net>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Juri Linkov <juri <at> linkov.net>
To: Augusto Stoffel <arstoffel <at> gmail.com>
Cc: 53126 <at> debbugs.gnu.org
Subject: 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?




This bug report was last modified 3 years and 39 days ago.

Previous Next


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