bug#26550: [PATCH] Fix js.el filling inline JSDoc tags

bug#26550: [PATCH] Fix js.el filling inline JSDoc tags

[Etienne Prud'homme]

[-- Attachment #1.1: Type: text/plain, Size: 1001 bytes --]

The attached patch fixes filling inline JSDoc tags in js-mode. JSDoc allows
using tags in the form of:

> /**
>  * @foo
>  * Description. Link to other symbol {@link document.body.style}
>  * @bar

Where "@link" is the inline tag.

Currently, the paragraph starts at the "@" sign. Therefore filling the
Description line would never make the "@bar" tag appear in an other column
(that's what we want).

However, the link tag can be broken arbitrary and would make the tag harder
to read (without considering font-lock that is implemented in an other
package).

> /**
>  * @foo
>  * Description. Link to other symbol {@link
>  * document.body.style}
>  * @bar

I searched the JSDoc documentation[1] about line breaks inside an inline
tags and didn't find explicit information about whether or not it was
valid. Given that inline litterally means on one line, I guess we can
pretend the specification doesn't allow line breaks inside an inline tag.


[1] http://usejsdoc.org/about-block-inline-tags.html

[-- Attachment #1.2: Type: text/html, Size: 1254 bytes --]

[-- Attachment #2: 0001-Fix-js.el-filling-inline-JSDoc-tags.patch --]
[-- Type: text/x-patch, Size: 3774 bytes --]

From a463d7b7c1a8e8734ac4bd7be4b32528c2a7614c Mon Sep 17 00:00:00 2001
From: Etienne <notetiene@users.noreply.github.com>
Date: Mon, 17 Apr 2017 23:35:21 -0400
Subject: [PATCH] Fix js.el filling inline JSDoc tags
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lisp/progmodes/js.el (js-inline-jsdoc-tag-p): Add this predicate
  function to check if the point is inside an inline JSDoc tag.
(js-mode): Add `js-inline-jsdoc-tag-p' to `fill-nobreak-predicate'
hook.
* test/lisp/progmodes/js-tests.el (js-mode-fill-fill-jsdoc-at-start):
  Add this test to check that a paragraph starts from “@” according to
  the local `c-paragraph-start' value.
* test/lisp/progmodes/js-tests.el (js-mode-fill-inline-jsdoc-tag): Add
  this test to check filling doesn’t break an inline JSDoc tag.
---
 lisp/progmodes/js.el            | 11 +++++++++++
 test/lisp/progmodes/js-tests.el | 31 +++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index bae9e52..6ea3590 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -2733,6 +2733,13 @@ current buffer.  Pushes a mark onto the tag ring just like
     (push-mark)
     (goto-char marker)))
 
+(defun js-inline-jsdoc-tag-p ()
+  "Return non-nil if the point is inside an inline JSDoc tag.
+This function is useful for adding to the hook `fill-nobreak-predicate',
+to prevent the breaking of a line inside an inline JSDoc tag."
+  ;; First check that we’re inside a comment
+  (thing-at-point-looking-at "{@\\(?:tutorial\\|link\\(?:code\\|plain\\)?\\).*}"))
+
 ;;; MozRepl integration
 
 (define-error 'js-moz-bad-rpc "Mozilla RPC Error") ;; '(timeout error))
@@ -3876,6 +3883,10 @@ If one hasn't been set, or if it's stale, prompt for a new one."
         c-line-comment-starter "//"
         c-comment-start-regexp "/[*/]\\|\\s!"
         comment-start-skip "\\(//+\\|/\\*+\\)\\s *")
+
+  ;; Prevent {@tag foo} from filling
+  (add-hook 'fill-nobreak-predicate #'js-inline-jsdoc-tag-p nil t)
+
   (setq-local comment-line-break-function #'c-indent-new-comment-line)
   (setq-local c-block-comment-start-regexp "/\\*")
   (setq-local comment-multi-line t)
diff --git a/test/lisp/progmodes/js-tests.el b/test/lisp/progmodes/js-tests.el
index 8e1bac1..973cac7 100644
--- a/test/lisp/progmodes/js-tests.el
+++ b/test/lisp/progmodes/js-tests.el
@@ -60,6 +60,37 @@
  * Load the inspector's shared head.js for use by tests that need to
  * open the something or other"))))
 
+(ert-deftest js-mode-fill-fill-jsdoc-at-start ()
+  (with-temp-buffer
+    (insert "/**\n")
+    (insert " * Load the inspector's shared head.js for use by tests that ")
+    (insert "need to open the something or other\n")
+    (insert " * @foo")
+    (js-mode)
+    (goto-char (point-min))
+    (fill-paragraph)
+    (should (equal (buffer-substring (point-min) (point-max))
+                   "/**
+ * Load the inspector's shared head.js for use by tests that need to
+ * open the something or other
+ * @foo"))))
+
+(ert-deftest js-mode-fill-inline-jsdoc-tag ()
+  (with-temp-buffer
+    (insert "/**\n")
+    (insert " * Load the inspector's shared head.js for use by tests that ")
+    (insert "{@link document.body.style} need to open the something or other\n")
+    (insert " * @foo")
+    (js-mode)
+    (goto-char (point-min))
+    (fill-paragraph)
+    (should (equal (buffer-substring (point-min) (point-max))
+                   "/**
+ * Load the inspector's shared head.js for use by tests
+ * that {@link document.body.style} need to open the something or
+ * other
+ * @foo"))))
+
 (ert-deftest js-mode-regexp-syntax ()
   (with-temp-buffer
     ;; Normally indentation tests are done in manual/indent, but in
-- 
2.7.4

bug#26550:

[Etienne Prud'homme]

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

I've went ahead and submitted an issue on GitHub[1] to see if it's a
valid syntax. I would ask not to apply that patch until then. And of
course I'm always open on suggestions.

[1] https://github.com/jsdoc3/jsdoc3.github.com/issues/152

ps: I've already made the copyright assignment paperwork.

[-- Attachment #2: Type: text/html, Size: 421 bytes --]

bug#26550:

[Etienne Prud'homme]

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

After checking the code, it seems that having a newline inside the inline
tag is valid [1]. However, I still think it removes clarity and it would be
better to implement the patch I proposed.

[1] https://github.com/jsdoc3/jsdoc/blob/master/lib/jsdoc/tag/inline.js#L55

2017-04-18 12:14 GMT-04:00 Etienne Prud'homme <e.e.f.prudhomme@gmail.com>:

> I've went ahead and submitted an issue on GitHub[1] to see if it's a
> valid syntax. I would ask not to apply that patch until then. And of
> course I'm always open on suggestions.
>
> [1] https://github.com/jsdoc3/jsdoc3.github.com/issues/152
>
> ps: I've already made the copyright assignment paperwork.
>

[-- Attachment #2: Type: text/html, Size: 1188 bytes --]

bug#26550: [PATCH] Fix js.el filling inline JSDoc tags

[Lars Ingebrigtsen]

"Etienne Prud'homme" <e.e.f.prudhomme@gmail.com> writes:

> However, the link tag can be broken arbitrary and would make the tag harder
> to read (without considering font-lock that is implemented in an other
> package).
>
>> /**
>>  * @foo
>>  * Description. Link to other symbol {@link
>>  * document.body.style}
>>  * @bar
>
> I searched the JSDoc documentation[1] about line breaks inside an inline
> tags and didn't find explicit information about whether or not it was valid.
> Given that inline litterally means on one line, I guess we can pretend the
> specification doesn't allow line breaks inside an inline tag.

[...]

> After checking the code, it seems that having a newline inside the inline tag
> is valid [1].

Given this, the proposed change may not be the right one, so I'm closing
this bug report.

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