[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-repl
From: |
Augusto Stoffel |
Subject: |
bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc. |
Date: |
Thu, 17 Mar 2022 20:10:34 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.0.91 (gnu/linux) |
On Thu, 17 Mar 2022 at 19:09, Juri Linkov <juri@linkov.net> wrote:
>> 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?
Of course there should an option, maybe occur-lazy-highlight. This is
why isearch.el itself should _not_ contain a customization option called
`minibuffer-lazy-highlight': each use (or group of uses) of this
functionality should define their own customization option.
>>>>> 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.
Okay, I can give my opinion about these changes if you wish.
>>>>>> 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.
I probably misunderstood you then. I thought you wanted to allow C-s
and C-r in isearch-edit-string to go back and forth in the search
buffer, but apparently this is not the case. (Then I need to see your
52-line patch to understand what exactly you mean...)
>>>>>> - 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.
I personally find it a bit unnecessary to have separate options for
query-replace-read-lazy-highlight and query-replace-lazy-highlight. Of
course it's nice to have fine-grained control, but at some point it just
becomes too difficult to configure things. But I don't mind adding it.
- bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc., Juri Linkov, 2022/03/15
- bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc., Augusto Stoffel, 2022/03/15
- bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc., Juri Linkov, 2022/03/16
- bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc., Augusto Stoffel, 2022/03/16
- bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc., Juri Linkov, 2022/03/17
- bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc.,
Augusto Stoffel <=
- bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc., Juri Linkov, 2022/03/17
- bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc., Augusto Stoffel, 2022/03/17
- bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc., Augusto Stoffel, 2022/03/20
- bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc., Juri Linkov, 2022/03/20
- bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc., Augusto Stoffel, 2022/03/24
- bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc., Juri Linkov, 2022/03/25
- bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc., Augusto Stoffel, 2022/03/25
- bug#53126: 29.0.50; [PATCH] Lazy highlight/count when reading query-replace string, etc., Juri Linkov, 2022/03/27