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

[Juri Linkov <juri@linkov.net>]

> My idea is:
>
> - The users of the feature are Elisp programmers / package authors.
> - I don't think end users can meaningfully do anything directly with
>   this new minibuffer hook.
> - If package X wants to take advantage of the feature, then it will
>   either add minibuffer-lazy-highlight-setup to the
>   minibuffer-setup-hook unconditionally, or it will define an
>   X-lazy-highlight customization option to control this.
>
> So I think the conclusion is that the current approach in my patch is an
> good way to proceed here?

So do you think end users should not be able to decide where they want
to use this feature?  For example, if one user will want it for 'occur',
but another user doesn't want it in the 'occur' regexp-reading minibuffer,
they should have no choice?

>>>> BTW, what is the relation between the minibuffer-lazy-highlight feature
>>>> and another proposed feature that immediately updates the search in
>>>> the buffer while editing the string in the minibuffer by isearch-edit-string?
>>>> Can minibuffer-lazy-highlight be considered as a lightweight version of
>>>> the buffer search from the minibuffer?
>>>
>>> Well, there's a package for that on ELPA (isearch-mb), so extending
>>> isearch-edit-string to do that seems superfluous now?
>>
>> It's still possible to add this feature to isearch-edit-string,
>> when the change would not be too enormous.  I recall squeezing
>> it into a small patch, but unfortunately it requires changes
>> in keymap priorities.
>
> I would suggest taking a look at isearch-mb.  I think the code is pretty
> tight, and I would be unable to shorten the implementation other than by
> deleting comment :-)

I already looked at isearch-mb.  Adding the same to isearch.el
will take only 52 lines.

>>>>> There are a few more we could add   (perhaps later),
>>>>> such as `occur' and `keep-lines'.
>>>>
>>>> I tried (add-hook 'minibuffer-setup-hook #'minibuffer-lazy-highlight-setup)
>>>> in the minibuffer of 'occur' and others, and it works nicely.
>>>> Maybe it could even semi-deprecate the package re-builder.el.
>>>>
>>>> Thanks for this generally usable feature.
>>>
>>> By the way, this is a byproduct of that long discussion that led to
>>> isearch-mb, so it was not all in vain :-).
>>
>> Are you sure these features can't be combined?  One feature basically
>> runs isearch-search-and-update in the buffer from the minibuffer,
>> and this feature runs isearch-lazy-highlight-new-loop.
>
> For one thing, isearch-mode has 2 essential commands (repeat forward and
> backwards), a couple more necessary ones (quit, abort, scroll,
> beginning/end of buffer, mode toggles), and then a number of commands
> that end the search with a special action (query-replace, etc.).
>
> These little details add up to the 283 lines in isearch-mb.el currently
> has.

I wonder how this is affected by scroll, beginning/end of buffer, mode toggles?
These commands don't use the minibuffer.

>>>>> - There's no customization variable to enable the minibuffer lazy
>>>>>   highlight.  The rationale is that each command that will use it should
>>>>>   define its own user option (or use an existing one).  For
>>>>>   `isearch-edit-string' it's `isearch-lazy-highlight'; for
>>>>>   `query-replace' it's `query-replace-lazy-highlight'; and so on.
>>>>
>>>> A common customizable option to enable this everywhere would be nice too.
>>>> Maybe disabling is already possible by customizing
>>>> 'minibuffer-lazy-count-format' to nil?  Then the checks for
>>>> non-nil 'minibuffer-lazy-count-format' could be added to
>>>> more places, such as to wrap the whole '(condition-case error'
>>>> in query-replace-read-args with the 'when' condition, etc.
>>>
>>> Yes, the user can set minibuffer-lazy-count-format to nil to get rid of
>>> the lazy count.
>>>
>>> Concerning query-replace, why would anyone want to have lazy highlight
>>> during the perform-replace loop, but not earlier?  I'm not a fan of
>>> adding a custom option here, not because it would be hard, but because
>>> it seems totally unnecessary.
>>
>> Maybe a new option would make sense for the same reason why there is
>> the option isearch-lazy-count?
>
> Okay, I'm not against this, but let's think about the names of these user
> options.  The existing option is named query-replace-lazy-highlight,
> which seems to exactly describe the new feature.  The existing feature
> would more specifically be called perform-replace-lazy highlight.

Do you mean lazy-highlight in the minibuffer that reads a string to replace?
Then it could be named query-replace-read-lazy-highlight.