bug#41742: 28.0.50; Derive gnus-edit-form-mode from lisp-data-mode

[Eric Abrahamsen <eric@ericabrahamsen.net>]

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


On 06/06/20 23:31 PM, Basil L. Contovounesios wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>>
>>> While there, would you mind cleaning up how gnus-edit-form-mode-map is
>>> defined?  It currently does a defvar+unless+setq dance, whereas it
>>> should ideally only be a defvar+let, as per the last few paragraphs of
>>> (info "(elisp) Tips for Defining").
>>
>> Huh, the last few paragraphs of that info page make it look like it's
>> _okay_ to do defvar+unless+setq, am I misreading this? Granted it only
>> says to do it so you can get the docstring closer to the defvar, and
>> there's no docstring in this case, but it does seem acceptable.
>
> I posit it's always better to defvar+let in one swell foop.

In principle, I agree.

>> I wonder what the point of writing it this way is, otherwise?
>
> In most places I've seen defvar+unless, it's due to the author thinking
> in terms of "if the user hasn't already defined this map before loading
> this file, then..."
>
> But defvar provides these semantics for free and in a cleaner way.
>
> Unless I'm missing something, that is.

I can't imagine anyone actually defining the whole map in their own
config, rather than just adding keybindings as necessary. And as you
(and the manual) point out, defvar already handles atomic
definition/double loading avoidance, etc.

Here's another version...


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Derive-gnus-edit-form-mode-from-lisp-data-mode-fix-m.patch --]
[-- Type: text/x-patch, Size: 1951 bytes --]

From 0ee576ed0819a15a37a01e2b3a3bc176e9827c04 Mon Sep 17 00:00:00 2001
From: Eric Abrahamsen <eric@ericabrahamsen.net>
Date: Sat, 6 Jun 2020 12:13:15 -0700
Subject: [PATCH] Derive gnus-edit-form-mode from lisp-data-mode, fix mode map

* lisp/gnus/gnus-eform.el (gnus-edit-form-mode): Handy for users who
have turned on things like paredit for lisp-data-mode.
(gnus-edit-form-mode-map): Put creation of the map inside the defvar.
---
 lisp/gnus/gnus-eform.el | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/lisp/gnus/gnus-eform.el b/lisp/gnus/gnus-eform.el
index 54118aad1e..1bc1261ee8 100644
--- a/lisp/gnus/gnus-eform.el
+++ b/lisp/gnus/gnus-eform.el
@@ -50,13 +50,13 @@ gnus-edit-form-menu-hook
 (defvar gnus-edit-form-buffer "*Gnus edit form*")
 (defvar gnus-edit-form-done-function nil)
 
-(defvar gnus-edit-form-mode-map nil)
-(unless gnus-edit-form-mode-map
-  (setq gnus-edit-form-mode-map (make-sparse-keymap))
-  (set-keymap-parent gnus-edit-form-mode-map emacs-lisp-mode-map)
-  (gnus-define-keys gnus-edit-form-mode-map
-    "\C-c\C-c" gnus-edit-form-done
-    "\C-c\C-k" gnus-edit-form-exit))
+(defvar gnus-edit-form-mode-map
+  (let ((map (make-sparse-keymap)))
+    (set-keymap-parent map emacs-lisp-mode-map)
+    (gnus-define-keys map
+      "\C-c\C-c" gnus-edit-form-done
+      "\C-c\C-k" gnus-edit-form-exit)
+    map))
 
 (defun gnus-edit-form-make-menu-bar ()
   (unless (boundp 'gnus-edit-form-menu)
@@ -67,9 +67,9 @@ gnus-edit-form-make-menu-bar
        ["Exit" gnus-edit-form-exit t]))
     (gnus-run-hooks 'gnus-edit-form-menu-hook)))
 
-(define-derived-mode gnus-edit-form-mode fundamental-mode "Edit Form"
+(define-derived-mode gnus-edit-form-mode lisp-data-mode "Edit Form"
   "Major mode for editing forms.
-It is a slightly enhanced emacs-lisp-mode.
+It is a slightly enhanced `lisp-data-mode'.
 
 \\{gnus-edit-form-mode-map}"
   (when (gnus-visual-p 'group-menu 'menu)
-- 
2.27.0