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

Update highlight.js 9.1.0 → 9.3.0 #1

Closed
wants to merge 1 commit into from

Conversation

chibicode
Copy link
Contributor

@chibicode chibicode commented Apr 23, 2016

Also updated tests.

@wooorm
Copy link
Owner

wooorm commented Apr 23, 2016

Thanks!

In the future, please add comments to your PR regarding why you did what; e.g., the trim in the tests, and why almost all from the js-jsx test was removed.

And, for lowlight in particular, which is basically a rewrite of hljs, every commit to src/highlight.js also needs to be included for an update.

@chibicode
Copy link
Contributor Author

chibicode commented Apr 23, 2016

@wooorm thanks and sorry about that, I will the next time. Just FYI:

  • I added trim because the test for http was failing on one thing, which was the trailing space in the test fixture. I assumed this was an issue w/ processing of the file and not lowlight itself, so I made sure that the input/output has no trailing spaces.
  • As for JSX, I meant to write a comment on this but forgot - on highlight.js 9.1.0 to 9.3.0 they simplified the test case: highlightjs/highlight.js@e5d2123 - so I tried this, but I couldn't get this test case to pass (the output diff was slightly different). After banging my head for about an hour, I incorrectly assumed that lowlight was just a thin wrapper of highlight, so I thought one test case would suffice.

And, for lowlight in particular, which is basically a rewrite of hljs, every commit to src/highlight.js also needs to be included for an update.

I didn't realize that lowlight was a rewrite - thought it was just a thin wrapper, and now it makes sense why the JSX test was failing.

I've ended up using prism because according to this thread Prism handles JSX better: highlightjs/highlight.js#931 but when I get time I'll make a new PR for lowlight.

@wooorm wooorm added the concept label May 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants