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

Elm syntax highlighting with PrismJS #32

Merged
merged 2 commits into from
Jan 22, 2018

Conversation

kachkaev
Copy link
Collaborator

@kachkaev kachkaev commented Jan 17, 2018

Prism now supports syntax highlighting for elm, thanks to PrismJS/prism#1174. This makes it possible to highlight elm code in the markdown output, which is what I implemented in this PR.

Because mume partially relies on dependencies/*.js instead of node_modules, here are the details of how I updated dependencies/prism.js:

  1. Went to http://prismjs.com/download.html
  2. Selected all languages
  3. Downloaded the js file and saved it as dependencies/prism.js
  4. In a rather large diff, searched for a line starting with Prism.languages.elm= and staged it

This means that PrismJS has not been updated as a whole. Although this ensures that mume does not break with this PR, it'd be great to generally keep in sync with prism as it gets updated. @shd101wyy could you please briefly describe how to generate dependencies/prism.js from source inside dependencies/README.md?

To be honest, I'd prefer to just sticking with node_modules, but I imagine there's been a really good reason to switch to a custom dependency management. Just curious: in which scenarios large node_modules ended up being a pain for mume? Can't remember that in other Node.js (i.e. not-in-browser) projects.

@shd101wyy
Copy link
Owner

Hi @kachkaev , I didn't choose node_modules for some packages because I had trouble packaging vscode extension. If I recall correctly, vscode extension has a size limit of 20mb. There are also some reasons but I don't remember exactly.

I will have a look at your pull request when I have time.

Thank you very much for your contribution.

@shd101wyy shd101wyy self-requested a review January 22, 2018 21:52
@shd101wyy
Copy link
Owner

Looks good to me. You can merge it at anytime. Thanks.

@kachkaev kachkaev merged commit e03c533 into shd101wyy:master Jan 22, 2018
@kachkaev
Copy link
Collaborator Author

Thanks @shd101wyy! Would you like me to prepare a new release? I can't publish on npm, but could change the version, update changelog etc. Happy if you do that too, just asking because I know you're a bit busy now

@shd101wyy
Copy link
Owner

Thanks @kachkaev ! I will finish reviewing your pull requset #34 and #31 by the end of this week. Then we can publish a new version.

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

Successfully merging this pull request may close these issues.

2 participants