GNU bug report logs - #49104
[PATCH] 28.0.50; Handle remapped commands for M-TAB in `flyspell-prog-mode'

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 49104 in the body.
You can then email your comments to 49104 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: Jim Porter <jporterbugs <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] 28.0.50;
 Handle remapped commands for M-TAB in `flyspell-prog-mode'
Date: Fri, 18 Jun 2021 16:09:55 -0700

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

`flyspell-prog-mode' uses the original M-TAB binding outside of
comments, which is nice (see bug#18533). However, it doesn't account
for remapped commands. For example, I remap `completion-at-point' to
`company-complete' when `company-mode' is active. If I hit M-TAB while
`company-mode' and `flyspell-prog-mode' are active, I get
`completion-at-point', which isn't what I want.

To reproduce this issue, you can do the following:

  emacs -Q
  (global-set-key [remap completion-at-point]
                  (lambda () (interactive) (message "remapped")))
  M-x flyspell-prog-mode
  M-TAB ;; Or C-M-i

I've attached a patch that fixes the issue (hopefully it works in all
cases; it works for me).

[0001-Handle-remapped-commands-for-original-M-TAB-binding-.patch (application/octet-stream, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 49104 <at> debbugs.gnu.org
Subject: Re: bug#49104: [PATCH] 28.0.50;
 Handle remapped commands for M-TAB in `flyspell-prog-mode'
Date: Sat, 19 Jun 2021 09:07:18 +0300

> From: Jim Porter <jporterbugs <at> gmail.com>
> Date: Fri, 18 Jun 2021 16:09:55 -0700
> 
> `flyspell-prog-mode' uses the original M-TAB binding outside of
> comments, which is nice (see bug#18533). However, it doesn't account
> for remapped commands. For example, I remap `completion-at-point' to
> `company-complete' when `company-mode' is active. If I hit M-TAB while
> `company-mode' and `flyspell-prog-mode' are active, I get
> `completion-at-point', which isn't what I want.
> 
> To reproduce this issue, you can do the following:
> 
>   emacs -Q
>   (global-set-key [remap completion-at-point]
>                   (lambda () (interactive) (message "remapped")))
>   M-x flyspell-prog-mode
>   M-TAB ;; Or C-M-i

Isn't it a bug that global/local-key-binding don't return such
remapped bindings, at least optionally?

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Jim Porter <jporterbugs <at> gmail.com>, 49104 <at> debbugs.gnu.org
Subject: Re: bug#49104: [PATCH] 28.0.50; Handle remapped commands for M-TAB
 in `flyspell-prog-mode'
Date: Sat, 19 Jun 2021 07:51:29 -0400

>> To reproduce this issue, you can do the following:
>> 
>>   emacs -Q
>>   (global-set-key [remap completion-at-point]
>>                   (lambda () (interactive) (message "remapped")))
>>   M-x flyspell-prog-mode
>>   M-TAB ;; Or C-M-i
>
> Isn't it a bug that global/local-key-binding don't return such
> remapped bindings, at least optionally?

I don't, but I wonder why flyspell-prog-mode uses those functions
instead of using just `key-binding` (which seems simpler and does pay
attention to remapping).

IOW, what would be the problem with the patch below?


        Stefan


diff --git a/lisp/textmodes/flyspell.el b/lisp/textmodes/flyspell.el
index fb9361e45d..30b44daf54 100644
--- a/lisp/textmodes/flyspell.el
+++ b/lisp/textmodes/flyspell.el
@@ -415,8 +415,7 @@ flyspell-prog-mode
   (setq flyspell-generic-check-word-predicate
         #'flyspell-generic-progmode-verify)
   (setq-local flyspell--prev-meta-tab-binding
-              (or (local-key-binding "\M-\t" t)
-                  (global-key-binding "\M-\t" t)))
+              (key-binding "\M-\t"))
   (flyspell-mode 1)
   (run-hooks 'flyspell-prog-mode-hook))
 

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: jporterbugs <at> gmail.com, 49104 <at> debbugs.gnu.org
Subject: Re: bug#49104: [PATCH] 28.0.50; Handle remapped commands for M-TAB
 in `flyspell-prog-mode'
Date: Sat, 19 Jun 2021 15:20:12 +0300

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Jim Porter <jporterbugs <at> gmail.com>,  49104 <at> debbugs.gnu.org
> Date: Sat, 19 Jun 2021 07:51:29 -0400
> 
> >> To reproduce this issue, you can do the following:
> >> 
> >>   emacs -Q
> >>   (global-set-key [remap completion-at-point]
> >>                   (lambda () (interactive) (message "remapped")))
> >>   M-x flyspell-prog-mode
> >>   M-TAB ;; Or C-M-i
> >
> > Isn't it a bug that global/local-key-binding don't return such
> > remapped bindings, at least optionally?
> 
> I don't

Why not?

> but I wonder why flyspell-prog-mode uses those functions
> instead of using just `key-binding`

Because the latter is not described in the ELisp manual where
global/local-key-binding are, and isn't referenced from there?

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: jporterbugs <at> gmail.com, 49104 <at> debbugs.gnu.org
Subject: Re: bug#49104: [PATCH] 28.0.50; Handle remapped commands for M-TAB
 in `flyspell-prog-mode'
Date: Sat, 19 Jun 2021 13:24:35 -0400

>> > Isn't it a bug that global/local-key-binding don't return such
>> > remapped bindings, at least optionally?
>> I don't
> Why not?

Good question.  I think I meant to write "I don't know" instead of just
"I don't".

>> but I wonder why flyspell-prog-mode uses those functions
>> instead of using just `key-binding`
> Because the latter is not described in the ELisp manual where
> global/local-key-binding are, and isn't referenced from there?

In that case, `key-binding` might be a good replacement.
I see that using `key-binding` could find flyspell's own binding, tho, so we
should probably use the (first) patch below instead.

Tho I also wonder why we do the lookup when flyspell is enabled instead
of doing directly in flyspell-auto-correct-word as in the second
patch below.


        Stefan


diff --git a/lisp/textmodes/flyspell.el b/lisp/textmodes/flyspell.el
index ba48e5de21..11ff0f426f 100644
--- a/lisp/textmodes/flyspell.el
+++ b/lisp/textmodes/flyspell.el
@@ -411,8 +411,8 @@ flyspell-prog-mode
   (setq flyspell-generic-check-word-predicate
         #'flyspell-generic-progmode-verify)
   (setq-local flyspell--prev-meta-tab-binding
-              (or (local-key-binding "\M-\t" t)
-                  (global-key-binding "\M-\t" t)))
+              (let ((flyspell-mode nil))
+                (key-binding "\M-\t")))
   (flyspell-mode 1)
   (run-hooks 'flyspell-prog-mode-hook))
 



diff --git a/lisp/textmodes/flyspell.el b/lisp/textmodes/flyspell.el
index ba48e5de21..649057270e 100644
--- a/lisp/textmodes/flyspell.el
+++ b/lisp/textmodes/flyspell.el
@@ -401,18 +401,12 @@ flyspell-generic-progmode-verify
     (let ((f (get-text-property (1- (point)) 'face)))
       (memq f flyspell-prog-text-faces))))
 
-(defvar flyspell--prev-meta-tab-binding nil
-  "Records the binding of M-TAB in effect before flyspell was activated.")
-
 ;;;###autoload
 (defun flyspell-prog-mode ()
   "Turn on `flyspell-mode' for comments and strings."
   (interactive)
   (setq flyspell-generic-check-word-predicate
         #'flyspell-generic-progmode-verify)
-  (setq-local flyspell--prev-meta-tab-binding
-              (or (local-key-binding "\M-\t" t)
-                  (global-key-binding "\M-\t" t)))
   (flyspell-mode 1)
   (run-hooks 'flyspell-prog-mode-hook))
 
@@ -1990,13 +1984,10 @@ flyspell-auto-correct-word
   (interactive)
   ;; If we are not in the construct where flyspell should be active,
   ;; invoke the original binding of M-TAB, if that was recorded.
-  (if (and (local-variable-p 'flyspell--prev-meta-tab-binding)
-           (commandp flyspell--prev-meta-tab-binding t)
-           (functionp flyspell-generic-check-word-predicate)
-           (not (funcall flyspell-generic-check-word-predicate))
-           (equal (where-is-internal 'flyspell-auto-correct-word nil t)
-                  [?\M-\t]))
-      (call-interactively flyspell--prev-meta-tab-binding)
+  (if (and (functionp flyspell-generic-check-word-predicate)
+           (not (funcall flyspell-generic-check-word-predicate)))
+      (let ((flyspell-mode nil))
+        (execut-command (key-binding (this-command-keys))))
     (let ((pos     (point))
           (old-max (point-max)))
       ;; Flush a possibly stale cache from previous invocations of

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 49104 <at> debbugs.gnu.org
Subject: Re: bug#49104: [PATCH] 28.0.50;
 Handle remapped commands for M-TAB in `flyspell-prog-mode'
Date: Sat, 19 Jun 2021 10:31:38 -0700

On Sat, Jun 19, 2021 at 10:24 AM Stefan Monnier
<monnier <at> iro.umontreal.ca> wrote:
> In that case, `key-binding` might be a good replacement.
> I see that using `key-binding` could find flyspell's own binding, tho, so we
> should probably use the (first) patch below instead.
>
> Tho I also wonder why we do the lookup when flyspell is enabled instead
> of doing directly in flyspell-auto-correct-word as in the second
> patch below.

Indeed, while I was looking into this some more, I saw that
`flyspell-auto-correct-word' is bound to both `M-TAB' and `C-.' in
`flyspell-mode-map'. Your second patch should properly delegate to the
right command for either binding, which would be very nice. The
original code (and my patch) always act as though you typed `M-TAB'
when delegating, which isn't quite right.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 49104 <at> debbugs.gnu.org
Subject: Re: bug#49104: [PATCH] 28.0.50; Handle remapped commands for M-TAB
 in `flyspell-prog-mode'
Date: Sun, 27 Jun 2021 15:07:19 -0400

I pushed a slightly cleaner version of my patch,


        Stefan


Jim Porter [2021-06-19 10:31:38] wrote:

> On Sat, Jun 19, 2021 at 10:24 AM Stefan Monnier
> <monnier <at> iro.umontreal.ca> wrote:
>> In that case, `key-binding` might be a good replacement.
>> I see that using `key-binding` could find flyspell's own binding, tho, so we
>> should probably use the (first) patch below instead.
>>
>> Tho I also wonder why we do the lookup when flyspell is enabled instead
>> of doing directly in flyspell-auto-correct-word as in the second
>> patch below.
>
> Indeed, while I was looking into this some more, I saw that
> `flyspell-auto-correct-word' is bound to both `M-TAB' and `C-.' in
> `flyspell-mode-map'. Your second patch should properly delegate to the
> right command for either binding, which would be very nice. The
> original code (and my patch) always act as though you typed `M-TAB'
> when delegating, which isn't quite right.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 49104 <at> debbugs.gnu.org
Subject: Re: bug#49104: [PATCH] 28.0.50;
 Handle remapped commands for M-TAB in `flyspell-prog-mode'
Date: Sun, 27 Jun 2021 21:44:27 -0700

On Sun, Jun 27, 2021 at 12:07 PM Stefan Monnier
<monnier <at> iro.umontreal.ca> wrote:
>
> I pushed a slightly cleaner version of my patch,

I tried it out locally and everything works perfectly with my config.
Thanks for the fix.

- Jim

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 49104 <at> debbugs.gnu.org
Subject: Re: bug#49104: [PATCH] 28.0.50; Handle remapped commands for M-TAB
 in `flyspell-prog-mode'
Date: Tue, 20 Jul 2021 14:13:18 +0200

Jim Porter <jporterbugs <at> gmail.com> writes:

> On Sun, Jun 27, 2021 at 12:07 PM Stefan Monnier
> <monnier <at> iro.umontreal.ca> wrote:
>>
>> I pushed a slightly cleaner version of my patch,
>
> I tried it out locally and everything works perfectly with my config.
> Thanks for the fix.

This bug report was left open, so I'm closing it now.  (I only skimmed
it lightly, but if I understood correctly, Stefan's patched fixed the
reported problem.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Previous Next

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