atom / language-gfm

GitHub Flavored Markdown in Atom
MIT License
101 stars 108 forks source link

Highlight JSX code blocks #173

Closed silvenon closed 5 years ago

silvenon commented 8 years ago

Some JSX packages might specify JSX as source.jsx, but I assumed that language-babel is the most popular, and it uses source.js.jsx.

onebree commented 5 years ago

Can this PR get merged? I am writing quite a bit of JSX in markdown, and this change would really help!

jasonrudolph commented 5 years ago

Some JSX packages might specify JSX as source.jsx, but I assumed that language-babel is the most popular, and it uses source.js.jsx.

👋 Hi @silvenon, Hi @onebree: Is this pull request still needed in order to get JSX highlighting in markdown documents?

When I install language-babel, it seems to provide the syntax highlighting for JSX codeblocks in markdown, without needing the changes in this pull request. For example, this screenshot is taken with Atom 1.36.1 and language-babel 2.85.0:

Screen Shot 2019-05-08 at 3 40 42 PM

It's entirely possible that I'm misunderstanding why these changes are needed. If so, please let me know. :bow:

silvenon commented 5 years ago

I no longer use Atom, so if you evaluate that this change is no longer useful, feel free to close. 😉

onebree commented 5 years ago

@jasonrudolph - I don't have that package installed (language-babel). Instead, I use language-javascript-jsx. I'll try the one you suggested and get back to you!

But IMO, I do think that -- if Github highlights JSX in markdown, then this GFM package should do it as well, without the help of additional plugins. Just a thought.

onebree commented 5 years ago

@jasonrudolph - I just tried installing the language package you suggested. When opening code files, it raised errors that are open PRs for the project.

My markdown is also not highlighting properly:

I made sure to disable the language-javascript-jsx package I have, in case it caused conflicts. But these problems occurred when:

ozydingo commented 5 years ago

@onebree in this case I believe the lack of a close } for your class is messing up the parser; what the expected behavior should be I'm not sure (but it'd be nice to be able to capture syntax errors in code blocks too!)

In any case there are also some additional highlighting errors related at least to (1) comments and (2) self-closing tags; see below.

Happy to take a stab at these if someone can help point me in the right direction.

jasonrudolph commented 5 years ago

if Github highlights JSX in markdown, then this GFM package should do it as well, without the help of additional plugins. Just a thought.

@onebree: That makes sense to me, and GitHub.com does indeed highlight JSX in markdown. Let's review this pull request with this idea in mind. 🙂

As far as I can tell, the changes in this pull request don't provide JSX syntax highlighting in markdown documents. Here's what I'm seeing when testing these changes locally:

Screen Shot 2019-05-13 at 10 49 43 AM

This is the same behavior I see when using the current version of language-gfm that ships with Atom (v0.90.6):

Screen Shot 2019-05-13 at 10 49 43 AM

If I use the current version of language-gfm that ships with Atom (v0.90.6) and I install the current version of language-babel (v2.85.0), then JSX syntax highlighting is available in markdown documents:

Screen Shot 2019-05-13 at 10 50 49 AM

With that in mind, unless I'm misunderstanding something, it doesn't seem like this pull request gets us any closer to supporting JSX syntax highlighting in markdown documents, so I'm going to close this pull request.

onebree commented 5 years ago

@jasonrudolph Looking at your example code, if I re-enable the babel package, I am getting the same highlighting as you. However, if I try to write self-closing tags, that's where the syntax highlighting for the babel package breaks. That's what you can see in my screenshots from my previous message. Looks like self-closing tags don't actually close, leaving the 3 backticks to end the code block to be ignored.

I can't speak to whether this PR will add JSX support to GFM, but I do think that such support would be helpful. No need to open this PR back up though, if it doesn't affect markdown documents.