GNU bug report logs - #24896
JSX prop indentation after fat arrow

Previous Next

Package: emacs;

Reported by: Felipe Ochoa <felipe.nospam.ochoa <at> gmail.com>

Date: Mon, 7 Nov 2016 16:35:02 UTC

Severity: minor

Merged with 26001, 30225, 32158

Found in versions 26.1, 27.0.50

Done: Dmitry Gutov <dgutov <at> yandex.ru>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 24896 in the body.
You can then email your comments to 24896 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#24896; Package emacs. (Mon, 07 Nov 2016 16:35:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Felipe Ochoa <felipe.nospam.ochoa <at> gmail.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Mon, 07 Nov 2016 16:35:03 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Felipe Ochoa <felipe.nospam.ochoa <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: JSX prop indentation after fat arrow
Date: Mon, 7 Nov 2016 10:56:21 +0100
[Message part 1 (text/plain, inline)]
(Preemptive apologies if this is the wrong list/format for this comment --
first time filer here!)

When indenting JSX code using js2- or js-mode, the indentation function
gets confused when there's a fat arrow function in a JSX prop. Compare the
way the following two code blocks are auto-indented:

const Component = props => ( // Incorrect indentation
    <FatArrow a={e => c}
      b={123}>
    </FatArrow>
);

const Component = props => ( // Correct indentation
    <NoFatArrow a={123}
                b={123}>
    </NoFatArrow>
);

I've tracked the problem down to `sgml-calculate-indent' using
`parse-partial-sexp' with `sgml-tag-syntax-table', where `>' is treated as
a close-parenthesis character (and thus the end-of-tag marker). I don't
think there's a way to patch the syntax table that would let `>' flip
between punctuation and close-parens based on context, but one possible fix
when using js2-mode (not sure about js-mode) is to apply a "."
'syntax-table text property to the `>' when parsing a fat arrow.

Unfortunately, `js-jsx-indent-line' calls `sgml-indent-line' using
`js--as-sgml', which sets `parse-sexp-lookup-properties' to nil.

Would there be any harm in setting `parse-sexp-lookup-properties' to t
instead? As far as I can tell, js-mode and js2-mode only use 'syntax-table
propeties for regex literals.


As a side-note, there may well be a different solution to this problem; I
still don't understand why the following block is indented correctly:

const Component = props => (
    <WithRegex a={/>/}
               b={123}>
    </WithRegex>
);
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24896; Package emacs. (Sat, 19 Nov 2016 22:48:01 GMT) Full text and rfc822 format available.

Message #8 received at 24896 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Felipe Ochoa <felipe.nospam.ochoa <at> gmail.com>, 24896 <at> debbugs.gnu.org,
 Jackson Hamilton <jackson <at> jacksonrayhamilton.com>
Subject: Re: bug#24896: JSX prop indentation after fat arrow
Date: Sun, 20 Nov 2016 00:47:20 +0200
Hi!

On 07.11.2016 11:56, Felipe Ochoa wrote:
> (Preemptive apologies if this is the wrong list/format for this comment
> -- first time filer here!)

Not at all, thanks for the report.

> I
> don't think there's a way to patch the syntax table that would let `>'
> flip between punctuation and close-parens based on context, but one
> possible fix when using js2-mode (not sure about js-mode)

I wonder what could be done in js-mode, too. A 
syntax-propertize-function rule, maybe.

> is to apply a
> "." 'syntax-table text property to the `>' when parsing a fat arrow.
>
> Unfortunately, `js-jsx-indent-line' calls `sgml-indent-line' using
> `js--as-sgml', which sets `parse-sexp-lookup-properties' to nil.
>
> Would there be any harm in setting `parse-sexp-lookup-properties' to t
> instead? As far as I can tell, js-mode and js2-mode only use
> 'syntax-table propeties for regex literals.

They also set that variable to t anyway. The only possible danger might 
come from sgml-mode, which does not do that.

I'm not sure which danger exactly, because all examples in 
test/indent/js-jsx.js seem to behave identically whether js--as-sgml 
includes the parse-sexp-lookup-properties binding or not.

Jackson, could you maybe shed some light on this?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24896; Package emacs. (Tue, 22 Nov 2016 05:51:02 GMT) Full text and rfc822 format available.

Message #11 received at 24896 <at> debbugs.gnu.org (full text, mbox):

From: Jackson Ray Hamilton <jackson <at> jacksonrayhamilton.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>,
 Felipe Ochoa <felipe.nospam.ochoa <at> gmail.com>, 24896 <at> debbugs.gnu.org
Subject: Re: bug#24896: JSX prop indentation after fat arrow
Date: Mon, 21 Nov 2016 21:48:45 -0800
Hi guys,

Sorry that I cannot provide a definitive answer for the purpose of that
line of code - I probably should have provided a more detailed comment
or committed in smaller hunks - but I'll make a guess informed by the
way I usually think, and thus probably thought, when I wrote that.

When figuring out how to get `sgml-indent-line' to behave correctly, and
upon discovering that I needed to use `with-syntax-table', I probably
read the manual entry on syntax tables and discovered the existence of
`parse-sexp-lookup-properties', and as a (redundant) safety measure
tried to emulate the sgml-mode environment as closely as possible.

If the tests still pass then let's try enabling it.

Jackson

On 11/19/2016 02:47 PM, Dmitry Gutov wrote:
> Hi!
> 
> On 07.11.2016 11:56, Felipe Ochoa wrote:
>> (Preemptive apologies if this is the wrong list/format for this comment
>> -- first time filer here!)
> 
> Not at all, thanks for the report.
> 
>> I
>> don't think there's a way to patch the syntax table that would let `>'
>> flip between punctuation and close-parens based on context, but one
>> possible fix when using js2-mode (not sure about js-mode)
> 
> I wonder what could be done in js-mode, too. A
> syntax-propertize-function rule, maybe.
> 
>> is to apply a
>> "." 'syntax-table text property to the `>' when parsing a fat arrow.
>>
>> Unfortunately, `js-jsx-indent-line' calls `sgml-indent-line' using
>> `js--as-sgml', which sets `parse-sexp-lookup-properties' to nil.
>>
>> Would there be any harm in setting `parse-sexp-lookup-properties' to t
>> instead? As far as I can tell, js-mode and js2-mode only use
>> 'syntax-table propeties for regex literals.
> 
> They also set that variable to t anyway. The only possible danger might
> come from sgml-mode, which does not do that.
> 
> I'm not sure which danger exactly, because all examples in
> test/indent/js-jsx.js seem to behave identically whether js--as-sgml
> includes the parse-sexp-lookup-properties binding or not.
> 
> Jackson, could you maybe shed some light on this?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24896; Package emacs. (Thu, 08 Dec 2016 11:13:02 GMT) Full text and rfc822 format available.

Message #14 received at 24896 <at> debbugs.gnu.org (full text, mbox):

From: Felipe Ochoa <felipe.nospam.ochoa <at> gmail.com>
To: 24896 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
Cc: Jackson Ray Hamilton <jackson <at> jacksonrayhamilton.com>
Subject: Re: bug#24896: JSX prop indentation after fat arrow
Date: Thu, 8 Dec 2016 12:12:42 +0100
[Message part 1 (text/plain, inline)]
Thanks -- seems reasonable. I figured as much but wanted to confirm.

I wonder what could be done in js-mode, too. A syntax-propertize-function
> rule, maybe.
>

syntax-propertize-function is outside my limited knowledge of emacs
internals. But to the extent that one could say "Outside of comments and
strings, propertize `=>' as punctuation," I imagine that would fix it.

Also, this may be a dumb question, but are the tests in the "manual"
directory meant to be run manually? If not, how would I run the tests there?

And then finally, should I just email a patch with the proposed change &
tests for this?

>

(Sorry Jackson for the double email -- forgot to reply all)
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24896; Package emacs. (Fri, 09 Dec 2016 00:19:01 GMT) Full text and rfc822 format available.

Message #17 received at 24896 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Felipe Ochoa <felipe.nospam.ochoa <at> gmail.com>, 24896 <at> debbugs.gnu.org
Cc: Jackson Ray Hamilton <jackson <at> jacksonrayhamilton.com>
Subject: Re: bug#24896: JSX prop indentation after fat arrow
Date: Fri, 9 Dec 2016 02:18:12 +0200
On 08.12.2016 13:12, Felipe Ochoa wrote:

> syntax-propertize-function is outside my limited knowledge of emacs
> internals. But to the extent that one could say "Outside of comments and
> strings, propertize `=>' as punctuation," I imagine that would fix it.

Yes, it can help with that.

> Also, this may be a dumb question, but are the tests in the "manual"
> directory meant to be run manually? If not, how would I run the tests there?

You can e.g. 'cd test/manual/indent' and run 'make js-jsx.js.test', to 
compare the indentation in js-jsx.js against what the current js.el does.

> And then finally, should I just email a patch with the proposed change &
> tests for this?

An email with a diff attached could be enough.

But if you wanted to include a "proper" commit message as well, see 
CONTRIBUTE in the top directory.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24896; Package emacs. (Fri, 06 Jan 2017 17:45:01 GMT) Full text and rfc822 format available.

Message #20 received at 24896 <at> debbugs.gnu.org (full text, mbox):

From: Felipe Ochoa <felipe.nospam.ochoa <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 24896 <at> debbugs.gnu.org,
 Jackson Ray Hamilton <jackson <at> jacksonrayhamilton.com>
Subject: Re: bug#24896: JSX prop indentation after fat arrow
Date: Fri, 6 Jan 2017 18:44:43 +0100
[Message part 1 (text/plain, inline)]
So I've thought about this some more, and realized that this won't fix
everything. There are still issues with greater-than and less-than as
binary operators. Maybe a better idea is to give '{' and '}' comment syntax
('<' and '>') so that SGML ignores all the bracketed JS stuff. I've been
trialing this with the following:

(defvar js-jsx-tag-syntax-table
  (let ((table (make-syntax-table sgml-tag-syntax-table)))
    (modify-syntax-entry ?\{ "<" table)
    (modify-syntax-entry ?\} ">" table)
    table))

(defun advice-js-jsx-indent-line (orig-fun)
  (interactive)
  (let ((sgml-tag-syntax-table js-jsx-tag-syntax-table))
    (apply orig-fun nil)))

(advice-add 'js-jsx-indent-line :around 'advice-js-jsx-indent-line)

and have gotten good results so far. This works for js-mode and js2-mode.
If you're both happy with this approach, I'll convert the advice into a
patch for `js-jsx-indent-line' and will send along!


On Fri, Dec 9, 2016 at 1:18 AM, Dmitry Gutov <dgutov <at> yandex.ru> wrote:

> On 08.12.2016 13:12, Felipe Ochoa wrote:
>
> syntax-propertize-function is outside my limited knowledge of emacs
>> internals. But to the extent that one could say "Outside of comments and
>> strings, propertize `=>' as punctuation," I imagine that would fix it.
>>
>
> Yes, it can help with that.
>
> Also, this may be a dumb question, but are the tests in the "manual"
>> directory meant to be run manually? If not, how would I run the tests
>> there?
>>
>
> You can e.g. 'cd test/manual/indent' and run 'make js-jsx.js.test', to
> compare the indentation in js-jsx.js against what the current js.el does.
>
> And then finally, should I just email a patch with the proposed change &
>> tests for this?
>>
>
> An email with a diff attached could be enough.
>
> But if you wanted to include a "proper" commit message as well, see
> CONTRIBUTE in the top directory.
>
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24896; Package emacs. (Sun, 15 Jan 2017 02:06:02 GMT) Full text and rfc822 format available.

Message #23 received at 24896 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Felipe Ochoa <felipe.nospam.ochoa <at> gmail.com>
Cc: 24896 <at> debbugs.gnu.org,
 Jackson Ray Hamilton <jackson <at> jacksonrayhamilton.com>
Subject: Re: bug#24896: JSX prop indentation after fat arrow
Date: Sun, 15 Jan 2017 05:04:51 +0300
On 06.01.2017 20:44, Felipe Ochoa wrote:
> So I've thought about this some more, and realized that this won't fix
> everything. There are still issues with greater-than and less-than as
> binary operators.

Inside XML literals, you mean?

> Maybe a better idea is to give '{' and '}' comment
> syntax ('<' and '>') so that SGML ignores all the bracketed JS stuff.
> I've been trialing this with the following:

How's your experience so far?

> (defvar js-jsx-tag-syntax-table
>   (let ((table (make-syntax-table sgml-tag-syntax-table)))
>     (modify-syntax-entry ?\{ "<" table)
>     (modify-syntax-entry ?\} ">" table)
>     table))
>
> (defun advice-js-jsx-indent-line (orig-fun)
>   (interactive)
>   (let ((sgml-tag-syntax-table js-jsx-tag-syntax-table))
>     (apply orig-fun nil)))

Here's the problem: js-indent-line uses syntax-ppss. sgml-indent-line 
doesn't (for now), but js-jsx-indent-line calls js-indent-line in 
certain contexts.

And this is a problem because calling syntax-ppss in different contexts 
with incompatible (paren-wise) syntax tables will make syntax-ppss cache 
broken, and lead to likewise broken behaviors.

So, one thing we could do here is let-bind the variables that constitute 
syntax-ppss cache around the call to orig-fun (i.e. around the context 
where we modify the syntax table).

Another, somewhat more difficult approach, would be to try to apply the 
"<" and ">" syntax classes in syntax-propertize-function, only to 
occurrences of "{" and "}" inside XML literals.

That would require knowing where the said literals begin and end, but we 
do know that somehow already, seeing as we know which indentation 
function to choose, right?

This way we don't depend on syntax-ppss internals (the cache is not 
really a public API), and reindenting the whole buffer might be faster, 
because we would keep syntax-ppss cache around more. Still, not sure how 
much faster that would be in practice.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24896; Package emacs. (Mon, 23 Jan 2017 16:12:02 GMT) Full text and rfc822 format available.

Message #26 received at 24896 <at> debbugs.gnu.org (full text, mbox):

From: "Felipe Ochoa" <felipeochoa0918 <at> gmail.com>
To: "'Dmitry Gutov'" <dgutov <at> yandex.ru>,
 "'Felipe Ochoa'" <felipe.nospam.ochoa <at> gmail.com>
Cc: 24896 <at> debbugs.gnu.org,
 'Jackson Ray Hamilton' <jackson <at> jacksonrayhamilton.com>
Subject: RE: bug#24896: JSX prop indentation after fat arrow
Date: Mon, 23 Jan 2017 10:26:06 +0100
>> There are still issues with greater-than and less-than 
>> as binary operators.
> Inside XML literals, you mean?

Yes, exactly.

> How's your experience so far?

It's actually worked very well. I had an issue once where indenting an entire region took several passes to get right, but now I'm not able to reproduce it :( 

> Here's the problem: js-indent-line uses syntax-ppss. 
> sgml-indent-line doesn't (for now), but js-jsx-indent-line 
> calls js-indent-line in certain contexts. And this is a problem
> because calling syntax-ppss in different contexts with 
> incompatible (paren-wise) syntax tables will make 
> syntax-ppss cache broken, and lead to likewise broken
> behaviors.

I'm not sure I'm grasping this part entirely. I understand conceptually that using syntax-ppss with incompatible syntax tables could lead to cache problems. But it seems to me that the js*-mode and sgml-*-mode syntax tables are already incompatible (namely, "<" and ">", which are causing all this grief!). Would introducing this additional incompatibility cause more problems? 

> So, one thing we could do here is let-bind the variables that 
> constitute syntax-ppss cache around the call to orig-fun 
> (i.e. around the context where we modify the syntax table).
> ... (the cache is not really a public API)

This sounds like a bit of a headache. E.g., indenting a region would require binding and unbinding the cache carefully as you stepped into and out of JSX. What if we just scrap the syntax-ppss cache altogether? Would the performance penalty be too great?

> Another, somewhat more difficult approach, would be to try
> to apply the "<" and ">" syntax classes in 
> syntax-propertize-function, only to occurrences of "{" and "}" 
> inside XML literals. That would require knowing where the said
> literals begin and end, but we do know that somehow already,
> seeing as we know which indentation function to choose, right?

This is based on a rough heuristic that essentially backtracks looking for "[(,]\n *<" (it also handles comments). This misses any JSX which is not at the start of the line, and it only tells us the start of the tag, not the end or where the body ends. In js2 and rjsx there is of course the full parser to give us this information. 

> This way we don't depend on syntax-ppss internals, and reindenting
> the whole buffer might be faster, because we would keep syntax-ppss 
> cache around more. Still, not sure how much faster that would be in 
> practice.

I think we could use a regex like the following to identify JSX start tokens:

(rx (seq (or (any "-+*/%=><?:&")
             (seq (or "return" "typeof" "delete" "instanceof") whitespace)
             (any "([{,;"))
         (* whitespace) ; Should also skip over comments
         "<"))

I.e., any "<" after an operator or at the beginning of an expression or statement. We'd have to filter out some false positives (postfix ++ and --, strings, and comments, possibly others), but this would get all the JSX start tags, I think. We could use a similar regex to find the ">" that close JSX tags:

(rx (seq ">"
         (* whitespace) ; Should also skip over comments
         (or (any "-+*/%=><?:&")
             (seq (or "return" "typeof" "delete" "instanceof") whitespace)
             (any "(}],;"))

Not sure how to go from there to the "{" and "}" tokens though. Is it possible to run syntax-ppss using different tables for different parts of the buffer?





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24896; Package emacs. (Mon, 23 Jan 2017 17:08:01 GMT) Full text and rfc822 format available.

Message #29 received at 24896 <at> debbugs.gnu.org (full text, mbox):

From: Jackson Ray Hamilton <jackson <at> jacksonrayhamilton.com>
To: Felipe Ochoa <felipeochoa0918 <at> gmail.com>,
 'Dmitry Gutov' <dgutov <at> yandex.ru>,
 'Felipe Ochoa' <felipe.nospam.ochoa <at> gmail.com>
Cc: 24896 <at> debbugs.gnu.org
Subject: Re: bug#24896: JSX prop indentation after fat arrow
Date: Mon, 23 Jan 2017 09:07:17 -0800
Hi Felipe,

Regarding,

> This is based on a rough heuristic that essentially backtracks looking for "[(,]\n *<" (it also handles comments). This misses any JSX which is not at the start of the line, and it only tells us the start of the tag, not the end or where the body ends. In js2 and rjsx there is of course the full parser to give us this information.

Please note Dmitry's comment:
https://github.com/mooz/js2-mode/issues/140#issuecomment-40887172,

> As for indentation . . . there's a question how one would determine whether point is inside an XML expression (and expression's bounds) without using the AST (using it was rejected in the past on the grounds of that being slow).

And see here for my explanation of that design:
https://github.com/mooz/js2-mode/issues/140#issuecomment-145325361

Feel free the improve upon this algorithm, although do take care to
benchmark the code before and after your changes, with buffers of
various sizes.  Large files won't hold up well if using an AST for
indentation.  Probably better to extend the current heuristic to be more
accurate.

Jackson

On 01/23/2017 01:26 AM, Felipe Ochoa wrote:
>>> There are still issues with greater-than and less-than 
>>> as binary operators.
>> Inside XML literals, you mean?
> 
> Yes, exactly.
> 
>> How's your experience so far?
> 
> It's actually worked very well. I had an issue once where indenting an entire region took several passes to get right, but now I'm not able to reproduce it :( 
> 
>> Here's the problem: js-indent-line uses syntax-ppss. 
>> sgml-indent-line doesn't (for now), but js-jsx-indent-line 
>> calls js-indent-line in certain contexts. And this is a problem
>> because calling syntax-ppss in different contexts with 
>> incompatible (paren-wise) syntax tables will make 
>> syntax-ppss cache broken, and lead to likewise broken
>> behaviors.
> 
> I'm not sure I'm grasping this part entirely. I understand conceptually that using syntax-ppss with incompatible syntax tables could lead to cache problems. But it seems to me that the js*-mode and sgml-*-mode syntax tables are already incompatible (namely, "<" and ">", which are causing all this grief!). Would introducing this additional incompatibility cause more problems? 
> 
>> So, one thing we could do here is let-bind the variables that 
>> constitute syntax-ppss cache around the call to orig-fun 
>> (i.e. around the context where we modify the syntax table).
>> ... (the cache is not really a public API)
> 
> This sounds like a bit of a headache. E.g., indenting a region would require binding and unbinding the cache carefully as you stepped into and out of JSX. What if we just scrap the syntax-ppss cache altogether? Would the performance penalty be too great?
> 
>> Another, somewhat more difficult approach, would be to try
>> to apply the "<" and ">" syntax classes in 
>> syntax-propertize-function, only to occurrences of "{" and "}" 
>> inside XML literals. That would require knowing where the said
>> literals begin and end, but we do know that somehow already,
>> seeing as we know which indentation function to choose, right?
> 
> This is based on a rough heuristic that essentially backtracks looking for "[(,]\n *<" (it also handles comments). This misses any JSX which is not at the start of the line, and it only tells us the start of the tag, not the end or where the body ends. In js2 and rjsx there is of course the full parser to give us this information. 
> 
>> This way we don't depend on syntax-ppss internals, and reindenting
>> the whole buffer might be faster, because we would keep syntax-ppss 
>> cache around more. Still, not sure how much faster that would be in 
>> practice.
> 
> I think we could use a regex like the following to identify JSX start tokens:
> 
> (rx (seq (or (any "-+*/%=><?:&")
>              (seq (or "return" "typeof" "delete" "instanceof") whitespace)
>              (any "([{,;"))
>          (* whitespace) ; Should also skip over comments
>          "<"))
> 
> I.e., any "<" after an operator or at the beginning of an expression or statement. We'd have to filter out some false positives (postfix ++ and --, strings, and comments, possibly others), but this would get all the JSX start tags, I think. We could use a similar regex to find the ">" that close JSX tags:
> 
> (rx (seq ">"
>          (* whitespace) ; Should also skip over comments
>          (or (any "-+*/%=><?:&")
>              (seq (or "return" "typeof" "delete" "instanceof") whitespace)
>              (any "(}],;"))
> 
> Not sure how to go from there to the "{" and "}" tokens though. Is it possible to run syntax-ppss using different tables for different parts of the buffer?
> 




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24896; Package emacs. (Wed, 25 Jan 2017 02:01:01 GMT) Full text and rfc822 format available.

Message #32 received at 24896 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: 'Felipe Ochoa' <felipe.nospam.ochoa <at> gmail.com>
Cc: 24896 <at> debbugs.gnu.org,
 'Jackson Ray Hamilton' <jackson <at> jacksonrayhamilton.com>
Subject: Re: bug#24896: JSX prop indentation after fat arrow
Date: Wed, 25 Jan 2017 04:59:59 +0300
On 23.01.2017 12:26, Felipe Ochoa wrote:

> It's actually worked very well. I had an issue once where indenting an entire region took several passes to get right, but now I'm not able to reproduce it :(

Unreproducible wonky behavior might be the result of busting syntax-ppss 
cache.

> But it seems to me that the js*-mode and sgml-*-mode syntax tables are already incompatible (namely, "<" and ">", which are causing all this grief!).

Right. For now, we seem to have avoided the problem because 
sgml-indent-line does not call syntax-ppss.

> Would introducing this additional incompatibility cause more problems?

...so if you change the syntax table just around the call to 
sgml-calculate-indent (not the whole js-jsx-indent-line), it shouldn't 
make things worse.

> This sounds like a bit of a headache. E.g., indenting a region would require binding and unbinding the cache carefully as you stepped into and out of JSX. What if we just scrap the syntax-ppss cache altogether? Would the performance penalty be too great?

Let-binding the cache variables to nil around the call to 
sgml-calculate-indent might be fast enough, because multiple calls to 
syntax-ppss (if any) inside that functions will still be amortized.

But this is really going to be more useful when sgml-calculate-indent 
starts using syntax-ppss, which really might never happen. So the 
previous solution (changing the syntax table only where needed) might be 
preferable in the meantime.

> This is based on a rough heuristic that essentially backtracks looking for "[(,]\n *<" (it also handles comments). This misses any JSX which is not at the start of the line, and it only tells us the start of the tag, not the end or where the body ends.

I think sgml-skip-tag-forward could help with the last one.

> In js2 and rjsx there is of course the full parser to give us this information.

Like Jackson mentioned, probably not a good idea.

> I think we could use a regex like the following to identify JSX start tokens:
>
> (rx (seq (or (any "-+*/%=><?:&")

Can all of these (e.g. >) be realistically expected before a JSX literal?

And it seems like the << operator would cause a false positive.

I like the general direction, though.

> We could use a similar regex to find the ">" that close JSX tags:

sgml-skip-tag-forward seems like the more reliable option to me. 
Although it might work worse on invalid code.

> Is it possible to run syntax-ppss using different tables for different parts of the buffer?

If you perform the previously mentioned cache fiddling, yes. If not, 
then the answer is maybe, and that depends on how and when you'll be 
calling it. So maybe you should get into the underlying mechanics first.




Forcibly Merged 24896 26001. Request was from Glenn Morris <rgm <at> gnu.org> to control <at> debbugs.gnu.org. (Mon, 06 Mar 2017 17:41:01 GMT) Full text and rfc822 format available.

Forcibly Merged 24896 26001 30225. Request was from Glenn Morris <rgm <at> gnu.org> to control <at> debbugs.gnu.org. (Tue, 23 Jan 2018 01:55:01 GMT) Full text and rfc822 format available.

Forcibly Merged 24896 26001 30225 32158. Request was from Glenn Morris <rgm <at> gnu.org> to control <at> debbugs.gnu.org. (Sat, 14 Jul 2018 16:20:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24896; Package emacs. (Wed, 05 Jun 2019 02:48:01 GMT) Full text and rfc822 format available.

Message #41 received at 24896 <at> debbugs.gnu.org (full text, mbox):

From: Jackson Ray Hamilton <jackson <at> jacksonrayhamilton.com>
To: 24896 <at> debbugs.gnu.org
Subject: Re: bug#24896: JSX prop indentation after fat arrow
Date: Tue, 4 Jun 2019 19:47:21 -0700
A set of changes I pushed to the Emacs master branch 2 months ago should 
resolve this issue.  (First fixed in be86ece42c, but there were several 
commits after that when I broke/fixed it again.  The master branch is 
your best bet.)





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24896; Package emacs. (Thu, 06 Jun 2019 07:00:02 GMT) Full text and rfc822 format available.

Message #44 received at 24896-done <at> debbugs.gnu.org (full text, mbox):

From: Jackson Ray Hamilton <jackson <at> jacksonrayhamilton.com>
To: 24896-done <at> debbugs.gnu.org
Subject: Re: bug#24896: JSX prop indentation after fat arrow
Date: Wed, 5 Jun 2019 23:59:22 -0700
Marking this done.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Thu, 04 Jul 2019 11:24:04 GMT) Full text and rfc822 format available.

This bug report was last modified 4 years and 269 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.