unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: "Basil L. Contovounesios" <contovob@tcd.ie>
To: Drew Adams <drew.adams@oracle.com>
Cc: Dario Gjorgjevski <dario.gjorgjevski@gmail.com>, 35771@debbugs.gnu.org
Subject: bug#35771: [PATCH] Customization type of recentf-max-saved-items
Date: Fri, 17 May 2019 15:17:57 +0100	[thread overview]
Message-ID: <87woipupuy.fsf@tcd.ie> (raw)
In-Reply-To: <42941bba-e6b4-4a46-b56f-97ffcfc2117f@default> (Drew Adams's message of "Fri, 17 May 2019 06:36:30 -0700 (PDT)")

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.

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

>> 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.

> 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.

> 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.

> 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.

Thanks,

-- 
Basil





  reply	other threads:[~2019-05-17 14:17 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-17 12:22 bug#35771: [PATCH] Customization type of recentf-max-saved-items Dario Gjorgjevski
2019-05-17 13:13 ` Basil L. Contovounesios
2019-05-17 13:33   ` Eli Zaretskii
2019-05-17 14:14     ` Basil L. Contovounesios
2019-05-17 14:39       ` Eli Zaretskii
2019-05-19  9:15   ` Dario Gjorgjevski
2019-05-23  0:21     ` Basil L. Contovounesios
2019-05-17 13:36 ` Drew Adams
2019-05-17 14:17   ` Basil L. Contovounesios [this message]
2019-05-17 15:30     ` Drew Adams
2019-05-18 16:58       ` Basil L. Contovounesios
2019-05-18 23:10         ` Drew Adams
2019-05-19  2:52           ` Basil L. Contovounesios
2019-05-22  5:28             ` Eli Zaretskii
2019-05-23  0:24               ` Basil L. Contovounesios

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87woipupuy.fsf@tcd.ie \
    --to=contovob@tcd.ie \
    --cc=35771@debbugs.gnu.org \
    --cc=dario.gjorgjevski@gmail.com \
    --cc=drew.adams@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).