bug#35771: [PATCH] Customization type of recentf-max-saved-items

[Basil L. Contovounesios]

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