GNU bug report logs - #74404
[PATCH] Give Completion Preview bindings higher precedence

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 74404 in the body.
You can then email your comments to 74404 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Give Completion Preview bindings higher precedence
Date: Sun, 17 Nov 2024 17:30:47 +0100

[Message part 1 (text/plain, inline)]

Tags: patch

Whenever Completion Preview mode shows a completion preview, it also
activates keybindings for commands that interact with that preview, most
notably TAB is bound to completion-preview-insert, which inserts the
suggested completion.  This is implemented via a minor mode
completion-preview-active-mode whose keymap provides these bindings, and
which is enabled when the preview is displayed. 

This all works well, except for when another minor mode, foo-mode, is
enabled, whose keybindings conflict with these active preview bindings.
If foo-mode is defined after completion-preview-active-mode, i.e. if
foo.el is loaded after completion-preview.el, then foo-mode comes first
in minor-mode-map-alist and takes precedence over the bindings in
completion-preview-active-mode-map.

This happens, for example, with eshell-cmpl-mode, a minor mode that is
enabled by default in Eshell, which binds TAB to completion-at-point.
So currently Completion Preview mode doesn't work as expected in Eshell:
when you try to accept the suggested completion (with TAB), you get
completion-at-point instead of completion-preview-insert.

To fix this issue, we need to give the active completion preview
bindings higher precedence than keymaps of minor modes that where
defined after loading completion-preview.el.  The patch below does that
by using minor-mode-overriding-map-alist.  Any thoughts?

[0001-Give-Completion-Preview-bindings-higher-precedence.patch (text/patch, attachment)]

Message #8 received at 74404 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Eshel Yaron <me <at> eshelyaron.com>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 74404 <at> debbugs.gnu.org
Subject: Re: bug#74404: [PATCH] Give Completion Preview bindings higher
 precedence
Date: Sun, 17 Nov 2024 19:04:33 +0200

> Date: Sun, 17 Nov 2024 17:30:47 +0100
> From:  Eshel Yaron via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> Whenever Completion Preview mode shows a completion preview, it also
> activates keybindings for commands that interact with that preview, most
> notably TAB is bound to completion-preview-insert, which inserts the
> suggested completion.  This is implemented via a minor mode
> completion-preview-active-mode whose keymap provides these bindings, and
> which is enabled when the preview is displayed. 
> 
> This all works well, except for when another minor mode, foo-mode, is
> enabled, whose keybindings conflict with these active preview bindings.
> If foo-mode is defined after completion-preview-active-mode, i.e. if
> foo.el is loaded after completion-preview.el, then foo-mode comes first
> in minor-mode-map-alist and takes precedence over the bindings in
> completion-preview-active-mode-map.
> 
> This happens, for example, with eshell-cmpl-mode, a minor mode that is
> enabled by default in Eshell, which binds TAB to completion-at-point.
> So currently Completion Preview mode doesn't work as expected in Eshell:
> when you try to accept the suggested completion (with TAB), you get
> completion-at-point instead of completion-preview-insert.
> 
> To fix this issue, we need to give the active completion preview
> bindings higher precedence than keymaps of minor modes that where
> defined after loading completion-preview.el.  The patch below does that
> by using minor-mode-overriding-map-alist.  Any thoughts?

Can't you instead use the 'keymap' text or overlay property?  That
would avoid the "arms race" of precedences.

Adding Stefan, in case he has comments or suggestions.

Message #11 received at 74404 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 74404 <at> debbugs.gnu.org, Eshel Yaron <me <at> eshelyaron.com>
Subject: Re: bug#74404: [PATCH] Give Completion Preview bindings higher
 precedence
Date: Sun, 17 Nov 2024 12:16:29 -0500

>> To fix this issue, we need to give the active completion preview
>> bindings higher precedence than keymaps of minor modes that where
>> defined after loading completion-preview.el.  The patch below does that
>> by using minor-mode-overriding-map-alist.  Any thoughts?
>
> Can't you instead use the 'keymap' text or overlay property?  That
> would avoid the "arms race" of precedences.
>
> Adding Stefan, in case he has comments or suggestions.

Assuming the minor mode is disabled as soon as point moves away from the
completion, I wouldn't have a clear preference between `keymap` and
`minor-mode-overriding-map-alist`.

BTW, maybe we should add some notion of minor mode precedence since such
problems are actually fairly common.  We could do something similar to
what we do with `add-hook`, so `add-minor-mode` takes care of obeying
the ordering constraints.


        Stefan

Message #14 received at 74404 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 74404 <at> debbugs.gnu.org
Subject: Re: bug#74404: [PATCH] Give Completion Preview bindings higher
 precedence
Date: Sun, 17 Nov 2024 18:31:03 +0100

Hi,

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>>> To fix this issue, we need to give the active completion preview
>>> bindings higher precedence than keymaps of minor modes that where
>>> defined after loading completion-preview.el.  The patch below does that
>>> by using minor-mode-overriding-map-alist.  Any thoughts?
>>
>> Can't you instead use the 'keymap' text or overlay property?  That
>> would avoid the "arms race" of precedences.

I thought about that, but no, we can't easily use that, since the
overlay is not necessarily at point: it is generally at the end of the
symbol, and point may be at the middle of the symbol.

>> Adding Stefan, in case he has comments or suggestions.
>
> Assuming the minor mode is disabled as soon as point moves away from the
> completion, I wouldn't have a clear preference between `keymap` and
> `minor-mode-overriding-map-alist`.

Indeed, the minor mode is disabled as soon as the completion preview is
dismissed for whatever reason, including moving point elsewhere.

> BTW, maybe we should add some notion of minor mode precedence since such
> problems are actually fairly common.  We could do something similar to
> what we do with `add-hook`, so `add-minor-mode` takes care of obeying
> the ordering constraints.

That'd be nice, I think.


Thanks,

Eshel

Message #17 received at 74404 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Eli Zaretskii <eliz <at> gnu.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 74404 <at> debbugs.gnu.org,  Eshel Yaron <me <at> eshelyaron.com>
Subject: Re: bug#74404: [PATCH] Give Completion Preview bindings higher
 precedence
Date: Fri, 22 Nov 2024 18:08:47 +0800

Hello,

On Sun 17 Nov 2024 at 12:16pm -05, Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" wrote:

> BTW, maybe we should add some notion of minor mode precedence since such
> problems are actually fairly common.  We could do something similar to
> what we do with `add-hook`, so `add-minor-mode` takes care of obeying
> the ordering constraints.

We could also have a customisation option that allows the user to
override the default priorities.  As minor modes are always interned
symbols, this would be possible in a way that it isn't for hook
priorities.

-- 
Sean Whitton

Message #20 received at 74404 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 74404 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#74404: [PATCH] Give Completion Preview bindings higher
 precedence
Date: Sat, 23 Nov 2024 15:02:56 +0100

close 74492 31.1
quit

Hi,

Sean Whitton <spwhitton <at> spwhitton.name> writes:

> Hello,
>
> On Sun 17 Nov 2024 at 12:16pm -05, Stefan Monnier via "Bug reports for
> GNU Emacs, the Swiss army knife of text editors" wrote:
>
>> BTW, maybe we should add some notion of minor mode precedence since such
>> problems are actually fairly common.  We could do something similar to
>> what we do with `add-hook`, so `add-minor-mode` takes care of obeying
>> the ordering constraints.
>
> We could also have a customisation option that allows the user to
> override the default priorities.  As minor modes are always interned
> symbols, this would be possible in a way that it isn't for hook
> priorities.

I've opened bug#74492 to discuss this broader topic of explicit minor
mode precedence.  For now, I've pushed my patch solving this particular
issue with Completion Preview mode to master (commit 30bcba27c8c).

With that, I'm closing this bug.


Thanks,

Eshel

Message #27 received at 74404-close <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: 74404-close <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Sean Whitton <spwhitton <at> spwhitton.name>
Subject: Re: bug#74404: [PATCH] Give Completion Preview bindings higher
 precedence
Date: Tue, 11 Feb 2025 11:16:16 -0800

Eshel Yaron <me <at> eshelyaron.com> writes:

> close 74492 31.1
> quit
>
> Hi,
>
> Sean Whitton <spwhitton <at> spwhitton.name> writes:
>
>> Hello,
>>
>> On Sun 17 Nov 2024 at 12:16pm -05, Stefan Monnier via "Bug reports for
>> GNU Emacs, the Swiss army knife of text editors" wrote:
>>
>>> BTW, maybe we should add some notion of minor mode precedence since such
>>> problems are actually fairly common.  We could do something similar to
>>> what we do with `add-hook`, so `add-minor-mode` takes care of obeying
>>> the ordering constraints.
>>
>> We could also have a customisation option that allows the user to
>> override the default priorities.  As minor modes are always interned
>> symbols, this would be possible in a way that it isn't for hook
>> priorities.
>
> I've opened bug#74492 to discuss this broader topic of explicit minor
> mode precedence.  For now, I've pushed my patch solving this particular
> issue with Completion Preview mode to master (commit 30bcba27c8c).
>
> With that, I'm closing this bug.

It seems like the bug was not closed, so I'm doing that now.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.