Saving minibuffer history

Saving minibuffer history

[Hrvoje Niksic]

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

[ Please keep me in the Cc as I am not subscribed to the list. ]

Is there a straightforward way to persist minibuffer histories across
Emacs invocations?

If not, I'd like to suggest importing this small package.  It was
originally written for XEmacs, but it also works in Emacs.  Its
copyright should be assigned to the FSF along with my other XEmacs
changes.  Usage is best desribed by this NEWS entry:

** Emacs can now save the minibuffer histories from various
minibuffers.  To use this feature, add the line:

   (savehist-load)

to your .emacs.  This will load the minibuffer histories (if any) at
startup, as well as instruct Emacs to save them before exiting.  You
can use Customize to add or remove the histories being saved.


The source follows:


[-- Attachment #2: savehist.el --]
[-- Type: application/emacs-lisp, Size: 6293 bytes --]

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

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

Re: Saving minibuffer history

[Hrvoje Niksic]

I've now improved this file somewhat (I wrote it eight years ago, and
it shows).  The new version is available at:

    http://fly.srk.fer.hr/~hniksic/emacs/savehist.el

RE: Saving minibuffer history

[Drew Adams]

    I've now improved this file somewhat (I wrote it eight years ago, and
    it shows).  The new version is available at:

        http://fly.srk.fer.hr/~hniksic/emacs/savehist.el


But the previous version didn't give me this byte-compile error (in Emacs
20):

  !! Invalid read syntax (("#"))

Re: Saving minibuffer history

[Richard M. Stallman]

I installed it.  Thanks.

Re: Saving minibuffer history

[Hrvoje Niksic]

"Richard M. Stallman" <rms@gnu.org> writes:

> I installed it.  Thanks.

Thank you.  An improved version is now available at
<http://fly.srk.fer.hr/~hniksic/emacs/savehist.el>.

RE: Saving minibuffer history

[Drew Adams]

    An improved version is now available at
    <http://fly.srk.fer.hr/~hniksic/emacs/savehist.el>.

FYI - I had some problems trying to use this library with Emacs 20, but if a
few minor changes are made, it works well.

Here are the problems I ran into:

1. (defcustom savehist-modes #o600
The reader choked on this. I changed it to this:
(defcustom savehist-modes 384   ; Octal: #o600

2. (defvar savehist-coding-system (if (string-match "XEmacs" emacs-version)
			                     'iso-2022-8
			                   'utf-8)

I had problems with the utf-8 coding system. I got this error message when I
tried to save any file, after loading the library: "Removing old name: no
such file or directory, <path>/#tmp#0. I could not even exit Emacs, getting
the same message. When I tried to exit without saving anything, I got this
error message: Invalid coding system: utf-8.

I changed the code to this: (defvar savehist-coding-system nil

With these simple changes, the library seems to work fine in Emacs 20.
Thanks for it.

Re: Saving minibuffer history

[David Kastrup]

"Drew Adams" <drew.adams@oracle.com> writes:

>     An improved version is now available at
>     <http://fly.srk.fer.hr/~hniksic/emacs/savehist.el>.
>
> FYI - I had some problems trying to use this library with Emacs 20, but if a
> few minor changes are made, it works well.
>
> Here are the problems I ran into:
>
> 1. (defcustom savehist-modes #o600
> The reader choked on this. I changed it to this:
> (defcustom savehist-modes 384   ; Octal: #o600
>
> 2. (defvar savehist-coding-system (if (string-match "XEmacs" emacs-version)
> 			                     'iso-2022-8
> 			                   'utf-8)
>
> I had problems with the utf-8 coding system. I got this error
> message when I tried to save any file, after loading the library:
> "Removing old name: no such file or directory, <path>/#tmp#0. I
> could not even exit Emacs, getting the same message. When I tried to
> exit without saving anything, I got this error message: Invalid
> coding system: utf-8.
>
> I changed the code to this: (defvar savehist-coding-system nil
>
> With these simple changes, the library seems to work fine in Emacs
> 20.  Thanks for it. 

Uh, Emacs 20?  Why would one need to be compatible with that?

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Saving minibuffer history

[Hrvoje Niksic]

"Drew Adams" <drew.adams@oracle.com> writes:

>     An improved version is now available at
>     <http://fly.srk.fer.hr/~hniksic/emacs/savehist.el>.
> 
> FYI - I had some problems trying to use this library with Emacs 20,
> but if a few minor changes are made, it works well.

Thanks for the testing, but this library was not designed to be
compatible with Emacs 20.

Re: Saving minibuffer history

[Stefan Monnier]

> 2. (defvar savehist-coding-system (if (string-match "XEmacs" emacs-version)
> 			                     'iso-2022-8
> 			                   'utf-8)

Shouldn't it just use `emacs-mule'?  This would ensure that no information
can be lost, contrary to what happens with utf-8.


        Stefan

Re: Saving minibuffer history

[Stefan Monnier]

>> I installed it.  Thanks.
> Thank you.  An improved version is now available at
> <http://fly.srk.fer.hr/~hniksic/emacs/savehist.el>.

Thank you.  Installed.  Along with a few minor changes, mostly to remove
run-time dependency on CL and to get rid of most checks for XEmacs, replaced
by checks for features.

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

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.


        Stefan


2005-10-16  Stefan Monnier  <monnier@iro.umontreal.ca>

	* savehist.el (savehist-file): Use the ~/.emacs.d directory.
	(savehist-all-history-variables): New var.
	(savehist-minibuffer-hook): New fun to populate it.
	(savehist-save): Use and save it.
	(savehist-history-variables): Default to nil, meaning "all".


Index: savehist.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/savehist.el,v
retrieving revision 1.3
diff -u -u -b -r1.3 savehist.el
--- savehist.el	16 Oct 2005 15:22:37 -0000	1.3
+++ savehist.el	16 Oct 2005 15:33:46 -0000
@@ -52,64 +52,20 @@
   "Save minibuffer history."
   :group 'minibuffer)
 
-(defcustom savehist-history-variables
-  '(
-    ;; Catch-all minibuffer history
-    minibuffer-history
-    ;; File-oriented commands
-    file-name-history
-    ;; Regexp-related reads
-    regexp-history
-    ;; Searches in minibuffer (via `M-r' and such)
-    minibuffer-history-search-history
-    ;; Query replace
-    query-replace-history
-    ;; eval-expression (`M-:')
-    read-expression-history
-    ;; shell-command (`M-!')
-    shell-command-history
-    ;; compile
-    compile-history
-    ;; find-tag (`M-.')
-    find-tag-history
-    ;; grep
-    grep-history
-    ;; Viper stuff
-    vip-ex-history vip-search-history
-    vip-replace1-history vip-replace2-history
-    vip-shell-history vip-search-history
-
-    ;; XEmacs-specific:
-    ;; Buffer-related commands
-    buffer-history
-    ;; Reads of variables and functions
-    variable-history function-history
-    ;; Extended commands
-    read-command-history
-
-    ;; Info, lookup, and bookmark historys
-    Info-minibuffer-history
-    Info-search-history
-    Manual-page-minibuffer-history
-
-    ;; Emacs-specific:
-    ;; Extended commands
-    extended-command-history)
+(defvar savehist-all-history-variables nil)
+
+(defcustom savehist-history-variables nil
   "*List of symbols to be saved.
 Every symbol should refer to a variable.  The variable will be saved
 only if it is bound and has a non-nil value.  Thus it is safe to
 specify a superset of the variables a user is expected to want to
 save.
 
-Default value contains minibuffer history variables used by Emacs, XEmacs,
-and Viper (uh-oh).  Note that, if you customize this variable, you
-can lose the benefit of future versions of Emacs adding new values to
-the list.  Because of that it might be more useful to add values using
-`add-to-list'."
+If nil, all history variables will be saved."
   :type '(repeat (symbol :tag "Variable"))
   :group 'savehist)
 
-(defcustom savehist-file "~/.emacs-history"
+(defcustom savehist-file "~/.emacs.d/history"
   "*File name to save minibuffer history to.
 The minibuffer history is a series of Lisp expressions, which should be
 loaded using `savehist-load' from your .emacs.  See `savehist-load' for
@@ -201,7 +157,11 @@
 	  (print-level nil)
 	  (print-readably t)
 	  (print-quoted t))
-      (dolist (sym savehist-history-variables)
+      (prin1
+       `(setq savehist-all-history-variables ',savehist-all-history-variables)
+       (current-buffer))
+      (dolist (sym (or savehist-history-variables
+                       savehist-all-history-variables))
 	(when (boundp sym)
 	  (let ((value (savehist-process-for-saving (symbol-value sym))))
 	    (prin1 `(setq ,sym ',value) (current-buffer))
@@ -261,6 +221,11 @@
 	  t)
       ;; The attempt failed: the object is not printable.
       (error nil)))))
+
+(defun savehist-minibuffer-hook ()
+  (add-to-list 'savehist-all-history-variables minibuffer-history-variable))
+
+(add-hook 'minibuffer-setup-hook 'savehist-minibuffer-hook)
 
 (provide 'savehist)

RE: Saving minibuffer history

[Drew Adams]

    Uh, Emacs 20?  Why would one need to be compatible with that?

Many, many people do use Emacs 20, for a variety of reasons.

Emacs development does not have as a goal to be backward compatible - I
understand that. I never said that Emacs 22 "needs" to be compatible with
Emacs 20.

If, however, other things are equal or can trivially be made equal, why not?
If nothing is gained by breaking backward compatibility, why break it?

I made some minor change suggestions. I don't know if they remove any
savehistory functionality - I didn't think so. If they do, then ignore them.
I am not requesting removing Emacs 22 functionality to make things
backward-compatible with Emacs 20.

I tested savehistory briefly with the changes, and it appeared to work.
Hrvoje says that it was not designed to work with Emacs 20, so my
observations were no doubt inadequate.

As the suggested changes were simply a defvar and a defcustom, if
functionality is lost by my changes, what about adding a usage note wrt
these variables? My concern is that the library could be useful to Emacs 20
users as well, if they know to change these variable values.

Re: Saving minibuffer history

[Stefan Monnier]

>     Uh, Emacs 20?  Why would one need to be compatible with that?
> Many, many people do use Emacs 20, for a variety of reasons.

Please remove emacs-devel from the Cc when discussing such issues.


        Stefan

Re: Saving minibuffer history

[Richard M. Stallman]

    Thank you.  An improved version is now available at
    <http://fly.srk.fer.hr/~hniksic/emacs/savehist.el>.

Someone reported trouble compiling it; could you say when it is ready?

I see that changes were installed in the file already.
Is it up-to-date now?

RE: Saving minibuffer history

[Drew Adams]

    >     Uh, Emacs 20?  Why would one need to be compatible with that?
    > Many, many people do use Emacs 20, for a variety of reasons.

    Please remove emacs-devel from the Cc when discussing such issues.

Why? This was the gist of my reply:

 If nothing is gained by breaking backward compatibility, why break it?

Is that not worthy of consideration as a design/development consideration
for Emacs?

I'm not sure what the "such issues" are that you object to discussing. I did
not discuss Emacs 20 and its relative virtues or disadvantages. My point was
only the one quoted here - a suggestion of consideration, for future
development, of not breaking old code gratuitously - that is, not breaking
backward compatibility unless there is some advantage in doing so. The
advantage could be as slight as more readable code - I wasn't asking for
more than opting for compatibility when things are _equal_.

I didn't suggest testing new code with Emacs 20, or placing the burden of
showing zero cost for compatibility on Emacs developers, or anything of the
sort. I simply raised the design/development question, "_if_ other things
are equal, why not choose the backward-compatible solution?"

Even if there might be disagreement over the suggestion, is this not a
proper place for its discussion?

Re: Saving minibuffer history

[Hrvoje Niksic]

"Richard M. Stallman" <rms@gnu.org> writes:

>     Thank you.  An improved version is now available at
>     <http://fly.srk.fer.hr/~hniksic/emacs/savehist.el>.
> 
> Someone reported trouble compiling it; could you say when it is
> ready?

It should be ready.  Feel free to forward problem reports to me.

Re: Saving minibuffer history

[David Kastrup]

"Drew Adams" <drew.adams@oracle.com> writes:

>     >     Uh, Emacs 20?  Why would one need to be compatible with
>     > that?  Many, many people do use Emacs 20, for a variety of
>     > reasons.
>
>     Please remove emacs-devel from the Cc when discussing such
>     issues.
>
> Why? This was the gist of my reply:
>
>  If nothing is gained by breaking backward compatibility, why break
>  it?
>
> Is that not worthy of consideration as a design/development
> consideration for Emacs?

No.  Emacs 22 code needs to be compatible with Emacs 22, not with any
other Emacs version.

For packages maintained outside of Emacs, the maintainer might have
different priorities, but that's not interesting for Emacs development
proper and this mailing list.  In many cases, XEmacs compatibility
actually is correlated to Emacs 20, since the XEmacs 21.4 core has
never thoroughly been synchronized with Emacs 21.

> I didn't suggest testing new code with Emacs 20, or placing the
> burden of showing zero cost for compatibility on Emacs developers,
> or anything of the sort. I simply raised the design/development
> question, "_if_ other things are equal, why not choose the
> backward-compatible solution?"

Because it is work finding out whether all other things are equal, and
which is the "backward-compatible" solution, if one considers it to
make sense to talk about "backward-compatible" for what _is_ supposed
to be a proper part of Emacs 22.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Saving minibuffer history

[David Kastrup]

"Richard M. Stallman" <rms@gnu.org> writes:

>     Thank you.  An improved version is now available at
>     <http://fly.srk.fer.hr/~hniksic/emacs/savehist.el>.
>
> Someone reported trouble compiling it; could you say when it is ready?

I think that was with regard to compiling it under Emacs 20.  A good
example why "Emacs 20 compatibility" for single files should not even
be an issue on this list: it confuses and detracts from more important
work.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Saving minibuffer history

[Stefan Monnier]

>     Please remove emacs-devel from the Cc when discussing such issues.
> Why? This was the gist of my reply:
>   If nothing is gained by breaking backward compatibility, why break it?
> Is that not worthy of consideration as a design/development consideration
> for Emacs?

We're not talking about breaking compatibility, but about fixing it.
I agree we shouldn't break compatibility for no reason, but I think that
preserving backward compatibility is a waste of our time in such a context.
Of course, if Hrvoje wants to distribute his package separately and wants it
to work with Emacs-20, that's perfectly fine and not necessarily a waste of
time, but emacs-devel has a different focus.


        Stefan

Re: Saving minibuffer history

[Hrvoje Niksic]

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

> We're not talking about breaking compatibility, but about fixing it.
> I agree we shouldn't break compatibility for no reason, but I think
> that preserving backward compatibility is a waste of our time in
> such a context.  Of course, if Hrvoje wants to distribute his
> package separately and wants it to work with Emacs-20, that's
> perfectly fine and not necessarily a waste of time, but emacs-devel
> has a different focus.

I do want to distribute the package separately, but I am not
interested in Emacs 20 compatibility.

I am interested in Emacs 21 compatibility, because it is what I now
use, and I am interested in XEmacs compatibility because the package
was written for and is still distributed by XEmacs.  Obviously, I also
care about the package working in the latest Emacs.

Re: Saving minibuffer history

[Hrvoje Niksic]

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.

Re: Saving minibuffer history

[Hrvoje Niksic]

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

> > 2. (defvar savehist-coding-system (if (string-match "XEmacs" emacs-version)
> > 			                     'iso-2022-8
> > 			                   'utf-8)
> 
> Shouldn't it just use `emacs-mule'?  This would ensure that no
> information can be lost, contrary to what happens with utf-8.

Using utf-8 seemed like a good idea because of various nice properties
of UTF-8.  Also, I didn't know about emacs-mule.

Feel free to change it to emacs-mule if that makes more sense.  The
encoding used by this file is more or less internal to Emacs, and only
Emacs needs be able to process it.  People who desperately want
another encoding can change it by customizing that variable.

Re: Saving minibuffer history

[Stefan Monnier]

>> 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)?

That's a good idea.  I'm in favor of preloading CL in the dumped Emacs, so
leaving in the original code is a way to remind people that preloadnig CL
would clean up various bits of code.

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

For the case of start-itimer vs run-with-idle-timer, I can see you point, so
I've "reverted" the change.  For the case of (if (coding-system-p 'utf-8)
'utf-8 'iso-2022-8), I think the new code is preferable, since the choice is
really not dependent on differences betwen Emacs and XEmacs but just on the
availablility of utf-8.  If you disagree, let me know and I'll also revert
this part.
I put "reverted" in quotes because I still have gotten rid of
savehist-xemacs and replaced it with (featurep 'xemacs) since this form is
recognized/optimized by the byte-compiler.

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

So if the buffer contents changed but that change is not reflected in the
saved file (e.g. the buffer contents has a latin-1 é replaced by a latin-9
é, but both get written (in utf-8) as the same byte-sequence) you end up
uselessly re-saving the exact same file.
I know it's not an important case, but I'm wondering: why not simply use
savehist-coding-system?
BTW, I think that instead of `no-conversion' we should use `emacs-mule'
under Emacs.

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

minibuffer-history-variable has a default value used for when no histvar is
specified, indeed, but it is also set by read-from-minibuffer to the history
variable that should be used.  So by recording its value in
minibuffer-setup-hook I accumulate all the histvars that have been seen.


        Stefan

Re: Saving minibuffer history

[Hrvoje Niksic]

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

> For the case of start-itimer vs run-with-idle-timer, I can see you
> point, so I've "reverted" the change.  For the case of (if
> (coding-system-p 'utf-8) 'utf-8 'iso-2022-8), I think the new code
> is preferable, since the choice is really not dependent on
> differences betwen Emacs and XEmacs but just on the availablility of
> utf-8.

That's true in a way, but it's also the case that under XEmacs the
utf-8 coding system is loaded from an external package.  So it can
happen that it is present in one XEmacs invocation and not in another.
Because of that I made savehist not use utf-8 in XEmacs.

> I put "reverted" in quotes because I still have gotten rid of
> savehist-xemacs and replaced it with (featurep 'xemacs) since this
> form is recognized/optimized by the byte-compiler.

That's fine.

> >> 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.
> 
> So if the buffer contents changed but that change is not reflected
> in the saved file (e.g. the buffer contents has a latin-1 é replaced
> by a latin-9 é, but both get written (in utf-8) as the same
> byte-sequence) you end up uselessly re-saving the exact same file.

That seems like a contrived example.  savehist's buffer contents
consists of lists with new entries pushed at the front -- it never
changes a single character.

Even if such a thing did happen, the file would be uselessly re-saved
exactly once.

> I know it's not an important case, but I'm wondering: why not simply
> use savehist-coding-system?

Because this use has nothing to do with saving to file or any external
representation, it just needs to be fast and do as little as possible.
Remember that this is done off a timer, and this needs to be fast to
not interrupt the user's work.  With large savehist buffers I prefer
to be on the safe side.

> > 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.
> 
> minibuffer-history-variable has a default value used for when no
> histvar is specified, indeed, but it is also set by
> read-from-minibuffer to the history variable that should be used.
> So by recording its value in minibuffer-setup-hook I accumulate all
> the histvars that have been seen.

What about those that haven't been seen yet?  Won't they be saved as
nil by savehist-save?

Re: Saving minibuffer history

[Stefan Monnier]

>> For the case of start-itimer vs run-with-idle-timer, I can see you
>> point, so I've "reverted" the change.  For the case of (if
>> (coding-system-p 'utf-8) 'utf-8 'iso-2022-8), I think the new code
>> is preferable, since the choice is really not dependent on
>> differences betwen Emacs and XEmacs but just on the availablility of
>> utf-8.

> That's true in a way, but it's also the case that under XEmacs the
> utf-8 coding system is loaded from an external package.  So it can
> happen that it is present in one XEmacs invocation and not in another.
> Because of that I made savehist not use utf-8 in XEmacs.

OK, I'll revert that part as well, then.  Thanks for the explanation.

>> >> 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.
>> So if the buffer contents changed but that change is not reflected
>> in the saved file (e.g. the buffer contents has a latin-1 ? replaced
>> by a latin-9 ?, but both get written (in utf-8) as the same
>> byte-sequence) you end up uselessly re-saving the exact same file.

> That seems like a contrived example.

It is.

>> I know it's not an important case, but I'm wondering: why not simply
>> use savehist-coding-system?

> Because this use has nothing to do with saving to file or any external
> representation, it just needs to be fast and do as little as possible.
> Remember that this is done off a timer, and this needs to be fast to
> not interrupt the user's work.  With large savehist buffers I prefer
> to be on the safe side.

OK, so it's done for performance reason, on the assumption that encoding
with no-conversion is significantly faster than with utf-8.  Thanks.

>> minibuffer-history-variable has a default value used for when no
>> histvar is specified, indeed, but it is also set by
>> read-from-minibuffer to the history variable that should be used.
>> So by recording its value in minibuffer-setup-hook I accumulate all
>> the histvars that have been seen.

> What about those that haven't been seen yet?  Won't they be saved as
> nil by savehist-save?

I don't think so: they won't be saved at all.  Notice also that
the variable savehist-all-history-variables is also saved along with the
actual history variables.


        Stefan

Re: Saving minibuffer history

[Hrvoje Niksic]

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

> >> minibuffer-history-variable has a default value used for when no
> >> histvar is specified, indeed, but it is also set by
> >> read-from-minibuffer to the history variable that should be used.
> >> So by recording its value in minibuffer-setup-hook I accumulate
> >> all the histvars that have been seen.
> 
> > What about those that haven't been seen yet?  Won't they be saved as
> > nil by savehist-save?
> 
> I don't think so: they won't be saved at all.  Notice also that the
> variable savehist-all-history-variables is also saved along with the
> actual history variables.

I now understand and I think it's a good change.  It basically saves
*all* minibuffer histories, which is exactly the effect I wanted to
achieve back when I wrote the file.  Thanks for improving it.

Re: Saving minibuffer history

[Hrvoje Niksic]

Hrvoje Niksic <hniksic@xemacs.org> writes:

> I now understand and I think it's a good change.  It basically saves
> *all* minibuffer histories, which is exactly the effect I wanted to
> achieve back when I wrote the file.  Thanks for improving it.

There is one problem, though: any customization of
savehist-history-variables now removes the saving of minibuffer
histories, doesn't it?  This means that, with your change, it is not
easy to just add one variable which you want to persist across Emacs
sessions.

Re: Saving minibuffer history

[Stefan Monnier]

>> I now understand and I think it's a good change.  It basically saves
>> *all* minibuffer histories, which is exactly the effect I wanted to
>> achieve back when I wrote the file.  Thanks for improving it.

> There is one problem, though: any customization of
> savehist-history-variables now removes the saving of minibuffer
> histories, doesn't it?  This means that, with your change, it is not
> easy to just add one variable which you want to persist across Emacs
> sessions.

Indeed.  The interaction between savehist-history-variables and
savehist-all-history-variables was chosen arbitrarily.  Feel free to
change it.

BTW, I didn't think that savehist-history-variables was meant to also allow
savnig non-history variables.  At least the savehist-length makes it
unusable for some (admittedly probably rare) variables.


        Stefan

Re: Saving minibuffer history

[Hrvoje Niksic]

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

> BTW, I didn't think that savehist-history-variables was meant to
> also allow savnig non-history variables.

It's probably a rarely-used feature, but I sort of like it.

> At least the savehist-length makes it unusable for some (admittedly
> probably rare) variables.

Lists are automatically assumed to be histories, yes.  But no one has
complained so far.

Re: Saving minibuffer history

[Hrvoje Niksic]

I've now applied your changes to my version.

    http://fly.srk.fer.hr/~hniksic/emacs/savehist.el

To summarize:

* Minibuffer histories are now picked up the way you suggested.  That
  way we no longer need the minibuffer history taxonomy -- individual
  histories get picked up automatically as they are used.  Thanks!

* The user can now separately choose whether to save minibuffer
  histories and/or specify additional variables he wants to save.

* Runtime dependency on CL is removed -- at the unfortunate cost of
  clarity.  Please reconsider allowing the use of CL extensions in
  Emacs.

* The copyright information is now as in your version.

These changes should make it possible for you to simply use my version
unchanged.

I also recommend that you mention in the NEWS file how the feature is
supposed to be used.  It can be as simple as:

    To use this feature, put (savehist-load) in your `.emacs' file.

RE: Saving minibuffer history

[Drew Adams]

    > > 2. (defvar savehist-coding-system (if (string-match
    "XEmacs" emacs-version)
    > > 			                     'iso-2022-8
    > > 			                   'utf-8)
    >
    > Shouldn't it just use `emacs-mule'?  This would ensure that no
    > information can be lost, contrary to what happens with utf-8.

    Using utf-8 seemed like a good idea because of various nice properties
    of UTF-8.  Also, I didn't know about emacs-mule.

    Feel free to change it to emacs-mule if that makes more sense.  The
    encoding used by this file is more or less internal to Emacs, and only
    Emacs needs be able to process it.  People who desperately want
    another encoding can change it by customizing that variable.


I missed the response to this, I guess, and I notice that the latest version
still has utf-8. What was the reason not to use emacs-mule instead?

RE: Saving minibuffer history

[Drew Adams]

Shouldn't this be named `savehist-save-minibuffer-history-flag'?

(defcustom savehist-save-minibuffer-history t
  "*If non-nil, save all recorded minibuffer histories."
  :type 'boolean :group 'savehist)

Re: Saving minibuffer history

[Stefan Monnier]

> I've now applied your changes to my version.
>     http://fly.srk.fer.hr/~hniksic/emacs/savehist.el

I don't have time to take care of it right now, but I'll do it later.
In the mean time, could you send it as a patch?  That makes it much easier
for me to install it.


        Stefan

Re: Saving minibuffer history

[Stefan Monnier]

> Shouldn't this be named `savehist-save-minibuffer-history-flag'?
> (defcustom savehist-save-minibuffer-history t
>   "*If non-nil, save all recorded minibuffer histories."
>   :type 'boolean :group 'savehist)

I strongly dislike the -flag convention.


        Stefan

Re: Saving minibuffer history

[Stefan Monnier]

> I've now applied your changes to my version.
>     http://fly.srk.fer.hr/~hniksic/emacs/savehist.el

Installed.

> These changes should make it possible for you to simply use my version
> unchanged.

Almost.  There were a few comments missing, and things like that.
E.g. please preserve the arch-tag.

> I also recommend that you mention in the NEWS file how the feature is
> supposed to be used.  It can be as simple as:
>     To use this feature, put (savehist-load) in your `.emacs' file.

Done.

As mentioned earlier, your code truncates non-history variables as well.
But now that you use it to save the list of variables as well, it means that
if the number of history variables you have is larger than savehist-length,
than you will lose some of your history as well.


        Stefan

Re: Saving minibuffer history

[Stefan Monnier]

>> I've now applied your changes to my version.
>> http://fly.srk.fer.hr/~hniksic/emacs/savehist.el

Here's yet another suggested patch.
It includes the change of default of savehist-file and introduces
savehist-mode, which would become the main advertized way to turn the
feature ON (can also be done via Customize) in place of savehist-load.

There are also minor changes: don't save empty histories, and try to avoid
computing the checksum when no minibuffer input occurred since last check.


        Stefan


--- savehist.el	24 oct 2005 13:15:48 -0400	1.8
+++ savehist.el	25 oct 2005 00:35:03 -0400	
@@ -70,8 +70,14 @@
   :type '(repeat (symbol :tag "Variable"))
   :group 'savehist)
 
-(defcustom savehist-file "~/.emacs-history"
-  "File name to save minibuffer history to.
+(defcustom savehist-file
+  (cond
+   ;; Backward compatibility with previous versions of savehist.
+   ((file-exists-p "~/.emacs-history") "~/.emacs-history")
+   ((and (not (featurep 'xemacs)) (file-directory-p "~/.emacs.d/"))
+    "~/.emacs.d/history")
+   (t "~/.emacs-history"))
+  "*File name to save minibuffer history to.
 The minibuffer history is a series of Lisp expressions, which should be
 loaded using `savehist-load' from your .emacs.  See `savehist-load' for
 more details."
@@ -139,7 +145,8 @@
 	      (start-itimer
 	       "savehist" 'savehist-autosave savehist-autosave-interval
 	       savehist-autosave-interval)
-	    (run-with-timer savehist-autosave-interval savehist-autosave-interval
+	    (run-with-timer savehist-autosave-interval
+                            savehist-autosave-interval
 			    'savehist-autosave)))))
 
 (defun savehist-uninstall ()
@@ -152,6 +159,17 @@
       (cancel-timer savehist-timer))
     (setq savehist-timer nil)))
 
+(defvar savehist-loaded nil)
+
+;;;###autoload
+(define-minor-mode savehist-mode
+  "Minor mode that preserves minibuffer history across Emacs sessions."
+  :lighter nil
+  (if (not savehist-mode)
+      (savehist-uninstall)
+    (unless savehist-loaded (savehist-load 'no-install))
+    (savehist-install)))
+
 ;;;###autoload
 (defun savehist-load (&optional no-install)
   "Load the minibuffer histories from `savehist-file'.
@@ -168,9 +186,17 @@
   ;; the user changes the value of savehist-coding-system, we can
   ;; still correctly load the old file.
   (load savehist-file t (not (interactive-p)))
+  ;; Remember we've already loaded it, so we can try and avoid loading it
+  ;; several times, which would risk throwing away history info.
+  (setq savehist-loaded t)
   (unless no-install
     (savehist-install)))
 
+(defvar savehist-changed nil
+  "If nil, the history hasn't changed.
+This variable is only used as an optimization, to avoid generating a new
+savehist state and checksum, only to discover that it hasn't changed.")
+
 ;;;###autoload
 (defun savehist-save (&optional auto-save)
   "Save the values of minibuffer history variables.
@@ -178,9 +204,11 @@
 If AUTO-SAVE is non-nil, compare the saved contents to the one last saved,
  and don't save the buffer if they are the same."
   (interactive)
+  (unless (and auto-save (not savehist-changed))
   (with-temp-buffer
     (insert
-     (format ";; -*- mode: emacs-lisp; coding: %s -*-\n" savehist-coding-system)
+       (format ";; -*- mode: emacs-lisp; coding: %s -*-\n"
+               savehist-coding-system)
      ";; Minibuffer history file, automatically generated by `savehist'.\n\n")
     (let ((print-length nil)
 	  (print-string-length nil)
@@ -195,8 +223,10 @@
       (dolist (sym symbol-list)
 	(when (boundp sym)
 	  (let ((value (savehist-process-for-saving (symbol-value sym))))
+              ;; Don't save empty histories.
+              (when value
 	    (prin1 `(setq ,sym ',value) (current-buffer))
-	    (insert ?\n)))))
+                (insert ?\n))))))
     ;; If autosaving, avoid writing if nothing has changed since the
     ;; last write.
     (let ((checksum (md5 (current-buffer) nil nil savehist-no-conversion)))
@@ -212,7 +242,8 @@
 			(unless (interactive-p) 'quiet)))
 	(when savehist-modes
 	  (set-file-modes savehist-file savehist-modes))
-	(setq savehist-last-checksum checksum)))))
+          (setq savehist-changed nil)
+          (setq savehist-last-checksum checksum))))))
 
 (defun savehist-autosave ()
   "Save the minibuffer history if it has been modified since the last save."
@@ -257,6 +288,7 @@
       (error nil)))))
 
 (defun savehist-minibuffer-hook ()
+  (setq savehist-changed t)
   (add-to-list 'savehist-minibuffer-history-variables
                minibuffer-history-variable))

Re: Saving minibuffer history

[Hrvoje Niksic]

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

>> I've now applied your changes to my version.
>>     http://fly.srk.fer.hr/~hniksic/emacs/savehist.el
>
> I don't have time to take care of it right now, but I'll do it
> later.  In the mean time, could you send it as a patch?  That makes
> it much easier for me to install it.

A patch against what?  I'm not sure which was the version you last
picked up.  A patch against the version in your CVS does not seem all
that useful.

Re: Saving minibuffer history

[Hrvoje Niksic]

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

> Almost.  There were a few comments missing, and things like that.

You never informed me about those changes so I missed them.  I did
notice the changes in the patch you sent me.  Please remember that
this package is normally also maintained for XEmacs, and as an
external package (although it probably doesn't have many external
users).

I also don't understand why you removed the "*" markers from the
beginning of docstrings.  Those variables *are* intended to be "set
and modified by users".

> E.g. please preserve the arch-tag.

I don't know what it does and I don't use arch.  Is it always the
same?  What purpose does it serve?

> As mentioned earlier, your code truncates non-history variables as
> well.  But now that you use it to save the list of variables as
> well, it means that if the number of history variables you have is
> larger than savehist-length, than you will lose some of your history
> as well.

Fixed now, thanks.

Re: Saving minibuffer history

[Hrvoje Niksic]

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

> It [...] introduces savehist-mode, which would become the main
> advertized way to turn the feature ON (can also be done via
> Customize) in place of savehist-load.

I'm not sure that's a good idea.  savehist-load replaces existing
minibuffer histories with the loaded ones, which is why it is
recommended to only call it from `.emacs'.  Since turning on a
savehist mode calls `savehist-load', it has the nasty side effect of
ruining existing minibuffer histories.  This is alleviated by the use
of `savehist-loaded', but it will still ruin the minibuffer histories
when savehist is first run.

savehist is IMO not a good candidate for a minor mode because turning
it on and off during an Emacs session does not make much sense.  Also
note that XEmacs doesn't support `define-minor-mode', so it would
complexify the code.

> There are also minor changes: don't save empty histories, and try to
> avoid computing the checksum when no minibuffer input occurred since
> last check.

That breaks savehist-additional-variables.  Not saving empty
minibuffer histories does not buy us much, now that we only save the
histories that have actually been used at least once.  And it would
also break savehist-additional-variables when the intent is to save a
nil variable.

I thought of removing the savehist-additional-variables feature
(unlike in 1997, you can save options using Customize now), but it
still seems the best way to persist modifiable variables like
`compile-command'.

Re: Saving minibuffer history

[Miles Bader]

2005/10/25, Hrvoje Niksic <hniksic@xemacs.org>:
> > E.g. please preserve the arch-tag.
>
> I don't know what it does and I don't use arch.  Is it always the
> same?

Yes

> What purpose does it serve?

It gives the file a unique identity.

-Miles
--
Do not taunt Happy Fun Ball.

Re: Saving minibuffer history

[Hrvoje Niksic]

Miles Bader <snogglethorpe@gmail.com> writes:

>> I don't know what it does and I don't use arch.  Is it always the
>> same?
>
> Yes
>
>> What purpose does it serve?
>
> It gives the file a unique identity.

I've now added it to the file.  Will it cause problems if the file is
added to other arch repositories?

Maybe I'm just being paranoid, but I tend to be extra careful with the
VC-generated stuff in source files.

Re: Saving minibuffer history

[Miles Bader]

2005/10/25, Hrvoje Niksic <hniksic@xemacs.org>:
> I've now added it to the file.  Will it cause problems if the file is
> added to other arch repositories?

It shouldn't.  If they're using tagline mode, it should give them a
unique id to use... :-)

-miles
--
Do not taunt Happy Fun Ball.

Re: Saving minibuffer history

[Stefan Monnier]

> You never informed me about those changes so I missed them.  I did
> notice the changes in the patch you sent me.  Please remember that
> this package is normally also maintained for XEmacs, and as an
> external package (although it probably doesn't have many external
> users).

Sorry.

> I also don't understand why you removed the "*" markers from the
> beginning of docstrings.  Those variables *are* intended to be "set
> and modified by users".

All defcustom vars can be set via M-x set-variable: the * has no effect on
them.

>> E.g. please preserve the arch-tag.
> I don't know what it does and I don't use arch.  Is it always the
> same?  What purpose does it serve?

By its very nature it should never ever change: it represents the identity
of the file, so that if you send it by email, edit it, move it or whatever
else, Arch can still figure out which file is which.

This can even be used for files which live in several repositories (e.g. in
Emacs and in Gnus), since the arch-tag is supposed to be globally unique.

> > It [...] introduces savehist-mode, which would become the main
> > advertized way to turn the feature ON (can also be done via
> > Customize) in place of savehist-load.

> I'm not sure that's a good idea.  savehist-load replaces existing
> minibuffer histories with the loaded ones, which is why it is
> recommended to only call it from `.emacs'.  Since turning on a
> savehist mode calls `savehist-load', it has the nasty side effect of
> ruining existing minibuffer histories.  This is alleviated by the use
> of `savehist-loaded', but it will still ruin the minibuffer histories
> when savehist is first run.

It's a risk, but the same risk exists with savehist-load.  So the same
"solution" should work as well: warn that it should only be used in your
.emacs.

Maybe an alternative is to not do the load immediately, but instead
place it on emacs-startup-hook, so that if it is executed after
emacs-startup-hook was run, it won't be run at all.

> savehist is IMO not a good candidate for a minor mode because turning
> it on and off during an Emacs session does not make much sense.

Yet you provided savehist-uninstall.  To me, the main benefit of making it
a minor mode is that it follows the usual rules of customization.  No need
to read the Commentary, or look through docstrings to find the entry point.
It can even be set via Custom.
Most minor modes are never turned off.

> Also note that XEmacs doesn't support `define-minor-mode', so it would
> complexify the code.

That can be fixed easily.


        Stefan

Re: Saving minibuffer history

[Stefan Monnier]

> Since turning on a savehist mode calls `savehist-load', it has the nasty
> side effect of ruining existing minibuffer histories.

BTW, one solution is to make sure savehist-load doesn't ruin
existing history.  For that ~/.emacs-history should be changed from

         (setq foo1 bar1)
         (setq foo2 bar2)
         ...
to
        (savehist-set-history
         foo1 bar1
         foo2 bar2
         ...)

and then define savehist-set-history to *merge* bar1/bar2/... with the
already existing value of foo1/foo2/...
"Merge" here is probably something along the lines of `append' or
(delete-duplicate (append ...)).


        Stefan

Re: Saving minibuffer history

[Richard M. Stallman]

    I also recommend that you mention in the NEWS file how the feature is
    supposed to be used.  It can be as simple as:

	To use this feature, put (savehist-load) in your `.emacs' file.

I'll do that.  Thanks.

Re: Saving minibuffer history

[Hrvoje Niksic]

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

>> I also don't understand why you removed the "*" markers from the
>> beginning of docstrings.  Those variables *are* intended to be "set
>> and modified by users".
>
> All defcustom vars can be set via M-x set-variable: the * has no
> effect on them.

That is because in Emacs user-variable-p also returns t on custom
variables.  That is not the case in XEmacs, which still expects *.

> It's a risk, but the same risk exists with savehist-load.  So the
> same "solution" should work as well: warn that it should only be
> used in your .emacs.

I believe you're on to something with the minor mode thing.  It
definitely looks better than the savehist-install/savehist-uninstall
pair (which I just introduced anyway, so it's not like we must support
them for compatibility).

>> savehist is IMO not a good candidate for a minor mode because
>> turning it on and off during an Emacs session does not make much
>> sense.
>
> Yet you provided savehist-uninstall.

Only now, and only to complement savehist-install, which I introduced
to clean up savehist-load.

I think I'll give the mode idea a shot.  I'll try to make it
compatible with XEmacs.

Re: Saving minibuffer history

[Hrvoje Niksic]

I've now given the minor mode a try.  I used your basic idea with some
minor changes:

* Code it using add-minor-mode to make it XEmacs-compatible.

* Make `savehist-load' an obsolete alias for (savehist-mode 1).

I also imported your change that empty minibuffer histories are not
saved, but restricted it to minibuffer histories.  While at it, made
savehist-length not apply to savehist-additional-variables.

As usual, the latest version is available at
<http://fly.srk.fer.hr/~hniksic/emacs/savehist.el>.

Thanks for your help.

Re: Saving minibuffer history

[Richard M. Stallman]

    A patch against what?  I'm not sure which was the version you last
    picked up.  A patch against the version in your CVS does not seem all
    that useful.

In general, the best thing to do is send a patch showing your changes
from the last version we sync'd up with.

Re: Saving minibuffer history

[Stefan Monnier]

> As usual, the latest version is available at
> <http://fly.srk.fer.hr/~hniksic/emacs/savehist.el>.

Installed.  I applied the following additional patch.


        Stefan


2005-11-01  Stefan Monnier  <monnier@iro.umontreal.ca>

	* savehist.el (savehist-mode) <defcustom>:
	Use custom-set-minor-mode if available.
	(savehist-mode) <defun>: Run the minor mode hook, set the custom state
	and emit a message if applicable.


--- savehist.el	01 nov 2005 00:31:48 -0500	1.9
+++ savehist.el	01 nov 2005 01:05:45 -0500	
@@ -40,7 +40,7 @@
 ;; or with customize: `M-x customize-option RET savehist-mode RET'.
 ;;
 ;; You can also explicitly save history with `M-x savehist-save' and
-;; load it by loading the `savehist-file' with `M-x load'.
+;; load it by loading the `savehist-file' with `M-x load-file'.
 
 ;; If you are using a version of Emacs that does not ship with this
 ;; package, be sure to have `savehist.el' in a directory that is in
@@ -64,7 +64,9 @@
 Set this by calling the `savehist-mode' function or using the customize
 interface."
   :type 'boolean
-  :set (lambda (symbol value) (savehist-mode (or value 0)))
+  :set (if (fboundp 'custom-set-minor-mode)
+           'custom-set-minor-mode
+         (lambda (symbol value) (funcall symbol (or value 0))))
   :initialize 'custom-initialize-default
   :require 'savehist
   :group 'savehist)
@@ -128,12 +130,17 @@
   :group 'savehist)
 
 (defcustom savehist-save-hook nil
-  "Hook called by savehist-save before saving the variables.
+  "Hook called by `savehist-save' before saving the variables.
 You can use this hook to influence choice and content of variables to
 save."
   :type 'hook)
 
-(defvar savehist-coding-system (if (featurep 'xemacs) 'iso-2022-8 'utf-8)
+(defvar savehist-coding-system
+  ;; UTF-8 is usually preferable to ISO-2022-8 when available, but under
+  ;; XEmacs, UTF-8 is provided by external packages, and may not always be
+  ;; available, so even if it currently is available, we prefer not to
+  ;; use is.
+  (if (featurep 'xemacs) 'iso-2022-8 'utf-8)
   "The coding system savehist uses for saving the minibuffer history.
 Changing this value while Emacs is running is supported, but considered
 unwise, unless you know what you are doing.")
@@ -150,14 +157,15 @@
 along with minibuffer history.  You can change its value off
 `savehist-save-hook' to influence which variables are saved.")
 
-;; Coding system without any conversion, used for calculating an
-;; internal checksum.  Should be as fast as possible, ideally simply
-;; exposing the internal representation of buffer text.
-(defconst savehist-no-conversion (if (featurep 'xemacs) 'binary 'no-conversion))
-
-;; Whether the history has already been loaded.  This prevents
-;; toggling savehist-mode from destroying existing minibuffer history.
-(defvar savehist-loaded nil)
+(defconst savehist-no-conversion (if (featurep 'xemacs) 'binary 'no-conversion)
+  "Coding system without conversion, used for calculating internal checksums.
+Should be as fast as possible, ideally simply exposing the internal
+representation of buffer text.")
+
+(defvar savehist-loaded nil
+  "Whether the history has already been loaded.
+This prevents toggling `savehist-mode' from destroying existing
+minibuffer history.")
 
 (eval-when-compile
   (when (featurep 'xemacs)
@@ -201,7 +208,18 @@
 	 (setq savehist-mode nil)
 	 (savehist-uninstall)
 	 (signal (car errvar) (cdr errvar)))))
-    (savehist-install)))
+    (savehist-install))
+
+  ;; End with the usual minor-mode conventions normally provided
+  ;; transparently by define-minor-mode.
+  (run-hooks 'savehist-mode-hook)
+  (if (interactive-p)
+      (progn
+        (customize-mark-as-set 'savehist-mode)
+        (unless (current-message)
+          (message "Savehist mode %sabled" (if savehist-mode "en" "dis")))))
+  ;; Return the new setting.
+  savehist-mode)
 (add-minor-mode 'savehist-mode "")
 
 (defun savehist-load ()