GNU bug report logs - #193
Fill for // (C++) style comments in C (C99)

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 193 in the body.
You can then email your comments to 193 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Richard Hansen <rhansen <at> bbn.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Fill for // (C++) style comments in C (C99)
Date: Tue, 06 May 2008 17:49:13 -0400

It appears that filling C++-style comments inside C99 code via 
c-fill-paragraph is still broken.  This bug was previously reported[1] 
in November 2006 and was extensively discussed, but it looks like no fix 
was ever committed.

[1] http://lists.gnu.org/archive/html/emacs-devel/2006-11/msg00868.html

-Richard

Message #14 received at 193 <at> debbugs.gnu.org (full text, mbox):

From: Deniz Dogan <deniz.a.m.dogan <at> gmail.com>
To: 193 <at> debbugs.gnu.org
Date: Sun, 27 Jun 2010 14:47:52 +0200

Did anyone ever agree on what needs to be done to fix the problem?

-- 
Deniz Dogan

Message #17 received at 193 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Deniz Dogan <deniz.a.m.dogan <at> gmail.com>
Cc: bug-cc-mode <at> gnu.org, 193 <at> debbugs.gnu.org
Subject: Re: bug#193:
Date: Mon, 5 Jul 2010 20:06:30 +0000

Hi, Deniz,

On Sun, Jun 27, 2010 at 02:47:52PM +0200, Deniz Dogan wrote:
> Did anyone ever agree on what needs to be done to fix the problem?

Er, it basically needs me to get out of bed and fix it.

It seems to be one of these strange bugs where lots of people talked
vaguely around a problem and it took 3 or 4 rounds of chat before
anybody formulated a reproducible recipe.  For some reason I missed it,
and you're the first person to repost the bug (even if a little vagely
;-) onto bug-cc-mode.

So, yes, I'll get around to fixing it with some celerity.  Promise!

> -- 
> Deniz Dogan

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #20 received at 193 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Deniz Dogan <deniz.a.m.dogan <at> gmail.com>
Cc: 193 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
	emacs-devel <at> gnu.org
Subject: Re: bug#193:
Date: Tue, 6 Jul 2010 19:29:34 +0000

On Sun, Jun 27, 2010 at 02:47:52PM +0200, Deniz Dogan wrote:
> Did anyone ever agree on what needs to be done to fix the problem?
 
A quick reminder of what the problem is.  In C mode (NOT C++ mode):
(i) Type M-q in a pair of short line comments like this

// Two short
// lines.

The command ought to join them into a single line comment.  Instead, it
does nothing;

(ii) Type M-q in a line comment which is longer than fill-column:

// A long line comment, which is far far far longer than a moderately or very small fill-column.

This miss-fills by splitting the line, but putting no comment marker on
the new second line:.

// A long line comment, which is far far far longer than a moderately or
very small fill-column.

These bugs are regressions between Emacs 21.4 and 22.1.

#########################################################################

Both these bugs seem to be caused by a strange clause in
`fill-paragraph', where what should be the fill-prefix ("// ") is made
a component of `paragraph-start'.  Thus (ii) every line in the comment
sequence is automatically made into its own paragraph; and (i) when
splitting lines, fill-paragraph doesn't insert the fill-prefix when this
is also a paragraph starter.

Here is the strange code:

     ;; Try to prevent code sections and comment sections from being
     ;; filled together.
     (when (and fill-paragraph-handle-comment comment-start-skip)
       (setq paragraph-start
             (concat paragraph-start "\\|[ \t]*\\(?:"
                     comment-start-skip "\\)")))

In C Mode, `comment-start-skip' is "\\(//+\\|/\\*+\\)\\s *".

The two bugs vanish when the above code is removed.

Stefan, can you remember why you put this code in (revision 86672 of
2008-04-11)?  Is there perhaps a way of achieving the same effect
without making the fill-prefix match `paragraph-start'?

> -- 
> Deniz Dogan

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #23 received at 193 <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Deniz Dogan <deniz.a.m.dogan <at> gmail.com>
Cc: 193 <at> debbugs.gnu.org
Subject: Fix for bug#193.
Date: Fri, 9 Jul 2010 18:51:16 +0000

Hi, Deniz,

On Sun, Jun 27, 2010 at 02:47:52PM +0200, Deniz Dogan wrote:
> Did anyone ever agree on what needs to be done to fix the problem?

Would you try this patch, please, and then tell me whether or not it
works fully.  Thanks!


*** orig/cc-cmds.el	2010-07-05 20:17:46.000000000 +0000
--- cc-cmds.el	2010-07-09 18:31:52.143059496 +0000
***************
*** 4229,4236 ****
    (let ((fill-paragraph-function
  	 ;; Avoid infinite recursion.
  	 (if (not (eq fill-paragraph-function 'c-fill-paragraph))
! 	     fill-paragraph-function)))
!     (c-mask-paragraph t nil 'fill-paragraph arg))
    ;; Always return t.  This has the effect that if filling isn't done
    ;; above, it isn't done at all, and it's therefore effectively
    ;; disabled in normal code.
--- 4229,4239 ----
    (let ((fill-paragraph-function
  	 ;; Avoid infinite recursion.
  	 (if (not (eq fill-paragraph-function 'c-fill-paragraph))
! 	     fill-paragraph-function))
! 	(start-point (point-marker)))
!     (c-mask-paragraph
!      t nil (lambda () (fill-region-as-paragraph (point-min) (point-max) arg)))
!     (goto-char start-point))
    ;; Always return t.  This has the effect that if filling isn't done
    ;; above, it isn't done at all, and it's therefore effectively
    ;; disabled in normal code.


> Deniz Dogan

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #26 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Christoph <cschol2112 <at> googlemail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#193: Fix for bug#193.
Date: Fri, 09 Jul 2010 19:41:12 -0600

Hi Alan,

On 7/9/2010 12:51 PM, Alan Mackenzie wrote:
> Hi, Deniz,
>
> On Sun, Jun 27, 2010 at 02:47:52PM +0200, Deniz Dogan wrote:
>> Did anyone ever agree on what needs to be done to fix the problem?
>
> Would you try this patch, please, and then tell me whether or not it
> works fully.  Thanks!

M-q works fine for me with C as well as C++ style comments, both ways, 
i.e. shorting long comments and extending short comments to fill the line.

The only "quirk" I found:

Using C-style comments, if the original comment looks like this:

/* This is a short comment
   which is extended                      */

the resulting comment is:

/* This is a short comment which is extended  */

Notice the two spaces after the last word 'extended', before the close 
comment marker.

I would have expected either:

/* This is a short comment which is extended                      */

i.e. the whitespace of the original comment is preserved, or

/* This is a short comment which is extended */

i.e. the whitespace is zapped except one space before the close comment 
marker.

This might be nitpicking but I thought I bring it up.

Christoph

Message #29 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Christoph <cschol2112 <at> googlemail.com>
Cc: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#193: Fix for bug#193.
Date: Sun, 11 Jul 2010 18:55:59 +0000

Hi, Christoph,

On Fri, Jul 09, 2010 at 07:41:12PM -0600, Christoph wrote:
> Hi Alan,

> On 7/9/2010 12:51 PM, Alan Mackenzie wrote:
> >Hi, Deniz,

> >On Sun, Jun 27, 2010 at 02:47:52PM +0200, Deniz Dogan wrote:
> >>Did anyone ever agree on what needs to be done to fix the problem?

> >Would you try this patch, please, and then tell me whether or not it
> >works fully.  Thanks!

> M-q works fine for me with C as well as C++ style comments, both ways, 
> i.e. shorting long comments and extending short comments to fill the line.

> The only "quirk" I found:

> Using C-style comments, if the original comment looks like this:

> /* This is a short comment
>    which is extended                      */

> the resulting comment is:

> /* This is a short comment which is extended  */

> Notice the two spaces after the last word 'extended', before the close 
> comment marker.

The reason being that the filling function assumed that a comment ends
with a sentence, and should therefore leave two spaces after it.
However, if there's no full stop (etc.), the two spaces is silly.

> This might be nitpicking but I thought I bring it up.

We need nitpickers.  ;-)  Thanks for drawing it to my attention.  Would
you please try this amended patch: It should leave just one space after
"is extended", but two spaces after "is extended.".


*** orig/cc-cmds.el	2010-07-05 20:17:46.000000000 +0000
--- cc-cmds.el	2010-07-11 18:46:13.066679040 +0000
***************
*** 3975,3980 ****
--- 3975,3988 ----
  				       (goto-char ender-start)
  				       (current-column)))
  		       (point-rel (- ender-start here))
+ 		       (sentence-ends-comment
+ 			(save-excursion
+ 			  (goto-char ender-start)
+ 			  (and (search-backward-regexp
+ 				(c-sentence-end) (c-point 'bol) t)
+ 			       (goto-char (match-end 0))
+ 			  (looking-at "[ \t]*")
+ 			  (= (match-end 0) ender-start))))
  		       spaces)
  
  		  (save-excursion
***************
*** 4017,4023 ****
  			      (setq spaces
  				    (max
  				     (min spaces
! 					  (if sentence-end-double-space 2 1))
  				     1)))
  			  ;; Insert the filler first to keep marks right.
  			  (insert-char ?x spaces t)
--- 4025,4033 ----
  			      (setq spaces
  				    (max
  				     (min spaces
! 					  (if (and sentence-ends-comment
! 						   sentence-end-double-space)
! 					      2 1))
  				     1)))
  			  ;; Insert the filler first to keep marks right.
  			  (insert-char ?x spaces t)
***************
*** 4229,4236 ****
    (let ((fill-paragraph-function
  	 ;; Avoid infinite recursion.
  	 (if (not (eq fill-paragraph-function 'c-fill-paragraph))
! 	     fill-paragraph-function)))
!     (c-mask-paragraph t nil 'fill-paragraph arg))
    ;; Always return t.  This has the effect that if filling isn't done
    ;; above, it isn't done at all, and it's therefore effectively
    ;; disabled in normal code.
--- 4239,4249 ----
    (let ((fill-paragraph-function
  	 ;; Avoid infinite recursion.
  	 (if (not (eq fill-paragraph-function 'c-fill-paragraph))
! 	     fill-paragraph-function))
! 	(start-point (point-marker)))
!     (c-mask-paragraph
!      t nil (lambda () (fill-region-as-paragraph (point-min) (point-max) arg)))
!     (goto-char start-point))
    ;; Always return t.  This has the effect that if filling isn't done
    ;; above, it isn't done at all, and it's therefore effectively
    ;; disabled in normal code.



> Christoph

Looking forward to hearing back from you,

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #32 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Christoph <cschol2112 <at> googlemail.com>
To: Alan Mackenzie <acm <at> muc.de>
Cc: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#193: Fix for bug#193.
Date: Tue, 13 Jul 2010 19:08:30 -0600

Alan,

On 7/11/2010 12:55 PM, Alan Mackenzie wrote:

> We need nitpickers.  ;-)  Thanks for drawing it to my attention.  Would
> you please try this amended patch: It should leave just one space after
> "is extended", but two spaces after "is extended.".

Germans are the best nitpickers. ;)

Anyway, it works fine now. Just like you described with an additional 
space after the period, none otherwise.

However, here is one more quirky case:

/*
this
is
a
test
.
*/

expands to

/*
this is a test */

I would have expected

/* this is a test */

However, this

/*
this
is
a
test
 */ (note the extra space at the beginning of the line)

expands to

/*
this is a test
 */

which I can understand, but I am not sure if this is intended or if it 
is supposed to also be

/* this is a test */

Thanks,
Christoph

Message #35 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Alan Mackenzie <acm <at> muc.de>
To: Christoph <cschol2112 <at> googlemail.com>
Cc: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#193: Fix for bug#193.
Date: Fri, 6 Aug 2010 18:59:06 +0000

Hi, Christoph!

On Tue, Jul 13, 2010 at 07:08:30PM -0600, Christoph wrote:
> Alan,

> Anyway, it works fine now. Just like you described with an additional 
> space after the period, none otherwise.

> However, here is one more quirky case:

> /*
> this
> is
> a
> test
> .
> */

> expands to

> /*
> this is a test */

It fills properly for me when I type C-q.  That is, it works right after
applying the patch I sent ~ 3 weeks ago.

> I would have expected

> /* this is a test */

> However, this

> /*
> this
> is
> a
> test
>  */ (note the extra space at the beginning of the line)

> expands to

> /*
> this is a test
>  */

> which I can understand, but I am not sure if this is intended or if it 
> is supposed to also be

> /* this is a test */

A comment marker alone on a line stays that way, on the assumption that
this is a matter of style.  A comment marker on a line together with
text always keeps at least one word with it after filling.

> Thanks,
> Christoph

-- 
Alan Mackenzie (Nuremberg, Germany).

Message #40 received at 193-done <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: 193-done <at> debbugs.gnu.org
Subject: Re: bug#193:
Date: Sat, 05 Mar 2011 15:43:34 -0500

Version: 24.1

Looks like a patch for this was applied 2010-08-06.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.