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

[Noam Postavsky <npostavs@users.sourceforge.net>]

severity 28863 minor
quit

Phil Sainty <psainty@orcon.net.nz> writes:

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

Giving a default value to a hook variable is problematic, because if the
user has (add-hook 'foo-mode-hook 'some-other-fun) then they lose the
default-value if foo-mode.el wasn't loaded yet.  So the usual way to do
it is like this:

(defcustom foo-mode-hook nil "docstring")
(add-hook 'foo-mode-hook 'default-value)

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

Seems fine to me.

> Patch attached for consideration.

My only nitpick is that you seem to be adding tab-indented lines.