GNU bug report logs - #71760
31.0.50; Filling jsdoc text doesn't work in js-ts-mode

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 71760 in the body.
You can then email your comments to 71760 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: Damien Cassou <damien <at> cassou.me>
To: bug-gnu-emacs <at> gnu.org
Subject: 31.0.50; Filling jsdoc text doesn't work in js-ts-mode
Date: Mon, 24 Jun 2024 22:23:06 +0200

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

Hi,

in a commit recently merged in master (0edacf2aa7e53), js-ts-mode has
been improved to use the jsdoc treesitter grammar on documentation. Font
lock works great but M-q doesn't fill long lines.

To reproduce, make sure your Emacs contains the above-mentioned commit
and make sure you have the jsdoc treesitter grammar available. Then,
save the attached JavaScript file to your disk and execute:

$ emacs -Q --eval "(progn (find-file \"/home/cassou/tmp/test.js\") (js-ts-mode))"

Move point to the middle of line 2 (or line 4) and press M-q.

Expected: The paragraph is filled.

Actual: Nothing changes.

I tried changing the value of `c-ts-common--comment-regexp' to include
"description" (as this is what the jsdoc parser uses for text) but the
result is worse. I also tried reusing `js-fill-paragraph' but that fails
as well.

I would be happy to implement something and send a patch but I have no
idea were to start.

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

[test.js (text/javascript, attachment)]

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

From: Damien Cassou <damien <at> cassou.me>
To: 71760 <at> debbugs.gnu.org
Cc: Yuan Fu <casouri <at> gmail.com>
Subject: [PATCH] js-ts-mode: Fix filling of jsdoc blocks
Date: Mon, 29 Jul 2024 21:36:02 +0200

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

Tags: patch

Hi,

the attached patches

1. add "description" (the kind of treesit node used for texts by the
jsdoc parser) to `c-ts-common--comment-regexp' in js-ts-mode;

2. make `c-ts-common--fill-block-comment' only remove a mask if it has
added it in the first place.

Best

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

[0001-js-ts-mode-Make-jsdoc-s-description-block-a-comment.patch (text/patch, attachment)]

[0002-c-ts-common-Don-t-insert-wrong-characters-during-fil.patch (text/x-patch, attachment)]

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

From: Damien Cassou <damien <at> cassou.me>
To: 71760 <at> debbugs.gnu.org
Cc: Yuan Fu <casouri <at> gmail.com>
Subject: Re: [PATCH] js-ts-mode: Fix filling of jsdoc blocks
Date: Wed, 31 Jul 2024 10:25:51 +0200

Hi,

after today's fix on c-ts-mode by Yuan Fu, the only patch still
necessary to apply for js-ts-mode to properly fill jsdoc descriptions
is 0001-js-ts-mode-Make-jsdoc-s-description-block-a-comment.patch.

Damien Cassou <damien <at> cassou.me> writes:

> Tags: patch
>
> Hi,
>
> the attached patches
>
> 1. add "description" (the kind of treesit node used for texts by the
> jsdoc parser) to `c-ts-common--comment-regexp' in js-ts-mode;
>
> 2. make `c-ts-common--fill-block-comment' only remove a mask if it has
> added it in the first place.
>
> Best
>
> -- 
> Damien Cassou
>
> "Success is the ability to go from one failure to another without
> losing enthusiasm." --Winston Churchill
> From e444692026cae97dfaecebf5972507ae95f5e488 Mon Sep 17 00:00:00 2001
> From: Damien Cassou <damien <at> cassou.me>
> Date: Sun, 21 Jul 2024 21:32:34 +0200
> Subject: [PATCH 1/2] js-ts-mode: Make jsdoc's "description" block a comment
>
> * lisp/progmodes/js.el (js-ts-mode): Add "description" to
> `c-ts-common--comment-regexp'.
> ---
>  lisp/progmodes/js.el | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
> index f8140c14a49..75c8111035c 100644
> --- a/lisp/progmodes/js.el
> +++ b/lisp/progmodes/js.el
> @@ -3946,7 +3946,9 @@ js-ts-mode
>                     :embed 'jsdoc
>                     :host 'javascript
>                     :local t
> -                   `(((comment) @capture (:match ,js--treesit-jsdoc-beginning-regexp @capture))))))
> +                   `(((comment) @capture (:match ,js--treesit-jsdoc-beginning-regexp @capture)))))
> +
> +      (setq c-ts-common--comment-regexp (rx (or "comment" "line_comment" "block_comment" "description"))))
>  
>      ;; Imenu
>      (setq-local treesit-simple-imenu-settings
> -- 
> 2.45.2
>
> From cf14e3b25245dac98adcf90723e4a0c601f65528 Mon Sep 17 00:00:00 2001
> From: Damien Cassou <damien <at> cassou.me>
> Date: Sun, 21 Jul 2024 21:33:59 +0200
> Subject: [PATCH 2/2] c-ts-common: Don't insert wrong characters during filling
>
> * lisp/progmodes/c-ts-common.el (c-ts-common--fill-block-comment): If
> masking hasn't been done, don't unmask.
> ---
>  lisp/progmodes/c-ts-common.el | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/lisp/progmodes/c-ts-common.el b/lisp/progmodes/c-ts-common.el
> index 3882a697c48..14dd29e07e2 100644
> --- a/lisp/progmodes/c-ts-common.el
> +++ b/lisp/progmodes/c-ts-common.el
> @@ -151,7 +151,9 @@ c-ts-common--fill-block-comment
>           (orig-point (point-marker))
>           (start-marker (point-marker))
>           (end-marker nil)
> -         (end-len 0))
> +         (end-len 0)
> +         (start-mask-done nil)
> +         (end-mask-done nil))
>      (move-marker start-marker start)
>      ;; We mask "/*" and the space before "*/" like
>      ;; `c-fill-paragraph' does.
> @@ -162,6 +164,7 @@ c-ts-common--fill-block-comment
>                              (group "/") "*"))
>          (goto-char (match-beginning 1))
>          (move-marker start-marker (point))
> +        (setq start-mask-done t)
>          (replace-match " " nil nil nil 1))
>  
>        ;; Include whitespaces before /*.
> @@ -179,6 +182,7 @@ c-ts-common--fill-block-comment
>          (goto-char (match-beginning 1))
>          (setq end-marker (point-marker))
>          (setq end-len (- (match-end 1) (match-beginning 1)))
> +        (setq end-mask-done t)
>          (replace-match (make-string end-len ?x)
>                         nil nil nil 1))
>  
> @@ -206,11 +210,11 @@ c-ts-common--fill-block-comment
>          (fill-region (max start-marker para-start) (min end para-end) arg))
>  
>        ;; Unmask.
> -      (when start-marker
> +      (when (and start-mask-done start-marker)
>          (goto-char start-marker)
>          (delete-char 1)
>          (insert "/"))
> -      (when end-marker
> +      (when (and end-mask-done start-marker)
>          (goto-char end-marker)
>          (delete-region (point) (+ end-len (point)))
>          (insert (make-string end-len ?\s)))
> -- 
> 2.45.2
>

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Damien Cassou <damien <at> cassou.me>, 71760 <at> debbugs.gnu.org
Cc: Yuan Fu <casouri <at> gmail.com>
Subject: Re: bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks
Date: Thu, 1 Aug 2024 04:00:55 +0300

Hi!

On 31/07/2024 11:25, Damien Cassou wrote:
> after today's fix on c-ts-mode by Yuan Fu, the only patch still
> necessary to apply for js-ts-mode to properly fill jsdoc descriptions
> is 0001-js-ts-mode-Make-jsdoc-s-description-block-a-comment.patch.

Since bug#72116 is only fixed on master, does this mean that the 
aforementioned patch is also best for Emacs 31, or would we want it in 
the upcoming release as well?

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

From: Yuan Fu <casouri <at> gmail.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Damien Cassou <damien <at> cassou.me>, 71760 <at> debbugs.gnu.org
Subject: Re: bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks
Date: Wed, 31 Jul 2024 18:07:57 -0700

> On Jul 31, 2024, at 6:00 PM, Dmitry Gutov <dmitry <at> gutov.dev> wrote:
> 
> Hi!
> 
> On 31/07/2024 11:25, Damien Cassou wrote:
>> after today's fix on c-ts-mode by Yuan Fu, the only patch still
>> necessary to apply for js-ts-mode to properly fill jsdoc descriptions
>> is 0001-js-ts-mode-Make-jsdoc-s-description-block-a-comment.patch.
> 
> Since bug#72116 is only fixed on master, does this mean that the aforementioned patch is also best for Emacs 31, or would we want it in the upcoming release as well?
> 

We should apply the masking patch to emacs-30. Thought I think it can be simplified. I’ll try make one for you guys to review tonight.

Yuan

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

From: Yuan Fu <casouri <at> gmail.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Damien Cassou <damien <at> cassou.me>, 71760 <at> debbugs.gnu.org
Subject: Re: bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks
Date: Thu, 1 Aug 2024 00:36:30 -0700

> On Jul 31, 2024, at 6:07 PM, Yuan Fu <casouri <at> gmail.com> wrote:
> 
> 
> 
>> On Jul 31, 2024, at 6:00 PM, Dmitry Gutov <dmitry <at> gutov.dev> wrote:
>> 
>> Hi!
>> 
>> On 31/07/2024 11:25, Damien Cassou wrote:
>>> after today's fix on c-ts-mode by Yuan Fu, the only patch still
>>> necessary to apply for js-ts-mode to properly fill jsdoc descriptions
>>> is 0001-js-ts-mode-Make-jsdoc-s-description-block-a-comment.patch.
>> 
>> Since bug#72116 is only fixed on master, does this mean that the aforementioned patch is also best for Emacs 31, or would we want it in the upcoming release as well?
>> 
> 
> We should apply the masking patch to emacs-30. Thought I think it can be simplified. I’ll try make one for you guys to review tonight.
> 
> Yuan

Actually, the original patch is perfectly good. The “simplification” I have in mind is more or less unnecessary.

Yuan

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

From: Damien Cassou <damien <at> cassou.me>
To: Yuan Fu <casouri <at> gmail.com>, Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 71760 <at> debbugs.gnu.org
Subject: Re: bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks
Date: Thu, 01 Aug 2024 12:26:14 +0200

Yuan Fu <casouri <at> gmail.com> writes:
> Actually, the original patch is perfectly good. The “simplification” I have in mind is more or less unnecessary.

if possible, I would appreciate Yuan's patches and
0001-js-ts-mode-Make-jsdoc-s-description-block-a-comment.patch to be
merged into emacs-30.

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Yuan Fu <casouri <at> gmail.com>
Cc: Damien Cassou <damien <at> cassou.me>, 71760 <at> debbugs.gnu.org
Subject: Re: bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks
Date: Fri, 2 Aug 2024 00:26:37 +0300

Hi Yuan,

On 01/08/2024 10:36, Yuan Fu wrote:
> Actually, the original patch is perfectly good. The “simplification” I have in mind is more or less unnecessary.

I think Damien is saying that patch#2 from his submission is not 
necessary with your fix (commit 74bb1e5897f, IIUC).

So do you think it's a good idea to backport it to emacs-30?

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

From: Yuan Fu <casouri <at> gmail.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Damien Cassou <damien <at> cassou.me>, 71760 <at> debbugs.gnu.org
Subject: Re: bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks
Date: Sat, 3 Aug 2024 00:34:24 -0700

> On Aug 1, 2024, at 2:26 PM, Dmitry Gutov <dmitry <at> gutov.dev> wrote:
> 
> Hi Yuan,
> 
> On 01/08/2024 10:36, Yuan Fu wrote:
>> Actually, the original patch is perfectly good. The “simplification” I have in mind is more or less unnecessary.
> 
> I think Damien is saying that patch#2 from his submission is not necessary with your fix (commit 74bb1e5897f, IIUC).
> 
> So do you think it's a good idea to backport it to emacs-30?

I’d rather apply Damien’s patch to emacs-30, that’s a simple fix and doesn’t change the filling behavior. The change I applied to master is more substantial, and apparently introduced another regression that I now need to fix :-(

Yuan

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Yuan Fu <casouri <at> gmail.com>
Cc: Damien Cassou <damien <at> cassou.me>, 71760 <at> debbugs.gnu.org
Subject: Re: bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks
Date: Sat, 3 Aug 2024 18:14:21 +0300

On 03/08/2024 10:34, Yuan Fu wrote:
>> I think Damien is saying that patch#2 from his submission is not necessary with your fix (commit 74bb1e5897f, IIUC).
>>
>> So do you think it's a good idea to backport it to emacs-30?
> I’d rather apply Damien’s patch to emacs-30, that’s a simple fix and doesn’t change the filling behavior. The change I applied to master is more substantial, and apparently introduced another regression that I now need to fix 🙁

Makes sense.

It would be great if you could do the applying - while I understand the 
tree-sitter part, I'm not quite familiar with the filling code, so not 
equipped to give it a "stamp of approval" myself.

Also, I would probably have to ask whether this patch if a replacement 
for that's been added to master, an addition, or a suitable alternative.

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

From: Yuan Fu <casouri <at> gmail.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Damien Cassou <damien <at> cassou.me>, 71760-done <at> debbugs.gnu.org
Subject: Re: bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks
Date: Sat, 3 Aug 2024 20:05:30 -0700

> On Aug 3, 2024, at 8:14 AM, Dmitry Gutov <dmitry <at> gutov.dev> wrote:
> 
> On 03/08/2024 10:34, Yuan Fu wrote:
>>> I think Damien is saying that patch#2 from his submission is not necessary with your fix (commit 74bb1e5897f, IIUC).
>>> 
>>> So do you think it's a good idea to backport it to emacs-30?
>> I’d rather apply Damien’s patch to emacs-30, that’s a simple fix and doesn’t change the filling behavior. The change I applied to master is more substantial, and apparently introduced another regression that I now need to fix 🙁
> 
> Makes sense.
> 
> It would be great if you could do the applying - while I understand the tree-sitter part, I'm not quite familiar with the filling code, so not equipped to give it a "stamp of approval" myself.
> 
> Also, I would probably have to ask whether this patch if a replacement for that's been added to master, an addition, or a suitable alternative.

Of course. I applied the patch. And I think this report can be closed, so closing :-)

Yuan

Previous Next

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