bug#73291: 31.0.50; `clean-buffer-list' activates `midnight-mode'

bug#73291: 31.0.50; `clean-buffer-list' activates `midnight-mode'

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi,

It seems that calling `clean-buffer-list' once activates
`midnight-mode'.  How to reproduce it:

    - emacs -Q
    - Open some buffers
    - M-x clean-buffer-list ;; Obviously not much buffers are killed
    - Wait one day and go into your emacs window
    - In the *Messages* buffer, you can see that cleaning occured
      recently

I think that one call to `clean-buffer-list' should only clean the
buffer list once.  The documentation does not seem to imply otherwise.

Best regards,


In GNU Emacs 31.0.50 (build 30, x86_64-unknown-openbsd7.6) of 2024-09-15
 built on computer
Repository revision: 2cd5774689f96522709250b7805370522dcd4334
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101013
System Description: OpenBSD computer 7.6 GENERIC.MP#312 amd64

Configured using:
 'configure CC=egcc CPPFLAGS=-I/usr/local/include
 LDFLAGS=-L/usr/local/lib MAKEINFO=gmakeinfo --prefix=/home/manuel/emacs
 --bindir=/home/manuel/bin --with-x-toolkit=no --without-cairo
 --without-compress-install'

Configured features:
DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG LCMS2 LIBOTF
LIBXML2 MODULES NOTIFY KQUEUE OLDXMENU PDUMPER PNG RSVG SQLITE3 THREADS
TIFF TREE_SITTER WEBP X11 XDBE XFT XIM XINPUT2 XPM ZLIB

Important settings:
  value of $LC_CTYPE: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Group

Minor modes in effect:
  gnus-topic-mode: t
  display-time-mode: t
  display-battery-mode: t
  desktop-save-mode: t
  exwm-randr-mode: t
  server-mode: t
  gnus-undo-mode: t
  electric-pair-mode: t
  override-global-mode: t
  repeat-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  minibuffer-regexp-mode: t
  buffer-read-only: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
/home/manuel/prog/elisp/exwm/exwm-randr hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm-randr
/home/manuel/prog/elisp/exwm/exwm hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm
/home/manuel/prog/elisp/exwm/exwm-xsettings hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm-xsettings
/home/manuel/prog/elisp/exwm/exwm-xim hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm-xim
/home/manuel/prog/elisp/exwm/exwm-workspace hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm-workspace
/home/manuel/prog/elisp/exwm/exwm-systemtray hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm-systemtray
/home/manuel/prog/elisp/exwm/exwm-manage hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm-manage
/home/manuel/prog/elisp/exwm/exwm-layout hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm-layout
/home/manuel/prog/elisp/exwm/exwm-input hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm-input
/home/manuel/prog/elisp/exwm/exwm-floating hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm-floating
/home/manuel/prog/elisp/exwm/exwm-core hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm-core
/home/manuel/prog/elisp/exwm/exwm-background hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm-background
/home/manuel/.emacs.d/elpa/ef-themes-1.8.0/theme-loaddefs hides /home/manuel/emacs/share/emacs/31.0.50/lisp/theme-loaddefs

Features:
(info-look dictionary external-completion dictionary-connection shortdoc
comp-common shadow emacsbug gnus-fun smerge-mode diff epa-file
find-dired flow-fill gnus-async gnus-bcklg gnus-ml smtpmail sort dabbrev
mailalias ispell gnus-cite mail-extr gnus-topic qp utf-7 imap rfc2104
nndoc nndraft nnmh nnfolder nnml gnus-agent gnus-srvr gnus-score
score-mode nnvirtual nntp gnus-cache nnrss org-duration cal-china lunar
solar cal-dst cal-bahai cal-islam cal-hebrew cal-julian holidays
holiday-loaddefs cal-iso face-remap org-indent oc-basic ol-eww ol-rmail
ol-mhe ol-irc ol-info ol-gnus nnselect ol-docview doc-view filenotify
ol-bibtex bibtex ol-bbdb ol-w3m ol-doi org-link-doi org-agenda
org-element org-persist org-id org-element-ast inline avl-tree
gnus-icalendar org-capture org-refile org ob ob-tangle ob-ref ob-lob
ob-table ob-exp org-macro org-src ob-comint org-pcomplete org-list
org-footnote org-faces org-entities org-version ob-emacs-lisp ob-core
ob-eval org-cycle org-table org-keys oc org-loaddefs ol org-fold
org-fold-core org-compat org-macs finder-inf mm-archive ef-winter-theme
ef-tritanopia-dark-theme ef-trio-dark-theme ef-rosa-theme ef-owl-theme
ef-night-theme ef-melissa-dark-theme ef-maris-dark-theme
ef-elea-dark-theme ef-duo-dark-theme ef-dream-theme
ef-deuteranopia-dark-theme ef-dark-theme ef-cherie-theme ef-bio-theme
ef-autumn-theme ef-symbiosis-theme man mule-util shr-color textsec
uni-scripts idna-mapping ucs-normalize uni-confusable textsec-check
network-stream url-cache vc-hg vc-bzr view tramp-cmds tramp-cache
time-stamp tramp-sh pulse ffap display-line-numbers tabify conf-mode
sh-script smie treesit executable misearch multi-isearch cl-print
midnight help-fns radix-tree cus-start make-mode vc-dir ewoc on-screen
eww url-queue mm-url pdf-occur ibuf-ext ibuffer ibuffer-loaddefs tablist
tablist-filter semantic/wisent/comp semantic/wisent
semantic/wisent/wisent semantic/util-modes semantic/util semantic
semantic/tag semantic/lex semantic/fw mode-local cedet pdf-isearch
let-alist pdf-misc imenu pdf-tools pdf-view bookmark jka-compr pdf-cache
pdf-info tq pdf-util pdf-macs image-mode exif gnus-dired vc macrostep-c
cmacexp vc-git diff-mode track-changes vc-dispatcher bug-reference time
battery desktop frameset exwm-randr xcb-randr exwm exwm-input
xcb-keysyms xcb-xkb exwm-manage exwm-floating xcb-cursor xcb-render
exwm-layout exwm-workspace exwm-core xcb-ewmh xcb-icccm xcb xcb-xproto
xcb-types xcb-debug server ef-kassio-theme ef-themes
modus-operandi-theme modus-themes zone speed-type url-http url-auth
url-gw nsm ytdious mpdired transmission color calc-bin calc-ext calc
calc-loaddefs rect calc-macs supercite regi ebdb-gnus gnus-msg gnus-art
mm-uu mml2015 mm-view mml-smime smime gnutls dig gnus-sum shr pixel-fill
kinsoku url-file svg dom gnus-group gnus-undo gnus-start gnus-dbus dbus
xml gnus-cloud nnimap nnmail mail-source utf7 nnoo gnus-spec gnus-int
gnus-range gnus-win ebdb-message message yank-media puny rfc822 mml
mml-sec epa epg rfc6068 epg-config mm-decode mm-bodies mm-encode
mail-parse rfc2231 gmm-utils mailheader ebdb-mua ebdb-com crm
ebdb-format ebdb mailabbrev eieio-opt speedbar ezimage dframe find-func
eieio-base timezone icalendar gnus nnheader gnus-util range sendmail
rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils web-mode
derived disp-table erlang-start skeleton cc-mode cc-fonts cc-guess
cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs slime-asdf
grep slime-tramp tramp rx trampver tramp-integration files-x
tramp-message tramp-compat xdg shell pcomplete parse-time iso8601
time-date format-spec tramp-loaddefs slime-fancy slime-indentation
slime-cl-indent cl-indent slime-trace-dialog slime-fontifying-fu
slime-package-fu slime-references slime-compiler-notes-tree advice
slime-scratch slime-presentations bridge slime-macrostep macrostep
compat slime-mdot-fu slime-enclosing-context slime-fuzzy
slime-fancy-trace slime-fancy-inspector slime-c-p-c
slime-editing-commands slime-autodoc slime-repl slime-parse slime
apropos compile text-property-search etags fileloop generator xref
project arc-mode archive-mode noutline outline comint ansi-osc
ansi-color ring hyperspec thingatpt elec-pair edmacro kmacro
use-package-bind-key bind-key appt diary-lib diary-loaddefs cal-menu
calendar cal-loaddefs pcase dired-x dired-aux dired dired-loaddefs
use-package-core repeat easy-mmode cus-edit pp cus-load wid-edit
debbugs-autoloads ebdb-autoloads cl-extra help-mode ef-themes-autoloads
exwm-autoloads hyperbole-autoloads kotl-autoloads hact set hhist
on-screen-autoloads osm-autoloads pdf-tools-autoloads
rust-mode-autoloads info slime-autoloads macrostep-autoloads
speed-type-autoloads tablist-autoloads transmission-autoloads
xelb-autoloads ytdious-autoloads package browse-url url url-proxy
url-privacy url-expand url-methods url-history url-cookie
generate-lisp-file url-domsuf url-util mailcap url-handlers url-parse
auth-source cl-seq eieio eieio-core cl-macs icons password-cache json
subr-x map byte-opt gv bytecomp byte-compile url-vars cl-loaddefs cl-lib
rmc iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook
vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
term/common-win x-dnd touch-screen tool-bar dnd fontset image regexp-opt
fringe tabulated-list replace newcomment text-mode lisp-mode prog-mode
register page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind kqueue lcms2 dynamic-setting system-font-setting
font-render-setting xinput2 x multi-tty move-toolbar
make-network-process emacs)

Memory information:
((conses 16 1839679 363033) (symbols 48 64933 4)
 (strings 32 496457 25800) (string-bytes 1 13760816)
 (vectors 16 193093) (vector-slots 8 2656445 237930)
 (floats 8 1346 9473) (intervals 56 62796 4763) (buffers 992 82))

-- 
Manuel Giraud

bug#73291: 31.0.50; `clean-buffer-list' activates `midnight-mode'

[Eli Zaretskii]

> Date: Mon, 16 Sep 2024 09:49:43 +0200
> From:  Manuel Giraud via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> It seems that calling `clean-buffer-list' once activates
> `midnight-mode'.  How to reproduce it:
> 
>     - emacs -Q
>     - Open some buffers
>     - M-x clean-buffer-list ;; Obviously not much buffers are killed
>     - Wait one day and go into your emacs window
>     - In the *Messages* buffer, you can see that cleaning occured
>       recently
> 
> I think that one call to `clean-buffer-list' should only clean the
> buffer list once.  The documentation does not seem to imply otherwise.

This is because clean-buffer-list is autoloaded, and loading
midnight.el evaluates midnight-delay, which :set function activates
the midnight timer.

I think the solution is to modify midnight-delay-set to activate the
timer only if is already non-nil.  Like this:

diff --git a/lisp/midnight.el b/lisp/midnight.el
index bfa99d6..73d049a 100644
--- a/lisp/midnight.el
+++ b/lisp/midnight.el
@@ -205,10 +205,11 @@ midnight-delay-set
   (cl-assert (eq symb 'midnight-delay) t
              "Invalid argument to `midnight-delay-set': `%s'")
   (set symb tm)
-  (when (timerp midnight-timer) (cancel-timer midnight-timer))
-  (setq midnight-timer
-        (run-at-time (if (numberp tm) (+ (midnight-next) tm) tm)
-                     midnight-period #'run-hooks 'midnight-hook)))
+  (when (timerp midnight-timer)
+    (cancel-timer midnight-timer)
+    (setq midnight-timer
+          (run-at-time (if (numberp tm) (+ (midnight-next) tm) tm)
+                       midnight-period #'run-hooks 'midnight-hook))))
 
 (defcustom midnight-delay 3600
   "The number of seconds after the midnight when the `midnight-timer' is run.

Can you try this and see if it solves the problem, and doesn't break
the other uses of midnight-mode and or clean-buffer-list?

bug#73291: 31.0.50; `clean-buffer-list' activates `midnight-mode'

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

[...]

> diff --git a/lisp/midnight.el b/lisp/midnight.el
> index bfa99d6..73d049a 100644
> --- a/lisp/midnight.el
> +++ b/lisp/midnight.el
> @@ -205,10 +205,11 @@ midnight-delay-set
>    (cl-assert (eq symb 'midnight-delay) t
>               "Invalid argument to `midnight-delay-set': `%s'")
>    (set symb tm)
> -  (when (timerp midnight-timer) (cancel-timer midnight-timer))
> -  (setq midnight-timer
> -        (run-at-time (if (numberp tm) (+ (midnight-next) tm) tm)
> -                     midnight-period #'run-hooks 'midnight-hook)))
> +  (when (timerp midnight-timer)
> +    (cancel-timer midnight-timer)
> +    (setq midnight-timer
> +          (run-at-time (if (numberp tm) (+ (midnight-next) tm) tm)
> +                       midnight-period #'run-hooks 'midnight-hook))))
>  
>  (defcustom midnight-delay 3600
>    "The number of seconds after the midnight when the `midnight-timer' is run.
>
> Can you try this and see if it solves the problem, and doesn't break
> the other uses of midnight-mode and or clean-buffer-list?

Thanks.  It solves the issue of activacting the midnight-mode: now
`clean-buffer-list' is just done once and won't run the next day.  But
now, when I do M-x midnight-mode, I get the following error:

--8<---------------cut here---------------start------------->8---
timer--activate: Invalid or uninitialized timer
--8<---------------cut here---------------end--------------->8---
-- 
Manuel Giraud

bug#73291: 31.0.50; `clean-buffer-list' activates `midnight-mode'

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: 73291@debbugs.gnu.org
> Date: Tue, 17 Sep 2024 10:19:41 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Can you try this and see if it solves the problem, and doesn't break
> > the other uses of midnight-mode and or clean-buffer-list?
> 
> Thanks.  It solves the issue of activacting the midnight-mode: now
> `clean-buffer-list' is just done once and won't run the next day.  But
> now, when I do M-x midnight-mode, I get the following error:
> 
> --8<---------------cut here---------------start------------->8---
> timer--activate: Invalid or uninitialized timer
> --8<---------------cut here---------------end--------------->8---

So not only did it activate the timer when loading the package, it
also relied on that for the mode to work...

The fix is to add

  (midnight-delay-set 'midnight-delay midnight-delay)

to midnight-mode function.

bug#73291: 31.0.50; `clean-buffer-list' activates `midnight-mode'

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Manuel Giraud <manuel@ledu-giraud.fr>
>> Cc: 73291@debbugs.gnu.org
>> Date: Tue, 17 Sep 2024 10:19:41 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > Can you try this and see if it solves the problem, and doesn't break
>> > the other uses of midnight-mode and or clean-buffer-list?
>> 
>> Thanks.  It solves the issue of activacting the midnight-mode: now
>> `clean-buffer-list' is just done once and won't run the next day.  But
>> now, when I do M-x midnight-mode, I get the following error:
>> 
>> --8<---------------cut here---------------start------------->8---
>> timer--activate: Invalid or uninitialized timer
>> --8<---------------cut here---------------end--------------->8---
>
> So not only did it activate the timer when loading the package, it
> also relied on that for the mode to work...
>
> The fix is to add
>
>   (midnight-delay-set 'midnight-delay midnight-delay)
>
> to midnight-mode function.

I tried building with the following patch but I get the same error when
doing M-x midnight-mode.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: midnight.patch --]
[-- Type: text/x-patch, Size: 1202 bytes --]

diff --git a/lisp/midnight.el b/lisp/midnight.el
index 6e1e7b5ccb7..a4b0ac00c83 100644
--- a/lisp/midnight.el
+++ b/lisp/midnight.el
@@ -57,6 +57,7 @@ midnight-mode
   ;; via setting 'midnight-delay', which calls 'midnight-delay-set',
   ;; which starts the timer.
   (when (timerp midnight-timer) (cancel-timer midnight-timer))
+  (midnight-delay-set 'midnight-delay midnight-delay)
   (if midnight-mode (timer-activate midnight-timer)))
 
 ;;; clean-buffer-list stuff
@@ -206,10 +207,11 @@ midnight-delay-set
   (cl-assert (eq symb 'midnight-delay) t
              "Invalid argument to `midnight-delay-set': `%s'")
   (set symb tm)
-  (when (timerp midnight-timer) (cancel-timer midnight-timer))
-  (setq midnight-timer
-        (run-at-time (if (numberp tm) (+ (midnight-next) tm) tm)
-                     midnight-period #'run-hooks 'midnight-hook)))
+  (when (timerp midnight-timer)
+    (cancel-timer midnight-timer)
+    (setq midnight-timer
+          (run-at-time (if (numberp tm) (+ (midnight-next) tm) tm)
+                       midnight-period #'run-hooks 'midnight-hook))))
 
 (defcustom midnight-delay 3600
   "The number of seconds after the midnight when the `midnight-timer' is run.

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

-- 
Manuel Giraud

bug#73291: 31.0.50; `clean-buffer-list' activates `midnight-mode'

[Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Manuel Giraud via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

> > -  (when (timerp midnight-timer) (cancel-timer midnight-timer))
> > -  (setq midnight-timer
> > -        (run-at-time (if (numberp tm) (+ (midnight-next) tm) tm)
> > -                     midnight-period #'run-hooks 'midnight-hook)))
> > +  (when (timerp midnight-timer)
> > +    (cancel-timer midnight-timer)
> > +    (setq midnight-timer
> > +          (run-at-time (if (numberp tm) (+ (midnight-next) tm) tm)
> > +                       midnight-period #'run-hooks 'midnight-hook))))
> >
> >  (defcustom midnight-delay 3600
> >    "The number of seconds after the midnight when the `midnight-timer' is run.
> >
> > Can you try this and see if it solves the problem, and doesn't break
> > the other uses of midnight-mode and or clean-buffer-list?
>
> Thanks.  It solves the issue of activacting the midnight-mode: now
> `clean-buffer-list' is just done once and won't run the next day.  But
> now, when I do M-x midnight-mode, I get the following error:
>
> timer--activate: Invalid or uninitialized timer

Maybe we could instead check whether `midnight-mode' is on, Eli?


Michael.

bug#73291: 31.0.50; `clean-buffer-list' activates `midnight-mode'

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: Eli Zaretskii <eliz@gnu.org>,  Manuel Giraud <manuel@ledu-giraud.fr>,
>   73291@debbugs.gnu.org
> Date: Tue, 17 Sep 2024 15:17:45 +0200
> 
> Manuel Giraud via "Bug reports for GNU Emacs, the Swiss army knife of
> text editors" <bug-gnu-emacs@gnu.org> writes:
> 
> > > -  (when (timerp midnight-timer) (cancel-timer midnight-timer))
> > > -  (setq midnight-timer
> > > -        (run-at-time (if (numberp tm) (+ (midnight-next) tm) tm)
> > > -                     midnight-period #'run-hooks 'midnight-hook)))
> > > +  (when (timerp midnight-timer)
> > > +    (cancel-timer midnight-timer)
> > > +    (setq midnight-timer
> > > +          (run-at-time (if (numberp tm) (+ (midnight-next) tm) tm)
> > > +                       midnight-period #'run-hooks 'midnight-hook))))
> > >
> > >  (defcustom midnight-delay 3600
> > >    "The number of seconds after the midnight when the `midnight-timer' is run.
> > >
> > > Can you try this and see if it solves the problem, and doesn't break
> > > the other uses of midnight-mode and or clean-buffer-list?
> >
> > Thanks.  It solves the issue of activacting the midnight-mode: now
> > `clean-buffer-list' is just done once and won't run the next day.  But
> > now, when I do M-x midnight-mode, I get the following error:
> >
> > timer--activate: Invalid or uninitialized timer
> 
> Maybe we could instead check whether `midnight-mode' is on, Eli?

Sorry, I don't understand what you have in mind: check where?

What needs to be done is to make sure midnight-timer is a timer
(calling timer-create if not) before we call midnight-delay-set,
because it will only initialize the delay if midnight-timer is already
a timer.

Alternatively, we could call run-at-time directly from midnight-mode.

But I guess you meant some third possibility?

bug#73291: 31.0.50; `clean-buffer-list' activates `midnight-mode'

[Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> Alternatively, we could call run-at-time directly from midnight-mode.

Short answer: This would be better.

Long answer: The current design is bad: `midnight-delay-set', the custom
setter of the user option `midnight-delay', is the _only_ place in the
whole library where a timer is ever created.  The mode function itself
does not start a timer (also not indirectly).

`midnight-delay-set' OTOH starts the timer unconditionally, no matter if
`midnight-mode' is on or off.  The timer function also doesn't check
whether the mode is on.

We need to redesign: we should make the mode start and stop the timer.
The setter function should only manipulate the timer, and only when the
mode is really turned on.


Michael.

bug#73291: 31.0.50; `clean-buffer-list' activates `midnight-mode'

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: 73291@debbugs.gnu.org,  manuel@ledu-giraud.fr
> Date: Tue, 17 Sep 2024 21:07:04 +0200
> 
> We need to redesign: we should make the mode start and stop the timer.
> The setter function should only manipulate the timer, and only when the
> mode is really turned on.

I agree; patches to that effect are welcome.

bug#73291: 31.0.50; `clean-buffer-list' activates `midnight-mode'

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Hi,

I think the following patch does the trick.  Here is what I tested with
it:

        - A call to `clean-buffer-list' cleans up the list once and does
          not activate the midnight-timer.

        - The midnight mode is toggled on/off by calling
          `midnight-mode'.

        - Calling (customize-variable 'midnight-delay) stops and
          restarts the midnight-timer with the updated delay.

Maybe the line ":initialize #'custom-initialize-default" could be
removed, I don't know.  WDYT?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: midnight.patch --]
[-- Type: text/x-patch, Size: 1660 bytes --]

diff --git a/lisp/midnight.el b/lisp/midnight.el
index 6e1e7b5ccb7..ff1392abc05 100644
--- a/lisp/midnight.el
+++ b/lisp/midnight.el
@@ -52,12 +52,11 @@ midnight-mode
   "Non-nil means run `midnight-hook' at midnight."
   :global t
   :initialize #'custom-initialize-default
-  ;; Disable first, since the ':initialize' function above already
-  ;; starts the timer when the mode is turned on for the first time,
-  ;; via setting 'midnight-delay', which calls 'midnight-delay-set',
-  ;; which starts the timer.
-  (when (timerp midnight-timer) (cancel-timer midnight-timer))
-  (if midnight-mode (timer-activate midnight-timer)))
+  ;; Call `midnight-delay-set' again because it takes care of starting
+  ;; the timer if the mode is on.  The ':initialize' function above
+  ;; (which ends up calling `midnight-delay-set') did not know yet if
+  ;; the mode was on or not.
+  (midnight-delay-set 'midnight-delay midnight-delay))
 
 ;;; clean-buffer-list stuff
 
@@ -207,9 +206,11 @@ midnight-delay-set
              "Invalid argument to `midnight-delay-set': `%s'")
   (set symb tm)
   (when (timerp midnight-timer) (cancel-timer midnight-timer))
-  (setq midnight-timer
-        (run-at-time (if (numberp tm) (+ (midnight-next) tm) tm)
-                     midnight-period #'run-hooks 'midnight-hook)))
+  ;; Only start the timer if the mode is turned on.
+  (when midnight-mode
+    (setq midnight-timer
+          (run-at-time (if (numberp tm) (+ (midnight-next) tm) tm)
+                       midnight-period #'run-hooks 'midnight-hook))))
 
 (defcustom midnight-delay 3600
   "The number of seconds after the midnight when the `midnight-timer' is run.

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

-- 
Manuel Giraud

bug#73291: 31.0.50; `clean-buffer-list' activates `midnight-mode'

[Stefan Kangas]

Manuel Giraud via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

> I think the following patch does the trick.

Previously, putting this in your init file enabled midnight-mode:

    (require 'midnight)

Now, one needs to say this instead:

    (midnight-mode +1)

Right?  If yes, this change is backwards-incompatible and should be
called out in NEWS.

I still think this is worth doing, because we want to avoid side-effects
like enabling modes just from loading an Emacs Lisp library.  This is
one of few such places that we have left.  Not having midnight-mode on
should also usually not be a very big problem.

Since the latter will work on previous versions of Emacs all the way
back to 25.1, I think we might as well install the below documentation
patch on the emacs-30 branch too.

diff --git a/lisp/midnight.el b/lisp/midnight.el
index bfa99d6a7a8..cc9150ccc9e 100644
--- a/lisp/midnight.el
+++ b/lisp/midnight.el
@@ -23,7 +23,7 @@

 ;;; Commentary:

-;; To use the file, put (require 'midnight) into your .emacs.  Then, at
+;; To use the file, put (midnight-mode) into your .emacs.  Then, at
 ;; midnight, Emacs will run the normal hook `midnight-hook'.  You can
 ;; put whatever you like there, say, `calendar'; by default there is
 ;; only one function there - `clean-buffer-list'.  It will kill the

bug#73291: 31.0.50; `clean-buffer-list' activates `midnight-mode'

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Wed, 18 Sep 2024 16:52:25 -0700
> Cc: Michael Heerdegen <michael_heerdegen@web.de>, 73291@debbugs.gnu.org
> 
> Manuel Giraud via "Bug reports for GNU Emacs, the Swiss army knife of
> text editors" <bug-gnu-emacs@gnu.org> writes:
> 
> > I think the following patch does the trick.
> 
> Previously, putting this in your init file enabled midnight-mode:
> 
>     (require 'midnight)
> 
> Now, one needs to say this instead:
> 
>     (midnight-mode +1)
> 
> Right?  If yes, this change is backwards-incompatible and should be
> called out in NEWS.

Agreed.  The user manual already says (in effect) to use the latter,
so we are good in that department.

> I still think this is worth doing, because we want to avoid side-effects
> like enabling modes just from loading an Emacs Lisp library.  This is
> one of few such places that we have left.  Not having midnight-mode on
> should also usually not be a very big problem.

Yes, agreed.

> Since the latter will work on previous versions of Emacs all the way
> back to 25.1, I think we might as well install the below documentation
> patch on the emacs-30 branch too.

Agreed.

So please install all the other changes on master and this one on the
release branch.

Thanks.