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

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

[Damien Cassou]

[-- Attachment #1: Type: text/plain, Size: 1050 bytes --]

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: test.js --]
[-- Type: text/javascript, Size: 227 bytes --]

/**
 * Test. rat art srs tr tr st rst artsar t arstienartsioenaroti eanrtsoi aerntso iaerst
 *
 * @param {string} bla - Some documentation of this parameter. ar tar star tsa rst arstar st arst art rs
 */
function f(bla) {
}

bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks

[Damien Cassou]

[-- Attachment #1: Type: text/plain, Size: 404 bytes --]

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-js-ts-mode-Make-jsdoc-s-description-block-a-comment.patch --]
[-- Type: text/patch, Size: 1067 bytes --]

From e444692026cae97dfaecebf5972507ae95f5e488 Mon Sep 17 00:00:00 2001
From: Damien Cassou <damien@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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-c-ts-common-Don-t-insert-wrong-characters-during-fil.patch --]
[-- Type: text/x-patch, Size: 2156 bytes --]

From cf14e3b25245dac98adcf90723e4a0c601f65528 Mon Sep 17 00:00:00 2001
From: Damien Cassou <damien@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

bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks

[Damien Cassou]

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@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@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@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

bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks

[Dmitry Gutov]

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?

bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks

[Yuan Fu]

> On Jul 31, 2024, at 6:00 PM, Dmitry Gutov <dmitry@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

bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks

[Yuan Fu]

> On Jul 31, 2024, at 6:07 PM, Yuan Fu <casouri@gmail.com> wrote:
> 
> 
> 
>> On Jul 31, 2024, at 6:00 PM, Dmitry Gutov <dmitry@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

bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks

[Damien Cassou]

Yuan Fu <casouri@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

bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks

[Dmitry Gutov]

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?

bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks

[Yuan Fu]

> On Aug 1, 2024, at 2:26 PM, Dmitry Gutov <dmitry@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

bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks

[Dmitry Gutov]

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.

bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks

[Yuan Fu]

> On Aug 3, 2024, at 8:14 AM, Dmitry Gutov <dmitry@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