* bug#28863: 26.0.90; [PATCH] Don't clobber docstrings of explicitly-defined mode hook variables
@ 2017-10-16 12:35 Phil Sainty
2017-10-21 23:03 ` Noam Postavsky
0 siblings, 1 reply; 4+ messages in thread
From: Phil Sainty @ 2017-10-16 12:35 UTC (permalink / raw)
To: 28863
[-- 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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* bug#28863: 26.0.90; [PATCH] Don't clobber docstrings of explicitly-defined mode hook variables
2017-10-16 12:35 bug#28863: 26.0.90; [PATCH] Don't clobber docstrings of explicitly-defined mode hook variables Phil Sainty
@ 2017-10-21 23:03 ` Noam Postavsky
2017-10-22 2:51 ` Phil Sainty
0 siblings, 1 reply; 4+ messages in thread
From: Noam Postavsky @ 2017-10-21 23:03 UTC (permalink / raw)
To: Phil Sainty; +Cc: 28863
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* bug#28863: 26.0.90; [PATCH] Don't clobber docstrings of explicitly-defined mode hook variables
2017-10-21 23:03 ` Noam Postavsky
@ 2017-10-22 2:51 ` Phil Sainty
2017-10-31 12:45 ` Noam Postavsky
0 siblings, 1 reply; 4+ messages in thread
From: Phil Sainty @ 2017-10-22 2:51 UTC (permalink / raw)
To: Noam Postavsky; +Cc: 28863
On 22/10/17 12:03, Noam Postavsky wrote:
> 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.
Yes indeed; but there are certainly examples of this pattern in use.
I'll follow up separately about this, but I don't it has any direct
bearing on this patch.
> Seems fine to me.
> My only nitpick is that you seem to be adding tab-indented lines.
Yes, that seemed consistent with the existing sources (or at least there
was a mixture of tabs vs spaces, as I recall).
...and I now see that repository's .dir-locals.el sets indent-tabs-mode
to nil for emacs-lisp-mode. I didn't realise that had happened.
(Has a read of bug #20323.)
I'll remember this in future.
If no one objects to this being committed, then let's just untabify the
changes and amend the commit before pushing.
-Phil
^ permalink raw reply [flat|nested] 4+ messages in thread
* bug#28863: 26.0.90; [PATCH] Don't clobber docstrings of explicitly-defined mode hook variables
2017-10-22 2:51 ` Phil Sainty
@ 2017-10-31 12:45 ` Noam Postavsky
0 siblings, 0 replies; 4+ messages in thread
From: Noam Postavsky @ 2017-10-31 12:45 UTC (permalink / raw)
To: Phil Sainty; +Cc: 28863
tags 28863 fixed
close 28863 26.1
quit
Phil Sainty <psainty@orcon.net.nz> writes:
> If no one objects to this being committed, then let's just untabify the
> changes and amend the commit before pushing.
Done and pushed to emacs-26.
[1: 3e7ebbe1bd]: 2017-10-31 08:20:30 -0400
Don't clobber docstrings of explicitly-defined mode hook variables
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=3e7ebbe1bd5d33476190c789d17e4dd2d5f1bdfb
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-10-31 12:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-16 12:35 bug#28863: 26.0.90; [PATCH] Don't clobber docstrings of explicitly-defined mode hook variables Phil Sainty
2017-10-21 23:03 ` Noam Postavsky
2017-10-22 2:51 ` Phil Sainty
2017-10-31 12:45 ` Noam Postavsky
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).