* Comprehensive JSX support in Emacs
@ 2019-02-14 5:06 Jackson Ray Hamilton
2019-02-14 8:03 ` Marcin Borkowski
` (4 more replies)
0 siblings, 5 replies; 33+ messages in thread
From: Jackson Ray Hamilton @ 2019-02-14 5:06 UTC (permalink / raw)
To: emacs-devel
[-- Attachment #1.1: Type: text/plain, Size: 10279 bytes --]
Hello Emacs maintainers,
A few years ago, I took up the challenge of implementing indentation support for the JavaScript extension “JSX” (by Facebook, for their “React” UI library), and we got my patches merged into js-mode. Now I think it’s time we reexamined and began to improve Emacs’ JSX+React support.
My initial implementation was not too great, because it failed to indent JSX code properly in many common scenarios. My hueristics for finding HTML-like code inside a JS buffer were too conservative, due to a (perhaps excessive/misguided) attention to performance in particular areas of the code. Since my patch’s release, I’ve been watching from a distance as the bug reports have piled up.
A package called “rjsx-mode” came onto the scene not too later, which seems to solve indentation problems using an AST (building on js2-mode’s AST); however, according to Steve Yegge’s post-mortem on js2-mode (http://steve-yegge.blogspot.com/2008/03/js2-mode-new-javascript-mode-for-emacs.html), relying on an AST being available after every keystroke is probably not optimal for performance, especially with large files. rjsx-mode also performs syntax highlighting (again, using its AST), which js-mode still does not do at all for JSX.
Recently, I finally resolved to “do my laundry,” with respect to addressing the JSX indentation issues accumulated within debbugs and the js2-mode GitHub repo (where lots of js-mode bugs accidentally get filed). In the past week, I assembled some test cases, drafted some algorithms, and proceeded to implement a number of improvements to the indentation logic.
As I started playing with the code again, many new ideas for improving JSX+React support began popping into my head, beyond just indentation improvements. I wanted to share those ideas here, and see if anyone wants to thumbs-up them before I go ahead and implement them (perhaps avoiding some bad ideas and/or rework).
Also, I wanted to share my work-in-progress indentation reimplementation, and determine if I am not going completely in the wrong direction, and if an alternate plan I’ll also present for indentation is at all viable in comparison.
Proposition: Comprehensive JSX support in Emacs
First: Consider reworking obtuse APIs
JSX indentation support is currently enabled by activating “js-jsx-mode”, using a function called “js-jsx-indent-line”, and “sgml-” variables control the indentation of the JSX code.
I think introducing JSX support in the form of a derivative mode may have been a mistake. It wasn’t initially obvious to some people to use this mode. There was not an auto-mode-alist item for it, either. It also had the effect of creating a “matrix” of modes, where, for instance, Flycheck has to support all the different JavaScript modes like this:
:modes (js-mode js-jsx-mode js2-mode js2-jsx-mode js3-mode rjsx-mode)
which seems messy to me.
And what if we want to support more JavaScript syntax extensions, like Facebook’s “Flow,” and support the use of multiple extensions at once? We wouldn’t want to also have a js-flow-mode, js-jsx-flow-mode, js2-flow-mode, js2-jsx-flow-mode… therefore, I think it’d be better to start scaling back, and instead enable JSX, Flow, et al support with a variable.
Also, upon reflection, I’m becoming more certain that controlling JavaScript indentation (even HTML-like indentation) with sgml- variables was unintuitive and therefore also a mistake. I think JSX should simply be indented using js-indent-level, perhaps optionally in combination with a js-jsx-attribute-offset, to complement the sgml-attribute-offset that we would otherwise be eliminating as an option. However, having added sgml-attribute-offset to Emacs myself merely as a precaution after a minor breaking change I made to SGML indentation a few years ago, I think we should let users actually demand JSX attribute indentation deltas, rather than assume they desire that level of granularity (considering we went so long without it in sgml-mode). Rather, users have indicated in bug reports that they’d prefer better defaults for the indentation (assuming indentation not matching “official” conventions is simply “erroneous;” and what a blessing to have conventions).
I think JSX shoud “just work” in Emacs; i.e., with js-mode, whenever a user opens a file using JSX syntax, adapting to the user’s conventions for JS.
Deprecation: Guide users to new, nicer APIs
* Deprecate js-jsx-mode (and js2-jsx-mode downstream, and have rjsx-mode derive from js2-mode). Both modes will still exist, but will be marked obsolete, and will simply make a file-local copy of js-syntax-extensions (see below) with 'jsx added to the front of the list, instead of binding indentation like they currently do.
* Deprecate js-jsx-indent-line, marking it obsolete. It will still exist, but it will simply call through to js-indent-line, with a copy of js-syntax-extensions let-bound, with 'jsx added to the front of the list.
New features:
* js-syntax-extensions: A list that will include symbols representing syntax extensions for which we will enable indentation/fontification.
o In theory, if we supported more syntax extensions, those would go here as well; also, if any of the extensions happened to conflict with each other in some ways but not others, we could use the order of the list to determine the “precedence” of the extensions.
o Having one variable for this gives us an opportunity to document all the supported syntaxes in one place, so more people will discover what’s all available.
o Add a Customize interface for adding to this list like a set, picking from a set of options. (Not sure which widget would be best to use for that.)
* js-indent-line will switch to a JSX indentation strategy when called while js-syntax-extensions contains 'jsx.
* js-mode will automatically detect whether a file uses JSX syntax using some heuristic. Suggestions:
o 'jsx is in js-syntax-extensions via init file, mode hook, or in .dir-locals.el
o buffer-file-name has extension “.jsx” (some people use this)
o “^\(var\|let\|const\|import\) React” matches within the first 1000 lines after any keystroke. (Under typical compiler configuration, “React” must be a JavaScript variable in scope in order for the compiled JSX to function properly, and most people import or require() stuff at the beginning of their files. React is usually used in combination with a JavaScript compiler, implying the user probably isn’t using React as a global variable, because compilers make that practice obsolete, so I expect this check will be pretty reliable)
* Add auto-mode-alist item mapping “.jsx” files to js-mode, enabling the buffer-file-name heuristic.
* As syntax extensions are enabled/disabled, update the mode name; ex:
o No syntax extensions: "JavaScript"
o js-syntax-extensions = (jsx): "JavaScript[JSX]"
o js-syntax-extensions = (flow jsx): "JavaScript[Flow,JSX]"
* Fill in current gaps in indentation support for JSX.
* Add font locking for JSX syntax, similar if not the same as sgml-mode’s font locking. It is enabled when js-syntax-extensions contains 'jsx.
Would appreciate feedback on these feature propositions; thanks!
Indentation: WIP
As I mentioned, I’ve already started hacking on the indentation. WIP patches are attached for preliminary review of approach (please don’t merge these yet, I haven’t vetted them thoroughly outside make test cases).
I started by reworking my “JSX detection” code to use sgml-get-context to figure out if we were inside JSX, which was a lot more reliable than the previous heuristic (JSX pretty much always had to be wrapped in parens). I then proceeded to mostly solve Bug#24896 by distinguishing “<” and “>” in JS and JSX, so the sgml-mode indentation code could parse the code without tripping over arrow functions (“=>”), thinking they were part of HTML “<elements>”. I disambiguated the symbols in a syntax-propertize-function by doing a quick parse near every “<” and “>” char to determine if the surrounding code could only be syntactically valid as JSX, and not as JS. This seems to have fixed a lot of the failing cases. However, it still fails in a weird case where JSX appears nested inside JS, where that JS is also nested inside an outer JSX, a situation that just isn’t relevant in SGML, and thus the indenter doesn’t seem to support it.
// JS expressions should not break indentation
// (https://github.com/mooz/js2-mode/issues/462).
return (
<Router>
<Bar>
<Route exact path="/foo" render={() => (
<div>nothing</div>
)} />
<Route exact path="/bar" />
</Bar>
</Router>
)
Maybe I could fix this recursive indentation issue by parsing the JSX further, and marking any “{”, “}” pairs therein with some syntax property that would get sgml-indent-line to treat it like some nested thing. However, that also might not work, and I don’t think I want to jump through any more hoops to make sgml-indent-line work inside js-mode.
Also, since I’ve already started parsing JSX to disambiguate it from JS, I figure I may as well finish the parse (it’s pretty simple), since I could use that parse as an opportunity to mark characters for JSX-contextual font locking. And, since I’m adding these markings to buffer positions, I now have a great new way to figure out if I’m inside JSX when I’m indenting. So I’m thinking it may be time to graduate from sgml-indent-line delegation, and use the new data we now have to implement the JSX indentation logic in its entirety in js-mode. This way we could definitely support recursive JSX indentation, and also align all the indentation conventions with those used by the React community, and potentially maximize performance by avoiding a lot of checks that poor sgml-mode has to do for HTML’s loose and more complex semantics.
Please let me know what you think of my current patches and the direction they’re going in, and if porting a simplified sgml-indent-line to js-mode is not too crazy of an idea!
Thanks, Jackson
[-- Attachment #1.2: Type: text/html, Size: 11719 bytes --]
[-- Attachment #2: 0001-Add-failing-tests-for-JSX-indentation-bugs.patch --]
[-- Type: text/x-patch, Size: 6488 bytes --]
From 896e196246b87f65bd65ebc29a96347792b578e5 Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Sat, 9 Feb 2019 15:42:42 -0800
Subject: [PATCH] Add failing tests for JSX indentation bugs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* test/manual/indent/js-jsx.js: Add failing tests for all the js-mode
and js2-mode JSX indentation bugs reported over the years that I could
find. Some may be duplicates, so I have grouped similar reports
together, for now; we’ll see for certain which distinct cases we need
once we start actually implementing fixes.
* test/manual/indent/js-jsx-quote.js: New file with a nasty test.
---
test/manual/indent/js-jsx-quote.js | 18 ++++
test/manual/indent/js-jsx.js | 183 +++++++++++++++++++++++++++++++++++++
2 files changed, 201 insertions(+)
create mode 100644 test/manual/indent/js-jsx-quote.js
diff --git a/test/manual/indent/js-jsx-quote.js b/test/manual/indent/js-jsx-quote.js
new file mode 100644
index 0000000000..4b71a65674
--- /dev/null
+++ b/test/manual/indent/js-jsx-quote.js
@@ -0,0 +1,18 @@
+// -*- mode: js-jsx; -*-
+
+// JSX text node values should be strings, but only JS string syntax
+// is considered, so quote marks delimit strings like normal, with
+// disastrous results (https://github.com/mooz/js2-mode/issues/409).
+function Bug() {
+ return <div>C'est Montréal</div>;
+}
+function Test(foo = /'/,
+ bar = 123) {}
+
+// This test is in a separate file because it can break other tests
+// when indenting the whole buffer (not sure why).
+
+// Local Variables:
+// indent-tabs-mode: nil
+// js-indent-level: 2
+// End:
diff --git a/test/manual/indent/js-jsx.js b/test/manual/indent/js-jsx.js
index 7401939d28..35ca4b275a 100644
--- a/test/manual/indent/js-jsx.js
+++ b/test/manual/indent/js-jsx.js
@@ -70,6 +70,189 @@ return (
</div>
);
+// Indent void expressions (no need for contextual parens / commas)
+// (https://github.com/mooz/js2-mode/issues/140#issuecomment-166250016).
+<div className="class-name">
+ <h2>Title</h2>
+ {array.map(() => {
+ return <Element />;
+ })}
+ {message}
+</div>
+// Another example of above issue
+// (https://github.com/mooz/js2-mode/issues/490).
+<App>
+ <div>
+ {variable1}
+ <Component/>
+ </div>
+</App>
+
+// Comments and arrows can break indentation (Bug#24896 /
+// https://github.com/mooz/js2-mode/issues/389).
+const Component = props => (
+ <FatArrow a={e => c}
+ b={123}>
+ </FatArrow>
+);
+const Component = props => (
+ <NoFatArrow a={123}
+ b={123}>
+ </NoFatArrow>
+);
+const Component = props => ( // Parse this comment, please.
+ <FatArrow a={e => c}
+ b={123}>
+ </FatArrow>
+);
+const Component = props => ( // Parse this comment, please.
+ <NoFatArrow a={123}
+ b={123}>
+ </NoFatArrow>
+);
+// Another example of above issue (Bug#30225).
+class {
+ render() {
+ return (
+ <select style={{paddingRight: "10px"}}
+ onChange={e => this.setState({value: e.target.value})}
+ value={this.state.value}>
+ <option>Hi</option>
+ </select>
+ );
+ }
+}
+
+// JSX attributes of an arrow function’s expression body’s JSX
+// expression should be indented with respect to the JSX opening
+// element (Bug#26001 /
+// https://github.com/mooz/js2-mode/issues/389#issuecomment-271869380).
+class {
+ render() {
+ const messages = this.state.messages.map(
+ message => <Message key={message.id}
+ text={message.text}
+ mine={message.mine} />
+ ); return messages;
+ }
+ render() {
+ const messages = this.state.messages.map(message =>
+ <Message key={message.timestamp}
+ text={message.text}
+ mine={message.mine} />
+ ); return messages;
+ }
+}
+
+// Users expect tag closers to align with the tag’s start; this is the
+// style used in the React docs, so it should be the default.
+// - https://github.com/mooz/js2-mode/issues/389#issuecomment-390766873
+// - https://github.com/mooz/js2-mode/issues/482
+// - Bug#32158
+const foo = (props) => (
+ <div>
+ <input
+ cat={i => i}
+ />
+ <button
+ className="square"
+ >
+ {this.state.value}
+ </button>
+ </div>
+);
+
+// Embedded JSX in parens breaks indentation
+// (https://github.com/mooz/js2-mode/issues/411).
+let a = (
+ <div>
+ {condition && <Component/>}
+ {condition && <Component/>}
+ <div/>
+ </div>
+)
+let b = (
+ <div>
+ {condition && (<Component/>)}
+ <div/>
+ </div>
+)
+let c = (
+ <div>
+ {condition && (<Component/>)}
+ {condition && "something"}
+ </div>
+)
+let d = (
+ <div>
+ {(<Component/>)}
+ {condition && "something"}
+ </div>
+)
+// Another example of the above issue (Bug#27000).
+function testA() {
+ return (
+ <div>
+ <div> { ( <div/> ) } </div>
+ </div>
+ );
+}
+function testB() {
+ return (
+ <div>
+ <div> { <div/> } </div>
+ </div>
+ );
+}
+// Another example of the above issue
+// (https://github.com/mooz/js2-mode/issues/451).
+class Classy extends React.Component {
+ render () {
+ return (
+ <div>
+ <ul className="tocListRoot">
+ { this.state.list.map((item) => {
+ return (<div />)
+ })}
+ </ul>
+ </div>
+ )
+ }
+}
+
+// Self-closing tags should be indented properly
+// (https://github.com/mooz/js2-mode/issues/459).
+export default ({ stars }) => (
+ <div className='overlay__container'>
+ <div className='overlay__header overlay--text'>
+ Congratulations!
+ </div>
+ <div className='overlay__reward'>
+ <Icon {...createIconProps(stars > 0)} size='large' />
+ <div className='overlay__reward__bottom'>
+ <Icon {...createIconProps(stars > 1)} size='small' />
+ <Icon {...createIconProps(stars > 2)} size='small' />
+ </div>
+ </div>
+ <div className='overlay__description overlay--text'>
+ You have created <large>1</large> reminder
+ </div>
+ </div>
+)
+
+// JS expressions should not break indentation
+// (https://github.com/mooz/js2-mode/issues/462).
+return (
+ <Router>
+ <Bar>
+ <Route exact path="/foo" render={() => (
+ <div>nothing</div>
+ )} />
+ <Route exact path="/bar" />
+ </Bar>
+ </Router>
+)
+
// Local Variables:
// indent-tabs-mode: nil
// js-indent-level: 2
--
2.11.0
[-- Attachment #3: 0002-Refactor-JSX-indentation-code-to-improve-enclosing-J.patch --]
[-- Type: text/x-patch, Size: 19599 bytes --]
From c232c5f9dba926d8a80f0ebfe620fdfa7bfcbcbf Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Sat, 9 Feb 2019 20:06:29 -0800
Subject: [PATCH] Refactor JSX indentation code to improve enclosing JSX
discovery
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Fix a number of bugs reported for JSX indentation (caused by poor JSX
detection):
- https://github.com/mooz/js2-mode/issues/140#issuecomment-166250016
- https://github.com/mooz/js2-mode/issues/490
- Bug#24896 / https://github.com/mooz/js2-mode/issues/389 (with
respect to comments)
- Bug#26001 /
https://github.com/mooz/js2-mode/issues/389#issuecomment-271869380
- https://github.com/mooz/js2-mode/issues/411 / Bug#27000 /
https://github.com/mooz/js2-mode/issues/451
Potentially manifest some new bugs (due to false positives with ‘<’
and ‘>’ and SGML detection). Slow down indentation a fair bit.
* list/progmodes/js.el (js-jsx-syntax, js--jsx-start-tag-re)
(js--looking-at-jsx-start-tag-p, js--looking-back-at-jsx-end-tag-p):
New variables and functions.
(js--jsx-find-before-tag, js--jsx-after-tag-re): Deleted.
(js--looking-at-operator-p): Don’t mistake a JSXOpeningElement for the
‘<’ operator.
(js--continued-expression-p): Don’t mistake a JSXClosingElement as a
fragment of a continued expression including the ‘>’ operator.
(js--as-sgml): Simplify. Probably needn’t bind forward-sexp-function
to nil (sgml-mode already does) and probably shouldn’t bind
parse-sexp-lookup-properties to nil either (see Bug#24896).
(js--outermost-enclosing-jsx-tag-pos): Find enclosing JSX more
accurately than js--jsx-find-before-tag. Use sgml-mode’s parsing
logic, rather than unreliable heuristics like paren-wrapping. This
implementation is much slower; the previous implementation was fast,
but at the expense of accuracy. To make up for all the grief we’ve
caused users, we will prefer accuracy over speed from now on. That
said, this can still probably be optimized a lot.
(js--jsx-indented-element-p): Rename to js--jsx-indentation, since it
doesn’t just return a boolean.
(js--jsx-indentation): Refactor js--jsx-indented-element-p to simplify
the implementation as the improved accuracy of other code allows (and
to repent for some awful stylistic choices I made earlier).
(js--expression-in-sgml-indent-line): Rename to
js--indent-line-in-jsx-expression, since it’s a private function and
we can give it a name that reads more like English.
(js--indent-line-in-jsx-expression): Restructure point adjustment
logic more like js-indent-line.
(js--indent-n+1th-jsx-line): New function to complement
js--indent-line-in-jsx-expression.
(js-jsx-indent-line): Refactor. Don’t bind js--continued-expression-p
to ignore any more; instead, rely on the improved accuracy of
js--continued-expression-p.
(js-jsx-mode): Set js-jsx-syntax to t. For now, this will be the flag
we use to determine whether ‘JSX is enabled.’ (Maybe later, we will
refactor the code to use this variable instead of requiring
js-jsx-mode to be enabled, thus rendering the mode obsolete.)
---
lisp/progmodes/js.el | 337 +++++++++++++++++++++------------------------------
1 file changed, 141 insertions(+), 196 deletions(-)
diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index b0bb8213dc..e16ed98023 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -572,6 +572,15 @@ js-chain-indent
:safe 'booleanp
:group 'js)
+(defcustom js-jsx-syntax nil
+ "When non-nil, parse JavaScript with consideration for JSX syntax.
+This fixes indentation of JSX code in some cases. It is set to
+be buffer-local when in `js-jsx-mode'."
+ :version "27.1"
+ :type 'boolean
+ :safe 'booleanp
+ :group 'js)
+
;;; KeyMap
(defvar js-mode-map
@@ -1774,6 +1783,14 @@ js--indent-operator-re
(js--regexp-opt-symbol '("in" "instanceof")))
"Regexp matching operators that affect indentation of continued expressions.")
+(defconst js--jsx-start-tag-re
+ (concat "<" sgml-name-re)
+ "Regexp matching code that looks like a JSXOpeningElement.")
+
+(defun js--looking-at-jsx-start-tag-p ()
+ "Non-nil if a JSXOpeningElement immediately follows point."
+ (looking-at js--jsx-start-tag-re))
+
(defun js--looking-at-operator-p ()
"Return non-nil if point is on a JavaScript operator, other than a comma."
(save-match-data
@@ -1796,7 +1813,9 @@ js--looking-at-operator-p
(js--backward-syntactic-ws)
;; We might misindent some expressions that would
;; return NaN anyway. Shouldn't be a problem.
- (memq (char-before) '(?, ?} ?{))))))))
+ (memq (char-before) '(?, ?} ?{)))))
+ ;; “<” isn’t necessarily an operator in JSX.
+ (not (and js-jsx-syntax (js--looking-at-jsx-start-tag-p))))))
(defun js--find-newline-backward ()
"Move backward to the nearest newline that is not in a block comment."
@@ -1816,6 +1835,14 @@ js--find-newline-backward
(setq result nil)))
result))
+(defconst js--jsx-end-tag-re
+ (concat "</" sgml-name-re ">\\|/>")
+ "Regexp matching a JSXClosingElement.")
+
+(defun js--looking-back-at-jsx-end-tag-p ()
+ "Non-nil if a JSXClosingElement immediately precedes point."
+ (looking-back js--jsx-end-tag-re (point-at-bol)))
+
(defun js--continued-expression-p ()
"Return non-nil if the current line continues an expression."
(save-excursion
@@ -1833,12 +1860,19 @@ js--continued-expression-p
(and (js--find-newline-backward)
(progn
(skip-chars-backward " \t")
- (or (bobp) (backward-char))
- (and (> (point) (point-min))
- (save-excursion (backward-char) (not (looking-at "[/*]/\\|=>")))
- (js--looking-at-operator-p)
- (and (progn (backward-char)
- (not (looking-at "+\\+\\|--\\|/[/*]"))))))))))
+ (and
+ ;; The “>” at the end of any JSXBoundaryElement isn’t
+ ;; part of a continued expression.
+ (not (and js-jsx-syntax (js--looking-back-at-jsx-end-tag-p)))
+ (progn
+ (or (bobp) (backward-char))
+ (and (> (point) (point-min))
+ (save-excursion
+ (backward-char)
+ (not (looking-at "[/*]/\\|=>")))
+ (js--looking-at-operator-p)
+ (and (progn (backward-char)
+ (not (looking-at "+\\+\\|--\\|/[/*]"))))))))))))
(defun js--skip-term-backward ()
"Skip a term before point; return t if a term was skipped."
@@ -2153,190 +2187,108 @@ js--proper-indentation
;;; 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 ()
+(defmacro js--as-sgml (&rest body)
+ "Execute BODY as if in sgml-mode."
+ `(with-syntax-table sgml-mode-syntax-table
+ ,@body))
+
+(defun js--outermost-enclosing-jsx-tag-pos ()
+ (let (context tag-pos last-tag-pos parse-status parens paren-pos curly-pos)
+ (js--as-sgml
+ ;; Search until we reach the top or encounter the start of a
+ ;; JSXExpressionContainer (implying nested JSX).
+ (while (and (setq context (sgml-get-context))
+ (progn
+ (setq tag-pos (sgml-tag-start (car (last context))))
+ (or (not curly-pos)
+ ;; Stop before curly brackets (start of a
+ ;; JSXExpressionContainer).
+ (> tag-pos curly-pos))))
+ ;; Record this position so it can potentially be returned.
+ (setq last-tag-pos tag-pos)
+ ;; Always parse sexps / search for the next context from the
+ ;; immediately enclosing tag (sgml-get-context may not leave
+ ;; point there).
+ (goto-char tag-pos)
+ (unless parse-status ; Don’t needlessly reparse.
+ ;; Search upward for an enclosing starting curly bracket.
+ (setq parse-status (syntax-ppss))
+ (setq parens (reverse (nth 9 parse-status)))
+ (while (and (setq paren-pos (car parens))
+ (not (when (= (char-after paren-pos) ?{)
+ (setq curly-pos paren-pos))))
+ (setq parens (cdr parens)))
+ ;; Always search for the next context from the immediately
+ ;; enclosing tag (calling syntax-ppss in the above loop
+ ;; may move point from there).
+ (goto-char tag-pos))))
+ last-tag-pos))
+
+(defun js--jsx-indentation ()
"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')
- );"
+Return nil for first JSXElement line (indent like JS).
+Return `n+1th' for second+ JSXElement lines (indent like SGML).
+Return `expression' for lines within embedded JS expressions
+ (indent like JS inside SGML).
+Return nil for non-JSX lines."
(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)
+ tag-start-pos 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
+ ;; Determine if inside a JSXElement.
+ (beginning-of-line) ; For exclusivity
+ (when (setq tag-start-pos (js--outermost-enclosing-jsx-tag-pos))
+ ;; Check if inside an embedded multi-line JS expression.
+ (goto-char current-pos)
+ (end-of-line) ; For exclusivity
+ (setq parens (nth 9 (syntax-ppss)))
+ (while
+ (and
+ (setq paren (car parens))
+ (if (and
+ (>= paren tag-start-pos)
+ ;; A curly bracket indicates the start of an
+ ;; embedded expression.
+ (= (char-after paren) ?{)
+ ;; 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.
+ ;; (exclusive, as the closing bracket is indented
+ ;; like SGML).
+ (if (progn
+ (goto-char paren)
+ (ignore-errors (let (forward-sexp-function)
+ (forward-sexp))))
+ (< current-line (line-number-at-pos))
+ ;; No matching bracket implies we’re inside!
+ t))
+ ;; Indicate this will be indented specially. Return
+ ;; nil to stop iterating too.
+ (progn (setq type 'expression) nil)
+ ;; Stop iterating when parens = nil.
+ (setq parens (cdr parens)))))
+ (or type 'n+1th)))))
+
+(defun js--indent-line-in-jsx-expression ()
+ "Indent the current line as JavaScript within JSX."
+ (let ((parse-status (save-excursion (syntax-ppss (point-at-bol))))
+ offset indent-col)
(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))))))
+ (save-excursion
+ (setq offset (- (point) (progn (back-to-indentation) (point)))
+ indent-col (js--as-sgml (sgml-calculate-indent))))
+ (if (null indent-col) 'noindent ; Like in sgml-mode
+ ;; Use whichever indentation column is greater, such that the
+ ;; SGML column is effectively a minimum.
+ (indent-line-to (max (js--proper-indentation parse-status)
+ (+ indent-col js-indent-level)))
+ (when (> offset 0) (forward-char offset))))))
+
+(defun js--indent-n+1th-jsx-line ()
+ "Indent the current line as JSX within JavaScript."
+ (js--as-sgml (sgml-indent-line)))
(defun js-indent-line ()
"Indent the current line as JavaScript."
@@ -2353,19 +2305,11 @@ js-jsx-indent-line
i.e., customize JSX element indentation with `sgml-basic-offset',
`sgml-attribute-offset' et al."
(interactive)
- (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)))
- ((eq indentation-type 'nth)
- (js--as-sgml (sgml-indent-line)))
- (t (js-indent-line)))))
+ (let ((type (js--jsx-indentation)))
+ (if type
+ (if (eq type 'n+1th) (js--indent-n+1th-jsx-line)
+ (js--indent-line-in-jsx-expression))
+ (js-indent-line))))
;;; Filling
@@ -3944,6 +3888,7 @@ js-jsx-mode
(setq-local sgml-basic-offset js-indent-level))
(add-hook \\='js-jsx-mode-hook #\\='set-jsx-indentation)"
:group 'js
+ (setq-local js-jsx-syntax t)
(setq-local indent-line-function #'js-jsx-indent-line))
;;;###autoload (defalias 'javascript-mode 'js-mode)
--
2.11.0
[-- Attachment #4: 0003-Add-new-failing-unclosed-JSX-test-and-separate-such-.patch --]
[-- Type: text/x-patch, Size: 2585 bytes --]
From 54b8b1cb4ab0d4041b4e0613af0bc71e594aecbc Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Sun, 10 Feb 2019 21:11:17 -0800
Subject: [PATCH] Add new (failing) unclosed JSX test and separate such tests
* test/manual/indent/js-jsx.js: Move test with intentional scan error to
its own file, js-jsx-unclosed-1.js.
* test/manual/indent/js-jsx-unclosed-1.js: New file.
* test/manual/indent/js-jsx-unclosed-2.js: New file with test for
regression caused by new ambiguous parsing of JS/JSX.
---
test/manual/indent/js-jsx-unclosed-1.js | 15 +++++++++++++++
test/manual/indent/js-jsx-unclosed-2.js | 17 +++++++++++++++++
test/manual/indent/js-jsx.js | 9 ---------
3 files changed, 32 insertions(+), 9 deletions(-)
create mode 100644 test/manual/indent/js-jsx-unclosed-1.js
create mode 100644 test/manual/indent/js-jsx-unclosed-2.js
diff --git a/test/manual/indent/js-jsx-unclosed-1.js b/test/manual/indent/js-jsx-unclosed-1.js
new file mode 100644
index 0000000000..9418aed7a1
--- /dev/null
+++ b/test/manual/indent/js-jsx-unclosed-1.js
@@ -0,0 +1,15 @@
+// -*- mode: js-jsx; -*-
+
+// Local Variables:
+// indent-tabs-mode: nil
+// js-indent-level: 2
+// End:
+
+// The following test goes below any comments to avoid including
+// misindented comments among the erroring lines.
+
+return (
+ <div>
+ {array.map(function () {
+ return {
+ a: 1
diff --git a/test/manual/indent/js-jsx-unclosed-2.js b/test/manual/indent/js-jsx-unclosed-2.js
new file mode 100644
index 0000000000..2d42cf70f8
--- /dev/null
+++ b/test/manual/indent/js-jsx-unclosed-2.js
@@ -0,0 +1,17 @@
+// -*- mode: js-jsx; -*-
+
+// Local Variables:
+// indent-tabs-mode: nil
+// js-indent-level: 2
+// End:
+
+// The following tests go below any comments to avoid including
+// misindented comments among the erroring lines.
+
+// Don’t misinterpret equality operators as JSX.
+for (; i < length;) void 0
+if (foo > bar) void 0
+
+// Don’t even misinterpret unary operators as JSX.
+if (foo < await bar) void 0
+while (await foo > bar) void 0
diff --git a/test/manual/indent/js-jsx.js b/test/manual/indent/js-jsx.js
index 35ca4b275a..af3c340559 100644
--- a/test/manual/indent/js-jsx.js
+++ b/test/manual/indent/js-jsx.js
@@ -257,12 +257,3 @@ return (
// indent-tabs-mode: nil
// js-indent-level: 2
// 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
--
2.11.0
[-- Attachment #5: 0004-js-syntax-propertize-Disambiguate-JS-from-JSX-fixing.patch --]
[-- Type: text/x-patch, Size: 7546 bytes --]
From 300a4b00af57f6e904f239ec24a85a78aa8989f5 Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Mon, 11 Feb 2019 03:00:34 -0800
Subject: [PATCH] js-syntax-propertize: Disambiguate JS from JSX, fixing some
indents
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Fix some JSX indentation bugs:
- Bug#24896 / https://github.com/mooz/js2-mode/issues/389
- Bug#30225
- https://github.com/mooz/js2-mode/issues/459
* lisp/progmodes/js.el (js--dotted-captured-name-re)
(js--unary-keyword-re, js--unary-keyword-p)
(js--disambiguate-beginning-of-jsx-tag)
(js--disambiguate-end-of-jsx-tag)
(js--disambiguate-js-from-jsx): New variables and functions.
(js-syntax-propertize): Additionally clarify when syntax is JS so that
‘(with-syntax-table sgml-mode-syntax-table …)’ does not mistake some
JS punctuation syntax for SGML parenthesis syntax, namely ‘<’ and ‘>’.
* test/manual/indent/js-jsx-unclosed-2.js: Add additional test for
unary operator parsing.
---
lisp/progmodes/js.el | 100 +++++++++++++++++++++++++++++++-
test/manual/indent/js-jsx-unclosed-2.js | 14 +++++
2 files changed, 113 insertions(+), 1 deletion(-)
diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index e16ed98023..aad8e232cd 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -82,6 +82,10 @@ js--dotted-name-re
(concat js--name-re "\\(?:\\." js--name-re "\\)*")
"Regexp matching a dot-separated sequence of JavaScript names.")
+(defconst js--dotted-captured-name-re
+ (concat "\\(" js--name-re "\\)\\(?:\\." js--name-re "\\)*")
+ "Like `js--dotted-name-re', but capture the first name.")
+
(defconst js--cpp-name-re js--name-re
"Regexp matching a C preprocessor name.")
@@ -1731,6 +1735,99 @@ js-syntax-propertize-regexp
'syntax-table (string-to-syntax "\"/"))
(goto-char end)))))
+(defconst js--unary-keyword-re
+ (js--regexp-opt-symbol '("await" "delete" "typeof" "void" "yield"))
+ "Regexp matching unary operator keywords.")
+
+(defun js--unary-keyword-p (string)
+ "Check if STRING is a unary operator keyword in JavaScript."
+ (string-match-p js--unary-keyword-re string))
+
+(defun js--disambiguate-beginning-of-jsx-tag ()
+ "Parse enough to determine if a JSX tag starts here.
+Disambiguate JSX from equality operators by testing for syntax
+only valid as JSX."
+ ;; “</…” - a JSXClosingElement.
+ ;; “<>” - a JSXOpeningFragment.
+ (if (memq (char-after) '(?\/ ?\>)) t
+ (save-excursion
+ (skip-chars-forward " \t\n")
+ (and
+ (looking-at js--dotted-captured-name-re)
+ ;; Don’t match code like “if (i < await foo)”
+ (not (js--unary-keyword-p (match-string 1)))
+ (progn
+ (goto-char (match-end 0))
+ (skip-chars-forward " \t\n")
+ (or
+ ;; “>”, “/>” - tag enders.
+ ;; “{” - a JSXExpressionContainer.
+ (memq (char-after) '(?\> ?\/ ?\{))
+ ;; Check if a JSXAttribute follows.
+ (looking-at js--name-start-re)))))))
+
+(defun js--disambiguate-end-of-jsx-tag ()
+ "Parse enough to determine if a JSX tag ends here.
+Disambiguate JSX from equality operators by testing for syntax
+only valid as JSX, or extremely unlikely except as JSX."
+ (save-excursion
+ (backward-char)
+ ;; “…/>” - a self-closing JSXOpeningElement.
+ ;; “</>” - a JSXClosingFragment.
+ (if (= (char-before) ?/) t
+ (let (last-tag-or-attr-name last-non-unary-p)
+ (catch 'match
+ (while t
+ (skip-chars-backward " \t\n")
+ ;; Check if the end of a JSXAttribute value or
+ ;; JSXExpressionContainer almost certainly precedes.
+ ;; The only valid JS this misses is
+ ;; - {} > foo
+ ;; - "bar" > foo
+ ;; which is no great loss, IMHO…
+ (if (memq (char-before) '(?\} ?\" ?\' ?\`)) (throw 'match t)
+ (if (and last-tag-or-attr-name last-non-unary-p
+ ;; “<”, “</” - tag starters.
+ (memq (char-before) '(?\< ?\/)))
+ ;; Leftmost name parsed was the name of a
+ ;; JSXOpeningElement.
+ (throw 'match t))
+ ;; Technically the dotted name could span multiple
+ ;; lines, but dear God WHY?! Also, match greedily to
+ ;; ensure the entire name is valid.
+ (if (looking-back js--dotted-captured-name-re (point-at-bol) t)
+ (if (and (setq last-non-unary-p (not (js--unary-keyword-p (match-string 1))))
+ last-tag-or-attr-name)
+ ;; Valid (non-unary) name followed rightwards by
+ ;; another name (any will do, including
+ ;; keywords) is invalid JS, but valid JSX.
+ (throw 'match t)
+ ;; Remember match and skip backwards over it when
+ ;; it is the first matched name or the N+1th
+ ;; matched unary name (unary names on the left are
+ ;; still ambiguously JS or JSX, so keep parsing to
+ ;; disambiguate).
+ (setq last-tag-or-attr-name (match-string 1))
+ (goto-char (match-beginning 0)))
+ ;; Nothing else to look for; give up parsing.
+ (throw 'match nil)))))))))
+
+(defun js--disambiguate-js-from-jsx (start end)
+ "Figure out which ‘<’ and ‘>’ chars (from START to END) aren’t JSX.
+
+Later, this info prevents ‘sgml-’ functions from treating some
+‘<’ and ‘>’ chars as parts of tokens of SGML tags — a good thing,
+since they are serving their usual function as some JS equality
+operator or arrow function, instead."
+ (goto-char start)
+ (while (re-search-forward "[<>]" end t)
+ (unless (if (eq (char-before) ?<) (js--disambiguate-beginning-of-jsx-tag)
+ (js--disambiguate-end-of-jsx-tag))
+ ;; Inform sgml- functions that this >, >=, >>>, <, <=, <<<, or
+ ;; => token is punctuation (and not an open or close parenthesis
+ ;; as per usual in sgml-mode).
+ (put-text-property (1- (point)) (point) 'syntax-table '(1)))))
+
(defun js-syntax-propertize (start end)
;; JavaScript allows immediate regular expression objects, written /.../.
(goto-char start)
@@ -1758,7 +1855,8 @@ js-syntax-propertize
'syntax-table (string-to-syntax "\"/"))
(js-syntax-propertize-regexp end)))))
("\\`\\(#\\)!" (1 "< b")))
- (point) end))
+ (point) end)
+ (if js-jsx-syntax (js--disambiguate-js-from-jsx start end)))
(defconst js--prettify-symbols-alist
'(("=>" . ?⇒)
diff --git a/test/manual/indent/js-jsx-unclosed-2.js b/test/manual/indent/js-jsx-unclosed-2.js
index 2d42cf70f8..8b6f33325d 100644
--- a/test/manual/indent/js-jsx-unclosed-2.js
+++ b/test/manual/indent/js-jsx-unclosed-2.js
@@ -15,3 +15,17 @@ if (foo > bar) void 0
// Don’t even misinterpret unary operators as JSX.
if (foo < await bar) void 0
while (await foo > bar) void 0
+
+// Allow unary keyword names as null-valued JSX attributes.
+// (As if this will EVER happen…)
+<Foo yield>
+ <Bar void>
+ <Baz
+ zorp
+ typeof>
+ <Please do_n0t delete this_stupidTest >
+ How would we ever live without unary support
+ </Please>
+ </Baz>
+ </Bar>
+</Foo>
--
2.11.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-02-14 5:06 Comprehensive JSX support in Emacs Jackson Ray Hamilton
@ 2019-02-14 8:03 ` Marcin Borkowski
2019-02-14 14:10 ` Stefan Monnier
` (3 subsequent siblings)
4 siblings, 0 replies; 33+ messages in thread
From: Marcin Borkowski @ 2019-02-14 8:03 UTC (permalink / raw)
To: Jackson Ray Hamilton; +Cc: emacs-devel
Hi Jackson,
this post is way to TL;DR for me ATM, but I just wanted to let you know
that I'm very interested.
Frankly, I think that the right solution to this problem would be to
throw JSX into a deep pit and cover it with a thick layer of cement. If
React accidentally falls down, too, I would probably rejoice even more.
But life is life, and I have to deal with those pesky JSX file from time
to time, and editing them in Emacs is a PITA.
So, thanks for your work on this, and expect some more (and less
rant-ish) input from me soon!
Best,
--
Marcin Borkowski
http://mbork.pl
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-02-14 5:06 Comprehensive JSX support in Emacs Jackson Ray Hamilton
2019-02-14 8:03 ` Marcin Borkowski
@ 2019-02-14 14:10 ` Stefan Monnier
2019-02-14 15:04 ` Clément Pit-Claudel
2019-02-15 8:21 ` Jackson Ray Hamilton
2019-02-15 14:39 ` Dmitry Gutov
` (2 subsequent siblings)
4 siblings, 2 replies; 33+ messages in thread
From: Stefan Monnier @ 2019-02-14 14:10 UTC (permalink / raw)
To: emacs-devel
[ Beware: I haven't written more than 1 or 2 lines of Javascript in
my life. ]
> instance, Flycheck has to support all the different JavaScript modes
> like this:
>
> :modes (js-mode js-jsx-mode js2-mode js2-jsx-mode js3-mode rjsx-mode)
>
> which seems messy to me.
AFAIK all these should derive from js-mode (and flycheck should pay
attention to parent modes), so `:modes (js-mode)` should cover
them all without having to use something else than major modes.
So this argument is really not convincing.
> And what if we want to support more JavaScript syntax extensions, like
> Facebook’s “Flow,” and support the use of multiple extensions
> at once?
This is a much more convincing argument. I don't know how those
various extensions might interact (both in the Javascript code and in
the js-mode support code), but it makes a lot of sense to try and make
them modular, so you can mix&match.
> Also, upon reflection, I’m becoming more certain that controlling
> JavaScript indentation (even HTML-like indentation) with sgml-
> variables was unintuitive and therefore also a mistake.
I think it's a good idea to *default* to the sgml- settings if it's
common for users to want SGML-style indentation conventions for those
parts of the code, but if many users want to use different settings in
sgml-mode and jsx-mode, then it makes sense to offer
jsx-specific overrides.
> * Deprecate js-jsx-mode (and js2-jsx-mode downstream, and have
> rjsx-mode derive from js2-mode). Both modes will still exist, but
> will be marked obsolete, and will simply make a file-local copy of
> js-syntax-extensions (see below) with 'jsx added to the front of the
> list, instead of binding indentation like they currently do.
> * Deprecate js-jsx-indent-line, marking it obsolete. It will still
> exist, but it will simply call through to js-indent-line, with a copy
> of js-syntax-extensions let-bound, with 'jsx added to the front of
> the list.
Fine.
> * js-mode will automatically detect whether a file uses JSX syntax
> using some heuristic.
Would there be any harm in activating JSX indentation&highlighting in
a Javascript buffer that doesn't use JSX?
In general, it sounds good.
Just a nitpick about the syntax-propertize part of the patch (haven't
looked at the rest, really):
- You use the word "equality" in docstrings to refer to < and > chars,
which to me aren't about equality. I think these are usually called
"inequalities". It's obviously not too important (I figured it out
in the end), but it got me confused for a little while.
- Please use a "js--jsx-" or "js-jsx--" prefix for all the code that's
related to JSX support (IOW all the code added for JSX support, or all
the code that could be removed if JSX support were dropped).
- Any reason why you add a call to js--disambiguate-js-from-jsx at the
end of js-syntax-propertize instead of adding it directly within the
syntax-propertize-rules? Doing it this way means that during
execution of syntax-propertize-rules some of the syntax-table
properties haven't been applied yet, so any call of syntax-ppss that
happens during syntax-propertize-rules may get incorrect results (and
more importantly will cache this incorrect result for later use).
Stefan
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-02-14 14:10 ` Stefan Monnier
@ 2019-02-14 15:04 ` Clément Pit-Claudel
2019-02-15 8:21 ` Jackson Ray Hamilton
1 sibling, 0 replies; 33+ messages in thread
From: Clément Pit-Claudel @ 2019-02-14 15:04 UTC (permalink / raw)
To: emacs-devel
On 14/02/2019 09.10, Stefan Monnier wrote:
> (and flycheck should pay attention to parent modes)
I can't find the discussion about this on our tracker at the moment. I remember Sebastian had a good reason for not doing that by default… but I can't remember what it was :/
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
@ 2019-02-15 6:38 Jackson Ray Hamilton
2019-02-17 6:10 ` Marcin Borkowski
0 siblings, 1 reply; 33+ messages in thread
From: Jackson Ray Hamilton @ 2019-02-15 6:38 UTC (permalink / raw)
To: mbork; +Cc: emacs-devel
Hi Marcin,
I’m glad to hear that you’re interested.
Sorry for the TL;DR. Indentation seems like a complex topic, so I
wanted to extract from my jumbled thoughts a sufficient amount of
context to help anyone else along.
I look forward to working with you to improve Emacs.
Jackson
(BTW, I’m currently figuring out the process for replying to a mailing
list — replying to you, CCing emacs-devel, trying to set In-Reply-To in
my mail client; hopefully that works.)
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-02-14 14:10 ` Stefan Monnier
2019-02-14 15:04 ` Clément Pit-Claudel
@ 2019-02-15 8:21 ` Jackson Ray Hamilton
2019-02-15 13:25 ` Stefan Monnier
2019-02-15 13:54 ` Dmitry Gutov
1 sibling, 2 replies; 33+ messages in thread
From: Jackson Ray Hamilton @ 2019-02-15 8:21 UTC (permalink / raw)
To: monnier; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 3910 bytes --]
Hi Stefan,
Thanks for your reply and your code review.
> I think it's a good idea to *default* to the sgml- settings if it's
> common for users to want SGML-style indentation conventions for those
> parts of the code, but if many users want to use different settings in
> sgml-mode and jsx-mode, then it makes sense to offer
> jsx-specific overrides.
Right. I think Emacs’ conventions for aligning SGML attributes are
mostly compatible with Reacts’, so sticking with those, it shouldn’t
feel too foreign.
I think one obvious reason to have JSX-specific settings is that
sgml-basic-offset defaults to 2, and js-indent-level defaults to 4; so
no matter what, OOTB users probably think Emacs indents JSX “wrong.” It
would surprise me if people were actually content with using 4 spaces
for JS and 2 spaces for JSX blocks within.
A philosophical and practical reason to switch will be decoupling JSX
from SGML. Using sgml-indent-line to indent JSX probably should have
been kept an implementation detail, but requiring the use of sgml-
variables uncovered that detail. If I end up changing the
implementation, as I’m considering, we could pay a price for that.
However, I expect that most users will have already configured their JSX
indentation to be consistent with JS, and thus will be unaffected; else,
I think most will welcome a change to consistent defaults as a “fix.”
> Would there be any harm in activating JSX indentation&highlighting in
> a Javascript buffer that doesn't use JSX?
Indeed, that could eliminate some false negatives and simplify the code,
and we may be able to get away with it indefinitely.
However, considering JavaScript has a well-respected specification, it
seems a bit risky to me to enable non-standard syntax extensions in
“.js” buffers without some precautions. The standards body could define
new syntax in the base language that invalidates userland syntax
extensions, or other syntax extensions with competing semantics could
become popular. JSX itself is a successor to E4X, with which it
overlaps partially.
I think long-term it’d be less disruptive to users’ configs to define
parameters for enabling syntaxes that we can reasonably expect to hold
up in an evolving ecosystem. Hence, looking for specific branding—like
“.jsx” files or a “React” variable—could help us to eventually
gracefully add support for a “TheoreticalAngleBracketsRedux” extension
also, with “.jsb” files / an “AngleBrackets” variable / a “// @Angles”
magic comment to enable it…
> - Any reason why you add a call to js--disambiguate-js-from-jsx at the
> end of js-syntax-propertize instead of adding it directly within the
> syntax-propertize-rules?
Reason: Me not knowing what I’m doing. :-) I figured that could be
written better.
Actually, I was hoping we’d be able to talk about that part. I wanted
to know if you thought I should repeat the pattern that I see near that
code, where js-syntax-propertize-regexp is called in
syntax-propertize-function first at the beginning of the function and
again later in syntax-propertize-rules — I haven’t yet figured out why
that pattern of calling the same propertize function in multiple places
exists in this mode and others (like ruby-mode’s
ruby-syntax-propertize/ruby-syntax-propertize-heredoc).
I’ve also been wondering in the back of my mind if I’ll need to use
syntax-propertize-extend-region-functions in this implementation.
Thanks again for talking this through (even though the language may feel
a bit foreign!)
Jackson
(BTW, I’m currently figuring out the process for replying to a mailing
list — replying to you, CCing emacs-devel, trying to set In-Reply-To in
my mail client. Hopefully that works this time.)
[-- Attachment #2: Type: text/html, Size: 4705 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-02-15 8:21 ` Jackson Ray Hamilton
@ 2019-02-15 13:25 ` Stefan Monnier
2019-02-15 13:54 ` Dmitry Gutov
1 sibling, 0 replies; 33+ messages in thread
From: Stefan Monnier @ 2019-02-15 13:25 UTC (permalink / raw)
To: Jackson Ray Hamilton; +Cc: emacs-devel
> I think one obvious reason to have JSX-specific settings is that
> sgml-basic-offset defaults to 2, and js-indent-level defaults to 4; so no
> matter what, OOTB users probably think Emacs indents JSX “wrong.” It would
> surprise me if people were actually content with using 4 spaces for JS and
> 2 spaces for JSX blocks within.
Makes sense.
> A philosophical and practical reason to switch will be decoupling JSX from
> SGML. Using sgml-indent-line to indent JSX probably should have been kept
> an implementation detail, but requiring the use of sgml-
> variables uncovered that detail. If I end up changing the implementation,
> as I’m considering, we could pay a price for that. However, I expect that
> most users will have already configured their JSX indentation to be
> consistent with JS, and thus will be unaffected; else, I think most will
> welcome a change to consistent defaults as a “fix.”
I think such breakage is usually not a big deal, as long as there's
a clear explanation somewhere of why it's changed and how to get back
the old behavior.
>> Would there be any harm in activating JSX indentation&highlighting in
>> a Javascript buffer that doesn't use JSX?
> Indeed, that could eliminate some false negatives and simplify the code, and
> we may be able to get away with it indefinitely.
BTW, I do think it's important to keep this feature conditional on some
setting, but if it's mostly harmless, then we can be a lot more lazy (in
the sense of a lazy programmer, not a lazy program) in terms of when we
enable/disable it.
>> - Any reason why you add a call to js--disambiguate-js-from-jsx at the
>> end of js-syntax-propertize instead of adding it directly within the
>> syntax-propertize-rules?
> Reason: Me not knowing what I’m doing. :-) I figured that could be
> written better.
OK, good.
> Actually, I was hoping we’d be able to talk about that part. I wanted to
> know if you thought I should repeat the pattern that I see near that code,
> where js-syntax-propertize-regexp is called in syntax-propertize-function
> first at the beginning of the function and again later in
> syntax-propertize-rules — I haven’t yet figured out why that pattern of
> calling the same propertize function in multiple places exists in this mode
> and others (like ruby-mode’s
> ruby-syntax-propertize/ruby-syntax-propertize-heredoc).
This pattern is used to deal with "modes". `syntax-propertize-rules`
doesn't try to find matched to all the regexps everywhere: once a regexp
has matched its code is run and that code can move point and after that
we start matching again at that position. This is typically used for
things that are "string-like" or "comment-like" so we don't mistakenly
treat the inside of one of those elements (which typically contains
arbitrary text rather than code) as code, and so we correctly recognize
and mark the "end".
The pattern you mention happens for those kinds of situations, in order
to handle the case where syntax-propertize-function is called with point
in the middle of one of those "string/comment-like" elements.
> I’ve also been wondering in the back of my mind if I’ll need to use
> syntax-propertize-extend-region-functions in this implementation.
If your code which propertizes the < and > also wants to skip text and
prevent the rest of the syntax-propertize-rules to apply to that skipped
text, then you'll probably want to follow that same pattern. But if
not, then it's not needed.
Stefan
> (BTW, I’m currently figuring out the process for replying to a mailing list
> — replying to you, CCing emacs-devel, trying to set In-Reply-To in my mail
> client. Hopefully that works this time.)
I just "reply-all".
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-02-15 8:21 ` Jackson Ray Hamilton
2019-02-15 13:25 ` Stefan Monnier
@ 2019-02-15 13:54 ` Dmitry Gutov
1 sibling, 0 replies; 33+ messages in thread
From: Dmitry Gutov @ 2019-02-15 13:54 UTC (permalink / raw)
To: Jackson Ray Hamilton, monnier; +Cc: emacs-devel
On 15.02.2019 11:21, Jackson Ray Hamilton wrote:
> However, considering JavaScript has a well-respected specification, it
> seems a bit risky to me to enable non-standard syntax extensions in
> “.js” buffers without some precautions. The standards body could define
> new syntax in the base language that invalidates userland syntax
> extensions, or other syntax extensions with competing semantics could
> become popular. JSX itself is a successor to E4X, with which it
> overlaps partially.
I think it's a very important point.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-02-14 5:06 Comprehensive JSX support in Emacs Jackson Ray Hamilton
2019-02-14 8:03 ` Marcin Borkowski
2019-02-14 14:10 ` Stefan Monnier
@ 2019-02-15 14:39 ` Dmitry Gutov
2019-02-16 20:50 ` Jostein Kjønigsen
2019-03-27 8:03 ` Jackson Ray Hamilton
4 siblings, 0 replies; 33+ messages in thread
From: Dmitry Gutov @ 2019-02-15 14:39 UTC (permalink / raw)
To: Jackson Ray Hamilton, emacs-devel
Hi Jackson,
On 14.02.2019 08:06, Jackson Ray Hamilton wrote:
> Recently, I finally resolved to “do my laundry,” with respect to
> addressing the JSX indentation issues accumulated within debbugs and the
> js2-mode GitHub repo (where lots of js-mode bugs accidentally get
> filed). In the past week, I assembled some test cases, drafted some
> algorithms, and proceeded to implement a number of improvements to the
> indentation logic.
Thanks a lot for working on this.
On my part, I can say I'm excited about indentation improvements.
Creating jsx-specific variables that default to sgml- values sounds good.
Regarding js?-jsx- mode deprecations, I'm ambivalent. The current design
seems okay from my POV, that the thing Flycheck ends up having to do is
not the end of the world (there are several major modes in that list
already anyway). But if other people agree, all the best. I'd just
prefer not to turn it on by default, in the absence of any indications
that the current file is JSX.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-02-14 5:06 Comprehensive JSX support in Emacs Jackson Ray Hamilton
` (2 preceding siblings ...)
2019-02-15 14:39 ` Dmitry Gutov
@ 2019-02-16 20:50 ` Jostein Kjønigsen
2019-02-18 7:17 ` Jackson Ray Hamilton
2019-03-27 8:03 ` Jackson Ray Hamilton
4 siblings, 1 reply; 33+ messages in thread
From: Jostein Kjønigsen @ 2019-02-16 20:50 UTC (permalink / raw)
To: Jackson Ray Hamilton; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 11834 bytes --]
Hey there Jackson!
This sounds *great*!
As a maintainer of (third-party) Typescript-mode, I just wanted to shout out that these days React is often not just written in Javascript, but also in TypeScript. With TypeScript you can get much better in-editor code-intelligence than you can with regular Javascript, and support for this is provided by (third-party) package "tide" which I also co-maintain.
For both these packages a good JSX/TSX-configuration and good JSX/TSX-support is often requested.
If you go through with this endeavor, it would be nice if it could be done in a way which meant the TypeScript-world would have to fork/clone your efforts, but could rather build on it or extend it.
I'm not exactly sure *how* that could best be done, but I just thought I'd bring this particular angle to your attention in case it wasn't already on your radar.
--
Kind Regards
Jostein Kjønigsen
jostein@kjonigsen.net 🍵 jostein@gmail.com
https://jostein.kjønigsen.no <https://jostein.xn--kjnigsen-64a.no/>
On Thu, Feb 14, 2019, at 6:08 AM, Jackson Ray Hamilton wrote:
> Hello Emacs maintainers,
> A few years ago, I took up the challenge of implementing indentation support for the JavaScript extension “JSX” (by Facebook, for their “React” UI library), and we got my patches merged into js-mode. Now I think it’s time we reexamined and began to improve Emacs’ JSX+React support.
> My initial implementation was not too great, because it failed to indent JSX code properly in many common scenarios. My hueristics for finding HTML-like code inside a JS buffer were too conservative, due to a (perhaps excessive/misguided) attention to performance in particular areas of the code. Since my patch’s release, I’ve been watching from a distance as the bug reports have piled up.
> A package called “rjsx-mode” came onto the scene not too later, which seems to solve indentation problems using an AST (building on js2-mode’s AST); however, according to Steve Yegge’s post-mortem on js2-mode (http://steve-yegge.blogspot.com/2008/03/js2-mode-new-javascript-mode-for-emacs.html), relying on an AST being available after every keystroke is probably not optimal for performance, especially with large files. rjsx-mode also performs syntax highlighting (again, using its AST), which js-mode still does not do at all for JSX.
> Recently, I finally resolved to “do my laundry,” with respect to addressing the JSX indentation issues accumulated within debbugs and the js2-mode GitHub repo (where lots of js-mode bugs accidentally get filed). In the past week, I assembled some test cases, drafted some algorithms, and proceeded to implement a number of improvements to the indentation logic.
> As I started playing with the code again, many new ideas for improving JSX+React support began popping into my head, beyond just indentation improvements. I wanted to share those ideas here, and see if anyone wants to thumbs-up them before I go ahead and implement them (perhaps avoiding some bad ideas and/or rework).
> Also, I wanted to share my work-in-progress indentation reimplementation, and determine if I am not going completely in the wrong direction, and if an alternate plan I’ll also present for indentation is at all viable in comparison.
> Proposition: Comprehensive JSX support in Emacs
> First: Consider reworking obtuse APIs
> JSX indentation support is currently enabled by activating “js-jsx-mode”, using a function called “js-jsx-indent-line”, and “sgml-” variables control the indentation of the JSX code.
> I think introducing JSX support in the form of a derivative mode may have been a mistake. It wasn’t initially obvious to some people to use this mode. There was not an auto-mode-alist item for it, either. It also had the effect of creating a “matrix” of modes, where, for instance, Flycheck has to support all the different JavaScript modes like this:
> ` :modes (js-mode js-jsx-mode js2-mode js2-jsx-mode js3-mode rjsx-mode)`
> which seems messy to me.
> And what if we want to support more JavaScript syntax extensions, like Facebook’s “Flow,” and support the use of multiple extensions at once? We wouldn’t want to also have a js-flow-mode, js-jsx-flow-mode, js2-flow-mode, js2-jsx-flow-mode… therefore, I think it’d be better to start scaling back, and instead enable JSX, Flow, et al support with a variable.
> Also, upon reflection, I’m becoming more certain that controlling JavaScript indentation (even HTML-like indentation) with sgml- variables was unintuitive and therefore also a mistake. I think JSX should simply be indented using js-indent-level, perhaps optionally in combination with a js-jsx-attribute-offset, to complement the sgml-attribute-offset that we would otherwise be eliminating as an option. However, having added sgml-attribute-offset to Emacs myself merely as a precaution after a minor breaking change I made to SGML indentation a few years ago, I think we should let users actually demand JSX attribute indentation deltas, rather than assume they desire that level of granularity (considering we went so long without it in sgml-mode). Rather, users have indicated in bug reports that they’d prefer better *defaults* for the indentation (assuming indentation not matching “official” conventions is simply “erroneous;” and what a blessing to have conventions).
> I think JSX shoud “just work” in Emacs; i.e., with js-mode, whenever a user opens a file using JSX syntax, adapting to the user’s conventions for JS.
> Deprecation: Guide users to new, nicer APIs
> * Deprecate js-jsx-mode (and js2-jsx-mode downstream, and have rjsx-mode derive from js2-mode). Both modes will still exist, but will be marked obsolete, and will simply make a file-local copy of js-syntax-extensions (see below) with 'jsx added to the front of the list, instead of binding indentation like they currently do.
> * Deprecate js-jsx-indent-line, marking it obsolete. It will still exist, but it will simply call through to js-indent-line, with a copy of js-syntax-extensions let-bound, with 'jsx added to the front of the list.
> New features:
> * js-syntax-extensions: A list that will include symbols representing syntax extensions for which we will enable indentation/fontification.
> * In theory, if we supported more syntax extensions, those would go here as well; also, if any of the extensions happened to conflict with each other in some ways but not others, we could use the order of the list to determine the “precedence” of the extensions.
> * Having one variable for this gives us an opportunity to document all the supported syntaxes in one place, so more people will discover what’s all available.
> * Add a Customize interface for adding to this list like a set, picking from a set of options. (Not sure which widget would be best to use for that.)
> * js-indent-line will switch to a JSX indentation strategy when called while js-syntax-extensions contains 'jsx.
> * js-mode will automatically detect whether a file uses JSX syntax using some heuristic. Suggestions:
> * 'jsx is in js-syntax-extensions via init file, mode hook, or in .dir-locals.el
> * buffer-file-name has extension “.jsx” (some people use this)
> * “^\(var\|let\|const\|import\) React” matches within the first 1000 lines after any keystroke. (Under typical compiler configuration, “React” must be a JavaScript variable in scope in order for the compiled JSX to function properly, and most people import or require() stuff at the beginning of their files. React is usually used in combination with a JavaScript compiler, implying the user probably isn’t using React as a global variable, because compilers make that practice obsolete, so I expect this check will be pretty reliable)
> * Add auto-mode-alist item mapping “.jsx” files to js-mode, enabling the buffer-file-name heuristic.
> * As syntax extensions are enabled/disabled, update the mode name; ex:
> * No syntax extensions: "JavaScript"
> * js-syntax-extensions = (jsx): "JavaScript[JSX]"
> * js-syntax-extensions = (flow jsx): "JavaScript[Flow,JSX]"
> * Fill in current gaps in indentation support for JSX.
> * Add font locking for JSX syntax, similar if not the same as sgml-mode’s font locking. It is enabled when js-syntax-extensions contains 'jsx.
> Would appreciate feedback on these feature propositions; thanks!
> Indentation: WIP
> As I mentioned, I’ve already started hacking on the indentation. WIP patches are attached for preliminary review of approach (please don’t merge these yet, I haven’t vetted them thoroughly outside make test cases).
> I started by reworking my “JSX detection” code to use sgml-get-context to figure out if we were inside JSX, which was a lot more reliable than the previous heuristic (JSX pretty much always had to be wrapped in parens). I then proceeded to mostly solve Bug#24896 by distinguishing “<” and “>” in JS and JSX, so the sgml-mode indentation code could parse the code without tripping over arrow functions (“=>”), thinking they were part of HTML “<elements>”. I disambiguated the symbols in a syntax-propertize-function by doing a quick parse near every “<” and “>” char to determine if the surrounding code could only be syntactically valid as JSX, and not as JS. This seems to have fixed a lot of the failing cases. However, it still fails in a weird case where JSX appears nested inside JS, where that JS is also nested inside an outer JSX, a situation that just isn’t relevant in SGML, and thus the indenter doesn’t seem to support it.
> `// JS expressions should not break indentation`
> `// (https://github.com/mooz/js2-mode/issues/462).`
> `return (`
> ` <Router>`
> ` <Bar>`
> ` <Route exact path="/foo" render={() => (`
> ` <div>nothing</div>`
> ` )} />`
> ` <Route exact path="/bar" />`
> ` </Bar>`
> ` </Router>`
> `)`
> Maybe I could fix this recursive indentation issue by parsing the JSX further, and marking any “{”, “}” pairs therein with some syntax property that would get sgml-indent-line to treat it like some nested thing. However, that also might *not* work, and I don’t think I want to jump through any more hoops to make sgml-indent-line work inside js-mode.
> Also, since I’ve already started parsing JSX to disambiguate it from JS, I figure I may as well finish the parse (it’s pretty simple), since I could use that parse as an opportunity to mark characters for JSX-contextual font locking. And, since I’m adding these markings to buffer positions, I now have a great new way to figure out if I’m inside JSX when I’m indenting. So I’m thinking it may be time to graduate from sgml-indent-line delegation, and use the new data we now have to implement the JSX indentation logic in its entirety in js-mode. This way we could definitely support recursive JSX indentation, and also align all the indentation conventions with those used by the React community, and potentially maximize performance by avoiding a lot of checks that poor sgml-mode has to do for HTML’s loose and more complex semantics.
> Please let me know what you think of my current patches and the direction they’re going in, and if porting a simplified sgml-indent-line to js-mode is not too crazy of an idea!
> Thanks, Jackson
>
> *Attachments:*
> * 0001-Add-failing-tests-for-JSX-indentation-bugs.patch
> * 0002-Refactor-JSX-indentation-code-to-improve-enclosing-J.patch
> * 0003-Add-new-failing-unclosed-JSX-test-and-separate-such-.patch
> * 0004-js-syntax-propertize-Disambiguate-JS-from-JSX-fixing.patch
[-- Attachment #2: Type: text/html, Size: 13740 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-02-15 6:38 Jackson Ray Hamilton
@ 2019-02-17 6:10 ` Marcin Borkowski
0 siblings, 0 replies; 33+ messages in thread
From: Marcin Borkowski @ 2019-02-17 6:10 UTC (permalink / raw)
To: Jackson Ray Hamilton; +Cc: emacs-devel
On 2019-02-15, at 07:38, Jackson Ray Hamilton <jackson@jacksonrayhamilton.com> wrote:
> Hi Marcin,
>
> I’m glad to hear that you’re interested.
>
> Sorry for the TL;DR. Indentation seems like a complex topic, so
> I wanted to extract from my jumbled thoughts a sufficient amount of
> context to help anyone else along.
That's not a bad thing. It's just that it will have to wait yet a few
days more for when I have some more time.
Best,
--
Marcin Borkowski
http://mbork.pl
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-02-16 20:50 ` Jostein Kjønigsen
@ 2019-02-18 7:17 ` Jackson Ray Hamilton
0 siblings, 0 replies; 33+ messages in thread
From: Jackson Ray Hamilton @ 2019-02-18 7:17 UTC (permalink / raw)
To: jostein; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 2851 bytes --]
Hi Jostein,
Thank you for bringing this use case to my attention.
I haven’t worked with TypeScript before, but I took a look at the docs,
and read through the whole JSX page. It looks like the syntax for JSX
is exactly the same in TS as in JS, so my work may have a chance of
integrating with typescript-mode.
However, skimming through the rest of the docs, there could be
challenges dealing with code that uses angle brackets that look like
“tags,” e.g.:
|let list: Array<number> = [1, 2, 3];|
|let square = <Square>{};|
|let counter = <Counter>function (start: number) { };|
|let output = identity<string>("myString");|
|function loggingIdentity<T>(arg: Array<T>): Array<T> {
console.log(arg.length);return arg; }|
|function loggingIdentity<T extends Lengthwise>(arg: T): T {
console.log(arg.length);return arg; }|
||
|class GenericNumber<T> { zeroValue: T; add: (x: T, y: T) => T; }|
I understand that “type assertions” will be written with an |as| keyword
in TSX, but it looks like angle brackets are still used in other places
in TSX:
|class Component extends React.Component<PropsType, {}> { render() {
return ( <h2> {this.props.children} </h2> ) } }|
The parser I’ve written to disambiguate JSX from JS in js-mode makes the
assumption that matching angle brackets around identifiers indicate the
presence of JSX with certainty. In order to adapt that code for TS, one
solution could be to parse more, looking at the surrounding context of
the angle brackets to determine whether the thing is a TS type or if
it’s JSX. If you’re up for that challenge, hooks could be added to the
parser to perform additional context checks defined by your mode.
Disclaimer: Maybe “parsing more” won’t work, in which case a different
strategy might be necessary. Maybe it’s impossible to know the
difference between a TS type and JSX without an AST… I dunno. In any
case, as long as you can figure out what is JSX in TS, and the JSX is
marked with the same text properties that js-mode will use, then
integration with the rest of the JSX code (which indents and font-locks)
should be possible.
Since typescript-mode doesn’t derive from js-mode, I expect you’ll
manually pull in the JSX functions and variables from js-mode, and call
the functions and use the variables in similar ways, mirroring js-mode’s
ways. I expect you will do these things:
* Merge JSX font-lock keywords into TypeScript’s
* Use JSX syntax propertizing
* Delegate to Emacs’ JSX indentation inside JSX, and go back to
typescript-mode’s TS indenting inside JSX expressions
* Enable JSX features for “.tsx” files
I will try to keep the code modular to make it easy for you to pull in
the parts you need.
Jackson
[-- Attachment #2: Type: text/html, Size: 5624 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Comprehensive JSX support in Emacs
2019-02-14 5:06 Comprehensive JSX support in Emacs Jackson Ray Hamilton
` (3 preceding siblings ...)
2019-02-16 20:50 ` Jostein Kjønigsen
@ 2019-03-27 8:03 ` Jackson Ray Hamilton
2019-03-27 9:36 ` Marcin Borkowski
` (2 more replies)
4 siblings, 3 replies; 33+ messages in thread
From: Jackson Ray Hamilton @ 2019-03-27 8:03 UTC (permalink / raw)
To: jackson; +Cc: dgutov, monnier, emacs-devel
[-- Attachment #1.1: Type: text/plain, Size: 4604 bytes --]
Hello Emacs maintainers,
Following up on my long email from a month ago, wherein I announced my
plan to fully support editing JSX in js-mode…
After many mornings and evenings of hacking (and many moonlight-melted
candlesticks, and many moments spent meandering and muttering), I’ve
finally got the JavaScript+JSX implementation to a state where I feel
it’s worth sharing again.
I’ve added pretty comprehensive indentation and font-locking support,
along with relatively simple automatic JSX detection support.
The indentation and font-locking code complimented each other well. In
order to fix issues like the one with an unterminated quote inside JSX
(https://github.com/mooz/js2-mode/issues/409) (which truly haunted me
for the past years) I had to thoroughly parse the JSX code. The
information I gained from parsing eventually led to the resolution of
all the failing indentation test cases I compiled, and some new ones I
added along the way.
js-mode no longer relies on sgml-mode for any parsing or indentation.
It’s all performed precisely according to the semantics of JSX now,
borrowing just two blocks of code from sgml-mode as a basis for
overlapping indentation logic. Also, the JSX code has pretty colors
now. And, a single unterminated quote doesn’t wreak havoc on the buffer
any more.
I implemented pretty much all of the JSX detection logic I proposed, at
least in spirit. I immediately found that using a list called
“js-syntax-extensions” would be inferior to the alternative of using a
boolean called “js-jsx-syntax”. Booleans are simpler, and easier for
users to set given all the available methods, plus they’re fun to
check-off in Customize.
I mentioned that a “js-syntax-extensions” list might have some esoteric
advantage by its ordering resolving theoretical conflicts between
multiple syntax extensions, but I figure 1) we can cross that bridge
when we get there, and 2) if we did add more syntax extensions for JS
/and/ those syntax extensions did partially conflict /and/ users wanted
to use conflicting extensions simultaneously, perhaps it’d be better to
add some booleans that would resolve such conflicts on a case-by-case
basis, anyway.
Taking into consideration Dmitry’s ambivalence on deprecating
js-jsx-mode and js-jsx-indent-line, I figured we could strike a
compromise. We can keep these APIs without marking them obsolete.
However, I’ve tried to make the automatic detection of JSX—and the
viable alternatives to manually enabling JSX support—/so effective/ and
numerous as to render these functions superfluous relics. The
go-forward answer to “how to enable JSX support in Emacs?” should be
“upgrade to Emacs 27—now it just works™”.
I also addressed the spelling / naming items that Stefan brought up.
I had a chance to play with the code in a few projects of mine over the
past week, and generally it is working well for me—much better than
before, in all the previously painful cases. However, there is a
noticeable delay when editing some lines in my 1000-line monolith.jsx
file, and in some rare cases I am seeing font-locking not working for JS
embedded in JSX, and font-locking could probably be more graceful when
typing new JSX code, too. So there are still some improvements that can
be made, but they may be entering the territory of “maintenance.”
Therefore, we might be willing to unleash this beast sooner rather than
later, in spite of some imperfections.
Time for the good part: a whole bunch of patches, freshly rebased on
master (see attached). The first 4 patches should be the same as the
ones in my original “announcement” email (except for one merge conflict
I fixed), and the following 15 patches add up to some pretty kick-butt
“HTML-in-JavaScript” splendiferousness (IMHO).
(Note that the patch
“0015-Indent-broken-arrow-function-bodies-as-an-N-1th-arg.patch” isn’t
really related to the JSX feature. I was just editing some code in a
project of mine, and found that this change provided more desirable
behavior with the code I was writing then. A lot of my code using JSX
also uses arrow functions, so this was a convenient patch for me to lump
in.)
I invite people to provide feedback as I continue to battle-test and
optimize the code. Feedback regarding improving the reliability and
performance of parsing and font-locking would be especially appreciated.
Thanks,
Jackson
[-- Attachment #1.2: Type: text/html, Size: 5389 bytes --]
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-failing-tests-for-JSX-indentation-bugs.patch --]
[-- Type: text/x-patch; name="0001-Add-failing-tests-for-JSX-indentation-bugs.patch", Size: 6735 bytes --]
From 0ccfa924e21dd9db92fa9ecfff6f1cfb4821a73e Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Sat, 9 Feb 2019 15:42:42 -0800
Subject: [PATCH 01/19] Add failing tests for JSX indentation bugs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* test/manual/indent/js-jsx.js: Add failing tests for all the js-mode
and js2-mode JSX indentation bugs reported over the years that I could
find. Some may be duplicates, so I have grouped similar reports
together, for now; we’ll see for certain which distinct cases we need
once we start actually implementing fixes.
* test/manual/indent/js-jsx-quote.js: New file with a nasty test.
---
test/manual/indent/js-jsx-quote.js | 18 ++++
test/manual/indent/js-jsx.js | 183 +++++++++++++++++++++++++++++++++++++
2 files changed, 201 insertions(+)
create mode 100644 test/manual/indent/js-jsx-quote.js
diff --git a/test/manual/indent/js-jsx-quote.js b/test/manual/indent/js-jsx-quote.js
new file mode 100644
index 0000000000..4b71a65674
--- /dev/null
+++ b/test/manual/indent/js-jsx-quote.js
@@ -0,0 +1,18 @@
+// -*- mode: js-jsx; -*-
+
+// JSX text node values should be strings, but only JS string syntax
+// is considered, so quote marks delimit strings like normal, with
+// disastrous results (https://github.com/mooz/js2-mode/issues/409).
+function Bug() {
+ return <div>C'est Montréal</div>;
+}
+function Test(foo = /'/,
+ bar = 123) {}
+
+// This test is in a separate file because it can break other tests
+// when indenting the whole buffer (not sure why).
+
+// Local Variables:
+// indent-tabs-mode: nil
+// js-indent-level: 2
+// End:
diff --git a/test/manual/indent/js-jsx.js b/test/manual/indent/js-jsx.js
index 7401939d28..35ca4b275a 100644
--- a/test/manual/indent/js-jsx.js
+++ b/test/manual/indent/js-jsx.js
@@ -70,6 +70,189 @@ return (
</div>
);
+// Indent void expressions (no need for contextual parens / commas)
+// (https://github.com/mooz/js2-mode/issues/140#issuecomment-166250016).
+<div className="class-name">
+ <h2>Title</h2>
+ {array.map(() => {
+ return <Element />;
+ })}
+ {message}
+</div>
+// Another example of above issue
+// (https://github.com/mooz/js2-mode/issues/490).
+<App>
+ <div>
+ {variable1}
+ <Component/>
+ </div>
+</App>
+
+// Comments and arrows can break indentation (Bug#24896 /
+// https://github.com/mooz/js2-mode/issues/389).
+const Component = props => (
+ <FatArrow a={e => c}
+ b={123}>
+ </FatArrow>
+);
+const Component = props => (
+ <NoFatArrow a={123}
+ b={123}>
+ </NoFatArrow>
+);
+const Component = props => ( // Parse this comment, please.
+ <FatArrow a={e => c}
+ b={123}>
+ </FatArrow>
+);
+const Component = props => ( // Parse this comment, please.
+ <NoFatArrow a={123}
+ b={123}>
+ </NoFatArrow>
+);
+// Another example of above issue (Bug#30225).
+class {
+ render() {
+ return (
+ <select style={{paddingRight: "10px"}}
+ onChange={e => this.setState({value: e.target.value})}
+ value={this.state.value}>
+ <option>Hi</option>
+ </select>
+ );
+ }
+}
+
+// JSX attributes of an arrow function’s expression body’s JSX
+// expression should be indented with respect to the JSX opening
+// element (Bug#26001 /
+// https://github.com/mooz/js2-mode/issues/389#issuecomment-271869380).
+class {
+ render() {
+ const messages = this.state.messages.map(
+ message => <Message key={message.id}
+ text={message.text}
+ mine={message.mine} />
+ ); return messages;
+ }
+ render() {
+ const messages = this.state.messages.map(message =>
+ <Message key={message.timestamp}
+ text={message.text}
+ mine={message.mine} />
+ ); return messages;
+ }
+}
+
+// Users expect tag closers to align with the tag’s start; this is the
+// style used in the React docs, so it should be the default.
+// - https://github.com/mooz/js2-mode/issues/389#issuecomment-390766873
+// - https://github.com/mooz/js2-mode/issues/482
+// - Bug#32158
+const foo = (props) => (
+ <div>
+ <input
+ cat={i => i}
+ />
+ <button
+ className="square"
+ >
+ {this.state.value}
+ </button>
+ </div>
+);
+
+// Embedded JSX in parens breaks indentation
+// (https://github.com/mooz/js2-mode/issues/411).
+let a = (
+ <div>
+ {condition && <Component/>}
+ {condition && <Component/>}
+ <div/>
+ </div>
+)
+let b = (
+ <div>
+ {condition && (<Component/>)}
+ <div/>
+ </div>
+)
+let c = (
+ <div>
+ {condition && (<Component/>)}
+ {condition && "something"}
+ </div>
+)
+let d = (
+ <div>
+ {(<Component/>)}
+ {condition && "something"}
+ </div>
+)
+// Another example of the above issue (Bug#27000).
+function testA() {
+ return (
+ <div>
+ <div> { ( <div/> ) } </div>
+ </div>
+ );
+}
+function testB() {
+ return (
+ <div>
+ <div> { <div/> } </div>
+ </div>
+ );
+}
+// Another example of the above issue
+// (https://github.com/mooz/js2-mode/issues/451).
+class Classy extends React.Component {
+ render () {
+ return (
+ <div>
+ <ul className="tocListRoot">
+ { this.state.list.map((item) => {
+ return (<div />)
+ })}
+ </ul>
+ </div>
+ )
+ }
+}
+
+// Self-closing tags should be indented properly
+// (https://github.com/mooz/js2-mode/issues/459).
+export default ({ stars }) => (
+ <div className='overlay__container'>
+ <div className='overlay__header overlay--text'>
+ Congratulations!
+ </div>
+ <div className='overlay__reward'>
+ <Icon {...createIconProps(stars > 0)} size='large' />
+ <div className='overlay__reward__bottom'>
+ <Icon {...createIconProps(stars > 1)} size='small' />
+ <Icon {...createIconProps(stars > 2)} size='small' />
+ </div>
+ </div>
+ <div className='overlay__description overlay--text'>
+ You have created <large>1</large> reminder
+ </div>
+ </div>
+)
+
+// JS expressions should not break indentation
+// (https://github.com/mooz/js2-mode/issues/462).
+return (
+ <Router>
+ <Bar>
+ <Route exact path="/foo" render={() => (
+ <div>nothing</div>
+ )} />
+ <Route exact path="/bar" />
+ </Bar>
+ </Router>
+)
+
// Local Variables:
// indent-tabs-mode: nil
// js-indent-level: 2
--
2.11.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Refactor-JSX-indentation-code-to-improve-enclosing-J.patch --]
[-- Type: text/x-patch; name="0002-Refactor-JSX-indentation-code-to-improve-enclosing-J.patch", Size: 20090 bytes --]
From 1d404e0c9256b6e18ca4ac9e8e663a611ab7e256 Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Sat, 9 Feb 2019 20:06:29 -0800
Subject: [PATCH 02/19] Refactor JSX indentation code to improve enclosing JSX
discovery
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Fix a number of bugs reported for JSX indentation (caused by poor JSX
detection):
- https://github.com/mooz/js2-mode/issues/140#issuecomment-166250016
- https://github.com/mooz/js2-mode/issues/490
- Bug#24896 / https://github.com/mooz/js2-mode/issues/389 (with
respect to comments)
- Bug#26001 /
https://github.com/mooz/js2-mode/issues/389#issuecomment-271869380
- https://github.com/mooz/js2-mode/issues/411 / Bug#27000 /
https://github.com/mooz/js2-mode/issues/451
Potentially manifest some new bugs (due to false positives with ‘<’
and ‘>’ and SGML detection). Slow down indentation a fair bit.
* list/progmodes/js.el (js-jsx-syntax, js--jsx-start-tag-re)
(js--looking-at-jsx-start-tag-p, js--looking-back-at-jsx-end-tag-p):
New variables and functions.
(js--jsx-find-before-tag, js--jsx-after-tag-re): Deleted.
(js--looking-at-operator-p): Don’t mistake a JSXOpeningElement for the
‘<’ operator.
(js--continued-expression-p): Don’t mistake a JSXClosingElement as a
fragment of a continued expression including the ‘>’ operator.
(js--as-sgml): Simplify. Probably needn’t bind forward-sexp-function
to nil (sgml-mode already does) and probably shouldn’t bind
parse-sexp-lookup-properties to nil either (see Bug#24896).
(js--outermost-enclosing-jsx-tag-pos): Find enclosing JSX more
accurately than js--jsx-find-before-tag. Use sgml-mode’s parsing
logic, rather than unreliable heuristics like paren-wrapping. This
implementation is much slower; the previous implementation was fast,
but at the expense of accuracy. To make up for all the grief we’ve
caused users, we will prefer accuracy over speed from now on. That
said, this can still probably be optimized a lot.
(js--jsx-indented-element-p): Rename to js--jsx-indentation, since it
doesn’t just return a boolean.
(js--jsx-indentation): Refactor js--jsx-indented-element-p to simplify
the implementation as the improved accuracy of other code allows (and
to repent for some awful stylistic choices I made earlier).
(js--expression-in-sgml-indent-line): Rename to
js--indent-line-in-jsx-expression, since it’s a private function and
we can give it a name that reads more like English.
(js--indent-line-in-jsx-expression): Restructure point adjustment
logic more like js-indent-line.
(js--indent-n+1th-jsx-line): New function to complement
js--indent-line-in-jsx-expression.
(js-jsx-indent-line): Refactor. Don’t bind js--continued-expression-p
to ignore any more; instead, rely on the improved accuracy of
js--continued-expression-p.
(js-jsx-mode): Set js-jsx-syntax to t. For now, this will be the flag
we use to determine whether ‘JSX is enabled.’ (Maybe later, we will
refactor the code to use this variable instead of requiring
js-jsx-mode to be enabled, thus rendering the mode obsolete.)
---
lisp/progmodes/js.el | 337 +++++++++++++++++++++------------------------------
1 file changed, 141 insertions(+), 196 deletions(-)
diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 4d91da7334..5b992535a8 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -572,6 +572,15 @@ js-chain-indent
:safe 'booleanp
:group 'js)
+(defcustom js-jsx-syntax nil
+ "When non-nil, parse JavaScript with consideration for JSX syntax.
+This fixes indentation of JSX code in some cases. It is set to
+be buffer-local when in `js-jsx-mode'."
+ :version "27.1"
+ :type 'boolean
+ :safe 'booleanp
+ :group 'js)
+
;;; KeyMap
(defvar js-mode-map
@@ -1774,6 +1783,14 @@ js--indent-operator-re
(js--regexp-opt-symbol '("in" "instanceof")))
"Regexp matching operators that affect indentation of continued expressions.")
+(defconst js--jsx-start-tag-re
+ (concat "<" sgml-name-re)
+ "Regexp matching code that looks like a JSXOpeningElement.")
+
+(defun js--looking-at-jsx-start-tag-p ()
+ "Non-nil if a JSXOpeningElement immediately follows point."
+ (looking-at js--jsx-start-tag-re))
+
(defun js--looking-at-operator-p ()
"Return non-nil if point is on a JavaScript operator, other than a comma."
(save-match-data
@@ -1796,7 +1813,9 @@ js--looking-at-operator-p
(js--backward-syntactic-ws)
;; We might misindent some expressions that would
;; return NaN anyway. Shouldn't be a problem.
- (memq (char-before) '(?, ?} ?{))))))))
+ (memq (char-before) '(?, ?} ?{)))))
+ ;; “<” isn’t necessarily an operator in JSX.
+ (not (and js-jsx-syntax (js--looking-at-jsx-start-tag-p))))))
(defun js--find-newline-backward ()
"Move backward to the nearest newline that is not in a block comment."
@@ -1816,6 +1835,14 @@ js--find-newline-backward
(setq result nil)))
result))
+(defconst js--jsx-end-tag-re
+ (concat "</" sgml-name-re ">\\|/>")
+ "Regexp matching a JSXClosingElement.")
+
+(defun js--looking-back-at-jsx-end-tag-p ()
+ "Non-nil if a JSXClosingElement immediately precedes point."
+ (looking-back js--jsx-end-tag-re (point-at-bol)))
+
(defun js--continued-expression-p ()
"Return non-nil if the current line continues an expression."
(save-excursion
@@ -1833,12 +1860,19 @@ js--continued-expression-p
(and (js--find-newline-backward)
(progn
(skip-chars-backward " \t")
- (or (bobp) (backward-char))
- (and (> (point) (point-min))
- (save-excursion (backward-char) (not (looking-at "[/*]/\\|=>")))
- (js--looking-at-operator-p)
- (and (progn (backward-char)
- (not (looking-at "\\+\\+\\|--\\|/[/*]"))))))))))
+ (and
+ ;; The “>” at the end of any JSXBoundaryElement isn’t
+ ;; part of a continued expression.
+ (not (and js-jsx-syntax (js--looking-back-at-jsx-end-tag-p)))
+ (progn
+ (or (bobp) (backward-char))
+ (and (> (point) (point-min))
+ (save-excursion
+ (backward-char)
+ (not (looking-at "[/*]/\\|=>")))
+ (js--looking-at-operator-p)
+ (and (progn (backward-char)
+ (not (looking-at "\\+\\+\\|--\\|/[/*]"))))))))))))
(defun js--skip-term-backward ()
"Skip a term before point; return t if a term was skipped."
@@ -2153,190 +2187,108 @@ js--proper-indentation
;;; 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 ()
+(defmacro js--as-sgml (&rest body)
+ "Execute BODY as if in sgml-mode."
+ `(with-syntax-table sgml-mode-syntax-table
+ ,@body))
+
+(defun js--outermost-enclosing-jsx-tag-pos ()
+ (let (context tag-pos last-tag-pos parse-status parens paren-pos curly-pos)
+ (js--as-sgml
+ ;; Search until we reach the top or encounter the start of a
+ ;; JSXExpressionContainer (implying nested JSX).
+ (while (and (setq context (sgml-get-context))
+ (progn
+ (setq tag-pos (sgml-tag-start (car (last context))))
+ (or (not curly-pos)
+ ;; Stop before curly brackets (start of a
+ ;; JSXExpressionContainer).
+ (> tag-pos curly-pos))))
+ ;; Record this position so it can potentially be returned.
+ (setq last-tag-pos tag-pos)
+ ;; Always parse sexps / search for the next context from the
+ ;; immediately enclosing tag (sgml-get-context may not leave
+ ;; point there).
+ (goto-char tag-pos)
+ (unless parse-status ; Don’t needlessly reparse.
+ ;; Search upward for an enclosing starting curly bracket.
+ (setq parse-status (syntax-ppss))
+ (setq parens (reverse (nth 9 parse-status)))
+ (while (and (setq paren-pos (car parens))
+ (not (when (= (char-after paren-pos) ?{)
+ (setq curly-pos paren-pos))))
+ (setq parens (cdr parens)))
+ ;; Always search for the next context from the immediately
+ ;; enclosing tag (calling syntax-ppss in the above loop
+ ;; may move point from there).
+ (goto-char tag-pos))))
+ last-tag-pos))
+
+(defun js--jsx-indentation ()
"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')
- );"
+Return nil for first JSXElement line (indent like JS).
+Return `n+1th' for second+ JSXElement lines (indent like SGML).
+Return `expression' for lines within embedded JS expressions
+ (indent like JS inside SGML).
+Return nil for non-JSX lines."
(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)
+ tag-start-pos 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
+ ;; Determine if inside a JSXElement.
+ (beginning-of-line) ; For exclusivity
+ (when (setq tag-start-pos (js--outermost-enclosing-jsx-tag-pos))
+ ;; Check if inside an embedded multi-line JS expression.
+ (goto-char current-pos)
+ (end-of-line) ; For exclusivity
+ (setq parens (nth 9 (syntax-ppss)))
+ (while
+ (and
+ (setq paren (car parens))
+ (if (and
+ (>= paren tag-start-pos)
+ ;; A curly bracket indicates the start of an
+ ;; embedded expression.
+ (= (char-after paren) ?{)
+ ;; 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.
+ ;; (exclusive, as the closing bracket is indented
+ ;; like SGML).
+ (if (progn
+ (goto-char paren)
+ (ignore-errors (let (forward-sexp-function)
+ (forward-sexp))))
+ (< current-line (line-number-at-pos))
+ ;; No matching bracket implies we’re inside!
+ t))
+ ;; Indicate this will be indented specially. Return
+ ;; nil to stop iterating too.
+ (progn (setq type 'expression) nil)
+ ;; Stop iterating when parens = nil.
+ (setq parens (cdr parens)))))
+ (or type 'n+1th)))))
+
+(defun js--indent-line-in-jsx-expression ()
+ "Indent the current line as JavaScript within JSX."
+ (let ((parse-status (save-excursion (syntax-ppss (point-at-bol))))
+ offset indent-col)
(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))))))
+ (save-excursion
+ (setq offset (- (point) (progn (back-to-indentation) (point)))
+ indent-col (js--as-sgml (sgml-calculate-indent))))
+ (if (null indent-col) 'noindent ; Like in sgml-mode
+ ;; Use whichever indentation column is greater, such that the
+ ;; SGML column is effectively a minimum.
+ (indent-line-to (max (js--proper-indentation parse-status)
+ (+ indent-col js-indent-level)))
+ (when (> offset 0) (forward-char offset))))))
+
+(defun js--indent-n+1th-jsx-line ()
+ "Indent the current line as JSX within JavaScript."
+ (js--as-sgml (sgml-indent-line)))
(defun js-indent-line ()
"Indent the current line as JavaScript."
@@ -2353,19 +2305,11 @@ js-jsx-indent-line
i.e., customize JSX element indentation with `sgml-basic-offset',
`sgml-attribute-offset' et al."
(interactive)
- (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)))
- ((eq indentation-type 'nth)
- (js--as-sgml (sgml-indent-line)))
- (t (js-indent-line)))))
+ (let ((type (js--jsx-indentation)))
+ (if type
+ (if (eq type 'n+1th) (js--indent-n+1th-jsx-line)
+ (js--indent-line-in-jsx-expression))
+ (js-indent-line))))
;;; Filling
@@ -3944,6 +3888,7 @@ js-jsx-mode
(setq-local sgml-basic-offset js-indent-level))
(add-hook \\='js-jsx-mode-hook #\\='set-jsx-indentation)"
:group 'js
+ (setq-local js-jsx-syntax t)
(setq-local indent-line-function #'js-jsx-indent-line))
;;;###autoload (defalias 'javascript-mode 'js-mode)
--
2.11.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-Add-new-failing-unclosed-JSX-test-and-separate-such-.patch --]
[-- Type: text/x-patch; name="0003-Add-new-failing-unclosed-JSX-test-and-separate-such-.patch", Size: 2675 bytes --]
From 2b062aef181ea2c5ed61c7e1e8a40d8a43925f2a Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Sun, 10 Feb 2019 21:11:17 -0800
Subject: [PATCH 03/19] Add new (failing) unclosed JSX test and separate such
tests
* test/manual/indent/js-jsx.js: Move test with intentional scan error to
its own file, js-jsx-unclosed-1.js.
* test/manual/indent/js-jsx-unclosed-1.js: New file.
* test/manual/indent/js-jsx-unclosed-2.js: New file with test for
regression caused by new ambiguous parsing of JS/JSX.
---
test/manual/indent/js-jsx-unclosed-1.js | 15 +++++++++++++++
test/manual/indent/js-jsx-unclosed-2.js | 17 +++++++++++++++++
test/manual/indent/js-jsx.js | 9 ---------
3 files changed, 32 insertions(+), 9 deletions(-)
create mode 100644 test/manual/indent/js-jsx-unclosed-1.js
create mode 100644 test/manual/indent/js-jsx-unclosed-2.js
diff --git a/test/manual/indent/js-jsx-unclosed-1.js b/test/manual/indent/js-jsx-unclosed-1.js
new file mode 100644
index 0000000000..9418aed7a1
--- /dev/null
+++ b/test/manual/indent/js-jsx-unclosed-1.js
@@ -0,0 +1,15 @@
+// -*- mode: js-jsx; -*-
+
+// Local Variables:
+// indent-tabs-mode: nil
+// js-indent-level: 2
+// End:
+
+// The following test goes below any comments to avoid including
+// misindented comments among the erroring lines.
+
+return (
+ <div>
+ {array.map(function () {
+ return {
+ a: 1
diff --git a/test/manual/indent/js-jsx-unclosed-2.js b/test/manual/indent/js-jsx-unclosed-2.js
new file mode 100644
index 0000000000..2d42cf70f8
--- /dev/null
+++ b/test/manual/indent/js-jsx-unclosed-2.js
@@ -0,0 +1,17 @@
+// -*- mode: js-jsx; -*-
+
+// Local Variables:
+// indent-tabs-mode: nil
+// js-indent-level: 2
+// End:
+
+// The following tests go below any comments to avoid including
+// misindented comments among the erroring lines.
+
+// Don’t misinterpret equality operators as JSX.
+for (; i < length;) void 0
+if (foo > bar) void 0
+
+// Don’t even misinterpret unary operators as JSX.
+if (foo < await bar) void 0
+while (await foo > bar) void 0
diff --git a/test/manual/indent/js-jsx.js b/test/manual/indent/js-jsx.js
index 35ca4b275a..af3c340559 100644
--- a/test/manual/indent/js-jsx.js
+++ b/test/manual/indent/js-jsx.js
@@ -257,12 +257,3 @@ return (
// indent-tabs-mode: nil
// js-indent-level: 2
// 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
--
2.11.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0004-js-syntax-propertize-Disambiguate-JS-from-JSX-fixing.patch --]
[-- Type: text/x-patch; name="0004-js-syntax-propertize-Disambiguate-JS-from-JSX-fixing.patch", Size: 7734 bytes --]
From 1b5df3e4dccd60f76fa42d626a898eba09224088 Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Mon, 11 Feb 2019 03:00:34 -0800
Subject: [PATCH 04/19] js-syntax-propertize: Disambiguate JS from JSX, fixing
some indents
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Fix some JSX indentation bugs:
- Bug#24896 / https://github.com/mooz/js2-mode/issues/389
- Bug#30225
- https://github.com/mooz/js2-mode/issues/459
* lisp/progmodes/js.el (js--dotted-captured-name-re)
(js--unary-keyword-re, js--unary-keyword-p)
(js--disambiguate-beginning-of-jsx-tag)
(js--disambiguate-end-of-jsx-tag)
(js--disambiguate-js-from-jsx): New variables and functions.
(js-syntax-propertize): Additionally clarify when syntax is JS so that
‘(with-syntax-table sgml-mode-syntax-table …)’ does not mistake some
JS punctuation syntax for SGML parenthesis syntax, namely ‘<’ and ‘>’.
* test/manual/indent/js-jsx-unclosed-2.js: Add additional test for
unary operator parsing.
---
lisp/progmodes/js.el | 100 +++++++++++++++++++++++++++++++-
test/manual/indent/js-jsx-unclosed-2.js | 14 +++++
2 files changed, 113 insertions(+), 1 deletion(-)
diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 5b992535a8..d0556f3538 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -82,6 +82,10 @@ js--dotted-name-re
(concat js--name-re "\\(?:\\." js--name-re "\\)*")
"Regexp matching a dot-separated sequence of JavaScript names.")
+(defconst js--dotted-captured-name-re
+ (concat "\\(" js--name-re "\\)\\(?:\\." js--name-re "\\)*")
+ "Like `js--dotted-name-re', but capture the first name.")
+
(defconst js--cpp-name-re js--name-re
"Regexp matching a C preprocessor name.")
@@ -1731,6 +1735,99 @@ js-syntax-propertize-regexp
'syntax-table (string-to-syntax "\"/"))
(goto-char end)))))
+(defconst js--unary-keyword-re
+ (js--regexp-opt-symbol '("await" "delete" "typeof" "void" "yield"))
+ "Regexp matching unary operator keywords.")
+
+(defun js--unary-keyword-p (string)
+ "Check if STRING is a unary operator keyword in JavaScript."
+ (string-match-p js--unary-keyword-re string))
+
+(defun js--disambiguate-beginning-of-jsx-tag ()
+ "Parse enough to determine if a JSX tag starts here.
+Disambiguate JSX from equality operators by testing for syntax
+only valid as JSX."
+ ;; “</…” - a JSXClosingElement.
+ ;; “<>” - a JSXOpeningFragment.
+ (if (memq (char-after) '(?\/ ?\>)) t
+ (save-excursion
+ (skip-chars-forward " \t\n")
+ (and
+ (looking-at js--dotted-captured-name-re)
+ ;; Don’t match code like “if (i < await foo)”
+ (not (js--unary-keyword-p (match-string 1)))
+ (progn
+ (goto-char (match-end 0))
+ (skip-chars-forward " \t\n")
+ (or
+ ;; “>”, “/>” - tag enders.
+ ;; “{” - a JSXExpressionContainer.
+ (memq (char-after) '(?\> ?\/ ?\{))
+ ;; Check if a JSXAttribute follows.
+ (looking-at js--name-start-re)))))))
+
+(defun js--disambiguate-end-of-jsx-tag ()
+ "Parse enough to determine if a JSX tag ends here.
+Disambiguate JSX from equality operators by testing for syntax
+only valid as JSX, or extremely unlikely except as JSX."
+ (save-excursion
+ (backward-char)
+ ;; “…/>” - a self-closing JSXOpeningElement.
+ ;; “</>” - a JSXClosingFragment.
+ (if (= (char-before) ?/) t
+ (let (last-tag-or-attr-name last-non-unary-p)
+ (catch 'match
+ (while t
+ (skip-chars-backward " \t\n")
+ ;; Check if the end of a JSXAttribute value or
+ ;; JSXExpressionContainer almost certainly precedes.
+ ;; The only valid JS this misses is
+ ;; - {} > foo
+ ;; - "bar" > foo
+ ;; which is no great loss, IMHO…
+ (if (memq (char-before) '(?\} ?\" ?\' ?\`)) (throw 'match t)
+ (if (and last-tag-or-attr-name last-non-unary-p
+ ;; “<”, “</” - tag starters.
+ (memq (char-before) '(?\< ?\/)))
+ ;; Leftmost name parsed was the name of a
+ ;; JSXOpeningElement.
+ (throw 'match t))
+ ;; Technically the dotted name could span multiple
+ ;; lines, but dear God WHY?! Also, match greedily to
+ ;; ensure the entire name is valid.
+ (if (looking-back js--dotted-captured-name-re (point-at-bol) t)
+ (if (and (setq last-non-unary-p (not (js--unary-keyword-p (match-string 1))))
+ last-tag-or-attr-name)
+ ;; Valid (non-unary) name followed rightwards by
+ ;; another name (any will do, including
+ ;; keywords) is invalid JS, but valid JSX.
+ (throw 'match t)
+ ;; Remember match and skip backwards over it when
+ ;; it is the first matched name or the N+1th
+ ;; matched unary name (unary names on the left are
+ ;; still ambiguously JS or JSX, so keep parsing to
+ ;; disambiguate).
+ (setq last-tag-or-attr-name (match-string 1))
+ (goto-char (match-beginning 0)))
+ ;; Nothing else to look for; give up parsing.
+ (throw 'match nil)))))))))
+
+(defun js--disambiguate-js-from-jsx (start end)
+ "Figure out which ‘<’ and ‘>’ chars (from START to END) aren’t JSX.
+
+Later, this info prevents ‘sgml-’ functions from treating some
+‘<’ and ‘>’ chars as parts of tokens of SGML tags — a good thing,
+since they are serving their usual function as some JS equality
+operator or arrow function, instead."
+ (goto-char start)
+ (while (re-search-forward "[<>]" end t)
+ (unless (if (eq (char-before) ?<) (js--disambiguate-beginning-of-jsx-tag)
+ (js--disambiguate-end-of-jsx-tag))
+ ;; Inform sgml- functions that this >, >=, >>>, <, <=, <<<, or
+ ;; => token is punctuation (and not an open or close parenthesis
+ ;; as per usual in sgml-mode).
+ (put-text-property (1- (point)) (point) 'syntax-table '(1)))))
+
(defun js-syntax-propertize (start end)
;; JavaScript allows immediate regular expression objects, written /.../.
(goto-char start)
@@ -1758,7 +1855,8 @@ js-syntax-propertize
'syntax-table (string-to-syntax "\"/"))
(js-syntax-propertize-regexp end)))))
("\\`\\(#\\)!" (1 "< b")))
- (point) end))
+ (point) end)
+ (if js-jsx-syntax (js--disambiguate-js-from-jsx start end)))
(defconst js--prettify-symbols-alist
'(("=>" . ?⇒)
diff --git a/test/manual/indent/js-jsx-unclosed-2.js b/test/manual/indent/js-jsx-unclosed-2.js
index 2d42cf70f8..8b6f33325d 100644
--- a/test/manual/indent/js-jsx-unclosed-2.js
+++ b/test/manual/indent/js-jsx-unclosed-2.js
@@ -15,3 +15,17 @@ if (foo > bar) void 0
// Don’t even misinterpret unary operators as JSX.
if (foo < await bar) void 0
while (await foo > bar) void 0
+
+// Allow unary keyword names as null-valued JSX attributes.
+// (As if this will EVER happen…)
+<Foo yield>
+ <Bar void>
+ <Baz
+ zorp
+ typeof>
+ <Please do_n0t delete this_stupidTest >
+ How would we ever live without unary support
+ </Please>
+ </Baz>
+ </Bar>
+</Foo>
--
2.11.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #6: 0005-Use-js-jsx-prefix-for-functions-and-variables.patch --]
[-- Type: text/x-patch; name="0005-Use-js-jsx-prefix-for-functions-and-variables.patch", Size: 9145 bytes --]
From 537ccf9bdfddd6e0e60f4eb55ce8b5f748f1cee8 Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Fri, 15 Feb 2019 22:15:11 -0800
Subject: [PATCH 05/19] Use js-jsx- prefix for functions and variables
* lisp/progmodes/js.el (js--disambiguate-beginning-of-jsx-tag): Rename
to js-jsx--disambiguate-beginning-of-tag.
(js--disambiguate-end-of-jsx-tag): Rename to
js-jsx--disambiguate-end-of-tag.
(js--disambiguate-js-from-jsx): Rename to js-jsx--disambiguate-syntax.
(js--jsx-start-tag-re): Rename to js-jsx--start-tag-re.
(js--looking-at-jsx-start-tag-p): Rename to
js-jsx--looking-at-start-tag-p.
(js--jsx-end-tag-re): Rename to js-jsx--end-tag-re.
(js--looking-back-at-jsx-end-tag-p): Rename to
js-jsx--looking-back-at-end-tag-p.
(js--as-sgml): Rename to js-jsx--as-sgml.
(js--outermost-enclosing-jsx-tag-pos): Rename to
js-jsx--outermost-enclosing-tag-pos.
(js--jsx-indentation): Rename to js-jsx--indentation-type.
(js--indent-line-in-jsx-expression): Rename to
js-jsx--indent-line-in-expression.
(js--indent-n+1th-jsx-line): Rename to js-jsx--indent-n+1th-line.
---
lisp/progmodes/js.el | 52 ++++++++++++++++++++++++++--------------------------
1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index d0556f3538..4404ea04a0 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -1743,7 +1743,7 @@ js--unary-keyword-p
"Check if STRING is a unary operator keyword in JavaScript."
(string-match-p js--unary-keyword-re string))
-(defun js--disambiguate-beginning-of-jsx-tag ()
+(defun js-jsx--disambiguate-beginning-of-tag ()
"Parse enough to determine if a JSX tag starts here.
Disambiguate JSX from equality operators by testing for syntax
only valid as JSX."
@@ -1766,7 +1766,7 @@ js--disambiguate-beginning-of-jsx-tag
;; Check if a JSXAttribute follows.
(looking-at js--name-start-re)))))))
-(defun js--disambiguate-end-of-jsx-tag ()
+(defun js-jsx--disambiguate-end-of-tag ()
"Parse enough to determine if a JSX tag ends here.
Disambiguate JSX from equality operators by testing for syntax
only valid as JSX, or extremely unlikely except as JSX."
@@ -1812,7 +1812,7 @@ js--disambiguate-end-of-jsx-tag
;; Nothing else to look for; give up parsing.
(throw 'match nil)))))))))
-(defun js--disambiguate-js-from-jsx (start end)
+(defun js-jsx--disambiguate-syntax (start end)
"Figure out which ‘<’ and ‘>’ chars (from START to END) aren’t JSX.
Later, this info prevents ‘sgml-’ functions from treating some
@@ -1821,8 +1821,8 @@ js--disambiguate-js-from-jsx
operator or arrow function, instead."
(goto-char start)
(while (re-search-forward "[<>]" end t)
- (unless (if (eq (char-before) ?<) (js--disambiguate-beginning-of-jsx-tag)
- (js--disambiguate-end-of-jsx-tag))
+ (unless (if (eq (char-before) ?<) (js-jsx--disambiguate-beginning-of-tag)
+ (js-jsx--disambiguate-end-of-tag))
;; Inform sgml- functions that this >, >=, >>>, <, <=, <<<, or
;; => token is punctuation (and not an open or close parenthesis
;; as per usual in sgml-mode).
@@ -1856,7 +1856,7 @@ js-syntax-propertize
(js-syntax-propertize-regexp end)))))
("\\`\\(#\\)!" (1 "< b")))
(point) end)
- (if js-jsx-syntax (js--disambiguate-js-from-jsx start end)))
+ (if js-jsx-syntax (js-jsx--disambiguate-syntax start end)))
(defconst js--prettify-symbols-alist
'(("=>" . ?⇒)
@@ -1881,13 +1881,13 @@ js--indent-operator-re
(js--regexp-opt-symbol '("in" "instanceof")))
"Regexp matching operators that affect indentation of continued expressions.")
-(defconst js--jsx-start-tag-re
+(defconst js-jsx--start-tag-re
(concat "<" sgml-name-re)
"Regexp matching code that looks like a JSXOpeningElement.")
-(defun js--looking-at-jsx-start-tag-p ()
+(defun js-jsx--looking-at-start-tag-p ()
"Non-nil if a JSXOpeningElement immediately follows point."
- (looking-at js--jsx-start-tag-re))
+ (looking-at js-jsx--start-tag-re))
(defun js--looking-at-operator-p ()
"Return non-nil if point is on a JavaScript operator, other than a comma."
@@ -1913,7 +1913,7 @@ js--looking-at-operator-p
;; return NaN anyway. Shouldn't be a problem.
(memq (char-before) '(?, ?} ?{)))))
;; “<” isn’t necessarily an operator in JSX.
- (not (and js-jsx-syntax (js--looking-at-jsx-start-tag-p))))))
+ (not (and js-jsx-syntax (js-jsx--looking-at-start-tag-p))))))
(defun js--find-newline-backward ()
"Move backward to the nearest newline that is not in a block comment."
@@ -1933,13 +1933,13 @@ js--find-newline-backward
(setq result nil)))
result))
-(defconst js--jsx-end-tag-re
+(defconst js-jsx--end-tag-re
(concat "</" sgml-name-re ">\\|/>")
"Regexp matching a JSXClosingElement.")
-(defun js--looking-back-at-jsx-end-tag-p ()
+(defun js-jsx--looking-back-at-end-tag-p ()
"Non-nil if a JSXClosingElement immediately precedes point."
- (looking-back js--jsx-end-tag-re (point-at-bol)))
+ (looking-back js-jsx--end-tag-re (point-at-bol)))
(defun js--continued-expression-p ()
"Return non-nil if the current line continues an expression."
@@ -1961,7 +1961,7 @@ js--continued-expression-p
(and
;; The “>” at the end of any JSXBoundaryElement isn’t
;; part of a continued expression.
- (not (and js-jsx-syntax (js--looking-back-at-jsx-end-tag-p)))
+ (not (and js-jsx-syntax (js-jsx--looking-back-at-end-tag-p)))
(progn
(or (bobp) (backward-char))
(and (> (point) (point-min))
@@ -2285,14 +2285,14 @@ js--proper-indentation
;;; JSX Indentation
-(defmacro js--as-sgml (&rest body)
+(defmacro js-jsx--as-sgml (&rest body)
"Execute BODY as if in sgml-mode."
`(with-syntax-table sgml-mode-syntax-table
,@body))
-(defun js--outermost-enclosing-jsx-tag-pos ()
+(defun js-jsx--outermost-enclosing-tag-pos ()
(let (context tag-pos last-tag-pos parse-status parens paren-pos curly-pos)
- (js--as-sgml
+ (js-jsx--as-sgml
;; Search until we reach the top or encounter the start of a
;; JSXExpressionContainer (implying nested JSX).
(while (and (setq context (sgml-get-context))
@@ -2322,7 +2322,7 @@ js--outermost-enclosing-jsx-tag-pos
(goto-char tag-pos))))
last-tag-pos))
-(defun js--jsx-indentation ()
+(defun js-jsx--indentation-type ()
"Determine if/how the current line should be indented as JSX.
Return nil for first JSXElement line (indent like JS).
@@ -2336,7 +2336,7 @@ js--jsx-indentation
(save-excursion
;; Determine if inside a JSXElement.
(beginning-of-line) ; For exclusivity
- (when (setq tag-start-pos (js--outermost-enclosing-jsx-tag-pos))
+ (when (setq tag-start-pos (js-jsx--outermost-enclosing-tag-pos))
;; Check if inside an embedded multi-line JS expression.
(goto-char current-pos)
(end-of-line) ; For exclusivity
@@ -2369,14 +2369,14 @@ js--jsx-indentation
(setq parens (cdr parens)))))
(or type 'n+1th)))))
-(defun js--indent-line-in-jsx-expression ()
+(defun js-jsx--indent-line-in-expression ()
"Indent the current line as JavaScript within JSX."
(let ((parse-status (save-excursion (syntax-ppss (point-at-bol))))
offset indent-col)
(unless (nth 3 parse-status)
(save-excursion
(setq offset (- (point) (progn (back-to-indentation) (point)))
- indent-col (js--as-sgml (sgml-calculate-indent))))
+ indent-col (js-jsx--as-sgml (sgml-calculate-indent))))
(if (null indent-col) 'noindent ; Like in sgml-mode
;; Use whichever indentation column is greater, such that the
;; SGML column is effectively a minimum.
@@ -2384,9 +2384,9 @@ js--indent-line-in-jsx-expression
(+ indent-col js-indent-level)))
(when (> offset 0) (forward-char offset))))))
-(defun js--indent-n+1th-jsx-line ()
+(defun js-jsx--indent-n+1th-line ()
"Indent the current line as JSX within JavaScript."
- (js--as-sgml (sgml-indent-line)))
+ (js-jsx--as-sgml (sgml-indent-line)))
(defun js-indent-line ()
"Indent the current line as JavaScript."
@@ -2403,10 +2403,10 @@ js-jsx-indent-line
i.e., customize JSX element indentation with `sgml-basic-offset',
`sgml-attribute-offset' et al."
(interactive)
- (let ((type (js--jsx-indentation)))
+ (let ((type (js-jsx--indentation-type)))
(if type
- (if (eq type 'n+1th) (js--indent-n+1th-jsx-line)
- (js--indent-line-in-jsx-expression))
+ (if (eq type 'n+1th) (js-jsx--indent-n+1th-line)
+ (js-jsx--indent-line-in-expression))
(js-indent-line))))
;;; Filling
--
2.11.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #7: 0006-Add-basic-JSX-font-locking.patch --]
[-- Type: text/x-patch; name="0006-Add-basic-JSX-font-locking.patch", Size: 13730 bytes --]
From 9c2271065aa5408544fb09fe386de3f357a1d3e6 Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Sun, 17 Feb 2019 00:38:01 -0800
Subject: [PATCH 06/19] Add basic JSX font-locking
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Font-lock JSX from the beginning of the buffer to the end. Tends to
break temporarily when editing lines, because the parser doesn’t yet
look backwards to determine if the end of a tag in the current range
starts before the range.
This also re-breaks some tests fixed by previous commits, as we begin
to take a different direction in our parsing code, looking for JSX,
rather than for non-JSX. The parsing code will eventually provide
information for indentation again.
* lisp/progmodes/js.el (js--dotted-captured-name-re)
(js-jsx--disambiguate-beginning-of-tag)
(js-jsx--disambiguate-end-of-tag, js-jsx--disambiguate-syntax):
Remove.
(js-jsx--font-lock-keywords): New variable.
(js--font-lock-keywords-3): Add JSX matchers.
(js-jsx--match-tag-name, js-jsx--match-attribute-name): New functions.
(js-jsx--syntax-propertize-tag): New function to aid in JSX
font-locking and eventually indentation.
(js-jsx--text-properties): New variable.
(js-syntax-propertize): Propertize JSX properly using
syntax-propertize-rules.
---
lisp/progmodes/js.el | 216 +++++++++++++++++++++++++++++----------------------
1 file changed, 124 insertions(+), 92 deletions(-)
diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 4404ea04a0..1319fa1939 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -82,10 +82,6 @@ js--dotted-name-re
(concat js--name-re "\\(?:\\." js--name-re "\\)*")
"Regexp matching a dot-separated sequence of JavaScript names.")
-(defconst js--dotted-captured-name-re
- (concat "\\(" js--name-re "\\)\\(?:\\." js--name-re "\\)*")
- "Like `js--dotted-name-re', but capture the first name.")
-
(defconst js--cpp-name-re js--name-re
"Regexp matching a C preprocessor name.")
@@ -1498,6 +1494,33 @@ js--variable-decl-matcher
;; Matcher always "fails"
nil)
+(defconst js-jsx--font-lock-keywords
+ `((js-jsx--match-tag-name 0 font-lock-function-name-face t)
+ (js-jsx--match-attribute-name 0 font-lock-variable-name-face t))
+ "JSX font lock faces.")
+
+(defun js-jsx--match-tag-name (limit)
+ "Match JSXBoundaryElement names, until LIMIT."
+ (when js-jsx-syntax
+ (let ((pos (next-single-char-property-change (point) 'js-jsx-tag-name nil limit))
+ value)
+ (when (and pos (> pos (point)))
+ (goto-char pos)
+ (or (and (setq value (get-text-property pos 'js-jsx-tag-name))
+ (progn (set-match-data value) t))
+ (js-jsx--match-tag-name limit))))))
+
+(defun js-jsx--match-attribute-name (limit)
+ "Match JSXAttribute names, until LIMIT."
+ (when js-jsx-syntax
+ (let ((pos (next-single-char-property-change (point) 'js-jsx-attribute-name nil limit))
+ value)
+ (when (and pos (> pos (point)))
+ (goto-char pos)
+ (or (and (setq value (get-text-property pos 'js-jsx-attribute-name))
+ (progn (set-match-data value) t))
+ (js-jsx--match-attribute-name limit))))))
+
(defconst js--font-lock-keywords-3
`(
;; This goes before keywords-2 so it gets used preferentially
@@ -1609,7 +1632,10 @@ js--font-lock-keywords-3
(forward-symbol -1)
(end-of-line))
'(end-of-line)
- '(0 font-lock-variable-name-face))))
+ '(0 font-lock-variable-name-face)))
+
+ ;; jsx (when enabled)
+ ,@js-jsx--font-lock-keywords)
"Level three font lock for `js-mode'.")
(defun js--inside-pitem-p (pitem)
@@ -1743,94 +1769,100 @@ js--unary-keyword-p
"Check if STRING is a unary operator keyword in JavaScript."
(string-match-p js--unary-keyword-re string))
-(defun js-jsx--disambiguate-beginning-of-tag ()
- "Parse enough to determine if a JSX tag starts here.
-Disambiguate JSX from equality operators by testing for syntax
-only valid as JSX."
- ;; “</…” - a JSXClosingElement.
- ;; “<>” - a JSXOpeningFragment.
- (if (memq (char-after) '(?\/ ?\>)) t
- (save-excursion
- (skip-chars-forward " \t\n")
- (and
- (looking-at js--dotted-captured-name-re)
- ;; Don’t match code like “if (i < await foo)”
- (not (js--unary-keyword-p (match-string 1)))
- (progn
- (goto-char (match-end 0))
- (skip-chars-forward " \t\n")
- (or
- ;; “>”, “/>” - tag enders.
- ;; “{” - a JSXExpressionContainer.
- (memq (char-after) '(?\> ?\/ ?\{))
- ;; Check if a JSXAttribute follows.
- (looking-at js--name-start-re)))))))
-
-(defun js-jsx--disambiguate-end-of-tag ()
- "Parse enough to determine if a JSX tag ends here.
-Disambiguate JSX from equality operators by testing for syntax
-only valid as JSX, or extremely unlikely except as JSX."
- (save-excursion
- (backward-char)
- ;; “…/>” - a self-closing JSXOpeningElement.
- ;; “</>” - a JSXClosingFragment.
- (if (= (char-before) ?/) t
- (let (last-tag-or-attr-name last-non-unary-p)
- (catch 'match
- (while t
- (skip-chars-backward " \t\n")
- ;; Check if the end of a JSXAttribute value or
- ;; JSXExpressionContainer almost certainly precedes.
- ;; The only valid JS this misses is
- ;; - {} > foo
- ;; - "bar" > foo
- ;; which is no great loss, IMHO…
- (if (memq (char-before) '(?\} ?\" ?\' ?\`)) (throw 'match t)
- (if (and last-tag-or-attr-name last-non-unary-p
- ;; “<”, “</” - tag starters.
- (memq (char-before) '(?\< ?\/)))
- ;; Leftmost name parsed was the name of a
- ;; JSXOpeningElement.
- (throw 'match t))
- ;; Technically the dotted name could span multiple
- ;; lines, but dear God WHY?! Also, match greedily to
- ;; ensure the entire name is valid.
- (if (looking-back js--dotted-captured-name-re (point-at-bol) t)
- (if (and (setq last-non-unary-p (not (js--unary-keyword-p (match-string 1))))
- last-tag-or-attr-name)
- ;; Valid (non-unary) name followed rightwards by
- ;; another name (any will do, including
- ;; keywords) is invalid JS, but valid JSX.
- (throw 'match t)
- ;; Remember match and skip backwards over it when
- ;; it is the first matched name or the N+1th
- ;; matched unary name (unary names on the left are
- ;; still ambiguously JS or JSX, so keep parsing to
- ;; disambiguate).
- (setq last-tag-or-attr-name (match-string 1))
- (goto-char (match-beginning 0)))
- ;; Nothing else to look for; give up parsing.
- (throw 'match nil)))))))))
-
-(defun js-jsx--disambiguate-syntax (start end)
- "Figure out which ‘<’ and ‘>’ chars (from START to END) aren’t JSX.
-
-Later, this info prevents ‘sgml-’ functions from treating some
-‘<’ and ‘>’ chars as parts of tokens of SGML tags — a good thing,
-since they are serving their usual function as some JS equality
-operator or arrow function, instead."
- (goto-char start)
- (while (re-search-forward "[<>]" end t)
- (unless (if (eq (char-before) ?<) (js-jsx--disambiguate-beginning-of-tag)
- (js-jsx--disambiguate-end-of-tag))
- ;; Inform sgml- functions that this >, >=, >>>, <, <=, <<<, or
- ;; => token is punctuation (and not an open or close parenthesis
- ;; as per usual in sgml-mode).
- (put-text-property (1- (point)) (point) 'syntax-table '(1)))))
+(defun js-jsx--syntax-propertize-tag (end)
+ "Determine if a JSXBoundaryElement is before END and propertize it.
+Disambiguate JSX from inequality operators and arrow functions by
+testing for syntax only valid as JSX."
+ (let ((tag-beg (1- (point))) tag-end (type 'open)
+ name-beg name-match-data unambiguous
+ forward-sexp-function) ; Use Lisp version.
+ (catch 'stop
+ (while (and (< (point) end)
+ (progn (skip-chars-forward " \t\n" end)
+ (< (point) end)))
+ (cond
+ ((= (char-after) ?>)
+ (forward-char)
+ (setq unambiguous t
+ tag-end (point))
+ (throw 'stop nil))
+ ;; Handle a JSXSpreadChild (“<Foo {...bar}”) or a
+ ;; JSXExpressionContainer as a JSXAttribute value
+ ;; (“<Foo bar={…}”). Check this early in case continuing a
+ ;; JSXAttribute parse.
+ ((and name-beg (= (char-after) ?{))
+ (setq unambiguous t) ; JSXExpressionContainer post tag name ⇒ JSX
+ (let (expr-end)
+ (condition-case nil
+ (save-excursion
+ (forward-sexp)
+ (setq expr-end (point)))
+ (scan-error nil))
+ (forward-char)
+ (if (>= (point) end) (throw 'stop nil))
+ (skip-chars-forward " \t\n" end)
+ (if (>= (point) end) (throw 'stop nil))
+ (if (= (char-after) ?}) (forward-char) ; Shortcut to bail.
+ ;; Recursively propertize the JSXExpressionContainer’s
+ ;; expression.
+ (js-syntax-propertize (point) (if expr-end (min (1- expr-end) end) end))
+ ;; Exit the JSXExpressionContainer if that’s possible,
+ ;; else move to the end of the propertized area.
+ (goto-char (if expr-end (min expr-end end) end)))))
+ ((= (char-after) ?/)
+ ;; Assume a tag is an open tag until a slash is found, then
+ ;; figure out what type it actually is.
+ (if (eq type 'open) (setq type (if name-beg 'self-closing 'close)))
+ (forward-char))
+ ((looking-at js--dotted-name-re)
+ (if (not name-beg)
+ (progn
+ ;; Don’t match code like “if (i < await foo)”
+ (if (js--unary-keyword-p (match-string 0)) (throw 'stop nil))
+ ;; Save boundaries for later fontification after
+ ;; unambiguously determining the code is JSX.
+ (setq name-beg (match-beginning 0)
+ name-match-data (match-data))
+ (goto-char (match-end 0)))
+ (setq unambiguous t) ; Non-unary name followed by 2nd name ⇒ JSX
+ ;; Save JSXAttribute’s name’s match data for font-locking later.
+ (put-text-property (match-beginning 0) (1+ (match-beginning 0))
+ 'js-jsx-attribute-name (match-data))
+ (goto-char (match-end 0))
+ (if (>= (point) end) (throw 'stop nil))
+ (skip-chars-forward " \t\n" end)
+ (if (>= (point) end) (throw 'stop nil))
+ ;; “=” is optional for null-valued JSXAttributes.
+ (when (= (char-after) ?=)
+ (forward-char)
+ (if (>= (point) end) (throw 'stop nil))
+ (skip-chars-forward " \t\n" end)
+ (if (>= (point) end) (throw 'stop nil))
+ ;; Skip over strings (if possible). Any
+ ;; JSXExpressionContainer here will be parsed in the
+ ;; next iteration of the loop.
+ (when (memq (char-after) '(?\" ?\' ?\`))
+ (condition-case nil
+ (forward-sexp)
+ (scan-error (throw 'stop nil)))))))
+ ;; There is nothing more to check; this either isn’t JSX, or
+ ;; the tag is incomplete.
+ (t (throw 'stop nil)))))
+ (when unambiguous
+ ;; Save JSXBoundaryElement’s name’s match data for font-locking.
+ (if name-beg (put-text-property name-beg (1+ name-beg) 'js-jsx-tag-name name-match-data))
+ ;; Mark beginning and end of tag for features like indentation.
+ (put-text-property tag-beg (1+ tag-beg) 'js-jsx-tag-beg type)
+ (if tag-end (put-text-property (1- tag-end) tag-end 'js-jsx-tag-end tag-beg)))))
+
+(defconst js-jsx--text-properties
+ '(js-jsx-tag-beg nil js-jsx-tag-end nil js-jsx-tag-name nil js-jsx-attribute-name nil)
+ "Plist of text properties added by `js-syntax-propertize'.")
(defun js-syntax-propertize (start end)
;; JavaScript allows immediate regular expression objects, written /.../.
(goto-char start)
+ (if js-jsx-syntax (remove-text-properties start end js-jsx--text-properties))
(js-syntax-propertize-regexp end)
(funcall
(syntax-propertize-rules
@@ -1854,9 +1886,9 @@ js-syntax-propertize
(put-text-property (match-beginning 1) (match-end 1)
'syntax-table (string-to-syntax "\"/"))
(js-syntax-propertize-regexp end)))))
- ("\\`\\(#\\)!" (1 "< b")))
- (point) end)
- (if js-jsx-syntax (js-jsx--disambiguate-syntax start end)))
+ ("\\`\\(#\\)!" (1 "< b"))
+ ("<" (0 (ignore (if js-jsx-syntax (js-jsx--syntax-propertize-tag end))))))
+ (point) end))
(defconst js--prettify-symbols-alist
'(("=>" . ?⇒)
--
2.11.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #8: 0007-Font-lock-JSX-while-editing-it-by-extending-regions.patch --]
[-- Type: text/x-patch; name="0007-Font-lock-JSX-while-editing-it-by-extending-regions.patch", Size: 7312 bytes --]
From 08712ee97b2b4f37840a8d275234a53dd2df421e Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Sun, 17 Feb 2019 21:16:13 -0800
Subject: [PATCH 07/19] Font-lock JSX while editing it by extending regions
* lisp/progmodes/js.el (js-jsx--font-lock-keywords):
Call tag beginning and end matchers.
(js-jsx--match-tag-beg, js-jsx--match-tag-end): New functions.
(js-jsx--syntax-propertize-tag): Record buffer positions of JSXElement
beginning and end for font-locking.
(js--syntax-propertize-extend-region)
(js-jsx--syntax-propertize-extend-region): New functions for extending
the syntax-propertize region backwards to the start of a JSXElement so
its JSXAttribute children on its n+1th lines can be parsed as such
while editing those lines.
(js-mode): Add js--syntax-propertize-extend-region to
syntax-propertize-extend-region-functions.
---
lisp/progmodes/js.el | 81 +++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 74 insertions(+), 7 deletions(-)
diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 1319fa1939..7fb4bcc808 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -1496,8 +1496,10 @@ js--variable-decl-matcher
(defconst js-jsx--font-lock-keywords
`((js-jsx--match-tag-name 0 font-lock-function-name-face t)
- (js-jsx--match-attribute-name 0 font-lock-variable-name-face t))
- "JSX font lock faces.")
+ (js-jsx--match-attribute-name 0 font-lock-variable-name-face t)
+ (js-jsx--match-tag-beg)
+ (js-jsx--match-tag-end))
+ "JSX font lock faces and multiline text properties.")
(defun js-jsx--match-tag-name (limit)
"Match JSXBoundaryElement names, until LIMIT."
@@ -1521,6 +1523,28 @@ js-jsx--match-attribute-name
(progn (set-match-data value) t))
(js-jsx--match-attribute-name limit))))))
+(defun js-jsx--match-tag-beg (limit)
+ "Match JSXBoundaryElements from start, until LIMIT."
+ (when js-jsx-syntax
+ (let ((pos (next-single-char-property-change (point) 'js-jsx-tag-beg nil limit))
+ value)
+ (when (and pos (> pos (point)))
+ (goto-char pos)
+ (or (and (setq value (get-text-property pos 'js-jsx-tag-beg))
+ (progn (put-text-property pos (cdr value) 'font-lock-multiline t) t))
+ (js-jsx--match-tag-beg limit))))))
+
+(defun js-jsx--match-tag-end (limit)
+ "Match JSXBoundaryElements from end, until LIMIT."
+ (when js-jsx-syntax
+ (let ((pos (next-single-char-property-change (point) 'js-jsx-tag-end nil limit))
+ value)
+ (when (and pos (> pos (point)))
+ (goto-char pos)
+ (or (and (setq value (get-text-property pos 'js-jsx-tag-end))
+ (progn (put-text-property value pos 'font-lock-multiline t) t))
+ (js-jsx--match-tag-end limit))))))
+
(defconst js--font-lock-keywords-3
`(
;; This goes before keywords-2 so it gets used preferentially
@@ -1769,11 +1793,53 @@ js--unary-keyword-p
"Check if STRING is a unary operator keyword in JavaScript."
(string-match-p js--unary-keyword-re string))
+(defun js--syntax-propertize-extend-region (start end)
+ "Extend the START-END region for propertization, if necessary.
+For use by `syntax-propertize-extend-region-functions'."
+ (if js-jsx-syntax (js-jsx--syntax-propertize-extend-region start end)))
+
+(defun js-jsx--syntax-propertize-extend-region (start end)
+ "Extend the START-END region for propertization, if necessary.
+If any “>” in the region appears to be the end of a tag starting
+before the start of the region, extend region backwards to the
+start of that tag so parsing may proceed from that point.
+For use by `syntax-propertize-extend-region-functions'."
+ (let (new-start
+ forward-sexp-function ; Use the Lisp version.
+ parse-sexp-lookup-properties) ; Fix backward-sexp error here.
+ (catch 'stop
+ (goto-char start)
+ (while (re-search-forward ">" end t)
+ (catch 'continue
+ ;; Check if this is really a right shift bitwise operator
+ ;; (“>>” or “>>>”).
+ (unless (or (eq (char-before (1- (point))) ?>)
+ (eq (char-after) ?>))
+ (save-excursion
+ (backward-char)
+ (while (progn (if (= (point) (point-min)) (throw 'continue nil))
+ (/= (char-before) ?<))
+ (skip-chars-backward " \t\n")
+ (if (= (point) (point-min)) (throw 'continue nil))
+ (cond
+ ((memq (char-before) '(?\" ?\' ?\` ?\}))
+ (condition-case nil
+ (backward-sexp)
+ (scan-error (throw 'continue nil))))
+ ((memq (char-before) '(?\/ ?\=)) (backward-char))
+ ((looking-back js--dotted-name-re (line-beginning-position) t)
+ (goto-char (match-beginning 0)))
+ (t (throw 'continue nil))))
+ (when (< (point) start)
+ (setq new-start (1- (point)))
+ (throw 'stop nil)))))))
+ (if new-start (cons new-start end))))
+
(defun js-jsx--syntax-propertize-tag (end)
"Determine if a JSXBoundaryElement is before END and propertize it.
Disambiguate JSX from inequality operators and arrow functions by
testing for syntax only valid as JSX."
- (let ((tag-beg (1- (point))) tag-end (type 'open)
+ (let ((tag-beg (1- (point))) (type 'open)
name-beg name-match-data unambiguous
forward-sexp-function) ; Use Lisp version.
(catch 'stop
@@ -1783,8 +1849,7 @@ js-jsx--syntax-propertize-tag
(cond
((= (char-after) ?>)
(forward-char)
- (setq unambiguous t
- tag-end (point))
+ (setq unambiguous t)
(throw 'stop nil))
;; Handle a JSXSpreadChild (“<Foo {...bar}”) or a
;; JSXExpressionContainer as a JSXAttribute value
@@ -1852,8 +1917,8 @@ js-jsx--syntax-propertize-tag
;; Save JSXBoundaryElement’s name’s match data for font-locking.
(if name-beg (put-text-property name-beg (1+ name-beg) 'js-jsx-tag-name name-match-data))
;; Mark beginning and end of tag for features like indentation.
- (put-text-property tag-beg (1+ tag-beg) 'js-jsx-tag-beg type)
- (if tag-end (put-text-property (1- tag-end) tag-end 'js-jsx-tag-end tag-beg)))))
+ (put-text-property tag-beg (1+ tag-beg) 'js-jsx-tag-beg (cons type (point)))
+ (put-text-property (point) (1+ (point)) 'js-jsx-tag-end tag-beg))))
(defconst js-jsx--text-properties
'(js-jsx-tag-beg nil js-jsx-tag-end nil js-jsx-tag-name nil js-jsx-attribute-name nil)
@@ -3945,6 +4010,8 @@ js-mode
'(font-lock-syntactic-face-function
. js-font-lock-syntactic-face-function)))
(setq-local syntax-propertize-function #'js-syntax-propertize)
+ (add-hook 'syntax-propertize-extend-region-functions
+ #'js--syntax-propertize-extend-region 'append 'local)
(setq-local prettify-symbols-alist js--prettify-symbols-alist)
(setq-local parse-sexp-ignore-comments t)
--
2.11.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #9: 0008-Propertize-and-font-lock-JSXText-and-JSXExpressionCo.patch --]
[-- Type: text/x-patch; name="0008-Propertize-and-font-lock-JSXText-and-JSXExpressionCo.patch", Size: 14721 bytes --]
From 2be948e6b608f8090dce58f8d33cde593ec84d9d Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Fri, 8 Mar 2019 16:29:02 -0800
Subject: [PATCH 08/19] Propertize and font-lock JSXText and
JSXExpressionContainers
This completes highlighting support for JSX, as requested in:
- https://github.com/mooz/js2-mode/issues/140
- https://github.com/mooz/js2-mode/issues/330
- https://github.com/mooz/js2-mode/issues/409
* lisp/progmodes/js.el (js--name-start-chars): Extract part of
js--name-start-re so it can be reused in another regexp.
(js--name-start-re): Use js--name-start-chars.
(js-jsx--font-lock-keywords): Use new matchers.
(js-jsx--match-text, js-jsx--match-expr): New matchers to remove
typical JS font-locking and extend the font-locked region,
respectively.
(js-jsx--tag-re, js-jsx--self-closing-re): New regexps matching JSX.
(js-jsx--matched-tag-type, js-jsx--matching-close-tag-pos)
(js-jsx--enclosing-curly-pos, js-jsx--enclosing-tag-pos)
(js-jsx--at-enclosing-tag-child-p): New functions for parsing and
analyzing JSX.
(js-jsx--text-range, js-jsx--syntax-propertize-tag-text): New
functions for propertizing JSXText.
(js-jsx--syntax-propertize-tag): Propertize JSXText children of tags.
(js-jsx--text-properties): Remove JSXText-related text properties when
repropertizing.
(js-mode): Extend the syntax-propertize region with
syntax-propertize-multiline; we are now adding the syntax-multiline
text property to buffer ranges that are JSXText to ensure the whole
multiline JSX construct is reidentified.
---
lisp/progmodes/js.el | 216 +++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 211 insertions(+), 5 deletions(-)
diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 7fb4bcc808..220cf97fdc 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -66,7 +66,10 @@ electric-layout-rules
;;; Constants
-(defconst js--name-start-re "[a-zA-Z_$]"
+(defconst js--name-start-chars "a-zA-Z_$"
+ "Character class chars matching the start of a JavaScript identifier.")
+
+(defconst js--name-start-re (concat "[" js--name-start-chars "]")
"Regexp matching the start of a JavaScript identifier, without grouping.")
(defconst js--stmt-delim-chars "^;{}?:")
@@ -1497,8 +1500,10 @@ js--variable-decl-matcher
(defconst js-jsx--font-lock-keywords
`((js-jsx--match-tag-name 0 font-lock-function-name-face t)
(js-jsx--match-attribute-name 0 font-lock-variable-name-face t)
+ (js-jsx--match-text 0 'default t) ; “Undo” keyword fontification.
(js-jsx--match-tag-beg)
- (js-jsx--match-tag-end))
+ (js-jsx--match-tag-end)
+ (js-jsx--match-expr))
"JSX font lock faces and multiline text properties.")
(defun js-jsx--match-tag-name (limit)
@@ -1523,6 +1528,19 @@ js-jsx--match-attribute-name
(progn (set-match-data value) t))
(js-jsx--match-attribute-name limit))))))
+(defun js-jsx--match-text (limit)
+ "Match JSXText, until LIMIT."
+ (when js-jsx-syntax
+ (let ((pos (next-single-char-property-change (point) 'js-jsx-text nil limit))
+ value)
+ (when (and pos (> pos (point)))
+ (goto-char pos)
+ (or (and (setq value (get-text-property pos 'js-jsx-text))
+ (progn (set-match-data value)
+ (put-text-property (car value) (cadr value) 'font-lock-multiline t)
+ t))
+ (js-jsx--match-text limit))))))
+
(defun js-jsx--match-tag-beg (limit)
"Match JSXBoundaryElements from start, until LIMIT."
(when js-jsx-syntax
@@ -1545,6 +1563,17 @@ js-jsx--match-tag-end
(progn (put-text-property value pos 'font-lock-multiline t) t))
(js-jsx--match-tag-end limit))))))
+(defun js-jsx--match-expr (limit)
+ "Match JSXExpressionContainers, until LIMIT."
+ (when js-jsx-syntax
+ (let ((pos (next-single-char-property-change (point) 'js-jsx-expr nil limit))
+ value)
+ (when (and pos (> pos (point)))
+ (goto-char pos)
+ (or (and (setq value (get-text-property pos 'js-jsx-expr))
+ (progn (put-text-property pos value 'font-lock-multiline t) t))
+ (js-jsx--match-expr limit))))))
+
(defconst js--font-lock-keywords-3
`(
;; This goes before keywords-2 so it gets used preferentially
@@ -1835,6 +1864,177 @@ js-jsx--syntax-propertize-extend-region
(throw 'stop nil)))))))
(if new-start (cons new-start end))))
+(defconst js-jsx--tag-re
+ (concat "<\\s-*\\("
+ "[/>]" ; JSXClosingElement, or JSXOpeningFragment, or JSXClosingFragment
+ "\\|"
+ js--dotted-name-re "\\s-*[" js--name-start-chars "{/>]" ; JSXOpeningElement
+ "\\)")
+ "Regexp unambiguously matching a JSXBoundaryElement.")
+
+(defun js-jsx--matched-tag-type ()
+ "Determine the tag type of the last match to `js-jsx--tag-re'.
+Return `close' for a JSXClosingElement/JSXClosingFragment match,
+return `self-closing' for some self-closing JSXOpeningElements,
+else return `other'."
+ (let ((chars (vconcat (match-string 1))))
+ (cond
+ ((= (aref chars 0) ?/) 'close)
+ ((= (aref chars (1- (length chars))) ?/) 'self-closing)
+ (t 'other))))
+
+(defconst js-jsx--self-closing-re "/\\s-*>"
+ "Regexp matching the end of a self-closing JSXOpeningElement.")
+
+(defun js-jsx--matching-close-tag-pos ()
+ "Return position of the closer of the opener before point.
+Assuming a JSXOpeningElement or a JSXOpeningFragment is
+immediately before point, find a matching JSXClosingElement or
+JSXClosingFragment, skipping over any nested JSXElements to find
+the match. Return nil if a match can’t be found."
+ (let ((tag-stack 1) self-closing-pos type)
+ (catch 'stop
+ (while (re-search-forward js-jsx--tag-re nil t)
+ (setq type (js-jsx--matched-tag-type))
+ ;; Balance the total of self-closing tags that we subtract
+ ;; from the stack, ignoring those tags which are never added
+ ;; to the stack (see below).
+ (unless (eq type 'self-closing)
+ (when (and self-closing-pos (> (point) self-closing-pos))
+ (setq tag-stack (1- tag-stack))))
+ (if (eq type 'close)
+ (progn
+ (setq tag-stack (1- tag-stack))
+ (when (= tag-stack 0)
+ (throw 'stop (match-beginning 0))))
+ ;; Tags that we know are self-closing aren’t added to the
+ ;; stack at all, because we only close the ones that we have
+ ;; anticipated after moving past those anticipated tags’
+ ;; ends, and if a self-closing tag is the first tag we
+ ;; encounter in this loop, then it will never be anticipated
+ ;; (due to an optimization where we sometimes can avoid
+ ;; looking for self-closing tags).
+ (unless (eq type 'self-closing)
+ (setq tag-stack (1+ tag-stack))))
+ ;; Don’t needlessly recalculate.
+ (unless (and self-closing-pos (<= (point) self-closing-pos))
+ (setq self-closing-pos nil) ; Reset if recalculating.
+ (save-excursion
+ ;; Anticipate a self-closing tag that we should make sure
+ ;; to subtract from the tag stack once we move past its
+ ;; end; we might might miss the end otherwise, due to the
+ ;; regexp-matching method we use to detect tags.
+ (when (re-search-forward js-jsx--self-closing-re nil t)
+ (setq self-closing-pos (match-beginning 0)))))))))
+
+(defun js-jsx--enclosing-curly-pos ()
+ "Return position of enclosing “{” in a “{/}” pair about point."
+ (let ((parens (reverse (nth 9 (syntax-ppss)))) paren-pos curly-pos)
+ (while
+ (and
+ (setq paren-pos (car parens))
+ (not (when (= (char-after paren-pos) ?{)
+ (setq curly-pos paren-pos)))
+ (setq parens (cdr parens))))
+ curly-pos))
+
+(defun js-jsx--enclosing-tag-pos ()
+ "Return beginning and end of a JSXElement about point.
+Look backward for a JSXElement that both starts before point and
+also ends after point. That may be either a self-closing
+JSXElement or a JSXOpeningElement/JSXClosingElement pair."
+ (let ((start (point))
+ (curly-pos (save-excursion (js-jsx--enclosing-curly-pos)))
+ tag-beg tag-beg-pos tag-end-pos close-tag-pos)
+ (while
+ (and
+ (setq tag-beg (js--backward-text-property 'js-jsx-tag-beg))
+ (progn
+ (setq tag-beg-pos (point)
+ tag-end-pos (cdr tag-beg))
+ (not
+ (or
+ (and (eq (car tag-beg) 'self-closing)
+ (< start tag-end-pos))
+ (and (eq (car tag-beg) 'open)
+ (save-excursion
+ (goto-char tag-end-pos)
+ (setq close-tag-pos (js-jsx--matching-close-tag-pos))
+ ;; The JSXOpeningElement may either be unclosed,
+ ;; else the closure must occur after the start
+ ;; point (otherwise, a miscellaneous previous
+ ;; JSXOpeningElement has been found, and we should
+ ;; keep looking back for an enclosing one).
+ (or (not close-tag-pos) (< start close-tag-pos))))))))
+ ;; Don’t return the last tag pos (if any; it wasn’t enclosing).
+ (setq tag-beg nil))
+ (and tag-beg
+ (or (not curly-pos) (> tag-beg-pos curly-pos))
+ (cons tag-beg-pos tag-end-pos))))
+
+(defun js-jsx--at-enclosing-tag-child-p ()
+ "Return t if point is at an enclosing tag’s child."
+ (let ((pos (save-excursion (js-jsx--enclosing-tag-pos))))
+ (and pos (>= (point) (cdr pos)))))
+
+(defun js-jsx--text-range (beg end)
+ "Identify JSXText within a “>/{/}/<” pair."
+ (when (> (- end beg) 0)
+ (save-excursion
+ (goto-char beg)
+ (while (and (skip-chars-forward " \t\n" end) (< (point) end))
+ ;; Comments and string quotes don’t serve their usual
+ ;; syntactic roles in JSXText; make them plain punctuation to
+ ;; negate those roles.
+ (when (or (= (char-after) ?/) ; comment
+ (= (syntax-class (syntax-after (point))) 7)) ; string quote
+ (put-text-property (point) (1+ (point)) 'syntax-table '(1)))
+ (forward-char)))
+ ;; Mark JSXText so it can be font-locked as non-keywords.
+ (put-text-property beg (1+ beg) 'js-jsx-text (list beg end (current-buffer)))
+ ;; Ensure future propertization beginning from within the
+ ;; JSXText determines JSXText context from earlier lines.
+ (put-text-property beg end 'syntax-multiline t)))
+
+(defun js-jsx--syntax-propertize-tag-text (end)
+ "Determine if JSXText is before END and propertize it.
+Text within an open/close tag pair may be JSXText. Temporarily
+interrupt JSXText by JSXExpressionContainers, and terminate
+JSXText when another JSXBoundaryElement is encountered. Despite
+terminations, all JSXText will be identified once all the
+JSXBoundaryElements within an outermost JSXElement’s tree have
+been propertized."
+ (let ((text-beg (point))
+ forward-sexp-function) ; Use Lisp version.
+ (catch 'stop
+ (while (re-search-forward "[{<]" end t)
+ (js-jsx--text-range text-beg (1- (point)))
+ (cond
+ ((= (char-before) ?{)
+ (let (expr-beg expr-end)
+ (condition-case nil
+ (save-excursion
+ (backward-char)
+ (setq expr-beg (point))
+ (forward-sexp)
+ (setq expr-end (point)))
+ (scan-error nil))
+ ;; Recursively propertize the JSXExpressionContainer’s
+ ;; (possibly-incomplete) expression.
+ (js-syntax-propertize (1+ expr-beg) (if expr-end (min (1- expr-end) end) end))
+ ;; Ensure future propertization beginning from within the
+ ;; (possibly-incomplete) expression can determine JSXText
+ ;; context from earlier lines.
+ (put-text-property expr-beg (1+ expr-beg) 'js-jsx-expr (or expr-end end)) ; font-lock
+ (put-text-property expr-beg (if expr-end (min expr-end end) end) 'syntax-multiline t) ; syntax-propertize
+ ;; Exit the JSXExpressionContainer if that’s possible,
+ ;; else move to the end of the propertized area.
+ (goto-char (if expr-end (min expr-end end) end))))
+ ((= (char-before) ?<)
+ (backward-char) ; Ensure the next tag can be propertized.
+ (throw 'stop nil)))
+ (setq text-beg (point))))))
+
(defun js-jsx--syntax-propertize-tag (end)
"Determine if a JSXBoundaryElement is before END and propertize it.
Disambiguate JSX from inequality operators and arrow functions by
@@ -1916,12 +2116,16 @@ js-jsx--syntax-propertize-tag
(when unambiguous
;; Save JSXBoundaryElement’s name’s match data for font-locking.
(if name-beg (put-text-property name-beg (1+ name-beg) 'js-jsx-tag-name name-match-data))
- ;; Mark beginning and end of tag for features like indentation.
+ ;; Mark beginning and end of tag for font-locking.
(put-text-property tag-beg (1+ tag-beg) 'js-jsx-tag-beg (cons type (point)))
- (put-text-property (point) (1+ (point)) 'js-jsx-tag-end tag-beg))))
+ (put-text-property (point) (1+ (point)) 'js-jsx-tag-end tag-beg))
+ (if (js-jsx--at-enclosing-tag-child-p) (js-jsx--syntax-propertize-tag-text end))))
(defconst js-jsx--text-properties
- '(js-jsx-tag-beg nil js-jsx-tag-end nil js-jsx-tag-name nil js-jsx-attribute-name nil)
+ (list
+ 'js-jsx-tag-beg nil 'js-jsx-tag-end nil
+ 'js-jsx-tag-name nil 'js-jsx-attribute-name nil
+ 'js-jsx-text nil 'js-jsx-expr nil)
"Plist of text properties added by `js-syntax-propertize'.")
(defun js-syntax-propertize (start end)
@@ -4011,6 +4215,8 @@ js-mode
. js-font-lock-syntactic-face-function)))
(setq-local syntax-propertize-function #'js-syntax-propertize)
(add-hook 'syntax-propertize-extend-region-functions
+ #'syntax-propertize-multiline 'append 'local)
+ (add-hook 'syntax-propertize-extend-region-functions
#'js--syntax-propertize-extend-region 'append 'local)
(setq-local prettify-symbols-alist js--prettify-symbols-alist)
--
2.11.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #10: 0009-Update-expectations-for-JSX-indentation-in-JSXAttrib.patch --]
[-- Type: text/x-patch; name="0009-Update-expectations-for-JSX-indentation-in-JSXAttrib.patch", Size: 1538 bytes --]
From edae9ccb0acd84cdd62566d0f96c167b29138965 Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Sat, 23 Mar 2019 12:33:20 -0700
Subject: [PATCH 09/19] Update expectations for JSX indentation in JSXAttribute
space
* test/manual/indent/js-jsx.js: Align expectations for dangling
closing constructs with other places in the tests.
---
test/manual/indent/js-jsx.js | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/test/manual/indent/js-jsx.js b/test/manual/indent/js-jsx.js
index af3c340559..2ec00c63bb 100644
--- a/test/manual/indent/js-jsx.js
+++ b/test/manual/indent/js-jsx.js
@@ -37,7 +37,7 @@ return (
React.render(
<input
- />,
+ />,
{
a: 1
}
@@ -242,12 +242,18 @@ export default ({ stars }) => (
// JS expressions should not break indentation
// (https://github.com/mooz/js2-mode/issues/462).
+//
+// In the referenced issue, the user actually wanted indentation which
+// was simply different than Emacs’ SGML attribute indentation.
+// Nevertheless, his issue highlighted our inability to properly
+// indent code with JSX inside JSXExpressionContainers inside JSX.
return (
<Router>
<Bar>
- <Route exact path="/foo" render={() => (
- <div>nothing</div>
- )} />
+ <Route exact path="/foo"
+ render={() => (
+ <div>nothing</div>
+ )} />
<Route exact path="/bar" />
</Bar>
</Router>
--
2.11.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #11: 0010-Indent-JSX-as-parsed-in-a-JS-context.patch --]
[-- Type: text/x-patch; name="0010-Indent-JSX-as-parsed-in-a-JS-context.patch", Size: 20564 bytes --]
From b9d31378f8e4ef0e2dc35a2c50862d6d56bcbebc Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Sat, 23 Mar 2019 14:22:35 -0700
Subject: [PATCH 10/19] Indent JSX as parsed in a JS context
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Fixes the following issues (and re-fixes indentation issues initially
fixed but later re-broken by previous commits in the process of adding
comprehensive JSX support):
- https://github.com/mooz/js2-mode/issues/389#issuecomment-390766873
- https://github.com/mooz/js2-mode/issues/482
- Bug#32158
- https://github.com/mooz/js2-mode/issues/462
Previously, we delegated to sgml-mode functions for JSX indentation.
However, there were some problems with this approach:
- sgml-mode does not anticipate tags inside attributes when indenting,
which compromises JSX indentation inside JSXExpressionContainers
inside JSXAttributes.
- In previous iterations to provide comprehensive JSX support, it
proved tedious to disambiguate “<” and “>” as JS inequality
operators and arrow functions from opening and closing angle
brackets as part of SGML tags. That code evolved into a more
complete JSX parsing implementation for syntax-propertize rules for
font-locking, discarding the superfluous “<”/“>” disambiguation in
anticipation of using the improved JSX analysis for indentation.
- Using sgml-mode functions, we controlled JSX indentation using SGML
variables. However, JSX is a different thing than SGML; referencing
SGML in JS was a leaky abstraction.
To resolve these issues, use the text properties added by the JSX
syntax-propertize code to determine the boundaries of various aspects
of JSX syntax, and reimplement the sgml-mode indentation code in
js-mode with better respect to JSX indentation conventions.
* lisp/progmodes/js.el (js-jsx-attribute-offset): New variable to
provide a way for users to still control JSX attribute offsets as they
could with sgml-attribute-offset before. The value of this feature is
dubious IMO, but it’s trivial to keep it, so let’s do it just in case.
(js-jsx--goto-outermost-enclosing-curly): New function.
(js-jsx--enclosing-tag-pos): Refactor to be unbounded by curlies, so
this function can be used to find JSXExpressionContainers within JSX.
Fix bug where an enclosing JSXElement couldn’t be found when point was
at the start of its JSXClosingElement. Return the JSXClosingElement’s
position as well, so the JSXClosingElement can be indentified when
indenting and be indented like the matching JSXOpeningElement.
(js-jsx--at-enclosing-tag-child-p): js-jsx--enclosing-tag-pos now
returns a list rather than a cons, so retrieve the JSXOpeningElement’s
end position from a list.
(js-jsx--context, js-jsx--indenting): New function and variable.
(js-jsx--indentation): New function replacing the prior
js-jsx--indent* functions and js-jsx-indent-line’s implementation.
Use the JSX parsing performed in a JS context to more accurately
calculate JSX indentation than by delegating to sgml-mode functions.
(js--proper-indentation): Use js-jsx--indentation as yet another type
of indentation.
(js-jsx--as-sgml, js-jsx--outermost-enclosing-tag-pos)
(js-jsx--indentation-type, js-jsx--indent-line-in-expression)
(js-jsx--indent-n+1th-line): Remove obsolete functions.
(js-jsx-indent-line): Refactor nearly-obsolete function to behave the
same as it usually would before these changes, without respect to the
binding of js-jsx-syntax.
(js-jsx-mode): Remove obsolete documentation about the use of SGML
variables to control indentation, and don’t bind indent-line-function
any more, because it is no longer necessary given the new
implementation of js-jsx-indent-line.
---
lisp/progmodes/js.el | 307 +++++++++++++++++++++++++++------------------------
1 file changed, 165 insertions(+), 142 deletions(-)
diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 220cf97fdc..af83e04df4 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -584,6 +584,29 @@ js-jsx-syntax
:safe 'booleanp
:group 'js)
+(defcustom js-jsx-attribute-offset 0
+ "Specifies a delta for JSXAttribute indentation.
+
+Let `js-indent-level' be 2. When this variable is also set to 0,
+JSXAttribute indentation looks like this:
+
+ <element
+ attribute=\"value\">
+ </element>
+
+Alternatively, when this variable is also set to 2, JSXAttribute
+indentation looks like this:
+
+ <element
+ attribute=\"value\">
+ </element>
+
+This variable is like `sgml-attribute-offset'."
+ :version "27.1"
+ :type 'integer
+ :safe 'integerp
+ :group 'js)
+
;;; KeyMap
(defvar js-mode-map
@@ -1938,14 +1961,21 @@ js-jsx--enclosing-curly-pos
(setq parens (cdr parens))))
curly-pos))
+(defun js-jsx--goto-outermost-enclosing-curly (limit)
+ "Set point to enclosing “{” at or closest after LIMIT."
+ (let (pos)
+ (while
+ (and
+ (setq pos (js-jsx--enclosing-curly-pos))
+ (if (>= pos limit) (goto-char pos))
+ (> pos limit)))))
+
(defun js-jsx--enclosing-tag-pos ()
"Return beginning and end of a JSXElement about point.
Look backward for a JSXElement that both starts before point and
also ends after point. That may be either a self-closing
JSXElement or a JSXOpeningElement/JSXClosingElement pair."
- (let ((start (point))
- (curly-pos (save-excursion (js-jsx--enclosing-curly-pos)))
- tag-beg tag-beg-pos tag-end-pos close-tag-pos)
+ (let ((start (point)) tag-beg tag-beg-pos tag-end-pos close-tag-pos)
(while
(and
(setq tag-beg (js--backward-text-property 'js-jsx-tag-beg))
@@ -1957,25 +1987,24 @@ js-jsx--enclosing-tag-pos
(and (eq (car tag-beg) 'self-closing)
(< start tag-end-pos))
(and (eq (car tag-beg) 'open)
- (save-excursion
- (goto-char tag-end-pos)
- (setq close-tag-pos (js-jsx--matching-close-tag-pos))
- ;; The JSXOpeningElement may either be unclosed,
- ;; else the closure must occur after the start
- ;; point (otherwise, a miscellaneous previous
- ;; JSXOpeningElement has been found, and we should
- ;; keep looking back for an enclosing one).
- (or (not close-tag-pos) (< start close-tag-pos))))))))
- ;; Don’t return the last tag pos (if any; it wasn’t enclosing).
- (setq tag-beg nil))
- (and tag-beg
- (or (not curly-pos) (> tag-beg-pos curly-pos))
- (cons tag-beg-pos tag-end-pos))))
+ (or (< start tag-end-pos)
+ (save-excursion
+ (goto-char tag-end-pos)
+ (setq close-tag-pos (js-jsx--matching-close-tag-pos))
+ ;; The JSXOpeningElement may be unclosed, else
+ ;; the closure must occur at/after the start
+ ;; point (otherwise, a miscellaneous previous
+ ;; JSXOpeningElement has been found, so keep
+ ;; looking backwards for an enclosing one).
+ (or (not close-tag-pos) (<= start close-tag-pos)))))))))
+ ;; Don’t return the last tag pos, as it wasn’t enclosing.
+ (setq tag-beg nil close-tag-pos nil))
+ (and tag-beg (list tag-beg-pos tag-end-pos close-tag-pos))))
(defun js-jsx--at-enclosing-tag-child-p ()
"Return t if point is at an enclosing tag’s child."
(let ((pos (save-excursion (js-jsx--enclosing-tag-pos))))
- (and pos (>= (point) (cdr pos)))))
+ (and pos (>= (point) (nth 1 pos)))))
(defun js-jsx--text-range (beg end)
"Identify JSXText within a “>/{/}/<” pair."
@@ -2515,6 +2544,118 @@ js--looking-at-broken-arrow-function-p
(t (looking-at-p
(concat js--name-re js--line-terminating-arrow-re)))))
+(defun js-jsx--context ()
+ "Determine JSX context and move to enclosing JSX."
+ (let ((pos (point))
+ (parse-status (syntax-ppss))
+ (enclosing-tag-pos (js-jsx--enclosing-tag-pos)))
+ (when enclosing-tag-pos
+ (if (< pos (nth 1 enclosing-tag-pos))
+ (if (nth 3 parse-status)
+ (list 'string (nth 8 parse-status))
+ (list 'tag (nth 0 enclosing-tag-pos) (nth 1 enclosing-tag-pos)))
+ (list 'text (nth 0 enclosing-tag-pos) (nth 2 enclosing-tag-pos))))))
+
+(defvar js-jsx--indenting nil
+ "Flag to prevent infinite recursion while indenting JSX.")
+
+(defun js-jsx--indentation (parse-status)
+ "Helper function for `js--proper-indentation'.
+Return the proper indentation of the current line if it is part
+of a JSXElement expression spanning multiple lines; otherwise,
+return nil."
+ (let ((current-line (line-number-at-pos))
+ (curly-pos (js-jsx--enclosing-curly-pos))
+ nth-context context expr-p beg-line col
+ forward-sexp-function) ; Use the Lisp version.
+ ;; Find the immediate context for indentation information, but
+ ;; keep going to determine that point is at the N+1th line of
+ ;; multiline JSX.
+ (save-excursion
+ (while
+ (and
+ (setq nth-context (js-jsx--context))
+ (progn
+ (unless context
+ (setq context nth-context)
+ (setq expr-p (and curly-pos (< (point) curly-pos))))
+ (setq beg-line (line-number-at-pos))
+ (and
+ (= beg-line current-line)
+ (or (not curly-pos) (> (point) curly-pos)))))))
+ (when (and context (> current-line beg-line))
+ (save-excursion
+ ;; The column calculation is based on `sgml-calculate-indent'.
+ (setq col (pcase (nth 0 context)
+
+ ('string
+ ;; Go back to previous non-empty line.
+ (while (and (> (point) (nth 1 context))
+ (zerop (forward-line -1))
+ (looking-at "[ \t]*$")))
+ (if (> (point) (nth 1 context))
+ ;; Previous line is inside the string.
+ (current-indentation)
+ (goto-char (nth 1 context))
+ (1+ (current-column))))
+
+ ('tag
+ ;; Special JSX indentation rule: a “dangling”
+ ;; closing angle bracket on its own line is
+ ;; indented at the same level as the opening
+ ;; angle bracket of the JSXElement. Otherwise,
+ ;; indent JSXAttribute space like SGML.
+ (if (progn
+ (goto-char (nth 2 context))
+ (and (= current-line (line-number-at-pos))
+ (looking-back "^\\s-*/?>" (line-beginning-position))))
+ (progn
+ (goto-char (nth 1 context))
+ (current-column))
+ ;; Indent JSXAttribute space like SGML.
+ (goto-char (nth 1 context))
+ ;; Skip tag name:
+ (skip-chars-forward " \t")
+ (skip-chars-forward "^ \t\n")
+ (skip-chars-forward " \t")
+ (if (not (eolp))
+ (current-column)
+ ;; This is the first attribute: indent.
+ (goto-char (+ (nth 1 context) js-jsx-attribute-offset))
+ (+ (current-column) js-indent-level))))
+
+ ('text
+ ;; Indent to reflect nesting.
+ (goto-char (nth 1 context))
+ (+ (current-column)
+ ;; The last line isn’t nested, but the rest are.
+ (if (or (not (nth 2 context)) ; Unclosed.
+ (< current-line (line-number-at-pos (nth 2 context))))
+ js-indent-level
+ 0)))
+
+ )))
+ ;; When indenting a JSXExpressionContainer expression, use JSX
+ ;; indentation as a minimum, and use regular JS indentation if
+ ;; it’s deeper.
+ (if expr-p
+ (max (+ col
+ ;; An expression in a JSXExpressionContainer in a
+ ;; JSXAttribute should be indented more, except on
+ ;; the ending line of the JSXExpressionContainer.
+ (if (and (eq (nth 0 context) 'tag)
+ (< current-line
+ (save-excursion
+ (js-jsx--goto-outermost-enclosing-curly
+ (nth 1 context))
+ (forward-sexp)
+ (line-number-at-pos))))
+ js-indent-level
+ 0))
+ (let ((js-jsx--indenting t)) ; Prevent recursion.
+ (js--proper-indentation parse-status)))
+ col))))
+
(defun js--proper-indentation (parse-status)
"Return the proper indentation for the current line."
(save-excursion
@@ -2522,6 +2663,8 @@ js--proper-indentation
(cond ((nth 4 parse-status) ; inside comment
(js--get-c-offset 'c (nth 8 parse-status)))
((nth 3 parse-status) 0) ; inside string
+ ((when (and js-jsx-syntax (not js-jsx--indenting))
+ (save-excursion (js-jsx--indentation parse-status))))
((eq (char-after) ?#) 0)
((save-excursion (js--beginning-of-macro)) 4)
;; Indent array comprehension continuation lines specially.
@@ -2584,111 +2727,6 @@ js--proper-indentation
(+ js-indent-level js-expr-indent-offset))
(t (prog-first-column)))))
-;;; JSX Indentation
-
-(defmacro js-jsx--as-sgml (&rest body)
- "Execute BODY as if in sgml-mode."
- `(with-syntax-table sgml-mode-syntax-table
- ,@body))
-
-(defun js-jsx--outermost-enclosing-tag-pos ()
- (let (context tag-pos last-tag-pos parse-status parens paren-pos curly-pos)
- (js-jsx--as-sgml
- ;; Search until we reach the top or encounter the start of a
- ;; JSXExpressionContainer (implying nested JSX).
- (while (and (setq context (sgml-get-context))
- (progn
- (setq tag-pos (sgml-tag-start (car (last context))))
- (or (not curly-pos)
- ;; Stop before curly brackets (start of a
- ;; JSXExpressionContainer).
- (> tag-pos curly-pos))))
- ;; Record this position so it can potentially be returned.
- (setq last-tag-pos tag-pos)
- ;; Always parse sexps / search for the next context from the
- ;; immediately enclosing tag (sgml-get-context may not leave
- ;; point there).
- (goto-char tag-pos)
- (unless parse-status ; Don’t needlessly reparse.
- ;; Search upward for an enclosing starting curly bracket.
- (setq parse-status (syntax-ppss))
- (setq parens (reverse (nth 9 parse-status)))
- (while (and (setq paren-pos (car parens))
- (not (when (= (char-after paren-pos) ?{)
- (setq curly-pos paren-pos))))
- (setq parens (cdr parens)))
- ;; Always search for the next context from the immediately
- ;; enclosing tag (calling syntax-ppss in the above loop
- ;; may move point from there).
- (goto-char tag-pos))))
- last-tag-pos))
-
-(defun js-jsx--indentation-type ()
- "Determine if/how the current line should be indented as JSX.
-
-Return nil for first JSXElement line (indent like JS).
-Return `n+1th' for second+ JSXElement lines (indent like SGML).
-Return `expression' for lines within embedded JS expressions
- (indent like JS inside SGML).
-Return nil for non-JSX lines."
- (let ((current-pos (point))
- (current-line (line-number-at-pos))
- tag-start-pos parens paren type)
- (save-excursion
- ;; Determine if inside a JSXElement.
- (beginning-of-line) ; For exclusivity
- (when (setq tag-start-pos (js-jsx--outermost-enclosing-tag-pos))
- ;; Check if inside an embedded multi-line JS expression.
- (goto-char current-pos)
- (end-of-line) ; For exclusivity
- (setq parens (nth 9 (syntax-ppss)))
- (while
- (and
- (setq paren (car parens))
- (if (and
- (>= paren tag-start-pos)
- ;; A curly bracket indicates the start of an
- ;; embedded expression.
- (= (char-after paren) ?{)
- ;; 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).
- (if (progn
- (goto-char paren)
- (ignore-errors (let (forward-sexp-function)
- (forward-sexp))))
- (< current-line (line-number-at-pos))
- ;; No matching bracket implies we’re inside!
- t))
- ;; Indicate this will be indented specially. Return
- ;; nil to stop iterating too.
- (progn (setq type 'expression) nil)
- ;; Stop iterating when parens = nil.
- (setq parens (cdr parens)))))
- (or type 'n+1th)))))
-
-(defun js-jsx--indent-line-in-expression ()
- "Indent the current line as JavaScript within JSX."
- (let ((parse-status (save-excursion (syntax-ppss (point-at-bol))))
- offset indent-col)
- (unless (nth 3 parse-status)
- (save-excursion
- (setq offset (- (point) (progn (back-to-indentation) (point)))
- indent-col (js-jsx--as-sgml (sgml-calculate-indent))))
- (if (null indent-col) 'noindent ; Like in sgml-mode
- ;; Use whichever indentation column is greater, such that the
- ;; SGML column is effectively a minimum.
- (indent-line-to (max (js--proper-indentation parse-status)
- (+ indent-col js-indent-level)))
- (when (> offset 0) (forward-char offset))))))
-
-(defun js-jsx--indent-n+1th-line ()
- "Indent the current line as JSX within JavaScript."
- (js-jsx--as-sgml (sgml-indent-line)))
-
(defun js-indent-line ()
"Indent the current line as JavaScript."
(interactive)
@@ -2700,15 +2738,9 @@ js-indent-line
(when (> offset 0) (forward-char offset)))))
(defun js-jsx-indent-line ()
- "Indent the current line as JSX (with SGML offsets).
-i.e., customize JSX element indentation with `sgml-basic-offset',
-`sgml-attribute-offset' et al."
+ "Indent the current line as JavaScript+JSX."
(interactive)
- (let ((type (js-jsx--indentation-type)))
- (if type
- (if (eq type 'n+1th) (js-jsx--indent-n+1th-line)
- (js-jsx--indent-line-in-expression))
- (js-indent-line))))
+ (let ((js-jsx-syntax t)) (js-indent-line)))
;;; Filling
@@ -4281,18 +4313,9 @@ js-mode
;;;###autoload
(define-derived-mode js-jsx-mode js-mode "JSX"
- "Major mode for editing JSX.
-
-To customize the indentation for this mode, set the SGML offset
-variables (`sgml-basic-offset', `sgml-attribute-offset' et al.)
-locally, like so:
-
- (defun set-jsx-indentation ()
- (setq-local sgml-basic-offset js-indent-level))
- (add-hook \\='js-jsx-mode-hook #\\='set-jsx-indentation)"
+ "Major mode for editing JSX."
:group 'js
- (setq-local js-jsx-syntax t)
- (setq-local indent-line-function #'js-jsx-indent-line))
+ (setq-local js-jsx-syntax t))
;;;###autoload (defalias 'javascript-mode 'js-mode)
--
2.11.0
[-- Attachment #12: 0011-Finish-replacing-SGML-based-JSX-detection-with-js-mo.patch --]
[-- Type: text/x-patch, Size: 2388 bytes --]
From c3a559b9b317a68f9bc7a35cc866c51d4ba27122 Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Sat, 23 Mar 2019 15:01:55 -0700
Subject: [PATCH 11/19] =?UTF-8?q?Finish=20replacing=20SGML-based=20JSX=20d?=
=?UTF-8?q?etection=20with=20js-mode=E2=80=99s=20parsing?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This removes the last dependency on sgml-mode for JSX-related logic.
* lisp/progmodes/js.el (js-jsx--start-tag-re)
(js-jsx--end-tag-re): Remove.
(js-jsx--looking-at-start-tag-p)
(js-jsx--looking-back-at-end-tag-p): Reimplement using text
properties, using syntax information which ought to be slightly more
accurate than regexps since it was found by complete parsing.
---
lisp/progmodes/js.el | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index af83e04df4..df2c41332e 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -50,7 +50,6 @@
(require 'imenu)
(require 'moz nil t)
(require 'json)
-(require 'sgml-mode)
(require 'prog-mode)
(eval-when-compile
@@ -2211,13 +2210,10 @@ js--indent-operator-re
(js--regexp-opt-symbol '("in" "instanceof")))
"Regexp matching operators that affect indentation of continued expressions.")
-(defconst js-jsx--start-tag-re
- (concat "<" sgml-name-re)
- "Regexp matching code that looks like a JSXOpeningElement.")
-
(defun js-jsx--looking-at-start-tag-p ()
"Non-nil if a JSXOpeningElement immediately follows point."
- (looking-at js-jsx--start-tag-re))
+ (let ((tag-beg (get-text-property (point) 'js-jsx-tag-beg)))
+ (and tag-beg (memq (car tag-beg) '(open self-closing)))))
(defun js--looking-at-operator-p ()
"Return non-nil if point is on a JavaScript operator, other than a comma."
@@ -2263,13 +2259,9 @@ js--find-newline-backward
(setq result nil)))
result))
-(defconst js-jsx--end-tag-re
- (concat "</" sgml-name-re ">\\|/>")
- "Regexp matching a JSXClosingElement.")
-
(defun js-jsx--looking-back-at-end-tag-p ()
"Non-nil if a JSXClosingElement immediately precedes point."
- (looking-back js-jsx--end-tag-re (point-at-bol)))
+ (get-text-property (point) 'js-jsx-tag-end))
(defun js--continued-expression-p ()
"Return non-nil if the current line continues an expression."
--
2.11.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #13: 0012-Automatically-detect-JSX-in-JavaScript-files.patch --]
[-- Type: text/x-patch; name="0012-Automatically-detect-JSX-in-JavaScript-files.patch", Size: 8338 bytes --]
From 2d1b5259825e70f8b95e0fc42213a3d141ee4e75 Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Sat, 23 Mar 2019 20:14:29 -0700
Subject: [PATCH 12/19] Automatically detect JSX in JavaScript files
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* lisp/files.el (auto-mode-alist): Simply enable
javascript-mode (js-mode) when opening “.jsx” files, since the “.jsx”
file extension will be used as an indicator of JSX syntax by js-mode,
and more code is likely to work in js-mode than js-jsx-mode, and we
probably want to guide users to use js-mode (with js-jsx-syntax)
instead. Code that used to work exclusively in js-jsx-mode (if anyone
ever wrote any) ought to be updated to work in js-mode too when
js-jsx-syntax is set to t.
* lisp/progmodes/js.el (js-jsx-detect-syntax, js-jsx-regexps)
(js-jsx--detect-and-enable, js-jsx--detect-after-change): New
variables and functions for detecting and enabling JSX.
(js-jsx-syntax): Update docstring with respect to the widened scope of
the effects and use of this variable.
(js-syntactic-mode-name, js--update-mode-name)
(js--idly-update-mode-name, js-jsx-enable): New variable and functions
for indicating when JSX is enabled.
(js-mode): Detect and enable JSX. Print all enabled syntaxes after
the mode name whenever Emacs goes idle; this ensures lately-enabled
syntaxes are evident.
(js-jsx-mode): Update mode name for consistency with the state in
which JSX is enabled in js-mode. Update docstring to suggest
alternative means of using JSX without this mode. Going forward, it
may be best to gently guide users away from js-jsx-mode, since a “one
mode per syntax extension” model would not scale well if more syntax
extensions were to be simultaneously supported (e.g. Facebook’s
“Flow”).
---
lisp/files.el | 3 +-
lisp/progmodes/js.el | 119 ++++++++++++++++++++++++++++++++++++++++++++++++---
2 files changed, 115 insertions(+), 7 deletions(-)
diff --git a/lisp/files.el b/lisp/files.el
index 77a194b085..6ef63fd4de 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -2705,9 +2705,8 @@ auto-mode-alist
("\\.dbk\\'" . xml-mode)
("\\.dtd\\'" . sgml-mode)
("\\.ds\\(ss\\)?l\\'" . dsssl-mode)
- ("\\.jsm?\\'" . javascript-mode)
+ ("\\.js[mx]?\\'" . javascript-mode)
("\\.json\\'" . javascript-mode)
- ("\\.jsx\\'" . js-jsx-mode)
("\\.[ds]?vh?\\'" . verilog-mode)
("\\.by\\'" . bovine-grammar-mode)
("\\.wy\\'" . wisent-grammar-mode)
diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index df2c41332e..0bba8159c1 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -574,10 +574,30 @@ js-chain-indent
:safe 'booleanp
:group 'js)
+(defcustom js-jsx-detect-syntax t
+ "When non-nil, automatically detect whether JavaScript uses JSX.
+`js-jsx-syntax' (which see) may be made buffer-local and set to
+t. The detection strategy can be customized by adding elements
+to `js-jsx-regexps', which see."
+ :version "27.1"
+ :type 'boolean
+ :safe 'booleanp
+ :group 'js)
+
(defcustom js-jsx-syntax nil
"When non-nil, parse JavaScript with consideration for JSX syntax.
-This fixes indentation of JSX code in some cases. It is set to
-be buffer-local when in `js-jsx-mode'."
+
+This enables proper font-locking and indentation of code using
+Facebook’s “JSX” syntax extension for JavaScript, for use with
+Facebook’s “React” library. Font-locking is like sgml-mode.
+Indentation is also like sgml-mode, although some indentation
+behavior may differ slightly to align more closely with the
+conventions of the React developer community.
+
+When `js-mode' is already enabled, you should call
+`js-jsx-enable' to set this variable.
+
+It is set to be buffer-local (and t) when in `js-jsx-mode'."
:version "27.1"
:type 'boolean
:safe 'booleanp
@@ -4223,6 +4243,79 @@ js--js-add-resource-alias
(when temp-name
(delete-file temp-name))))))
+;;; Syntax extensions
+
+(defvar js-syntactic-mode-name t
+ "If non-nil, print enabled syntaxes in the mode name.")
+
+(defun js--update-mode-name ()
+ "Print enabled syntaxes if `js-syntactic-mode-name' is t."
+ (when js-syntactic-mode-name
+ (setq mode-name (concat "JavaScript"
+ (if js-jsx-syntax "+JSX" "")))))
+
+(defun js--idly-update-mode-name ()
+ "Update `mode-name' whenever Emacs goes idle.
+In case `js-jsx-syntax' is updated, especially by features of
+Emacs like .dir-locals.el or file variables, this ensures the
+modeline eventually reflects which syntaxes are enabled."
+ (let (timer)
+ (setq timer
+ (run-with-idle-timer
+ 0 t
+ (lambda (buffer)
+ (if (buffer-live-p buffer)
+ (with-current-buffer buffer
+ (js--update-mode-name))
+ (cancel-timer timer)))
+ (current-buffer)))))
+
+(defun js-jsx-enable ()
+ "Enable JSX in the current buffer."
+ (interactive)
+ (setq-local js-jsx-syntax t)
+ (js--update-mode-name))
+
+(defvar js-jsx-regexps
+ (list "\\_<\\(?:var\\|let\\|const\\|import\\)\\_>.*?React")
+ "Regexps for detecting JSX in JavaScript buffers.
+When `js-jsx-detect-syntax' is non-nil and any of these regexps
+match text near the beginning of a JavaScript buffer,
+`js-jsx-syntax' (which see) will be made buffer-local and set to
+t.")
+
+(defun js-jsx--detect-and-enable (&optional arbitrarily)
+ "Detect if JSX is likely to be used, and enable it if so.
+Might make `js-jsx-syntax' buffer-local and set it to t. Matches
+from the beginning of the buffer, unless optional arg ARBITRARILY
+is non-nil. Return t after enabling, nil otherwise."
+ (when (or (and (buffer-file-name)
+ (string-match-p "\\.jsx\\'" (buffer-file-name)))
+ (and js-jsx-detect-syntax
+ (save-excursion
+ (unless arbitrarily
+ (goto-char (point-min)))
+ (catch 'match
+ (mapc
+ (lambda (regexp)
+ (if (re-search-forward regexp 4000 t) (throw 'match t)))
+ js-jsx-regexps)
+ nil))))
+ (js-jsx-enable)
+ t))
+
+(defun js-jsx--detect-after-change (beg end _len)
+ "Detect if JSX is likely to be used after a change.
+This function is intended for use in `after-change-functions'."
+ (when (<= end 4000)
+ (save-excursion
+ (goto-char beg)
+ (beginning-of-line)
+ (save-restriction
+ (narrow-to-region (point) end)
+ (when (js-jsx--detect-and-enable 'arbitrarily)
+ (remove-hook 'after-change-functions #'js-jsx--detect-after-change t))))))
+
;;; Main Function
;;;###autoload
@@ -4259,6 +4352,12 @@ js-mode
;; Frameworks
(js--update-quick-match-re)
+ ;; Syntax extensions
+ (unless (js-jsx--detect-and-enable)
+ (add-hook 'after-change-functions #'js-jsx--detect-after-change nil t))
+ (js--update-mode-name) ; If `js-jsx-syntax' was set from outside.
+ (js--idly-update-mode-name)
+
;; Imenu
(setq imenu-case-fold-search nil)
(setq imenu-create-index-function #'js--imenu-create-index)
@@ -4304,10 +4403,20 @@ js-mode
)
;;;###autoload
-(define-derived-mode js-jsx-mode js-mode "JSX"
- "Major mode for editing JSX."
+(define-derived-mode js-jsx-mode js-mode "JavaScript+JSX"
+ "Major mode for editing JavaScript+JSX.
+
+Simply makes `js-jsx-syntax' buffer-local and sets it to t.
+
+`js-mode' may detect and enable support for JSX automatically if
+it appears to be used in a JavaScript file. You could also
+customize `js-jsx-regexps' to improve that detection; or, you
+could set `js-jsx-syntax' to t in your init file, or in a
+.dir-locals.el file, or using file variables; or, you could call
+`js-jsx-enable' in `js-mode-hook'. You may be better served by
+one of the aforementioned options instead of using this mode."
:group 'js
- (setq-local js-jsx-syntax t))
+ (js-jsx-enable))
;;;###autoload (defalias 'javascript-mode 'js-mode)
--
2.11.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #14: 0013-Improve-JSX-syntax-propertization.patch --]
[-- Type: text/x-patch; name="0013-Improve-JSX-syntax-propertization.patch", Size: 7240 bytes --]
From d3710261f53aa72d018c873761592ad4d2185bc5 Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Sun, 24 Mar 2019 09:55:14 -0700
Subject: [PATCH 13/19] Improve JSX syntax propertization
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* lisp/progmodes/js.el (js-jsx--attribute-name-re): New variable.
(js-jsx--syntax-propertize-tag): Allow “-” in JSXAttribute names. Fix
“out of range” error when typing at the end of a buffer. Fix/improve
future propertization of unfinished JSXBoundaryElements.
* test/manual/indent/js-jsx-unclosed-2.js: Add tests for allowed
characters in JSX.
---
lisp/progmodes/js.el | 74 +++++++++++++++++++--------------
test/manual/indent/js-jsx-unclosed-2.js | 8 ++++
2 files changed, 51 insertions(+), 31 deletions(-)
diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 0bba8159c1..5d87489b52 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -2083,11 +2083,15 @@ js-jsx--syntax-propertize-tag-text
(throw 'stop nil)))
(setq text-beg (point))))))
+(defconst js-jsx--attribute-name-re (concat js--name-start-re
+ "\\(?:\\s_\\|\\sw\\|-\\)*")
+ "Like `js--name-re', but matches “-” as well.")
+
(defun js-jsx--syntax-propertize-tag (end)
"Determine if a JSXBoundaryElement is before END and propertize it.
Disambiguate JSX from inequality operators and arrow functions by
testing for syntax only valid as JSX."
- (let ((tag-beg (1- (point))) (type 'open)
+ (let ((tag-beg (1- (point))) tag-end (type 'open)
name-beg name-match-data unambiguous
forward-sexp-function) ; Use Lisp version.
(catch 'stop
@@ -2127,46 +2131,54 @@ js-jsx--syntax-propertize-tag
;; figure out what type it actually is.
(if (eq type 'open) (setq type (if name-beg 'self-closing 'close)))
(forward-char))
- ((looking-at js--dotted-name-re)
- (if (not name-beg)
- (progn
- ;; Don’t match code like “if (i < await foo)”
- (if (js--unary-keyword-p (match-string 0)) (throw 'stop nil))
- ;; Save boundaries for later fontification after
- ;; unambiguously determining the code is JSX.
- (setq name-beg (match-beginning 0)
- name-match-data (match-data))
- (goto-char (match-end 0)))
- (setq unambiguous t) ; Non-unary name followed by 2nd name ⇒ JSX
- ;; Save JSXAttribute’s name’s match data for font-locking later.
- (put-text-property (match-beginning 0) (1+ (match-beginning 0))
- 'js-jsx-attribute-name (match-data))
- (goto-char (match-end 0))
+ ((and (not name-beg) (looking-at js--dotted-name-re))
+ ;; Don’t match code like “if (i < await foo)”
+ (if (js--unary-keyword-p (match-string 0)) (throw 'stop nil))
+ ;; Save boundaries for later fontification after
+ ;; unambiguously determining the code is JSX.
+ (setq name-beg (match-beginning 0)
+ name-match-data (match-data))
+ (goto-char (match-end 0)))
+ ((and name-beg (looking-at js-jsx--attribute-name-re))
+ (setq unambiguous t) ; Non-unary name followed by 2nd name ⇒ JSX
+ ;; Save JSXAttribute’s name’s match data for font-locking later.
+ (put-text-property (match-beginning 0) (1+ (match-beginning 0))
+ 'js-jsx-attribute-name (match-data))
+ (goto-char (match-end 0))
+ (if (>= (point) end) (throw 'stop nil))
+ (skip-chars-forward " \t\n" end)
+ (if (>= (point) end) (throw 'stop nil))
+ ;; “=” is optional for null-valued JSXAttributes.
+ (when (= (char-after) ?=)
+ (forward-char)
(if (>= (point) end) (throw 'stop nil))
(skip-chars-forward " \t\n" end)
(if (>= (point) end) (throw 'stop nil))
- ;; “=” is optional for null-valued JSXAttributes.
- (when (= (char-after) ?=)
- (forward-char)
- (if (>= (point) end) (throw 'stop nil))
- (skip-chars-forward " \t\n" end)
- (if (>= (point) end) (throw 'stop nil))
- ;; Skip over strings (if possible). Any
- ;; JSXExpressionContainer here will be parsed in the
- ;; next iteration of the loop.
- (when (memq (char-after) '(?\" ?\' ?\`))
- (condition-case nil
- (forward-sexp)
- (scan-error (throw 'stop nil)))))))
+ ;; Skip over strings (if possible). Any
+ ;; JSXExpressionContainer here will be parsed in the
+ ;; next iteration of the loop.
+ (when (memq (char-after) '(?\" ?\' ?\`))
+ (condition-case nil
+ (forward-sexp)
+ (scan-error (throw 'stop nil))))))
;; There is nothing more to check; this either isn’t JSX, or
;; the tag is incomplete.
(t (throw 'stop nil)))))
(when unambiguous
;; Save JSXBoundaryElement’s name’s match data for font-locking.
(if name-beg (put-text-property name-beg (1+ name-beg) 'js-jsx-tag-name name-match-data))
+ ;; Prevent “out of range” errors when typing at the end of a buffer.
+ (setq tag-end (if (eobp) (1- (point)) (point)))
;; Mark beginning and end of tag for font-locking.
- (put-text-property tag-beg (1+ tag-beg) 'js-jsx-tag-beg (cons type (point)))
- (put-text-property (point) (1+ (point)) 'js-jsx-tag-end tag-beg))
+ (put-text-property tag-beg (1+ tag-beg) 'js-jsx-tag-beg (cons type tag-end))
+ (put-text-property tag-end (1+ tag-end) 'js-jsx-tag-end tag-beg)
+ ;; Use text properties to extend the syntax-propertize region
+ ;; backward to the beginning of the JSXBoundaryElement in the
+ ;; future. Typically the closing angle bracket could suggest
+ ;; extending backward, but that would also involve more rigorous
+ ;; parsing, and the closing angle bracket may not even exist yet
+ ;; if the JSXBoundaryElement is still being typed.
+ (put-text-property tag-beg (1+ tag-end) 'syntax-multiline t))
(if (js-jsx--at-enclosing-tag-child-p) (js-jsx--syntax-propertize-tag-text end))))
(defconst js-jsx--text-properties
diff --git a/test/manual/indent/js-jsx-unclosed-2.js b/test/manual/indent/js-jsx-unclosed-2.js
index 8b6f33325d..843ef9b6a8 100644
--- a/test/manual/indent/js-jsx-unclosed-2.js
+++ b/test/manual/indent/js-jsx-unclosed-2.js
@@ -29,3 +29,11 @@ while (await foo > bar) void 0
</Baz>
</Bar>
</Foo>
+
+// “-” is not allowed in a JSXBoundaryElement’s name.
+<ABC />
+ <A-B-C /> // Weirdly-indented “continued expression.”
+
+// “-” may be used in a JSXAttribute’s name.
+<Foo a-b-c=""
+ x-y-z="" />
--
2.11.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #15: 0014-Rename-tests-to-use-the-.jsx-file-extension.patch --]
[-- Type: text/x-patch; name="0014-Rename-tests-to-use-the-.jsx-file-extension.patch", Size: 3500 bytes --]
From f1685dc9eb44fcc6371374ba7dd89bd24213d234 Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Sun, 24 Mar 2019 10:05:28 -0700
Subject: [PATCH 14/19] =?UTF-8?q?Rename=20tests=20to=20use=20the=20?=
=?UTF-8?q?=E2=80=9C.jsx=E2=80=9D=20file=20extension?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* test/manual/indent/js-jsx-quote.js: Renamed to “jsx-quote.jsx”.
* test/manual/indent/js-jsx-unclosed-1.js: Renamed to
“jsx-unclosed-1.jsx”.
* test/manual/indent/js-jsx-unclosed-2.js: Renamed to
“jsx-unclosed-2.jsx”.
* test/manual/indent/js-jsx.js: Renamed to “jsx.jsx”.
* test/manual/indent/jsx-quote.jsx: Renamed from “js-jsx-quote.js”.
* test/manual/indent/jsx-unclosed-1.jsx: Renamed from
“js-jsx-unclosed-1.js”.
* test/manual/indent/jsx-unclosed-2.jsx: Renamed from
“js-jsx-unclosed-2.js”.
* test/manual/indent/jsx.jsx: Renamed from “js-jsx.js”.
---
test/manual/indent/{js-jsx-quote.js => jsx-quote.jsx} | 2 --
test/manual/indent/{js-jsx-unclosed-1.js => jsx-unclosed-1.jsx} | 2 --
test/manual/indent/{js-jsx-unclosed-2.js => jsx-unclosed-2.jsx} | 2 --
test/manual/indent/{js-jsx.js => jsx.jsx} | 2 --
4 files changed, 8 deletions(-)
rename test/manual/indent/{js-jsx-quote.js => jsx-quote.jsx} (95%)
rename test/manual/indent/{js-jsx-unclosed-1.js => jsx-unclosed-1.jsx} (91%)
rename test/manual/indent/{js-jsx-unclosed-2.js => jsx-unclosed-2.jsx} (97%)
rename test/manual/indent/{js-jsx.js => jsx.jsx} (99%)
diff --git a/test/manual/indent/js-jsx-quote.js b/test/manual/indent/jsx-quote.jsx
similarity index 95%
rename from test/manual/indent/js-jsx-quote.js
rename to test/manual/indent/jsx-quote.jsx
index 4b71a65674..1b2c652873 100644
--- a/test/manual/indent/js-jsx-quote.js
+++ b/test/manual/indent/jsx-quote.jsx
@@ -1,5 +1,3 @@
-// -*- mode: js-jsx; -*-
-
// JSX text node values should be strings, but only JS string syntax
// is considered, so quote marks delimit strings like normal, with
// disastrous results (https://github.com/mooz/js2-mode/issues/409).
diff --git a/test/manual/indent/js-jsx-unclosed-1.js b/test/manual/indent/jsx-unclosed-1.jsx
similarity index 91%
rename from test/manual/indent/js-jsx-unclosed-1.js
rename to test/manual/indent/jsx-unclosed-1.jsx
index 9418aed7a1..1f5c3fba8d 100644
--- a/test/manual/indent/js-jsx-unclosed-1.js
+++ b/test/manual/indent/jsx-unclosed-1.jsx
@@ -1,5 +1,3 @@
-// -*- mode: js-jsx; -*-
-
// Local Variables:
// indent-tabs-mode: nil
// js-indent-level: 2
diff --git a/test/manual/indent/js-jsx-unclosed-2.js b/test/manual/indent/jsx-unclosed-2.jsx
similarity index 97%
rename from test/manual/indent/js-jsx-unclosed-2.js
rename to test/manual/indent/jsx-unclosed-2.jsx
index 843ef9b6a8..8db25aa67f 100644
--- a/test/manual/indent/js-jsx-unclosed-2.js
+++ b/test/manual/indent/jsx-unclosed-2.jsx
@@ -1,5 +1,3 @@
-// -*- mode: js-jsx; -*-
-
// Local Variables:
// indent-tabs-mode: nil
// js-indent-level: 2
diff --git a/test/manual/indent/js-jsx.js b/test/manual/indent/jsx.jsx
similarity index 99%
rename from test/manual/indent/js-jsx.js
rename to test/manual/indent/jsx.jsx
index 2ec00c63bb..c2351a8cf1 100644
--- a/test/manual/indent/js-jsx.js
+++ b/test/manual/indent/jsx.jsx
@@ -1,5 +1,3 @@
-// -*- mode: js-jsx; -*-
-
var foo = <div></div>;
return (
--
2.11.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #16: 0015-Indent-broken-arrow-function-bodies-as-an-N-1th-arg.patch --]
[-- Type: text/x-patch; name="0015-Indent-broken-arrow-function-bodies-as-an-N-1th-arg.patch", Size: 3544 bytes --]
From 126ac966a632fb4cb67a2033e9adc8528e6b269c Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Sun, 24 Mar 2019 13:17:12 -0700
Subject: [PATCH 15/19] Indent broken arrow function bodies as an N+1th arg
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* lisp/progmodes/js.el (js--line-terminating-arrow-re): Revise regexp
for use with re-search-backward.
(js--looking-at-broken-arrow-function-p): Remove.
(js--broken-arrow-terminates-line-p): Replacement for
js--looking-at-broken-arrow-function-p. Don’t consider whether an
arrow appears at point (in an arglist); instead, just look for an
arrow that terminates the line.
(js--proper-indentation): Use js--broken-arrow-terminates-line-p.
* test/manual/indent/js.js: Add test for a broken arrow as an N+1th
arg.
---
lisp/progmodes/js.el | 22 ++++++++--------------
test/manual/indent/js.js | 5 +++++
2 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 5d87489b52..f8dd72c22b 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -2550,23 +2550,17 @@ js--maybe-goto-declaration-keyword-end
(when comma-p
(goto-char (1+ declaration-keyword-end))))))))
-(defconst js--line-terminating-arrow-re "\\s-*=>\\s-*\\(/[/*]\\|$\\)"
+(defconst js--line-terminating-arrow-re "=>\\s-*\\(/[/*]\\|$\\)"
"Regexp matching the last \"=>\" (arrow) token on a line.
Whitespace and comments around the arrow are ignored.")
-(defun js--looking-at-broken-arrow-function-p ()
+(defun js--broken-arrow-terminates-line-p ()
"Helper function for `js--proper-indentation'.
-Return t if point is at the start of a (possibly async) arrow
-function and the last non-comment, non-whitespace token of the
-current line is the \"=>\" token."
- (when (looking-at "\\s-*async\\s-*")
- (goto-char (match-end 0)))
- (cond
- ((eq (char-after) ?\()
- (forward-list)
- (looking-at-p js--line-terminating-arrow-re))
- (t (looking-at-p
- (concat js--name-re js--line-terminating-arrow-re)))))
+Return t if the last non-comment, non-whitespace token of the
+current line is the \"=>\" token (of an arrow function)."
+ (let ((from (point)))
+ (end-of-line)
+ (re-search-backward js--line-terminating-arrow-re from t)))
(defun js-jsx--context ()
"Determine JSX context and move to enclosing JSX."
@@ -2713,7 +2707,7 @@ js--proper-indentation
(goto-char (nth 1 parse-status)) ; go to the opening char
(if (or (not js-indent-align-list-continuation)
(looking-at "[({[]\\s-*\\(/[/*]\\|$\\)")
- (save-excursion (forward-char) (js--looking-at-broken-arrow-function-p)))
+ (save-excursion (forward-char) (js--broken-arrow-terminates-line-p)))
(progn ; nothing following the opening paren/bracket
(skip-syntax-backward " ")
(when (eq (char-before) ?\)) (backward-list))
diff --git a/test/manual/indent/js.js b/test/manual/indent/js.js
index 647d7438f4..9658c95701 100644
--- a/test/manual/indent/js.js
+++ b/test/manual/indent/js.js
@@ -160,6 +160,11 @@ foo.bar.baz(very => // A comment
snorf
);
+// Continuation of bug#25904; support broken arrow as N+1th arg
+map(arr, (val) =>
+ val
+)
+
// Local Variables:
// indent-tabs-mode: nil
// js-indent-level: 2
--
2.11.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #17: 0016-Fix-counting-of-nested-self-closing-JSXOpeningElemen.patch --]
[-- Type: text/x-patch; name="0016-Fix-counting-of-nested-self-closing-JSXOpeningElemen.patch", Size: 4499 bytes --]
From 9f73db7929cfe2501e21a1f268240a5227435342 Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Mon, 25 Mar 2019 20:39:48 -0700
Subject: [PATCH 16/19] Fix counting of nested self-closing JSXOpeningElements
* lisp/progmodes/js.el (js-jsx--matching-close-tag-pos): Fix bug where
self-closing JSXOpeningElements might be missed if one was nested
within another.
* test/manual/indent/jsx-self-closing.jsx: Add test for bug concerning
self-closing JSXOpeningElement counting.
---
lisp/progmodes/js.el | 39 ++++++++++++---------------------
test/manual/indent/jsx-self-closing.jsx | 13 +++++++++++
2 files changed, 27 insertions(+), 25 deletions(-)
create mode 100644 test/manual/indent/jsx-self-closing.jsx
diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index f8dd72c22b..f22c68cff9 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -1934,40 +1934,29 @@ js-jsx--matching-close-tag-pos
immediately before point, find a matching JSXClosingElement or
JSXClosingFragment, skipping over any nested JSXElements to find
the match. Return nil if a match can’t be found."
- (let ((tag-stack 1) self-closing-pos type)
+ (let ((tag-stack 1) type tag-pos last-pos pos)
(catch 'stop
(while (re-search-forward js-jsx--tag-re nil t)
- (setq type (js-jsx--matched-tag-type))
- ;; Balance the total of self-closing tags that we subtract
- ;; from the stack, ignoring those tags which are never added
- ;; to the stack (see below).
- (unless (eq type 'self-closing)
- (when (and self-closing-pos (> (point) self-closing-pos))
+ (setq type (js-jsx--matched-tag-type)
+ tag-pos (match-beginning 0))
+ ;; Clear the stack of any JSXOpeningElements which turned out
+ ;; to be self-closing.
+ (when last-pos
+ (setq pos (point))
+ (goto-char last-pos)
+ (while (re-search-forward js-jsx--self-closing-re pos 'move)
(setq tag-stack (1- tag-stack))))
(if (eq type 'close)
(progn
(setq tag-stack (1- tag-stack))
(when (= tag-stack 0)
- (throw 'stop (match-beginning 0))))
- ;; Tags that we know are self-closing aren’t added to the
- ;; stack at all, because we only close the ones that we have
- ;; anticipated after moving past those anticipated tags’
- ;; ends, and if a self-closing tag is the first tag we
- ;; encounter in this loop, then it will never be anticipated
- ;; (due to an optimization where we sometimes can avoid
- ;; looking for self-closing tags).
+ (throw 'stop tag-pos)))
+ ;; JSXOpeningElements that we know are self-closing aren’t
+ ;; added to the stack at all (since re-search-forward moves
+ ;; point after their self-closing syntax).
(unless (eq type 'self-closing)
(setq tag-stack (1+ tag-stack))))
- ;; Don’t needlessly recalculate.
- (unless (and self-closing-pos (<= (point) self-closing-pos))
- (setq self-closing-pos nil) ; Reset if recalculating.
- (save-excursion
- ;; Anticipate a self-closing tag that we should make sure
- ;; to subtract from the tag stack once we move past its
- ;; end; we might might miss the end otherwise, due to the
- ;; regexp-matching method we use to detect tags.
- (when (re-search-forward js-jsx--self-closing-re nil t)
- (setq self-closing-pos (match-beginning 0)))))))))
+ (setq last-pos (point))))))
(defun js-jsx--enclosing-curly-pos ()
"Return position of enclosing “{” in a “{/}” pair about point."
diff --git a/test/manual/indent/jsx-self-closing.jsx b/test/manual/indent/jsx-self-closing.jsx
new file mode 100644
index 0000000000..f8ea7a138a
--- /dev/null
+++ b/test/manual/indent/jsx-self-closing.jsx
@@ -0,0 +1,13 @@
+// Local Variables:
+// indent-tabs-mode: nil
+// js-indent-level: 2
+// End:
+
+// The following test goes below any comments to avoid including
+// misindented comments among the erroring lines.
+
+// Properly parse/indent code with a self-closing tag inside the
+// attribute of another self-closing tag.
+<div>
+ <div attr={() => <div attr="" />} />
+</div>
--
2.11.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #18: 0017-Indent-expressions-in-JSXAttributes-relative-to-the-.patch --]
[-- Type: text/x-patch; name="0017-Indent-expressions-in-JSXAttributes-relative-to-the-.patch", Size: 10396 bytes --]
From ee40eeb24ceb89e9bddebb94483c50f36d7facc0 Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Tue, 26 Mar 2019 18:18:39 -0700
Subject: [PATCH 17/19] =?UTF-8?q?Indent=20expressions=20in=20JSXAttributes?=
=?UTF-8?q?=20relative=20to=20the=20attribute=E2=80=99s=20name?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* lisp/progmodes/js.el (js-jsx--syntax-propertize-tag): Refer to the
beginning of a JSXExpressionContainer’s associated JSXAttribute (so
line numbers can be calculated later).
(js-jsx--text-properties): Also clear the new text property
js-jsx-expr-attribute.
(js-jsx--indenting): Remove.
(js-jsx--indent-col, js-jsx--indent-attribute-line): New variables.
(js-jsx--indentation): Instead of alternating between two separate
column calculations, neither necessarily correct, bind the JSX column
such that the second call to js--proper-indentation can use it as a
base column.
(js--proper-indentation): Use JSX as the base column for some indents
while indenting JSX.
* test/manual/indent/jsx.jsx: Add more tests for expression indents.
---
lisp/progmodes/js.el | 97 +++++++++++++++++++++++++++-------------------
test/manual/indent/jsx.jsx | 25 ++++++++++++
2 files changed, 83 insertions(+), 39 deletions(-)
diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index f22c68cff9..679633fc83 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -2081,7 +2081,7 @@ js-jsx--syntax-propertize-tag
Disambiguate JSX from inequality operators and arrow functions by
testing for syntax only valid as JSX."
(let ((tag-beg (1- (point))) tag-end (type 'open)
- name-beg name-match-data unambiguous
+ name-beg name-match-data expr-attribute-beg unambiguous
forward-sexp-function) ; Use Lisp version.
(catch 'stop
(while (and (< (point) end)
@@ -2096,8 +2096,16 @@ js-jsx--syntax-propertize-tag
;; JSXExpressionContainer as a JSXAttribute value
;; (“<Foo bar={…}”). Check this early in case continuing a
;; JSXAttribute parse.
- ((and name-beg (= (char-after) ?{))
+ ((or (and name-beg (= (char-after) ?{))
+ (setq expr-attribute-beg nil))
(setq unambiguous t) ; JSXExpressionContainer post tag name ⇒ JSX
+ (when expr-attribute-beg
+ ;; Remember that this JSXExpressionContainer is part of a
+ ;; JSXAttribute, as that can affect its expression’s
+ ;; indentation.
+ (put-text-property
+ (point) (1+ (point)) 'js-jsx-expr-attribute expr-attribute-beg)
+ (setq expr-attribute-beg nil))
(let (expr-end)
(condition-case nil
(save-excursion
@@ -2146,10 +2154,14 @@ js-jsx--syntax-propertize-tag
;; Skip over strings (if possible). Any
;; JSXExpressionContainer here will be parsed in the
;; next iteration of the loop.
- (when (memq (char-after) '(?\" ?\' ?\`))
- (condition-case nil
- (forward-sexp)
- (scan-error (throw 'stop nil))))))
+ (if (memq (char-after) '(?\" ?\' ?\`))
+ (condition-case nil
+ (forward-sexp)
+ (scan-error (throw 'stop nil)))
+ ;; Save JSXAttribute’s beginning in case we find a
+ ;; JSXExpressionContainer as the JSXAttribute’s value which
+ ;; we should associate with the JSXAttribute.
+ (setq expr-attribute-beg (match-beginning 0)))))
;; There is nothing more to check; this either isn’t JSX, or
;; the tag is incomplete.
(t (throw 'stop nil)))))
@@ -2174,7 +2186,7 @@ js-jsx--text-properties
(list
'js-jsx-tag-beg nil 'js-jsx-tag-end nil
'js-jsx-tag-name nil 'js-jsx-attribute-name nil
- 'js-jsx-text nil 'js-jsx-expr nil)
+ 'js-jsx-text nil 'js-jsx-expr nil 'js-jsx-expr-attribute nil)
"Plist of text properties added by `js-syntax-propertize'.")
(defun js-syntax-propertize (start end)
@@ -2563,8 +2575,11 @@ js-jsx--context
(list 'tag (nth 0 enclosing-tag-pos) (nth 1 enclosing-tag-pos)))
(list 'text (nth 0 enclosing-tag-pos) (nth 2 enclosing-tag-pos))))))
-(defvar js-jsx--indenting nil
- "Flag to prevent infinite recursion while indenting JSX.")
+(defvar js-jsx--indent-col nil
+ "Baseline column for JS indentation within JSX.")
+
+(defvar js-jsx--indent-attribute-line nil
+ "Line relative to which indentation uses JSX as a baseline.")
(defun js-jsx--indentation (parse-status)
"Helper function for `js--proper-indentation'.
@@ -2642,25 +2657,22 @@ js-jsx--indentation
0)))
)))
- ;; When indenting a JSXExpressionContainer expression, use JSX
- ;; indentation as a minimum, and use regular JS indentation if
- ;; it’s deeper.
+ ;; To indent a JSXExpressionContainer’s expression, calculate
+ ;; the JS indentation, possibly using JSX indentation as the
+ ;; base column.
(if expr-p
- (max (+ col
- ;; An expression in a JSXExpressionContainer in a
- ;; JSXAttribute should be indented more, except on
- ;; the ending line of the JSXExpressionContainer.
- (if (and (eq (nth 0 context) 'tag)
- (< current-line
- (save-excursion
- (js-jsx--goto-outermost-enclosing-curly
- (nth 1 context))
- (forward-sexp)
- (line-number-at-pos))))
- js-indent-level
- 0))
- (let ((js-jsx--indenting t)) ; Prevent recursion.
- (js--proper-indentation parse-status)))
+ (let* ((js-jsx--indent-col col)
+ (expr-attribute-pos
+ (save-excursion
+ (goto-char curly-pos) ; Skip first curly.
+ ;; Skip any remaining enclosing curlies up until
+ ;; the contextual JSXElement’s beginning position.
+ (js-jsx--goto-outermost-enclosing-curly (nth 1 context))
+ (get-text-property (point) 'js-jsx-expr-attribute)))
+ (js-jsx--indent-attribute-line
+ (when expr-attribute-pos
+ (line-number-at-pos expr-attribute-pos))))
+ (js--proper-indentation parse-status))
col))))
(defun js--proper-indentation (parse-status)
@@ -2670,7 +2682,7 @@ js--proper-indentation
(cond ((nth 4 parse-status) ; inside comment
(js--get-c-offset 'c (nth 8 parse-status)))
((nth 3 parse-status) 0) ; inside string
- ((when (and js-jsx-syntax (not js-jsx--indenting))
+ ((when (and js-jsx-syntax (not js-jsx--indent-col))
(save-excursion (js-jsx--indentation parse-status))))
((eq (char-after) ?#) 0)
((save-excursion (js--beginning-of-macro)) 4)
@@ -2708,17 +2720,24 @@ js--proper-indentation
(and switch-keyword-p
in-switch-p)))
(indent
- (cond (same-indent-p
- (current-column))
- (continued-expr-p
- (+ (current-column) (* 2 js-indent-level)
- js-expr-indent-offset))
- (t
- (+ (current-column) js-indent-level
- (pcase (char-after (nth 1 parse-status))
- (?\( js-paren-indent-offset)
- (?\[ js-square-indent-offset)
- (?\{ js-curly-indent-offset)))))))
+ (+
+ (cond
+ ((and js-jsx--indent-attribute-line
+ (eq js-jsx--indent-attribute-line
+ (line-number-at-pos)))
+ js-jsx--indent-col)
+ (t
+ (current-column)))
+ (cond (same-indent-p 0)
+ (continued-expr-p
+ (+ (* 2 js-indent-level)
+ js-expr-indent-offset))
+ (t
+ (+ js-indent-level
+ (pcase (char-after (nth 1 parse-status))
+ (?\( js-paren-indent-offset)
+ (?\[ js-square-indent-offset)
+ (?\{ js-curly-indent-offset))))))))
(if in-switch-p
(+ indent js-switch-indent-offset)
indent)))
diff --git a/test/manual/indent/jsx.jsx b/test/manual/indent/jsx.jsx
index c2351a8cf1..5004d57a0b 100644
--- a/test/manual/indent/jsx.jsx
+++ b/test/manual/indent/jsx.jsx
@@ -68,6 +68,31 @@ return (
</div>
);
+return (
+ <div attribute={{
+ a: 1, // Indent relative to “attribute” column.
+ b: 2
+ } && { // Dedent to “attribute” column.
+ a: 1,
+ b: 2
+ }} /> // Also dedent.
+);
+
+return (
+ <div attribute=
+ { // Indent properly on another line, too.
+ {
+ a: 1,
+ b: 2,
+ } && (
+ // Indent other forms, too.
+ a ? b :
+ c ? d :
+ e
+ )
+ } />
+)
+
// Indent void expressions (no need for contextual parens / commas)
// (https://github.com/mooz/js2-mode/issues/140#issuecomment-166250016).
<div className="class-name">
--
2.11.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #19: 0018-Split-JSX-indentation-calculation-into-several-funct.patch --]
[-- Type: text/x-patch; name="0018-Split-JSX-indentation-calculation-into-several-funct.patch", Size: 8665 bytes --]
From 0aea38556e7c132dd9b0b15f534f60845e15d465 Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Tue, 26 Mar 2019 20:14:46 -0700
Subject: [PATCH 18/19] Split JSX indentation calculation into several
functions
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* lisp/progmodes/js.el (js-jsx--contextual-indentation)
(js-jsx--expr-attribute-pos, js-jsx--expr-indentation): Extract logic
from js-jsx--indentation, and improve the logic’s documentation.
(js-jsx--indentation): Simplify by splitting into several
functions (see above) and improve the logic’s documentation.
---
lisp/progmodes/js.el | 146 ++++++++++++++++++++++++++++-----------------------
1 file changed, 81 insertions(+), 65 deletions(-)
diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 679633fc83..2d29d4e443 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -2575,12 +2575,86 @@ js-jsx--context
(list 'tag (nth 0 enclosing-tag-pos) (nth 1 enclosing-tag-pos)))
(list 'text (nth 0 enclosing-tag-pos) (nth 2 enclosing-tag-pos))))))
+(defun js-jsx--contextual-indentation (line context)
+ "Calculate indentation column for LINE from CONTEXT.
+The column calculation is based off of `sgml-calculate-indent'."
+ (pcase (nth 0 context)
+
+ ('string
+ ;; Go back to previous non-empty line.
+ (while (and (> (point) (nth 1 context))
+ (zerop (forward-line -1))
+ (looking-at "[ \t]*$")))
+ (if (> (point) (nth 1 context))
+ ;; Previous line is inside the string.
+ (current-indentation)
+ (goto-char (nth 1 context))
+ (1+ (current-column))))
+
+ ('tag
+ ;; Special JSX indentation rule: a “dangling” closing angle
+ ;; bracket on its own line is indented at the same level as the
+ ;; opening angle bracket of the JSXElement. Otherwise, indent
+ ;; JSXAttribute space like SGML.
+ (if (progn
+ (goto-char (nth 2 context))
+ (and (= line (line-number-at-pos))
+ (looking-back "^\\s-*/?>" (line-beginning-position))))
+ (progn
+ (goto-char (nth 1 context))
+ (current-column))
+ ;; Indent JSXAttribute space like SGML.
+ (goto-char (nth 1 context))
+ ;; Skip tag name:
+ (skip-chars-forward " \t")
+ (skip-chars-forward "^ \t\n")
+ (skip-chars-forward " \t")
+ (if (not (eolp))
+ (current-column)
+ ;; This is the first attribute: indent.
+ (goto-char (+ (nth 1 context) js-jsx-attribute-offset))
+ (+ (current-column) js-indent-level))))
+
+ ('text
+ ;; Indent to reflect nesting.
+ (goto-char (nth 1 context))
+ (+ (current-column)
+ ;; The last line isn’t nested, but the rest are.
+ (if (or (not (nth 2 context)) ; Unclosed.
+ (< line (line-number-at-pos (nth 2 context))))
+ js-indent-level
+ 0)))
+
+ ))
+
+(defun js-jsx--expr-attribute-pos (start limit)
+ "Look back from START to LIMIT for a JSXAttribute."
+ (save-excursion
+ (goto-char start) ; Skip the first curly.
+ ;; Skip any remaining enclosing curlies until the JSXElement’s
+ ;; beginning position; the last curly ought to be one of a
+ ;; JSXExpressionContainer, which may refer to its JSXAttribute’s
+ ;; beginning position (if it has one).
+ (js-jsx--goto-outermost-enclosing-curly limit)
+ (get-text-property (point) 'js-jsx-expr-attribute)))
+
(defvar js-jsx--indent-col nil
"Baseline column for JS indentation within JSX.")
(defvar js-jsx--indent-attribute-line nil
"Line relative to which indentation uses JSX as a baseline.")
+(defun js-jsx--expr-indentation (parse-status pos col)
+ "Indent using PARSE-STATUS; relative to POS, use base COL.
+To indent a JSXExpressionContainer’s expression, calculate the JS
+indentation, using JSX indentation as the base column when
+indenting relative to the beginning line of the
+JSXExpressionContainer’s JSXAttribute (if any)."
+ (let* ((js-jsx--indent-col col)
+ (js-jsx--indent-attribute-line
+ (if pos (line-number-at-pos pos))))
+ (js--proper-indentation parse-status)))
+
(defun js-jsx--indentation (parse-status)
"Helper function for `js--proper-indentation'.
Return the proper indentation of the current line if it is part
@@ -2605,74 +2679,16 @@ js-jsx--indentation
(and
(= beg-line current-line)
(or (not curly-pos) (> (point) curly-pos)))))))
+ ;; When on the second or later line of JSX, indent as JSX,
+ ;; possibly switching back to JS indentation within
+ ;; JSXExpressionContainers, possibly using the JSX as a base
+ ;; column while switching back to JS indentation.
(when (and context (> current-line beg-line))
(save-excursion
- ;; The column calculation is based on `sgml-calculate-indent'.
- (setq col (pcase (nth 0 context)
-
- ('string
- ;; Go back to previous non-empty line.
- (while (and (> (point) (nth 1 context))
- (zerop (forward-line -1))
- (looking-at "[ \t]*$")))
- (if (> (point) (nth 1 context))
- ;; Previous line is inside the string.
- (current-indentation)
- (goto-char (nth 1 context))
- (1+ (current-column))))
-
- ('tag
- ;; Special JSX indentation rule: a “dangling”
- ;; closing angle bracket on its own line is
- ;; indented at the same level as the opening
- ;; angle bracket of the JSXElement. Otherwise,
- ;; indent JSXAttribute space like SGML.
- (if (progn
- (goto-char (nth 2 context))
- (and (= current-line (line-number-at-pos))
- (looking-back "^\\s-*/?>" (line-beginning-position))))
- (progn
- (goto-char (nth 1 context))
- (current-column))
- ;; Indent JSXAttribute space like SGML.
- (goto-char (nth 1 context))
- ;; Skip tag name:
- (skip-chars-forward " \t")
- (skip-chars-forward "^ \t\n")
- (skip-chars-forward " \t")
- (if (not (eolp))
- (current-column)
- ;; This is the first attribute: indent.
- (goto-char (+ (nth 1 context) js-jsx-attribute-offset))
- (+ (current-column) js-indent-level))))
-
- ('text
- ;; Indent to reflect nesting.
- (goto-char (nth 1 context))
- (+ (current-column)
- ;; The last line isn’t nested, but the rest are.
- (if (or (not (nth 2 context)) ; Unclosed.
- (< current-line (line-number-at-pos (nth 2 context))))
- js-indent-level
- 0)))
-
- )))
- ;; To indent a JSXExpressionContainer’s expression, calculate
- ;; the JS indentation, possibly using JSX indentation as the
- ;; base column.
+ (setq col (js-jsx--contextual-indentation current-line context)))
(if expr-p
- (let* ((js-jsx--indent-col col)
- (expr-attribute-pos
- (save-excursion
- (goto-char curly-pos) ; Skip first curly.
- ;; Skip any remaining enclosing curlies up until
- ;; the contextual JSXElement’s beginning position.
- (js-jsx--goto-outermost-enclosing-curly (nth 1 context))
- (get-text-property (point) 'js-jsx-expr-attribute)))
- (js-jsx--indent-attribute-line
- (when expr-attribute-pos
- (line-number-at-pos expr-attribute-pos))))
- (js--proper-indentation parse-status))
+ (js-jsx--expr-indentation
+ parse-status (js-jsx--expr-attribute-pos curly-pos (nth 1 context)) col)
col))))
(defun js--proper-indentation (parse-status)
--
2.11.0
[-- Attachment #20: 0019-Add-tests-for-miscellaneous-JSX-parsing-feats.patch --]
[-- Type: text/x-patch, Size: 1263 bytes --]
From d986e2ef8cf6077bce60ffc06259e73ad75fb6de Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Tue, 26 Mar 2019 21:47:34 -0700
Subject: [PATCH 19/19] Add tests for miscellaneous JSX parsing feats
* test/manual/indent/jsx.jsx: Add tests for JSXMemberExpression names
and JSXOpeningFragment/JSXClosingFragment support (already supported).
---
test/manual/indent/jsx.jsx | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/test/manual/indent/jsx.jsx b/test/manual/indent/jsx.jsx
index 5004d57a0b..c200979df8 100644
--- a/test/manual/indent/jsx.jsx
+++ b/test/manual/indent/jsx.jsx
@@ -93,6 +93,32 @@ return (
} />
)
+// JSXMemberExpression names are parsed/indented:
+<Foo.Bar>
+ <div>
+ <Foo.Bar>
+ Hello World!
+ </Foo.Bar>
+ <Foo.Bar>
+ <div>
+ </div>
+ </Foo.Bar>
+ </div>
+</Foo.Bar>
+
+// JSXOpeningFragment and JSXClosingFragment are parsed/indented:
+<>
+ <div>
+ <>
+ Hello World!
+ </>
+ <>
+ <div>
+ </div>
+ </>
+ </div>
+</>
+
// Indent void expressions (no need for contextual parens / commas)
// (https://github.com/mooz/js2-mode/issues/140#issuecomment-166250016).
<div className="class-name">
--
2.11.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-03-27 8:03 ` Jackson Ray Hamilton
@ 2019-03-27 9:36 ` Marcin Borkowski
2019-03-30 2:18 ` Jackson Ray Hamilton
2019-03-30 2:08 ` Jackson Ray Hamilton
2019-04-02 1:07 ` Dmitry Gutov
2 siblings, 1 reply; 33+ messages in thread
From: Marcin Borkowski @ 2019-03-27 9:36 UTC (permalink / raw)
To: Jackson Ray Hamilton; +Cc: dgutov, monnier, emacs-devel
Thanks Jackson,
I took the liberty of making a branch (feature/jsx, see
http://git.savannah.gnu.org/cgit/emacs.git/log/?h=feature/jsx) so that
people could test your work without having to manually apply almost 20
patches.
In fact, I am currently working on a heavily React-infested project, so
your timing was great!
Best,
--
Marcin Borkowski
http://mbork.pl
^ permalink raw reply [flat|nested] 33+ messages in thread
* Comprehensive JSX support in Emacs
2019-03-27 8:03 ` Jackson Ray Hamilton
2019-03-27 9:36 ` Marcin Borkowski
@ 2019-03-30 2:08 ` Jackson Ray Hamilton
2019-04-02 1:07 ` Dmitry Gutov
2 siblings, 0 replies; 33+ messages in thread
From: Jackson Ray Hamilton @ 2019-03-30 2:08 UTC (permalink / raw)
To: jackson; +Cc: emacs-devel
I tried to make my March 27th message one in reply to my message from
February, but a new thread seems to have been created instead. Maybe I
messed up the headers; hopefully that doesn’t happen with this message
either.
For the sake of continuity, here is a link to that thread:
http://lists.gnu.org/archive/html/emacs-devel/2019-02/msg00430.html
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-03-27 9:36 ` Marcin Borkowski
@ 2019-03-30 2:18 ` Jackson Ray Hamilton
2019-04-02 1:12 ` Dmitry Gutov
0 siblings, 1 reply; 33+ messages in thread
From: Jackson Ray Hamilton @ 2019-03-30 2:18 UTC (permalink / raw)
To: mbork; +Cc: emacs-devel
Hi Marcin,
Thanks for creating a branch, it makes sense to do that.
Do you know the workflow for managing branches in this repo? I suppose
we might occasionally rebase the branch on the latest version of master,
and I expect we’d do that again right before merging it? And I suppose
anyone else will be free to add commits to the branch now, too.
Regarding your project, that’s great (well, a great first test subject
perhaps :-) ). What’s the project if you’re at liberty to share?
Jackson
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-03-27 8:03 ` Jackson Ray Hamilton
2019-03-27 9:36 ` Marcin Borkowski
2019-03-30 2:08 ` Jackson Ray Hamilton
@ 2019-04-02 1:07 ` Dmitry Gutov
2019-04-02 11:23 ` Stefan Monnier
2019-04-06 15:55 ` Jackson Ray Hamilton
2 siblings, 2 replies; 33+ messages in thread
From: Dmitry Gutov @ 2019-04-02 1:07 UTC (permalink / raw)
To: Jackson Ray Hamilton; +Cc: monnier, emacs-devel
Hi Jackson!
On 27.03.2019 10:03, Jackson Ray Hamilton wrote:
> Following up on my long email from a month ago, wherein I announced my
> plan to fully support editing JSX in js-mode…
>
> After many mornings and evenings of hacking (and many moonlight-melted
> candlesticks, and many moments spent meandering and muttering), I’ve
> finally got the JavaScript+JSX implementation to a state where I feel
> it’s worth sharing again.
>
> I’ve added pretty comprehensive indentation and font-locking support,
> along with relatively simple automatic JSX detection support.
Thanks a lot!
I've checked out Marcin's branch and played with it a bit. To the
untrained eye, at least, things work quite well. So as far as I'm
concerned, you can just push the new code to master and continue
refining it there. Especially since you seem to add tests quite regularly.
I personally don't work with JSX (or even JavaScript much, these days),
so I hope others who do will provide more valuable feedback regarding
missing features and regressions. It's probably still worth it to get
the feature into master soon.
Do you have commit access?
> The indentation and font-locking code complimented each other well. In
> order to fix issues like the one with an unterminated quote inside JSX
> (https://github.com/mooz/js2-mode/issues/409) (which truly haunted me
> for the past years) I had to thoroughly parse the JSX code. The
> information I gained from parsing eventually led to the resolution of
> all the failing indentation test cases I compiled, and some new ones I
> added along the way.
:thumbsup:
> js-mode no longer relies on sgml-mode for any parsing or indentation.
> It’s all performed precisely according to the semantics of JSX now,
> borrowing just two blocks of code from sgml-mode as a basis for
> overlapping indentation logic. Also, the JSX code has pretty colors
> now.
The colors are very nice to have. Do you think this approach is portable
to js2-mode?
> I mentioned that a “js-syntax-extensions” list might have some esoteric
> advantage by its ordering resolving theoretical conflicts between
> multiple syntax extensions, but I figure 1) we can cross that bridge
> when we get there, and 2) if we did add more syntax extensions for JS
> /and/ those syntax extensions did partially conflict /and/ users wanted
> to use conflicting extensions simultaneously, perhaps it’d be better to
> add some booleans that would resolve such conflicts on a case-by-case
> basis, anyway.
Maybe, yes.
> Taking into consideration Dmitry’s ambivalence on deprecating
> js-jsx-mode and js-jsx-indent-line, I figured we could strike a
> compromise. We can keep these APIs without marking them obsolete.
*shrug* The way you've implemented jsx-jsx-mode is basically begging for
an obsoletion marker. I don't really mind, though.
> However, I’ve tried to make the automatic detection of JSX—and the
> viable alternatives to manually enabling JSX support—/so effective/ and
> numerous as to render these functions superfluous relics. The
> go-forward answer to “how to enable JSX support in Emacs?” should be
> “upgrade to Emacs 27—now it just works™”.
>
> I also addressed the spelling / naming items that Stefan brought up.
>
> I had a chance to play with the code in a few projects of mine over the
> past week, and generally it is working well for me—much better than
> before, in all the previously painful cases. However, there is a
> noticeable delay when editing some lines in my 1000-line monolith.jsx
> file,
This sounds a bit worrisome. Slow font-lock rules?
But since you're working on this already, just let us know if you hit
some strong obstacles there.
> (Note that the patch
> “0015-Indent-broken-arrow-function-bodies-as-an-N-1th-arg.patch” isn’t
> really related to the JSX feature. I was just editing some code in a
> project of mine, and found that this change provided more desirable
> behavior with the code I was writing then. A lot of my code using JSX
> also uses arrow functions, so this was a convenient patch for me to lump
> in.)
LGTM.
Thanks!
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-03-30 2:18 ` Jackson Ray Hamilton
@ 2019-04-02 1:12 ` Dmitry Gutov
2019-04-06 16:09 ` Jackson Ray Hamilton
0 siblings, 1 reply; 33+ messages in thread
From: Dmitry Gutov @ 2019-04-02 1:12 UTC (permalink / raw)
To: Jackson Ray Hamilton, mbork; +Cc: emacs-devel
On 30.03.2019 04:18, Jackson Ray Hamilton wrote:
> Do you know the workflow for managing branches in this repo? I suppose
> we might occasionally rebase the branch on the latest version of master,
> and I expect we’d do that again right before merging it?
Until the branch is ready for merging (and we respect is to be rebased,
amended, etc), we normally call it scratch/something-something.
force-pushing probably doesn't work, but deleting and re-pushing works
fine (and for scratch branches, it's okay).
> And I suppose
> anyone else will be free to add commits to the branch now, too.
I don't remember the last time that happened, though.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-04-02 1:07 ` Dmitry Gutov
@ 2019-04-02 11:23 ` Stefan Monnier
2019-04-06 16:02 ` Jackson Ray Hamilton
2019-04-09 6:06 ` Jackson Ray Hamilton
2019-04-06 15:55 ` Jackson Ray Hamilton
1 sibling, 2 replies; 33+ messages in thread
From: Stefan Monnier @ 2019-04-02 11:23 UTC (permalink / raw)
To: emacs-devel
>> However, there is a noticeable delay when editing some lines in my
>> 1000-line monolith.jsx file,
> This sounds a bit worrisome. Slow font-lock rules?
Regarding that: if you can provide a concrete example, maybe we can help
figure out where's the source of the performance issue and how to avoid it.
That reminds me: could you add a comment somewhere explaining what
you do in syntax-propertize-function vs what you do in font-lock
and inlining (some of those choices are completely obvious, but others
are much less so with non-obvious tradeoffs). And also why you decided
to do it this way, obviously.
E.g. have you considered marking the < and > in the JSX parts with
a parenthesis-syntax?
Stefan
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-04-02 1:07 ` Dmitry Gutov
2019-04-02 11:23 ` Stefan Monnier
@ 2019-04-06 15:55 ` Jackson Ray Hamilton
2019-04-07 0:31 ` Dmitry Gutov
1 sibling, 1 reply; 33+ messages in thread
From: Jackson Ray Hamilton @ 2019-04-06 15:55 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: monnier, emacs-devel
[-- Attachment #1: Type: text/plain, Size: 2315 bytes --]
Hi Dmitry,
First of all, thanks for taking the time to review my work. Regarding
your comments/questions…
> It's probably still worth it to get the feature into master soon.
I agree. I’ll see if I can tackle the performance issue first, then
I’ll rebase/merge the branch into master.
> Do you have commit access?
I’m not sure. If not, will someone please grant it to me?
> The colors are very nice to have. Do you think this approach is
> portable to js2-mode?
Possibly… js-mode is using font-lock keywords to add the colors, but
js2-mode apparently isn’t. js2-mode would need to pay attention to the
js-jsx-* text properties like in js-jsx--font-lock-keywords and apply
colors like in the associated matcher functions (js-jsx--match-tag-name,
js-jsx--match-attribute-name, et al).
I get the feeling from looking at the js2-mode code that it colors
solely according to the AST structure. Perhaps in the long run, the
best solution for that mode will be to extend or replace the ESX parsing
with JSX parsing, adding JSXElement nodes to the AST and coloring them
along the way, and suppressing JS coloring in the JSXText nodes.
Until someone steps up to add proper JSX parsing to the JS2 AST… perhaps
a fusion of the keywords-style and AST-style colorings could provide JSX
and JS coloring (respectively) in JS2.
>> However, there is a noticeable delay when editing some lines in my
>> 1000-line monolith.jsx file,
> This sounds a bit worrisome. Slow font-lock rules?
I’ll check for that. I’m also planning on analyzing the results of
elp-instrument-package and doing some step-debugging to deduce where and
why it’s lagging.
I’ve already found that it’s lagging when I’m editing a large chunk of
JSX code or the code before it. However, if I edit code in the buffer
following the chunk of JSX, then the lag noticeably drops. You can see
this by opening the attached “Slow JSX.jsx” file and editing it. This
might be related to how I extended syntax propertization and
font-locking to the boundaries of a root enclosing JSXElement; if a root
is hundreds of lines long, there could be negative consequences. Maybe
we can be more conservative with the parsing/coloring, somehow.
Jackson
[-- Attachment #2: Slow JSX.jsx --]
[-- Type: text/plain, Size: 13735 bytes --]
class Component {
render () {
return (
<div>Is it slow to edit this node? Very much for me.</div>
)
}
render () {
return (
<div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {5} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {10} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {15} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {20} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {25} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {30} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {35} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {40} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {45} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {50} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {55} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {60} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {65} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {70} things. Can we add more? {'That’s enough for now.'}</div>
<div>Is it slow to edit this node? Very much for me.</div>
</div>
)
}
render () {
return (
<div>Is it slow to edit this node? Not at all for me.</div>
)
}
}
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-04-02 11:23 ` Stefan Monnier
@ 2019-04-06 16:02 ` Jackson Ray Hamilton
2019-04-07 23:19 ` Jackson Ray Hamilton
2019-04-09 6:06 ` Jackson Ray Hamilton
1 sibling, 1 reply; 33+ messages in thread
From: Jackson Ray Hamilton @ 2019-04-06 16:02 UTC (permalink / raw)
To: monnier; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 1120 bytes --]
Hi Stefan,
Thanks for taking another look at my work. Regarding your message…
>>> However, there is a noticeable delay when editing some lines in my
>>> 1000-line monolith.jsx file,
>> This sounds a bit worrisome. Slow font-lock rules?
> Regarding that: if you can provide a concrete example, maybe we can help
> figure out where's the source of the performance issue and how to
> avoid it.
Sure, I’ve attached an example. If you try editing the three <div>
elements with the text “Is it slow to edit this node?” then you can
observe the performance issue with the first two elements.
> That reminds me: could you add a comment somewhere explaining what
> you do in syntax-propertize-function vs what you do in font-lock
> and inlining (some of those choices are completely obvious, but others
> are much less so with non-obvious tradeoffs). And also why you decided
> to do it this way, obviously.
Yes, I’ll do that.
> E.g. have you considered marking the < and > in the JSX parts with
> a parenthesis-syntax?
Good idea, I’ll see if I can make that work.
Jackson
[-- Attachment #2: Slow JSX.jsx --]
[-- Type: text/plain, Size: 13735 bytes --]
class Component {
render () {
return (
<div>Is it slow to edit this node? Very much for me.</div>
)
}
render () {
return (
<div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {5} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {10} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {15} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {20} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {25} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {30} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {35} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {40} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {45} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {50} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {55} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {60} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {65} things. Can we add more? {'Yes I think so.'}</div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div class="class" expr={funcall(() => {
stuff: 'more stuff',
lotsOfStuff: 'stuff is good, more stuff is better'
})}></div>
<div>That was {70} things. Can we add more? {'That’s enough for now.'}</div>
<div>Is it slow to edit this node? Very much for me.</div>
</div>
)
}
render () {
return (
<div>Is it slow to edit this node? Not at all for me.</div>
)
}
}
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-04-02 1:12 ` Dmitry Gutov
@ 2019-04-06 16:09 ` Jackson Ray Hamilton
0 siblings, 0 replies; 33+ messages in thread
From: Jackson Ray Hamilton @ 2019-04-06 16:09 UTC (permalink / raw)
To: Dmitry Gutov, mbork; +Cc: emacs-devel
>> Do you know the workflow for managing branches in this repo? I
>> suppose we might occasionally rebase the branch on the latest version
>> of master, and I expect we’d do that again right before merging it?
> Until the branch is ready for merging (and we respect is to be
> rebased, amended, etc), we normally call it
> scratch/something-something. force-pushing probably doesn't work, but
> deleting and re-pushing works fine (and for scratch branches, it's okay).
Got it. Let’s leave this branch name as “feature/jsx”, for continuity,
and since it seems almost ready for merging. For future features, I’ll
use scratch branches until they’re ready.
Jackson
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-04-06 15:55 ` Jackson Ray Hamilton
@ 2019-04-07 0:31 ` Dmitry Gutov
2019-04-09 6:16 ` Jackson Ray Hamilton
0 siblings, 1 reply; 33+ messages in thread
From: Dmitry Gutov @ 2019-04-07 0:31 UTC (permalink / raw)
To: Jackson Ray Hamilton; +Cc: monnier, emacs-devel
On 06.04.2019 18:55, Jackson Ray Hamilton wrote:
>> It's probably still worth it to get the feature into master soon.
> I agree. I’ll see if I can tackle the performance issue first, then
> I’ll rebase/merge the branch into master.
It might be worth it to merge even if the issue turns out to be too
difficult to deal with right away, though.
>> Do you have commit access?
> I’m not sure. If not, will someone please grant it to me?
You'll need to register at https://savannah.gnu.org/, find the 'emacs'
group and request access there (there's a button).
>> The colors are very nice to have. Do you think this approach is
>> portable to js2-mode?
> Possibly… js-mode is using font-lock keywords to add the colors, but
> js2-mode apparently isn’t. js2-mode would need to pay attention to the
> js-jsx-* text properties like in js-jsx--font-lock-keywords and apply
> colors like in the associated matcher functions (js-jsx--match-tag-name,
> js-jsx--match-attribute-name, et al).
*shrug*, font-lock-keywords also usually work there (e.g. when added
from a hook or a minor mode), so maybe the easiest approach would work too.
> Until someone steps up to add proper JSX parsing to the JS2 AST… perhaps
> a fusion of the keywords-style and AST-style colorings could provide JSX
> and JS coloring (respectively) in JS2.
Yup.
> I’ll check for that. I’m also planning on analyzing the results of
> elp-instrument-package and doing some step-debugging to deduce where and
> why it’s lagging.
>
> I’ve already found that it’s lagging when I’m editing a large chunk of
> JSX code or the code before it. However, if I edit code in the buffer
> following the chunk of JSX, then the lag noticeably drops. You can see
> this by opening the attached “Slow JSX.jsx” file and editing it. This
> might be related to how I extended syntax propertization and
> font-locking to the boundaries of a root enclosing JSXElement; if a root
> is hundreds of lines long, there could be negative consequences. Maybe
> we can be more conservative with the parsing/coloring, somehow.
I'll leave that investigation to you and Stefan, for now at least.
Thanks for working on it.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-04-06 16:02 ` Jackson Ray Hamilton
@ 2019-04-07 23:19 ` Jackson Ray Hamilton
0 siblings, 0 replies; 33+ messages in thread
From: Jackson Ray Hamilton @ 2019-04-07 23:19 UTC (permalink / raw)
To: jackson; +Cc: monnier, emacs-devel
I pushed a few changes (“Optimize js-jsx--matching-close-tag-pos”,
“Optimize js-jsx--enclosing-tag-pos”) which made a big difference in
performance.
I was running a function thousands of extra times redundantly. ELP
showed that by caching the results, I was able to cut the average
waiting time immediately after editing a line by 80%. This made editing
some code noticeably more bearable.
There might be some better algorithms that can improve the performance
even more, but it should be good enough (especially for non-monolithic,
well-modularized code) to no longer be a limiting factor for merging it.
Jackson
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-04-02 11:23 ` Stefan Monnier
2019-04-06 16:02 ` Jackson Ray Hamilton
@ 2019-04-09 6:06 ` Jackson Ray Hamilton
2019-04-09 8:12 ` Eli Zaretskii
1 sibling, 1 reply; 33+ messages in thread
From: Jackson Ray Hamilton @ 2019-04-09 6:06 UTC (permalink / raw)
To: monnier; +Cc: emacs-devel
Stefan,
I’ve pushed a commit adding a few comments to the code explaining what I
do in syntax-propertize-function and the rest (“Explain reasonings for
JSX syntax support design decisions”).
I also pushed a commit marking < and > in the JSX parts with parenthesis
syntax (“Add open/close parenthesis syntax to “<” and “>” in JSX”).
Jackson
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-04-07 0:31 ` Dmitry Gutov
@ 2019-04-09 6:16 ` Jackson Ray Hamilton
2019-04-09 7:10 ` Marcin Borkowski
0 siblings, 1 reply; 33+ messages in thread
From: Jackson Ray Hamilton @ 2019-04-09 6:16 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: monnier, emacs-devel
To all,
I’ve just rebased and merged feature/jsx into master, and deleted the
feature/jsx branch.
To Dmitry and Marcin,
Good news: I was able to successfully integrate all of the recent JSX
improvements into js2-mode. For Marcin, this will mean that he can soon
regain access to JS2 bouncing indentation — I briefly tested that, and
it seemed to work well. A PR will be submitted presently to js2-mode.
Jackson
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-04-09 6:16 ` Jackson Ray Hamilton
@ 2019-04-09 7:10 ` Marcin Borkowski
2019-04-09 8:00 ` Jackson Ray Hamilton
0 siblings, 1 reply; 33+ messages in thread
From: Marcin Borkowski @ 2019-04-09 7:10 UTC (permalink / raw)
To: Jackson Ray Hamilton; +Cc: emacs-devel, monnier, Dmitry Gutov
On 2019-04-09, at 08:16, Jackson Ray Hamilton <jackson@jacksonrayhamilton.com> wrote:
> To all,
>
> I’ve just rebased and merged feature/jsx into master, and deleted the
> feature/jsx branch.
>
> To Dmitry and Marcin,
>
> Good news: I was able to successfully integrate all of the recent JSX
> improvements into js2-mode. For Marcin, this will mean that he can
> soon regain access to JS2 bouncing indentation — I briefly tested
> that, and it seemed to work well. A PR will be submitted presently to
> js2-mode.
Wow, that is great! Does that mean that if I compile master today, your
JSX support will be working in js2-mode?
Best,
--
Marcin Borkowski
http://mbork.pl
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-04-09 7:10 ` Marcin Borkowski
@ 2019-04-09 8:00 ` Jackson Ray Hamilton
2019-04-11 19:51 ` Marcin Borkowski
0 siblings, 1 reply; 33+ messages in thread
From: Jackson Ray Hamilton @ 2019-04-09 8:00 UTC (permalink / raw)
To: Marcin Borkowski; +Cc: emacs-devel, monnier, Dmitry Gutov
Hi Marcin,
Yes! However, you will also need to use my changes to js2-mode:
https://github.com/mooz/js2-mode/pull/523
Jackson
On 4/9/19 12:10 AM, Marcin Borkowski wrote:
> On 2019-04-09, at 08:16, Jackson Ray Hamilton <jackson@jacksonrayhamilton.com> wrote:
>
>> To all,
>>
>> I’ve just rebased and merged feature/jsx into master, and deleted the
>> feature/jsx branch.
>>
>> To Dmitry and Marcin,
>>
>> Good news: I was able to successfully integrate all of the recent JSX
>> improvements into js2-mode. For Marcin, this will mean that he can
>> soon regain access to JS2 bouncing indentation — I briefly tested
>> that, and it seemed to work well. A PR will be submitted presently to
>> js2-mode.
> Wow, that is great! Does that mean that if I compile master today, your
> JSX support will be working in js2-mode?
>
> Best,
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-04-09 6:06 ` Jackson Ray Hamilton
@ 2019-04-09 8:12 ` Eli Zaretskii
2019-04-10 1:56 ` Jackson Ray Hamilton
0 siblings, 1 reply; 33+ messages in thread
From: Eli Zaretskii @ 2019-04-09 8:12 UTC (permalink / raw)
To: Jackson Ray Hamilton; +Cc: monnier, emacs-devel
> From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
> Date: Mon, 8 Apr 2019 23:06:25 -0700
> Cc: emacs-devel@gnu.org
>
> I’ve pushed a commit adding a few comments to the code explaining what I
> do in syntax-propertize-function and the rest (“Explain reasonings for
> JSX syntax support design decisions”).
Thanks. I've made minor changes in NEWS, but please look into fixing
this one:
*** Indentation uses 'js-indent-level' instead of 'sgml-basic-offset'.
It was never really intuitive that JSX indentation would be controlled
by an SGML variable. JSX is a syntax extension of JavaScript, so it
should be indented just like any other expression in JavaScript. This
is technically a breaking change, but it will probably align with how
you would normally expect for this indentation to be controlled, and
you probably won't need to change your config.
Backward-incompatible changes should have a way of reverting to old
behavior. If there is such a way already, please mention it in NEWS;
if there isn't, can we add a feature to revert to old behavior?
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-04-09 8:12 ` Eli Zaretskii
@ 2019-04-10 1:56 ` Jackson Ray Hamilton
2019-04-10 15:43 ` Eli Zaretskii
0 siblings, 1 reply; 33+ messages in thread
From: Jackson Ray Hamilton @ 2019-04-10 1:56 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: monnier, emacs-devel
Hi Eli,
I pushed a feature that should allow users to revert to the old behavior
(“Add new defcustom js-jsx-indent-level”) and I mentioned it in the
commit “* etc/NEWS: Document way to revert to old JSX indentation behavior”.
Jackson
On 4/9/19 1:12 AM, Eli Zaretskii wrote:
>> From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
>> Date: Mon, 8 Apr 2019 23:06:25 -0700
>> Cc: emacs-devel@gnu.org
>>
>> I’ve pushed a commit adding a few comments to the code explaining what I
>> do in syntax-propertize-function and the rest (“Explain reasonings for
>> JSX syntax support design decisions”).
> Thanks. I've made minor changes in NEWS, but please look into fixing
> this one:
>
> *** Indentation uses 'js-indent-level' instead of 'sgml-basic-offset'.
> It was never really intuitive that JSX indentation would be controlled
> by an SGML variable. JSX is a syntax extension of JavaScript, so it
> should be indented just like any other expression in JavaScript. This
> is technically a breaking change, but it will probably align with how
> you would normally expect for this indentation to be controlled, and
> you probably won't need to change your config.
>
> Backward-incompatible changes should have a way of reverting to old
> behavior. If there is such a way already, please mention it in NEWS;
> if there isn't, can we add a feature to revert to old behavior?
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-04-10 1:56 ` Jackson Ray Hamilton
@ 2019-04-10 15:43 ` Eli Zaretskii
0 siblings, 0 replies; 33+ messages in thread
From: Eli Zaretskii @ 2019-04-10 15:43 UTC (permalink / raw)
To: Jackson Ray Hamilton; +Cc: monnier, emacs-devel
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
> Date: Tue, 9 Apr 2019 18:56:00 -0700
>
> I pushed a feature that should allow users to revert to the old behavior
> (“Add new defcustom js-jsx-indent-level”) and I mentioned it in the
> commit “* etc/NEWS: Document way to revert to old JSX indentation behavior”.
Thanks.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-04-09 8:00 ` Jackson Ray Hamilton
@ 2019-04-11 19:51 ` Marcin Borkowski
2019-10-20 16:57 ` Steinar Bang
0 siblings, 1 reply; 33+ messages in thread
From: Marcin Borkowski @ 2019-04-11 19:51 UTC (permalink / raw)
To: Jackson Ray Hamilton; +Cc: emacs-devel, monnier, Dmitry Gutov
On 2019-04-09, at 10:00, Jackson Ray Hamilton <jackson@jacksonrayhamilton.com> wrote:
> Hi Marcin,
>
> Yes! However, you will also need to use my changes to js2-mode:
> https://github.com/mooz/js2-mode/pull/523
Thanks, I'll check this out!
Best,
--
Marcin Borkowski
http://mbork.pl
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: Comprehensive JSX support in Emacs
2019-04-11 19:51 ` Marcin Borkowski
@ 2019-10-20 16:57 ` Steinar Bang
0 siblings, 0 replies; 33+ messages in thread
From: Steinar Bang @ 2019-10-20 16:57 UTC (permalink / raw)
To: emacs-devel
>>>>> Marcin Borkowski <mbork@mbork.pl>:
> On 2019-04-09, at 10:00, Jackson Ray Hamilton <jackson@jacksonrayhamilton.com> wrote:
>> Yes! However, you will also need to use my changes to js2-mode:
>> https://github.com/mooz/js2-mode/pull/523
> Thanks, I'll check this out!
I tried to replace rjsx-mode with js2-mode today, using the melpa
package of js2-mode, but I discovered that the PR hadn't been accepted
into js2-mode. It had been split into multiple PRs where only a
documentation PR has been accepted in.
I added a comment to the original PR requesting that all of the PRs
would be accepted and a new release made to melpa
https://github.com/mooz/js2-mode/pull/523#issuecomment-544268471
^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2019-10-20 16:57 UTC | newest]
Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-14 5:06 Comprehensive JSX support in Emacs Jackson Ray Hamilton
2019-02-14 8:03 ` Marcin Borkowski
2019-02-14 14:10 ` Stefan Monnier
2019-02-14 15:04 ` Clément Pit-Claudel
2019-02-15 8:21 ` Jackson Ray Hamilton
2019-02-15 13:25 ` Stefan Monnier
2019-02-15 13:54 ` Dmitry Gutov
2019-02-15 14:39 ` Dmitry Gutov
2019-02-16 20:50 ` Jostein Kjønigsen
2019-02-18 7:17 ` Jackson Ray Hamilton
2019-03-27 8:03 ` Jackson Ray Hamilton
2019-03-27 9:36 ` Marcin Borkowski
2019-03-30 2:18 ` Jackson Ray Hamilton
2019-04-02 1:12 ` Dmitry Gutov
2019-04-06 16:09 ` Jackson Ray Hamilton
2019-03-30 2:08 ` Jackson Ray Hamilton
2019-04-02 1:07 ` Dmitry Gutov
2019-04-02 11:23 ` Stefan Monnier
2019-04-06 16:02 ` Jackson Ray Hamilton
2019-04-07 23:19 ` Jackson Ray Hamilton
2019-04-09 6:06 ` Jackson Ray Hamilton
2019-04-09 8:12 ` Eli Zaretskii
2019-04-10 1:56 ` Jackson Ray Hamilton
2019-04-10 15:43 ` Eli Zaretskii
2019-04-06 15:55 ` Jackson Ray Hamilton
2019-04-07 0:31 ` Dmitry Gutov
2019-04-09 6:16 ` Jackson Ray Hamilton
2019-04-09 7:10 ` Marcin Borkowski
2019-04-09 8:00 ` Jackson Ray Hamilton
2019-04-11 19:51 ` Marcin Borkowski
2019-10-20 16:57 ` Steinar Bang
-- strict thread matches above, loose matches on Subject: below --
2019-02-15 6:38 Jackson Ray Hamilton
2019-02-17 6:10 ` Marcin Borkowski
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).