bug#24706: 26.0.50; Minor mode functions should do strict argument type checking

[Philipp Stephani]

Philipp Stephani <p.stephani2@gmail.com> schrieb am So., 16. Okt. 2016 um 20:13 Uhr:

Drew Adams <drew.adams@oracle.com> schrieb am So., 16. Okt. 2016 um 18:12 Uhr:

> (define-minor-mode foo-mode nil)
> its docstring will be
>
> "Toggle Foo mode on or off.
> With a prefix argument ARG, enable Foo mode if ARG is
> positive, and disable it otherwise.  If called from Lisp, enable
> the mode if ARG is omitted or nil, and toggle it if ARG is ‘toggle’."
>
> This appears to indicate that (foo-mode 'banana)
> should disable foo-mode, but it enables it.

No, it does not suggest that.  But to be clearer, it should
probably explicitly address the non-nil and non-`toggle' case,
like so:

  If called from Lisp, enable the mode if ARG is omitted or
  nil, toggle it if ARG is ‘toggle’, and disable it if ARG is
  any other non-nil value.

(And place the Lisp description in a separate paragraph
from the interactive description.)

> I think minor modes should simply not allow anything but
> integers and 'toggle for ARG, avoiding this confusion.

Why?  There is no confusion possible, once the doc string
explicitly speaks about all possible argument values.

Why would you change the behavior, instead of just clarifying
the doc?

I generally prefer the behavior to be as strict as possible. Consider

(foo-mode 'disable)

If you read such code, do you assume that this enables foo-mode?

However, in this case I guess it's too late, and fixing the documentation is indeed more appropriate. BTW, the Elisp manual has the same issue:

     The mode command should accept one optional argument.  If called

     interactively with no prefix argument, it should toggle the mode

     (i.e., enable if it is disabled, and disable if it is enabled).  If

     called interactively with a prefix argument, it should enable the

     mode if the argument is positive and disable it otherwise.

     If the mode command is called from Lisp (i.e., non-interactively),

     it should enable the mode if the argument is omitted or ‘nil’; it

     should toggle the mode if the argument is the symbol ‘toggle’;

     otherwise it should treat the argument in the same way as for an

     interactive call with a numeric prefix argument, as described

     above.

Probably this should be reworded so that the Lisp case doesn't refer to the interactive case at all. Making the documentation obvious is more important than avoiding repetition.

Attached a patch that uses the wording from `define-minor-mode'. 

0001-Clarify-the-behavior-of-minor-mode-commands.txt
Description: Text document