Skip to content
This repository has been archived by the owner on Sep 6, 2018. It is now read-only.

plaintext inside JSX not easily discerned from embedded JS #42

Closed
vith opened this issue Jul 18, 2015 · 3 comments
Closed

plaintext inside JSX not easily discerned from embedded JS #42

vith opened this issue Jul 18, 2015 · 3 comments

Comments

@vith
Copy link

vith commented Jul 18, 2015

I would like to see this mistake:
screenshot from 2015-07-18 06 21 00

be more visually dissimilar from the intended code:
screenshot from 2015-07-18 06 22 13

like so:
screenshot from 2015-07-18 06 21 24

But I'm afraid my implementation is too tightly coupled to language-babel.

Would a PR for such an effect be wanted? If so, is .meta.brace.curly.js an appropriate thing to target to reset the style back to normal for JS blocks escaped by { }?

@simurai
Copy link
Contributor

simurai commented Jul 21, 2015

Would a PR for such an effect be wanted? If so, is .meta.brace.curly.js an appropriate thing to target to reset the style back to normal for JS blocks escaped by { }?

Good question. I checked out the react and language-javascript-jsx packages, but both use different highlighting. So yeah, your PR would be kinda too tied to the language-babel package.

I'm not too familiar with JSX, is omitting the braces invalid? If so, another option might be to make a PR for language-babel that changes the highlighting to hint that braces are required. Maybe add .invalid.illegal https://github.com/atom/one-dark-syntax/blob/master/styles/language.less#L250? Although that might be too strong and annoying.

Or if you just like to change it personally, you could add it to your styles.less (for now):

// customize language-babel
.theme-one-dark-syntax atom-text-editor::shadow {
  .source.jsx {
    .markup.raw.jsx {
      color: #828997;
      .meta.brace.curly.js {
        color: #abb2bf;
      }
    }
  }
}

@vith
Copy link
Author

vith commented Jul 22, 2015

It's not actually invalid without the braces, it's just treated as a plaintext string in the rendered HTML output instead of executed as javascript code. It doesn't mean what it looks like in my example without the braces, which is why I'd like the syntax highlighting to make that more obvious.

It looks like language-javascript-jsx has the best labeling of the embedded section as it's consistent with how language-php labels PHP sections in HTML, or language-ruby labeling Ruby in HTML.

I updated my one-dark-syntax fork to do this instead:

// styles/languages/jsx.less
.source.jsx {
  .markup.raw.jsx {
    color: @mono-2;
  }
}
// styles/language.less
.source {
  color: @mono-1;
}

And I've made a pull request to language-babel to fix the embedded section labeling here: gandm/language-babel#29

@simurai
Copy link
Contributor

simurai commented Jul 24, 2015

Or just

// styles/languages/jsx.less
.source.jsx {
  color: @mono-1;

  .markup.raw.jsx {
    color: @mono-2;
  }
}

so it's scoped to JSX only? Although that still only affects language-babel.

Ok, this might be a stupid question, but isn't it a bit strange that language-babel also includes JSX highlighting? Wouldn't it better if there is a separate language-jsx package that becomes the "official" one? Currently there are a few packages that all do JSX highlighting but in a different way. Maybe it's still a bit early and one will emerge over time.

Hmm.. https://atom.io/packages/language-jsx seems to be taken by another language with the same name. 😣

Anyways, are you ok to keep it in your personal fork or styles.less for now? Although if you don't mind making a PR with the JSX spec only, that would be great to have.

@simurai simurai closed this as completed Aug 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants