bug#30452: 25.3; tabulated-list-mode-map should inherit from special-mode-map

unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed

* bug#30452: 25.3; tabulated-list-mode-map should inherit from special-mode-map
@ 2018-02-14  3:52 Stephen Jung
  2019-01-15 17:57 ` bug#30452: [PATCH] Use 'make-sparse-keymap' rather than 'copy-keymap' Alex Branham
  0 siblings, 1 reply; 6+ messages in thread
From: Stephen Jung @ 2018-02-14  3:52 UTC (permalink / raw)
  To: 30452

tabulated-list-mode inherits from special-mode, so
tabulated-list-mode-map should probably also inherit from
special-mode-map. However, it actually copies special-mode-map in its
definition (https://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/emacs-lisp/tabulated-list.el?h=emacs-25.3#n152).

This appears to be the way that the code was written when
tabulated-list-mode was first created. I haven't read the mailing list
archives, so I'm not sure if there was an explanation of why it was
done this way, and there isn't a comment providing details on this
approach. Therefore, I think it should just be making a sparse keymap
here and inheriting from its parent, like most modes do.

In GNU Emacs 25.3.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.26)
 of 2018-02-08 built on bisson
Windowing system distributor 'The X.Org Foundation', version 11.0.11906000
Configured using:
 'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
 --localstatedir=/var --with-x-toolkit=gtk3 --with-xft --with-modules
 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong
 -fno-plt' CPPFLAGS=-D_FORTIFY_SOURCE=2
 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now'

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GCONF GSETTINGS
NOTIFY ACL GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 MODULES

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

-- 
Stephen

^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#30452: [PATCH] Use 'make-sparse-keymap' rather than 'copy-keymap'
  2018-02-14  3:52 bug#30452: 25.3; tabulated-list-mode-map should inherit from special-mode-map Stephen Jung
@ 2019-01-15 17:57 ` Alex Branham
  2019-01-19  8:05   ` Eli Zaretskii
  0 siblings, 1 reply; 6+ messages in thread
From: Alex Branham @ 2019-01-15 17:57 UTC (permalink / raw)
  To: 30452

[-- Attachment #1: Type: text/plain, Size: 1095 bytes --]

Hello -

Here's a tiny patch that changes the call from copy-keymap to make-sparse-keymap.

Thanks,
Alex

From c7f6db630f720fd26f0f1cc45631326bdcec136c Mon Sep 17 00:00:00 2001
From: Alex Branham <alex.branham@gmail.com>
Date: Tue, 15 Jan 2019 11:50:55 -0600
Subject: [PATCH] Make tabulated-list-mode-map inherit from special-mode-map

* lisp/emacs-lisp/tabulated-list.el (tabulated-list-mode-map): Use
  'make-sparse-keymap' rather than copying 'special-mode-map'.

Bug #30452
---
 lisp/emacs-lisp/tabulated-list.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/tabulated-list.el b/lisp/emacs-lisp/tabulated-list.el
index 6fdca2cd08..8c99728fd4 100644
--- a/lisp/emacs-lisp/tabulated-list.el
+++ b/lisp/emacs-lisp/tabulated-list.el
@@ -151,7 +151,7 @@ tabulated-list-put-tag
       (forward-line)))
 
 (defvar tabulated-list-mode-map
-  (let ((map (copy-keymap special-mode-map)))
+  (let ((map (make-sparse-keymap)))
     (set-keymap-parent map button-buffer-map)
     (define-key map "n" 'next-line)
     (define-key map "p" 'previous-line)
-- 
2.19.2



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-tabulated-list-mode-map-inherit-from-special-mo.patch --]
[-- Type: text/x-patch, Size: 988 bytes --]

From c7f6db630f720fd26f0f1cc45631326bdcec136c Mon Sep 17 00:00:00 2001
From: Alex Branham <alex.branham@gmail.com>
Date: Tue, 15 Jan 2019 11:50:55 -0600
Subject: [PATCH] Make tabulated-list-mode-map inherit from special-mode-map

* lisp/emacs-lisp/tabulated-list.el (tabulated-list-mode-map): Use
  'make-sparse-keymap' rather than copying 'special-mode-map'.

Bug #30452
---
 lisp/emacs-lisp/tabulated-list.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/tabulated-list.el b/lisp/emacs-lisp/tabulated-list.el
index 6fdca2cd08..8c99728fd4 100644
--- a/lisp/emacs-lisp/tabulated-list.el
+++ b/lisp/emacs-lisp/tabulated-list.el
@@ -151,7 +151,7 @@ tabulated-list-put-tag
       (forward-line)))
 
 (defvar tabulated-list-mode-map
-  (let ((map (copy-keymap special-mode-map)))
+  (let ((map (make-sparse-keymap)))
     (set-keymap-parent map button-buffer-map)
     (define-key map "n" 'next-line)
     (define-key map "p" 'previous-line)
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* bug#30452: [PATCH] Use 'make-sparse-keymap' rather than 'copy-keymap'
  2019-01-15 17:57 ` bug#30452: [PATCH] Use 'make-sparse-keymap' rather than 'copy-keymap' Alex Branham
@ 2019-01-19  8:05   ` Eli Zaretskii
  2019-01-19 15:33     ` Stefan Monnier
  0 siblings, 1 reply; 6+ messages in thread
From: Eli Zaretskii @ 2019-01-19  8:05 UTC (permalink / raw)
  To: Alex Branham, Stefan Monnier; +Cc: 30452

> From: Alex Branham <alex.branham@gmail.com>
> Date: Tue, 15 Jan 2019 11:57:52 -0600
> 
> Here's a tiny patch that changes the call from copy-keymap to make-sparse-keymap.
> 
> Thanks,
> Alex
> 
> >From c7f6db630f720fd26f0f1cc45631326bdcec136c Mon Sep 17 00:00:00 2001
> From: Alex Branham <alex.branham@gmail.com>
> Date: Tue, 15 Jan 2019 11:50:55 -0600
> Subject: [PATCH] Make tabulated-list-mode-map inherit from special-mode-map
> 
> * lisp/emacs-lisp/tabulated-list.el (tabulated-list-mode-map): Use
>   'make-sparse-keymap' rather than copying 'special-mode-map'.
> 
> Bug #30452

Maybe I'm missing something, but I thought the original bug report
said it should inherit from special-mode's keymap?  Your patch doesn't
seem to be doing that, or did I miss something?

More generally, I wonder why we don't say in the ELisp manual that a
derived mode should do this with its keymap.  Should we?

Thanks.

>  lisp/emacs-lisp/tabulated-list.el | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lisp/emacs-lisp/tabulated-list.el b/lisp/emacs-lisp/tabulated-list.el
> index 6fdca2cd08..8c99728fd4 100644
> --- a/lisp/emacs-lisp/tabulated-list.el
> +++ b/lisp/emacs-lisp/tabulated-list.el
> @@ -151,7 +151,7 @@ tabulated-list-put-tag
>        (forward-line)))
>  
>  (defvar tabulated-list-mode-map
> -  (let ((map (copy-keymap special-mode-map)))
> +  (let ((map (make-sparse-keymap)))
>      (set-keymap-parent map button-buffer-map)
>      (define-key map "n" 'next-line)
>      (define-key map "p" 'previous-line)
> -- 
> 2.19.2





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#30452: [PATCH] Use 'make-sparse-keymap' rather than 'copy-keymap'
  2019-01-19  8:05   ` Eli Zaretskii
@ 2019-01-19 15:33     ` Stefan Monnier
  2019-01-21 15:53       ` Alex Branham
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Monnier @ 2019-01-19 15:33 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Alex Branham, 30452

>>  (defvar tabulated-list-mode-map
>> -  (let ((map (copy-keymap special-mode-map)))
>> +  (let ((map (make-sparse-keymap)))
>>      (set-keymap-parent map button-buffer-map)

Nowadays we can inherit from both with something like

    (set-keymap-parent map (make-composed-keymap
                            button-buffer-map
                            special-mode-map))


-- Stefan





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#30452: [PATCH] Use 'make-sparse-keymap' rather than 'copy-keymap'
  2019-01-19 15:33     ` Stefan Monnier
@ 2019-01-21 15:53       ` Alex Branham
  2019-01-25  8:50         ` Eli Zaretskii
  0 siblings, 1 reply; 6+ messages in thread
From: Alex Branham @ 2019-01-21 15:53 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 30452


On Sat 19 Jan 2019 at 09:33, Stefan Monnier <monnier@IRO.UMontreal.CA> wrote:

>>>  (defvar tabulated-list-mode-map
>>> -  (let ((map (copy-keymap special-mode-map)))
>>> +  (let ((map (make-sparse-keymap)))
>>>      (set-keymap-parent map button-buffer-map)
>
> Nowadays we can inherit from both with something like
>
>     (set-keymap-parent map (make-composed-keymap
>                             button-buffer-map
>                             special-mode-map))

Thanks, that's better! Here's the updated patch.

Alex

From 6c21fb2434fa9f0499ebff5beabdbfda7b03f534 Mon Sep 17 00:00:00 2001
From: Alex Branham <alex.branham@gmail.com>
Date: Mon, 21 Jan 2019 09:50:11 -0600
Subject: [PATCH] Make tabulated-list-mode-map inherit from special-mode-map

* lisp/emacs-lisp/tabulated-list.el (tabulated-list-mode-map): Use
  'make-composed-keymap'.

Bug #30452
---
 lisp/emacs-lisp/tabulated-list.el | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lisp/emacs-lisp/tabulated-list.el b/lisp/emacs-lisp/tabulated-list.el
index 6fdca2cd08..12d0151d67 100644
--- a/lisp/emacs-lisp/tabulated-list.el
+++ b/lisp/emacs-lisp/tabulated-list.el
@@ -151,8 +151,10 @@ If ADVANCE is non-nil, move forward by one line afterwards."
       (forward-line)))
 
 (defvar tabulated-list-mode-map
-  (let ((map (copy-keymap special-mode-map)))
-    (set-keymap-parent map button-buffer-map)
+  (let ((map (make-sparse-keymap)))
+    (set-keymap-parent map (make-composed-keymap
+                            button-buffer-map
+                            special-mode-map))
     (define-key map "n" 'next-line)
     (define-key map "p" 'previous-line)
     (define-key map "S" 'tabulated-list-sort)
-- 
2.19.2







^ permalink raw reply related	[flat|nested] 6+ messages in thread

* bug#30452: [PATCH] Use 'make-sparse-keymap' rather than 'copy-keymap'
  2019-01-21 15:53       ` Alex Branham
@ 2019-01-25  8:50         ` Eli Zaretskii
  0 siblings, 0 replies; 6+ messages in thread
From: Eli Zaretskii @ 2019-01-25  8:50 UTC (permalink / raw)
  To: Alex Branham; +Cc: monnier, 30452-done

> From: Alex Branham <alex.branham@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, 30452@debbugs.gnu.org
> Date: Mon, 21 Jan 2019 09:53:29 -0600
> 
> 
> On Sat 19 Jan 2019 at 09:33, Stefan Monnier <monnier@IRO.UMontreal.CA> wrote:
> 
> >>>  (defvar tabulated-list-mode-map
> >>> -  (let ((map (copy-keymap special-mode-map)))
> >>> +  (let ((map (make-sparse-keymap)))
> >>>      (set-keymap-parent map button-buffer-map)
> >
> > Nowadays we can inherit from both with something like
> >
> >     (set-keymap-parent map (make-composed-keymap
> >                             button-buffer-map
> >                             special-mode-map))
> 
> Thanks, that's better! Here's the updated patch.

Thanks, pushed to the master branch.





^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-01-25  8:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-14  3:52 bug#30452: 25.3; tabulated-list-mode-map should inherit from special-mode-map Stephen Jung
2019-01-15 17:57 ` bug#30452: [PATCH] Use 'make-sparse-keymap' rather than 'copy-keymap' Alex Branham
2019-01-19  8:05   ` Eli Zaretskii
2019-01-19 15:33     ` Stefan Monnier
2019-01-21 15:53       ` Alex Branham
2019-01-25  8:50         ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).