Subject: bug#21799: closed (Re: bug#21799: Requesting review for change to js.el)

From: [email-masked] (GNU bug Tracking System)
Date: Tue, 23 Feb 2016 10:15:03 +0000

Your bug report

#21799: Requesting review for change to js.el

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
21799: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=21799
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#21799: Requesting review for change to js.el
From: Dmitry Gutov <[email-masked]>
To: Lars Ingebrigtsen <[email-masked]>, Jackson Hamilton <[email-masked]>
Cc: [email-masked]
Date: Tue, 23 Feb 2016 12:14:35 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

On 02/23/2016 07:15 AM, Lars Ingebrigtsen wrote:

> Looks good to me, I think, but I think it should probably be separated
> out into its own file: jsx-mode.el.

Sorry, we've already applied it and forgot to close the bug.

Calling it jsx-mode.el might be suboptimal due to the conflict: 
https://github.com/jsx/jsx-mode.el (and that one's for a different JSX).

> Also, I can't find your name in the Emacs copyright assignment file.

It's there, as "Jackson Ray Hamilton".

Subject: Requesting review for change to js.el
From: Jackson Hamilton <[email-masked]>
To: "[email-masked]" <[email-masked]>
Date: Sat, 31 Oct 2015 02:37:33 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

See attached patch. Adds JSX indentation support to js-mode.
diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 5a4f383..c0e5655 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -52,6 +52,7 @@
 (require 'imenu)
 (require 'moz nil t)
 (require 'json nil t)
+(require 'sgml-mode)
 
 (eval-when-compile
   (require 'cl-lib)
@@ -551,6 +552,18 @@ don't indent the first one's initializer; otherwise, indent it.
   :safe 'symbolp
   :group 'js)
 
+(defcustom js-indent-jsx nil
+  "Non-nil to indent JSX elements.
+
+`sgml-basic-offset' and `sgml-attribute-offset' determine the
+indentation level of lines containing JSX.
+
+Enabling this may slow down indentation of entire large files."
+  :version "25.1"
+  :type 'boolean
+  :safe 'booleanp
+  :group 'js)
+
 ;;; KeyMap
 
 (defvar js-mode-map
@@ -1998,9 +2011,195 @@ indentation is aligned to that column."
            (+ js-indent-level js-expr-indent-offset))
           (t 0))))
 
-(defun js-indent-line ()
+;;; JSX Indentation
+
+(defsubst js--jsx-find-before-tag ()
+  "Find where JSX starts.
+
+Assume JSX appears in the following instances:
+- Inside parentheses, when returned or as the first argument
+  to a function, and after a newline
+- When assigned to variables or object properties, but only
+  on a single line
+- As the N+1th argument to a function
+
+This is an optimized version of (re-search-backward \"[(,]\n\"
+nil t), except set point to the end of the match.  This logic
+executes up to the number of lines in the file, so it should be
+really fast to reduce that impact."
+  (let (pos)
+    (while (and (> (point) (point-min))
+                (not (progn
+                       (end-of-line 0)
+                       (when (or (eq (char-before) 40)   ; (
+                                 (eq (char-before) 44))  ; ,
+                         (setq pos (1- (point))))))))
+    pos))
+
+(defconst js--jsx-end-tag-re
+  (concat "</" sgml-name-re ">\\|/>")
+  "Find the end of a JSX element.")
+
+(defconst js--jsx-after-tag-re "[),]"
+  "Find where JSX ends.
+This complements the assumption of where JSX appears from
+`js--jsx-before-tag-re', which see.")
+
+(defun js--jsx-indented-element-p ()
+  "Determine if/how the current line should be indented as JSX.
+
+Return `first' for the first JSXElement on its own line.
+Return `nth' for subsequent lines of the first JSXElement.
+Return `expression' for an embedded JS expression.
+Return `after' for anything after the last JSXElement.
+Return nil for non-JSX lines.
+
+Currently, JSX indentation supports the following styles:
+
+- Single-line elements (indented like normal JS):
+
+  var element = <div></div>;
+
+- Multi-line elements (enclosed in parentheses):
+
+  function () {
+    return (
+      <div>
+        <div></div>
+      </div>
+    );
+ }
+
+- Function arguments:
+
+  React.render(
+    <div></div>,
+    document.querySelector('.root')
+  );"
+  (let ((current-pos (point))
+        (current-line (line-number-at-pos))
+        last-pos
+        before-tag-pos before-tag-line
+        tag-start-pos tag-start-line
+        tag-end-pos tag-end-line
+        after-tag-line
+        parens paren type)
+    (save-excursion
+      (and
+       ;; Determine if we're inside a jsx element
+       (progn
+         (end-of-line)
+         (while (and (not tag-start-pos)
+                     (setq last-pos (js--jsx-find-before-tag)))
+           (while (forward-comment 1))
+           (when (= (char-after) 60) ; <
+             (setq before-tag-pos last-pos
+                   tag-start-pos (point)))
+           (goto-char last-pos))
+         tag-start-pos)
+       (progn
+         (setq before-tag-line (line-number-at-pos before-tag-pos)
+               tag-start-line (line-number-at-pos tag-start-pos))
+         (and
+          ;; A "before" line which also starts an element begins with js, so
+          ;; indent it like js
+          (> current-line before-tag-line)
+          ;; Only indent the jsx lines like jsx
+          (>= current-line tag-start-line)))
+       (cond
+        ;; Analyze bounds if there are any
+        ((progn
+           (while (and (not tag-end-pos)
+                       (setq last-pos (re-search-forward js--jsx-end-tag-re nil t)))
+             (while (forward-comment 1))
+             (when (looking-at js--jsx-after-tag-re)
+               (setq tag-end-pos last-pos)))
+           tag-end-pos)
+         (setq tag-end-line (line-number-at-pos tag-end-pos)
+               after-tag-line (line-number-at-pos after-tag-line))
+         (or (and
+              ;; Ensure we're actually within the bounds of the jsx
+              (<= current-line tag-end-line)
+              ;; An "after" line which does not end an element begins with
+              ;; js, so indent it like js
+              (<= current-line after-tag-line))
+             (and
+              ;; Handle another case where there could be e.g. comments after
+              ;; the element
+              (> current-line tag-end-line)
+              (< current-line after-tag-line)
+              (setq type 'after))))
+        ;; They may not be any bounds (yet)
+        (t))
+       ;; Check if we're inside an embedded multi-line js expression
+       (cond
+        ((not type)
+         (goto-char current-pos)
+         (end-of-line)
+         (setq parens (nth 9 (syntax-ppss)))
+         (while (and parens (not type))
+           (setq paren (car parens))
+           (cond
+            ((and (>= paren tag-start-pos)
+                  ;; Curly bracket indicates the start of an embedded expression
+                  (= (char-after paren) 123) ; {
+                  ;; The first line of the expression is indented like sgml
+                  (> current-line (line-number-at-pos paren))
+                  ;; Check if within a closing curly bracket (if any)
+                  ;; (exclusive, as the closing bracket is indented like sgml)
+                  (cond
+                   ((progn
+                      (goto-char paren)
+                      (ignore-errors (let (forward-sexp-function)
+                                       (forward-sexp))))
+                    (< current-line (line-number-at-pos)))
+                   (t)))
+             ;; Indicate this guy will be indented specially
+             (setq type 'expression))
+            (t (setq parens (cdr parens)))))
+         t)
+        (t))
+       (cond
+        (type)
+        ;; Indent the first jsx thing like js so we can indent future jsx things
+        ;; like sgml relative to the first thing
+        ((= current-line tag-start-line) 'first)
+        ('nth))))))
+
+(defmacro js--as-sgml (&rest body)
+  "Execute BODY as if in sgml-mode."
+  `(with-syntax-table sgml-mode-syntax-table
+     (let (forward-sexp-function
+           parse-sexp-lookup-properties)
+       ,@body)))
+
+(defun js--expression-in-sgml-indent-line ()
+  "Indent the current line as JavaScript or SGML (whichever is farther)."
+  (let* (indent-col
+         (savep (point))
+         ;; Don't whine about errors/warnings when we're indenting.
+         ;; This has to be set before calling parse-partial-sexp below.
+         (inhibit-point-motion-hooks t)
+         (parse-status (save-excursion
+                         (syntax-ppss (point-at-bol)))))
+    ;; Don't touch multiline strings.
+    (unless (nth 3 parse-status)
+      (setq indent-col (save-excursion
+                         (back-to-indentation)
+                         (if (>= (point) savep) (setq savep nil))
+                         (js--as-sgml (sgml-calculate-indent))))
+      (if (null indent-col)
+          'noindent
+        ;; Use whichever indentation column is greater, such that the sgml
+        ;; column is effectively a minimum
+        (setq indent-col (max (js--proper-indentation parse-status)
+                              (+ indent-col js-indent-level)))
+        (if savep
+            (save-excursion (indent-line-to indent-col))
+          (indent-line-to indent-col))))))
+
+(defun js--indent-line-as-js ()
   "Indent the current line as JavaScript."
-  (interactive)
   (let* ((parse-status
           (save-excursion (syntax-ppss (point-at-bol))))
          (offset (- (point) (save-excursion (back-to-indentation) (point)))))
@@ -2008,6 +2207,29 @@ indentation is aligned to that column."
       (indent-line-to (js--proper-indentation parse-status))
       (when (> offset 0) (forward-char offset)))))
 
+(defun js-indent-line ()
+  "Indent the current line as JavaScript or JSX source text.
+If `js-indent-jsx' is non-nil, which see, indent the current line
+as JSX source text."
+  (interactive)
+  (cond
+   (js-indent-jsx
+    (let ((indentation-type (js--jsx-indented-element-p)))
+      (cond
+       ((eq indentation-type 'expression)
+        (js--expression-in-sgml-indent-line))
+       ((or (eq indentation-type 'first)
+            (eq indentation-type 'after))
+        ;; Don't treat this first thing as a continued expression (often a "<" or
+        ;; ">" causes this misinterpretation)
+        (cl-letf (((symbol-function #'js--continued-expression-p) 'ignore))
+          (js--indent-line-as-js)))
+       ((eq indentation-type 'nth)
+        (js--as-sgml (sgml-indent-line)))
+       (t (js--indent-line-as-js)))))
+   (t
+    (js--indent-line-as-js))))
+
 ;;; Filling
 
 (defvar js--filling-paragraph nil)
diff --git a/test/indent/js-indent-jsx.js b/test/indent/js-indent-jsx.js
new file mode 100644
index 0000000..1ea65ed
--- /dev/null
+++ b/test/indent/js-indent-jsx.js
@@ -0,0 +1,84 @@
+var foo = <div></div>;
+
+return (
+  <div>
+  </div>
+  <div>
+    <div></div>
+    <div>
+      <div></div>
+    </div>
+  </div>
+);
+
+React.render(
+  <div>
+    <div></div>
+  </div>,
+  {
+    a: 1
+  },
+  <div>
+    <div></div>
+  </div>
+);
+
+return (
+  // Sneaky!
+  <div></div>
+);
+
+return (
+  <div></div>
+  // Sneaky!
+);
+
+React.render(
+  <input
+    />,
+  {
+    a: 1
+  }
+);
+
+return (
+  <div>
+    {array.map(function () {
+      return {
+        a: 1
+      };
+    })}
+  </div>
+);
+
+return (
+  <div attribute={array.map(function () {
+         return {
+           a: 1
+         };
+
+         return {
+           a: 1
+         };
+
+         return {
+           a: 1
+         };
+       })}>
+  </div>
+);
+
+// Local Variables:
+// indent-tabs-mode: nil
+// js-indent-level: 2
+// js-indent-jsx: t
+// End:
+
+// The following test has intentionally unclosed elements and should
+// be placed below all other tests to prevent awkward indentation.
+
+return (
+  <div>
+    {array.map(function () {
+      return {
+        a: 1