unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#25428: 25.1; Incorrect doc string for `delete-selection-mode'
@ 2017-01-12 16:24 Drew Adams
  2017-08-17  1:55 ` N. Jackson
  0 siblings, 1 reply; 5+ messages in thread
From: Drew Adams @ 2017-01-12 16:24 UTC (permalink / raw)
  To: 25428

The doc string says this:

 If called from Lisp, enable the mode if ARG is omitted or nil.

That is completely wrong.  This is an ordinary minor mode, defined using
`define-minor-mode', whose doc string says this:

 When called from Lisp, the mode command toggles the mode if the
 argument is ‘toggle’, disables the mode if the argument is a
 non-positive integer, and enables the mode otherwise (including
 if the argument is omitted or nil or a positive integer).

E.g., `(delete-selection-mode t)' ENABLES the mode, even though the
`delete-selection-mode' doc string says that it DISABLES it.


In GNU Emacs 25.1.1 (x86_64-w64-mingw32)
 of 2016-09-17 built on LAPHROAIG
Windowing system distributor 'Microsoft Corp.', version 6.1.7601
Configured using:
 'configure --without-dbus --without-compress-install CFLAGS=-static'





^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#25428: 25.1; Incorrect doc string for `delete-selection-mode'
  2017-01-12 16:24 bug#25428: 25.1; Incorrect doc string for `delete-selection-mode' Drew Adams
@ 2017-08-17  1:55 ` N. Jackson
  2017-08-17  2:24   ` npostavs
  2017-08-17  3:25   ` Drew Adams
  0 siblings, 2 replies; 5+ messages in thread
From: N. Jackson @ 2017-08-17  1:55 UTC (permalink / raw)
  To: Drew Adams; +Cc: 25428

At 07:24 -0800 on Thursday 2017-01-12, Drew Adams wrote:

> The doc string says this:
>
>   If called from Lisp, enable the mode if ARG is omitted or nil.
>
> That is completely wrong. This is an ordinary minor mode,
> defined using `define-minor-mode', whose doc string says this:
>
>   When called from Lisp, the mode command toggles the mode if
>   the argument is ‘toggle’, disables the mode if the argument is
>   a non-positive integer, and enables the mode otherwise
>   (including if the argument is omitted or nil or a positive
>   integer).

Hi Drew,

I do not see any contradiction between these two statements. Maybe
one of us is misreading them?

In particular, I find that the behaviour of
`delete-selection-mode' (the function) when called from Lisp
matches both statements in every particular, so I don't see what
you mean by "completely wrong".

> E.g., `(delete-selection-mode t)' ENABLES the mode, even though the
> `delete-selection-mode' doc string says that it DISABLES it.

Yes, this enables the mode, but no, the doc string does *not* say
this disables it. (In fact, it is silent on how to disable it from
Lisp.)

Am I missing something?

Admittedly, the part of its doc string that says how to use the
`delete-selection-mode' function from Lisp seems inadequate: It
doesn't say how to toggle the mode, it doesn't say how to disable
the mode, and the methods it mentions for enabling the mode do not
include the most straightforward one (passing it `t').

[Temporary minor rant: Personally it seems absurd to me that a
`nil' argument doesn't turn a minor mode off; and that calling the
minor mode function with no argument doesn't toggle it. But that
is all history of course.]

Anyway, does it make sense to re-document the canonical behaviour
of minor mode commands in the doc string of every minor mode
command?

Maybe it would be better (as I think you might have suggested in
another bug report back in January) if the doc string of the each
minor mode command had a link to the documentation of the
canonical behaviour.

N.






^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#25428: 25.1; Incorrect doc string for `delete-selection-mode'
  2017-08-17  1:55 ` N. Jackson
@ 2017-08-17  2:24   ` npostavs
  2017-08-17  3:25   ` Drew Adams
  1 sibling, 0 replies; 5+ messages in thread
From: npostavs @ 2017-08-17  2:24 UTC (permalink / raw)
  To: N. Jackson; +Cc: 25428

nljlistbox2@gmail.com (N. Jackson) writes:

> [Temporary minor rant: Personally it seems absurd to me that a
> `nil' argument doesn't turn a minor mode off; and that calling the
> minor mode function with no argument doesn't toggle it. But that
> is all history of course.]

There is a good reason to have it like that: it allows doing

    (add-hook 'some-hook #'the-minor-mode)

Rather than having to do

    (add-hook 'some-hook (lambda () (the-minor-mode +1)))

or

    (defun turn-on-the-minor-mode () (the-minor-mode +1))
    (add-hook 'some-hook #'turn-on-the-minor-mode)





^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#25428: 25.1; Incorrect doc string for `delete-selection-mode'
  2017-08-17  1:55 ` N. Jackson
  2017-08-17  2:24   ` npostavs
@ 2017-08-17  3:25   ` Drew Adams
  2017-08-26  9:03     ` Eli Zaretskii
  1 sibling, 1 reply; 5+ messages in thread
From: Drew Adams @ 2017-08-17  3:25 UTC (permalink / raw)
  To: nljlistbox2; +Cc: 25428

Let's just say that the doc string is incomplete and can easily
mislead.

If it says anything about interactive behavior (and it should)
then it should say just what the description of `define-minor-mode'
says for minor modes.

If it says anything about the behavior when called from
Lisp (and it should) then it should say just what the d-m-m
description says. This (i.e., _all_ of the d-m-m description)
is missing from the d-s-m doc string:

* `toggle' toggles
* non-positive integer disables
* anything else enables

The only bit of info you get about Lisp behavior is that
nil/absent enables - part of the "anything else".

Yes, the doc string does not say that (d-s-m t) disables.
But a cursory reading of this:

 "If called from Lisp, enable the mode if ARG is omitted or nil."

can give that impression.  Sure, it says "if" and not "only if",
but we all know that may people will understand "only if" or
"if and only if".

[And you can see that explicitly in the thread of bug #25435:
after reading such a description the filer thought it implied
that passing `off' should turn it off.  To which the snarky
reply was to belittle the filer's sense telling him:

 "Surely common sense tells you that is the intended meaning?
  There are about 130 instances of this form in Emacs.
  There are 4 instances of ", also enable", maybe you prefer
  that?
  Feel feel to correct them all, I guess, if it bothers you
  that much. :)"]

And as I said in the thread for bug #13926:

 "This makes no connection between the interactive prefix arg
  and the arg when called from Lisp.  In particular, it can
  also give the incorrect impression that the mode is enabled
  ONLY if ARG is omitted or nil.  There is nothing that suggests
  the behavior of a non-positive or positive ARG when called
  from Lisp."

It is also misleading for the doc string to talk about ARG
as if it were the prefix arg.

Right away that is going to feed confusion: absence of a
prefix arg toggles, but in Lisp absence of ARG enables.

Rather than the doc making clear the difference between
prefix arg interactively and ARG from Lisp, it seems to go
out of its way to confuse the two, saying "With a prefix ARG"
instead of "With a prefix arg" (or better, "prefix argument").
Better still would be to name the parameter something else
than ARG, to avoid just such confusion.

The behavior of minor-mode arguments is complex enough,
without bad doc to confuse things further.

It really should be clear by now that clearer doc is needed
about the behavior of minor-mode arguments (and there
continue to be questions and confusions about on reddit,
stackexchange, etc.).  How hard is it get this right now,
especially when writing new doc strings?

The point is that there is no reason to say anything
different for this than what is true for all minor modes
and what is said in the doc for d-m-m.

No, a user of a minor-mode function should not need to read
the doc of d-m-m to obtain info about its behavior.

Yes, each command should have a doc string that describes
its behavior: (1) interactively and (2) via Lisp.  (And if
a command is not intended for use from Lisp then that should
be stated explicitly.)

Do I think that it would be sufficient for every command
defined with d-m-m to have a link to the doc string of
d-m-m?  It would be OK, I think.  But barring that, each
minor-mode doc string needs to stand on its own and provide
that same info - not something partial or misleading.





^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#25428: 25.1; Incorrect doc string for `delete-selection-mode'
  2017-08-17  3:25   ` Drew Adams
@ 2017-08-26  9:03     ` Eli Zaretskii
  0 siblings, 0 replies; 5+ messages in thread
From: Eli Zaretskii @ 2017-08-26  9:03 UTC (permalink / raw)
  To: Drew Adams; +Cc: 25428-done, nljlistbox2

> Date: Wed, 16 Aug 2017 20:25:31 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: 25428@debbugs.gnu.org
> 
> Let's just say that the doc string is incomplete and can easily
> mislead.
> 
> If it says anything about interactive behavior (and it should)
> then it should say just what the description of `define-minor-mode'
> says for minor modes.
> 
> If it says anything about the behavior when called from
> Lisp (and it should) then it should say just what the d-m-m
> description says. This (i.e., _all_ of the d-m-m description)
> is missing from the d-s-m doc string:
> 
> * `toggle' toggles
> * non-positive integer disables
> * anything else enables

Thanks, I fixed the doc string to include the missing information.





^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-08-26  9:03 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-12 16:24 bug#25428: 25.1; Incorrect doc string for `delete-selection-mode' Drew Adams
2017-08-17  1:55 ` N. Jackson
2017-08-17  2:24   ` npostavs
2017-08-17  3:25   ` Drew Adams
2017-08-26  9:03     ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).