Re: obsolete comment in tool-bar.el

[Luc Teirlinck]

Stefan Monnier wrote:

   >    Miles complained loudly (in the form of comments, some of which
   >    may still be in the current elisp code ;-)
   > These comments make no sense.

   If you do not understand them, I fear you may not understand the problem
   well enough to judge what's the least bad solution.

Well I should not have said "these comments make no sense", as I may
not have found all of them.  It would also have been more accurate to
say "your solution to these comments makes no sense".  What I found
were two instances of the following comment:

;;; Note this definition must be at the end of the file, because
;;; `define-minor-mode' actually calls the mode-function if the
;;; associated variable is non-nil, which requires that all needed
;;; functions be already defined.  [This is arguably a bug in d-m-m]

You can solve the only problem pointed out in this comment in two
ways.  The ideal one is to come up with a solution that does not
require define-minor-mode to actually call the mode function.  The
second is to put the call to the define-minor-mode after all functions
used by the minor mode, for instance at the end of the file.  That may
require a compiler defvar, but that is not the end of the world.

There are several things wrong with your solution.  The call to the
minor mode becomes de facto part of the initialization of the minor
mode defcustom.  Therefore the ":initialize 'custom-initialize-default"
is misleading.  The motivation given for that :initialize function is
that loading the file should not call the mode function.  But the file
calls the minor mode function anyway (via an eval-after-load), in a
way that misleads people into believing it will not be.

The second problem is much worse.  In `(elisp)Hooks for Loading' it is
stated that well-designed Lisp programs should not use
eval-after-load.  There is a very good reason for that.  The kind of
use of eval-after-load made by define-minor-mode means that
define-minor-mode behaves differently when called interactively than
when called from a file.  This destroys the interactive nature of
Elisp.  For instance, the eval-after-load makes the interactive
testing (for instance in ielm) of any Elisp code that directly or
indirectly calls define-minor-mode unreliable.  You can only reliably
test such code by loading files.  (As I noticed.)

The two above problems seem to me to be worse than the problems caused
by a compiler defvar and a call to define-minor-mode late in the file.
If it really is impossible to get rid of the call to the mode function
(something I am not convinced of), then I believe that you should get
rid of the eval-after-load and clearly state in the define-minor-mode
docstring that it can call the mode function and state what the
consequences of that are.

Sincerely,

Luc.