Fill for // (C++) style comments in C (C99)

Fill for // (C++) style comments in C (C99)

[Richard Hansen]

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

bug#193:

[Deniz Dogan]

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

-- 
Deniz Dogan

bug#193:

[Alan Mackenzie]

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).

bug#193:

[Glenn Morris]

Version: 24.1

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

bug#193:

[Alan Mackenzie]

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).

Re: bug#193:

[Alan Mackenzie]

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).

bug#193: Fix for bug#193.

[Alan Mackenzie]

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).

bug#193: Fix for bug#193.

[Christoph]

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

bug#193: Fix for bug#193.

[Alan Mackenzie]

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).



------------------------------------------------------------------------------
This SF.net email is sponsored by Sprint
What will you do first with EVO, the first 4G phone?
Visit sprint.com/first -- http://p.sf.net/sfu/sprint-com-first

bug#193: Fix for bug#193.

[Christoph]

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

bug#193: Fix for bug#193.

[Alan Mackenzie]

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).