Re: emacs-30 ab7c40ea52a: Fix Imenu in 'emacs-news-view-mode'

Re: emacs-30 ab7c40ea52a: Fix Imenu in 'emacs-news-view-mode'

[Robert Pluim]

>>>>> On Sat, 20 Jul 2024 05:34:04 -0400 (EDT), Eli Zaretskii <eliz@gnu.org> said:

    Eli> branch: emacs-30
    Eli> commit ab7c40ea52a1db0cfc8b6605363f4e1378c9b471
    Eli> Author: Eli Zaretskii <eliz@gnu.org>
    Eli> Commit: Eli Zaretskii <eliz@gnu.org>

    Eli>     Fix Imenu in 'emacs-news-view-mode'
    
    Eli>     * lisp/textmodes/emacs-news-mode.el (emacs-news-view-mode): Make
    Eli>     it derived from emacs-news-mode.  Add useful key bindings.
    Eli>     (Bug#72080)

If you want all the bindings of `special-mode', you could just inherit
from it:

diff --git a/lisp/textmodes/emacs-news-mode.el b/lisp/textmodes/emacs-news-mode.el
index ca897ec4567..2c6a5f174ed 100644
--- a/lisp/textmodes/emacs-news-mode.el
+++ b/lisp/textmodes/emacs-news-mode.el
@@ -75,12 +75,10 @@ emacs-news-mode-menu
     "--"
     ["Enter View Mode" emacs-news-view-mode :help "Enter view-only mode"]))
 
-(defvar emacs-news-view-mode-map
-  ;; This is defined this way instead of inheriting because we're
-  ;; deriving the mode from `special-mode' and want the keys from there.
-  (let ((map (copy-keymap emacs-news-common-map)))
-    (keymap-set map "C-x C-q" #'emacs-news-mode)
-    map))
+(defvar-keymap emacs-news-view-mode-map
+  :parent (make-composed-keymap emacs-news-mode-map special-mode-map)
+  "C-x C-q" #'emacs-news-mode
+)
 
 (defvar emacs-news-mode-font-lock-keywords
   `(("^---$" 0 'emacs-news-does-not-need-documentation)
@@ -111,17 +109,7 @@ emacs-news-view-mode
   "Major mode for viewing the Emacs NEWS file."
   (setq buffer-read-only t)
   (emacs-news--buttonize)
-  (button-mode)
-  ;; Bind useful browsing keys.
-  (keymap-local-set "q" 'quit-window)
-  (keymap-local-set "SPC" 'scroll-up-command)
-  (keymap-local-set "S-SPC" 'scroll-down-command)
-  (keymap-local-set "DEL" 'scroll-down-command)
-  (keymap-local-set "?" 'describe-mode)
-  (keymap-local-set "h" 'describe-mode)
-  (keymap-local-set ">" 'end-of-buffer)
-  (keymap-local-set "<" 'beginning-of-buffer)
-  (keymap-local-set "g" 'revert-buffer))
+  (button-mode))
 
 (defun emacs-news--fill-paragraph (&optional justify)
   (cond


Robert
-- 

Re: emacs-30 ab7c40ea52a: Fix Imenu in 'emacs-news-view-mode'

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>
> Date: Mon, 22 Jul 2024 13:19:55 +0200
> 
> >>>>> On Sat, 20 Jul 2024 05:34:04 -0400 (EDT), Eli Zaretskii <eliz@gnu.org> said:
> 
>     Eli> branch: emacs-30
>     Eli> commit ab7c40ea52a1db0cfc8b6605363f4e1378c9b471
>     Eli> Author: Eli Zaretskii <eliz@gnu.org>
>     Eli> Commit: Eli Zaretskii <eliz@gnu.org>
> 
>     Eli>     Fix Imenu in 'emacs-news-view-mode'
>     
>     Eli>     * lisp/textmodes/emacs-news-mode.el (emacs-news-view-mode): Make
>     Eli>     it derived from emacs-news-mode.  Add useful key bindings.
>     Eli>     (Bug#72080)
> 
> If you want all the bindings of `special-mode', you could just inherit
> from it:

I tried, but it didn't work.  I guess this keymap stuff is above my
paygrade (make-composed-keymap? what's that?)  So I ended up doing
what did work.

Feel free to make any changes you see fit, but please be sure to test
them well, since this is the release branch.  One requirement is that
if you go back top emacs-news-mode, those key bindings go away.
Another requirement is not to disable outline-minor-mode bindings and
menus.

Re: emacs-30 ab7c40ea52a: Fix Imenu in 'emacs-news-view-mode'

[Robert Pluim]

>>>>> On Mon, 22 Jul 2024 14:39:28 +0300, Eli Zaretskii <eliz@gnu.org> said:


    Eli> I tried, but it didn't work.  I guess this keymap stuff is above my
    Eli> paygrade (make-composed-keymap? what's that?)  So I ended up doing
    Eli> what did work.

`make-composed-keymap' just makes a new keymap referring to the
keymap(s) you pass it. I guess my mind just twists the right way 🙂

    Eli> Feel free to make any changes you see fit, but please be sure to test
    Eli> them well, since this is the release branch.  One requirement is that
    Eli> if you go back top emacs-news-mode, those key bindings go away.
    Eli> Another requirement is not to disable outline-minor-mode bindings and
    Eli> menus.

Iʼd be stunned if changing major mode messed with a minor mode. But
Iʼll test it.

Should `emacs-news-view-mode' still have bindings for the
`emacs-news-mode' commands that change the buffer? Theyʼd all fail
because of the read-only buffer, but we could remove them. (hmm, the
menus are wrong for `emacs-news-view-mode' anyway. Iʼll fix that as
well).

Robert
-- 

Re: emacs-30 ab7c40ea52a: Fix Imenu in 'emacs-news-view-mode'

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Mon, 22 Jul 2024 14:36:55 +0200
> 
> Should `emacs-news-view-mode' still have bindings for the
> `emacs-news-mode' commands that change the buffer?

No.

> Theyʼd all fail because of the read-only buffer, but we could remove
> them. (hmm, the menus are wrong for `emacs-news-view-mode'
> anyway. Iʼll fix that as well).

Thanks.

Re: emacs-30 ab7c40ea52a: Fix Imenu in 'emacs-news-view-mode'

[Robert Pluim]

>>>>> On Mon, 22 Jul 2024 15:48:09 +0300, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Robert Pluim <rpluim@gmail.com>
    >> Cc: emacs-devel@gnu.org
    >> Date: Mon, 22 Jul 2024 14:36:55 +0200
    >> 
    >> Should `emacs-news-view-mode' still have bindings for the
    >> `emacs-news-mode' commands that change the buffer?

    Eli> No.

Done.

    >> Theyʼd all fail because of the read-only buffer, but we could remove
    >> them. (hmm, the menus are wrong for `emacs-news-view-mode'
    >> anyway. Iʼll fix that as well).

Done in de9f9add138

The only difference in key-bindings between `emacs-news-mode' and
`emacs-news-view-mode' are the buffer editing commands and the
special mode commands (and `emacs-news-view-mode' has a menu without
the editing commands).

Robert
--