RE: 23.0.50; savehist save invalid syntax

["Drew Adams" <drew.adams@oracle.com>]

[-- Attachment #1: Type: text/plain, Size: 1472 bytes --]

> > 1. The most common problem I've run into here is that
> > strings with text properties are printed so that they
> > cannot be read - for example:
> > #("foobar" 0 6 (face font-lock-comment-face)).
>
> That string can be read back just fine.

Hmm, you're right. It must have been something else with # that was
sometimes getting into my saved histories and provoking a load error. But
what's odd is that deleting those strings seemed to remove the error. Oh
well, I must have missed something along the way.

I looked a bit closer at the rest as well, and I think that
`savehist-prin1-readable' needs to actually read what it prints (like it did
before Davis's patch). It should raise an error (hence remove the written
entry and return nil) if either the write or the read fails.

Emacs 20 serves as a good test for this, because it has a bug: If you do
`M-x cancel-debug-on-entry RET', then Emacs 20 inserts this invalid entry in
the `command-history': (cancel-debug-on-entry ') - note the quote mark
before the right paren. This provoked a read error at load time, but the
attached patch correctly does not include that invalid entry in the saved
`command-history'.

[savehist.el does not actually work with Emacs 20, for other reasons (e.g.
md5), but I have a version that does work, and I used that to test
`savehist-prin1-readable'.]

The attached patch fixes this and the other problems that I noted earlier
(and it does not remove text properties).

HTH - Drew

[-- Attachment #2: savehist-2007-09-09b.patch --]
[-- Type: application/octet-stream, Size: 7662 bytes --]

*** savehist-CVS-2007-09-09a.el	Sun Sep  9 09:32:10 2007
--- savehist-patched-2007-09-09b.el	Sun Sep  9 19:49:06 2007
***************
*** 79,86 ****
  
  (defcustom savehist-additional-variables ()
    "*List of additional variables to save.
! Each element is a symbol whose value will be persisted across Emacs
! sessions that use savehist.  The contents of variables should be
  printable with the Lisp printer.  You don't need to add minibuffer
  history variables to this list, all minibuffer histories will be
  saved automatically as long as `savehist-save-minibuffer-history' is
--- 79,86 ----
  
  (defcustom savehist-additional-variables ()
    "*List of additional variables to save.
! Each element is a symbol whose value is persisted across Emacs
! sessions that use `savehist'.  The contents of variables should be
  printable with the Lisp printer.  You don't need to add minibuffer
  history variables to this list, all minibuffer histories will be
  saved automatically as long as `savehist-save-minibuffer-history' is
***************
*** 118,124 ****
    :type 'file
    :group 'savehist)
  
! (defcustom savehist-file-modes #o600
    "*Default permissions of the history file.
  This is decimal, not octal.  The default is 384 (0600 in octal).
  Set to nil to use the default permissions that Emacs uses, typically
--- 118,124 ----
    :type 'file
    :group 'savehist)
  
! (defcustom savehist-file-modes 384      ; Octal: #o600
    "*Default permissions of the history file.
  This is decimal, not octal.  The default is 384 (0600 in octal).
  Set to nil to use the default permissions that Emacs uses, typically
***************
*** 237,243 ****
  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
--- 237,243 ----
  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
***************
*** 299,327 ****
  	  (print-string-length nil)
  	  (print-level nil)
  	  (print-readably t)
! 	  (print-quoted t))
        ;; Save the minibuffer histories, along with the value of
        ;; savehist-minibuffer-history-variables itself.
        (when savehist-save-minibuffer-history
  	(prin1 `(setq savehist-minibuffer-history-variables
! 		      ',savehist-minibuffer-history-variables)
! 	       (current-buffer))
  	(insert ?\n)
  	(dolist (symbol savehist-minibuffer-history-variables)
  	  (when (boundp symbol)
  	    (let ((value (savehist-trim-history (symbol-value symbol))))
! 	      (when value		; don't save empty histories
! 		(prin1 `(setq ,symbol ',value) (current-buffer))
! 		(insert ?\n))))))
        ;; Save the additional variables.
        (dolist (symbol savehist-additional-variables)
  	(when (boundp symbol)
! 	  (let ((value (symbol-value symbol)))
! 	    (when (savehist-printable value)
! 	      (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-no-conversion)))
        (unless (and auto-save (equal checksum savehist-last-checksum))
  	;; Set file-precious-flag when saving the buffer because we
--- 299,328 ----
  	  (print-string-length nil)
  	  (print-level nil)
  	  (print-readably t)
! 	  (print-quoted t)
!           (print-unreadable-function t)
!           (standard-output (current-buffer)))
        ;; Save the minibuffer histories, along with the value of
        ;; savehist-minibuffer-history-variables itself.
        (when savehist-save-minibuffer-history
  	(prin1 `(setq savehist-minibuffer-history-variables
!                  ',savehist-minibuffer-history-variables))
  	(insert ?\n)
  	(dolist (symbol savehist-minibuffer-history-variables)
  	  (when (boundp symbol)
  	    (let ((value (savehist-trim-history (symbol-value symbol))))
!               (when value		; Don't save empty histories.
!  		(insert "(setq ") (prin1 symbol) (insert " '(")
!  		(while value
!  		  (savehist-prin1-readable (car value))
!                   (setq value (cdr value)) (insert " "))
!  		(insert "))\n"))))))
        ;; Save the additional variables.
        (dolist (symbol savehist-additional-variables)
  	(when (boundp symbol)
!           (savehist-prin1-readable `(setq ,symbol ',(symbol-value symbol)))
!           (insert ?\n))))
!     ;; If autosaving, avoid writing if nothing has changed since the last write.
      (let ((checksum (md5 (current-buffer) nil nil savehist-no-conversion)))
        (unless (and auto-save (equal checksum savehist-last-checksum))
  	;; Set file-precious-flag when saving the buffer because we
***************
*** 340,347 ****
  (defun savehist-autosave ()
    "Save the minibuffer history if it has been modified since the last save.
  Does nothing if `savehist-mode' is off."
!   (when savehist-mode
!     (savehist-save t)))
  
  (defun savehist-trim-history (value)
    "Retain only the first `history-length' items in VALUE.
--- 341,347 ----
  (defun savehist-autosave ()
    "Save the minibuffer history if it has been modified since the last save.
  Does nothing if `savehist-mode' is off."
!   (condition-case nil (when savehist-mode (savehist-save t)) (error nil)))
  
  (defun savehist-trim-history (value)
    "Retain only the first `history-length' items in VALUE.
***************
*** 355,380 ****
        (loop repeat history-length collect (pop value))
      value))
  
! (defun savehist-printable (value)
!   "Return non-nil if VALUE is printable."
!   (cond
!    ;; Quick response for oft-encountered types known to be printable.
!    ((stringp value))
!    ((numberp value))
!    ((symbolp value))
!    (t
!     ;; For others, check explicitly.
!     (with-temp-buffer
        (condition-case nil
! 	  (let ((print-readably t) (print-level nil))
! 	  ;; Print the value into a buffer...
  	  (prin1 value (current-buffer))
! 	  ;; ...and attempt to read it.
! 	  (read (point-min-marker))
! 	  ;; The attempt worked: the object is printable.
  	  t)
! 	;; The attempt failed: the object is not printable.
! 	(error nil))))))
  
  (defun savehist-minibuffer-hook ()
    (unless (or (eq minibuffer-history-variable t)
--- 355,372 ----
        (loop repeat history-length collect (pop value))
      value))
  
! (defun savehist-prin1-readable (value)
!   "Print VALUE in the current buffer, but only if it's also readable.
! Return non-nil if it was printed."
!   (let ((opoint (copy-marker (point)))
!         (opoint-cpy (copy-marker (point))))
      (condition-case nil
! 	(progn ;;$$$ (when (stringp value) (setq value (format "%s" value)))
                 (prin1 value (current-buffer))
!                (read opoint)
                 t)
!       (error (delete-region opoint-cpy (point))
!              nil))))
  
  (defun savehist-minibuffer-hook ()
    (unless (or (eq minibuffer-history-variable t)

Diff finished at Sun Sep 09 19:50:32

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

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel