bug#39638: 26.3; recentf-auto-cleanup deceptive

[Allen Li <darkfeline@felesatra.moe>]

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

This covers two bugs around recentf-auto-cleanup being
deceptive/unintuitive.

Both bugs are present at 26.3 and on Emacs master as of
556cc727e5076d590f8286406e4f46cff3cee41e
at Sun, 16 Feb 2020 11:37:07 -0800

1. When setting recentf-auto-cleanup to a string, the timer does not
repeat.  It is only set once.  This is in contrast to midnight-mode,
which repeats its timer every day.  The documentation for
recentf-auto-cleanup does not make this clear, and I'm not even sure if
this was the intended behavior.

(defun recentf-auto-cleanup ()
  "Automatic cleanup of the recent list."
  (when (timerp recentf-auto-cleanup-timer)
    (cancel-timer recentf-auto-cleanup-timer))
  (when recentf-mode
    (setq recentf-auto-cleanup-timer
          (cond
           ;; snipped
           ((stringp recentf-auto-cleanup)
            (run-at-time
             recentf-auto-cleanup nil 'recentf-cleanup))))))

2. Due to the behavior of run-at-time, if the time string set was in the
past for today, recentf-cleanup runs immediately when recentf-mode is
turned on (e.g., at Emacs startup).  This makes it pointless to set it
to something like "3:00am" if I want recentf-cleanup to run at a time
when I'm likely not using Emacs and I have also set
recentf-max-saved-items to something large like 2000.  The docstring
does not make this obvious.  This is also how one would usually
customize midnight-mode.

2a. midnight-mode suffers from the same problem of using run-at-time,
but the default behavior of midnight-mode does not make it expensive.
But this means that adding recentf-cleanup to midnight-hook when using a large
recentf-max-saved-items will still be expensive at startup.

I have attached a number of patches:

1. Simply fix some awkward wording that is not directly related to this bug.
2. Document the current behavior.
3. Make recentf-auto-cleanup repeat for time strings.

The third patch can be skipped if deemed too aggressive, but I think
that's the more reasonable behavior to expect.

I have not fixed the problem of recentf-cleanup running immediately if
the time is in the past for today, since I'm not sure the best way to do
it.

In GNU Emacs 26.3 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.10)
 of 2019-08-29 built on juergen
Windowing system distributor 'The X.Org Foundation', version 11.0.12007000
System Description:	Arch Linux


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-recentf-auto-cleanup-customize-wording.patch --]
[-- Type: text/x-patch, Size: 875 bytes --]

From 6882a948b60c03f19f53597acb92b85f52a41cd0 Mon Sep 17 00:00:00 2001
From: Allen Li <darkfeline@felesatra.moe>
Date: Sun, 16 Feb 2020 22:59:18 -0800
Subject: [PATCH 1/3] Fix recentf-auto-cleanup customize wording

* lisp/recentf.el (recentf-auto-cleanup): Fix wording
---
 lisp/recentf.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/recentf.el b/lisp/recentf.el
index 27918a9739..94241b00d5 100644
--- a/lisp/recentf.el
+++ b/lisp/recentf.el
@@ -257,7 +257,7 @@ recentf-auto-cleanup
                         :value mode)
                 (const  :tag "Never"
                         :value never)
-                (number :tag "When idle that seconds"
+                (number :tag "When idle after (seconds)"
                         :value 300)
                 (string :tag "At time"
                         :value "11:00pm"))
-- 
2.25.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Clarify-behavior-of-setting-recentf-auto-cleanup-to-.patch --]
[-- Type: text/x-patch, Size: 973 bytes --]

From a47a121c983b9062079ae4aa2e687bcf138dd8a5 Mon Sep 17 00:00:00 2001
From: Allen Li <darkfeline@felesatra.moe>
Date: Sun, 16 Feb 2020 23:06:02 -0800
Subject: [PATCH 2/3] Clarify behavior of setting recentf-auto-cleanup to
 string

* lisp/recentf.el (recentf-auto-cleanup): Clarify docstring
---
 lisp/recentf.el | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lisp/recentf.el b/lisp/recentf.el
index 94241b00d5..9c4ef21b52 100644
--- a/lisp/recentf.el
+++ b/lisp/recentf.el
@@ -247,6 +247,12 @@ recentf-auto-cleanup
 - A time string
     Cleanup at specified time string, for example at \"11:00pm\".
 
+If a time string is provided, cleanup is only scheduled once each
+time the mode is turned on; it does not repeat daily.
+Furthermore, the time is for the current day.  If it is already
+past the specified time for the day, cleanup happens immediately
+as for `mode'.
+
 Setting this variable directly does not take effect;
 use \\[customize].
 
-- 
2.25.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-Make-recentf-auto-cleanup-repeat-when-set-to-string.patch --]
[-- Type: text/x-patch, Size: 2223 bytes --]

From b1c40cdeac3bf806a140bbc527604163d1818468 Mon Sep 17 00:00:00 2001
From: Allen Li <darkfeline@felesatra.moe>
Date: Sun, 16 Feb 2020 23:15:35 -0800
Subject: [PATCH 3/3] Make recentf-auto-cleanup repeat when set to string

* lisp/recentf.el (recentf-auto-cleanup): Make timer repeat, update docstring
* etc/NEWS: Update news
---
 etc/NEWS        |  7 +++++++
 lisp/recentf.el | 12 +++++-------
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 1f8e6049a8..b79d4f8fb8 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -146,6 +146,13 @@ key             binding
 / v             package-menu-filter-by-version
 / /             package-menu-filter-clear
 
+** recentf
+
+---
+*** 'recentf-auto-cleanup' time string now repeats.
+When 'recentf-auto-cleanup' is set to a time string, it now repeats
+every day, rather than only running once after the mode is turned on.
+
 \f
 * New Modes and Packages in Emacs 28.1
 
diff --git a/lisp/recentf.el b/lisp/recentf.el
index 9c4ef21b52..dd8c2135e8 100644
--- a/lisp/recentf.el
+++ b/lisp/recentf.el
@@ -245,13 +245,10 @@ recentf-auto-cleanup
 - A number
     Cleanup each time Emacs has been idle that number of seconds.
 - A time string
-    Cleanup at specified time string, for example at \"11:00pm\".
+    Cleanup at specified time string daily, for example at \"11:00pm\".
 
-If a time string is provided, cleanup is only scheduled once each
-time the mode is turned on; it does not repeat daily.
-Furthermore, the time is for the current day.  If it is already
-past the specified time for the day, cleanup happens immediately
-as for `mode'.
+If a time string is provided and it is already past the specified time
+for the current day, the first cleanup happens immediately as for `mode'.
 
 Setting this variable directly does not take effect;
 use \\[customize].
@@ -377,7 +374,8 @@ recentf-auto-cleanup
              recentf-auto-cleanup t 'recentf-cleanup))
            ((stringp recentf-auto-cleanup)
             (run-at-time
-             recentf-auto-cleanup nil 'recentf-cleanup))))))
+             ;; Repeat every 24 hours.
+             recentf-auto-cleanup (* 24 60 60) 'recentf-cleanup))))))
 \f
 ;;; File functions
 ;;
-- 
2.25.0