bug#79665: 31.0.50; ERC 5.7: Only perform erc-log-insert-log-on-open setup once

"J.P." <[email protected]> Mon, 20 Oct 2025 19:36:43 -0700
Newsgroups gmane.emacs.erc.general
Message-ID <[email protected]>
--=-=-=
Content-Type: text/plain

Tags: patch

This change aims to make the option `erc-log-insert-log-on-open' more
usable by expanding its type to include a predicate function and by
providing one such predicate to cover the most popular use case. As with
anything involving chat logs, it's probably best to exercise extra
caution here, so input from anyone familiar with the option or the
module is most welcome.

AFAICT, the main issue with this option is that its implementation is
hampered by the fact that the module itself tries to make its own
functionality available buffer-locally, in a bid to gain some of the
flexibility now offered by local modules. In doing so, the setup code
responsible for inserting logs on behalf of `erc-log-insert-log-on-open'
ends up potentially running multiple times per session, when it should
only be doing so once.

Users annoyed at seeing the same overlapping log contents inserted
back-to-back atop their buffers can do

  (setopt erc-log-insert-log-on-open 'erc-log-new-target-buffer-p)

which should ensure the insertion code only runs once, when a target
buffer is first created. It also suppresses insertion completely in
server buffers, which most users probably prefer, I'm guessing. The
behavior associated with the option's old "on" value of t should remain
unchanged, so anyone with a decades old log-rotation scheme designed to
counteract such redundancy shouldn't be affected.

This patch also fixes a related issue in which `erc-last-saved-position'
gets stuck after the prompt, thus preventing logs from being saved when
`erc-log-insert-log-on-open' is non-nil. Thanks to Libera user Lionyx
for reporting this, which has likely been around since at least ERC 5.4
and Emacs 28.1. Based on the nature of 5caa4dea "Don't insert erc logs
at the end", which touches adjacent code, I'm speculating that the issue
may have been introduced sometime afterwards, which would put it around
Emacs 25-ish.



In GNU Emacs 31.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.49, cairo version 1.18.2) of 2025-10-20 built on localhost
Repository revision: 72255c99925b7856c71f1655c53d68473d87f803
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12401008
System Description: Fedora Linux 42 (Workstation Edition)

Configured using:
 'configure --enable-check-lisp-object-type --enable-checking=yes,glyphs
 'CFLAGS=-O0 -g3'
 PKG_CONFIG_PATH=:/usr/lib64/pkgconfig:/usr/share/pkgconfig'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NATIVE_COMP
NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS TREE_SITTER WEBP X11 XDBE XIM XINERAMA XINPUT2 XPM
XRANDR GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-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-nonselected-mode: t
  minibuffer-regexp-mode: 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:
None found.

Features:
(shadow sort mail-extr erc auth-source cl-seq eieio eieio-core cl-macs
icons json map format-spec erc-backend erc-networks byte-opt gv
erc-common erc-compat compat erc-loaddefs compile comint ansi-osc
ansi-color ring comp-run bytecomp byte-compile comp-common rx emacsbug
lisp-mnt message mailcap yank-media puny dired dired-loaddefs rfc822 mml
mml-sec password-cache epa derived epg rfc6068 epg-config gnus-util
text-property-search time-date subr-x mm-decode mm-bodies mm-encode
mailabbrev gmm-utils mailheader cl-loaddefs cl-lib sendmail mail-parse
rfc2231 rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils 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 inotify lcms2 dynamic-setting system-font-setting
font-render-setting cairo gtk x-toolkit xinput2 x multi-tty move-toolbar
make-network-process tty-child-frames native-compile emacs)

Memory information:
((conses 16 95908 10301) (symbols 48 10150 0) (strings 32 28134 4447)
 (string-bytes 1 836666) (vectors 16 19082)
 (vector-slots 8 194269 9450) (floats 8 30 1) (intervals 56 347 0)
 (buffers 1064 12))


--=-=-=
Content-Type: text/x-patch
Content-Disposition: attachment;
 filename=0001-5.7-Only-perform-erc-log-insert-log-on-open-setup-on.patch

From 424ffe9299de15f5528b7ff4fdcb00e333c32d4e Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <[email protected]>
Date: Sun, 19 Oct 2025 05:29:18 -0700
Subject: [PATCH] [5.7] Only perform erc-log-insert-log-on-open setup once

FIXME: when new section is added to ERC-NEWS for 5.7, add entry like:

** Improved behavior for option 'erc-log-insert-log-on-open'.

Rather than insert redundant logs into all buffers when reconnecting,
which is what currently happens when setting the option to t, ERC 5.7
allows users to exercise more discernment by specifying a predicate.
The provided 'erc-log-new-target-buffer-p' tells ERC to only insert logs
when creating a new target buffer, such as when issuing a /JOIN or a
/QUERY or when connecting for the first time with autojoin configured.

And in the library API section, add something like:

*** Function 'erc-log-setup-logging' deprecated.
The function performed destructive tasks that should only run once per
session, yet it was used to perform module setup in contexts that expect
nondestructive, repeated execution.

* etc/ERC-NEWS: Add entry for option `erc-log-insert-log-on-open'.
* lisp/erc/erc-log.el (erc-log-insert-log-on-open): Expand type from
boolean to choice of boolean, predicate, and function item
`erc-log-new-target-buffer-p'.
(erc-log-mode, erc-log-enable, erc-log-disable): Replace
`erc-log-setup-logging' on `erc-connect-pre-hook' with
`erc-log--insert-log-on-open'.  Replace calls to `erc-log-setup-logging'
with ones to `erc-log--setup'.
(erc-log-new-target-buffer-p): New function.  While the name could
perhaps do more to indicate that it's only useful when called from
`erc-open', that's the only place ERC typically sets up new target
buffers.
(erc-log-setup-logging): Deprecate and replace body with adapter that
calls `erc-log--setup' and `erc-log--insert-log-on-open'.
(erc-log--setup): New function whose body is mostly from the
nondestructive portion of `erc-log-setup-logging'.
(erc-log--insert-log-on-open): New function whose body is mostly from
the destructive portion of `erc-log-setup-logging'.  Unlike the old one,
if `erc-log-insert-log-on-open' is a function, call it instead of taking
its symbol value when deciding whether to insert a log.  Also, don't
leave `erc-last-saved-position' marker after the prompt, where it gets
stuck and prevents logs from being saved when the option
`erc-log-insert-log-on-open' is non-nil.  Thanks to Libera user Lionyx
for reporting this bug, which has been around since at least ERC 5.4 and
Emacs 28.1.
---
 lisp/erc/erc-log.el | 71 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 56 insertions(+), 15 deletions(-)

diff --git a/lisp/erc/erc-log.el b/lisp/erc/erc-log.el
index 173e6724a8f..9730a02e4cd 100644
--- a/lisp/erc/erc-log.el
+++ b/lisp/erc/erc-log.el
@@ -157,8 +157,17 @@ erc-log-channels-directory
 		 (const :tag "Disable logging" nil)))
 
 (defcustom erc-log-insert-log-on-open nil
-  "Insert log file contents into the buffer if a log file exists."
-  :type 'boolean)
+  "Insert an existing log file's contents into its associated buffer.
+A legacy value of t does so upon connecting and reconnecting in all
+buffers, often with overlapping, redundant content.  A value of
+`erc-log-new-target-buffer-p' does so in new target buffers only.  If
+set to an arbitrary predicate, ERC calls it with no args in the
+associated buffer."
+  :package-version '("ERC" . "5.7") ; FIXME sync on release
+  :type '(choice boolean
+                 (function-item :tag "Only new target buffers"
+                                erc-log-new-target-buffer-p)
+                 (function "User-defined predicate taking no args")))
 
 (defcustom erc-save-buffer-on-part t
   "Save the channel buffer content using `erc-save-buffer-in-logs' on PART.
@@ -231,10 +240,9 @@ log
    (add-hook 'erc-quit-hook #'erc-conditional-save-queries)
    (add-hook 'erc-part-hook #'erc-conditional-save-buffer)
    ;; append, so that 'erc-initialize-log-marker runs first
-   (add-hook 'erc-connect-pre-hook #'erc-log-setup-logging 'append)
-   ;; FIXME use proper local "setup" function and major-mode hook.
-   (dolist (buffer (erc-buffer-list))
-     (erc-log-setup-logging buffer))
+   (add-hook 'erc-connect-pre-hook #'erc-log--insert-log-on-open 80)
+   (add-hook 'erc-mode-hook #'erc-log--setup)
+   (unless erc--updating-modules-p (erc-buffer-do #'erc-log--setup))
    (erc--modify-local-map t "C-c C-l" #'erc-save-buffer-in-logs))
   ;; disable
   ((remove-hook 'erc-insert-post-hook #'erc-save-buffer-in-logs)
@@ -244,36 +252,69 @@ log
    (remove-hook 'kill-emacs-hook #'erc-log-save-all-buffers)
    (remove-hook 'erc-quit-hook #'erc-conditional-save-queries)
    (remove-hook 'erc-part-hook #'erc-conditional-save-buffer)
-   (remove-hook 'erc-connect-pre-hook #'erc-log-setup-logging)
-   (dolist (buffer (erc-buffer-list))
-     (erc-log-disable-logging buffer))
+   (remove-hook 'erc-connect-pre-hook #'erc-log--insert-log-on-open)
+   (remove-hook 'erc-mode-hook #'erc-log--setup)
+   (erc-buffer-do #'erc-log--setup)
    (erc--modify-local-map nil "C-c C-l" #'erc-save-buffer-in-logs)))
 
-;;; functionality referenced from erc.el
+(defun erc-log-new-target-buffer-p ()
+  "Return non-nil during `erc-open' if the current buffer is a new target.
+That is, return nil if it's a server buffer or a target being
+reassociated from a previous session."
+  (and (erc-target) (null erc--target-priors)))
+
+;; This function served double duty as the local setup function for both
+;; idempotent tasks and destructive ones typically confined to
+;; `erc-open'.  The caller was implicitly tasked with selectively
+;; inhibiting the destructive portion by binding
+;; `erc-log-insert-log-on-open' to nil when calling it, which led to
+;; bugs.
 (defun erc-log-setup-logging (buffer)
   "Setup the buffer-local logging variables in the current buffer.
 This function is destined to be run from `erc-connect-pre-hook'.
 The current buffer is given by BUFFER."
-  (when (erc-logging-enabled buffer)
-    (with-current-buffer buffer
+  (declare (obsolete "use `erc-log-mode' or mimic `erc-log--setup'" "31.1"))
+  (with-current-buffer buffer
+    (let ((erc-log-mode t))
+      (erc-log--setup)
+      (erc-log--insert-log-on-open))))
+
+;; This module's differs from other global modules in that it allows for
+;; effectively disabling itself in a subset of buffers by setting the
+;; option `erc-enable-logging' locally to nil.  Though not
+;; permanent-local, this option's variable is never explicitly killed
+;; when the module is disabled, such as via its mode command.
+(defun erc-log--setup ()
+  "Perform buffer-local setup for ERC's log module."
+  (if erc-log-mode
+    (when (erc-logging-enabled)
       (auto-save-mode -1)
       (setq buffer-file-name nil)
       (add-hook 'write-file-functions #'erc-save-buffer-in-logs nil t)
       (add-function :before (local 'erc--clear-function)
-                    #'erc-log--save-on-clear '((depth . 50)))
-      (when erc-log-insert-log-on-open
+                    #'erc-log--save-on-clear '((depth . 50))))
+    (erc-log-disable-logging (current-buffer))))
+
+(defun erc-log--insert-log-on-open (&rest _)
+  "Conditionally perform insertion for `erc-log-insert-log-on-open'."
+  (when (if (functionp erc-log-insert-log-on-open)
+            (funcall erc-log-insert-log-on-open)
+          erc-log-insert-log-on-open)
+    (with-silent-modifications
+      (progn
 	(ignore-errors
 	  (save-excursion
 	    (goto-char (point-min))
 	    (insert-file-contents (erc-current-logfile)))
 	  (move-marker erc-last-saved-position
-		       (1- (point-max))))))))
+                       (1- (marker-position erc-insert-marker))))))))
 
 (defun erc-log-disable-logging (buffer)
   "Disable logging in BUFFER."
   (when (erc-logging-enabled buffer)
     (with-current-buffer buffer
       (remove-function (local 'erc--clear-function) #'erc-log--save-on-clear)
+      (remove-hook 'write-file-functions #'erc-save-buffer-in-logs t)
       (setq buffer-offer-save nil
 	    erc-enable-logging nil))))
 
-- 
2.51.0


--=-=-=--