Eliminate savehist duplicated symbols

* lisp/savehist.el (savehist-save): Do not save symbols
duplicated between 'savehist-minibuffer-history-variables'
and 'savehist-additional-variables'.  (Bug#76123)
* test/lisp/savehist-tests.el: New file.

(cherry picked from commit 3123562866130d4ab45c5af7b80aaf0815060ce5)

diff --git a/lisp/savehist.el b/lisp/savehist.el
index 7cfb358dd6d40fc9d209ab0161c6f3b3ce02e465..32985bac0e1a782f8cd6ea50d1a781aa719d4e68 100644 (file)
--- a/lisp/savehist.el
+++ b/lisp/savehist.el
@@ -299,20 +299,21 @@ If AUTO-SAVE is non-nil, compare the saved contents to the one last saved,
                (insert "))\n"))))))
       ;; Save the additional variables.
       (dolist (elem savehist-additional-variables)
-        (let ((symbol (if (consp elem)
-                          (car elem)
-                        elem)))
-         (when (boundp symbol)
-           (let ((value (symbol-value symbol)))
-             (when (savehist-printable value)
-                ;; When we have a max-size, chop off the last elements.
-                (when (and (consp elem)
-                           (listp value)
-                           (length> value (cdr elem)))
-                  (setq value (copy-sequence value))
-                  (setcdr (nthcdr (cdr elem) value) nil))
-               (prin1 `(setq ,symbol ',value) (current-buffer))
-               (insert ?\n)))))))
+        (when (not (memq elem savehist-minibuffer-history-variables))
+          (let ((symbol (if (consp elem)
+                            (car elem)
+                          elem)))
+           (when (boundp symbol)
+             (let ((value (symbol-value symbol)))
+               (when (savehist-printable value)
+                  ;; When we have a max-size, chop off the last elements.
+                  (when (and (consp elem)
+                             (listp value)
+                             (length> value (cdr elem)))
+                    (setq value (copy-sequence value))
+                    (setcdr (nthcdr (cdr elem) value) nil))
+                 (prin1 `(setq ,symbol ',value) (current-buffer))
+                 (insert ?\n))))))))
     ;; If autosaving, avoid writing if nothing has changed since the
     ;; last write.
     (let ((checksum (md5 (current-buffer) nil nil savehist-coding-system)))

diff --git a/test/lisp/savehist-tests.el b/test/lisp/savehist-tests.el
new file mode 100644 (file)
index 0000000..e37dcec
--- /dev/null
+++ b/test/lisp/savehist-tests.el
@@ -0,0 +1,104 @@
+;;; savehist-tests.el --- Tests for savehist.el  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2025 Free Software Foundation, Inc.
+
+;; Author: Stephane Marks <shipmints@gmail.com>
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; These tests emulate what `read-from-minibuffer' would do via
+;; `savehist-minibuffer-hook' without calling `read-from-minibuffer'.
+
+;;; Code:
+
+(require 'ert)
+(require 'ert-x)
+(require 'savehist)
+
+(ert-deftest savehist-test-saved-variables ()
+  ;; These accommodate symbol-value.
+  (defvar t1)
+  (defvar t2)
+  (ert-with-temp-file tmpfile
+    (let* ((savehist-file tmpfile)
+           (savehist-save-minibuffer-history t)
+           (savehist-save-hook)
+           (savehist-loaded)
+           (savehist-minibuffer-history-variables)
+           (savehist-additional-variables '(t2))
+           (savehist-ignored-variables '(t3))
+           (t1 '("t1-value"))
+           (t2 '("t2-value"))
+           (t3 '("t3-value"))
+           (t1-copy (copy-tree t1))
+           (t2-copy (copy-tree t2))
+           (t3-copy (copy-tree t3))
+           (save-var (lambda (x)
+                       (let ((minibuffer-history-variable x))
+                         (savehist-minibuffer-hook)))))
+      (savehist-mode)
+      (funcall save-var 't1)
+      (funcall save-var 't2)
+      (funcall save-var 't3) ; should be ignored
+      (savehist-save)
+      (setq t1 nil t2 nil t3 nil)
+      (progn
+        ;; Force reloading the file.
+        (savehist-mode -1)
+        (setq savehist-loaded nil)
+        (savehist-mode))
+      (should (equal t1 t1-copy))
+      (should (equal t2 t2-copy))
+      (should (equal t3 nil)))))
+
+(ert-deftest savehist-test-duplicated-saved-symbols ()
+  (defvar t1)
+  (defvar t2)
+  (ert-with-temp-file tmpfile
+    (let* ((savehist-file tmpfile)
+           (savehist-save-minibuffer-history t)
+           (savehist-save-hook)
+           (savehist-loaded)
+           (savehist-minibuffer-history-variables) ; will be '(t2 t1)
+           (savehist-additional-variables '(t2)) ; t2 should not be saved twice
+           (t1 '("t1-value"))
+           (t2 '("t2-value"))
+           (save-var (lambda (x)
+                       (let ((minibuffer-history-variable x))
+                         (savehist-minibuffer-hook)))))
+      (savehist-mode)
+      (funcall save-var 't1)
+      (funcall save-var 't2)
+      (savehist-save)
+      (progn
+        ;; Force reloading the file.
+        (savehist-mode -1)
+        (setq savehist-loaded nil)
+        (savehist-mode))
+      (let ((saved-variables))
+        (with-temp-buffer
+          (insert-file-contents tmpfile)
+          (goto-char 1)
+          ;; alnum bypasses savehist-minibuffer-history-variables
+          (while (re-search-forward "(setq \\([[:alnum:]]+\\) " nil t 1)
+            (push (match-string 1) saved-variables)))
+        (should (= (length saved-variables)
+                   (length (seq-uniq saved-variables #'equal))))))))
+
+(provide 'savehist-tests)
+;;; savehist-tests.el ends here