bug#3688: Also need (auto-fill-mode nil) to trigger this bug

bug#3688: Also need (auto-fill-mode nil) to trigger this bug

[Kees Bakker]

Hi,

In my report I forgot to run emacs with -Q, but the problem is still
present with -Q. But here is extra information to reproduce the error.

After entering the suggested C text (and emacs is in a basic C mode)
you have to evaluate the following before the error can be reproduced.
   (auto-fill-mode nil)

In the status bar it shows "*C/l Abbrev Fill)". In the buffer you'll
have the following.

   /*type a space here=>
   /* 3 tabs following after this=>			*/

(Notice that there are 3 TABs before the closing C comment.)
If you type a space on the first comment line, you'll see that it jumps
back 21 positions.

If you evaluate this again, the problem goes away.
   (auto-fill-mode nil)
Now, the "Fill" flag is gone from the status bar.

Kind regards,
Kees Bakker

bug#3688: Also need (auto-fill-mode nil) to trigger this bug

[Alan Mackenzie]

Hi, Kees,

On Fri, Jun 26, 2009 at 01:56:48PM +0200, Kees Bakker wrote:
> Hi,

> In my report I forgot to run emacs with -Q, but the problem is still
> present with -Q. But here is extra information to reproduce the error.

> After entering the suggested C text (and emacs is in a basic C mode)
> you have to evaluate the following before the error can be reproduced.
>    (auto-fill-mode nil)

> In the status bar it shows "*C/l Abbrev Fill)". In the buffer you'll
> have the following.

>    /*type a space here=>
>    /* 3 tabs following after this=>			*/

> (Notice that there are 3 TABs before the closing C comment.) If you
> type a space on the first comment line, you'll see that it jumps back
> 21 positions.

OK, I've got it now.  As a matter of interest, M-x auto-fill-mode is
equivalent to your expression.

It also happens in Emacs 22, and it also happens on the console, not just
on a GUI system.  So it looks like a CC mode bug.

Give me a few days to track this down.

> If you evaluate this again, the problem goes away.
>    (auto-fill-mode nil)
> Now, the "Fill" flag is gone from the status bar.

Again, thanks for reporting this bug.

> Kind regards,
> Kees Bakker

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#3688: Also need (auto-fill-mode nil) to trigger this bug

[Alan Mackenzie]

Hi, Kees!

On Fri, Jun 26, 2009 at 01:56:48PM +0200, Kees Bakker wrote:
> Hi,

> In my report I forgot to run emacs with -Q, but the problem is still
> present with -Q. But here is extra information to reproduce the error.

> After entering the suggested C text (and emacs is in a basic C mode)
> you have to evaluate the following before the error can be reproduced.
>    (auto-fill-mode nil)

> In the status bar it shows "*C/l Abbrev Fill)". In the buffer you'll
> have the following.

>    /*type a space here=>
>    /* 3 tabs following after this=>			*/

> (Notice that there are 3 TABs before the closing C comment.) If you
> type a space on the first comment line, you'll see that it jumps back
> 21 positions.  (The 21 appears to be equal to NumOfTabs * (8-1)).

Yes, that's exactly where the 21 comes from.  :-)

> If you evaluate this again, the problem goes away.
>    (auto-fill-mode nil)
> Now, the "Fill" flag is gone from the status bar.

The mechanism for the bug is thus: when doing filling, c-mask-paragraph
temporarily replaces the whitespace before "*/" with a string of "x"s, so
that Emacs's low level filling routine can't put "*/" on a line of its
own.  At the end of the routine, the calculation to restore point does a
spurious correction to correct for an imagined difference between the
size of the WS (24 columns) and the number of bytes (3 tab characters).

Removing this "uncorrection" fixes things.  Please test out the patch
below.

Yidong and Stefan: would it be OK to install this patch for Emacs 23?
The original coding error was my own, made on 2005-08-18 in version 5.314
of cc-cmds.el at SourceForge
(http://cc-mode.cvs.sourceforge.net/viewvc/cc-mode/cc-mode/cc-cmds.el).



2009-06-26  Alan Mackenzie  <acm@muc.de>

	* progmodes/cc-cmds.el (c-mask-paragraph): Remove a spurious
	correction between the visible width of TABs and their number of
	bytes.

Index: cc-cmds.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/progmodes/cc-cmds.el,v
retrieving revision 1.82
diff -c -r1.82 cc-cmds.el
*** cc-cmds.el	13 Feb 2009 14:54:27 -0000	1.82
--- cc-cmds.el	26 Jun 2009 20:54:39 -0000
***************
*** 4202,4209 ****
  	  (forward-char (- hang-ender-stuck))
  	  (if (or fill-paragraph (not auto-fill-spaces))
  	      (insert-char ?\  hang-ender-stuck t)
! 	    (insert auto-fill-spaces)
! 	    (setq here (- here (- hang-ender-stuck (length auto-fill-spaces)))))
  	  (delete-char hang-ender-stuck)
  	  (goto-char here))
  	(set-marker tmp-post nil))
--- 4202,4208 ----
  	  (forward-char (- hang-ender-stuck))
  	  (if (or fill-paragraph (not auto-fill-spaces))
  	      (insert-char ?\  hang-ender-stuck t)
! 	    (insert auto-fill-spaces))
  	  (delete-char hang-ender-stuck)
  	  (goto-char here))
  	(set-marker tmp-post nil))

> Kind regards,
> Kees Bakker

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: bug#3688: Also need (auto-fill-mode nil) to trigger this bug

[Alan Mackenzie]

Hi, Kees!

On Fri, Jun 26, 2009 at 01:56:48PM +0200, Kees Bakker wrote:
> Hi,

> In my report I forgot to run emacs with -Q, but the problem is still
> present with -Q. But here is extra information to reproduce the error.

> After entering the suggested C text (and emacs is in a basic C mode)
> you have to evaluate the following before the error can be reproduced.
>    (auto-fill-mode nil)

> In the status bar it shows "*C/l Abbrev Fill)". In the buffer you'll
> have the following.

>    /*type a space here=>
>    /* 3 tabs following after this=>			*/

> (Notice that there are 3 TABs before the closing C comment.) If you
> type a space on the first comment line, you'll see that it jumps back
> 21 positions.  (The 21 appears to be equal to NumOfTabs * (8-1)).

Yes, that's exactly where the 21 comes from.  :-)

> If you evaluate this again, the problem goes away.
>    (auto-fill-mode nil)
> Now, the "Fill" flag is gone from the status bar.

The mechanism for the bug is thus: when doing filling, c-mask-paragraph
temporarily replaces the whitespace before "*/" with a string of "x"s, so
that Emacs's low level filling routine can't put "*/" on a line of its
own.  At the end of the routine, the calculation to restore point does a
spurious correction to correct for an imagined difference between the
size of the WS (24 columns) and the number of bytes (3 tab characters).

Removing this "uncorrection" fixes things.  Please test out the patch
below.

Yidong and Stefan: would it be OK to install this patch for Emacs 23?
The original coding error was my own, made on 2005-08-18 in version 5.314
of cc-cmds.el at SourceForge
(http://cc-mode.cvs.sourceforge.net/viewvc/cc-mode/cc-mode/cc-cmds.el).



2009-06-26  Alan Mackenzie  <acm@muc.de>

	* progmodes/cc-cmds.el (c-mask-paragraph): Remove a spurious
	correction between the visible width of TABs and their number of
	bytes.

Index: cc-cmds.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/progmodes/cc-cmds.el,v
retrieving revision 1.82
diff -c -r1.82 cc-cmds.el
*** cc-cmds.el	13 Feb 2009 14:54:27 -0000	1.82
--- cc-cmds.el	26 Jun 2009 20:54:39 -0000
***************
*** 4202,4209 ****
  	  (forward-char (- hang-ender-stuck))
  	  (if (or fill-paragraph (not auto-fill-spaces))
  	      (insert-char ?\  hang-ender-stuck t)
! 	    (insert auto-fill-spaces)
! 	    (setq here (- here (- hang-ender-stuck (length auto-fill-spaces)))))
  	  (delete-char hang-ender-stuck)
  	  (goto-char here))
  	(set-marker tmp-post nil))
--- 4202,4208 ----
  	  (forward-char (- hang-ender-stuck))
  	  (if (or fill-paragraph (not auto-fill-spaces))
  	      (insert-char ?\  hang-ender-stuck t)
! 	    (insert auto-fill-spaces))
  	  (delete-char hang-ender-stuck)
  	  (goto-char here))
  	(set-marker tmp-post nil))

> Kind regards,
> Kees Bakker

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#3688: Also need (auto-fill-mode nil) to trigger this bug

[Kees Bakker]

On Friday 26 June 2009, Alan Mackenzie wrote:
> Hi, Kees!

Hi Alan,

> 
> The mechanism for the bug is thus: when doing filling, c-mask-paragraph
> temporarily replaces the whitespace before "*/" with a string of "x"s, so
> that Emacs's low level filling routine can't put "*/" on a line of its
> own.  At the end of the routine, the calculation to restore point does a
> spurious correction to correct for an imagined difference between the
> size of the WS (24 columns) and the number of bytes (3 tab characters).
> 
> Removing this "uncorrection" fixes things.  Please test out the patch
> below.

Yes, the patch fixes it. Thanks for looking into this, and having it solved
so quickly.

BTW. I never intended to enable auto-fill. But strangly enough the following
will _toggle_ auto-fill-mode. (I know, this function is "Toggle Auto Fill mode".
Besides a clear description to turn it on, it should also have a clear
description to turn it off.)

  (auto-fill-mode nil)
-- 
Kees

Re: bug#3688: Also need (auto-fill-mode nil) to trigger this bug

[Stefan Monnier]

> BTW. I never intended to enable auto-fill. But strangly enough the following
> will _toggle_ auto-fill-mode. (I know, this function is "Toggle Auto Fill mode".
> Besides a clear description to turn it on, it should also have a clear
> description to turn it off.)

>   (auto-fill-mode nil)

All minor modes are turned on with a positive arg and turned off with
a negative arg.  A nil argument may toggle it, but that might change in
the future.  A `toggle' argument toggles most (but not all) minor modes.


        Stefan

Re: bug#3688: Also need (auto-fill-mode nil) to trigger this bug

[Kees Bakker]

On Monday 29 June 2009, Stefan Monnier wrote:
> > BTW. I never intended to enable auto-fill. But strangly enough the following
> > will _toggle_ auto-fill-mode. (I know, this function is "Toggle Auto Fill mode".
> > Besides a clear description to turn it on, it should also have a clear
> > description to turn it off.)
> 
> >   (auto-fill-mode nil)
> 
> All minor modes are turned on with a positive arg and turned off with
> a negative arg.  A nil argument may toggle it, but that might change in
> the future.  A `toggle' argument toggles most (but not all) minor modes.

OK. I didn't know that (and I'm using emacs for almost two decades now). Still I
want to express my dislike about the way a "nil" arg is handled.

First, is 0 a positive arg? Some people might think so (I do). It is a bit confusing.
(auto-fill-mode 0) will turn the mode off, so in that sense a zero is not a positive
arg. To summarize

   (auto-fill-mode)         ;;; toggles
   (auto-fill-mode 0)       ;;; turn off
   (auto-fill-mode nil)     ;;; toggles
   (auto-fill-mode -1)      ;;; turn off
   (auto-fill-mode 1)       ;;; turn on
   (auto-fill-mode t)       ;;; turn on

Toggling for a "nil" argument does not make sense to me.
-- 
Kees

Re: bug#3688: Also need (auto-fill-mode nil) to trigger this bug

[Stefan Monnier]

>> All minor modes are turned on with a positive arg and turned off with
>> a negative arg.  A nil argument may toggle it, but that might change in
>> the future.  A `toggle' argument toggles most (but not all) minor modes.

> OK. I didn't know that (and I'm using emacs for almost two decades now).

The use of positive/negative to turn the mode on/off is described in the
docstring as:

  With ARG, turn Auto Fill mode on if and only if arg is positive.

> Still I want to express my dislike about the way a "nil" arg is handled.

I agree.  I'd like to change it to behave like a positive arg, but the
way it works is just a result of the fact that when used interactively
you want the command to toggle when it is not given any prefix argument
and the easiest way to do that is to consider nil as "toggle".

All minor modes defined using `define-minor-mode' now accept (and use,
when called interactively) the value `toggle' to indicate that we want
to toggle.  They also still accept nil to mean "toggle" but this is the
part that I intend to change at some point.

> First, is 0 a positive arg?

If you don't know, don't use it.  There are plenty of obviously-positive
arguments to choose from.

>    (auto-fill-mode -1)      ;;; turn off
>    (auto-fill-mode 1)       ;;; turn on

These are the two canonical ways to turn the mode ON/OFF.

>    (auto-fill-mode)         ;;; toggles
>    (auto-fill-mode nil)     ;;; toggles

auto-fill-mode cannot distinguish between these two cases, so obviously
if one toggles, the other will too.
[ Well, yes, if it used &rest, it could actually distinguish, but since
it doesn't, it can't. ]

>    (auto-fill-mode 0)       ;;; turn off
>    (auto-fill-mode t)       ;;; turn on

These two have been used fairly often, but I strongly discourage
their use.  I think they make about as much sense as

    (auto-fill-mode "Hi John!")
or
    (auto-fill-mode (make-overlay))

> Toggling for a "nil" argument does not make sense to me.

I actually agree.  For various reasons (mostly, for use of the function
on mode hooks), nil should turn the mode ON unconditionally; but history
decided otherwise.


        Stefan

Re: bug#3688: Also need (auto-fill-mode nil) to trigger this bug

[Alan Mackenzie]

Hi, Stefan,

On Mon, Jun 29, 2009 at 10:54:54AM +0200, Stefan Monnier wrote:
> > BTW. I never intended to enable auto-fill. But strangly enough the following
> > will _toggle_ auto-fill-mode. (I know, this function is "Toggle Auto Fill mode".
> > Besides a clear description to turn it on, it should also have a clear
> > description to turn it off.)

> >   (auto-fill-mode nil)

> All minor modes are turned on with a positive arg and turned off with
> a negative arg.  A nil argument may toggle it, but that might change in
> the future.  A `toggle' argument toggles most (but not all) minor modes.

Getting back to the bug again, I've committed the patch to the (new)
trunk, but not yet to the EMACS_23_1_RC branch.  Would it be OK to do
this?

>         Stefan

-- 
Alan Mackenzie (Nürnberg).

Re: bug#3688: Also need (auto-fill-mode nil) to trigger this bug

[Alan Mackenzie]

Hi, Kees!

On Mon, Jun 29, 2009 at 11:18:10AM +0200, Kees Bakker wrote:
> On Monday 29 June 2009, Stefan Monnier wrote:

> > >   (auto-fill-mode nil)

> > All minor modes are turned on with a positive arg and turned off with
> > a negative arg.  A nil argument may toggle it, but that might change
> > in the future.  A `toggle' argument toggles most (but not all) minor
> > modes.

> OK. I didn't know that (and I'm using emacs for almost two decades
> now). Still I want to express my dislike about the way a "nil" arg is
> handled.

OK, but look on the bright side - your confusion led directly to a bug
being fixed.  Your discomfort for everybody's gain.  Think of yourself as
a hero, not a victim.  ;-)

> First, is 0 a positive arg? Some people might think so (I do).

I don't think so, and I'm a graduate mathematician.  But I still remember
long unresolved discussions amongst maths people about whether zero is
positive, and sometimes terms like "strictly positive" and "non-negative"
were used.  Trouble is, "strictly positive" is a bit of a mouthful, and
"non-negative" is a bit, well, negative - do we really want to describe
something by what it isn't rather than what it is?

> It is a bit confusing.  (auto-fill-mode 0) will turn the mode off, so
> in that sense a zero is not a positive arg. To summarize

>    (auto-fill-mode)         ;;; toggles
>    (auto-fill-mode 0)       ;;; turn off
>    (auto-fill-mode nil)     ;;; toggles
>    (auto-fill-mode -1)      ;;; turn off
>    (auto-fill-mode 1)       ;;; turn on
>    (auto-fill-mode t)       ;;; turn on

(auto-fill-mode 0) shouldn't be troublesome to a C hacker.  :-)

> Toggling for a "nil" argument does not make sense to me.

`auto-fill-mode' is also an interactive command, and as such it has an
&optional argument.  The Right Thing to do with "M-x auto-fill-mode" is
to toggle.  The omitted argument becomes nil, hence the the above is
(auto-fill-mode nil) in lisp.  It does make sense, sort of.  But I admit,
I sometimes find it confusing too.

Actually, looking at the doc string for this command:

    Toggle Auto Fill mode.
    With ARG, turn Auto Fill mode on if and only if ARG is positive.
    In Auto Fill mode, inserting a space at a column beyond
    `current-fill-column' automatically breaks the line at a previous
    space.

, it is incomplete in that it doesn't specify what happens when ARG isn't
positive.  Maybe this would be better:

    Turn AutoFill mode on if ARG is positive, turn it off if ARG is zero
    or negative, toggle it if ARG is nil (or absent).

> Kees

-- 
Alan Mackenzie (Nuremberg, Germany).