Re: [ELPA?] Controlling Isearch from the minibuffer

[Augusto Stoffel <arstoffel@gmail.com>]

On Thu, 13 May 2021 at 19:31, Juri Linkov <juri@linkov.net> wrote:

Hi Juri,

I'm pretty convinced that the approach of my first patch
(`with-isearch-window' macro, etc.) is the right way to implement this
and possibly other future features.  On the other hand, I understand
that it would be premature to just change isearch.el in this direction
without a deeper reflection and further testing.

So it seems that providing the feature in a package would be the best
way to proceed here.  Then we can observe which hurdles arise and make a
more informed change to isearch.el in the future.  I just worry a bit
this may end up being a very distant future :-).

What to you think?  I would then maintain the package on Github.  How
does the submission process to ELPA work?  Once again, here's the link:

    https://github.com/astoff/isearch-mb


>> I mean keyboard quit.  You can see the error message by evaluating this
>> and then pressing C-g:
>>
>>     (add-hook 'post-command-hook
>>               (lambda () (read-from-minibuffer "test")) t t)
>
> It's possible to handle C-g when needed:
>
>   (add-hook 'post-command-hook
>             (lambda () (condition-case nil
>                            (read-from-minibuffer "test")
>                          (quit)))
>             t t)
>
>> Using a timer with zero delay instead works better.
>
> Using a timer is still a hack.  What could avoid a hack is
> using the existing solution like recursive-edit is called
> at the end of isearch-mode.
>
>> Setting `isearch-message-function' is of no help, because there are some
>> tests for `(null isearch-message-function)' as well as some explicit
>> calls to `(isearch-message)' in isearch.el.  As far as I can see, there
>> is no alternative to modifying the function `isearch-message' itself.
>
> Tests for `(null isearch-message-function)' were added as a temporary
> workaround until lazy count will be implemented in the minibuffer.
> We need to remove these workarounds anyway.  So using isearch-message-function
> should be the right thing to do.

Okay, that' a low hanging fruit then.

>
>> Moreover, one still has to manually keep a list of commands that need to
>> switch to the search buffer (cf. `isearch-edit--buffer-commands') and
>> commands that end the search (cf. `isearch-edit--quitting-commands'); I
>> see no way around that.  Therefore, I see no advantage here over the
>> proposed `with-isearch-window' macro.
>
> There is no need to keep commands in a list.  The isearch-allow-scroll
> feature uses symbol properties like
>
>   (put 'recenter 'isearch-scroll t)
>   (put 'recenter-top-bottom 'isearch-scroll t)
>   (put 'reposition-window 'isearch-scroll t)
>

Fine, but this still means manually curating the list of relevant
commands.  I guess there's no way out of it.

>> I admit that the `with-isearch-window-quitting-edit' macro of my old
>> patch seems pretty ad-hoc.  However, it could be replaced by a
>> `with-isearch-done' macro which is of more general nature.  If you
>> search isearch.el for calls to `isearch-done', you'll see that some are
>> of the form
>>
>>      (let (;; Set `isearch-recursive-edit' to nil to prevent calling
>>            ;; `exit-recursive-edit' in `isearch-done' that terminates
>>            ;; the execution of this command when it is non-nil.
>>            ;; We call `exit-recursive-edit' explicitly at the end below.
>>            (isearch-recursive-edit nil))
>>        (isearch-done nil t)
>>
>> while a few others seem to just don't take into account that we may be
>> ending a recursive edit.
>
> Indeed, this is an old unsolved problem, and setting isearch-recursive-edit
> to prevent calling exit-recursive-edit is a workaround.  We need to fix this
> anyway.  Then calling isearch-edit-string with read-from-minibuffer
> at the end of isearch-mode should not be different from the current
> calling of recursive-edit at the end of isearch-mode.
> Both problems will use the same solution.
>
>> So an `with-isearch-done' macro (which ends isearch, executes the body,
>> then possibly ends a recursive edit, and at the same time takes care to
>> unwind the minibuffer if we happen to be in `isearch-edit-string') would
>> seem a reasonable addition to isearch.el.
>
> Do you mean to pack the current isearch-recursive-edit/exit-recursive-edit
> as a macro and use it in commands like isearch-query-replace, and also to
> call exit-minibuffer to handle minibuffer exiting too.  This could be fine
> if no better solution is found.
>

Yes.  Like I said, maybe this needs more time to mature, and seeing how
well the advice-based implementation of the isearch-mb package works may
inform this question.

>> Sorry for the long message :-)
>
> It would be more appropriate to be sorry for the long patch ;-)
> Usually a long patch means there are still unsolved workarounds
> for old problems such as with isearch-message-function and
> exit-recursive-edit above, etc.  After solving these problems
> the patch should become much shorter.

Okay, I'll try to be more granular next time.  I still want to work on
the lazy highlight for `isearch-edit-string' and `query-replace' and
friends, for instance.