savehist and multiple Emacs sessions

[Stefan Monnier <monnier@iro.umontreal.ca>]

It turns out that I almost always have 2 Emacs sessions open at the same
time and the longer-lived one is the Gnus one which hence tends to "win"
w.r.t savehist: when I log in the .emacs.d/history file only holds the
history info from my last Gnus session, which lacks a lot of history info.

So I think it's worth it to make savehist.el handle such situations better.
The patch below does just that.  With it, Emacs will automatically notice
when the file has been changed by some other process and reload the history
file, merging the new data without losing the old one.

This also makes it safe (in the sense that it won't lose history data) to
call savehist-load at any point in an Emacs session.

It's only be lightly tested.  It also includes the changes to the minor mode
definition that make it handle Custom setting slightly better and which make
it run the minor mode hook in the same way as all other minor modes defined
by define-minor-mode.


        Stefan


--- orig/lisp/savehist.el
+++ mod/lisp/savehist.el
@@ -170,8 +169,9 @@
 
 (defvar savehist-loaded nil
   "Whether the history has already been loaded.
-This prevents toggling savehist-mode from destroying existing
-minibuffer history.")
+This prevents toggling `savehist-mode' from destroying existing
+minibuffer history.  If non-nil, holds the modtime when the file and
+Emacs's internal variables where last synchronized.")
 
 (when (featurep 'xemacs)
   ;; Must declare this under XEmacs, which doesn't have built-in
@@ -198,53 +198,75 @@
 	  (> (prefix-numeric-value arg) 0)))
   (if (not savehist-mode)
       (savehist-uninstall)
-    (when (and (not savehist-loaded)
-	       (file-exists-p savehist-file))
-      (condition-case errvar
-	  (progn
-	    ;; Don't set coding-system-for-read -- we rely on the
-	    ;; coding cookie to convey that information.  That way, if
-	    ;; the user changes the value of savehist-coding-system,
-	    ;; we can still correctly load the old file.
-	    (load savehist-file nil (not (interactive-p)))
-	    (setq savehist-loaded t))
-	(error
-	 ;; Don't install the mode if reading failed.  Doing so would
-	 ;; effectively destroy the user's data at the next save.
-	 (setq savehist-mode nil)
-	 (savehist-uninstall)
-	 (signal (car errvar) (cdr errvar)))))
-    (savehist-install)
-    (run-hooks 'savehist-mode-hook))
+    (savehist-load)
+    (savehist-install))
+  (if (interactive-p) (customize-mark-as-set 'savehist-mode))
+  (run-hooks 'savehist-mode-hook)
   ;; Return the new setting.
   savehist-mode)
-(add-minor-mode 'savehist-mode "")
+(add-minor-mode 'savehist-mode nil)
 
 (defun savehist-load ()
   "Obsolete function provided for transition from old versions of savehist.
 Don't call this from new code, use (savehist-mode 1) instead.
 
 This function loads the variables stored in `savehist-file' and turns on
-savehist-mode.  If savehist-file is in the old format that doesn't record
+`savehist-mode'.  If savehist-file is in the old format that doesn't record
 the value of `savehist-minibuffer-history-variables', that value is
 deducted from the contents of the file."
-  (savehist-mode 1)
-  ;; Old versions of savehist distributed with XEmacs didn't save
-  ;; savehist-minibuffer-history-variables.  If that variable is nil
-  ;; after loading the file, try to intuit the intended value.
-  (when (null savehist-minibuffer-history-variables)
-    (setq savehist-minibuffer-history-variables
-          (with-temp-buffer
-	    (ignore-errors
-	      (insert-file-contents savehist-file))
-            (let ((vars ()) form)
-              (while (setq form (condition-case nil
-				    (read (current-buffer)) (error nil)))
-		;; Each form read is of the form (setq VAR VALUE).
-		;; Collect VAR, i.e. (nth form 1).
-                (push (nth 1 form) vars))
-              vars)))))
-(make-obsolete 'savehist-load 'savehist-mode)
+  (let ((savehist-old-minibuffer-history-variables
+         (mapcar (lambda (s) (and (boundp s) (cons s (symbol-value s))))
+                 (cons 'savehist-minibuffer-history-variables
+                       savehist-minibuffer-history-variables))))
+    (when (and (file-exists-p savehist-file)
+               (not (equal savehist-loaded
+                           (nth 5 (file-attributes savehist-file)))))
+      (condition-case errvar
+          (progn
+            ;; Don't set coding-system-for-read -- we rely on the
+            ;; coding cookie to convey that information.  That way, if
+            ;; the user changes the value of savehist-coding-system,
+            ;; we can still correctly load the old file.
+            (load savehist-file nil (not (interactive-p)))
+            (setq savehist-loaded (nth 5 (file-attributes savehist-file))))
+        (error
+         ;; Don't install the mode if reading failed.  Doing so would
+         ;; effectively destroy the user's data at the next save.
+         (setq savehist-mode nil)
+         (savehist-uninstall)
+         (signal (car errvar) (cdr errvar)))))
+
+    (when (null savehist-minibuffer-history-variables)
+      ;; Old versions of savehist distributed with XEmacs didn't save
+      ;; savehist-minibuffer-history-variables.  If that variable is nil
+      ;; after loading the file, try to intuit the intended value.
+      (setq savehist-minibuffer-history-variables
+            (with-temp-buffer
+              (ignore-errors
+                (insert-file-contents savehist-file))
+              (let ((vars ()) form)
+                (while (setq form (ignore-errors (read (current-buffer))))
+                  ;; Each form read is of the form (setq VAR VALUE).
+                  ;; Collect VAR, i.e. (nth form 1).
+                  (push (nth 1 form) vars))
+                vars))))
+
+    ;; In case we're loading the file late, there was info in the history
+    ;; variables that may have been overwritten by the info extracted from
+    ;; the file, so put it back in.
+    (dolist (sv savehist-old-minibuffer-history-variables)
+      ;; For each histvar that we knew about, make sure all the entries that
+      ;; were there before are still here now and in the same order.
+      (ignore-errors            ; Maybe some var is not a list or something.
+        (let ((s (car sv))
+              (v (cdr sv))
+              (newv (symbol-value s)))
+          (unless (equal newv v)       ;Simple optimization.
+            (dolist (x v) (setq newv (delete x newv)))
+            (set s (append v newv)))))))
+
+  ;; If it's called the old obsolete way, turn on savehist-mode.
+  (unless savehist-mode (savehist-mode 1)))
 
 (defun savehist-install ()
   "Hook savehist into Emacs.
@@ -283,6 +305,12 @@
 If AUTO-SAVE is non-nil, compare the saved contents to the one last saved,
  and don't save the buffer if they are the same."
   (interactive)
+  (unless (equal savehist-loaded (nth 5 (file-attributes savehist-file)))
+    ;; The file has been changed since last time we touched it.
+    ;; Probably some other Emacs session.  Load the corresponding info so we
+    ;; don't end up throwing it away by blindly overwriting it.  There's
+    ;; still a race-condition, but I don't think it's that important.
+    (savehist-load))
   (with-temp-buffer
     (insert
      (format ";; -*- mode: emacs-lisp; coding: %s -*-\n" savehist-coding-system)
@@ -328,6 +356,7 @@
 			(unless (interactive-p) 'quiet)))
 	(when savehist-file-modes
 	  (set-file-modes savehist-file savehist-file-modes))
+        (setq savehist-loaded (nth 5 (file-attributes savehist-file)))
 	(setq savehist-last-checksum checksum)))))
 
 (defun savehist-autosave ()