-
-
Notifications
You must be signed in to change notification settings - Fork 186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use JSX highlighting and indentation added in Emacs 27 #523
Conversation
This should be a much safer way of silencing compiler warnings (I was encountering warnings, not errors).
This should be safer.
Since all uses of these vars and functions are now guarded by “bound” checks, these are no longer necessary; the compiler seems to determine that proper “bound” guards will make guarded code safe.
Since I first opened the PR last night, I got some feedback via email on the code style, and made updates accordingly (a59c7b1...63b7a72). |
I pushed a commit to Emacs |
Hmm. Have you tried using |
Looks like that works! Much simpler. While I just pushed that change, I don’t mean to discount the alternative change that Stef has suggested via email. This push is merely an improvement upon what I initially pushed. Not sure what you think about his idea, or how to test it. |
I added a section to the README which should hopefully encourage some users to download a Emacs 27 snapshot and test out my improvements. |
After a long-ish private discussion, this is the step I have decided on for now: recommend This proposal might be better in a sense (makes it easier for the users to follow), but it's harder for me to test any breakage with earlier versions of In the meantime, I'd like to know what @felipeochoa thinks about it and whether he'd like to reimplement some of the features in @jacksonrayhamilton, further comments welcome. |
Here's hoping |
Backgroundjs-mode is now using
This PR would have copied some code from js-mode into js2-mode, such that js2-mode would also do those things in the same way that js-mode did them. But, long term, there may be a better solution for js2-mode than copying code. Currently, js2-mode uses js-mode’s code for indentation (and, in older emacsen, it backports indentation logic from Emacs 26 and earlier). However, js2-mode doesn’t set text properties with A disadvantage to the scheme described in the above paragraph is that the JSX features added in js-mode won’t be accessible when js2-mode is enabled; instead, as Dmitry has commented above, one must instead use js-mode with js2-minor-mode enabled. Feature WishlistIn order for complete support for JSX to be available in js2-mode (not just js2-minor-mode), the following things should be done:
As has been pointed out by Stefan in our email thread—and I’m in agreement with him on this—long term, it may be best to set text properties related to indentation using Therefore, another long-term change we may wish to work towards would be:
However, before we make that leap, we’d like to ensure that this change will be compatible with derivative modes (like @felipeochoa’s rjsx-mode) which set The Future of rjsx-modeI recognized that users were finding it challenging to work with JSX code in Emacs and also with configuring Emacs to do that. Therefore, I resolved it would be best to include support for JSX in Emacs’ core, and that work has already been compeleted. What does this mean for rjsx-mode?
Given these points, I can envision the following futures for this package:
Assuming future 1 is pursued, rjsx-mode’s purpose could transition to be to provide interesting React/JSX features like electricity. Or, electricity could also be migrated to JS2 or even to js-mode, and rjsx-mode could be retired. But those are just my ideas, I would like to hear from @felipeochoa what he thinks of them, and how he thinks the future should unfold for his package. |
Relying on imports sounds like an awful idea to me, there are many libraries which rely on JSX, and looking only React (down to import names) is just a bad idea, especially because it doesn't take the possibility of a different JSX pragma (and another one for fragments) into account. These problems might crop up even in React projects that use something like In fact, making JS2 pragma-aware would allow to catch errors where the user forgets to import the pragma (which for React projects would be |
@DoMiNeLa10 Since React is the bearer of JSX and a Also, Emacs 27 makes available a customizable variable called |
Since the spirit of the changes provided by this PR won’t be merged, I’ll close it. There are still a few changes in relation to the addition of JSX that I think should be addressed:
Also, several threads could be spun from my big post above. I’ll create separate issues / PRs for all of these things. |
@dgutov As an emacs 26 user, It would be nice if all of the JSX PRs could be merged into js2-mode and a new release of js2-mode made to melpa. If so, I could throw out rjsx-mode and use js2-mode instead. |
There's no open PR that will make it all magically work in Emacs 26. |
The
master
branch of Emacs now provides support for highlighting JSX, greatly improves the current JSX indentation support, and automatically detects the use of JSX (without needing to enterjs-jsx-mode
). These changes allow js2-mode to tap into these improvements when using Emacs on themaster
branch, or eventually in version 27.Fixes #140, #330, #389, #411, #451, #459, #462, #482, #490
I was hoping this would address #409, but I think we may have a little more work to do for that. Indentation is right in js-mode, but I think js2-mode is clearing thesyntax-table
text property needed to prevent JSXText from introducing unterminated strings. To solve this I will probably provide more changes (here or in another PR) that re-adds thesyntax-table
property here after it is cleared elsewhere.As noted below, this now also fixes #409.