GNU bug report logs - #66670
[PATCH] Use buffer-local comment-continue in comment-indent-new-line

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 66670 in the body.
You can then email your comments to 66670 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: Spencer Baugh <sbaugh <at> janestreet.com>
To: bug-gnu-emacs <at> gnu.org
Cc: Stefan Monnier <monnier <at> gnu.org>
Subject: [PATCH] Use buffer-local comment-continue in comment-indent-new-line
Date: Sat, 21 Oct 2023 16:08:42 -0400

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

Tags: patch


This fixes a remaining issue with whitespace values of comment-continue,
as described in https://github.com/ocaml/tuareg/issues/216

Namely, the fixes done so far don't cover comment-indent-new-line.

In GNU Emacs 29.1.50 (build 12, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.15.12, Xaw scroll bars) of 2023-10-19 built on
 igm-qws-u22796a
Repository revision: 9163e634e296435aa7a78bc6b77b4ee90666d2ac
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Rocky Linux 8.8 (Green Obsidian)

Configured using:
 'configure --config-cache --with-x-toolkit=lucid
 --with-gif=ifavailable'

[0001-Use-buffer-local-comment-continue-in-comment-indent-.patch (text/patch, attachment)]

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: bug-gnu-emacs <at> gnu.org, Stefan Monnier <monnier <at> gnu.org>
Subject: Re: [PATCH] Use buffer-local comment-continue in
 comment-indent-new-line
Date: Sat, 21 Oct 2023 17:23:35 -0400

>                       ;; Recreate comment-continue from comment-start.
> -                     ;; FIXME: wrong if comment-continue was set explicitly!
>                       ;; FIXME: use prev line's continuation if available.
> -                     (comment-continue nil))
> +                     (comment-continue (if (local-variable-p 'comment-continue)
> +                                           comment-continue
> +                                         nil)))

Are you sure?  `comment-continue` is very rarely set globally.
Usually it's set buffer-locally by `comment-normalize-vars`.

> +(ert-deftest local-comment-continue-in-comment-indent-new-line ()
> +  (with-temp-buffer
> +    (setq-local comment-start "/* ")
> +    (setq-local comment-end "*/")
                               ^^
Out of symmetry, I'd have expected a SPC here.

> +    (insert "foo")
> +    (newline)
> +    (insert "bar")
> +    (forward-line -1)
> +    (end-of-line)
> +    (comment-region (point-min) (point-max))
> +    (should (equal (thing-at-point 'line) "/* foo\n"))
> +    (comment-indent-new-line)

You should also test it with that very same comment but when
`comment-start` and `comment-end` have been set to something like
"// " and "".


        Stefan

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

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 66670 <at> debbugs.gnu.org, monnier <at> gnu.org
Subject: Re: bug#66670: [PATCH] Use buffer-local comment-continue in
 comment-indent-new-line
Date: Sat, 21 Oct 2023 18:15:24 -0400

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>>                       ;; Recreate comment-continue from comment-start.
>> -                     ;; FIXME: wrong if comment-continue was set explicitly!
>>                       ;; FIXME: use prev line's continuation if available.
>> -                     (comment-continue nil))
>> +                     (comment-continue (if (local-variable-p 'comment-continue)
>> +                                           comment-continue
>> +                                         nil)))
>
> Are you sure?  `comment-continue` is very rarely set globally.
> Usually it's set buffer-locally by `comment-normalize-vars`.

Or by the major-mode!  But yes, I see your point, this patch is
effectively removing the (comment-continue nil) definition.

I guess that the (comment-continue nil) definition is there in the first
place so that comment-normalize-vars recalculates it based on
comment-start, which is the comment prefix from the previous line?  So
we therefore copy the prefix from the previous line?

Should we just recalculate comment-continue directly in
comment-indent-new-line instead, if necessary?

Although even if we did that, I don't see any clear way to know that we
should use the configured comment-continue instead of trying to copy the
previous line.  Should we maybe just not copy the comment prefix from
the previous line at all, if comment-continue is non-nil?

>> +(ert-deftest local-comment-continue-in-comment-indent-new-line ()
>> +  (with-temp-buffer
>> +    (setq-local comment-start "/* ")
>> +    (setq-local comment-end "*/")
>                                ^^
> Out of symmetry, I'd have expected a SPC here.
>
>> +    (insert "foo")
>> +    (newline)
>> +    (insert "bar")
>> +    (forward-line -1)
>> +    (end-of-line)
>> +    (comment-region (point-min) (point-max))
>> +    (should (equal (thing-at-point 'line) "/* foo\n"))
>> +    (comment-indent-new-line)
>
> You should also test it with that very same comment but when
> `comment-start` and `comment-end` have been set to something like
> "// " and "".

Will do.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 66670 <at> debbugs.gnu.org, monnier <at> gnu.org
Subject: Re: bug#66670: [PATCH] Use buffer-local comment-continue in
 comment-indent-new-line
Date: Sun, 22 Oct 2023 00:11:03 -0400

>>>                       ;; Recreate comment-continue from comment-start.
>>> -                     ;; FIXME: wrong if comment-continue was set explicitly!
>>>                       ;; FIXME: use prev line's continuation if available.
>>> -                     (comment-continue nil))
>>> +                     (comment-continue (if (local-variable-p 'comment-continue)
>>> +                                           comment-continue
>>> +                                         nil)))
>>
>> Are you sure?  `comment-continue` is very rarely set globally.
>> Usually it's set buffer-locally by `comment-normalize-vars`.
>
> Or by the major-mode!  But yes, I see your point, this patch is
> effectively removing the (comment-continue nil) definition.

Yup.

> I guess that the (comment-continue nil) definition is there in the first
> place so that comment-normalize-vars recalculates it based on
> comment-start,

Indeed, that's what the comment tries to say.

> which is the comment prefix from the previous line?

Something like that, yes.

> So we therefore copy the prefix from the previous line?

Not necessarily.  If the previous line was the beginning of the comment
with "/*" we should use " *".

> Should we just recalculate comment-continue directly in
> comment-indent-new-line instead, if necessary?

We could.  But we still need to decide whether to do that or to just
trust the `comment-continue` value set by those major modes which set
it by hand :-(


        Stefan

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Spencer Baugh <sbaugh <at> janestreet.com>, 66670 <at> debbugs.gnu.org,
 monnier <at> gnu.org
Subject: Re: bug#66670: [PATCH] Use buffer-local comment-continue in
 comment-indent-new-line
Date: Wed, 12 Feb 2025 23:09:15 -0800

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>>>>                       ;; Recreate comment-continue from comment-start.
>>>> -                     ;; FIXME: wrong if comment-continue was set explicitly!
>>>>                       ;; FIXME: use prev line's continuation if available.
>>>> -                     (comment-continue nil))
>>>> +                     (comment-continue (if (local-variable-p 'comment-continue)
>>>> +                                           comment-continue
>>>> +                                         nil)))
>>>
>>> Are you sure?  `comment-continue` is very rarely set globally.
>>> Usually it's set buffer-locally by `comment-normalize-vars`.
>>
>> Or by the major-mode!  But yes, I see your point, this patch is
>> effectively removing the (comment-continue nil) definition.
>
> Yup.
>
>> I guess that the (comment-continue nil) definition is there in the first
>> place so that comment-normalize-vars recalculates it based on
>> comment-start,
>
> Indeed, that's what the comment tries to say.
>
>> which is the comment prefix from the previous line?
>
> Something like that, yes.
>
>> So we therefore copy the prefix from the previous line?
>
> Not necessarily.  If the previous line was the beginning of the comment
> with "/*" we should use " *".
>
>> Should we just recalculate comment-continue directly in
>> comment-indent-new-line instead, if necessary?
>
> We could.  But we still need to decide whether to do that or to just
> trust the `comment-continue` value set by those major modes which set
> it by hand :-(

Spencer, what is the latest here?  Did you make any progress with this
patch?

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

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 66670 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 monnier <at> gnu.org
Subject: Re: bug#66670: [PATCH] Use buffer-local comment-continue in
 comment-indent-new-line
Date: Thu, 13 Feb 2025 07:15:58 -0500

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

On Thu, Feb 13, 2025, 2:09 AM Stefan Kangas <stefankangas <at> gmail.com> wrote:

> Spencer, what is the latest here?  Did you make any progress with this
> patch?
>

No further progress, it seems hard.  Feel free to close this bug if
appropriate.

>

[Message part 2 (text/html, inline)]

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 66670-done <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 monnier <at> gnu.org
Subject: Re: bug#66670: [PATCH] Use buffer-local comment-continue in
 comment-indent-new-line
Date: Sun, 23 Feb 2025 01:17:53 +0000

Spencer Baugh <sbaugh <at> janestreet.com> writes:

> On Thu, Feb 13, 2025, 2:09 AM Stefan Kangas <stefankangas <at> gmail.com> wrote:
>
>  Spencer, what is the latest here?  Did you make any progress with this
>  patch?
>
> No further progress, it seems hard.  Feel free to close this bug if appropriate.

Thanks, no further comments with 10 days.  I'm therefore closing this
bug report.

Previous Next

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