Handling of [mouse-movement] and exiting isearch

Handling of [mouse-movement] and exiting isearch

[Daniel Martín]

I'd like commands bound to [mouse-movement] to not exit isearch, and I'm
confused by this code in isearch.el:

    ;; Pass frame events transparently so they won't exit the search.
    ;; In particular, if we have more than one display open, then a
    ;; switch-frame might be generated by someone typing at another keyboard.
    (define-key map [switch-frame] nil)
    (define-key map [delete-frame] nil)
    (define-key map [iconify-frame] nil)
    (define-key map [make-frame-visible] nil)
    (define-key map [mouse-movement] nil)
    (define-key map [language-change] nil)

The comment seems to imply that [mouse-movement], [language-change] and
some frame-related events won't exit isearch.  However, the code is
calling define-key and passsing KEY as nil, which means that the events
will be undefined in isearch-mode-map, which by default means that they
_will_ exit isearch, right? I think the code that implements this logic
is in function isearch-pre-command-hook:

   ;; Don't exit Isearch for isearch key bindings.
   ((or (commandp (lookup-key isearch-mode-map key nil))
        (commandp
         (lookup-key
          `(keymap (tool-bar menu-item nil ,isearch-tool-bar-map))
   key))))

What am I missing? I'm not sure if making [mouse-movement] not exit
isearch worked in the past but has regressed in recent versions of
Emacs, or if I'm not understanding the logic correctly.

Thanks.

Re: Handling of [mouse-movement] and exiting isearch

[Stefan Monnier]

> What am I missing? I'm not sure if making [mouse-movement] not exit
> isearch worked in the past but has regressed in recent versions of
> Emacs, or if I'm not understanding the logic correctly.

It's probably a regression, but even if it isn't, it's a bug.


        Stefan

Re: Handling of [mouse-movement] and exiting isearch

[Eli Zaretskii]

> From: Daniel Martín <mardani29@yahoo.es>
> Date: Mon, 09 Nov 2020 01:09:50 +0100
> 
> 
> I'd like commands bound to [mouse-movement] to not exit isearch, and I'm
> confused by this code in isearch.el:
> 
>     ;; Pass frame events transparently so they won't exit the search.
>     ;; In particular, if we have more than one display open, then a
>     ;; switch-frame might be generated by someone typing at another keyboard.
>     (define-key map [switch-frame] nil)
>     (define-key map [delete-frame] nil)
>     (define-key map [iconify-frame] nil)
>     (define-key map [make-frame-visible] nil)
>     (define-key map [mouse-movement] nil)
>     (define-key map [language-change] nil)
> 
> The comment seems to imply that [mouse-movement], [language-change] and
> some frame-related events won't exit isearch.  However, the code is
> calling define-key and passsing KEY as nil, which means that the events
> will be undefined in isearch-mode-map, which by default means that they
> _will_ exit isearch, right? I think the code that implements this logic
> is in function isearch-pre-command-hook:
> 
>    ;; Don't exit Isearch for isearch key bindings.
>    ((or (commandp (lookup-key isearch-mode-map key nil))
>         (commandp
>          (lookup-key
>           `(keymap (tool-bar menu-item nil ,isearch-tool-bar-map))
>    key))))
> 
> What am I missing?

Stab in the dark: the isearch-scroll property of commands, perhaps?

Re: Handling of [mouse-movement] and exiting isearch

[Daniel Martín]

Eli Zaretskii <eliz@gnu.org> writes:
>
> Stab in the dark: the isearch-scroll property of commands, perhaps?

Yes, that works for commands.  I was looking for some option that could
work for events like mouse-movement.  I'm not sure if that would be a
good idea in general because a minor mode may bind the mouse-movement
event to a command that is incompatible with isearch, right? (for
example, because the command moves the point).

So I think the safest way is that users/developers manually mark those
commands that are compatible with isearch with the isearch-scroll
property.

Re: Handling of [mouse-movement] and exiting isearch

[Eli Zaretskii]

> From: Daniel Martín <mardani29@yahoo.es>
> Cc: emacs-devel@gnu.org
> Date: Thu, 12 Nov 2020 23:56:04 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> >
> > Stab in the dark: the isearch-scroll property of commands, perhaps?
> 
> Yes, that works for commands.  I was looking for some option that could
> work for events like mouse-movement.

Maybe I'm missing something: an event is meaningless unless it's bound
to some command.  And we exit Isearch when certain commands are
invoked, not because some event arrived (this level in Emacs is
completely oblivious to events, anyway).

> I'm not sure if that would be a good idea in general because a minor
> mode may bind the mouse-movement event to a command that is
> incompatible with isearch, right? (for example, because the command
> moves the point).

Indeed, which is why we perform this filtering on the level of
commands.

> So I think the safest way is that users/developers manually mark those
> commands that are compatible with isearch with the isearch-scroll
> property.

That's what we support now.  We could, of course, provide some minor
mode that marks many commands with that property, if that's
convenient.

Re: Handling of [mouse-movement] and exiting isearch

[Daniel Martín]

Eli Zaretskii <eliz@gnu.org> writes:
>
> That's what we support now.  We could, of course, provide some minor
> mode that marks many commands with that property, if that's
> convenient.

Yes, that may be convenient.  If I understand correctly, the minor mode
would set isearch-allow-scroll to t on enter and leave it nil on exit.
And it could expose some functions like

(add-isearch-compatible-commands COMMAND1 COMMAND2 ...)
(remove-isearch-compatible-commands COMMAND1 COMMAND2 ...)
(isearch-compatible-command-p COMMAND)

that would transparently add/remove/query the isearch-scroll property of
their arguments.

A nice side effect of this new minor mode is that the naming will make
it more clear that not every command is about scrolling (for example,
enlarge-window), while keeping backwards compatibility.

I need to think about how this feature would play with the rest of the
configuration options in isearch.  What if the user wants
isearch-allow-scroll set to 'unlimited, for example.

Opinions?