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

[Drew Adams]

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

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

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

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

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.

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.

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

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.

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

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

Do I think we should use `restricted-sexp' even when a
simpler type spec is available to accomplish the same
thing?  Of course not.