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

[Basil L. Contovounesios]

Drew Adams <drew.adams@oracle.com> 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,
>
> Irrelevant here.  This is no different from
> suggesting clearer or more correct doc-string
> wording or fixing an off-by-one error.
>
> It pertains to _this_ bug.  Whether there might
> be other, similar bugs is irrelevant to whether
> it should be taken into account for fixing this one.

This bug report is about an incorrect custom :type.  You're arguing
about restricting the custom :type.  Having a looser type (integer) than
desirable (natnum) is a separate issue, one which I would rather be
discussed elsewhere.

>> and concerns a potential change in general Elisp conventions,
>
> What potential change would that be?

Either adding and using a new custom :type, or encouraging usage of
restricted-sexp instead of integer for nonnegative values.

> Is there some existing Elisp convention that says :type should be
> mistyped or should be the loosest possible type in preference to the
> most accurate type?  Does some convention recommend omitting :type
> altogether, to keep things simple?

Straw man.  The de facto convention is indeed that using the integer
type is fine even for nonnegative values.

>> so I would rather it were discussed on its own
>> and with other people included, and any conclusions
>> reported as wishlists and/or documented.
>
> Don't know what you're aiming for.

A more democratic discussion in a more appropriate and less
opportunistic place.

> There's no reason not to fix this bugged occurrence just because you
> also see the possibility that similar problems can exist elsewhere.

I don't consider this a bug/problem.  I consider the addition of a
natnum type a nice-to-have, and the use of restricted-sexp here a
nonsolution.

> I already provided simple code to fix this one.
> What's the problem?  Why not help users now by
> letting Customize properly support the allowable/
> expected set of values for this option?

Dario's patch already lets "Customize properly support the allowable
expected set of values for this option."

>> > 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.
>
> Then please fix it properly in a second step, if you
> prefer.  There's no need for you (or me) to file
> another report to get the customization type fixed
> as it should be for this option.

As far as I'm concerned, Dario's patch is the proper fix for this bug,
and the second step would be to add a natnum widget, but I can't promise
to work on that any time soon, sorry.

>> in this case there is nothing wrong with using the integer type.
>
> Nothing wrong with using `number' either, in that case.
> Nothing wrong with using `sexp' either, in that case.
> If you don't want to specify the type then don't specify
> the type - anything at all will do: nothing wrong, as
> you say.
>
> To me that flies in the face of why we even have :type.
>
> But hey - we _don't_ have :type!  It's optional.  Who
> needs pesky old :type?  Do as you like, including doing
> nothing.

I didn't say there's nothing wrong with using sexp where integer will
do, no need to exaggerate.

>> > 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.
>
> Tautology, if you define "truly needed" by never needed
> at all, which seems to be your point of view.

It's not.

> But if that's not really your view, what would you say
> is "needed" in the attached cases (from my code)?  `sexp'?
> Something more than `sexp', but avoiding `restricted-sexp'
> (what)?  Would you submit a bug report for each case, to
> add new simple types to avoid use of `restricted-sexp'?
>
> When do you think `restricted-sexp' should be used?
> It sounds like the answer is "never".

It's not.  It has its uses, but trying to emulate natnum in an ad hoc
way is not one of them.

> Since Lisp does not have typed variables (it does have
> typed values, of course), I suppose you'd just rely on
> the doc string as sole help for users trying to customize
> the value.  Is that it?

No, but docstrings are indispensable regardless.

>> 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.
>
> With that point of view the `sexp' type is fine when
> dealing with _any_ defcustom.  It will surely help
> avoid the danger of "overspecifying".  Go for it.

You're misconstruing my point.

> IMO "existing precedent" when it comes to defcustom
> is sometimes not so great.  Just like some developers
> don't spend enough attention on doc strings, so some
> don't spend enough attention on defcustom types.
> (This bug report is a case in point, no?)

No, this bug report is likely a result of an oversight.

> That's one reason users give up on using Customize:
> it's too often not so helpful (no completion when
> completion could help, for example).  We've fixed
> some such oversights in the past, but there are
> likely more.
>
> Emacs developers themselves have been clear now and
> then that they mostly don't use Customize, and that
> shows in the lack of love and attention they give
> defcustoms sometimes.  Emacs can help users more.

Sure, which is why I think discussing this on emacs-devel or in a
separate report would be more appropriate and productive.

>> > 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; 
>
> That has nothing to do with "not everyone uses
> Customize".  Even if everyone did use Customize you
> would not be able to rely on :type alone to tell
> users what values are acceptable.
>
> And no one has said that one can, or should be able
> to, rely on :type alone.  Totally a red herring.
> You may not see it as irrelevant, but I do.
>
>> the docstring must also contain this information.
>
> You make it sound like having to describe the type
> of the option is an unfortunate _extra_ thing that
> authors have to do.

That's not what I intended to convey.

> It's not.  A doc string is a
> normal part of defun, defvar, defconst, defmacro,
> etc.
>
> (Just because `interactive' might control input
> values, that doesn't mean that we don't need to
> also document them in the doc string.  Code
> controlling things properly doesn't obviate the
> need for user help.  Nothing replaces doc strings.)
>
> Having to describe the accepted value types in
> the doc string is a red herring - unrelated to
> the separate need to specify a proper :type.  In
> one case you're talking directly to the user.  In
> the other case you're communicating to Customize
> (which in turn talks to the user in its own way).

I agree; you seem to have misconstrued my aside.

>> This encourages writing better docstrings 
>
> What?  What encourages writing better doc strings?
> Not having good :type values?  By that logic `sexp'
> will be ideal as :type, _really_ encouraging good
> doc strings.  Just don't use :type at all - get
> great doc.

You seem to have misunderstood me.  What I meant is that the docstring
anyway has to include the same information that Customize can display,
and more.

>> 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.
>
> Again, if your argument is that doc strings alone
> suffice and are the best way or the only good way
> to specify the type, then :type 'sexp is called for
> in all cases (or just don't use :type ever, which
> amounts to the same thing).

That is not my argument.

>> Second, application code must work correctly regardless of the custom
>> :type.
>
> Again, irrelevant.  Of course it must work correctly.
> Doc strings are needed anyway.  And code must work
> correctly anyway.  So what?  How does either of those
> requirements suggest that we don't need to tell
> Customize what the :type is?

Neither of those was meant to suggest that, I'm not sure how you reached
that conclusion.

>> 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.
>
> Why do you think defcustom has a :type at all?  Was
> adding that just misguided "overspecifying" by some
> overeager implementor?

No.

>> > 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.
>
> It means using :type to specify the relevant/good
> values; :set to specify any special behavior needed
> each time it is set; :init to specify any special
> behavior needed when it's initialized; correct and
> complete doc strings to help users understand what
> the option is for - what the option does/means; and
> so on.
>
> That you _can_ get away with specifying a minimal
> :type is not a reason to do so.  That Lisp variables
> are untyped is not a reason not to specify and
> document the expected/allowed values.

Agreed.

>> > 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.
>
> A good start.
>
>> >> FWIW, my vote is against both trying to overspecify custom types, and
>> >> using restricted-sexp for such simple examples.
>> >
>> > "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.
>
> Define "necessary".

In the case of recentf-max-saved-items, it's Dario's patch.

> Lisp variables being untyped,
> and especially given a doc string that specifies
> the type (expected/allowed values), wanting to be
> strictly and minimally "necessary", which I guess
> is what you espouse, calls for :type 'sexp in all
> cases (i.e., never use :type at all).

I disagree.

> No danger, if you do that, of accidentally writing
> the wrong expression for :type and introducing a
> bug.  No need for anything more complex, no
> "overspecifying", since the doc string does all
> that's needed.  Go for it.
>
> Oh, and since not everyone uses Customize, no real
> need to use defcustom at all - just use defvar in
> all cases.  No danger of a miscoded :set, no
> confusing users with the Customize UI - LOTS of
> nasty problems evacuated.  Go for it.

No thanks.

>> > 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.
>
> Just use :type 'sexp (or just omit :type).
> Easier for everyone.  KISS, as you say.

I disagree.

I have already presented my arguments and recommended discussing this
elsewhere, so I will now remain quiet to give others a chance to chime
in.

Thanks,

-- 
Basil