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.

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

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

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

I did the second part.

Manuel, could you please resubmit your patch as formatted by

    git format-patch -1

?

That way, we'll get the attribution right.  Please also add a ChangeLog
entry as always.

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: 337 bytes --]

Stefan Kangas <stefankangas@gmail.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> So please install all the other changes on master and this one on the
>> release branch.
>
> I did the second part.
>
> Manuel, could you please resubmit your patch as formatted by

Here.  I also add the NEWS entry as suggested by Eli.  Thanks.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-clean-buffer-list-don-t-activate-midnight-mode.patch --]
[-- Type: text/x-patch, Size: 2748 bytes --]

From 8d668972fad20b1baba3d7a56f3612e0d7689e25 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Sat, 21 Sep 2024 14:51:21 +0200
Subject: [PATCH] clean-buffer-list don't activate midnight-mode

Bug#73291

* lisp/midnight.el (midnight-mode): Call `midnight-delay-set'
again when mode is activated.
(midnight-delay-set): Only start the timer if the mode is turned
on.
* etc/NEWS: Document the change in behaviour.
---
 etc/NEWS         |  7 +++++++
 lisp/midnight.el | 19 ++++++++++---------
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 6c97ffacd15..46abc908c6e 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -390,6 +390,13 @@ functionality of the standard 'xref' commands in TeX buffers.  You can
 restore the standard 'etags' backend with the 'M-x xref-etags-mode'
 toggle.
 
+** Midnight mode
+
+---
+*** Change for activating the mode
+Putting (require 'midnight) in your init file no longer activates the
+mode.  Now, one needs to say (midnight-mode +1) instead.
+
 \f
 * New Modes and Packages in Emacs 31.1
 
diff --git a/lisp/midnight.el b/lisp/midnight.el
index ea30f5462f2..e5dd7221591 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.
-- 
2.46.0


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

-- 
Manuel Giraud

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

[Stefan Kangas]

Version: 31.1

Manuel Giraud <manuel@ledu-giraud.fr> writes:

> Here.  I also add the NEWS entry as suggested by Eli.  Thanks.

Thanks, pushed to master (commit 4745bafa6a1) with some minor copy
editing of the commit message.

I'm therefore closing this bug report.