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''
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'.
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
[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
> > @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.
> 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.
> 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
[[[ 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)
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.
> 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.]
> 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.
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
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.
[[[ 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)
[[[ 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)
> 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.
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).
> 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.
> 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.
> > 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.
> 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).
>> 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").
> 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.
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.
[[[ 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)
[[[ 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)
[[[ 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)
[[[ 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)
> 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?
[[[ 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)
> 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?
[[[ 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)
> 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?
>
> 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?
[-- 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
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
>> * 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).
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
>>> 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?
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
>
> 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.
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
>> 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.
[[[ 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)
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
[-- 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
>> 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)
[-- 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
> 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?
[-- 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
> 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.
[-- 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 --]
> 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.
> 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.
[-- 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
>
> 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.
[[[ 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)
[[[ 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)
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
> 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
>> + 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?
>> 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-:.
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
> 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.
> 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.
> 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
>> 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.
>> 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.
> 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.
> 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
>> 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.
[[[ 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)
> > > 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.
[[[ 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)
> 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
>
> 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?