[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: savehist -- save minibuffer history between Emacs invocations
From: |
Hrvoje Niksic |
Subject: |
Re: savehist -- save minibuffer history between Emacs invocations |
Date: |
Mon, 31 Oct 2005 17:49:58 +0100 |
User-agent: |
Gnus/5.1006 (Gnus v5.10.6) Emacs/21.4 (gnu/linux) |
Simon Josefsson <address@hidden> writes:
>> And from the available documentation I understand define-minor-mode
>> to be pretty much equivalent to the combination of `add-minor-mode'
>> and the code I've written.
>
> define-minor-mode also create a customizable foo-mode variable,
> which load and enable the mode if it is customized in ~/.emacs.
Hmm.
(define-minor-mode foo-mode "blah")
(symbol-plist 'foo-mode)
-> (variable-documentation "Non-nil if Foo mode is enabled.
Use the command `foo-mode' to change this variable.")
Are you sure it gives the variable any special properties recognized
by custom? I should at least have a `custom-type' property.
>> If you know how to correctly achieve that with `defcustom', by all
>> means let me know.
>
> How about this patch?
[...]
> -;; (savehist-mode 1)
> +;; (require 'savehist)
This is bad style -- libraries are not supposed to change the state of
Emacs by merely being loaded. From the "Coding Conventions" node of
the Elisp manual:
* When a package provides a modification of ordinary Emacs behavior,
it is good to include a command to enable and disable the feature,
Provide a command named `WHATEVER-mode' which turns the feature on
or off, and make it autoload (*note Autoload::). Design the
package so that simply loading it has no visible effect--that
should not enable the feature.(2) Users will request the feature
by invoking the command.
(2) Consider that the package may be loaded arbitrarily by Custom
for instance.
> -(defvar savehist-mode nil
> - "Mode for automatic saving of minibuffer history.
> -Set this by calling `savehist-mode'.")
> +(defcustom savehist-mode nil
> + "Mode for automatic saving of minibuffer history."
> + :type 'boolean
> + :require 'savehist
> + :group 'savehist)
That requires loading savehist to install savehist-mode, which is
wrong. I was thinking of something along the lines of
:set (lambda (val) (savehist-mode (if val 1 0))), but I couldn't
really make it work.