GNU bug report logs - #67946
30.0.50; [RFC PATCH] Add an option to not report "Wrote ..." when saving recentf.

Previous Next

Full log

View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: Vladimir Nikishkin <for_emacs-recentf-patch_2023-12-21 <at> lockywolf.net>
Cc: 67946 <at> debbugs.gnu.org, stefankangas <at> gmail.com
Subject: bug#67946: 30.0.50; [RFC PATCH] Add an option to not report "Wrote ..." when saving recentf.
Date: Tue, 26 Dec 2023 14:41:56 +0200

> From: Vladimir Nikishkin <for_emacs-recentf-patch_2023-12-21 <at> lockywolf.net>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 67946 <at> debbugs.gnu.org
> Date: Tue, 26 Dec 2023 13:58:45 +0800
> 
> Stefan Kangas <stefankangas <at> gmail.com> writes:
> 
> > Eli Zaretskii <eliz <at> gnu.org> writes:
> >
> >> If we decide to turn it off, we will need a knob to revert to the old
> >> behavior, because after so many years of the existing behavior,
> >> someone somewhere might be used to it.
> >
> > I think the existing patch basically has that already, but it seems to
> > be lacking a NEWS entry and a :version tag for the defcustom before it
> > can go in.
> >
> > Vladimir, could you fix that and send a new patch?
> 
> Added an entry to etc/NEWS, and, seems, fixed the actual verbosity
> message printing condition.

Thanks, a few minor comments:

> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -1046,7 +1046,11 @@ desktop restoration to continue.
>  Similarly to buffer restoration by Desktop, 'recentf-mode' checking
>  of the accessibility of remote files can now time out if
>  'remote-file-name-access-timeout' is set to a positive number.
> -

That empty separator line should not be deleted.

> +*** New use option 'recentf-verbose'.
           ^^^
"user"

> +By default 'recentf-save-list' prints a message when saving the
> +recentf list. The new option, if set to nil, suppresses this message.
               ^^
Two spaces between sentences, please.

> +If more debugging info suppression switches are to be added, this
> +option might become more useful.

I don't think we need this last sentence in NEWS.  NEWS is about
changes that are actually done, not about potential changes in the
future.

> +(defcustom recentf-verbose 1
> +  "0 means to not show debugging messages related to the recentf machinery.
> +1 means show messages that were printed by default on Emacs <= 29.1."

A doc string should start with a single sentence that summarizes what
the variable or function does.  In this case, I suggest

  Whether to show verbose messages about low-level recentf actions.

Also, if we are going to use an integer here, the variable should be
called something like recentf-verbosity.  Alternatively, we could make
it a simple boolean, since we have no plans to extend it to more than
2 levels.

> +  :group 'recentf

Do you need this :group tag?  It shouldn't be needed in this case, I
think.

Finally, the commit log message is still missing.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.