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

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

[Dario Gjorgjevski]

[-- Attachment #1: Type: text/plain, Size: 444 bytes --]

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Change customization type of recentf-max-saved-items to allow for nil --]
[-- Type: text/x-diff, Size: 856 bytes --]

From a62b4c6208f9d64bc49499855e65ae9b9a55b01e Mon Sep 17 00:00:00 2001
From: Dario Gjorgjevski <dario.gjorgjevski+git@gmail.com>
Date: Fri, 17 May 2019 11:46:54 +0200
Subject: [PATCH] Customization type of recentf-max-saved-items

---
 lisp/recentf.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/recentf.el b/lisp/recentf.el
index 9b70017a38..4112b44e48 100644
--- a/lisp/recentf.el
+++ b/lisp/recentf.el
@@ -67,7 +67,8 @@ You should define the options of your own filters in this group."
 A nil value means to save the whole list.
 See the command `recentf-save-list'."
   :group 'recentf
-  :type 'integer)
+  :type '(choice (integer :tag "Entries" :value 1)
+		 (const :tag "No Limit" nil)))
 
 (defcustom recentf-save-file (locate-user-emacs-file "recentf" ".recentf")
   "File to save the recent list into."
-- 
2.20.1


[-- Attachment #3: Type: text/plain, Size: 76 bytes --]


-- 
Dario Gjorgjevski :: dario.gjorgjevski@gmail.com :: +389 (0)70 784 142

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

[Basil L. Contovounesios]

severity 35771 minor
quit

Dario Gjorgjevski <dario.gjorgjevski@gmail.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.

Indeed.

> This patch changes the customization type to explicitly allow for the
> variable to be set to nil through the Customization interface and
> similar.

The change LGTM, thanks.

> (Please note that I copied the type from save-place-limit in order to
> be consistent.)

(I'm curious as to why :value is set to 1 rather than the default values
20 and 400 of the user options recentf-max-saved-items and
save-place-limit, respectively.  Not an important issue, though.)

> From a62b4c6208f9d64bc49499855e65ae9b9a55b01e Mon Sep 17 00:00:00 2001
> From: Dario Gjorgjevski <dario.gjorgjevski+git@gmail.com>
> Date: Fri, 17 May 2019 11:46:54 +0200
> Subject: [PATCH] Customization type of recentf-max-saved-items

See the outlines "Commit messages" and "Generating ChangeLog entries"
in the file CONTRIBUTE for the preferred commit message format.
In particular, it should mention the names of the file and variable
changed, as well as the bug#number.

The change is small enough to be exempt from copyright paperwork,
but if you are interested in contributing more in the future,
I recommend starting the assignment process early;
see (info "(emacs) Copyright Assignment").

Thanks,

-- 
Basil

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

[Eli Zaretskii]

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Date: Fri, 17 May 2019 14:13:09 +0100
> Cc: 35771@debbugs.gnu.org
> 
> The change is small enough to be exempt from copyright paperwork,
> but if you are interested in contributing more in the future,
> I recommend starting the assignment process early;
> see (info "(emacs) Copyright Assignment").

Dario has an assignment on file, so we are good.

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

[Drew Adams]

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

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

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.

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

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

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

[Basil L. Contovounesios]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: "Basil L. Contovounesios" <contovob@tcd.ie>
>> Date: Fri, 17 May 2019 14:13:09 +0100
>> Cc: 35771@debbugs.gnu.org
>> 
>> The change is small enough to be exempt from copyright paperwork,
>> but if you are interested in contributing more in the future,
>> I recommend starting the assignment process early;
>> see (info "(emacs) Copyright Assignment").
>
> Dario has an assignment on file, so we are good.

Great, sorry for not asking first.

Once Dario submits a log message, will this change be suitable for
emacs-26?

Thanks,

-- 
Basil

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

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

[Eli Zaretskii]

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Cc: <dario.gjorgjevski@gmail.com>,  <35771@debbugs.gnu.org>
> Date: Fri, 17 May 2019 15:14:56 +0100
> 
> Once Dario submits a log message, will this change be suitable for
> emacs-26?

Assuming the patch won't become significantly more complex, yes.

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.

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

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

[Drew Adams]

[-- Attachment #1: Type: text/plain, Size: 10857 bytes --]

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

> and concerns a potential change in general Elisp conventions,

What potential change would that be?  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?

> 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.  There's no
reason not to fix this bugged occurrence just
because you also see the possibility that similar
problems can exist elsewhere.

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?

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

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

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

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

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?

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

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

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.

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

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

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

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

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

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

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

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.

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

[-- Attachment #2: foo.el --]
[-- Type: application/octet-stream, Size: 4166 bytes --]

(defcustom bar ()
  "List of custom themes for..."
  :type '(repeat (restricted-sexp
                  :match-alternatives
                  ((lambda (symb) (memq symb (custom-available-themes))))))
  :group 'foobar)

(defcustom foo ()
  "...
The option value is a list.  Each element defines a submenu or a menu
item.  A null element (`nil') is ignored.

Several alternative entry formats are available.  When customizing,
choose an alternative in the Customize `Value Menu'.

In this description:
 SYMBOL      is a symbol identifying the menu entry.
 `menu-item' is just that text, literally.
 NAME        is a string naming the menu item or submenu.
 COMMAND     is the command to be invoked by an item.
 MENU-KEYMAP is a menu keymap or a var whose value is a menu keymap.
 KEYWORDS    is a property list of menu keywords (`:enable',
             `:visible', `:filter', `:keys', etc.).

1. Single menu item.  For a selectable item, use
   (SYMBOL menu-item NAME COMMAND . KEYWORDS).  For a non-selectable
   item such as a separator, use (SYMBOL NAME) or
   (SYMBOL menu-item NAME nil . KEYWORDS).

2. Items taken from a menu-keymap variable, such as
   `menu-bar-edit-menu'.  Just use the name of the variable (a
   symbol).  The items appear at the top level of the popup menu, not
   in a submenu.

3. Submenu.  Use (SYMBOL menu-item NAME MENU-KEYMAP . KEYWORDS) or
   (SYMBOL NAME . MENU-KEYMAP).  Remember that MENU-KEYMAP can also be
   a variable (symbol) whose value is a menu keymap.

All of these are standard menu elements, with the exception of the use
of a keymap variable to represent its value.

See also:
 * (elisp) Format of Keymaps
 * (elisp) Classifying Events
 * (elisp) Extended Menu Items

Example submenu element:
 (toto menu-item \"Toto\" menu-bar-toto-menu)

Example selectable menu-item element:
 (foo menu-item \"Foo\"   foo-command
       :visible (not buffer-read-only))"
  :type  '(repeat
           (choice
            ;; These could be combined, but it's better for users to see separate choices.
            (restricted-sexp
             :tag "Submenu (SYMBOL menu-item NAME MENU-KEYMAP . KEYWORDS) or (SYMBOL NAME . MENU-KEYMAP)"
             :match-alternatives
             ((lambda (x)
                (and (consp x) (symbolp (car x))
                     (or (and (stringp (cadr x)) (cddr x)) ; (SYMBOL NAME . MENU-KEYMAP)
                         ;; (SYMBOL menu-item NAME MENU-KEYMAP . KEYWORDS)
                         (and (eq 'menu-item (cadr x))
                              (stringp (car (cddr x)))
                              (or (keymapp (car (cdr (cddr x)))) ; Can be a keymap var.
                                  (and (symbolp (car (cdr (cddr x))))
                                       (boundp (car (cdr (cddr x))))
                                       (keymapp (symbol-value (car (cdr (cddr x)))))))))))
              'nil))
            (restricted-sexp
             :tag "Items from a keymap variable's value."
             :match-alternatives ((lambda (x) (and (symbolp x)  (keymapp (symbol-value x))))
                                  'nil))
            (restricted-sexp
             :tag "Selectable item (SYMBOL menu-item NAME COMMAND . KEYWORDS)"
             :match-alternatives ((lambda (x) (and (consp x)  (symbolp (car x))
                                              (eq 'menu-item (cadr x))
                                              (stringp (car (cddr x)))
                                              (commandp (car (cdr (cddr x))))))
                                  'nil))
            (restricted-sexp
             :tag "Non-selectable item (SYMBOL NAME) or (SYMBOL menu-item NAME nil . KEYWORDS)"
             :match-alternatives ((lambda (x) (and (consp x)  (symbolp (car x))
                                              (or (and (stringp (cadr x))  (null (caddr x)))
                                                  (and (eq 'menu-item (cadr x))
                                                       (stringp (car (cddr x)))
                                                       (null (car (cdr (cddr x))))))))
                                  'nil))))
  :group 'foobar)

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

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

[Dario Gjorgjevski]

[-- Attachment #1: Type: text/plain, Size: 623 bytes --]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> (I'm curious as to why :value is set to 1 rather than the default values
> 20 and 400 of the user options recentf-max-saved-items and
> save-place-limit, respectively.  Not an important issue, though.)

I was also wondering the same thing.

> See the outlines "Commit messages" and "Generating ChangeLog entries"
> in the file CONTRIBUTE for the preferred commit message format.
> In particular, it should mention the names of the file and variable
> changed, as well as the bug#number.

Thanks.  I am attaching a patch file with a properly formatted commit
message.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Change customization type of recentf-max-saved-items to allow for nil --]
[-- Type: text/x-diff, Size: 1061 bytes --]

From 46a0d4715ce49aee376a4d253a129c5d6b5b97a8 Mon Sep 17 00:00:00 2001
From: Dario Gjorgjevski <dario.gjorgjevski+git@gmail.com>
Date: Fri, 17 May 2019 11:46:54 +0200
Subject: [PATCH] Customization type of recentf-max-saved-items

Change the customization type of recentf-max-saved-items to include
nil, as it is an allowed value (Bug#35771).
* lisp/recentf.el (recentf-max-saved-items): Change the customization
type in the defcustom.
---
 lisp/recentf.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/recentf.el b/lisp/recentf.el
index 9b70017a38..4112b44e48 100644
--- a/lisp/recentf.el
+++ b/lisp/recentf.el
@@ -67,7 +67,8 @@ You should define the options of your own filters in this group."
 A nil value means to save the whole list.
 See the command `recentf-save-list'."
   :group 'recentf
-  :type 'integer)
+  :type '(choice (integer :tag "Entries" :value 1)
+		 (const :tag "No Limit" nil)))
 
 (defcustom recentf-save-file (locate-user-emacs-file "recentf" ".recentf")
   "File to save the recent list into."
-- 
2.20.1


[-- Attachment #3: Type: text/plain, Size: 76 bytes --]


-- 
Dario Gjorgjevski :: dario.gjorgjevski@gmail.com :: +389 (0)70 784 142

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

[Eli Zaretskii]

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Date: Sun, 19 May 2019 03:52:13 +0100
> Cc: Dario Gjorgjevski <dario.gjorgjevski@gmail.com>, 35771@debbugs.gnu.org
> 
> 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.

Chiming in: I think we should install the latest version of the patch,
and disregard all the rest.

Thanks.

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

[Basil L. Contovounesios]

Dario Gjorgjevski <dario.gjorgjevski@gmail.com> writes:

> I am attaching a patch file with a properly formatted commit message.

Thanks, pushed to emacs-26[1].

[1: e61349c224]: Fix customization type of recentf-max-saved-items
  2019-05-23 01:14:21 +0100
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=e61349c22412c0eaa7c03e08bfb0467a0a79708a

-- 
Basil

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

[Basil L. Contovounesios]

tags 35771 fixed
close 35771 26.3
quit

Eli Zaretskii <eliz@gnu.org> writes:

>> From: "Basil L. Contovounesios" <contovob@tcd.ie>
>> Date: Sun, 19 May 2019 03:52:13 +0100
>> Cc: Dario Gjorgjevski <dario.gjorgjevski@gmail.com>, 35771@debbugs.gnu.org
>> 
>> 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.
>
> Chiming in: I think we should install the latest version of the patch,
> and disregard all the rest.

Done, so I'm closing this report.

Thanks,

-- 
Basil