bug#10773: set-variable can't change values of user options

bug#10773: set-variable can't change values of user options

[Juri Linkov]

Package: emacs
Version: 24.0.93
Severity: wishlist

emacs -Q C-h C-t
M-x set-variable RET outline-mode-hook RET (hide-body) RET
M-x set-variable RET outline-minor-mode-hook RET [No match]

Why users are allowed to change the value of `outline-mode-hook',
but not `outline-minor-mode-hook'?

bug#10773: set-variable can't change values of user options

[Glenn Morris]

Juri Linkov wrote:

> Why users are allowed to change the value of `outline-mode-hook',
> but not `outline-minor-mode-hook'?

The former is defined with (defvar ... "*...").
The latter is not defined anywhere, but as define-derived-mode says
about hooks and has been covered several times:

  No problems result if this variable is not bound.
  `add-hook' automatically binds it.  (This is true for all hook variables.)

I would says it's not very useful to pass a hook to set-variable anyway,
since you need to type a lisp expression, and probably should use
an explicit add-hook statement.

bug#10773: set-variable can't change values of user options

[Kevin Rodgers]

On 2/9/12 11:28 AM, Glenn Morris wrote:
> Juri Linkov wrote:
>
>> Why users are allowed to change the value of `outline-mode-hook',
>> but not `outline-minor-mode-hook'?
>
> The former is defined with (defvar ... "*...").
> The latter is not defined anywhere, but as define-derived-mode says
> about hooks and has been covered several times:
>
>    No problems result if this variable is not bound.
>    `add-hook' automatically binds it.  (This is true for all hook variables.)
>
> I would says it's not very useful to pass a hook to set-variable anyway,
> since you need to type a lisp expression, and probably should use
> an explicit add-hook statement.

Or allow add-hook (and remove-hook) to be called interactively, for convenience:

--- -	2012-02-10 01:14:16.000000000 -0700
+++ /tmp/subr.el	2012-02-10 01:13:17.000000000 -0700
@@ -1216,15 +1216,17 @@
  unless the optional argument APPEND is non-nil, in which case
  FUNCTION is added at the end.

-The optional fourth argument, LOCAL, if non-nil, says to modify
-the hook's buffer-local value rather than its default value.
-This makes the hook buffer-local if needed, and it makes t a member
-of the buffer-local value.  That acts as a flag to run the hook
-functions in the default value as well as in the local value.
+The optional fourth argument, LOCAL, if non-nil or an interactive
+prefix arg, says to modify the hook's buffer-local value rather
+than its default value.  This makes the hook buffer-local if
+needed, and it makes t a member of the buffer-local value.  That
+acts as a flag to run the hook functions in the default value as
+well as in the local value.

  HOOK should be a symbol, and FUNCTION may be any valid function.  If
  HOOK is void, it is first set to nil.  If HOOK's value is a single
  function, it is changed to a list of functions."
+  (interactive "SHook: \naFunction: \ni\nP")
    (or (boundp hook) (set hook nil))
    (or (default-boundp hook) (set-default hook nil))
    (if local (unless (local-variable-if-set-p hook)
@@ -1264,8 +1266,10 @@
  FUNCTION isn't the value of HOOK, or, if FUNCTION doesn't appear in the
  list of hooks to run in HOOK, then nothing is done.  See `add-hook'.

-The optional third argument, LOCAL, if non-nil, says to modify
-the hook's buffer-local value rather than its default value."
+The optional third argument, LOCAL, if non-nil (interactively,
+with prefix argument) says to modify the hook's buffer-local
+value rather than its default value."
+  (interactive "SHook: \naFunction: \nP")
    (or (boundp hook) (set hook nil))
    (or (default-boundp hook) (set-default hook nil))
    ;; Do nothing if LOCAL is t but this hook has no local binding.

-- 
Kevin Rodgers
Denver, Colorado, USA

bug#10773: set-variable can't change values of user options

[Juri Linkov]

>> Why users are allowed to change the value of `outline-mode-hook',
>> but not `outline-minor-mode-hook'?
>
> The former is defined with (defvar ... "*...").
> The latter is not defined anywhere, but as define-derived-mode says
> about hooks and has been covered several times:
>
>   No problems result if this variable is not bound.
>   `add-hook' automatically binds it.  (This is true for all hook variables.)
>
> I would says it's not very useful to pass a hook to set-variable anyway,
> since you need to type a lisp expression, and probably should use
> an explicit add-hook statement.

Currently William Stevenson is working on converting minor modes to use
`define-minor-mode', so more minor mode hooks will lose an ability to be
changed using `set-variable'.  I don't have an opinion whether this is
good or bad.  I just discovered the inconsistency between `outline-mode-hook'
and `outline-minor-mode-hook' when trying to set temporarily them
to the same value with `set-variable'.

What I still don't understand is why some hooks have "*" in the docstring.

bug#10773: set-variable can't change values of user options

[Glenn Morris]

Juri Linkov wrote:

> Currently William Stevenson is working on converting minor modes to use
> `define-minor-mode', so more minor mode hooks will lose an ability to be
> changed using `set-variable'.  I don't have an opinion whether this is
> good or bad. 

I think it is irrelevant / good.

> What I still don't understand is why some hooks have "*" in the docstring.

Perhaps it depends whether one interprets "*" as meaning "something a
user might want to set in .emacs", or "something a user might want to
set interactively" (this is what "Tips and Conventions" says).

bug#10773: set-variable can't change values of user options

[Glenn Morris]

Glenn Morris wrote:

>> Currently William Stevenson is working on converting minor modes to use
>> `define-minor-mode', so more minor mode hooks will lose an ability to be
>> changed using `set-variable'.  I don't have an opinion whether this is
>> good or bad. 
>
> I think it is irrelevant / good.

PS If it is thought to be bad, the right solution would seem to be to
make define-minor-mode declare the hook with an appropriate defvar,
which it easily could. But I think that has been previously rejected.

bug#10773: set-variable can't change values of user options

[Stefan Monnier]

> But I think that has been previously rejected.

That was a long time ago.  We should do it for 24.2.


        Stefan

bug#10773: set-variable can't change values of user options

[Glenn Morris]

Stefan Monnier wrote:

>> But I think that has been previously rejected.
>
> That was a long time ago.  We should do it for 24.2.

That's a surprise. What's changed?
This feature was taken out 2005-05-17.
Why does define-derived-mode deliberately go out of its way to NOT do
this? Will that be changed too?

bug#10773: set-variable can't change values of user options

[Stefan Monnier]

>>> But I think that has been previously rejected.
>> That was a long time ago.  We should do it for 24.2.
> That's a surprise.  What's changed?

Resources.

> This feature was taken out 2005-05-17.
> Why does define-derived-mode deliberately go out of its way to NOT do
> this?  Will that be changed too?

Yes,


        Stefan

bug#10773: set-variable can't change values of user options

[Juri Linkov]

In my opinion, the best way to eliminate the distinction
between user variables (with the leading '*' in the docstring)
and customizable variables in 24.2 is:

1. Make `user-variable-p' an alias for `custom-variable-p'.

2. Remove the `user-variable-p' condition from `set-variable'.

This will fix the reported case where `set-variable' can change
the value of `outline-mode-hook', but not `outline-minor-mode-hook';
and many other similar cases.

bug#10773: set-variable can't change values of user options

[Drew Adams]

> In my opinion, the best way to eliminate the distinction
> between user variables (with the leading '*' in the docstring)
> and customizable variables in 24.2 is:
> 
> 1. Make `user-variable-p' an alias for `custom-variable-p'.
> 
> 2. Remove the `user-variable-p' condition from `set-variable'.
> 
> This will fix the reported case where `set-variable' can change
> the value of `outline-mode-hook', but not `outline-minor-mode-hook';
> and many other similar cases.

And how will that help code that uses
(defvar foo "*A user variable.")?

Users have always been able to set such a user variable using `set-variable'.

bug#10773: set-variable can't change values of user options

[Juri Linkov]

>> In my opinion, the best way to eliminate the distinction
>> between user variables (with the leading '*' in the docstring)
>> and customizable variables in 24.2 is:
>>
>> 1. Make `user-variable-p' an alias for `custom-variable-p'.
>>
>> 2. Remove the `user-variable-p' condition from `set-variable'.
>>
>> This will fix the reported case where `set-variable' can change
>> the value of `outline-mode-hook', but not `outline-minor-mode-hook';
>> and many other similar cases.
>
> And how will that help code that uses
> (defvar foo "*A user variable.")?

IIUC, the plan is to deprecate this feature, to not maintain two parallel
systems of user customizable variables.

> Users have always been able to set such a user variable using `set-variable'.

With the proposed change above, users can continue to set variables
using `set-variable'.

bug#10773: set-variable can't change values of user options

[Lars Ingebrigtsen]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> But I think that has been previously rejected.
>
> That was a long time ago.  We should do it for 24.2.

Reintroducing defvars for define-minor-mode hook variables was done the
following year:

commit 7f17cc40ef9120ba1b1715399432f26e8850505a
Author:     Stefan Monnier <monnier@iro.umontreal.ca>
AuthorDate: Mon May 27 12:12:52 2013 -0400

    Always defvar a mode's hook and provide a docstring.
    * lisp/emacs-lisp/easy-mmode.el (define-minor-mode):
    * lisp/emacs-lisp/derived.el (define-derived-mode): Always defvar the
    mode hook and provide a docstring.

There's still the question of using allowing these variables to be
customized, and that still doesn't work:

(customize-variable 'flyspell-mode-hook)
-> "NO CUSTOMIZATION DATA; not intended to be customized."

But...  why not?  So I've now added the patch below to Emacs 28.

diff --git a/lisp/emacs-lisp/easy-mmode.el b/lisp/emacs-lisp/easy-mmode.el
index e3eb9294ed..fdc1233540 100644
--- a/lisp/emacs-lisp/easy-mmode.el
+++ b/lisp/emacs-lisp/easy-mmode.el
@@ -335,6 +335,9 @@ define-minor-mode
 No problems result if this variable is not bound.
 `add-hook' automatically binds it.  (This is true for all hook variables.)"
                        modefun)))
+       ;; Allow using using `M-x customize-variable' on the hook.
+       (put ',hook 'custom-type 'hook)
+       (put ',hook 'standard-value (list nil))
 
        ;; Define the minor-mode keymap.
        ,(unless (symbolp keymap)	;nil is also a symbol.


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

bug#10773: set-variable can't change values of user options

[Stefan Monnier]

> There's still the question of using allowing these variables to be
> customized, and that still doesn't work:
>
> (customize-variable 'flyspell-mode-hook)
> -> "NO CUSTOMIZATION DATA; not intended to be customized."
>
> But...  why not?

I don't have a strong opinion either way.

>  So I've now added the patch below to Emacs 28.
>
> diff --git a/lisp/emacs-lisp/easy-mmode.el b/lisp/emacs-lisp/easy-mmode.el
> index e3eb9294ed..fdc1233540 100644
> --- a/lisp/emacs-lisp/easy-mmode.el
> +++ b/lisp/emacs-lisp/easy-mmode.el
> @@ -335,6 +335,9 @@ define-minor-mode
>  No problems result if this variable is not bound.
>  `add-hook' automatically binds it.  (This is true for all hook variables.)"
>                         modefun)))
> +       ;; Allow using using `M-x customize-variable' on the hook.
> +       (put ',hook 'custom-type 'hook)
> +       (put ',hook 'standard-value (list nil))
>  
>         ;; Define the minor-mode keymap.
>         ,(unless (symbolp keymap)	;nil is also a symbol.

Shouldn't we be using `defcustom` instead of hard-coding Custom's
"internal" properties?


        Stefan

bug#10773: set-variable can't change values of user options

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Shouldn't we be using `defcustom` instead of hard-coding Custom's
> "internal" properties?

Yes.  But I was worried that defcustom would be too heavy-handed here.
And it seems unlikely that these properties would change much.

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

bug#10773: set-variable can't change values of user options

[Stefan Monnier]

>> Shouldn't we be using `defcustom` instead of hard-coding Custom's
>> "internal" properties?
> Yes.  But I was worried that defcustom would be too heavy-handed here.

I think if you're worried about the "cost" of `defcustom`, then we may
as well stick to `defvar` (after all, it's not like it'll be very
easy/convenient for the user to use the Custom UI to add a function on
those hooks).


        Stefan

bug#10773: set-variable can't change values of user options

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> Shouldn't we be using `defcustom` instead of hard-coding Custom's
>>> "internal" properties?
>> Yes.  But I was worried that defcustom would be too heavy-handed here.
>
> I think if you're worried about the "cost" of `defcustom`, then we may
> as well stick to `defvar` 

I was mainly worried that defcustom had further other subtle effects
that I'm not aware of (:group? :version? the mandatory doc string?), but
looking at the code in custom-declare-variable, it looks pretty
straightforward...  (Famous last words.)

> (after all, it's not like it'll be very easy/convenient for the user
> to use the Custom UI to add a function on those hooks).

I think there's probably several bug reports about doing something about
that.  :-/

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