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

[Jostein Kjønigsen]

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

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

[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.

[Alan Mackenzie]

Hello, Vasilij.

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!

[Vasilij Schneidermann]

Post by Alan Mackenzie
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...

Post by Alan Mackenzie
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.

[Vasilij Schneidermann]

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

[Alan Mackenzie]

Hello, Vasilij.

Post by Vasilij Schneidermann
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
(eval-after-load 'cc-mode
'(set-keymap-parent c-mode-base-map prog-mode-map))

Thanks for the patch.

[ .... ]

Post by Vasilij Schneidermann
(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?

[Vasilij Schneidermann]

Post by Alan Mackenzie
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?

Post by Alan Mackenzie
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
#########################################################################
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.

[Alan Mackenzie]

Hello, Vasilij.

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

Post by Vasilij Schneidermann

Post by Alan Mackenzie
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.

Post by Vasilij Schneidermann
Anyway, here's an updated patch.

Post by Alan Mackenzie

From 1e4a92e05b78edb2f58c138aa7792d39a6d9a9cb Mon Sep 17 00:00:00 2001

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