Re: obsolete comment in tool-bar.el

[Luc Teirlinck]

Since the current version of `define-minor-mode', is incompatible with
the new :initialize function customize-initialize-safe-default,
_something_ needs to be done.  (It currently breaks the building of
Emacs on certain operating systems and additional uses of
customize-initialize-safe-default could, with the current
define-minor-mode, easily break it on all operating systems.)

So either we apply the patch below, or we have to introduce a
condition-case form in the code removed by that patch.

I propose to apply the patch below.  It removes completely useless
code at the end of define-minor-mode.  I explained in the message I am
replying to and in my message before that one, why that code is not
only useless, but causes several very annoying misfeatures.

Here is a summary of these misfeatures:

1) The code uses eval-after-load, which, according to
   `(elisp)Hooks for Loading', well-designed Lisp programs should not
   use.  For good reason: it destroys the interactive nature of
   Elisp.  Any code that directly or indirectly uses define-minor-mode
   can not be reliably tested interactively, say in the *scratch*
   buffer or ielm.  It can only be tested reliably by loading files,
   as I found out.  My patch would remove this misfeature.

2) Just loading the file may result in the minor mode function being
   called.  It is true that there is precedent for this: the default
   :set function `custom-initialize-reset' has the same problem.  But
   reportedly, define-minor-mode uses custom-initialize-default,
   exactly to avoid that problem.  In vain, unless we apply the patch
   below.

3) The unnatural code I propose to erase makes `define-minor-mode'
   prone to errors if anything else in Emacs changes.  We are
   currently struggling with an example of that.

4) It disguises potential bugs in Elisp code where somebody sets the
   minor mode variable instead of calling the minor mode function, by
   more or less randomly "correcting" the bug for the current session
   only.  This makes debugging Emacs more difficult.

5) By more or less at random correcting _user_ bugs of the type
   discussed in (4), namely setting the minor mode variable with setq
   outside Custom, it confuses the user, because setting the variable
   sometimes appears to work and sometimes not.  If it never worked,
   it would be much easier for the user to realize that it was not the
   correct way to customize the minor mode.  Note that a user using
   setq instead of Custom is probably trying to learn and
   define-minor-mode should not interfere with his learning process.

The code I propose to remove does not take care of a problem which the
original code it replaced did address and which the current comment
still falsely claims it addresses: loading the file does currently
_not_ enable the minor mode if the standard value is non-nil and the
minor mode variable is unbound when the defcustom is evaluated.
_Maybe_ that problem should be taken care of, although that would
inevitably also reintroduce misfeature (2) above, but not in a very
bad way.  As I explained earlier, I believe that the best way to do
that would be to make define-minor-mode use a new :initialize
function.  The patch below does _not_ do that.

===File ~/easy-mmode-diff===================================
*** easy-mmode.el       15 Jul 2005 19:36:26 -0500      1.69
--- easy-mmode.el       18 Jul 2005 19:03:51 -0500      
***************
*** 267,278 ****
         (add-minor-mode ',mode ',lighter
                       ,(if keymap keymap-sym
                          `(if (boundp ',keymap-sym)
!                              (symbol-value ',keymap-sym))))
! 
!        ;; If the mode is global, call the function according to the default.
!        ,(if globalp
!           `(if (and load-file-name (not (equal ,init-value ,mode)))
!                (eval-after-load load-file-name '(,mode (if ,mode 1 -1))))))))
  
  ;;;
  ;;; make global minor mode
--- 267,273 ----
         (add-minor-mode ',mode ',lighter
                       ,(if keymap keymap-sym
                          `(if (boundp ',keymap-sym)
!                              (symbol-value ',keymap-sym)))))))
  
  ;;;
  ;;; make global minor mode
============================================================