From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Mauro Aranda Newsgroups: gmane.emacs.bugs Subject: bug#59937: 28.2; Bad defcustom behavior Date: Tue, 13 Dec 2022 19:50:11 -0300 Message-ID: <533dba58-e543-f356-664f-5dfa0b85467c@gmail.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="858"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Cc: "59937@debbugs.gnu.org" <59937@debbugs.gnu.org> To: Drew Adams Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue Dec 13 23:51:17 2022 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1p5E7I-000AUs-PY for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 13 Dec 2022 23:51:16 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1p5E77-0001wf-7Q; Tue, 13 Dec 2022 17:51:05 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1p5E75-0001wX-GB for bug-gnu-emacs@gnu.org; Tue, 13 Dec 2022 17:51:03 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1p5E75-0007mU-7x for bug-gnu-emacs@gnu.org; Tue, 13 Dec 2022 17:51:03 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1p5E74-0006on-D1 for bug-gnu-emacs@gnu.org; Tue, 13 Dec 2022 17:51:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Mauro Aranda Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 13 Dec 2022 22:51:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 59937 X-GNU-PR-Package: emacs Original-Received: via spool by 59937-submit@debbugs.gnu.org id=B59937.167097182226202 (code B ref 59937); Tue, 13 Dec 2022 22:51:02 +0000 Original-Received: (at 59937) by debbugs.gnu.org; 13 Dec 2022 22:50:22 +0000 Original-Received: from localhost ([127.0.0.1]:35116 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1p5E6Q-0006oY-3V for submit@debbugs.gnu.org; Tue, 13 Dec 2022 17:50:22 -0500 Original-Received: from mail-oo1-f47.google.com ([209.85.161.47]:33439) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1p5E6P-0006oR-1T for 59937@debbugs.gnu.org; Tue, 13 Dec 2022 17:50:21 -0500 Original-Received: by mail-oo1-f47.google.com with SMTP id j6-20020a4ab1c6000000b004809a59818cso2635169ooo.0 for <59937@debbugs.gnu.org>; Tue, 13 Dec 2022 14:50:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=sYzpQxlMKAm3t2cId5VuJwQE8PvVFxc8Q87OpoulidM=; b=R/IPQFjDgLTt+KwqVdLf+NDZ9vSATsxAaFiCWMW33w48I+5ggY3RwUNyDvX5L6xmLS dDRELGc1hwpq3Jgg8wBmipyaXL6j3U+vIKpt9QlWPXZipPdVVnjU+HgYGc0lWH7FKlXK 4fBPVIgFM0nXki2bV3eBw9myDxVVNnVNO82OJe+xFnGT49HSIr5qb0WY77PGX3obS92K 3k+w0kQbDzZkXYEgh1FTrR9Gy4K7G6MwjYgeYjVRLqsRfnMmbvzQ5b6w3YjNBcmNqSNl xXQ9Qq/iCYSLvEgvZCx9Nu9WQulabySSDJ3rGt4ACSdGbuhK3xXIwze9VXStQHW9PerY YpPA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=sYzpQxlMKAm3t2cId5VuJwQE8PvVFxc8Q87OpoulidM=; b=gRuQ3sJWQiKcVUIKA+oRyK5420/ExmD4fQyHlBYP3aWOpMoHGD17fEwwR6daidg3Ov UAQm3zIrkM+klImSX9qhL9Irpv+M8K0rUn30mLKjJv7OMTQqfiQFWF2lp79Bbz/z2L/Q keNWA3no+A1Zf0huHclK1FLe52dmCN2Yexju5NoI64oOixw9hsH2C17pt+O8ZSWQO4uJ tgKmzXCT4uxhv3T3vsTkr6g52eZ9DAyfowoIlkZ9AG2TQUW7ViHvpBfL0x9l4rSgR8VM h5GdM3DxeJ27u3oLvyRW3mMwRAROm0UyDkKxn3JLFYjaeVqtNPFf+/A/FyXAICulOzcP 9Wxw== X-Gm-Message-State: ANoB5pnk+VbqSmeX7/EQ3f81udEp6QBlm6yUbT62vwK55Aj4ANAHAkBc hQtdgkXVvOPeAtEr7klq3nY9aHv+TrQ= X-Google-Smtp-Source: AA0mqf7K712noP1inByJZpIt1weFJvHZfU7CN4NRmzkx6/12fkTRxBTJYuKgu+1hD2Qp9iiH7207JA== X-Received: by 2002:a05:6820:199b:b0:4a0:16a0:1fbb with SMTP id bp27-20020a056820199b00b004a016a01fbbmr10542544oob.2.1670971815136; Tue, 13 Dec 2022 14:50:15 -0800 (PST) Original-Received: from [192.168.0.234] ([181.228.28.240]) by smtp.gmail.com with ESMTPSA id j7-20020a4ad6c7000000b0049b17794d19sm1663992oot.20.2022.12.13.14.50.13 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 13 Dec 2022 14:50:14 -0800 (PST) Content-Language: en-US In-Reply-To: X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:250901 Archived-At: Drew Adams writes: >>  > 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. It's the very first keyword mentioned under the Type Keywords node in the ELisp manual.  Usage would be like this: (defcustom foo nil   "..."   :type '(repeat (restricted-sexp                   :value :key                   :tag "Keyword"                   :match-alternatives (keywordp)))) Clicking INS creates a restricted-sexp without trouble. > 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'? I didn't mean that.  I meant something like the code above. >> 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. Yes. > 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). I'll work on a patch some day this week, then. > 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). Yes. > 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! I'd say don't focus too much on the prompt.  It really shouldn't be there, and I consider it a bug in the Widget code, but it's really an implementation detail.  Without going into a lot of details, we want to READ a string like this: (read var) where VAR is a string, the representation of the value of the widget, whatever that is, but because the widget's value didn't match, it is not a string that we read and VAR is nil (Note: not "nil", but nil), so we end up calling (read nil) and that's the unintended prompt. > 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. I don't know if you're suggesting to improve it or to get rid of it. I'd like to make it more helpful, as I think it should be there. > 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. I don't think that's an improvement because of what I said above. No prompt should be there for starters.  At least that's what I understand about the code. Note that in Bug#25152 you ended up with a weird buffer state after hitting C-g at that prompt.  That's because the Widget library is not ready to take user input at that moment. > 3. Don't show any warning when prompting.  Just try > to have the inputting itself be clearer (#2). Because of my response, I don't think #3 applies.  I hope you agree with me after reading my response. > 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. Here again, I don't think this is the path we want to follow.  And the manual already emphasizes that providing a valid default value is essential, when nil isn't it. > (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'. As I said, I consider the prompt a bug in the Widget code, so I don't think we should mention it in docs. >>  > 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). We'll have to disagree here, about two things. 1.  I still think there's a bug in the defcustoms.  Any widget needs a valid default value, and it's up to the person that's defining the widget to provide it.  If he/she doesn't, then that's a bug. 2.  I do think a warning (or an error) is in place. > I may still be misunderstanding things.  Let me > know.  But if so then I'm guessing others will also > misunderstand. I think you understand now, but the prompt being there is really confusing (it was to me when I first read your bug report in Bug#25152).  Hopefully I clarified a little more with my response. > 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 I missed that.  I usually lurk on SE for questions with custom or widget tags, but you do a better job than me :-). > Thanks for your efforts with this. You're welcome, I'm happy to help.