GNU bug report logs - #30452
25.3; tabulated-list-mode-map should inherit from special-mode-map

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 30452 in the body.
You can then email your comments to 30452 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: Stephen Jung <tummychow511 <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.3; tabulated-list-mode-map should inherit from special-mode-map
Date: Tue, 13 Feb 2018 19:52:17 -0800

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

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

From: Alex Branham <alex.branham <at> gmail.com>
To: 30452 <at> debbugs.gnu.org
Subject: [PATCH] Use 'make-sparse-keymap' rather than 'copy-keymap'
Date: Tue, 15 Jan 2019 11:57:52 -0600

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

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 <at> 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

[0001-Make-tabulated-list-mode-map-inherit-from-special-mo.patch (text/x-patch, inline)]

From c7f6db630f720fd26f0f1cc45631326bdcec136c Mon Sep 17 00:00:00 2001
From: Alex Branham <alex.branham <at> 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

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alex Branham <alex.branham <at> gmail.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 30452 <at> debbugs.gnu.org
Subject: Re: bug#30452: [PATCH] Use 'make-sparse-keymap' rather than
 'copy-keymap'
Date: Sat, 19 Jan 2019 10:05:17 +0200

> From: Alex Branham <alex.branham <at> 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 <at> 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

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Alex Branham <alex.branham <at> gmail.com>, 30452 <at> debbugs.gnu.org
Subject: Re: bug#30452: [PATCH] Use 'make-sparse-keymap' rather than
 'copy-keymap'
Date: Sat, 19 Jan 2019 10:33:12 -0500

>>  (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

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

From: Alex Branham <alex.branham <at> gmail.com>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 30452 <at> debbugs.gnu.org
Subject: Re: bug#30452: [PATCH] Use 'make-sparse-keymap' rather than
 'copy-keymap'
Date: Mon, 21 Jan 2019 09:53:29 -0600

On Sat 19 Jan 2019 at 09:33, Stefan Monnier <monnier <at> 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 <at> 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

Message #22 received at 30452-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alex Branham <alex.branham <at> gmail.com>
Cc: monnier <at> IRO.UMontreal.CA, 30452-done <at> debbugs.gnu.org
Subject: Re: bug#30452: [PATCH] Use 'make-sparse-keymap' rather than
 'copy-keymap'
Date: Fri, 25 Jan 2019 10:50:29 +0200

> From: Alex Branham <alex.branham <at> gmail.com>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 30452 <at> debbugs.gnu.org
> Date: Mon, 21 Jan 2019 09:53:29 -0600
> 
> 
> On Sat 19 Jan 2019 at 09:33, Stefan Monnier <monnier <at> 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.

Previous Next

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