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: Augusto Stoffel <arstoffel <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 53126 <at> debbugs.gnu.org
Subject: bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.
Date: Thu, 07 Apr 2022 21:32:21 +0200
[Message part 1 (text/plain, inline)]
On Tue,  5 Apr 2022 at 20:12, Juri Linkov <juri <at> linkov.net> wrote:

>> But that's a good point, we don't need a macro.  Among several
>> variations, we could make the setup code look like this:
>>
>>      (minibuffer-with-setup-hook
>>            (minibuffer-lazy-highlight-init :case-fold case-fold-search
>>                                            :regexp regexp-flag
>>                                            ...)
>>        (query-replace-read-from prompt regexp-flag))
>>
>> where now `minibuffer-lazy-highlight-init' is not the function that
>> initializes stuff, but rather a function that returns a closure that
>> initializes stuff.
>
> Looks good.

Okay, I've refactored my code like this.  I actually like it better that
way.  (As a downside, the stuff that was already merged to isearch.el is
completely changed.)

[0001-Display-lazy-highlight-and-match-count-in-query-repl.patch (text/x-patch, attachment)]
[Message part 3 (text/plain, inline)]

>>> Please also note that condition-case can be replaced by
>>> a hook in minibuffer-exit-hook that can remove highlighting
>>> after exiting the minibuffer.
>>
>> If it was a `unwind-protect', I would agree.  But I don't know how to
>> simulate a `condition-case'.  Specifically, how can we determine if some
>> hook (the minibuffer-exit-hook in this case) is being run "normally" or
>> as part of the recovery from a signaled error?
>
> Shouldn't both cases clean up highlight from the buffer?
> Then I see no need to distinguish each case.  Or if really needed,
> you can try to bind the cleanup to command-error-function.

My previous patch had only one case: if the user quits, we clean up the
highlighting.

I can only see one simpler alternative, which is to always
unconditionally clean up the highlight.  This is not as nice, but if
keeping the code as simple as possible is important here, then I guess
this is the way forward.  So that's what the current patch does.

I suspect people will see this as a bug, but maybe discussing this issue
by itself later will be easier.

>>> Alternatively, the same lambda above could be added to
>>>
>>>   (add-hook 'minibuffer-setup-hook (lambda () ...))
>>
>> Why was it again that we want to avoid saying something like this?
>>
>>     (let ((case-fold-search whatever)
>>           (isearch-regexp regexp-flag))
>>        (minibuffer-with-setup-hook #'minibuffer-lazy-highlight-init
>>          (query-replace-read-from prompt regexp-flag)))
>
> 4 lines look nice, unlike 20 lines in one of your patches ;-)

When you add all the bells and whistles, 4 lines just won't do it.

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.