Re: obsolete comment in tool-bar.el

[Luc Teirlinck <teirllm@dms.auburn.edu>]

Stefan Monnier wrote:
   
   Please check the commit-history of this piece of code (e.g. with
   vc-annotate) to see that it used to do what you say it "should" do
   (i.e. also call the minor-mode function if the init-value is non-nil).

First of all, I assume the lack of reaction to my patch means that you
no longer object to it.  I will wait another day or so to make sure
that this is indeed the case.  I will have to do something reasonably
soon, since without that patch (or proper alternative) Emacs fails to
build on certain operating systems.  To answer the above quote, the
define-minor-mode code used to contain:

;; If the mode is global, call the function according to the default.
,(if globalp `(if ,mode (,mode 1))))))

until you started making various changes to this section of
define-minor-mode.  The original code I quoted above had the
shortcoming of performing part of the initialization outside of the
defcustom, which can be confusing.  But it _did_ enable the mode if
the defcustom sat the variable to t and that was the intended purpose
of the code, as the comment makes clear.

   The current code follows the following idea:

   Loading a file should change Emacs's state as little as possible, so the
   init-value of a minor mode should *describe* (not determine) the default
   state of the minor-mode (in the case where the minor-mode function is not
   executed).

I understood that.  However setting the mode variable to t without
enabling the mode has some very bad consequences, to the point that I
believe it could be considered an outright bug.  If the mode has a
lighter, the mode line claims that the mode is enabled, whereas it is
not, confusing the user.  Even though the mode variable is non-nil,
the mode is disabled, so `M-x foo-mode' should enable it.  Instead,
`M-x foo-mode' is actually a no-op in this situation, which is also
very confusing to the user.

With the current code (with or without the code I propose to delete),
if you want to enable a global minor mode by default, you have to
preload the library that defines the minor mode before startup.el is
loaded and then enable the mode by calling the minor mode function in
startup.el.  (Calling it in startup.el instead of in the file defining
the minor mode prevents calling it again if for some reason the file
gets reloaded.)  This way you avoid the bugs described above.  Of
course, the author of a package not included with Emacs can not
preload his file.  So the author of such a file should put
`(if foo-mode (foo-mode 1))' in his file, at a place late enough that
calling foo-mode does not produce any errors.

One solution is to just document the above.  (Many people do not know
it.)  This would have the advantage that, even after the code I
propose to remove is removed, define-minor-mode does not need to be
called near the end of the file, because it would never call the minor
mode function.

Another solution is to change the :initialize function.  There are
some reasons why this would appear to be cleaner.  However, it would
require changing the order in which define-minor-mode does things and,
_if_ a global minor mode is enabled by default, then define-minor-mode
would have to be called at a place in the file where calling the minor
mode function will not lead to an error.

Sincerely,

Luc.