cua: quiet warning messages

cua: quiet warning messages

[Michael Mauger]

I've noticed that when CUA starts up under 21.3.50 that it issues warning
messages when delete-selection-mode and pc-selection-mode are disabled. 
Actually the error is issued by the modes themselves.  The warnings are:

Toggling delete-selection-mode off; better pass an explicit argument.
Toggling pc-selection-mode off; better pass an explicit argument.

This patch corrects this by passing the explicit argument.

Index: emacs/lisp/emulation/cua-base.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/emulation/cua-base.el,v
retrieving revision 1.24
diff -b -u -r1.24 cua-base.el
--- emacs/lisp/emulation/cua-base.el	18 Apr 2003 22:49:20 -0000	1.24
+++ emacs/lisp/emulation/cua-base.el	14 Jun 2003 23:20:19 -0000
@@ -1204,9 +1204,9 @@
 	   (and (boundp 'delete-selection-mode) delete-selection-mode)
 	   (and (boundp 'pc-selection-mode) pc-selection-mode)))
     (if (and (boundp 'delete-selection-mode) delete-selection-mode)
-	(delete-selection-mode))
+	(delete-selection-mode -1))
     (if (and (boundp 'pc-selection-mode) pc-selection-mode)
-	(pc-selection-mode))
+	(pc-selection-mode -1))
     (setq transient-mark-mode (and cua-mode
 				   (if cua-highlight-region-shift-only
 				       (not cua--explicit-region-start)


__________________________________
Do you Yahoo!?
SBC Yahoo! DSL - Now only $29.95 per month!
http://sbc.yahoo.com

Re: cua: quiet warning messages

[Luc Teirlinck]

Michael Mauger wrote:

   I've noticed that when CUA starts up under 21.3.50 that it issues warning
   messages when delete-selection-mode and pc-selection-mode are disabled. 
   Actually the error is issued by the modes themselves.  The warnings are:

   Toggling delete-selection-mode off; better pass an explicit argument.
   Toggling pc-selection-mode off; better pass an explicit argument.

These warnings are issued by define-minor-mode.  As a consequence,
modes defined by define-minor-mode behave differently from modes
defined by other means.  The behavior of these minor modes is
inconsistent with the behavior as explained in the Emacs manual:

    With no argument, the function turns the mode on if it was
    off and off if it was on.  This is known as "toggling".  A positive
    argument always turns the mode on, and an explicit zero argument or a
    negative argument always turns it off.

Elisp manual:

     The command should accept one optional argument.  If the argument
     is `nil', it should toggle the mode (turn it on if it is off, and
     off if it is on).  Otherwise, it should turn the mode on if the
     argument is a positive integer, a symbol other than `nil' or `-',
     or a list whose CAR is such an integer or symbol; it should turn
     the mode off otherwise.

Note that _any_ symbol other than `nil' or `-' should turn the mode
_on_.  'toggle is a symbol, it is not `nil' and not `-'.

>From the Elisp documentation of define-minor-mode:

     This macro defines a new minor mode whose name is MODE (a
     symbol).  It defines a command named MODE to toggle the minor
     mode, with DOC as its documentation string.

>From the documentation string of define-minor-mode:

    Define a new minor mode MODE.
    This function defines the associated control variable MODE, keymap
    MODE-map, toggle command MODE, and hook MODE-hook.

Nowhere anywhere any mention whatsoever about the argument 'toggle
which according to define-minor-mode's _code_ seems to be the only
stylistically acceptable way to toggle a minor mode non-interactively.
You have to read the code of define-minor-mode to find that out.

If an author fails to carefully read the code and follows the
stylistic conventions documented in all relevant manuals, as well as
the documentation string of define-minor-mode itself, and fails to
follow this completely undocumented stylistic "convention" instead,
define-minor-mode expresses its distaste for the _programmer's_ style,
not through a compiler warning, but by printing an error message to
the _user_ complaining about the author's style.  This seems highly
unusual.

I believe that the bug is in define-minor-mode and should be corrected
there.

Sincerely,

Luc.

Re: cua: quiet warning messages

[Stefan Monnier]

>     With no argument, the function turns the mode on if it was
>     off and off if it was on.  This is known as "toggling".  A positive
>     argument always turns the mode on, and an explicit zero argument or a
>     negative argument always turns it off.

That's exactly how modes defined by define-minor-mode behave.

> Elisp manual:
> 
>      The command should accept one optional argument.  If the argument
>      is `nil', it should toggle the mode (turn it on if it is off, and
>      off if it is on).  Otherwise, it should turn the mode on if the
>      argument is a positive integer, a symbol other than `nil' or `-',
>      or a list whose CAR is such an integer or symbol; it should turn
>      the mode off otherwise.
> 
> Note that _any_ symbol other than `nil' or `-' should turn the mode
> _on_.  'toggle is a symbol, it is not `nil' and not `-'.

That's also how define-minor-mode behaves.

> If an author fails to carefully read the code and follows the
> stylistic conventions documented in all relevant manuals, as well as
> the documentation string of define-minor-mode itself, and fails to
> follow this completely undocumented stylistic "convention" instead,
> define-minor-mode expresses its distaste for the _programmer's_ style,
> not through a compiler warning, but by printing an error message to
> the _user_ complaining about the author's style.  This seems highly
> unusual.

The 90% of the cases where a minor mode is called non-interactively with
a nil argument is when an unsuspecting user says

	(add-hook 'foo-mode-hook 'bar-minor-mode)

The warning is thus there so that when it ends up turning the mode
OFF, it tells the user that it might not be doing what the
user expected.  If the user (or author) really meant to turn the
mode off, she should pass a -1 argument to make it clear that
she knows what she's doing.


	Stefan

Re: cua: quiet warning messages

[Luc Teirlinck]

Stefan Monnier wrote:

   > Note that _any_ symbol other than `nil' or `-' should turn the mode
   > _on_.  'toggle is a symbol, it is not `nil' and not `-'.

   That's also how define-minor-mode behaves.

I defined a minor mode, vis-mode using define-minor-mode.
M-: (vis-mode 'toggle) _does_ toggle.

   The 90% of the cases where a minor mode is called non-interactively with
   a nil argument is when an unsuspecting user says

	   (add-hook 'foo-mode-hook 'bar-minor-mode)

   The warning is thus there so that when it ends up turning the mode
   OFF, it tells the user that it might not be doing what the
   user expected.  If the user (or author) really meant to turn the
   mode off, she should pass a -1 argument to make it clear that
   she knows what she's doing.

If foo-mode always turns bar-minor-mode on, then the above code seems
intentional and should not produce a warning.  Note that, in your
example foo-mode-hook is probably a normal hook, so you can not just
pass an explicit argument, you have to define your own function,
disable-bar-minor-mode or whatever.  The user may not know how to do
that and (add-hook 'foo-mode-hook 'bar-minor-mode) might actually be
stylistically inferior, but works.  For inclusion in Emacs' source
code I _would_ define a disable-bar-minor-mode instead, because it
seems more robust, but why be that stylistically picky for a user's
.emacs.  How does she turn the warning off if she gets tired of it?

If the current behavior would not be changed, then _at least_ it
should be _documented_ in _all_ places I mentioned.

Sincerely,

Luc.

Re: cua: quiet warning messages

[Richard Stallman]

    > Note that _any_ symbol other than `nil' or `-' should turn the mode
    > _on_.  `toggle' is a symbol, it is not `nil' and not `-'.

    That's also how define-minor-mode behaves.

I don't think so.  This code

	 ;; Use `toggle' rather than (if ,mode 0 1) so that using
	 ;; repeat-command still does the toggling correctly.
	 (interactive (list (or current-prefix-arg 'toggle)))
	 (setq ,mode
	       (cond
		((eq arg 'toggle) (not ,mode))

seems to handling `toggle' by toggling the mode,
not by turning it off, which the spec says it should do.

    The warning is thus there so that when it ends up turning the mode
    OFF, it tells the user that it might not be doing what the
    user expected.  If the user (or author) really meant to turn the
    mode off, she should pass a -1 argument to make it clear that
    she knows what she's doing.

You are right, and yet it is also true that this contradicts the
spec.

The spec is not cast in stone; we might want to change it.  But
changes in such a general and basic spec should be thought about as
such, and I don't think they have been.

Can I have your comments and proposals for what to do?

Re: cua: quiet warning messages

[Stefan Monnier]

>     > Note that _any_ symbol other than `nil' or `-' should turn the mode
>     > _on_.  `toggle' is a symbol, it is not `nil' and not `-'.
> 
>     That's also how define-minor-mode behaves.
> 
> I don't think so.  This code
> 
> 	 ;; Use `toggle' rather than (if ,mode 0 1) so that using
> 	 ;; repeat-command still does the toggling correctly.
> 	 (interactive (list (or current-prefix-arg 'toggle)))
> 	 (setq ,mode
> 	       (cond
> 		((eq arg 'toggle) (not ,mode))
> 
> seems to handling `toggle' by toggling the mode,
> not by turning it off, which the spec says it should do.

Indeed.  Although I doubt anybody expects (minor-mode 'toggle) to turn
the mode off unconditionally.

> changes in such a general and basic spec should be thought about as
> such, and I don't think they have been.
> 
> Can I have your comments and proposals for what to do?

As far as I know, minor modes are only ever called with the
following arguments:

	- integers
	- nil
	- t
	- a one-element list containing an integer
	- toggle

This last one is new and only happens when the minor mode is
called interactively.  I think the doc should only describe
the behavior is the above cases and leave the others unspecified.
It could even not specify the behavior in the `toggle' case since
it's only used internally between the interactive spec and
the minor mode's body.

After all, why should (minor-mode (make-hash-table)) turn the mode off
rather than on and why should the doc specify it at all ?


	Stefan

Re: cua: quiet warning messages

[Richard Stallman]

    As far as I know, minor modes are only ever called with the
    following arguments:

	    - integers
	    - nil
	    - t
	    - a one-element list containing an integer
	    - toggle

I think there are others, such as the symbol `-'.

    This last one is new and only happens when the minor mode is
    called interactively.

One problem with specifying this behavior for `toggle' is that many
minor modes don't currently implement it.  They would need to be
changed.

    It could even not specify the behavior in the `toggle' case since
    it's only used internally between the interactive spec and
    the minor mode's body.

That makes it feasible.

How about text for modes.texi?



The command should accept one optional argument.  If the argument is
@code{nil}, it should toggle the mode (turn it on if it is off, and
off if it is on).  It should turn the mode on if the argument is a
positive integer, the symbol @code{t}, or a list whose @sc{car} is one
of those.  It should turn the mode off if the argument is a negative
integer or zero, the symbol @code{-}, or a list whose @sc{car} is one
of those.  The meaning of other arguments is not specified.

Re: cua: quiet warning messages

[Stefan Monnier]

>     As far as I know, minor modes are only ever called with the
>     following arguments:
> 
> 	    - integers
> 	    - nil
> 	    - t
> 	    - a one-element list containing an integer
> 	    - toggle
> 
> I think there are others, such as the symbol `-'.

I can't remember seeing it, but that sounds right.

> How about text for modes.texi?
> 
> 
> 
> The command should accept one optional argument.  If the argument is
> @code{nil}, it should toggle the mode (turn it on if it is off, and
> off if it is on).  It should turn the mode on if the argument is a
> positive integer, the symbol @code{t}, or a list whose @sc{car} is one
> of those.  It should turn the mode off if the argument is a negative
> integer or zero, the symbol @code{-}, or a list whose @sc{car} is one
> of those.  The meaning of other arguments is not specified.

Sounds perfect to me,


	Stefan