GNU bug report logs - #47167
Multi-line comment-region with empty comment-continue

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 47167 in the body.
You can then email your comments to 47167 AT debbugs.gnu.org in the normal way.

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

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

From: Juri Linkov <juri <at> linkov.net>
To: bug-gnu-emacs <at> gnu.org
Subject: Multi-line comment-region with empty comment-continue
Date: Mon, 15 Mar 2021 19:03:22 +0200

[Message part 1 (text/plain, inline)]

Tags: patch

Tried to customize 'comment-region' to use multi-line comments,
so when commenting out such region in html-mode:

line 1
line 2
line 3

instead of adding comments to each line:

<!-- line 1 -->
<!-- line 2 -->
<!-- line 3 -->

to add comments only at the beginning/end:

<!--
line 1
line 2
line 3
  -->

According to the documentation, a legitimate way to do this is
to use such customization:

(setq-local comment-style 'extra-line
            comment-continue "")

But currently it has no effect.  So needed to fix newcomment.el.
One possible fix is to return the original string from 'comment-padright'
even when the string contains only whitespace.  Maybe this would be
the right fix, but still not sure about other calls of 'comment-padright'.
So a safer fix would be to do this in the caller:

[comment-region-padright-continue.patch (text/x-diff, inline)]

diff --git a/lisp/newcomment.el b/lisp/newcomment.el
index ea47eec4fd..a5bfb06795 100644
--- a/lisp/newcomment.el
+++ b/lisp/newcomment.el
@@ -1300,7 +1300,11 @@ comment-region-default-1
 	 (let ((s (comment-padleft comment-end numarg)))
 	   (and s (if (string-match comment-end-skip s) s
 		    (comment-padright comment-end))))
-	 (if multi (comment-padright comment-continue numarg))
+	 (if multi
+             (or (comment-padright comment-continue numarg)
+                 ;; `comment-padright' returns nil when
+                 ;; `comment-continue' contains only whitespace
+                 (and (stringp comment-continue) comment-continue)))
 	 (if multi
 	     (comment-padleft (comment-string-reverse comment-continue) numarg))
 	 block

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

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>, "47167 <at> debbugs.gnu.org"
 <47167 <at> debbugs.gnu.org>
Subject: RE: [External] : bug#47167: Multi-line comment-region with empty
 comment-continue
Date: Mon, 15 Mar 2021 17:35:24 +0000

> Tried to customize 'comment-region' to use multi-line comments,
> so when commenting out such region in html-mode:
> 
> line 1
> line 2
> line 3
> 
> instead of adding comments to each line:
> 
> <!-- line 1 -->
> <!-- line 2 -->
> <!-- line 3 -->
> 
> to add comments only at the beginning/end:
> 
> <!--
> line 1
> line 2
> line 3
>   -->

(Haven't looked at your patch.)

Sounds like a reasonable, and useful thing to do.

However, `comment-region' also has optional behaviors
that involve its use of a prefix arg.  How with those
interact with what you propose?  Do they even make
sense (e.g. nesting comment levels and unnesting them)?

My guess is that what's needed is a separate command.

I'm guessing completely different prefix-arg behavior
would be appropriate for multi-line comments.  Maybe
you can find a way to merge the two without confusing
users (and the doc).  Or maybe it's not worth trying
to do that, and a separate command makes more sense.
Dunno.

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

From: Juri Linkov <juri <at> linkov.net>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: "47167 <at> debbugs.gnu.org" <47167 <at> debbugs.gnu.org>
Subject: Re: [External] : bug#47167: Multi-line comment-region with empty
 comment-continue
Date: Tue, 16 Mar 2021 19:47:37 +0200

> (Haven't looked at your patch.)
>
> Sounds like a reasonable, and useful thing to do.
>
> However, `comment-region' also has optional behaviors
> that involve its use of a prefix arg.  How with those
> interact with what you propose?  Do they even make
> sense (e.g. nesting comment levels and unnesting them)?

With (setq-local comment-style 'extra-line comment-continue ""),
`comment-region' currently breaks HTML syntax
because HTML doesn't support nested comments:

<!--
<!-- line 1 -->
<!-- line 2 -->
<!-- line 3 -->
  -->

With the proposed patch, HTML syntax stays valid:

<!--
line 1
line 2
line 3
  -->

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

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: "47167 <at> debbugs.gnu.org" <47167 <at> debbugs.gnu.org>
Subject: RE: [External] : bug#47167: Multi-line comment-region with empty
 comment-continue
Date: Tue, 16 Mar 2021 19:24:47 +0000

> > (Haven't looked at your patch.)
> > Sounds like a reasonable, and useful thing to do.
> >
> > However, `comment-region' also has optional behaviors
> > that involve its use of a prefix arg.  How with those
> > interact with what you propose?  Do they even make
> > sense (e.g. nesting comment levels and unnesting them)?
> 
> With (setq-local comment-style 'extra-line comment-continue ""),
> `comment-region' currently breaks HTML syntax
> because HTML doesn't support nested comments:
> 
> <!--
> <!-- line 1 -->
> <!-- line 2 -->
> <!-- line 3 -->
>   -->
> 
> With the proposed patch, HTML syntax stays valid:
> 
> <!--
> line 1
> line 2
> line 3
>   -->

I see, I guess.  But why change `comment-region',
instead of creating a new command for multi-line
(aka block) commenting?

If there's a bug now (e.g. wrt `comment-continue')
when someone uses `comment-region', then maybe
either that can be fixed (without involving
multiline) or that command can be made to ignore,
DTRT, or raise an error when `comment-continue'
is used.  (Just a wild guess, without looking at
any code.)

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

From: Juri Linkov <juri <at> linkov.net>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: "47167 <at> debbugs.gnu.org" <47167 <at> debbugs.gnu.org>
Subject: Re: [External] : bug#47167: Multi-line comment-region with empty
 comment-continue
Date: Wed, 17 Mar 2021 19:04:20 +0200

>> With (setq-local comment-style 'extra-line comment-continue ""),
>> `comment-region' currently breaks HTML syntax
>> because HTML doesn't support nested comments:
>>
>> <!--
>> <!-- line 1 -->
>> <!-- line 2 -->
>> <!-- line 3 -->
>>   -->
>>
>> With the proposed patch, HTML syntax stays valid:
>>
>> <!--
>> line 1
>> line 2
>> line 3
>>   -->
>
> I see, I guess.  But why change `comment-region',
> instead of creating a new command for multi-line
> (aka block) commenting?

'multi-line' is just one of possible styles in 'comment-styles'.
I'm not interested in creating separate commands for each style.
If you want, you can create a separate bug report.

> If there's a bug now (e.g. wrt `comment-continue')
> when someone uses `comment-region', then maybe
> either that can be fixed (without involving
> multiline) or that command can be made to ignore,

Indeed, there is a bug.  The documentation of 'comment-continue' says:

  Continuation string to insert for multiline comments.

  If it is nil a value will be automatically derived from ‘comment-start’
  by replacing its first character with a space.

The problem is that currently the empty string behaves as if it's nil.
The patch fixes it.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 47167 <at> debbugs.gnu.org
Subject: Re: bug#47167: Multi-line comment-region with empty comment-continue
Date: Thu, 18 Mar 2021 06:19:12 +0100

Juri Linkov <juri <at> linkov.net> writes:

> But currently it has no effect.  So needed to fix newcomment.el.
> One possible fix is to return the original string from 'comment-padright'
> even when the string contains only whitespace.  Maybe this would be
> the right fix, but still not sure about other calls of 'comment-padright'.
> So a safer fix would be to do this in the caller:

Looks good to me.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 47167 <at> debbugs.gnu.org
Subject: Re: bug#47167: Multi-line comment-region with empty comment-continue
Date: Thu, 18 Mar 2021 20:01:42 +0200

tags 47167 fixed
close 47167 28.0.50
quit

>> But currently it has no effect.  So needed to fix newcomment.el.
>> One possible fix is to return the original string from 'comment-padright'
>> even when the string contains only whitespace.  Maybe this would be
>> the right fix, but still not sure about other calls of 'comment-padright'.
>> So a safer fix would be to do this in the caller:
>
> Looks good to me.

Now pushed to master.

Previous Next

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