bug#14502: 24.3; dired-load-hook should not be customizable

bug#14502: 24.3; dired-load-hook should not be customizable

[Reuben Thomas]

dired-load-hook is customizable, but dired-x's manual correctly says
that you should add code to your init file to load dired-x in
dired-load-hook. I tried customizing the hook instead, and that doesn't
work. As with find-file-hook (see bug #13870), I suggest that
dired-load-hook either be made non-customizable, or a warning added to
the docstring.


In GNU Emacs 24.3.1 (x86_64-pc-linux-gnu, GTK+ Version 3.6.4)
 of 2013-04-14 on marid, modified by Debian
Windowing system distributor `The X.Org Foundation', version 11.0.11303000
System Description:	Ubuntu 13.04

Configured using:
 `configure '--build' 'x86_64-linux-gnu' '--build' 'x86_64-linux-gnu'
 '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib'
 '--localstatedir=/var/lib' '--infodir=/usr/share/info'
 '--mandir=/usr/share/man' '--with-pop=yes'
 '--enable-locallisppath=/etc/emacs24:/etc/emacs:/usr/local/share/emacs/24.3/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.3/site-lisp:/usr/share/emacs/site-lisp'
 '--with-crt-dir=/usr/lib/x86_64-linux-gnu' '--with-x=yes'
 '--with-x-toolkit=gtk3' '--with-toolkit-scroll-bars'
 'build_alias=x86_64-linux-gnu' 'CFLAGS=-g -O2 -fstack-protector
 --param=ssp-buffer-size=4 -Wformat -Werror=format-security -Wall'
 'LDFLAGS=-Wl,-Bsymbolic-functions -Wl,-z,relro'
 'CPPFLAGS=-D_FORTIFY_SOURCE=2''

Important settings:
  value of $LC_MONETARY: en_GB.UTF-8
  value of $LC_NUMERIC: en_GB.UTF-8
  value of $LC_TIME: en_GB.UTF-8
  value of $LANG: en_GB.UTF-8
  value of $XMODIFIERS: @im=local
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: Dired by name

Minor modes in effect:
  recentf-mode: t
  show-paren-mode: t
  server-mode: t
  savehist-mode: t
  minibuffer-electric-default-mode: t
  iswitchb-mode: t
  ido-everywhere: t
  icomplete-mode: t
  global-auto-revert-mode: t
  dired-omit-mode: t
  desktop-save-mode: t
  global-undo-tree-mode: t
  global-whitespace-mode: t
  TeX-PDF-mode: t
  TeX-source-correlate-mode: t
  dtrt-indent-mode: t
  tooltip-mode: t
  mouse-wheel-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
  buffer-read-only: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
<down-mouse-1> <mouse-1> M-< C-s d i r e d C-s C-s 
C-s C-s C-s C-s C-s C-a C-x k <return> C-x d <return> 
C-x k <return> C-x b v a r <return> C-x d <return> 
M-x r e p o r t - e m a c s - b u g <return>

Recent messages:
Mark set
Mark saved where search started
Omitting...
(Nothing to omit)
Omitting...
Omitted 2 lines.
Omitting...
(Nothing to omit)
Omitting...
Omitted 5 lines.

Load-path shadows:
/home/rrt/.emacs.d/el-get/dictionary/lpath hides /home/rrt/.emacs.d/el-get/auctex/lpath
/home/rrt/.emacs.d/el-get/magit/.dir-locals hides ~/.emacs.d/el-get/el-get/.dir-locals
/home/rrt/.emacs.d/el-get/package/elpa/csv-mode-1.50/csv-mode hides /usr/share/emacs24/site-lisp/emacs-goodies-el/csv-mode
/home/rrt/.emacs.d/el-get/quack/quack hides /usr/share/emacs24/site-lisp/emacs-goodies-el/quack
/home/rrt/.emacs.d/el-get/filladapt/filladapt hides /usr/share/emacs24/site-lisp/emacs-goodies-el/filladapt
/home/rrt/.emacs.d/el-get/markdown-mode/markdown-mode hides /usr/share/emacs24/site-lisp/emacs-goodies-el/markdown-mode
/home/rrt/.emacs.d/el-get/apache-mode/apache-mode hides /usr/share/emacs24/site-lisp/emacs-goodies-el/apache-mode
/home/rrt/.emacs.d/el-get/graphviz-dot-mode/graphviz-dot-mode hides /usr/share/emacs24/site-lisp/emacs-goodies-el/graphviz-dot-mode
/home/rrt/.emacs.d/el-get/browse-kill-ring/browse-kill-ring hides /usr/share/emacs24/site-lisp/emacs-goodies-el/browse-kill-ring
/home/rrt/local/share/emacs/site-lisp/lilypond-indent hides /usr/share/emacs/site-lisp/lilypond-indent
/home/rrt/local/share/emacs/site-lisp/lilypond-what-beat hides /usr/share/emacs/site-lisp/lilypond-what-beat
/usr/share/emacs/24.3/site-lisp/debian-startup hides /usr/share/emacs/site-lisp/debian-startup
/home/rrt/local/share/emacs/site-lisp/lilypond-init hides /usr/share/emacs/site-lisp/lilypond-init
/usr/share/emacs/24.3/site-lisp/cmake-data/cmake-mode hides /usr/share/emacs/site-lisp/cmake-mode
/home/rrt/local/share/emacs/site-lisp/lilypond-font-lock hides /usr/share/emacs/site-lisp/lilypond-font-lock
/usr/share/emacs/24.3/site-lisp/cdargs hides /usr/share/emacs/site-lisp/cdargs
/home/rrt/local/share/emacs/site-lisp/lilypond-mode hides /usr/share/emacs/site-lisp/lilypond-mode
/home/rrt/local/share/emacs/site-lisp/lilypond-song hides /usr/share/emacs/site-lisp/lilypond-song
/home/rrt/local/share/emacs/site-lisp/whitespace hides /usr/share/emacs/24.3/lisp/whitespace
/usr/share/emacs/site-lisp/rst hides /usr/share/emacs/24.3/lisp/textmodes/rst
/usr/share/emacs24/site-lisp/dictionaries-common/ispell hides /usr/share/emacs/24.3/lisp/textmodes/ispell
/usr/share/emacs24/site-lisp/dictionaries-common/flyspell hides /usr/share/emacs/24.3/lisp/textmodes/flyspell
/home/rrt/.emacs.d/el-get/cperl-mode/cperl-mode hides /usr/share/emacs/24.3/lisp/progmodes/cperl-mode
/home/rrt/.emacs.d/el-get/flymake/flymake hides /usr/share/emacs/24.3/lisp/progmodes/flymake
/home/rrt/.emacs.d/el-get/magit/.dir-locals hides /usr/share/emacs/24.3/lisp/gnus/.dir-locals

Features:
(shadow sort mail-extr emacsbug message format-spec rfc822 mml mml-sec
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader sendmail rfc2047 rfc2045 ietf-drums mail-utils dired-aux
misearch multi-isearch js json cperl-mode css-mode sgml-mode preview
prv-emacs noutline outline tex-buf font-latex latex tex-style
inform-mode imenu term disp-table ehelp electric cc-langs make-mode
nroff-mode vc-git sh-script smie executable jka-compr info flymake
face-remap goto-addr smart-quotes csv-mode-autoloads completing-help
ff-paths recentf tree-widget wid-edit uniquify paren server savehist
minibuf-eldef iswitchb ido icomplete autorevert cus-start cus-load
ropemacs pymacs dired-x desktop undo-tree diff whitespace tex dbus xml
crm remember .loaddefs yasnippet dropdown-list derived quack thingatpt
compile cmuscheme comint ansi-color scheme po-mode php-mode etags ring
cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine
cc-vars cc-defs speedbar sb-image ezimage dframe init-paredit paredit
edmacro kmacro go-mode url url-proxy url-privacy url-expand url-methods
url-history url-cookie url-domsuf url-util mailcap ffap url-parse
auth-source eieio gnus-util mm-util mail-prsvr password-cache url-vars
geiser-load geiser frame-restore flymake-point init-eldoc
eldoc-extension eldoc dtrt-indent byte-opt warnings darkroom-mode
frame-local-vars browse-kill-ring advice advice-preload auto-dictionary
flyspell ispell preview-latex tex-site auto-loads el-get
el-get-autoloads el-get-list-packages el-get-notify help-mode easymenu
el-get-dependencies el-get-build el-get-status pp el-get-recipes
el-get-byte-compile el-get-methods el-get-fossil el-get-svn
el-get-pacman el-get-github-zip el-get-github-tar el-get-http-zip
el-get-http-tar el-get-hg el-get-git-svn el-get-fink el-get-emacswiki
el-get-http el-get-emacsmirror el-get-github el-get-git el-get-elpa
package cl-macs gv el-get-darcs el-get-cvs el-get-bzr el-get-brew
el-get-builtin el-get-apt-get el-get-custom el-get-core autoload
help-fns bytecomp byte-compile cconv cl cl-lib dired user-site-loaddefs
emacs-goodies-el emacs-goodies-custom emacs-goodies-loaddefs easy-mmode
time-date tooltip ediff-hook vc-hooks lisp-float-type mwheel x-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list newcomment
lisp-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core frame cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev
minibuffer loaddefs button faces cus-face macroexp files text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process dbusbind
dynamic-setting system-font-setting font-render-setting move-toolbar gtk
x-toolkit x multi-tty emacs)

-- 
http://rrt.sc3d.org/

bug#14502: 24.3; dired-load-hook should not be customizable

[Lars Ingebrigtsen]

Reuben Thomas <rrt@sc3d.org> writes:

> dired-load-hook is customizable, but dired-x's manual correctly says
> that you should add code to your init file to load dired-x in
> dired-load-hook. I tried customizing the hook instead, and that doesn't
> work. As with find-file-hook (see bug #13870), I suggest that
> dired-load-hook either be made non-customizable, or a warning added to
> the docstring.

(I'm going through old bug reports that have unfortunately gotten no
responses yet.)

I'm not sure I understand the problem, and #13870 does not talk about
`find-file-hook'.

Does it matter whether you use `add-hook' or Customize for this
particular hook?  Most hooks are customisable, surely, including
`find-file-hook'.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#14502: 24.3; dired-load-hook should not be customizable

[Lars Ingebrigtsen]

(Please keep the debbugs address in the Cc header.)

Reuben Thomas <rrt@sc3d.org> writes:

>  I'm not sure I understand the problem, and #13870 does not talk about
>  `find-file-hook'.
>
> Sorry, that should say #13780. The problem is that add-hook (often in
> 3rd-party code) can be in a race with custom.el.
>
> As far as this bug report goes, if you try to customize dired-load-hook to load
> dired-x, it doesn't work.
>
>  Does it matter whether you use `add-hook' or Customize for this
>  particular hook?  Most hooks are customisable, surely, including
>  `find-file-hook'.
>
> Yes, and this can cause problems. See #13780.

The problem in #13780 is that if you mix add-hook with Customize
settings of a hook, then (depending on load order), the Customize value
for the hook may overwrite those add-hooks.  The solution proposed is
that one shouldn't have defcustoms on the hooks one may somehow expect
that packages add stuff to.

But dired-load-hook doesn't seem like one of those hooks, and if you
use Customize to be

(load-x)

where load-x is

(defun load-x ()
  (require 'dired-x))

then surely dired-x will be loaded with dired is loaded?  (Not that I've
tried myself.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#14502: 24.3; dired-load-hook should not be customizable

[Reuben Thomas via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

On Fri, 16 Aug 2019 at 02:29, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> (Please keep the debbugs address in the Cc header.)
>

Sorry!

The problem in #13780 is that if you mix add-hook with Customize
> settings of a hook, then (depending on load order), the Customize value
> for the hook may overwrite those add-hooks.  The solution proposed is
> that one shouldn't have defcustoms on the hooks one may somehow expect
> that packages add stuff to.
>
> But dired-load-hook doesn't seem like one of those hooks, and if you
> use Customize to be
>
> (load-x)
>
> where load-x is
>
> (defun load-x ()
>   (require 'dired-x))
>
> then surely dired-x will be loaded with dired is loaded?  (Not that I've
> tried myself.)
>

dired-x adds dired-extra-startup to dired-mode-hook…

dired-mode-hook in my setup also has sml/set-buffer-identification added by
smart-mode-line, so yes, it is "one of those hooks"

-- 
https://rrt.sc3d.org

[-- Attachment #2: Type: text/html, Size: 2035 bytes --]

bug#14502: 24.3; dired-load-hook should not be customizable

[Lars Ingebrigtsen]

Reuben Thomas <rrt@sc3d.org> writes:

> dired-x adds dired-extra-startup to dired-mode-hook…

I missed that bit.

> dired-mode-hook in my setup also has sml/set-buffer-identification added by
> smart-mode-line, so yes, it is "one of those hooks"

It seems rather unsatisfactory to have to make a decision between "one
of those hooks" and "the rest of the hooks" on a hook by hook basis...

Stefan wrote in 2013:

> So until Custom is fixed to actually properly handle customisation of
> hooks (e.g. by storing "add functions foo, bar, and baz" rather than
> "ignore current value and just set it to (foo bar baz)").

So I wondered what it would take to fix this in Customize properly.  But:

(define-widget 'hook 'list
  "An Emacs Lisp hook."

[...]

  :set (lambda (symbol value)
	 (dolist (elt value)
	   (if (fboundp elt)
	       (add-hook symbol elt))))

The :set action for hooks doesn't just overwrite the value, but uses
`add-hook' the proper way?

Now I'm even more confused.  Hm...  Perhaps that's not the code that's
used when actually restoring hook values?  Let's see.

(defun customize-set-variable (variable value &optional comment)

[...]

  (funcall (or (get variable 'custom-set) #'set-default) variable value)

(get 'gnus-load-hook 'custom-set)
=> nil

So the setter is not used?  Hm...  Oh, it's only set if you say

(defcustom ...
  :set ...)

Right.  So the :set in the widget is only used interactively?  *tests*
Nope, if I try to modify a hook in a Customize buffer, then it always
overwrites the hook value.

So, as usual, when trying to figure out how Customize works, it's...
just...  rather obscure.

But I think one (probably controversial solution) would be to add a
`custom-set' property on all hook variables that would make it use
`add-hook' instead of just overwriting the value.  That would make
Customize and add-hook coexist...  but would also mean that you can't
set a hook to a specific value with Customize, which will probably break
something for somebody.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#14502: 24.3; dired-load-hook should not be customizable

[Mauro Aranda]

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

Hello Lars.

Lars Ingebrigtsen <larsi@gnus.org> writes:

> So I wondered what it would take to fix this in Customize properly.  But:
>
> (define-widget 'hook 'list
>   "An Emacs Lisp hook."
>
> [...]
>
>   :set (lambda (symbol value)
> (dolist (elt value)
>   (if (fboundp elt)
>       (add-hook symbol elt))))

I don't know how things were when that code was added, but currently it
has no effect, because no code exists to handle that :set
property.  No widget type recognizes that property.  IOW, there is a new
property defined, but there's no code to make that property do
something.

> (defun customize-set-variable (variable value &optional comment)
>
> [...]
>
>   (funcall (or (get variable 'custom-set) #'set-default) variable value)
>
> (get 'gnus-load-hook 'custom-set)
> => nil
>
> So the setter is not used?  Hm...  Oh, it's only set if you say
>
> (defcustom ...
>   :set ...)
>
> Right.  So the :set in the widget is only used interactively?  *tests*
> Nope, if I try to modify a hook in a Customize buffer, then it always
> overwrites the hook value.
>
> So, as usual, when trying to figure out how Customize works, it's...
> just...  rather obscure.

See above.  I hope it clarifies something, at least.

> But I think one (probably controversial solution) would be to add a
> `custom-set' property on all hook variables that would make it use
> `add-hook' instead of just overwriting the value.  That would make
> Customize and add-hook coexist...  but would also mean that you can't
> set a hook to a specific value with Customize, which will probably break
> something for somebody.

I think this solution would be good.  As for the controversial part, I
think it would be possible to add some specific option that controls
whether to overwrite or not a hook when using Customize.  Also, a
checkbox could toggle that option when customizing a hook interactively.

But one problem is that using defcustom for the option would complicate
things a little more than using plain defvar.  For starters, every
hook would have to have a :set-after property with that option.

Best regards,
Mauro.

[-- Attachment #2: Type: text/html, Size: 2601 bytes --]

bug#14502: 24.3; dired-load-hook should not be customizable

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

> I think this solution would be good.  As for the controversial part, I
> think it would be possible to add some specific option that controls
> whether to overwrite or not a hook when using Customize.  Also, a
> checkbox could toggle that option when customizing a hook interactively.
>
> But one problem is that using defcustom for the option would complicate
> things a little more than using plain defvar.  For starters, every
> hook would have to have a :set-after property with that option.

Whatever solution we have should work generally and consistently.  So
defcustoms of type `hook' should either overwrite (as they do currently)
or not (which would be more logical, perhaps).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#14502: 24.3; dired-load-hook should not be customizable

[Stefan Kangas]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Reuben Thomas <rrt@sc3d.org> writes:
>
>> dired-load-hook is customizable, but dired-x's manual correctly says
>> that you should add code to your init file to load dired-x in
>> dired-load-hook. I tried customizing the hook instead, and that doesn't
>> work. As with find-file-hook (see bug #13870), I suggest that
>> dired-load-hook either be made non-customizable, or a warning added to
>> the docstring.
>
> (I'm going through old bug reports that have unfortunately gotten no
> responses yet.)
>
> I'm not sure I understand the problem, and #13870 does not talk about
> `find-file-hook'.
>
> Does it matter whether you use `add-hook' or Customize for this
> particular hook?  Most hooks are customisable, surely, including
> `find-file-hook'.

We now have that:

(make-obsolete-variable 'dired-load-hook
                        "use `with-eval-after-load' instead." "28.1")

So I think this problem has resolved itself, and I'm closing this bug.