From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: "Drew Adams" Newsgroups: gmane.emacs.devel,gmane.emacs.pretest.bugs Subject: RE: 23.0.50; savehist save invalid syntax Date: Sun, 9 Sep 2007 14:58:19 -0700 Message-ID: References: <60848.128.165.123.18.1189016180.squirrel@webmail.lanl.gov> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_NextPart_000_0030_01C7F2F1.DBFEFDB0" X-Trace: sea.gmane.org 1189376235 18653 80.91.229.12 (9 Sep 2007 22:17:15 GMT) X-Complaints-To: usenet@sea.gmane.org NNTP-Posting-Date: Sun, 9 Sep 2007 22:17:15 +0000 (UTC) Cc: emacs-pretest-bug@gnu.org, sdl.web@gmail.com To: , Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon Sep 10 08:17:01 2007 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.50) id 1IUcIV-0001lm-5a for ged-emacs-devel@m.gmane.org; Mon, 10 Sep 2007 07:59:00 +0200 Original-Received: from localhost ([127.0.0.1] helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1IUUo8-0005gA-BS for ged-emacs-devel@m.gmane.org; Sun, 09 Sep 2007 17:59:08 -0400 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1IUUo4-0005eb-7c for emacs-devel@gnu.org; Sun, 09 Sep 2007 17:59:04 -0400 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1IUUo2-0005bi-Nk for emacs-devel@gnu.org; Sun, 09 Sep 2007 17:59:02 -0400 Original-Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1IUUo2-0005ba-Da for emacs-devel@gnu.org; Sun, 09 Sep 2007 17:59:02 -0400 Original-Received: from fencepost.gnu.org ([140.186.70.10]) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1IUUo2-0003UY-0P for emacs-devel@gnu.org; Sun, 09 Sep 2007 17:59:02 -0400 Original-Received: from monty-python.gnu.org ([199.232.76.173]) by fencepost.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1IUUnf-00008F-Q0 for emacs-pretest-bug@gnu.org; Sun, 09 Sep 2007 17:58:39 -0400 Original-Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1IUUny-0003UA-5h for emacs-pretest-bug@gnu.org; Sun, 09 Sep 2007 17:59:01 -0400 Original-Received: from agminet01.oracle.com ([141.146.126.228]) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1IUUnt-0003TN-Lg; Sun, 09 Sep 2007 17:58:54 -0400 Original-Received: from rgmgw1.us.oracle.com (rgmgw1.us.oracle.com [138.1.186.110]) by agminet01.oracle.com (Switch-3.2.4/Switch-3.1.7) with ESMTP id l89Lwm2I003600; Sun, 9 Sep 2007 16:58:49 -0500 Original-Received: from acsmt351.oracle.com (acsmt351.oracle.com [141.146.40.151]) by rgmgw1.us.oracle.com (Switch-3.2.4/Switch-3.2.4) with ESMTP id l89LukL9000728; Sun, 9 Sep 2007 15:58:47 -0600 Original-Received: from dhcp-amer-csvpn-gw1-141-144-64-187.vpn.oracle.com by acsmt351.oracle.com with ESMTP id 3197748251189375094; Sun, 09 Sep 2007 14:58:14 -0700 X-Priority: 3 (Normal) X-MSMail-Priority: Normal X-Mailer: Microsoft Outlook IMO, Build 9.0.6604 (9.0.2911.0) In-reply-to: <60848.128.165.123.18.1189016180.squirrel@webmail.lanl.gov> Importance: Normal X-MimeOLE: Produced By Microsoft MimeOLE V6.00.2900.3138 X-Brightmail-Tracker: AAAAAQAAAAI= X-Brightmail-Tracker: AAAAAQAAAAI= X-Whitelist: TRUE X-Whitelist: TRUE X-Detected-Kernel: Linux 2.4-2.6 X-Detected-Kernel: Linux 2.6, seldom 2.4 (older, 4) X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:78356 gmane.emacs.pretest.bugs:19797 Archived-At: This is a multi-part message in MIME format. ------=_NextPart_000_0030_01C7F2F1.DBFEFDB0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit 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)). IIUC, your patch would just eliminate such strings from the history. It is better, IMO, to convert them to unpropertized strings in `savehist-prin1-readable': (when (stringp value) (setq value (format "%s" value))) (prin1 value (current-buffer)) Note that this only works for whole history elements that are strings; it does not convert any strings that might be contained in consp history elements. I don't know if the latter are even possible - are the elements in a history list that someone might use savehist for always atoms? In particular, the user can set `savehist-additional-variables' to anything, so the code is not protected against all possible read errors. This approach of depropertizing strings unfortunately throws away useful information (text properties), so it is not ideal. Minibuffer input that has text properties is useful, and it will become more useful, IMO. Similarly, other variables that savehist might save, such as `kill-ring', can have strings with properties. Better than removing string properties would be to print a sexp in place of the string that, when read, would reconstitute the string with its properties in the right places. Is there already a function that does that? It would need to apply the pertinent properties to the right portions of the string. In fact, shouldn't `prin1' do this? Shouldn't it print a string that has text properties as a sexp that, when read, creates a string with the proper text properties? 2. I think your patch was missing a right paren here: (savehist-prin1-readable `(setq ,symbol ',(symbol-value symbol)))))) 3. I think your patch was missing inserting the second right paren and the newline character here: (insert "))\n") 4. I think your patch was missing inserting the space, quote, and left paren here (and a newline is not needed here): (insert "(setq ") (prin1 symbol) (insert " '(") 5. I don't see how the condition-case in `savehist-prin1-readable' can work. How would an `invalid-read-syntax' error ever be raised here? Isn't it only the Lisp reader that raises that error? I think the error type should be just `error'. Attached is a patch that seems to work. (See #1 above - this patch removes text properties, rather than printing a sexp to reestablish them, and this patch only treats strings that are at the top level of a history list.) In addition to what is mentioned above: 6. In keeping with the doc string, I replaced octal 600 with decimal 384 as the default value of `savehist-file-modes'. 7. I wrapped a condition-case around the body of `savehist-autosave'. I added this long ago to my version, but I cannot recall exactly why it was needed. 8. I left in your binding of `print-unreadable-function', but I did not test that part (I did not build Emacs from C sources). HTH - Drew ------=_NextPart_000_0030_01C7F2F1.DBFEFDB0 Content-Type: application/octet-stream; name="savehist-2007-09-09.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="savehist-2007-09-09.patch" *** savehist-CVS-2007-09-09a.el Sun Sep 9 09:32:10 2007=0A= --- savehist-patched-2007-09-09.el Sun Sep 9 13:55:20 2007=0A= ***************=0A= *** 118,124 ****=0A= :type 'file=0A= :group 'savehist)=0A= =0A= ! (defcustom savehist-file-modes #o600=0A= "*Default permissions of the history file.=0A= This is decimal, not octal. The default is 384 (0600 in octal).=0A= Set to nil to use the default permissions that Emacs uses, typically=0A= --- 118,124 ----=0A= :type 'file=0A= :group 'savehist)=0A= =0A= ! (defcustom savehist-file-modes 384 ; Octal: #o600=0A= "*Default permissions of the history file.=0A= This is decimal, not octal. The default is 384 (0600 in octal).=0A= Set to nil to use the default permissions that Emacs uses, typically=0A= ***************=0A= *** 299,325 ****=0A= (print-string-length nil)=0A= (print-level nil)=0A= (print-readably t)=0A= ! (print-quoted t))=0A= ;; Save the minibuffer histories, along with the value of=0A= ;; savehist-minibuffer-history-variables itself.=0A= (when savehist-save-minibuffer-history=0A= (prin1 `(setq savehist-minibuffer-history-variables=0A= ! ',savehist-minibuffer-history-variables)=0A= ! (current-buffer))=0A= (insert ?\n)=0A= (dolist (symbol savehist-minibuffer-history-variables)=0A= (when (boundp symbol)=0A= (let ((value (savehist-trim-history (symbol-value symbol))))=0A= (when value ; don't save empty histories=0A= ! (prin1 `(setq ,symbol ',value) (current-buffer))=0A= ! (insert ?\n))))))=0A= ;; Save the additional variables.=0A= (dolist (symbol savehist-additional-variables)=0A= (when (boundp symbol)=0A= ! (let ((value (symbol-value symbol)))=0A= ! (when (savehist-printable value)=0A= ! (prin1 `(setq ,symbol ',value) (current-buffer))=0A= ! (insert ?\n))))))=0A= ;; If autosaving, avoid writing if nothing has changed since the=0A= ;; last write.=0A= (let ((checksum (md5 (current-buffer) nil nil = savehist-no-conversion)))=0A= --- 299,326 ----=0A= (print-string-length nil)=0A= (print-level nil)=0A= (print-readably t)=0A= ! (print-quoted t)=0A= ! (print-unreadable-function t)=0A= ! (standard-output (current-buffer)))=0A= ;; Save the minibuffer histories, along with the value of=0A= ;; savehist-minibuffer-history-variables itself.=0A= (when savehist-save-minibuffer-history=0A= (prin1 `(setq savehist-minibuffer-history-variables=0A= ! ',savehist-minibuffer-history-variables))=0A= (insert ?\n)=0A= (dolist (symbol savehist-minibuffer-history-variables)=0A= (when (boundp symbol)=0A= (let ((value (savehist-trim-history (symbol-value symbol))))=0A= (when value ; don't save empty histories=0A= ! (insert "(setq ") (prin1 symbol) (insert " '(")=0A= ! (while value=0A= ! (and (savehist-prin1-readable (car value))=0A= ! (setq value (cdr value)) (insert " ")))=0A= ! (insert "))\n"))))))=0A= ;; Save the additional variables.=0A= (dolist (symbol savehist-additional-variables)=0A= (when (boundp symbol)=0A= ! (savehist-prin1-readable `(setq ,symbol ',(symbol-value = symbol))))))=0A= ;; If autosaving, avoid writing if nothing has changed since the=0A= ;; last write.=0A= (let ((checksum (md5 (current-buffer) nil nil = savehist-no-conversion)))=0A= ***************=0A= *** 340,347 ****=0A= (defun savehist-autosave ()=0A= "Save the minibuffer history if it has been modified since the last = save.=0A= Does nothing if `savehist-mode' is off."=0A= ! (when savehist-mode=0A= ! (savehist-save t)))=0A= =0A= (defun savehist-trim-history (value)=0A= "Retain only the first `history-length' items in VALUE.=0A= --- 341,347 ----=0A= (defun savehist-autosave ()=0A= "Save the minibuffer history if it has been modified since the last = save.=0A= Does nothing if `savehist-mode' is off."=0A= ! (condition-case nil (when savehist-mode (savehist-save t)) (error = nil)))=0A= =0A= (defun savehist-trim-history (value)=0A= "Retain only the first `history-length' items in VALUE.=0A= ***************=0A= *** 376,381 ****=0A= --- 376,391 ----=0A= ;; The attempt failed: the object is not printable.=0A= (error nil))))))=0A= =0A= + (defun savehist-prin1-readable (value)=0A= + "Print VALUE in the current buffer, if it's readable.=0A= + Return non-nil if it was printed."=0A= + (let ((opoint (point)))=0A= + (condition-case nil=0A= + (progn (when (stringp value) (setq value (format "%s" value)))=0A= + (prin1 value (current-buffer))=0A= + t)=0A= + (error (delete-region opoint (point)) nil))))=0A= + =0A= (defun savehist-minibuffer-hook ()=0A= (unless (or (eq minibuffer-history-variable t)=0A= ;; XEmacs sets minibuffer-history-variable to t to mean "no=0A= ------=_NextPart_000_0030_01C7F2F1.DBFEFDB0 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Emacs-devel mailing list Emacs-devel@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-devel ------=_NextPart_000_0030_01C7F2F1.DBFEFDB0--