Re: Saving minibuffer history

[Hrvoje Niksic <hniksic@xemacs.org>]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Thank you.  Installed.  Along with a few minor changes, mostly to
> remove run-time dependency on CL

These changes seem to make the code less readable.  For example:

    (delete-if-not #'savehist-printable value)

is changed to:

    (delq nil (mapcar (lambda (x) (if (savehist-printable x) x)) value))

And:

    (subseq value 0 savehist-length)

is changed to:

    (setq value (copy-sequence value))
    (setcdr (nthcdr savehist-length value) nil)

While I understand the desire to leave the namespace unfettered by cl
compatibility code, maybe you should at least leave the original forms
commented-out, so that the intention is clear(er)?

> and to get rid of most checks for XEmacs, replaced by checks for
> features.

This is a bad idea.  There are extensions out there that approach
cross-emacs compatibility by defining functions normally not present
in the other implementation.  For example, w3 used to define (and
maybe still does) `map-extents' under Emacs.  There are extensions
that define `run-with-timer' under XEmacs.  Such emulations are
incomplete and one does not wish to use them unless necessary.

In other words, I intentionally checked for XEmacs to decide which API
to use.  I would appreciate if you left this intact.

> I have one question: what's the point of savehist-no-conversion?

The code simply needs to check whether the buffer contents has changed
since the last save.  I wanted to checksum the raw buffer data, which
'no-conversion achieves in Emacs, and 'binary in XEmacs.  To make the
intentions clear and remove an `if' from the code, I used a defconst.

> Also see attached a proposed patch (100% guaranteed untested).  If
> nobody objects, I'll install it.
> 
> I could also get rid of savehist-history-variables altogether.

What is your intended replacement?  If I understand
minibuffer-history-variable, it only records the minibuffer history
for minibuffer input whose history is not otherwise specified.  Which
means that it doesn't record the history of M-x, M-:, C-x C-f, etc.

But the point of savehist is to be as easy as possible to set up,
because other editors have such a feature built-in.  Being easy to set
up means saving the most important minibuffer histories by default,
with the ability to save more (or less!) if one chooses to.  As the
commentary says, putting (savehist-load) to `.emacs' should be enough.