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

[Juri Linkov]

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

Thanks, now it looks much better!

In `isearch-edit-string' you replaced:
  (add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
with
  (minibuffer-with-setup-hook (minibuffer-lazy-highlight-setup)
but the docstring of `minibuffer-lazy-highlight-setup' in your patch is:

  This function return a closure intended to be added to
  `minibuffer-setup-hook'.

Maybe either a typo that needs to mention `minibuffer-with-setup-hook',
or `minibuffer-setup-hook' needs to evaluate such closure with something like

  (add-hook 'minibuffer-setup-hook (funcall 'minibuffer-lazy-highlight-setup))

But since this is more ugly, then using `minibuffer-with-setup-hook' is fine.

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

Yep, let's do this later when people will ask for it.

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

Now the parameters of minibuffer-lazy-highlight-setup look nice,
so line count doesn't matter when code keeps simplicity.