all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#71760: 31.0.50; Filling jsdoc text doesn't work in js-ts-mode
@ 2024-06-24 20:23 Damien Cassou
  2024-07-29 19:36 ` bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks Damien Cassou
  0 siblings, 1 reply; 11+ messages in thread
From: Damien Cassou @ 2024-06-24 20:23 UTC (permalink / raw)
  To: 71760

[-- 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) {
}




^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks
  2024-06-24 20:23 bug#71760: 31.0.50; Filling jsdoc text doesn't work in js-ts-mode Damien Cassou
@ 2024-07-29 19:36 ` Damien Cassou
  2024-07-31  8:25   ` Damien Cassou
  0 siblings, 1 reply; 11+ messages in thread
From: Damien Cassou @ 2024-07-29 19:36 UTC (permalink / raw)
  To: 71760; +Cc: Yuan Fu

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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks
  2024-07-29 19:36 ` bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks Damien Cassou
@ 2024-07-31  8:25   ` Damien Cassou
  2024-08-01  1:00     ` Dmitry Gutov
  0 siblings, 1 reply; 11+ messages in thread
From: Damien Cassou @ 2024-07-31  8:25 UTC (permalink / raw)
  To: 71760; +Cc: Yuan Fu

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





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks
  2024-07-31  8:25   ` Damien Cassou
@ 2024-08-01  1:00     ` Dmitry Gutov
  2024-08-01  1:07       ` Yuan Fu
  0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Gutov @ 2024-08-01  1:00 UTC (permalink / raw)
  To: Damien Cassou, 71760; +Cc: Yuan Fu

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?






^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks
  2024-08-01  1:00     ` Dmitry Gutov
@ 2024-08-01  1:07       ` Yuan Fu
  2024-08-01  7:36         ` Yuan Fu
  0 siblings, 1 reply; 11+ messages in thread
From: Yuan Fu @ 2024-08-01  1:07 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Damien Cassou, 71760



> 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




^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks
  2024-08-01  1:07       ` Yuan Fu
@ 2024-08-01  7:36         ` Yuan Fu
  2024-08-01 10:26           ` Damien Cassou
  2024-08-01 21:26           ` Dmitry Gutov
  0 siblings, 2 replies; 11+ messages in thread
From: Yuan Fu @ 2024-08-01  7:36 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Damien Cassou, 71760



> 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





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks
  2024-08-01  7:36         ` Yuan Fu
@ 2024-08-01 10:26           ` Damien Cassou
  2024-08-01 21:26           ` Dmitry Gutov
  1 sibling, 0 replies; 11+ messages in thread
From: Damien Cassou @ 2024-08-01 10:26 UTC (permalink / raw)
  To: Yuan Fu, Dmitry Gutov; +Cc: 71760

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





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks
  2024-08-01  7:36         ` Yuan Fu
  2024-08-01 10:26           ` Damien Cassou
@ 2024-08-01 21:26           ` Dmitry Gutov
  2024-08-03  7:34             ` Yuan Fu
  1 sibling, 1 reply; 11+ messages in thread
From: Dmitry Gutov @ 2024-08-01 21:26 UTC (permalink / raw)
  To: Yuan Fu; +Cc: Damien Cassou, 71760

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?





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks
  2024-08-01 21:26           ` Dmitry Gutov
@ 2024-08-03  7:34             ` Yuan Fu
  2024-08-03 15:14               ` Dmitry Gutov
  0 siblings, 1 reply; 11+ messages in thread
From: Yuan Fu @ 2024-08-03  7:34 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Damien Cassou, 71760



> 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




^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks
  2024-08-03  7:34             ` Yuan Fu
@ 2024-08-03 15:14               ` Dmitry Gutov
  2024-08-04  3:05                 ` Yuan Fu
  0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Gutov @ 2024-08-03 15:14 UTC (permalink / raw)
  To: Yuan Fu; +Cc: Damien Cassou, 71760

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.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks
  2024-08-03 15:14               ` Dmitry Gutov
@ 2024-08-04  3:05                 ` Yuan Fu
  0 siblings, 0 replies; 11+ messages in thread
From: Yuan Fu @ 2024-08-04  3:05 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Damien Cassou, 71760-done



> 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




^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-08-04  3:05 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-24 20:23 bug#71760: 31.0.50; Filling jsdoc text doesn't work in js-ts-mode Damien Cassou
2024-07-29 19:36 ` bug#71760: [PATCH] js-ts-mode: Fix filling of jsdoc blocks Damien Cassou
2024-07-31  8:25   ` Damien Cassou
2024-08-01  1:00     ` Dmitry Gutov
2024-08-01  1:07       ` Yuan Fu
2024-08-01  7:36         ` Yuan Fu
2024-08-01 10:26           ` Damien Cassou
2024-08-01 21:26           ` Dmitry Gutov
2024-08-03  7:34             ` Yuan Fu
2024-08-03 15:14               ` Dmitry Gutov
2024-08-04  3:05                 ` Yuan Fu

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.