From: "Davis Herring" <herring@lanl.gov>
To: "Drew Adams" <drew.adams@oracle.com>
Cc: emacs-pretest-bug@gnu.org, rms@gnu.org, sdl.web@gmail.com
Subject: RE: 23.0.50; savehist save invalid syntax
Date: Mon, 10 Sep 2007 14:59:07 -0700 (PDT) [thread overview]
Message-ID: <38272.128.165.123.18.1189461547.squirrel@webmail.lanl.gov> (raw)
In-Reply-To: <DNEMKBNJBGPAOPIJOOICEEKBDPAA.drew.adams@oracle.com>
> 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)).
As Andreas said, no problem here.
> 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 " '(")
You're right, of course; I fixed some parentheses, but obviously didn't
find all of the problems. But there was a reason for the newline: all
elements but the first will start at the beginning of a line, so I thought
it best that the first do so as well. I've adjusted my own copy
appropriately, for when I find the time to write up the patch
documentation.
> 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'.
The applicability of the error symbol may be questionable, but there is no
bug: with my patch to print.c (and `print-unreadable-function' bound to
t), the Lisp printer raises that error if the Lisp reader -would- raise
it.
> 6. In keeping with the doc string, I replaced octal 600 with decimal 384
> as
> the default value of `savehist-file-modes'.
The doc string has to explain the variable's appearance to the user; in
the code it's probably best to use the octal constant since that shows
what's meant. I question why the docstring should mention its own default
value, except perhaps to say that 384==0600 in case that happens to be its
value.
> 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.
I can't see why that function should be particularly likely to throw,
unless `savehist-minibuffer-history-variables' contained unprintable
objects (and it should contain only symbols). If it does, it is
presumably a bug to fix, yes?
> 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).
As stated just above, this is of course critical.
Davis
--
This product is sold by volume, not by mass. If it appears too dense or
too sparse, it is because mass-energy conversion has occurred during
shipping.
next prev parent reply other threads:[~2007-09-10 21:59 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-09-02 10:38 23.0.50; savehist save invalid syntax Leo
2007-09-02 13:21 ` Drew Adams
2007-09-03 3:04 ` Richard Stallman
2007-09-04 22:48 ` Davis Herring
2007-09-05 3:20 ` Stefan Monnier
2007-09-05 6:16 ` Richard Stallman
2007-09-05 18:16 ` Davis Herring
2007-09-06 5:00 ` Richard Stallman
2007-09-09 21:58 ` Drew Adams
2007-09-09 23:14 ` Andreas Schwab
2007-09-10 3:01 ` Drew Adams
2007-09-10 3:07 ` Drew Adams
2007-09-10 22:11 ` Davis Herring
2007-09-10 23:42 ` Drew Adams
2007-09-10 23:54 ` Richard Stallman
2007-09-11 20:27 ` Davis Herring
2007-09-10 21:59 ` Davis Herring [this message]
2007-09-10 23:42 ` Drew Adams
2007-09-11 0:55 ` Davis Herring
2007-09-11 1:11 ` Stefan Monnier
2007-09-11 21:06 ` [Released] " Davis Herring
2007-09-11 21:29 ` Stefan Monnier
2007-09-14 7:04 ` Richard Stallman
2007-09-11 1:18 ` Drew Adams
2007-09-05 19:57 ` Leo
2007-10-18 21:08 ` Leo
2007-10-19 8:15 ` Leo
2007-10-19 14:01 ` Stefan Monnier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=38272.128.165.123.18.1189461547.squirrel@webmail.lanl.gov \
--to=herring@lanl.gov \
--cc=drew.adams@oracle.com \
--cc=emacs-pretest-bug@gnu.org \
--cc=rms@gnu.org \
--cc=sdl.web@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.