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

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

[Eric Abrahamsen]

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


Hopefully this is fairly uncontroversial: Gnus has a gnus-edit-form-mode
for editing group parameters and other things, which presents the data
as elisp forms and is a perfect use-case for lisp-data-mode -- turning
on paredit or what have you. This patch derives the former from the
latter.

Eric


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

From c8785b43abf7db5721950001e93f5a0c57b76d28 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

* lisp/gnus/gnus-eform.el (gnus-edit-form-mode): Handy for users who
have turned on things like paredit for lisp-data-mode.
---
 lisp/gnus/gnus-eform.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/gnus/gnus-eform.el b/lisp/gnus/gnus-eform.el
index 54118aad1e..ee573fb736 100644
--- a/lisp/gnus/gnus-eform.el
+++ b/lisp/gnus/gnus-eform.el
@@ -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

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

[Basil L. Contovounesios]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Hopefully this is fairly uncontroversial: Gnus has a gnus-edit-form-mode
> for editing group parameters and other things, which presents the data
> as elisp forms and is a perfect use-case for lisp-data-mode -- turning
> on paredit or what have you. This patch derives the former from the
> latter.

LGTM, FWIW.

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

Thanks,

-- 
Basil

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

[Eric Abrahamsen]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Hopefully this is fairly uncontroversial: Gnus has a gnus-edit-form-mode
>> for editing group parameters and other things, which presents the data
>> as elisp forms and is a perfect use-case for lisp-data-mode -- turning
>> on paredit or what have you. This patch derives the former from the
>> latter.
>
> LGTM, FWIW.
>
> 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 wonder what the point of writing it this way is, otherwise?

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

[Basil L. Contovounesios]

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.

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

-- 
Basil

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

[Eric Abrahamsen]

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

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

[Basil L. Contovounesios]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Here's another version...

Thanks, LGTM except for one tiny nit.

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

I'd say "Derive from lisp-data-mode" here as well, since the sentence
does not otherwise stand on its own without the preceding commit
summary's context.

-- 
Basil

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

[Eric Abrahamsen]

On 06/07/20 00:41 AM, Basil L. Contovounesios wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Here's another version...
>
> Thanks, LGTM except for one tiny nit.
>
>> 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.
>
> I'd say "Derive from lisp-data-mode" here as well, since the sentence
> does not otherwise stand on its own without the preceding commit
> summary's context.

Sure! I've made the change locally. I'll wait a few days for possible
objections (though I don't expect any) and then push.

Thanks,
Eric

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

[Eric Abrahamsen]

On 06/07/20 00:41 AM, Basil L. Contovounesios wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Here's another version...
>
> Thanks, LGTM except for one tiny nit.
>
>> 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.
>
> I'd say "Derive from lisp-data-mode" here as well, since the sentence
> does not otherwise stand on its own without the preceding commit
> summary's context.

Done, and pushed. Thanks!