bug#28863: 26.0.90; [PATCH] Don't clobber docstrings of explicitly-defined mode hook variables

[Phil Sainty <psainty@orcon.net.nz>]

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

`define-derived-mode' and `define-minor-mode' each define their mode
hook variable, including a standard docstring, using:
(defvar ,hook nil ".....")

In the uncommon (but not unheard of) scenario whereby a library
defines a mode and also explicitly defines that mode's hook variable
with defvar or defcustom, the library author will encounter problems
no matter which way around they define them.


1. If the mode is defined first:

(define-derived-mode foo-mode ...)
(defcustom foo-mode-hook '(default value) "docstring")

Then `foo-mode-hook' has already been defvar'd with a value of nil,
and so the defcustom default value never gets set, so that sequence
is no good.


2. If the variable is defined first:

(defcustom foo-mode-hook '(default value) "docstring")
(define-derived-mode foo-mode ...)

Then `foo-mode-hook' gets its default value, but its original docstring
is clobbered by the one provided by `define-derived-mode' -- because
while the value of a bound variable is not affected by evaling its
definition again, the docstring does get updated.


3. We can work around that with a sequence such as:

(defcustom foo-mode-hook '(default value) "" ...)
(define-derived-mode foo-mode ...)
(put 'foo-mode-hook 'variable-documentation "docstring")

which works, but is cumbersome, as the hook variable's docstring is
now separated from the hook variable definition by the entirety of
the mode definition.


My suggestion is that `define-derived-mode' and `define-minor-mode'
should only add the standard docstring to the mode hook variable
conditional on that variable not *already* having a docstring.

That would then allow approach (2) to have the same effect as (3)
but in a much tidier manner.

Patch attached for consideration.


-Phil





In GNU Emacs 26.0.90 (build 1, x86_64-pc-linux-gnu, X toolkit, Xaw3d
scroll bars)
 of 2017-10-17 built on shodan
Repository revision: ead257cbfc2c7e9196210b899f6d4e6496c0a42b
Windowing system distributor 'The X.Org Foundation', version 11.0.11804000
System Description:	Ubuntu 16.04.3 LTS

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

Configured using:
 'configure --prefix=/home/phil/emacs/trunk/usr/local
 --with-x-toolkit=lucid --without-sound'

Configured features:
XAW3D XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK DBUS GSETTINGS NOTIFY
GNUTLS LIBXML2 FREETYPE XFT ZLIB TOOLKIT_SCROLL_BARS LUCID X11 LCMS2

Important settings:
  value of $LANG: en_NZ.UTF-8
  value of $XMODIFIERS:
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  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

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny seq byte-opt gv
bytecomp byte-compile cconv cl-loaddefs cl-lib dired dired-loaddefs
format-spec rfc822 mml easymenu mml-sec password-cache epa derived epg
epg-config gnus-util rmail rmail-loaddefs mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils elec-pair time-date
mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page menu-bar
rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core term/tty-colors 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 composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray 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 lcms2
dynamic-setting system-font-setting font-render-setting x-toolkit x
multi-tty make-network-process emacs)

Memory information:
((conses 16 96192 5551)
 (symbols 48 20686 1)
 (miscs 40 41 93)
 (strings 32 30399 1252)
 (string-bytes 1 771583)
 (vectors 16 13978)
 (vector-slots 8 492632 7908)
 (floats 8 51 66)
 (intervals 56 232 0)
 (buffers 992 11)
 (heap 1024 32112 1181))

[-- Attachment #2: 0001-Don-t-clobber-docstrings-of-explicitly-defined-mode-.patch --]
[-- Type: text/x-patch, Size: 2191 bytes --]

From ead257cbfc2c7e9196210b899f6d4e6496c0a42b Mon Sep 17 00:00:00 2001
From: Phil Sainty <psainty@orcon.net.nz>
Date: Mon, 16 Oct 2017 23:38:42 +1300
Subject: [PATCH] Don't clobber docstrings of explicitly-defined mode hook
 variables

* lisp/emacs-lisp/derived.el (define-derived-mode):
* lisp/emacs-lisp/easy-mmode.el (define-minor-mode): When defining the
mode hook variable, do not clobber pre-existing docstrings.
---
 lisp/emacs-lisp/derived.el    | 8 +++++---
 lisp/emacs-lisp/easy-mmode.el | 8 +++++---
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/lisp/emacs-lisp/derived.el b/lisp/emacs-lisp/derived.el
index 3fa3818..e8910d1 100644
--- a/lisp/emacs-lisp/derived.el
+++ b/lisp/emacs-lisp/derived.el
@@ -203,11 +203,13 @@ define-derived-mode
 		     parent child docstring syntax abbrev))
 
     `(progn
-       (defvar ,hook nil
-         ,(format "Hook run after entering %s mode.
+       (defvar ,hook nil)
+       (unless (get ',hook 'variable-documentation)
+	 (put ',hook 'variable-documentation
+	      ,(format "Hook run after entering %s mode.
 No problems result if this variable is not bound.
 `add-hook' automatically binds it.  (This is true for all hook variables.)"
-                  name))
+		       name)))
        (unless (boundp ',map)
 	 (put ',map 'definition-name ',child))
        (with-no-warnings (defvar ,map (make-sparse-keymap)))
diff --git a/lisp/emacs-lisp/easy-mmode.el b/lisp/emacs-lisp/easy-mmode.el
index bf087fc..220691c 100644
--- a/lisp/emacs-lisp/easy-mmode.el
+++ b/lisp/emacs-lisp/easy-mmode.el
@@ -309,11 +309,13 @@ define-minor-mode
        ;; up-to-here.
        :autoload-end
 
-       (defvar ,hook nil
-         ,(format "Hook run after entering or leaving `%s'.
+       (defvar ,hook nil)
+       (unless (get ',hook 'variable-documentation)
+	 (put ',hook 'variable-documentation
+	      ,(format "Hook run after entering or leaving `%s'.
 No problems result if this variable is not bound.
 `add-hook' automatically binds it.  (This is true for all hook variables.)"
-		  modefun))
+		       modefun)))
 
        ;; Define the minor-mode keymap.
        ,(unless (symbolp keymap)	;nil is also a symbol.
-- 
2.8.3