* bug#49104: [PATCH] 28.0.50; Handle remapped commands for M-TAB in `flyspell-prog-mode'
@ 2021-06-18 23:09 Jim Porter
2021-06-19 6:07 ` Eli Zaretskii
0 siblings, 1 reply; 9+ messages in thread
From: Jim Porter @ 2021-06-18 23:09 UTC (permalink / raw)
To: 49104
[-- Attachment #1: Type: text/plain, Size: 696 bytes --]
`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).
[-- Attachment #2: 0001-Handle-remapped-commands-for-original-M-TAB-binding-.patch --]
[-- Type: application/octet-stream, Size: 1299 bytes --]
From 734890adff84cf8fbec9cff0a5bb6ad620877216 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Fri, 18 Jun 2021 16:02:07 -0700
Subject: [PATCH] Handle remapped commands for original M-TAB binding in
'flyspell-prog-mode'
* lisp/textmodes/flyspell.el (flyspell-prog-mode): Get the remapped
command for the original M-TAB binding if necessary.
---
lisp/textmodes/flyspell.el | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/lisp/textmodes/flyspell.el b/lisp/textmodes/flyspell.el
index ba48e5de21..eb4c5cde28 100644
--- a/lisp/textmodes/flyspell.el
+++ b/lisp/textmodes/flyspell.el
@@ -410,9 +410,11 @@ flyspell-prog-mode
(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)))
+ (let ((prev-meta-tab-binding (or (local-key-binding "\M-\t" t)
+ (global-key-binding "\M-\t" t))))
+ (setq-local flyspell--prev-meta-tab-binding
+ (or (command-remapping prev-meta-tab-binding)
+ prev-meta-tab-binding)))
(flyspell-mode 1)
(run-hooks 'flyspell-prog-mode-hook))
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* bug#49104: [PATCH] 28.0.50; Handle remapped commands for M-TAB in `flyspell-prog-mode'
2021-06-18 23:09 bug#49104: [PATCH] 28.0.50; Handle remapped commands for M-TAB in `flyspell-prog-mode' Jim Porter
@ 2021-06-19 6:07 ` Eli Zaretskii
2021-06-19 11:51 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2021-06-19 6:07 UTC (permalink / raw)
To: Jim Porter, Stefan Monnier; +Cc: 49104
> From: Jim Porter <jporterbugs@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?
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#49104: [PATCH] 28.0.50; Handle remapped commands for M-TAB in `flyspell-prog-mode'
2021-06-19 6:07 ` Eli Zaretskii
@ 2021-06-19 11:51 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-06-19 12:20 ` Eli Zaretskii
0 siblings, 1 reply; 9+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-06-19 11:51 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Jim Porter, 49104
>> 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))
^ permalink raw reply related [flat|nested] 9+ messages in thread
* bug#49104: [PATCH] 28.0.50; Handle remapped commands for M-TAB in `flyspell-prog-mode'
2021-06-19 11:51 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-06-19 12:20 ` Eli Zaretskii
2021-06-19 17:24 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2021-06-19 12:20 UTC (permalink / raw)
To: Stefan Monnier; +Cc: jporterbugs, 49104
> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Jim Porter <jporterbugs@gmail.com>, 49104@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?
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#49104: [PATCH] 28.0.50; Handle remapped commands for M-TAB in `flyspell-prog-mode'
2021-06-19 12:20 ` Eli Zaretskii
@ 2021-06-19 17:24 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-06-19 17:31 ` Jim Porter
0 siblings, 1 reply; 9+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-06-19 17:24 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: jporterbugs, 49104
>> > 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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* bug#49104: [PATCH] 28.0.50; Handle remapped commands for M-TAB in `flyspell-prog-mode'
2021-06-19 17:24 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-06-19 17:31 ` Jim Porter
2021-06-27 19:07 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 1 reply; 9+ messages in thread
From: Jim Porter @ 2021-06-19 17:31 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 49104
On Sat, Jun 19, 2021 at 10:24 AM Stefan Monnier
<monnier@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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#49104: [PATCH] 28.0.50; Handle remapped commands for M-TAB in `flyspell-prog-mode'
2021-06-19 17:31 ` Jim Porter
@ 2021-06-27 19:07 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-06-28 4:44 ` Jim Porter
0 siblings, 1 reply; 9+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-06-27 19:07 UTC (permalink / raw)
To: Jim Porter; +Cc: Eli Zaretskii, 49104
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@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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#49104: [PATCH] 28.0.50; Handle remapped commands for M-TAB in `flyspell-prog-mode'
2021-06-27 19:07 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-06-28 4:44 ` Jim Porter
2021-07-20 12:13 ` Lars Ingebrigtsen
0 siblings, 1 reply; 9+ messages in thread
From: Jim Porter @ 2021-06-28 4:44 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 49104
On Sun, Jun 27, 2021 at 12:07 PM Stefan Monnier
<monnier@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
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#49104: [PATCH] 28.0.50; Handle remapped commands for M-TAB in `flyspell-prog-mode'
2021-06-28 4:44 ` Jim Porter
@ 2021-07-20 12:13 ` Lars Ingebrigtsen
0 siblings, 0 replies; 9+ messages in thread
From: Lars Ingebrigtsen @ 2021-07-20 12:13 UTC (permalink / raw)
To: Jim Porter; +Cc: Stefan Monnier, 49104
Jim Porter <jporterbugs@gmail.com> writes:
> On Sun, Jun 27, 2021 at 12:07 PM Stefan Monnier
> <monnier@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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-07-20 12:13 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-06-18 23:09 bug#49104: [PATCH] 28.0.50; Handle remapped commands for M-TAB in `flyspell-prog-mode' Jim Porter
2021-06-19 6:07 ` Eli Zaretskii
2021-06-19 11:51 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-06-19 12:20 ` Eli Zaretskii
2021-06-19 17:24 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-06-19 17:31 ` Jim Porter
2021-06-27 19:07 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-06-28 4:44 ` Jim Porter
2021-07-20 12:13 ` Lars Ingebrigtsen
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.