Package: emacs;
Reported by: Drew Adams <drew.adams <at> oracle.com>
Date: Sat, 10 Dec 2022 05:08:02 UTC
Severity: normal
Found in version 28.2
Done: Eli Zaretskii <eliz <at> gnu.org>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Drew Adams <drew.adams <at> oracle.com> To: Mauro Aranda <maurooaranda <at> gmail.com> Cc: "59937 <at> debbugs.gnu.org" <59937 <at> debbugs.gnu.org> Subject: bug#59937: 28.2; Bad defcustom behavior Date: Tue, 13 Dec 2022 22:10:18 +0000
> > Expecting a defcustom definer to understand this > > and figure out what a "valid default value for > > the restricted-sexp widget" might be, is a bridge > > too far, IMO. > > I don't think so. The defcustom definer is specifying the matching > alternatives, he/she should be able to think of a valid default > value. How does s?he define/provide a default value for the `restricted-sexp' widget? I see no way to do that. Or did you mean that s?he should always provide, within the _option's_ default value, a value for each part of it that corresponds to a `restricted-sexp'? That would be too restrictive, IMO. It should still be possible to provide a default value of () for an alist or plist that makes use of `restricted-sexp' for some element keys or values. Or a default value that in some other way doesn't provide a value for parts that are defined by `restricted-sexp' but might not be needed in the option value. > Maybe having some examples in the documentation could help here. I > could write one if you and others think it could be helpful. I _think_ I understand this now. The problem is that for the Customize UI to present a field for inputting/defining the part of the option value that corresponds to a plist key (which is defined by a `restricted-sexp'), it needs to know just what kind of input/edit widget to build. It needs to build an editable-field that also demands respect of the `restricted-sexp' predicates. And that's the case whether or not the _option's_ default value has a part that corresponds to a plist key. (If yes, the default value must match the `restricted-sexp', if no, you're prompted for the sexp type, so it knows what kind of field to make.) And yes, a simple example with `restricted-sexp' would help (maybe 2 examples: bad & good). The idea/problem isn't limited to `restricted-sexp', IIUC. But in other cases it's much less likely to be a gotcha, because the parts of the defcustom value that correspond to each field in the Customize UI will have types that correspond to existing widgets (they don't require additional input/prompting to know what kind of UI field to create). The problem really stems, I guess, from the fact that `restricted-sexp' can involve any kinds of predicates, and depending on what those do, the UI field can be different. Put differently, the UI field takes into account the `restricted-sexp' predicates. But the prompting does not take them into account! My thoughts about this - let me know what you think: 1. The warning(s) are not very helpful. They will mainly confuse, I think. First, end users _will_ see them, as the defcustom author may not have tested every possibility well. Second, many defcustom authors also won't understand them. 2. I think a big improvement could be to make use of any :tag that the defcustom author provides for the `restricted-sexp' field - using the :tag also as the prompt, instead of "Lisp expression: ". When you see that generic prompt you have _no clue_ what it wants, or why. The :tag should tell you what to enter. In the case used in the bug example, the :tag is "Plist key (keyword):". With that as prompt there's little possible misunderstanding of what we're asking the user to enter. (And if the :tag isn't clear then it also isn't very clear when used as a field label, though someone might figure it out from UI context.) And if the user enters a value that doesn't satisfy the `restricted-sexp' predicates we can still raise an error. Better yet would be to put the sexp prompt in a loop till the input (after `read-from-string') satisfies the `restricted-sexp' predicates (or until C-g). IOW, don't just use `widget-sexp-prompt-value' (it just gets a string and then Lisp-reads that), but also apply the predicates as part of the expression reading. That is, provide the same behavior/support we provide for the UI input field. 3. Don't show any warning when prompting. Just try to have the inputting itself be clearer (#2). With those changes, the manual could also be improved: (1) Tell defcustom definers that if they use `restricted-sexp' then good practice is to provide a :tag for the field. And tell them that the :tag will also be used as a prompt for creating the appropriate editable field. Tell them that otherwise the prompt is just "Lisp expression: ", which makes no semantic connection to the type of data needed for the field. If a defcustom definer is defining a complex option then it behooves them to make clear what the parts are. And if a part is based on `restricted-sexp' then part of making that clear is to add a :tag. (2) Explain that such prompting happens whenever the default value of the option doesn't provide a value for each of its parts that corresponds to the use of a `restricted-sexp'. So if the default value does provide all such parts then there's no prompting (and the need for a :tag is reduced). > > I am curious whether you think there's actually > > a bug or not. It's hard for me to believe that > > we should expect _anyone_ defining a defcustom > > (let alone anyone using Customize) to understand > > the `restricted-sexp' widget, what it requires > > wrt its "default value", and how to adjust a > > defcustom to give it what it needs, to DTRT. > > I think a better behavior would be to avoid the prompting altogether > (there should be no prompt at that moment, for starters). But again, > this situation arises when there is a bug on the defcustom :type, so I'd > be happier if people can help with improving the warning message. See above. I don't think there's a bug in the defcustoms in the examples shown. And I do think users should be prompted if Emacs needs to know what kind of input (UI) field to create. And I don't think we should show any warnings. We can raise an error if the user input is, in the end, invalid (and I think we already take care of that). I may still be misunderstanding things. Let me know. But if so then I'm guessing others will also misunderstand. The current state is, I think, poor support for the flexible, powerful feature that is `restricted-sexp'. Understandable, but we should somehow try to do better. I, for one, wish more definers of defcustoms spent more time defining tighter types. And often that could mean using `restricted-sexp'. FWIW, this bug report came directly from a user question on emacs.SE, here: https://emacs.stackexchange.com/q/74913 Thanks for your efforts with this.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.