bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'

[Drew Adams]

Please (case by case) change examples that use `setq' to set an option
value, so that they instead use one of the Customize functions
(`customize-set-variable', `custom-set-variables', etc.).

Using `setq' in an init file to set option values is error prone.  It is
not a good habit to develop, because (1) there are some options whose
`defcustom's use `:set', `:initialize' etc. (and `setq' ignores such),
and (2) `setq' does no type checking.

There are currently no occurrences of `customize-set-variable' or
`custom-set-variables' in the Emacs manual.  There are lots of uses of
`setq' in the Emacs manual.  Yet many (most?) of them are for setting
user-option values.  No distinction is made between setting options and
other variables (`defvar's).

Sure, many `defcustom's have a simple `:type' and no `:set' or
`:initialize' etc., so in practice it often does not really matter
whether you use `setq' or one of the Customize functions to set the
value.  Still, the manual should set a good example, and it is better to
not get in the habit of assuming that `setq' is sufficient to properly
set a user option.  I've see questions about this from users many times.
(Most often it is a problem with type and not with setter or initializer
functions.)  It is often not obvious (how should it be) to users what is
wrong when they get into trouble by using `setq'.

In GNU Emacs 25.0.50.1 (i686-pc-mingw32)
 of 2015-10-06
Bzr revision: a4a98a1b2568793ead43e824ecf227768759df12
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --prefix=/c/Devel/emacs/snapshot/trunk
 --enable-checking=yes,glyphs 'CFLAGS=-O0 -g3'
 LDFLAGS=-Lc:/Devel/emacs/lib 'CPPFLAGS=-DGC_MCHECK=1
 -Ic:/Devel/emacs/include''

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'

[Drew Adams]

The same is apparently true for the manual "An Introduction
to Programming in Emacs Lisp" (which is a fine intro to learning
Elisp).

What's more, it even states:

 "The 'custom-set-variables' function works somewhat differently than a
 'setq'.  While I have never learned the differences, I modify the
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 'custom-set-variables' expressions in my '.emacs' file by hand: I make
 the changes in what appears to me to be a reasonable manner and have not
 had any problems.  Others prefer to use the Customization command and
 let Emacs do the work for them."

It might have been OK to say that when it was first written,
but by now the manual should understand the differences, and
even point them out.  Especially in that section, which is
about `defcustom'.  The node says:

"Although you can use 'defvar' or 'setq' for variables that
users set, the 'defcustom' macro is designed for the job."

Part of its design for that job is handling `:type', `:set'
`:initialize', et.c, precisely what makes options different
from ordinary variables and thus `custom-set-variables'
different from `setq'.

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'

[Lars Ingebrigtsen]

Drew Adams <drew.adams@oracle.com> writes:

> Please (case by case) change examples that use `setq' to set an option
> value, so that they instead use one of the Customize functions
> (`customize-set-variable', `custom-set-variables', etc.).

No, using setq for user options is fine.  Closing.

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

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'

[Drew Adams]

[Sending again, after unarchiving...]

> >> Please (case by case) change examples that use `setq' to set an option
> >> value, so that they instead use one of the Customize functions
> >> (`customize-set-variable', `custom-set-variables', etc.).
> >
> > No, using setq for user options is fine.  Closing.
> 
> The Emacs manual should mention that some user options must be set
> in a different way as explained in the Emacs Lisp Reference Manual.

Yes, indeed.  (But the bug was closed.)

`setq' for user options is _not_, in general, fine.
It is fine for many - even most - user options.
But there is definitely something to teach users
about this, IMHO.  And it doesn't take much to tell.

Thanks for chiming in here.  I expect that in another
decade or two, after someone else files a bug about
this perhaps, Emacs will eventually provide what's
missing.
 
> @Drew: You should review your answer on Emacs Stack
> Exchange because it is misleading.

Thanks for letting me know, but that's too vague.
It's not clear to me what you think is misleading.

Maybe add a comment there, or here, or send me an
email?  You've apparently reviewed it and found
something misleading.  What did you find?

https://emacs.stackexchange.com/a/17389/105

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'

[Drew Adams]

> > @Drew: You should review your answer on Emacs Stack
> > Exchange because it is misleading.
> 
> Thanks for letting me know, but that's too vague.
> It's not clear to me what you think is misleading.
> 
> Maybe add a comment there, or here, or send me an
> email?  You've apparently reviewed it and found
> something misleading.  What did you find?

Dunno if this is what you had in mind, but I just
added mention of minor-mode variables - use the
minor-mode function to set them.  That's a special
case, and yes, in general using a Customize function
to set such an option directly is not TRT.

(The other thing mentioned in (elisp) `Init Syntax'
is buffer-local variables, which doesn't apply to
user options.)
___

IMHO, not only the Emacs manual but the Init File
sections of the Elisp manual as well should be
updated to _generally guide against_ using `setq'
for user options.

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'

[Drew Adams]

> Dunno if this is what you had in mind, but I just
> added mention of minor-mode variables - use the
> minor-mode function to set them.  That's a special
> case, and yes, in general using a Customize function
> to set such an option directly is not TRT.

I also removed mention of using `customize-set-value'
in an init file - yes, that was misleading.  Let me
if I missed something.  Thx.

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'

[Drew Adams]

> I also removed mention of using `customize-set-value'
> in an init file - yes, that was misleading.  Let me
> if I missed something.  Thx.
 ^
know

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

There are a few customization options that are not really variables
and need to be set with `customize-set-variable'.

However, the rest of them are variables and normally set with `setq'
and friends.

Telling users to set them all with `customize-set-variable' will be a
significant hassle.  What's more, users will disregard this instruction.
`setq' is easy to remember and we all know it.

Having a recommendation in the manual which users will generally
reject is in itself a problem.

I think we should address this some other way.  Here are some ideas.

* indicate the few customization options
   which are not really variables.
* tell people how to check whether
   a customization options is really a variable.
* warn about setq for a customization option that isn't really a variable.


-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Arthur Miller]

Richard Stallman <rms@gnu.org> writes:

> [[[ To any NSA and FBI agents reading my email: please consider    ]]]
> [[[ whether defending the US Constitution against all enemies,     ]]]
> [[[ foreign or domestic, requires you to follow Snowden's example. ]]]
>
> There are a few customization options that are not really variables
> and need to be set with `customize-set-variable'.
>
> However, the rest of them are variables and normally set with `setq'
> and friends.
>
> Telling users to set them all with `customize-set-variable' will be a
> significant hassle.  What's more, users will disregard this instruction.
> `setq' is easy to remember and we all know it.
>
> Having a recommendation in the manual which users will generally
> reject is in itself a problem.
>
> I think we should address this some other way.  Here are some ideas.
>
> * indicate the few customization options
>    which are not really variables.
> * tell people how to check whether
>    a customization options is really a variable.
> * warn about setq for a customization option that isn't really a variable.

Not many people seem to prefer customize to simply setq-ing in their init file,
so I think that what you propose is a much better alternative.

bug#21695: [External] : Re: bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Drew Adams]

> Not many people seem to prefer customize to simply setq-ing in their init
> file, so I think that what you propose is a much better alternative.

Not many people have a clue about not using
`setq' with user options.  Not many people
have a clue about the functions designed to
set user options.

The fact that many people don't use the
Customize UI is irrelevant here, IMO.  This
is about what Elisp to guide users toward
using, to set user options.

bug#21695: [External] : Re: bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Drew Adams]

> There are a few customization options that are not really variables
> and need to be set with `customize-set-variable'.

Please specify what you mean by not really being
variables, and give examples (which options?).

Are you maybe just defining not-really-variable-p
as being an option that has a :set or :initialize?

To my mind (unless I see a persuasive argument),
user options are variables.  They're variables
defined with defcustom; variables whose values
typically have a :type restriction; variables
that are really designed to be set using
Customize.

If I had to come up with a class of options that
are kinda something else, I'd say it's the
minor-mode options (those that correspond to a
minor-mode command).  Those we already (and
rightfully) tell users not to use directly, i.e.,
as variables (`setq').  We tell users to set them
indirectly, by using the mode function instead.

(We do tell them that some mode vars _can_ be set
using `setq' or Customize, but we give them the
general guideline to use the minor-mode function
instead.)

Sure, an option with a :set or :initialize is not
a _simple_ variable, if simple means that you can
just set it with `setq'.  But with that we've
just turned in a circle.

> However, the rest of them are variables and
> normally set with `setq' and friends.

Circular, I think.  I'm guessing you're really
just defining options that can, without problem,
be set with `setq' to be "variables".

> Telling users to set them all with `customize-set-variable' will be a
> significant hassle.  What's more, users will disregard this instruction.
> `setq' is easy to remember and we all know it.
> 
> Having a recommendation in the manual which users will generally
> reject is in itself a problem.

Why would they reject it?  Only because `setq' is
easy to remember?  It's a question of habit.  If
you've never been told to set options differently,
then sure, `setq' is your hammer.

I do agree that a short name would be better.
But that's easily solved by adding an alias for
`customize-set-variable' such as `setq-opt' or
`setqo' or `setopt' .  What's missing is the
guideline to use that instead of `setq'.

Here's my (different) take, FWIW -

1. Yes, for most user options `setq' is sufficient.

   But that makes setting options seem to be
   nothing special - so it's a gotcha when it
   "doesn't work".

2. Yes, if an option's defcustom uses :set then its
   doc string is _supposed_ to tell users how to set
   it properly.

   (Similarly, for :initialize, IMO - même combat.
   But that one's _not_ stated in the manual.)

3. Yes, IF such an option's doc does that (does what
   it's supposed to do), and IF a user reads the doc
   string, and IF s?he understands it, and IF s?he
   does what the doc string says to do (or uses
   `setq' if the doc says nothing particular), THEN
   no problem - the problem is avoided.

OR if a user generally applies the heuristic to not
use `setq' with user options, then no problem.
(Only one "if".  Only one thing to learn, once and
for all - no case-by-case hunt needed.)

In the Emacs manual we use `customize-set-variable'
with options `display-buffer-alist' and
`display-buffer-base-action' (see nodes `Window
Choice' and `Temporary Displays').  Why?  Neither
of those even has a :set or :initialize.

And of course `custom-set-variables' is used in
init files, even if that's done by Customize.
(And there's no attempt there to use `setq' for
those options that are "really variables".)

Users who pay attention thus can see `setq' not
being used with options in some cases - but with
no explanation.

The problem/gotcha of users using `setq' when an
option is defined with :set is real, regardless
of how few options are so defined.

How best to help users avoid this gotcha?  I
think that's the question.

Relying on 3rd-party code that uses :set to also
correctly specify what's advised for doc strings
is problematic.

Relying on users, especially non-lisper or new
users, to hunt for whether an option uses :set,
and if so to DTRT for it, is problematic.

That few options actually do use :set makes it
all the more problematic when some do, because
the behavior is too often unexpected and not
known to be happening.

Better to get users in the good habit of setting
options without `setq'.  And yes, better for the
doc (the Emacs manual, not just the Elisp manual)
to give them a heads-up about the existence of
options that depend on a :set function.

Even better is to get users to use Customize,
and to use a `custom-file'.  But if they don't,
they should at least use a customize function,
which we provide precisely for that purpose: to
DTRT when it comes to setting an option value.

> I think we should address this some other way.
> Here are some ideas.
> 
> * indicate the few customization options
>    which are not really variables.

1. See above - how so "not really variables"?
2. It's not just about the options that Emacs
   delivers predefined.  It's also about the
   many options defined by 3rd-party code.

> * tell people how to check whether
>    a customization options is really a variable.

See above.  A rely-on-users-to-check approach
is problematic, IMO.  Far better to just tell
users, as a general rule, not to use `setq'
with options.

Options are _not_ your ordinary vars (what you
call "really variables", I guess).  That should
be our main (first approximation) message to
users.  Options - not some options - are not
ordinary variables.

The fact that `setq' does in fact "work" for
most user options is a problem, not a solution.
It fosters a bad habit and false idea of what's
involved.

> * warn about setq for a customization option
>   that isn't really a variable.

That's the problem: which option is that?  You
can put this check-carefully burden on users,
but that's not the best approach.

That's like teaching kids to check the chemical
composition of any candy offered them by any
person, instead of teaching them a general
(first approximation) rule of thumb to just not
accept candy from strangers.

IOW, reverse it: if you _don't_ know that some
option needs _no_ special handling then don't
use `setq'.

That's a sane general rule; it'll never get you
in trouble.  And it means you don't have to go
looking into each option definition (or rely on
a DTRT doc string), to see if you can use `setq'
on it.

Easier on users - no gotcha, no careful paying
attention.  If it's an option, don't use `setq'.

Just one opinion.
___

[I also think more defcustoms could benefit
from better (tighter) :type specs, and I suspect
that more options than now might benefit from
:set or :initialize functions.]

bug#21695: [External] : Re: bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

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

The potential issue is to hide the developer's view of how to proceed
or how things work by presenting them with another interface. It would
be much better to bridge the gap between the user and the developer.
Choosing to replace `setq' with `customize-set-variable' would be like
cutting yourself off from an alternative path.

In other words, this would lead to the following situations.

‣ customize-set-variable: “Okay, so I choose the value of this
  parameter.”

‣ setq: “I need to change the value of this variable to change the
  operation.”

Even though `customize-set-variable' is more convenient at first, it is
a bit like discouraging the user from taking another route by blocking
the way: too complicated. With the other approach you can always use
both `setq' and the *Customize* interface (simultaneously and as the
last resort).

bug#21695: [External] : Re: bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Drew Adams]

> The potential issue is to hide the developer's view of how to proceed
> or how things work by presenting them with another interface.

What developer's view would be hidden?

In what I proposed more would be made explicit
to users.  We'd point out that `setq' with
options can be problematic, and how/why so.

> It would be much better to bridge the gap between
> the user and the developer.

What developer are you talking about, here?
What gap?

> Choosing to replace `setq' with `customize-set-variable' would be like
> cutting yourself off from an alternative path.

How so?  What alternative path?  Alternative
to what?

> In other words, this would lead to the following situations.
> 
> ‣ customize-set-variable: “Okay, so I choose the value of this
>   parameter.”
> 
> ‣ setq: “I need to change the value of this variable to change the
>   operation.”

Sorry, but I don't understand those descriptions
or what you intend as the difference.

The difference, IMO, is that the former is for
user options.  The latter has been presented as
for all variables, but for some options it can
be problematic.  The former is never problematic
for an option (or even for a non-option - but
that's another story).

> Even though `customize-set-variable' is more convenient at first, it is
> a bit like discouraging the user from taking another route by blocking
> the way: too complicated.

It's been argued by some that it's less
convenient.  But what "way" do you think it
blocks, and how do you think it does so?

> With the other approach you can always use
> both `setq' and the *Customize* interface
> (simultaneously and as the last resort).

Dunno what the "other approach" is.  But if
by that you mean use `setq' with options,
then I guess you're saying that users can
use `setq', and if they run into the gotcha
(which is seldom, admittedly) they can always
use the Customize UI to take care of that.

Sure, they can.  Once they figure out what's
happened.  Once they've fallen into the hole
in the black of night, and they figure out
that they're in a hole, and they grope and
find a rope to climb out with, they'll be
fine.

And hopefully by that experience they'll
have learned not to use `setq' with (at least
some) user options.  And maybe from then on
they'll hunt down each option's defcustom,
to see whether it uses :set or :initialize,
and _only_ if so will they use Customize.

IOW, a lot of trouble (falling into the pit,
learning how to get out) and bother (from
then on, checking each option to see if
`setq' can safely be used with it, of if
not bothering to check, then risking the
pit again).

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'

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

In my opinion, the issue is that the language (ELisp) should always be
able to express things, in a general way. But here, the risk is to not
be able to understand how things work without digging into the
“Customize” interface or to be overwhelmed by a ton of code. At first
sight it is easier but the path is already paved. The alternative I was
talking about and which is recommended in the ELisp manual: each user
option should indicate how to proceed (without using too many tricks).
Otherwise, I don't see much point in learning Emacs Lisp anymore (you
might as well refer to the Customize interface entirely, a kind of
“black box”). In short, what is proposed is in my opinion
counterproductive.

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Howard Melman]

Richard Stallman <rms@gnu.org> writes:

> There are a few customization options that are not really variables
> and need to be set with `customize-set-variable'.

> * indicate the few customization options
>    which are not really variables.

Searching emacs 27.2 elisp source I find about 400 hits for :set.

Searching my installed packages I find about 30 more.

That seems like more than a few.

-- 

Howard

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > Searching emacs 27.2 elisp source I find about 400 hits for :set.

That is more than I expected.  However, there around 8000 defcustoms
in master as of May 11, and only 462 instances of :set.

Telling people to use customize-set-variable for all 8000 of them
feels like the tail wagging the dog.

I have a feeling that most of those 462 with :set actually require
that people use customize-set-variable to set them in the init file.
I suspect that, for most of them, :set is meant to handle the case
where you change the setting once the feature is already in use.

I think that if we eliminate these, we will get a much smaller
number of exceptions, and we could find a nicer way to handle them.

But even with 462 exceptions, it could be easy enough to warn about
setting one of those in .emacs with setq.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Date: Wed, 01 Sep 2021 23:42:04 -0400
> Cc: 21695@debbugs.gnu.org
> 
>   > Searching emacs 27.2 elisp source I find about 400 hits for :set.
> 
> That is more than I expected.  However, there around 8000 defcustoms
> in master as of May 11, and only 462 instances of :set.
> 
> Telling people to use customize-set-variable for all 8000 of them
> feels like the tail wagging the dog.

Yes.  But I don't see why the numbers matter here.  An option which
cannot be usefully change via setq mentions that in its doc string (or
at least it should; if it doesn't, that's a documentation bug), so all
we need to say in the manual is that such options exist, and they
announce the need to use customize-set-variable in their doc string by
such-and-such text.  Then the users will have enough information to
figure out which variable needs what method.

> I have a feeling that most of those 462 with :set actually require
> that people use customize-set-variable to set them in the init file.
> I suspect that, for most of them, :set is meant to handle the case
> where you change the setting once the feature is already in use.

That's an orthogonal issue, I think.  The issue at hand is how to
prevent users from mistakenly using setq where doing that is
insufficient.  We could independently see to it that the number of
options that actually need this is kept at a minimum.

bug#21695: [External] : bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Drew Adams]

> > Telling people to use customize-set-variable for all 8000 of them
> > feels like the tail wagging the dog.
> 
> Yes.  But I don't see why the numbers matter here.

I agree.

> An option which cannot be usefully change via setq
> mentions that in its doc string (or at least it
> should; if it doesn't, that's a documentation bug),

Yes, but what about 3rd-party code that doesn't
bother saying that in doc strings?  Sure, it's
wrong; but does that recommendation solve the
problem?

> so all we need to say in the manual is that such
> options exist, and they announce the need to use
> customize-set-variable in their doc string by
> such-and-such text.  Then the users will have
> enough information to figure out which variable
> needs what method.

See above, about 3rd-party code.

And that approach requires users to be on the
lookout for this.

That's a bit like not having stop signs and
just telling people to always look both ways
before going through an intersection.  Sure,
they've been warned.  But they then need to
check every intersection, even if there are
few cars on the crossroads.  And it's little
comfort after an incident to be say "Told
you to watch out."

> > I have a feeling that most of those 462 with :set actually require
> > that people use customize-set-variable to set them in the init file.
> > I suspect that, for most of them, :set is meant to handle the case
> > where you change the setting once the feature is already in use.
> 
> That's an orthogonal issue, I think.  The issue at hand is how to
> prevent users from mistakenly using setq where doing that is
> insufficient.  We could independently see to it that the number of
> options that actually need this is kept at a minimum.

Agreed.

bug#21695: [External] : bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Eli Zaretskii]

> From: Drew Adams <drew.adams@oracle.com>
> CC: "21695@debbugs.gnu.org" <21695@debbugs.gnu.org>,
>         "hmelman@gmail.com"
> 	<hmelman@gmail.com>
> Date: Thu, 2 Sep 2021 17:08:01 +0000
> 
> > An option which cannot be usefully change via setq
> > mentions that in its doc string (or at least it
> > should; if it doesn't, that's a documentation bug),
> 
> Yes, but what about 3rd-party code that doesn't
> bother saying that in doc strings?  Sure, it's
> wrong; but does that recommendation solve the
> problem?

Our manual documents Emacs, not every piece of Lisp out there.  If
customizable options in 3rd-party packages need different
instructions, the developers of those packages should document them.

bug#21695: [External] : bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > Our manual documents Emacs, not every piece of Lisp out there.  If
  > customizable options in 3rd-party packages need different
  > instructions, the developers of those packages should document them.

Well said.

We can't take responsibility for everything in the software world that
relates to our work.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > > Telling people to use customize-set-variable for all 8000 of them
  > > feels like the tail wagging the dog.

  > Yes.  But I don't see why the numbers matter here.  An option which
  > cannot be usefully change via setq mentions that in its doc string (or
  > at least it should; if it doesn't, that's a documentation bug), so all
  > we need to say in the manual is that such options exist, and they
  > announce the need to use customize-set-variable in their doc string by
  > such-and-such text.  Then the users will have enough information to
  > figure out which variable needs what method.

I agree, that is adequate.  So I think we agree on this issue.

However, it may be easy to warn if an init file sets one of these
variables with setq.  If that's easy, I think it would be good to do.
It would help users detect and correct these mistakes.

Maybe we could arrange to make it work right to set even those
variables with setq in init files.

Here's a way: after the init files finish, look at the value of the
variable, and if it does not equal the default, and this value was not
properly installed with the :set method, invoke that variable's :set
method.

WDYT?


-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Cc: hmelman@gmail.com, 21695@debbugs.gnu.org
> Date: Sat, 04 Sep 2021 23:43:05 -0400
> 
> Here's a way: after the init files finish, look at the value of the
> variable, and if it does not equal the default, and this value was not
> properly installed with the :set method, invoke that variable's :set
> method.

How would we know which variables to look at?

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > > Here's a way: after the init files finish, look at the value of the
  > > variable, and if it does not equal the default, and this value was not
  > > properly installed with the :set method, invoke that variable's :set
  > > method.

  > How would we know which variables to look at?

We would determine which variables could actually have a problem.

1. Get the list of variables that use :set.

2. For each of those variables, see if it is used in any source file
other than the one that contains the defcustom.  A script can do this.
Make a list of only those variables.

3. Now we have a much shorter list.  We could use the whole of that list.

4. Or we could check some of these variables by hand and see whether
we can prove some of them have no real problem.  That could make the
list shorter.

5. After the init files, we add code to check each of those variables.
If the current value != the default, set the variable again to the
same value using customize-set-variable.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Cc: hmelman@gmail.com, 21695@debbugs.gnu.org
> Date: Tue, 07 Sep 2021 23:23:49 -0400
> 
>   > How would we know which variables to look at?
> 
> We would determine which variables could actually have a problem.
> 
> 1. Get the list of variables that use :set.
> 
> 2. For each of those variables, see if it is used in any source file
> other than the one that contains the defcustom.  A script can do this.

"Any source file" should include all the Lisp files installed on the
user's machine, right?  So such a script would need to be run every
startup (because the installed files can change)?

Perhaps it would be better to walk the obarray instead?

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > > 1. Get the list of variables that use :set.
  > > 
  > > 2. For each of those variables, see if it is used in any source file
  > > other than the one that contains the defcustom.  A script can do this.

  > "Any source file" should include all the Lisp files installed on the
  > user's machine, right?

I don't think so.  There is no need.

To make language clearer, let's say the variable is foofoo, its defcustom
is in foofoo.el, and all its other uses in Emacs are in foofoo.el 
after the defcustom.

That option doesn't have any problems, I think.

Setting it with (setq foofoo ...) is something that should work
in .emacs.  But we don't say it will work in Lisp code in general.
To do that, the user has to look up the right way to set it and do it that way.

So I think what I proposed is correct.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Cc: 21695@debbugs.gnu.org, hmelman@gmail.com
> Date: Wed, 08 Sep 2021 23:11:33 -0400
> 
>   > "Any source file" should include all the Lisp files installed on the
>   > user's machine, right?
> 
> I don't think so.  There is no need.
> 
> To make language clearer, let's say the variable is foofoo, its defcustom
> is in foofoo.el, and all its other uses in Emacs are in foofoo.el 
> after the defcustom.
> 
> That option doesn't have any problems, I think.

I don't see the significance of being defined and used in the same
file, for this matter.  A defcustom could need to use the :set
attribute for such variables, if just changing the value doesn't
produce the expected effect.  For example, some other foofoo.el
variables could depend on the value of foofoo, so any change in the
value of the latter needs to recalculate the values of its
dependencies.

Now suppose .emacs uses (setq foofoo ...), and foofoo.el is from some
third-party package, or even one of user's own init files.  How will
we be able to account for that by using a precompiled list of
variables produced from just the bundled Emacs Lisp files?

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Gregory Heytings]

>
> We would determine which variables could actually have a problem.
>
> 1. Get the list of variables that use :set.
>
> 2. For each of those variables, see if it is used in any source file 
> other than the one that contains the defcustom.  A script can do this. 
> Make a list of only those variables.
>
> 3. Now we have a much shorter list.  We could use the whole of that 
> list.
>
> 4. Or we could check some of these variables by hand and see whether we 
> can prove some of them have no real problem.  That could make the list 
> shorter.
>
> 5. After the init files, we add code to check each of those variables. 
> If the current value != the default, set the variable again to the same 
> value using customize-set-variable.
>

Instead of building such a list, is this not something that could be done 
inside Fsetq, by checking whether Fget (sym, Qcustom_set) is non-nil, and 
if so, issue an error/warning?

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Gregory Heytings]

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


>
> Instead of building such a list, is this not something that could be 
> done inside Fsetq, by checking whether Fget (sym, Qcustom_set) is 
> non-nil, and if so, issue an error/warning?
>

Patch attached.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=Warn-when-defcustom-is-wrongly-set.patch, Size: 1066 bytes --]

From 50d1e07ab098c3c98259f435167cb1cb4bfeb3b5 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Thu, 9 Sep 2021 11:40:50 +0000
Subject: [PATCH] Warn when defcustom is wrongly set.

* src/eval.c (Fsetq): Display warning when a custom variable with a :set
property is set with setq.
---
 src/eval.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/eval.c b/src/eval.c
index 48104bd0f4..b72577e791 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -525,8 +525,16 @@ DEFUN ("setq", Fsetq, Ssetq, 0, UNEVALLED, 0,
 	   : Qnil);
       if (!NILP (lex_binding))
 	XSETCDR (lex_binding, val); /* SYM is lexically bound.  */
-      else
+      else {
+	if (!NILP (Fget (sym, intern ("custom-set"))))
+	  call2 (intern ("display-warning"),
+		 intern ("initialization"),
+		 CALLN (Fformat,
+			build_string
+			("`%s' should be set with `customize-set-variable', not `setq'"),
+			sym));
 	Fset (sym, val);	/* SYM is dynamically bound.  */
+      }
     }
 
   return val;
-- 
2.33.0

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Lars Ingebrigtsen]

Gregory Heytings <gregory@heytings.org> writes:

> * src/eval.c (Fsetq): Display warning when a custom variable with a :set
> property is set with setq.

It's usually fine to set user options that have a :set form with setq --
if you do it before the .el file it belongs in is loaded.  Finding a
variable it's problematic to use setq on in .emacs took some digging
(when I added the example in the manual).

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

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Gregory Heytings]

>> * src/eval.c (Fsetq): Display warning when a custom variable with a 
>> :set property is set with setq.
>
> It's usually fine to set user options that have a :set form with setq -- 
> if you do it before the .el file it belongs in is loaded.  Finding a 
> variable it's problematic to use setq on in .emacs took some digging 
> (when I added the example in the manual).
>

Hmm...  Unless I'm missing something, the case you mention (setq'ing a 
variable before the .el file is loaded) will not issue a warning.  The 
example I used is (setq gdb-many-windows t).

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Lars Ingebrigtsen]

Gregory Heytings <gregory@heytings.org> writes:

> Hmm...  Unless I'm missing something, the case you mention (setq'ing a
> variable before the .el file is loaded) will not issue a warning.  The
> example I used is (setq gdb-many-windows t).

That's not an autoloaded variable (well, minor mode), so Emacs doesn't
know that it has a :set when .emacs is loaded.

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

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Gregory Heytings]

>>> It's usually fine to set user options that have a :set form with setq 
>>> -- if you do it before the .el file it belongs in is loaded.  Finding 
>>> a variable it's problematic to use setq on in .emacs took some digging 
>>> (when I added the example in the manual).
>>
>> Hmm...  Unless I'm missing something, the case you mention (setq'ing a 
>> variable before the .el file is loaded) will not issue a warning.  The 
>> example I used is (setq gdb-many-windows t).
>
> That's not an autoloaded variable (well, minor mode), so Emacs doesn't 
> know that it has a :set when .emacs is loaded.
>

Oh, I see.  You mean that setq'ing autoloaded variables, which are loaded 
before their .el files is loaded, can usually be setq'd, disregarding 
their :set property.  But I don't see how one could distinguish between 
those for which it is problematic and those for which is isn't.  ISTM that 
in this case displaying a warning even when it's not really problematic is 
better than not displaying warnings for those variable for which it is 
problematic?

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Lars Ingebrigtsen]

Gregory Heytings <gregory@heytings.org> writes:

> Oh, I see.  You mean that setq'ing autoloaded variables, which are
> loaded before their .el files is loaded, can usually be setq'd,
> disregarding their :set property.

No, I'm saying that (for most variables) Emacs doesn't know that there's
a :set on the variable when loading .emacs.  And that's fine.

Also for variables where Emacs knows there a :set (which would then
issue the warning), it's also fine, because loading the .el file works
perfectly.  Most of the :set thingies are for when you've already
started the mode/package and then change things "in flight".

> But I don't see how one could distinguish between those for which it
> is problematic and those for which is isn't.  ISTM that in this case
> displaying a warning even when it's not really problematic is better
> than not displaying warnings for those variable for which it is
> problematic?

No, displaying a useless warning (and it will be useless in the vast
majority of the cases) isn't good.

I haven't really been paying attention to this bug report after closing
it -- we're not going to change the manual as requested, and it's fine
to use setq in .emacs.  If we're to do something about this, then
somebody would have to go through all the defcustoms with :set and try
to identify some that should, indeed, not be set with setq and tag them
up.  (There aren't a lot of these.)

Which is why I closed this bug report in the first place.

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

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Gregory Heytings]

>
> If we're to do something about this, then somebody would have to go 
> through all the defcustoms with :set and try to identify some that 
> should, indeed, not be set with setq and tag them up.  (There aren't a 
> lot of these.)
>

You mean, adding something like a :set-with-customize tag to those 
variables?

Do you have an example of a variable with a :set for which it isn't 
problematic to use setq, and an example for which it is?  I tried to find 
the problematic example you mentioned in the manual, but did not find it.

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Lars Ingebrigtsen]

Gregory Heytings <gregory@heytings.org> writes:

> Do you have an example of a variable with a :set for which it isn't
> problematic to use setq, and an example for which it is?

Do you mean after loading/starting the package?  I guess most of them
are problematic after loading, like (at random) vcursor-key-bindings.

Most variable that's autoloaded and has a :set can probably be set
safely with setq (before loading the .el file).

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

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Gregory Heytings]

>> Do you have an example of a variable with a :set for which it isn't 
>> problematic to use setq, and an example for which it is?
>
> Do you mean after loading/starting the package?  I guess most of them 
> are problematic after loading, like (at random) vcursor-key-bindings.
>
> Most variable that's autoloaded and has a :set can probably be set 
> safely with setq (before loading the .el file).
>

Then I still don't get what you mean.  An autoloaded but not yet loaded 
variable doesn't have its custom-set property set, so in that case there 
would be no warning with the proposed patch.  Warnings are displayed only 
for customs variables that are "fully" loaded, and who have a :set 
property.

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Lars Ingebrigtsen]

Gregory Heytings <gregory@heytings.org> writes:

> Then I still don't get what you mean.  An autoloaded but not yet
> loaded variable doesn't have its custom-set property set, so in that
> case there would be no warning with the proposed patch.  Warnings are
> displayed only for customs variables that are "fully" loaded, and who
> have a :set property.

Doesn't ;;;###autoload pull the entire defcustom form into loaddefs.el?

But I see that we autoload basically no defcustoms (five, apparently),
so I guess it's a moot point.

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

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Gregory Heytings]

>> Then I still don't get what you mean.  An autoloaded but not yet loaded 
>> variable doesn't have its custom-set property set, so in that case 
>> there would be no warning with the proposed patch.  Warnings are 
>> displayed only for customs variables that are "fully" loaded, and who 
>> have a :set property.
>
> Doesn't ;;;###autoload pull the entire defcustom form into loaddefs.el?
>
> But I see that we autoload basically no defcustoms (five, apparently), 
> so I guess it's a moot point.
>

One of them is allout-auto-activation.  To see that autoloaded variables 
do not have a custom-set property until they are actually loaded, you can 
use:

(defmacro check (sym)
   `(when (get ,sym 'custom-set)
      (display-warning 'setq (format "`%s' should be set with `customize-set-variable'" ,sym))))
(check 'allout-auto-activation)
(load "allout")
(check 'allout-auto-activation)

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Gregory Heytings]

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


I updated the patch with some documentation.

Is there any chance that it gets applied?

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=Warn-when-custom-variable-is-wrongly-set.patch, Size: 2997 bytes --]

From ea9eeb65d6514c11ae9d0ac5c4fad36213e303f9 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Sun, 12 Sep 2021 08:19:25 +0000
Subject: [PATCH] Warn when custom variable is wrongly set.

* src/eval.c (Fsetq): Display warning when a custom variable with a :set
property is set with setq.  Warnings are displayed only for custom variables
whose files have been loaded and that have a :set property.  No warnings are
displayed for custom variables whose files have merely been preloaded,
for custom variables that do not have a :set property, and for non-custom
variables.  See bug#21695.

* etc/NEWS: Document the warning.

* doc/emacs/custom.texi: Mention the warning.
---
 doc/emacs/custom.texi | 3 ++-
 etc/NEWS              | 6 ++++++
 src/eval.c            | 9 ++++++++-
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/doc/emacs/custom.texi b/doc/emacs/custom.texi
index 9220a2078f..1b7e7d9361 100644
--- a/doc/emacs/custom.texi
+++ b/doc/emacs/custom.texi
@@ -2378,7 +2378,8 @@ do that; to enable the mode in your init file, call the minor mode
 command.  Finally, a few customizable user options are initialized in
 complex ways, and these have to be set either via the customize
 interface (@pxref{Customization}) or by using
-@code{customize-set-variable} (@pxref{Examining}).
+@code{customize-set-variable} (@pxref{Examining}).  If such options
+are inadvertently set with @code{setq}, a warning is displayed.
 
   The second argument to @code{setq} is an expression for the new
 value of the variable.  This can be a constant, a variable, or a
diff --git a/etc/NEWS b/etc/NEWS
index ca269aabaa..dfdd925ec5 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -3401,6 +3401,12 @@ truncating precision field, such as "%.2a".
 Such mixes are always signs that the outer lexical binding was an
 error and should have used dynamic binding instead.
 
+--
+** 'setq' displays a warning when 'customize-set-variable' should have been used.
+Some custom variables need to be set with 'customize-set-variable', because
+they were designed to be set through the Customization interface and have a
+:set lambda form which does other things after they have been set.
+
 ---
 ** New variable 'inhibit-mouse-event-check'.
 If bound to non-nil, a command with '(interactive "e")' doesn't signal
diff --git a/src/eval.c b/src/eval.c
index 48104bd0f4..168a7749a0 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -525,8 +525,15 @@ usage: (setq [SYM VAL]...)  */)
 	   : Qnil);
       if (!NILP (lex_binding))
 	XSETCDR (lex_binding, val); /* SYM is lexically bound.  */
-      else
+      else {
+	if (!NILP (Fget (sym, intern ("custom-set"))))
+	  call2 (intern ("display-warning"), intern ("setq"),
+		 CALLN (Fformat,
+			build_string
+			("`%s' should be set with `customize-set-variable'"),
+			sym));
 	Fset (sym, val);	/* SYM is dynamically bound.  */
+      }
     }
 
   return val;
-- 
2.33.0

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Eli Zaretskii]

> Date: Sun, 12 Sep 2021 08:23:01 +0000
> From: Gregory Heytings <gregory@heytings.org>
> Cc: 21695@debbugs.gnu.org, hmelman@gmail.com, Richard Stallman <rms@gnu.org>
> 
> +--
> +** 'setq' displays a warning when 'customize-set-variable' should have been used.
> +Some custom variables need to be set with 'customize-set-variable', because
> +they were designed to be set through the Customization interface and have a
> +:set lambda form which does other things after they have been set.

I thought the conclusion was that most variables with :set can be
safely set by setq, isn't that so?  If so, these warnings will mostly
annoy.

> --- a/src/eval.c
> +++ b/src/eval.c
> @@ -525,8 +525,15 @@ usage: (setq [SYM VAL]...)  */)
>  	   : Qnil);
>        if (!NILP (lex_binding))
>  	XSETCDR (lex_binding, val); /* SYM is lexically bound.  */
> -      else
> +      else {
> +	if (!NILP (Fget (sym, intern ("custom-set"))))
> +	  call2 (intern ("display-warning"), intern ("setq"),
> +		 CALLN (Fformat,
> +			build_string
> +			("`%s' should be set with `customize-set-variable'"),
> +			sym));
>  	Fset (sym, val);	/* SYM is dynamically bound.  */
> +      }

What will happen if setq is in the user's init file?  We generally
delay warnings until after the startup in those cases.

Also, warnings.el is not preloaded, so this call could barf in some
valid cases.  OTOH, setq is a primitive written in C, so ther should
be no need to call intern for it.

And finally, do we really want to slow down each setq by calling
intern and Fget?  setq is many times used inside tight loops.  I'm not
sure the resulting run-time penalty is justified.  Did you measure the
effect of this on performance?

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Gregory Heytings]

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


>> +** 'setq' displays a warning when 'customize-set-variable' should have been used.
>> +Some custom variables need to be set with 'customize-set-variable', because
>> +they were designed to be set through the Customization interface and have a
>> +:set lambda form which does other things after they have been set.
>
> I thought the conclusion was that most variables with :set can be safely 
> set by setq, isn't that so?  If so, these warnings will mostly annoy.
>

No, that wasn't the conclusion.  Most variable with :set can safely be set 
by setq *before* the file in which they are declared is loaded.  And no 
warnings are displayed in that case.  But all variables with :set cannot 
be safely be set by setq *after* the file in which they are declared has 
been loaded.  And warnings are displayed in that case.

>
> What will happen if setq is in the user's init file?  We generally delay 
> warnings until after the startup in those cases.
>

With the following .emacs:

(require 'allout)
(setq allout-auto-activation t)

a warning is displayed, but I don't know if this is during of after the 
startup.  At least it is visible when startup has completed.

>
> Also, warnings.el is not preloaded, so this call could barf in some 
> valid cases.
>

That's not what I see:

;;;###autoload
(defun display-warning (type message &optional level buffer-name)

>
> OTOH, setq is a primitive written in C, so ther should be no need to 
> call intern for it.
>

Okay, updated patch attached.

>
> And finally, do we really want to slow down each setq by calling intern 
> and Fget?  setq is many times used inside tight loops.  I'm not sure the 
> resulting run-time penalty is justified.  Did you measure the effect of 
> this on performance?
>

With the updated patch, on my laptop, the execution of setq takes ~48 
nanoseconds instead of ~40 nanoseconds.  Which seems reasonable.

[-- Attachment #2: Type: text/x-diff, Size: 3231 bytes --]

From 723e83b20f077ee36e8784b7101b4e20ace648f9 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Sun, 12 Sep 2021 09:22:13 +0000
Subject: [PATCH] Warn when custom variable is wrongly set.

* src/eval.c (Fsetq): Display warning when a custom variable with a :set
property is set with setq.  Warnings are displayed only for custom variables
whose files have been loaded and that have a :set property.  No warnings are
displayed for custom variables whose files have merely been preloaded,
for custom variables that do not have a :set property, and for non-custom
variables.  See bug#21695.
(syms_of_eval): Three new symbols.

* etc/NEWS: Document the warning.

* doc/emacs/custom.texi: Mention the warning.
---
 doc/emacs/custom.texi |  3 ++-
 etc/NEWS              |  6 ++++++
 src/eval.c            | 12 +++++++++++-
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/doc/emacs/custom.texi b/doc/emacs/custom.texi
index 9220a2078f..1b7e7d9361 100644
--- a/doc/emacs/custom.texi
+++ b/doc/emacs/custom.texi
@@ -2378,7 +2378,8 @@ Init Syntax
 command.  Finally, a few customizable user options are initialized in
 complex ways, and these have to be set either via the customize
 interface (@pxref{Customization}) or by using
-@code{customize-set-variable} (@pxref{Examining}).
+@code{customize-set-variable} (@pxref{Examining}).  If such options
+are inadvertently set with @code{setq}, a warning is displayed.
 
   The second argument to @code{setq} is an expression for the new
 value of the variable.  This can be a constant, a variable, or a
diff --git a/etc/NEWS b/etc/NEWS
index ca269aabaa..dfdd925ec5 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -3401,6 +3401,12 @@ truncating precision field, such as "%.2a".
 Such mixes are always signs that the outer lexical binding was an
 error and should have used dynamic binding instead.
 
+--
+** 'setq' displays a warning when 'customize-set-variable' should have been used.
+Some custom variables need to be set with 'customize-set-variable', because
+they were designed to be set through the Customization interface and have a
+:set lambda form which does other things after they have been set.
+
 ---
 ** New variable 'inhibit-mouse-event-check'.
 If bound to non-nil, a command with '(interactive "e")' doesn't signal
diff --git a/src/eval.c b/src/eval.c
index 48104bd0f4..84500f7c1a 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -525,8 +525,15 @@ DEFUN ("setq", Fsetq, Ssetq, 0, UNEVALLED, 0,
 	   : Qnil);
       if (!NILP (lex_binding))
 	XSETCDR (lex_binding, val); /* SYM is lexically bound.  */
-      else
+      else {
+	if (!NILP (Fget (sym, Qcustom_set)))
+	  call2 (Qdisplay_warning, Qsetq,
+		 CALLN (Fformat,
+			build_string
+			("`%s' should be set with `customize-set-variable'"),
+			sym));
 	Fset (sym, val);	/* SYM is dynamically bound.  */
+      }
     }
 
   return val;
@@ -4556,4 +4563,7 @@ syms_of_eval (void)
   defsubr (&Sbacktrace__locals);
   defsubr (&Sspecial_variable_p);
   defsubr (&Sfunctionp);
+  DEFSYM (Qcustom_set, "custom-set");
+  DEFSYM (Qdisplay_warning, "display-warning");
+  DEFSYM (Qsetq, "setq");
 }
-- 
2.33.0

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Eli Zaretskii]

> Date: Sun, 12 Sep 2021 09:30:21 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: larsi@gnus.org, 21695@debbugs.gnu.org, hmelman@gmail.com, rms@gnu.org
> 
> > What will happen if setq is in the user's init file?  We generally delay 
> > warnings until after the startup in those cases.
> >
> 
> With the following .emacs:
> 
> (require 'allout)
> (setq allout-auto-activation t)
> 
> a warning is displayed, but I don't know if this is during of after the 
> startup.  At least it is visible when startup has completed.

You need a much larger init file with several setq like this.  The
warnings usually fly by you without giving enough chance to read them.

> > Also, warnings.el is not preloaded, so this call could barf in some 
> > valid cases.
> >
> 
> That's not what I see:
> 
> ;;;###autoload
> (defun display-warning (type message &optional level buffer-name)

That's autoloaded, not preloaded; the latter is in loadup.el.

> > And finally, do we really want to slow down each setq by calling intern 
> > and Fget?  setq is many times used inside tight loops.  I'm not sure the 
> > resulting run-time penalty is justified.  Did you measure the effect of 
> > this on performance?
> >
> 
> With the updated patch, on my laptop, the execution of setq takes ~48 
> nanoseconds instead of ~40 nanoseconds.  Which seems reasonable.

Is this multiplicative, i.e. if you perform it many times, does it
indeed take 20% longer overall?  If so, this is not an acceptable
performance hit, I think, not for such a minor feature.

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Gregory Heytings]

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


>> I don't know if this is during of after the startup.  At least it is 
>> visible when startup has completed.
>
> You need a much larger init file with several setq like this.  The 
> warnings usually fly by you without giving enough chance to read them.
>

That's not what I see, after the startup has completed I see a *Warnings* 
buffer in the lower half of the frame.  See attached screenshot.

>>> Also, warnings.el is not preloaded, so this call could barf in some 
>>> valid cases.
>>
>> That's not what I see:
>>
>> ;;;###autoload
>> (defun display-warning (type message &optional level buffer-name)
>
> That's autoloaded, not preloaded; the latter is in loadup.el.
>

Hmm, then I don't see what you mean.  I did not know that there is a 
difference between "autoload" and "preload".

>
> Is this multiplicative, i.e. if you perform it many times, does it 
> indeed take 20% longer overall?  If so, this is not an acceptable 
> performance hit, I think, not for such a minor feature.
>

The "(get sym 'custom-set)" call adds about ~8 nanoseconds to each call to 
setq.  I don't see how this could be avoided, if the idea is to display a 
warning when setq is used when customize-set-variable should be used 
instead.  Of course every new feature comes at a cost.

Perhaps a new defcustom could be created, e.g. customize-warn-setq, 
defaulting to t, to make it possible to avoid that call to "(get sym 
'custom-set)"?

[-- Attachment #2: Type: image/png, Size: 125520 bytes --]

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Eli Zaretskii]

> Date: Sun, 12 Sep 2021 09:54:51 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: larsi@gnus.org, 21695@debbugs.gnu.org, hmelman@gmail.com, rms@gnu.org
> 
> >>> Also, warnings.el is not preloaded, so this call could barf in some 
> >>> valid cases.
> >>
> >> That's not what I see:
> >>
> >> ;;;###autoload
> >> (defun display-warning (type message &optional level buffer-name)
> >
> > That's autoloaded, not preloaded; the latter is in loadup.el.
> >
> 
> Hmm, then I don't see what you mean.  I did not know that there is a 
> difference between "autoload" and "preload".

The difference is that while Emacs is being built, especially
bootstrapped, calls to Lisp code that isn't preloaded by autoload.el
could fail.  So such calls need to be careful not to call a symbol if
the call could fail, or at least use internal_condition_case*
functions to protect themselves against failure.

> > Is this multiplicative, i.e. if you perform it many times, does it 
> > indeed take 20% longer overall?  If so, this is not an acceptable 
> > performance hit, I think, not for such a minor feature.
> >
> 
> The "(get sym 'custom-set)" call adds about ~8 nanoseconds to each call to 
> setq.  I don't see how this could be avoided, if the idea is to display a 
> warning when setq is used when customize-set-variable should be used 
> instead.  Of course every new feature comes at a cost.
> 
> Perhaps a new defcustom could be created, e.g. customize-warn-setq, 
> defaulting to t, to make it possible to avoid that call to "(get sym 
> 'custom-set)"?

Are we only doing this only for initializations in the init files?
Then perhaps this could be enabled only until the startup is
completed.  Even then, some people will frown on 20% slowdown of the
startup.

Let's see what others think about this aspect.

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Eli Zaretskii]

> Date: Sun, 12 Sep 2021 13:11:52 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: larsi@gnus.org, hmelman@gmail.com, 21695@debbugs.gnu.org, rms@gnu.org
> 
> The difference is that while Emacs is being built, especially
> bootstrapped, calls to Lisp code that isn't preloaded by autoload.el
> could fail.  So such calls need to be careful not to call a symbol if
> the call could fail, or at least use internal_condition_case*
> functions to protect themselves against failure.

Basically, you should use safe_call and its ilk.

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Gregory Heytings]

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


>> The difference is that while Emacs is being built, especially 
>> bootstrapped, calls to Lisp code that isn't preloaded by autoload.el 
>> could fail.  So such calls need to be careful not to call a symbol if 
>> the call could fail, or at least use internal_condition_case* functions 
>> to protect themselves against failure.
>
> Basically, you should use safe_call and its ilk.
>

Thank you for that clarification, done.

>
> Even then, some people will frown on 20% slowdown of the startup.
>

It is not a slowdown of the startup, it is a slowdown of setq, which 
becomes visible for the user when say 100 million setq's are executed.

Anyway, given your remark, I improved the patch.  With this optimized 
version, there is no noticeable difference for variables without a plist 
(~40 nanoseconds for each call to setq, with and without the patch), and a 
small difference for variables with a plist (~40 nanoseconds for each call 
to setq without the patch, ~42 nanoseconds with the patch).

[-- Attachment #2: Type: text/x-diff, Size: 3335 bytes --]

From 1f81ad6c12e4494c3a6434b55923a6e4be3b38fb Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Sun, 12 Sep 2021 21:10:08 +0000
Subject: [PATCH] Warn when custom variable is wrongly set.

* src/eval.c (Fsetq): Display warning when a custom variable with a :set
property is set with setq.  Warnings are displayed only for custom variables
whose files have been loaded and that have a :set property.  No warnings are
displayed for custom variables whose files have merely been preloaded,
for custom variables that do not have a :set property, and for non-custom
variables.  See bug#21695.
(syms_of_eval): Three new symbols.

* etc/NEWS: Document the warning.

* doc/emacs/custom.texi: Mention the warning.
---
 doc/emacs/custom.texi |  3 ++-
 etc/NEWS              |  6 ++++++
 src/eval.c            | 13 ++++++++++++-
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/doc/emacs/custom.texi b/doc/emacs/custom.texi
index 9220a2078f..1b7e7d9361 100644
--- a/doc/emacs/custom.texi
+++ b/doc/emacs/custom.texi
@@ -2378,7 +2378,8 @@ Init Syntax
 command.  Finally, a few customizable user options are initialized in
 complex ways, and these have to be set either via the customize
 interface (@pxref{Customization}) or by using
-@code{customize-set-variable} (@pxref{Examining}).
+@code{customize-set-variable} (@pxref{Examining}).  If such options
+are inadvertently set with @code{setq}, a warning is displayed.
 
   The second argument to @code{setq} is an expression for the new
 value of the variable.  This can be a constant, a variable, or a
diff --git a/etc/NEWS b/etc/NEWS
index ca269aabaa..dfdd925ec5 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -3401,6 +3401,12 @@ truncating precision field, such as "%.2a".
 Such mixes are always signs that the outer lexical binding was an
 error and should have used dynamic binding instead.
 
+--
+** 'setq' displays a warning when 'customize-set-variable' should have been used.
+Some custom variables need to be set with 'customize-set-variable', because
+they were designed to be set through the Customization interface and have a
+:set lambda form which does other things after they have been set.
+
 ---
 ** New variable 'inhibit-mouse-event-check'.
 If bound to non-nil, a command with '(interactive "e")' doesn't signal
diff --git a/src/eval.c b/src/eval.c
index 48104bd0f4..a81001749d 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -525,8 +525,16 @@ DEFUN ("setq", Fsetq, Ssetq, 0, UNEVALLED, 0,
 	   : Qnil);
       if (!NILP (lex_binding))
 	XSETCDR (lex_binding, val); /* SYM is lexically bound.  */
-      else
+      else {
+	Lisp_Object plist = XSYMBOL (sym)->u.s.plist;
+	if (!EQ (plist, Qnil) && !NILP (Fplist_get (plist, Qcustom_set)))
+	  safe_call2 (Qdisplay_warning, Qsetq,
+		      CALLN (Fformat,
+			     build_string
+			     ("`%s' should be set with `customize-set-variable'"),
+			     sym));
 	Fset (sym, val);	/* SYM is dynamically bound.  */
+      }
     }
 
   return val;
@@ -4556,4 +4564,7 @@ syms_of_eval (void)
   defsubr (&Sbacktrace__locals);
   defsubr (&Sspecial_variable_p);
   defsubr (&Sfunctionp);
+  DEFSYM (Qcustom_set, "custom-set");
+  DEFSYM (Qdisplay_warning, "display-warning");
+  DEFSYM (Qsetq, "setq");
 }
-- 
2.33.0

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Gregory Heytings]

>
> Anyway, given your remark, I improved the patch.  With this optimized 
> version, there is no noticeable difference for variables without a plist 
> (~40 nanoseconds for each call to setq, with and without the patch), and 
> a small difference for variables with a plist (~40 nanoseconds for each 
> call to setq without the patch, ~42 nanoseconds with the patch).
>

Correction: it adds ~2 nanoseconds for each element in the plist.

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Lars Ingebrigtsen]

Gregory Heytings <gregory@heytings.org> writes:

> +	if (!EQ (plist, Qnil) && !NILP (Fplist_get (plist, Qcustom_set)))
> +	  safe_call2 (Qdisplay_warning, Qsetq,
> +		      CALLN (Fformat,
> +			     build_string
> +			     ("`%s' should be set with `customize-set-variable'"),
> +			     sym));

I haven't tried the patch, but won't this be triggered by the code
that's handling the variables?  For instance, `latin1-display' has a
:set, but the file itself does `(setq latin1-display t)' etc.  (This is
a very common pattern.)

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

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Gregory Heytings]

>> +	if (!EQ (plist, Qnil) && !NILP (Fplist_get (plist, Qcustom_set)))
>> +	  safe_call2 (Qdisplay_warning, Qsetq,
>> +		      CALLN (Fformat,
>> +			     build_string
>> +			     ("`%s' should be set with `customize-set-variable'"),
>> +			     sym));
>
> I haven't tried the patch, but won't this be triggered by the code 
> that's handling the variables?  For instance, `latin1-display' has a 
> :set, but the file itself does `(setq latin1-display t)' etc.  (This is 
> a very common pattern.)
>

No, (latin1-display 'latin-2) does not raise a warning with the patch, 
because latin1-disp.el is bytecompiled, and Fsetq is not used when 
bytecompiled code is executed (bytecompiled code only uses Fset).

That being said, I would say that using (setq latin1-display t) when 
latin1-display has a :set is always a bug, in the init file, 
interactively, or in built-in code (bytecompiled or not), and that (set 
'latin1-display t) should be used instead.  WDYT?

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Lars Ingebrigtsen]

Gregory Heytings <gregory@heytings.org> writes:

> No, (latin1-display 'latin-2) does not raise a warning with the patch,
> because latin1-disp.el is bytecompiled, and Fsetq is not used when
> bytecompiled code is executed (bytecompiled code only uses Fset).

Right.  But it's perfectly valid to run uncompiled code, and these
warnings would be false positives in that case.

> That being said, I would say that using (setq latin1-display t) when
> latin1-display has a :set is always a bug, in the init file,
> interactively, or in built-in code (bytecompiled or not), and that
> (set 'latin1-display t) should be used instead.  WDYT?

I have not studied the code -- I just picked a variable at random.  The
point is that code in a package will commonly use setq on the user
options to implement the code, and this is how it should be.

(Also note that your patch will issue a bunch of warnings if the user
does an `eval-buffer' after loading .emacs into a buffer and then
evaling it (in a running Emacs).)

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

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Gregory Heytings]

>> That being said, I would say that using (setq latin1-display t) when 
>> latin1-display has a :set is always a bug, in the init file, 
>> interactively, or in built-in code (bytecompiled or not), and that (set 
>> 'latin1-display t) should be used instead.  WDYT?
>
> I have not studied the code -- I just picked a variable at random.  The 
> point is that code in a package will commonly use setq on the user 
> options to implement the code, and this is how it should be.
>

My point is that code that sets a user option that has a :set should set 
it with (set 'foo val) instead of (setq foo val).

I just checked: on the current trunk, there are 331 defcustoms with a 
:set.  Out of these, only 49 are set with setq, 33 only at one place, and 
16 more than once.  That would require changing 85 setq's into set's.

>
> (Also note that your patch will issue a bunch of warnings if the user 
> does an `eval-buffer' after loading .emacs into a buffer and then 
> evaling it (in a running Emacs).)
>

There are not that many custom variables that will behave differently when 
the init file is first loaded and when it is eval-buffer'd.  AFAICS this 
will only happen for variables whose files have been loaded in the 
meantime after they have been setq'd.

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Eli Zaretskii]

> Date: Mon, 13 Sep 2021 13:00:15 +0000
> From: Gregory Heytings <gregory@heytings.org>
> Cc: 21695@debbugs.gnu.org, hmelman@gmail.com, rms@gnu.org
> 
> I just checked: on the current trunk, there are 331 defcustoms with a 
> :set.  Out of these, only 49 are set with setq, 33 only at one place, and 
> 16 more than once.  That would require changing 85 setq's into set's.

Please don't.  This is a tail wagging the dog.  We will not disallow
using 'setq' in a setter of a defcustom, it would be ridiculous if we
did.  I'd rather leave this minor issue unsolved than taking such
drastic actions.

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Eli Zaretskii]

> Date: Mon, 13 Sep 2021 09:11:11 +0000
> From: Gregory Heytings <gregory@heytings.org>
> Cc: 21695@debbugs.gnu.org, hmelman@gmail.com, rms@gnu.org
> 
> 
> >> +	if (!EQ (plist, Qnil) && !NILP (Fplist_get (plist, Qcustom_set)))
> >> +	  safe_call2 (Qdisplay_warning, Qsetq,
> >> +		      CALLN (Fformat,
> >> +			     build_string
> >> +			     ("`%s' should be set with `customize-set-variable'"),
> >> +			     sym));
> >
> > I haven't tried the patch, but won't this be triggered by the code 
> > that's handling the variables?  For instance, `latin1-display' has a 
> > :set, but the file itself does `(setq latin1-display t)' etc.  (This is 
> > a very common pattern.)
> >
> 
> No, (latin1-display 'latin-2) does not raise a warning with the patch, 
> because latin1-disp.el is bytecompiled, and Fsetq is not used when 
> bytecompiled code is executed (bytecompiled code only uses Fset).

So the warning will not be shown for byte-compiled code?  I think it
might be not a good idea, as many users byte-compile their init files.

> That being said, I would say that using (setq latin1-display t) when 
> latin1-display has a :set is always a bug, in the init file, 
> interactively, or in built-in code (bytecompiled or not), and that (set 
> 'latin1-display t) should be used instead.  WDYT?

I think forcing people to use 'set' instead is too much of a
punishment.

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Eli Zaretskii]

> Date: Sun, 12 Sep 2021 21:26:18 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: larsi@gnus.org, hmelman@gmail.com, 21695@debbugs.gnu.org, rms@gnu.org
> 
> > Even then, some people will frown on 20% slowdown of the startup.
> 
> It is not a slowdown of the startup, it is a slowdown of setq

Init files use a lot of setq's, at least mine do.

> which becomes visible for the user when say 100 million setq's are
> executed.

Tell that to those who complain when startup takes more than 100 msec.

> Anyway, given your remark, I improved the patch.  With this optimized 
> version, there is no noticeable difference for variables without a plist 
> (~40 nanoseconds for each call to setq, with and without the patch), and a 
> small difference for variables with a plist (~40 nanoseconds for each call 
> to setq without the patch, ~42 nanoseconds with the patch).

Thanks, this is much better.  But I think this still needs some
discussion (and one is already going on), so let's wait until it
reaches some conclusions.

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[martin rudalics]

 > Let's see what others think about this aspect.

A naive question probably: Why do we want to do "that" at run time?
Can't we make the byte compiler detect such occurrences?

martin

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Gregory Heytings]

>> Let's see what others think about this aspect.
>
> A naive question probably: Why do we want to do "that" at run time? 
> Can't we make the byte compiler detect such occurrences?
>

Because init files are typically not bytecompiled, and because that would 
not catch interactive uses with e.g. M-:.

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[martin rudalics]

 > Because init files are typically not bytecompiled, and because that
 > would not catch interactive uses with e.g. M-:.

Both would be most unwelcome here.  I often use `setq' in short "init"
files for debugging purposes and sometimes use `setq' via M-: during
debugging.  In neither of these cases I wanted to be disturbed by any
warnings.  I would, however, welcome being warned when a user option is
overridden by a package I load, either by a local binding or `setq'.  At
compile time.

martin

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Gregory Heytings]

>> Because init files are typically not bytecompiled, and because that 
>> would not catch interactive uses with e.g. M-:.
>
> Both would be most unwelcome here.  I often use `setq' in short "init" 
> files for debugging purposes and sometimes use `setq' via M-: during 
> debugging.  In neither of these cases I wanted to be disturbed by any 
> warnings.  I would, however, welcome being warned when a user option is 
> overridden by a package I load, either by a local binding or `setq'. 
> At compile time.
>

The point is not to display unnecessary warnings.  The point is only to 
display warnings for custom variables that should not be set with setq 
because they have a :set form, or IOW, to display warnings for custom 
variables that should be set with customize-set-variables.

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[martin rudalics]

 > The point is not to display unnecessary warnings.  The point is only
 > to display warnings for custom variables that should not be set with
 > setq because they have a :set form, or IOW, to display warnings for
 > custom variables that should be set with customize-set-variables.

I checked most of our uses of :set and agree that they usually behave
well-mannered so using `setq' instead is practically always a bug
indeed.  But if the warnings are not displayed for byte-compiled code I
miss who their major addressee would be.

martin

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Gregory Heytings]

>> The point is not to display unnecessary warnings.  The point is only to 
>> display warnings for custom variables that should not be set with setq 
>> because they have a :set form, or IOW, to display warnings for custom 
>> variables that should be set with customize-set-variables.
>
> I checked most of our uses of :set and agree that they usually behave 
> well-mannered so using `setq' instead is practically always a bug 
> indeed.
>

Thank you.  So this has now been (at least) double-checked.

>
> But if the warnings are not displayed for byte-compiled code I miss who 
> their major addressee would be.
>

Users who use setq in their init files where customize-set-variable should 
be used, and users who use setq interactively (with e.g. M-:) when they 
should have used customize-set-variable instead.  In particular newcomers 
who might be puzzled to see that (setq foo t) does not work as they think 
it should.

Anyway, I don't feel a lot of enthusiasm for that feature, so I think it's 
probably better to let the patch rest in peace.

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

   > > The point is not to display unnecessary warnings.  The point is only
   > > to display warnings for custom variables that should not be set with
   > > setq because they have a :set form, or IOW, to display warnings for
   > > custom variables that should be set with customize-set-variables.

  > I checked most of our uses of :set and agree that they usually behave
  > well-mannered so using `setq' instead is practically always a bug
  > indeed.

I am puzzled by that statement.  First, it seems to say that the
definitions of the variables are careful, so that they DTRT if the
user sets one with setq in an init file.

Then it seems to say that setting one of these variable with setq (in
an init file?) is almost surely a bug.

Those two statements are almost opposites.

Did I misunderstand?


-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[martin rudalics]

 >     > > The point is not to display unnecessary warnings.  The point is only
 >     > > to display warnings for custom variables that should not be set with
 >     > > setq because they have a :set form, or IOW, to display warnings for
 >     > > custom variables that should be set with customize-set-variables.
 >
 >    > I checked most of our uses of :set and agree that they usually behave
 >    > well-mannered so using `setq' instead is practically always a bug
 >    > indeed.
 >
 > I am puzzled by that statement.  First, it seems to say that the
 > definitions of the variables are careful,

I probably should have said "that the definitions of the variables have
been written carefully" so nobody would have come to a conclusion like

 > so that they DTRT if the
 > user sets one with setq in an init file.

which is not what I had in mind.

 > Then it seems to say that setting one of these variable with setq (in
 > an init file?) is almost surely a bug.

This is the conclusion I had in mind.

 > Those two statements are almost opposites.
 >
 > Did I misunderstand?

Either you did or my formulation was bad.

martin.

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > I probably should have said "that the definitions of the variables have
  > been written carefully" so nobody would have come to a conclusion like

  >  > so that they DTRT if the
  >  > user sets one with setq in an init file.

  > which is not what I had in mind.

I am now not sure what you had in mind.
However, we seem to have reached conflicting conclusions.
You say that

 > Then it seems to say that setting one of these variable with setq (in
 > an init file?) is almost surely a bug.

but everything else in this discussion seems to support the opposite
conclusion: setting one of these variables with setq in an init file,
in the usual simple case (you haven't loaded the definition yet), is
perfectly ok.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[martin rudalics]

 > I am now not sure what you had in mind.
 > However, we seem to have reached conflicting conclusions.
 > You say that
 >
 >   > Then it seems to say that setting one of these variable with setq (in
 >   > an init file?) is almost surely a bug.
 >
 > but everything else in this discussion seems to support the opposite
 > conclusion: setting one of these variables with setq in an init file,
 > in the usual simple case (you haven't loaded the definition yet), is
 > perfectly ok.

Note that Gregory's patch addresses only a small subset of customizable
variables - those whose values should be assigned with a :set form.
Typical examples are timer based options like 'blink-cursor-delay' where
setting the variable requires to start or restart a timer or options
where you want to update things on all live frames or windows as with
'frame-background-mode'.  With most of these options a simple 'setq'
would have none or at least not the desired visible effect.

martin

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Gregory Heytings]

>
> everything else in this discussion seems to support the opposite 
> conclusion: setting one of these variables with setq in an init file, in 
> the usual simple case (you haven't loaded the definition yet), is 
> perfectly ok.
>

Understanding statements always requires some context.  Using setq on a 
variable that hasn't been loaded is indeed okay, but that does not 
contradict what Martin said, because a variable that hasn't been loaded 
never has a :set form (even if its definition contains a :set form).  In 
summary:

1. 'foo' does not have a :set form in its definition => it's okay to set 
it with setq, no warning is displayed

2. 'foo' has a :set form in its definition, but its definition hasn't been 
loaded => it's okay to set it with setq, no warning is displayed

3. 'foo' has a :set form in its definition, and its definition has been 
loaded => it's a bug to set it with setq, a warning is displayed

There are a number of corner cases that are not covered by the above 
description, however:

1. if you eval-buffer your init file during an Emacs session, definitions 
have been loaded in the meantime, so you get warnings you didn't get when 
you started Emacs; are these legitimate warnings?

2. if you byte-compile your init file, setq's become set's, so the 
warnings you would see with your non-byte-compiled init files are not 
displayed anymore; should they be displayed?

3. in (some) files in which variables with a :set form are defined, setq 
is used to set them (which means that warnings would be displayed if they 
were eval-buffer'd or loaded without byte-compiling them); are these 
legitimate warnings? is it a bug to use setq in those files?

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > >> Then I still don't get what you mean.  An autoloaded but not yet loaded 
  > >> variable doesn't have its custom-set property set, so in that case 
  > >> there would be no warning with the proposed patch.  Warnings are 
  > >> displayed only for customs variables that are "fully" loaded, and who 
  > >> have a :set property.

This is not a problem.

If at the end of the in it file the defcustom has not been executed
then the setq of the variable will cause no trouble.  Executing the defcustom
will fix things up.

If at the end of the in it file the defcustom HAS been executed,
then the variable will be on all the lists it is supposed to be on,
and the fix I proposed will DTRT with it.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > > That's not an autoloaded variable (well, minor mode), so Emacs doesn't 
  > > know that it has a :set when .emacs is loaded.
  > >

  > Oh, I see.  You mean that setq'ing autoloaded variables, which are loaded 
  > before their .el files is loaded, can usually be setq'd, disregarding 
  > their :set property.  But I don't see how one could distinguish between 
  > those for which it is problematic and those for which is isn't.

Please see the solution I proposed in the past few days.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Gregory Heytings]

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


>
> Please see the solution I proposed in the past few days.
>

The solution you proposed is not optimal I think, in particular because 
variables can also be set interactively, e.g. with M-: or C-x C-e, so 
checking that they have been set correctly after loading the init file is 
not enough.

The proposed (two lines!) patch works in all cases.  Its commit message 
was perhaps not clear enough, so I made it longer to make it (hopefully) 
crystal clear that it does TRT: there are no warnings for preloaded 
variables whose files are loaded after those variables have been setq'd.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=Warn-when-custom-variable-is-wrongly-set.patch, Size: 1347 bytes --]

From b74e9dfcf9c67455223b2a3be99692669b3b5205 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Fri, 10 Sep 2021 13:35:50 +0000
Subject: [PATCH] Warn when custom variable is wrongly set.

* src/eval.c (Fsetq): Display warning when a custom variable with a :set
property is set with setq.  Warnings are displayed only for custom variables
whose files have been loaded and that have a :set property.  No warnings are
displayed for custom variables whose files have merely been preloaded,
for custom variables that do not have a :set property, and for non-custom
variables.  See bug#21695.
---
 src/eval.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/eval.c b/src/eval.c
index 48104bd0f4..168a7749a0 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -525,8 +525,15 @@ DEFUN ("setq", Fsetq, Ssetq, 0, UNEVALLED, 0,
 	   : Qnil);
       if (!NILP (lex_binding))
 	XSETCDR (lex_binding, val); /* SYM is lexically bound.  */
-      else
+      else {
+	if (!NILP (Fget (sym, intern ("custom-set"))))
+	  call2 (intern ("display-warning"), intern ("setq"),
+		 CALLN (Fformat,
+			build_string
+			("`%s' should be set with `customize-set-variable'"),
+			sym));
 	Fset (sym, val);	/* SYM is dynamically bound.  */
+      }
     }
 
   return val;
-- 
2.33.0

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > The solution you proposed is not optimal I think, in particular because 
  > variables can also be set interactively, e.g. with M-: or C-x C-e, so 
  > checking that they have been set correctly after loading the init file is 
  > not enough.

Yes they can, but why is there a problem?  I don't see one.
Could you spell out the scenario?

  > * src/eval.c (Fsetq): Display warning when a custom variable with a :set
  > property is set with setq.  Warnings are displayed only for custom variables
  > whose files have been loaded and that have a :set property.

That might be a good solution too.

I'm not trying to argue that my solution is best.
I'm only arguing against telling people to change their
init files to replace `setq' with `customize-set-variable'.
If your solution avoids that, I don't mind.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#21695: [External] : bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Drew Adams]

>> Searching emacs 27.2 elisp source I find about 400 hits for :set.
> 
> That is more than I expected.  However, there around 8000 defcustoms
> in master as of May 11, and only 462 instances of :set.

Numbers are good to have.  But we've said from the
beginning that a small minority of defcustoms use
:set.  That's 6%, which doesn't surprise me at all.

(Of course that checks only code that's part of
Emacs as distributed, not 3rd-party code.  But I
expect the proportion to be even smaller there.)

> Telling people to use customize-set-variable for all 8000 of them
> feels like the tail wagging the dog.

No one, I think, has suggested that users should
privilege using Lisp to set user options.  I said
we should recommend that they use the Customize UI.

For users who do sometimes use Lisp for that, no
one has suggested that they customize all 8000
options.  What was suggested is that for Lisp use
it's appropriate to use `customize-set-variable',
not `setq'.

> I have a feeling that most of those 462 with :set actually require
> that people use customize-set-variable to set them in the init file.

Why that feeling?  Why not a feeling that :set and
:initialize are there mainly with the expectation
that users use the Customize UI?

> I suspect that, for most of them, :set is meant to handle the case
> where you change the setting once the feature is already in use.

Why do you suspect that?  And what difference does
it make when (or why) you change the value?  Maybe
I'm missing something here - could you elaborate?

> I think that if we eliminate these, we will get a much smaller
> number of exceptions, and we could find a nicer way to handle them.

Again, I'm not clear about what you're saying, or
why.  I guess you mean eliminate the use of :set
in some of those defcustoms that use it?

> But even with 462 exceptions, it could be easy enough to warn about
> setting one of those in .emacs with setq.

I don't think that's the right approach, but I
hear you.  (Eli spoke to the use of warnings for
init-file loading.)
________


I suggest that we create a short alias, such as
`cset' for `customize-set-variable' - for "set
custom variable".

And I suggest that we recommend, for options,
that users use, in order of priority/favor:

 1. The Customize UI.
 2 `cset' if they don't use the UI.

Simple.  Both just DTRT for options.  No gotcha.

And I suggest that we motivate this by telling
users why - tell them that setting some options
requires additional behavior, besides just
setting the value (i.e., besides `setq').

Not the end of the world for users who don't
read or follow that recommendation - just what we
have now (gotchas in a small minority of cases).

And I recommend that the doc examples that use
`setq' with user options be changed to use `cset'.

I don't think this is a radical or cumbersome
proposal.  Others can disagree, of course.  If
implemented, who would be bothered by it, in
practice?  Not those who would continue to use
`setq' with options, I expect.

How much doc would actually need to be fixed?
Likely very little, but it would mean checking
occurrences of `setq'.  How many example
occurrences involve options?  I expect few.
___

Here's another alternative (I'm _not_ suggesting
it):

`customize-set-variable' in fact does just a
`set' if applied to a non-option.  This means
that `set' and `setq' could do just that, i.e.,
they could take care of the option case.

Looking at the code for `customize-set-variable',
it seems like is should first test whether the
arg is in fact an option, before doing a bunch
of custom stuff.  Maybe I'm misreading, and
there's no quicker way.

Or maybe `customize-set-variable' should _not_
set non-options?  Maybe it should raise an error
for a non-option?  (Someone will say that's not
backward-compatible...)
___

Anyway, I remind everyone posting in this thread
that the bug was already summarily dismissed
("won't fix").

bug#21695: [External] : bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  >   What was suggested is that for Lisp use
  > it's appropriate to use `customize-set-variable',
  > not `setq'.

Yes, that was the suggestion that I wrote to oppose.
I claim that using setq is valid and reliable in nearly all cases,
so the cost in inconvenience of that suggestion
would outweigh the benefit.

  > I have a feeling that most of those 462 with :set actually require
  > that people use customize-set-variable to set them in the init file.

I accidentally omitted "don't" here.  Sorry.  It should say

  > I have a feeling that most of those 462 with :set don't actually require
  > that people use customize-set-variable to set them in the init file.


  > > I suspect that, for most of them, :set is meant to handle the case
  > > where you change the setting once the feature is already in use.

  > Why do you suspect that?

Because all uses of the variable are in the same file that contains
the defcustom.  When .emacs is running, the variable is not defined,

We designed Customize to handle the case where the variable has
already been set when the defcustom is executed.  For these variables,
there should be no problem, even if the variable uses :set.

  >    And what difference does
  > it make when (or why) you change the value?

The issue was defined that way.  The case we are talking about is
where .emacs sets the variable and doesn't use
`customize-set-variable' to do it.

There may be a real problem with a few variables.  I can't be sure
there isn't.  But it will be just a few variables -- those that use
:set and _are already defined_ when .emacs runs.

This means they are variables that have a special, important role in
Emacs, more important than customizing some Lisp library.  There are
not many of these which use :set.

And perhaps not even all of those variables will have an actual
problem.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

I wrote

  > I have a feeling that most of those 462 with :set actually require
  > that people use customize-set-variable to set them in the init file.
  > I suspect that, for most of them, :set is meant to handle the case
  > where you change the setting once the feature is already in use.

but I meant to add "not".

  > I have a feeling that most of those 462 with :set don't actually require
  > that people use customize-set-variable to set them in the init file.
  > I suspect that, for most of them, :set is meant to handle the case
  > where you change the setting once the feature is already in use.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Augusto Stoffel]

On Mon, 30 Aug 2021 at 23:06, Richard Stallman <rms@gnu.org> wrote:

> [[[ To any NSA and FBI agents reading my email: please consider    ]]]
> [[[ whether defending the US Constitution against all enemies,     ]]]
> [[[ foreign or domestic, requires you to follow Snowden's example. ]]]
>
> There are a few customization options that are not really variables
> and need to be set with `customize-set-variable'.
>
> However, the rest of them are variables and normally set with `setq'
> and friends.
>
> Telling users to set them all with `customize-set-variable' will be a
> significant hassle.  What's more, users will disregard this instruction.
> `setq' is easy to remember and we all know it.
>
> Having a recommendation in the manual which users will generally
> reject is in itself a problem.
>
> I think we should address this some other way.  Here are some ideas.
>
> * indicate the few customization options
>    which are not really variables.
> * tell people how to check whether
>    a customization options is really a variable.
> * warn about setq for a customization option that isn't really a variable.

I have encountered one pitfall with setq in my init files, but it has
nothing to do with customize.

Namely, if a variable is marked to automatically become buffer-local
when set, then setq will only set it in the temporary buffer where
init.el is evaluated.

Maybe calling setq during init time on automatically buffer-local
variables should generate a warning.

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > Namely, if a variable is marked to automatically become buffer-local
  > when set, then setq will only set it in the temporary buffer where
  > init.el is evaluated.

With those variables, you're supposed to use setq-default if your
aim is to change the default value.

  > Maybe calling setq during init time on automatically buffer-local
  > variables should generate a warning.

Perhaps it should do that only if you're in the buffer that is
current by default during the init files.

I think the basic idea of this warning is good.
There are various possble ways to implement a warning,
so it would be useful to look, now, for the best way.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Date: Wed, 01 Sep 2021 23:38:37 -0400
> Cc: ke.vigouroux@laposte.net, 21695@debbugs.gnu.org, larsi@gnus.org
> 
>   > Maybe calling setq during init time on automatically buffer-local
>   > variables should generate a warning.
> 
> Perhaps it should do that only if you're in the buffer that is
> current by default during the init files.
> 
> I think the basic idea of this warning is good.
> There are various possble ways to implement a warning,
> so it would be useful to look, now, for the best way.

That could be done, but let's please keep in mind that warnings shown
during startup are notoriously easy to miss, especially if your
startup sequence does a lot.  Those messages flush in the echo-area,
interspersed with a lot of informative messages from features that are
initialized at startup, and the only way to see them is to look in
*Messages* after startup, something that is unlikely to be done by a
lot of us.

bug#21695: [External] : bug#21695: 25.0.50; Change most occurrences of `setq' in Emacs manual to `customize-set-variable'? Really?

[Drew Adams]

> With those variables, you're supposed to use setq-default if your
> aim is to change the default value.
> 
>   > Maybe calling setq during init time on automatically buffer-local
>   > variables should generate a warning.
> 
> Perhaps it should do that only if you're in the buffer that is
> current by default during the init files.
> 
> I think the basic idea of this warning is good.
> There are various possble ways to implement a warning,
> so it would be useful to look, now, for the best way.

I really should have made clear in the bug title and
write-up that I was talking about the use of `setq'
with user options.

The use of `setq' with buffer-local variables, when
what someone really wants is to use `setq-default',
is an orthogonal issue.

It's fine to examine occurrences of `setq' in the
docs for both issues, but the buffer-local (which
also means non-option) case was not what I had in
mind when filing the report.