all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Philipp Stephani <p.stephani2@gmail.com>
To: Drew Adams <drew.adams@oracle.com>, 24706@debbugs.gnu.org
Subject: bug#24706: 26.0.50; Minor mode functions should do strict argument type checking
Date: Sun, 16 Oct 2016 18:13:01 +0000	[thread overview]
Message-ID: <CAArVCkQ92m0tibx_ApbLj-Zo0514zQmv3cQCHV+26BbXwq8BtQ@mail.gmail.com> (raw)
In-Reply-To: <dbc67c4f-173b-45e7-a1c3-5c588824cd73@default>

[-- Attachment #1: Type: text/plain, Size: 2460 bytes --]

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.

[-- Attachment #2: Type: text/html, Size: 3623 bytes --]

  parent reply	other threads:[~2016-10-16 18:13 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-16 15:45 bug#24706: 26.0.50; Minor mode functions should do strict argument type checking Philipp Stephani
2016-10-16 16:12 ` Drew Adams
2016-10-16 16:31   ` Noam Postavsky
2016-10-16 16:49     ` Drew Adams
2016-10-16 18:01     ` Eli Zaretskii
2016-10-16 18:13   ` Philipp Stephani [this message]
2016-10-16 18:25     ` Philipp Stephani
2016-10-16 18:51       ` Eli Zaretskii
2017-04-23 17:51         ` Philipp Stephani
2017-04-26 11:26           ` Eli Zaretskii
2020-08-01 20:47             ` Philipp Stephani
2020-08-02 16:13               ` Eli Zaretskii
2020-10-01 12:12                 ` Stefan Kangas
2016-10-16 18:53     ` Eli Zaretskii
2016-10-16 19:50     ` Drew Adams

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAArVCkQ92m0tibx_ApbLj-Zo0514zQmv3cQCHV+26BbXwq8BtQ@mail.gmail.com \
    --to=p.stephani2@gmail.com \
    --cc=24706@debbugs.gnu.org \
    --cc=drew.adams@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.