bug#26658: 25.2; cc-mode doesn't use prog-mode-map as parent for c-mode-base-map

bug#26658: 25.2; cc-mode doesn't use prog-mode-map as parent for c-mode-base-map

[Vasilij Schneidermann]

See title.  Modes derived from cc-mode inherit its keybindings, however
they won't inherit keys defined in prog-mode-map.  This could probably
be rectified by using (set-keymap-parent c-mode-base-map prog-mode-map)
at the strategically correct location (like the lengthy top-level if
form setting up that map).  Is there any reason to *not* have this
change?  I'd otherwise write a patch for it.



In GNU Emacs 25.2.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.22.10)
 of 2017-04-22 built on juergen
Windowing system distributor 'The X.Org Foundation', version 11.0.11903000
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' CPPFLAGS=-D_FORTIFY_SOURCE=2
 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro'

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

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message dired format-spec rfc822 mml
mml-sec password-cache epg epg-config gnus-util mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util help-fns help-mode easymenu
cl-loaddefs pcase cl-lib mail-prsvr mail-utils time-date mule-util
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt
fringe tabulated-list newcomment elisp-mode lisp-mode prog-mode register
page menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock
font-lock syntax facemenu font-core frame cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese charscript case-table epa-hook
jka-cmpr-hook help simple abbrev minibuffer cl-preloaded nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote dbusbind inotify dynamic-setting
system-font-setting font-render-setting move-toolbar gtk x-toolkit x
multi-tty make-network-process emacs)

Memory information:
((conses 16 86143 6257)
 (symbols 48 19774 0)
 (miscs 40 45 146)
 (strings 32 14298 4240)
 (string-bytes 1 408381)
 (vectors 16 11746)
 (vector-slots 8 430626 5147)
 (floats 8 166 174)
 (intervals 56 205 0)
 (buffers 976 18))

bug#26658: 25.2; cc-mode doesn't use prog-mode-map as parent for c-mode-base-map

[Glenn Morris]

This seems to be a duplicate of http://debbugs.gnu.org/23175

bug#26658: cc-mode doesn't use prog-mode-map as parent for c-mode-base-map

[Jostein Kjønigsen]

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

While this feature isn't by any means critical to me, the response seems
a bit overly cautious w.r.t. change IMO.
From what I can tell the main reason this has been tagged "WONT FIX" in
the past is that you don't want cc-mode to depend on prog-mode. That's a
bit odd, isn't it?
What makes that particularly strange is how pretty much every single major-
mode which derives from cc-mode probably will derive from prog-mode, so
for the loading of any mode derived from cc-mode, such a change would
probably lead to very few effective changes.
--
Regards
Jostein Kjønigsen

jostein@kjonigsen.net 🍵 jostein@gmail.com
https://jostein.kjonigsen.net



[-- Attachment #2: Type: text/html, Size: 1204 bytes --]

bug#26658: cc-mode doesn't use prog-mode-map as parent for c-mode-base-map

[Vasilij Schneidermann]

Another thing to keep in mind is that the assumption there isn't
anything interesting in prog-mode-map for cc-mode to use is wrong: The
bug report that prompted me to open this one was about the user putting
keys into it that end up being shared from all derived modes.

bug#26658: 25.2; cc-mode doesn't use prog-mode-map as parent for c-mode-base-map

[Alan Mackenzie]

Hello, Vasilij.

On Tue, Apr 25, 2017 at 20:52:45 +0200, Vasilij Schneidermann wrote:
> See title.  Modes derived from cc-mode inherit its keybindings, however
> they won't inherit keys defined in prog-mode-map.  This could probably
> be rectified by using (set-keymap-parent c-mode-base-map prog-mode-map)
> at the strategically correct location (like the lengthy top-level if
> form setting up that map).  Is there any reason to *not* have this
> change?  I'd otherwise write a patch for it.

I've been trying to think of good reasons not to do this (on the CC Mode
within Emacs), and haven't been able to come up with any yet.  ;-)

I think it's more likely that as define-derived-mode and prog-mode
gradually congealed into existence, prog-mode-map got left out of
c-mode-base-map, sort of forgotten.

So yes, this change would surely be a good idea, given that this CC Mode
is derived from prog-mode anyway.

Do you want to write the patch?  If so, have you got copyright
assignments, or can you manage to write it as an "insignificant" (i.e.
very small) change?  I think the limit for insignificant changes is less
than 15 lines, but I'm not absolutely sure.  This wouldn't even need any
changes in the manual.  :-)

If you don't really want to write the patch, just say so, and I'll do
it.

Thanks for the bug report!

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#26658: 25.2; cc-mode doesn't use prog-mode-map as parent for c-mode-base-map

[Vasilij Schneidermann]

> So yes, this change would surely be a good idea, given that this CC Mode
> is derived from prog-mode anyway.

The more I think about it, the less sure I become.  Pretty much every
CC-derived mode does (define-derived-mode my-mode prog-mode ...) anyway
and according to the macro expansion, its keymap's parent is set to
(current-local-map) which should be equivalent to `prog-mode-map'.
Could very well be a non-issue in practice and merely faulty testing by
the user...

> Do you want to write the patch?  If so, have you got copyright
> assignments, or can you manage to write it as an "insignificant" (i.e.
> very small) change?  I think the limit for insignificant changes is less
> than 15 lines, but I'm not absolutely sure.  This wouldn't even need any
> changes in the manual.  :-)
> 
> If you don't really want to write the patch, just say so, and I'll do
> it.

I've assigned copyright already, but reckon this is a one-line patch.
Before I submit anything, I'd like to make sure it's needed and the
change works out as expected.

bug#26658: 25.2; cc-mode doesn't use prog-mode-map as parent for c-mode-base-map

[Vasilij Schneidermann]

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

Hello again,

I've ensured the bug is present and wrote a patch that fixes the issue.
A workaround for people not using the latest cc-mode is to put the
following into their init file:

(eval-after-load 'cc-mode
  '(set-keymap-parent c-mode-base-map prog-mode-map))

[-- Attachment #2: 0001-Set-c-mode-base-map-parent-to-prog-mode-map.patch --]
[-- Type: text/x-diff, Size: 2267 bytes --]

From d60e354b809f8a551fcdf7322ab16a623153f02f Mon Sep 17 00:00:00 2001
From: Vasilij Schneidermann <vasilij.schneidermann@bevuta.com>
Date: Tue, 2 May 2017 23:12:21 +0200
Subject: [PATCH] Set c-mode-base-map parent to prog-mode-map

* cc-mode.el (c-set-keymap-parent):
New helper function for XEmacs and GNU Emacs.
(c-mode-base-map): Set keymap parent for the map to prog-mode-map
---
 lisp/progmodes/cc-mode.el | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/lisp/progmodes/cc-mode.el b/lisp/progmodes/cc-mode.el
index 20c63d4dbe..dc06a30f29 100644
--- a/lisp/progmodes/cc-mode.el
+++ b/lisp/progmodes/cc-mode.el
@@ -223,20 +223,23 @@ c-initialize-cc-mode
 (defvar c-mode-base-map ()
   "Keymap shared by all CC Mode related modes.")
 
+(defun c-set-keymap-parent (map parent)
+  ;; Necessary to use `cc-bytecomp-fboundp' below since this
+  ;; function is called from top-level forms that are evaluated
+  ;; while cc-bytecomp is active when one does M-x eval-buffer.
+  (cond
+   ;; Emacs
+   ((cc-bytecomp-fboundp 'set-keymap-parent)
+    (set-keymap-parent map parent))
+   ;; XEmacs
+   ((fboundp 'set-keymap-parents)
+    (set-keymap-parents map parent))
+   ;; incompatible
+   (t (error "CC Mode is incompatible with this version of Emacs"))))
+
 (defun c-make-inherited-keymap ()
   (let ((map (make-sparse-keymap)))
-    ;; Necessary to use `cc-bytecomp-fboundp' below since this
-    ;; function is called from top-level forms that are evaluated
-    ;; while cc-bytecomp is active when one does M-x eval-buffer.
-    (cond
-     ;; Emacs
-     ((cc-bytecomp-fboundp 'set-keymap-parent)
-      (set-keymap-parent map c-mode-base-map))
-     ;; XEmacs
-     ((fboundp 'set-keymap-parents)
-      (set-keymap-parents map c-mode-base-map))
-     ;; incompatible
-     (t (error "CC Mode is incompatible with this version of Emacs")))
+    (c-set-keymap-parent map c-mode-base-map)
     map))
 
 (defun c-define-abbrev-table (name defs &optional doc)
@@ -276,6 +279,7 @@ c-bind-special-erase-keys
     nil
 
   (setq c-mode-base-map (make-sparse-keymap))
+  (c-set-keymap-parent c-mode-base-map prog-mode-map)
 
   ;; Separate M-BS from C-M-h.  The former should remain
   ;; backward-kill-word.
-- 
2.12.2

bug#26658: 25.2; cc-mode doesn't use prog-mode-map as parent for c-mode-base-map

[Alan Mackenzie]

Hello, Vasilij.

On Tue, May 02, 2017 at 23:17:14 +0200, Vasilij Schneidermann wrote:

> I've ensured the bug is present and wrote a patch that fixes the issue.
> A workaround for people not using the latest cc-mode is to put the
> following into their init file:

> (eval-after-load 'cc-mode
>   '(set-keymap-parent c-mode-base-map prog-mode-map))

Thanks for the patch.

[ .... ]

>    (setq c-mode-base-map (make-sparse-keymap))
> +  (c-set-keymap-parent c-mode-base-map prog-mode-map)

It doesn't seem to be quite right in the way it handles XEmacs stuff,
but I don't think it's possible to be "right" here, at least not
sensibly.  If this patch were to be applied to standalone CC Mode, it
would test (derived-mode-p 'prog-mode) before calling
(c-set-keymap-parent c-mode-base-map prog-mode-map).  prog-mode does not
exist in XEmacs, and is unlikely ever to exist there.

But this code is purely for Emacs.  What do you think?

For the commit message, the format is to aim for a complete first line
which makes sense on its own.  (It's a sentence, but without a
terminating full stop.)  So something like this would do:

#########################################################################
Make prog-mode-map the keymap parent of c-mode-base-map

Fixes bug #26658.

* lisp/progmodes/cc-mode.el (c-set-keymap-parent): New function
extracted from c-make-inherited-keymap.
(c-mode-base-map): .......

#########################################################################

Note that the path is given before "cc-mode.el".

Do you have commit access to the Emacs repository?

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#26658: 25.2; cc-mode doesn't use prog-mode-map as parent for c-mode-base-map

[Vasilij Schneidermann]

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

> It doesn't seem to be quite right in the way it handles XEmacs stuff,
> but I don't think it's possible to be "right" here, at least not
> sensibly.  If this patch were to be applied to standalone CC Mode, it
> would test (derived-mode-p 'prog-mode) before calling
> (c-set-keymap-parent c-mode-base-map prog-mode-map).  prog-mode does not
> exist in XEmacs, and is unlikely ever to exist there.
> 
> But this code is purely for Emacs.  What do you think?

Hm, I don't really have a way to test this in XEmacs anyway.  Testing
for prog-mode sounds like a good idea, but will the above example do the
right thing?  The issue with `derived-mode-p` is that it works upon the
*current* major mode instead of accepting a major mode to test against
other major modes.  What is the current major mode when this line is
evaluated?  Wouldn't it make more sense to use (boundp 'prog-mode-map),
considering that we're using the variable, not the mode?

> For the commit message, the format is to aim for a complete first line
> which makes sense on its own.  (It's a sentence, but without a
> terminating full stop.)  So something like this would do:
> 
> #########################################################################
> Make prog-mode-map the keymap parent of c-mode-base-map
> 
> Fixes bug #26658.
> 
> * lisp/progmodes/cc-mode.el (c-set-keymap-parent): New function
> extracted from c-make-inherited-keymap.
> (c-mode-base-map): .......
> 
> #########################################################################
> 
> Note that the path is given before "cc-mode.el".
> 
> Do you have commit access to the Emacs repository?

No and I doubt I'll ever get it because I don't understand the commit
message rules at all.  They appear to be loosely based upon GNU
changelog guidelines, but with everyone I send patches to interpreting
them differently, probably because there isn't much substance to them in
the first place.  I've resorted to using M-x add-change-log-entry,
copying the result into the commit message and adjusting it describing
my changes, but there's apparently more to it than that.

Anyway, here's an updated patch.

[-- Attachment #2: 0001-Make-prog-mode-map-the-parent-of-c-mode-base-map.patch --]
[-- Type: text/x-diff, Size: 944 bytes --]

From 1e4a92e05b78edb2f58c138aa7792d39a6d9a9cb Mon Sep 17 00:00:00 2001
From: Vasilij Schneidermann <vasilij.schneidermann@bevuta.com>
Date: Sun, 7 May 2017 15:55:49 +0200
Subject: [PATCH] Make prog-mode-map the parent of c-mode-base-map

Fixes #26658.

* lisp/progmodes/cc-mode.el (c-mode-base-map): Make prog-mode-map the
  parent of c-mode-map if possible.
---
 lisp/progmodes/cc-mode.el | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lisp/progmodes/cc-mode.el b/lisp/progmodes/cc-mode.el
index 20c63d4dbe..8f9e40bb0d 100644
--- a/lisp/progmodes/cc-mode.el
+++ b/lisp/progmodes/cc-mode.el
@@ -276,6 +276,9 @@ c-bind-special-erase-keys
     nil
 
   (setq c-mode-base-map (make-sparse-keymap))
+  (when (and (cc-bytecomp-fboundp 'set-keymap-parent)
+	     (boundp 'prog-mode-map))
+    (set-keymap-parent c-mode-base-map prog-mode-map))
 
   ;; Separate M-BS from C-M-h.  The former should remain
   ;; backward-kill-word.
-- 
2.12.2

bug#26658: 25.2; cc-mode doesn't use prog-mode-map as parent for c-mode-base-map

[Alan Mackenzie]

Hello, Vasilij.

On Sun, May 07, 2017 at 15:58:34 +0200, Vasilij Schneidermann wrote:
[ .... ]

> > Do you have commit access to the Emacs repository?

> No and I doubt I'll ever get it because I don't understand the commit
> message rules at all.  They appear to be loosely based upon GNU
> changelog guidelines, but with everyone I send patches to interpreting
> them differently, probably because there isn't much substance to them in
> the first place.  I've resorted to using M-x add-change-log-entry,
> copying the result into the commit message and adjusting it describing
> my changes, but there's apparently more to it than that.

I've committed your patch, slightly altered, and am closing the bug.  I
suspect that the commit message I used will be yet another variant.  ;-(
Thanks again for that patch.

> Anyway, here's an updated patch.

> >>From 1e4a92e05b78edb2f58c138aa7792d39a6d9a9cb Mon Sep 17 00:00:00 2001
> From: Vasilij Schneidermann <vasilij.schneidermann@bevuta.com>
> Date: Sun, 7 May 2017 15:55:49 +0200
> Subject: [PATCH] Make prog-mode-map the parent of c-mode-base-map

> Fixes #26658.

> * lisp/progmodes/cc-mode.el (c-mode-base-map): Make prog-mode-map the
>   parent of c-mode-map if possible.
> ---
>  lisp/progmodes/cc-mode.el | 3 +++
>  1 file changed, 3 insertions(+)

> diff --git a/lisp/progmodes/cc-mode.el b/lisp/progmodes/cc-mode.el
> index 20c63d4dbe..8f9e40bb0d 100644
> --- a/lisp/progmodes/cc-mode.el
> +++ b/lisp/progmodes/cc-mode.el
> @@ -276,6 +276,9 @@ c-bind-special-erase-keys
>      nil

>    (setq c-mode-base-map (make-sparse-keymap))
> +  (when (and (cc-bytecomp-fboundp 'set-keymap-parent)
> +	     (boundp 'prog-mode-map))
> +    (set-keymap-parent c-mode-base-map prog-mode-map))

>    ;; Separate M-BS from C-M-h.  The former should remain
>    ;; backward-kill-word.
> -- 
> 2.12.2

-- 
Alan Mackenzie (Nuremberg, Germany).