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

[Augusto Stoffel]

On Tue,  5 Apr 2022 at 20:12, Juri Linkov <juri@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
Description: Text Data

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