Skip to content
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

Embedded JSX part in .js.jsx.coffee file not syntax highlighted. #10

Open
isundaylee opened this issue Mar 5, 2017 · 8 comments
Open

Comments

@isundaylee
Copy link

I notice that the embedded JavaScript part in a CoffeeScript file (which is used for embedding JSX) is not syntax highlighted, and instead, is treated like a string (as in the attached screenshot). Is this a BUG, or am I using it incorrectly? Thanks!

screen shot 2017-03-04 at 11 19 40 pm

@cooncesean
Copy link
Contributor

Probably a bug. We're not meticulously supporting this library atm, but feel free to submit a patch if you'd like :)

@schneidmaster
Copy link
Contributor

schneidmaster commented Mar 6, 2017

It looks like you aren't using it correctly (or rather, your code may be correct for your case, but the syntax highlighting is already behaving correctly). You're using backticks to embed all of your JSX in CoffeeScript -- there's no reasonable way for the syntax highlighting to detect that a particular embedded string in your code is actually CJSX and then parse/format it. You should instead consider using cjsx-loader (webpack), gulp-cjsx, etc. to correctly compile the CJSX and prevent the need for the backticks (this will also let you use CoffeeScript in your JSX itself).

Edit: I'm guessing you're actually using sprockets from the .js.jsx.coffee -- in which case this may be helpful.

@isundaylee
Copy link
Author

That's totally reasonable. Thanks for pointing me to sprockets-coffee-jsx! However, it seems that according to http://coffeescript.org/#embedded, the backtick format is reserved for embedding vanilla JavaScript in CoffeeScript, instead of arbitrary strings. So do you think it makes sense for us to fall back to the JavaScript (or in our case, JSX) scope within backticks? I can work on a PR if you think this might be useful. Again thanks for linking me to the resources!

@schneidmaster
Copy link
Contributor

It seems fairly reasonable to me but I have a few concerns:

  1. This wouldn't really help in your case since vanilla Javascript syntax highlighting would not include JSX.
  2. At least from my understanding of how Sublime syntax files work, the only way to accomplish this would be to embed the entire Javascript syntax definition within the CJSX one (to be applied when a regex matches from the backticks) -- there's no way that I'm aware of to just defer to a different syntax on a certain match. Might not be a showstopper but it would at least double the size of the file for a somewhat marginal benefit.
  3. I'm a bit worried it would be a detriment -- it might make it harder to actually see when something is embedded in backticks and cause confusion. This could perhaps be solved by making the backticks stand out a lot.

Just my thoughts though. I'm not the repo owner, just a contributor.

@cooncesean
Copy link
Contributor

but it would at least double the size of the file for a somewhat marginal benefit.

If that's the case, then lets not go down that road; we don't want to add superfluous weight to this small library.

With that in mind @isundaylee, if you want to work on a PR that delineates back-ticked strings from more traditional strings, have at it.

@isundaylee
Copy link
Author

isundaylee commented Mar 6, 2017

We can reference the Packages/JavaScript/JavaScript.sublime-syntax file, without having to duplicate anything in our syntax. I have a proof-of-concept of this working. I think this approach is fine because the JavaScript package is a Sublime built-in package, so we're not introducing any additional dependency. However this way we don't get JSX highlighting, and it wouldn't help my case. But I think this way we're being more semantically correct in the embedded JavaScript case.

I do see how @schneidmaster's 3rd point can become an issue. I guess it boils down to which highlighting is more useful for people who use embedded JavaScript: to have the entire section stand out, or to have highlighting within the section? I personally lean towards the latter, but would love some more opinions on that.

@cooncesean
Copy link
Contributor

We can reference the Packages/JavaScript/JavaScript.sublime-syntax file, without having to duplicate anything in our syntax.

👍

As for:

...which highlighting is more useful for people who use embedded JavaScript: to have the entire section stand out, or to have highlighting within the section?

I have no preference and leave it to you two to come to some sort of reasonable conclusion.

Can we make it an option that defaults to existing behavior, but allows for the proposed customization should the user want it?

@schneidmaster
Copy link
Contributor

Ah, fair enough, I didn't realize that it was possible to reference external syntax files. Good to learn 😄

I'm still conflicted about readability. I'm not aware of any major use cases for blocks of inline javascript that couldn't be better solved by using a different processor (in your example, I think the happy path is to use the CJSX processor rather than to syntax highlight the backtick content). The only other common case I can think of is Relay graphql queries, which are hoary in CoffeeScript regardless and wouldn't benefit from JS syntax highlighting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants