From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: "Basil L. Contovounesios" Newsgroups: gmane.emacs.bugs Subject: bug#35771: [PATCH] Customization type of recentf-max-saved-items Date: Sat, 18 May 2019 17:58:15 +0100 Message-ID: <87r28vwvh4.fsf@tcd.ie> References: <87pnohb79x.fsf@gmail.com> <42941bba-e6b4-4a46-b56f-97ffcfc2117f@default> <87woipupuy.fsf@tcd.ie> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="193428"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) Cc: Dario Gjorgjevski , 35771@debbugs.gnu.org To: Drew Adams Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sat May 18 19:15:55 2019 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hS2w5-000o8w-PE for geb-bug-gnu-emacs@m.gmane.org; Sat, 18 May 2019 19:15:54 +0200 Original-Received: from localhost ([127.0.0.1]:36588 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hS2w4-0001c3-Jq for geb-bug-gnu-emacs@m.gmane.org; Sat, 18 May 2019 13:15:52 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:46261) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hS2ur-0000RC-Mo for bug-gnu-emacs@gnu.org; Sat, 18 May 2019 13:14:40 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hS2fl-0001w8-W7 for bug-gnu-emacs@gnu.org; Sat, 18 May 2019 12:59:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:47572) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hS2fl-0001ve-MD for bug-gnu-emacs@gnu.org; Sat, 18 May 2019 12:59:01 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1hS2fl-0007Cd-JL for bug-gnu-emacs@gnu.org; Sat, 18 May 2019 12:59:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: "Basil L. Contovounesios" Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 18 May 2019 16:59:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 35771 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 35771-submit@debbugs.gnu.org id=B35771.155819871227645 (code B ref 35771); Sat, 18 May 2019 16:59:01 +0000 Original-Received: (at 35771) by debbugs.gnu.org; 18 May 2019 16:58:32 +0000 Original-Received: from localhost ([127.0.0.1]:32883 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hS2fH-0007Bo-LO for submit@debbugs.gnu.org; Sat, 18 May 2019 12:58:32 -0400 Original-Received: from mail-wr1-f44.google.com ([209.85.221.44]:44616) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hS2fD-0007BV-ID for 35771@debbugs.gnu.org; Sat, 18 May 2019 12:58:29 -0400 Original-Received: by mail-wr1-f44.google.com with SMTP id c5so10117015wrs.11 for <35771@debbugs.gnu.org>; Sat, 18 May 2019 09:58:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tcd-ie.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=wcLuMpS9ndLdgdFQE8POChTvJ4JmazKk0MzAEJ2VjwM=; b=JiQVhQqYcsGTtXNhv4ZZRQsdL9iGjvkX8NgpB9vUyIJ4E53rQmXNKDR0PG7o+zANoL GPdVmO80FMZZlyow4M1J0wpE+8rcNf7pD5918ummwlDE0srTm7hhpm/1aEnGeU4KqWmy ReAP754n6kI0pPziS39Je8VwOuZYXKDXqJSjlqqztWxWpcVgnQ/M3wHXBDHaWaXkLxeo 59q7VjEzZIzZCZbIo/q83JuMSCIO+zgA8mNu1gI4hQ5rAfOmtznQosrA23yB9eFc/+f2 mIjV9RQhkKn1LU5z8UFyVvtYXp9VHor1hwauYMYZjjt+79S5dqGBB3JaH3oyARFJfqzC eP3A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version; bh=wcLuMpS9ndLdgdFQE8POChTvJ4JmazKk0MzAEJ2VjwM=; b=s8MOYYYQQhXwWNcePfc3MpHeI/0pLKlXS4wYm4LoC3yWppLlveRZrkVXkQ8Mm7imsk 3vJIaaRRvnYaMfPV4MgbK5NRxYn9jUW6gYdazU1zCtYHUdIsA7tvnxgD1wgrsHxpoSJN Mnzl0dKzpLe5fOUDK6rf46S4IvGZBUJnGeYG1ZbfM4PV/jrQON1J07pTu+PPiVSKlwgr ytnP85CQC/jamu+Q4OqZijx6yWHq2Qrkc748aH/CHIPcydZ4tyLLC1OBtAS/oV5ctXKe G2+dCnvuviaUNI21ZWtxY3TzxESBo7JcKKQRz+kwVjab42ZWcFF+6GGe6AJOWxpJwOrN 262Q== X-Gm-Message-State: APjAAAXQxtlxlMOD6Aoy8OESZiAKeRMCt3BidrahNi2vRaX6M270NexR ez3JVWNBFE93zqFriZ2euZd6aQ== X-Google-Smtp-Source: APXvYqwiBCOy8ywB2AlK6su/6jTg8IgpJ5OlWPuJNeIc0hLiPo8rX+eR5yylMq+5ydC6zvJ+SoR2ug== X-Received: by 2002:adf:ec0b:: with SMTP id x11mr2777674wrn.88.1558198701577; Sat, 18 May 2019 09:58:21 -0700 (PDT) Original-Received: from localhost ([163.172.211.46]) by smtp.gmail.com with ESMTPSA id e8sm27154872wrc.34.2019.05.18.09.58.19 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Sat, 18 May 2019 09:58:20 -0700 (PDT) In-Reply-To: (Drew Adams's message of "Fri, 17 May 2019 08:30:49 -0700 (PDT)") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.51.188.43 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.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:159501 Archived-At: Drew Adams writes: >> I don't know whether this has been discussed before, but it seems more >> suited for emacs-devel or its own bug report, rather than the current >> one. > > Well, it certainly also applies to this bug report, I think, > which is purportedly about the "Customization _type_ of > recentf-max-saved-items". Sure, but it applies also to all other user options of a similar nature, and concerns a potential change in general Elisp conventions, so I would rather it were discussed on its own and with other people included, and any conclusions reported as wishlists and/or documented. >> >> The customization type of recentf-max-saved-items is currently defined >> >> as integer, which does not include nil in its domain. However, setting >> >> this variable to nil is supported in the code and also documented. >> >> >> >> This patch changes the customization type to explicitly allow for the >> >> variable to be set to nil through the Customization interface and >> >> similar. (Please note that I copied the type from save-place-limit in >> >> order to be consistent.) >> > >> > (I'm looking at Emacs 26.2, so apologies if the Emacs 27 >> > code has already fixed this.) >> > >> > The code should also be changed to do one of the following: >> > >> > 1. Use `abs' when the variable value is used. >> >> I disagree. It does not make sense for a size to be set to a negative >> number without indication that this is a supported value; it is clearly >> a user error to do so. Silently interpreting negative numbers as their >> absolute value further restricts any future modifications to the >> interpretation of this user option. The programmer should neither be >> punished for such user errors, nor have to spoon-feed the user. >> >> If there is ambiguity as to whether an integral user option can take a >> negative value, the simplest and IMO best solution is to make the >> documentation clearer, not to try to outsmart the user. > > I agree that #1 is not the best way to go (#2 is). But #1 > is certainly better than allowing a negative value to > percolate through the code. No-one is letting a negative value percolate through the code. > (Not that a negative value would be disastrous in this case. For this > particular bug it's not a big deal. But see, again, the Subject line > - why not fix it right? This bug report is about fixing the custom :type to include nil as an accepted value, which the submitted patch fixes in a way suitable for inclusion in emacs-26. I would rather we dealt with one issue at a time. >> > 2. Use `restricted-sexp' in the defcustom :type, to require >> > the value to be a non-negative (or possibly a positive?) >> > integer (or nil). >> > >> > I'm guessing there are additional places in Emacs code >> > where :type 'integer is used but a non-negative integer is >> > really needed/appropriate. It would be good to improve >> > those :type specs as well. >> > >> > (We might also want to consider adding `natnum' or >> > `nonnegative-integer', `positive-integer' and >> > `negative-integer' as possible :type values.) >> >> I'd welcome a natnum type. >> >> > But it is simple to use `restricted-sexp' to control such >> > things. And not only would that improve the behavior for >> > users; it would also, by way of model, encourage users to >> > use `restricted-sexp' in their own code. >> >> I don't see why restricted-sexp should be encouraged. It is far simpler >> to use and harder to abuse combinations of predefined simple types. >> Besides, not everyone uses the Customize interface. > > There is no alternative, when the type you want to express > is not available as any "combination of predefined simple > types". Hence my welcoming of a new simple type natnum. But in this case there is nothing wrong with using the integer type. > Use of `restricted-sexp' should be encouraged when it's > _needed_, and that's when the type is not currently as > restrictive as it should be AND there is no simpler way > to define the more accurate type. > > That's the point. What we have today is not people > avoiding/discouraging use of `restricted-sexp' in > favor of just-as-useful, just-as-accurate, but simpler > :type specs. We have people not using `restricted-sexp' > out of ignorance of it, or perhaps out of laziness. > (That's my guess, until convinced otherwise.) FWIW, I am neither ignorant of it, nor too lazy to use it; rather, in my limited experience, I have yet to author or review a case where it is truly "needed", this report included. Existing precedent says the integer type is fine even when dealing with nonnegative sizes. If you prefer to use a more accurate natnum type in these cases, which I sympathise with, then please submit a feature request for this, if one does not already exist. I think it is wrong to start using restricted-sexp to emulate a natnum type in an ad hoc way. > As for "not everyone uses Customize" - that's irrelevant > here. This is about those who do use it, obviously. > It's about the :type spec of a defcustom. It is not irrelevant. First, authors cannot rely on the custom :type alone to tell users what qualifies as an expected value; the docstring must also contain this information. This encourages writing better docstrings and is how users may know not to set recentf-max-saved-items to a negative number, regardless of whether its custom :type is integer or natnum. Emacs customisations have worked fine like this until now. Second, application code must work correctly regardless of the custom :type. Since Elisp is not a strongly typed language, the programmer can only assume that the user has understood the docstring and hasn't set the user option to a silly value. > More accurate defcustoms, using more appropriate :type > specs, and sometimes using :set etc., is something we > should encourage. Customize and defcustoms could use > more love by Emacs developers, not less. As long as "more love" means smarter, not more, use, then I agree. >> > Emacs-Lisp code delivered with Emacs is not a great model >> > in this respect. It rarely uses `restricted-sexp' - at >> > least it uses it much less than it could/should (IMHO). >> >> IMO, that's one of the many things that makes Emacs a great and fun >> model - the freedom from having to fight a (easily badly specified) type >> system. Custom types should be as simple and declarative as possible. >> Anything else should be reserved for exceptional cases. > > No idea what you're saying there. On the face of it > it sounds like an argument for using only :type 'sexp, > or perhaps an argument for not using defcustom at all. > I think we probably disagree about 90% here (but I > would glad to learn that I'm wrong about that guess). I meant neither of those things. What I meant is KISS: a good-enough but simple custom :type is far better than a more accurate but more complicated one, especially in the context of something as trivial as a natnum. There is no need to encourage complicated (and very easily buggy) logic in defcustoms when a simpler solution will do. > Using an accurate :type spec doesn't limit/hurt users. > It helps them. Likewise, using a widget edit field > that provides completion when appropriate etc. Agreed. >> > More generally, the distributed Emacs code is pretty >> > "lazy" when it comes to providing defcustom definitions - >> > few :tag descriptions, overly general :type specs, etc. >> > >> > E.g., >> > >> > (defcustom recentf-max-saved-items 20 >> > "Maximum number of recently used file names to save. >> > `nil' means save the whole list. >> > See command `recentf-save-list'." >> > :group 'recentf >> > :type '(choice >> > (restricted-sexp >> > :tag "Save no more than this many file names" >> > :match-alternatives >> > ((lambda (x) (and (natnump x) (not (zerop x))))) >> > :value ignore) >> > (const :tag "Save all file names" nil))) >> >> FWIW, my vote is against both trying to overspecify custom types, and >> using restricted-sexp for such simple examples. If a particular type >> such as natnum keeps cropping up, TRT is to add that type, not emulate >> and duplicate it each time as a restricted-sexp. If you agree, and >> there is no existing bug report for this, please submit one. > > "Overspecify"? "Trying to overspecify"? Please elaborate. > The value should be a natural number (or perhaps a positive > integer), no? How is specifying that exactly overspecifying? > Specifying `integer' is underspecifying. You have that > exactly backward. No, I'm voting against the general notion of trying to specify more than is necessary, just because we can. > Why shouldn't users be helped to provide the most appropriate > values? Why did you say you would welcome a :type of `natnum', > if you argue for unrestricted typing? Why prefer using > `natnum' to `integer' - or even to `sexp' - for such a value, > in that case? I welcome a natnum type because it is simple, informative, and declarative and can be used universally. Elisp already has unrestricted typing, so I don't pretend otherwise. > I don't object to adding a `natnum' :type - I suggested it. > But I also don't have a problem with expressing the same > thing even if we don't have such a type. I think it's silly > to _not_ specify such behavior, and to just use `integer' (or > `sexp') simply because we don't have a `natnum'. That makes > no sense to me. Then please submit a report for that, if one does not already exist. > Do I think we should use `restricted-sexp' even when a > simpler type spec is available to accomplish the same > thing? Of course not. Agreed. Just one opinion, -- Basil