From fea8d9471b1e4c4b92d904e6f16c0377e159eb9b Mon Sep 17 00:00:00 2001 From: shipmints Date: Fri, 7 Feb 2025 14:25:50 -0500 Subject: [PATCH] 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) --- lisp/savehist.el | 29 +++++----- test/lisp/savehist-tests.el | 104 ++++++++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 14 deletions(-) create mode 100644 test/lisp/savehist-tests.el diff --git a/lisp/savehist.el b/lisp/savehist.el index 7cfb358dd6d..32985bac0e1 100644 --- 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 index 00000000000..e37dceccc10 --- /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 + +;; 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 . + +;;; 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 -- 2.39.5