RE: 23.0.50; savehist save invalid syntax

[Davis Herring]

> 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.