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

Better Markdown rendering / syntax highlighting #95

Open
tdd opened this issue Feb 20, 2015 · 12 comments
Open

Better Markdown rendering / syntax highlighting #95

tdd opened this issue Feb 20, 2015 · 12 comments

Comments

@tdd
Copy link
Member

tdd commented Feb 20, 2015

Hey all,

So, currently we're relying on the released-once-never-updated msee module for rendering Markdown and doing syntax highlighting on the code snippets.

It's doing a pretty good job right now. It actually relies on a number of other modules, and the split of responsibilities is not always super-clear.

  • marked parses the Markdown and renders it in terminal-oriented text. Its actual rendering is hijacked by msee, that only relies on its inline lexers and processes tokens internally, deferring code blocks highlighting to…
  • cardinal, that processes the parsed code blocks and renders them in terminal-based colors (this thing's even themable!). It specifically deports JavaScript code parsing (better not parse anything else!) to…
    • redeyed, which in turn is esprima-based. Unfortunately, it has next to no ES6 support.

This way, any ES6 code is not properly colored, which is kinda too bad. Can't we figure out another way to highlight our code? I mean, marked will happily honor a highlight callback option that could use whatever we want (bundled pygmentize, highlight.js, etc.), so there might be a way there…

msee also makes a number of other questionable calls, such as rendering all types of lists as bullet lists, and not rendering anything inside a list item (most notably inline code), and also not honoring source code line breaks in list items, all of which regularly forces me to "fake" lists with undetected syntaxes so they look alright (yeah, no kidding).

What do you guys (mostly @rvagg and @martinheidegger) think?

@martinheidegger
Copy link
Contributor

Very important, havnt found time for it. Formatting atm is suboptimal at best.

@rvagg
Copy link
Contributor

rvagg commented Feb 22, 2015

Right, I had a bit of difficulty with the initial version when we introduced msee and had to get an update there to make it all work nicely but since then it's been a bit of a struggle and something we obviously need to rectify.

My first suggestion would be for someone with time on their hands and that cares enough to go ask the msee author if we can take over maintaining it (well, "we" meaning one of us probably). Then we could have our way with it and be responsive to user requests.

Next would be to either fork or reimplement it. We're having the same problems with terminal-menu and it's due for a fork so we can do our own special stuff with it. I'm 👍 on anything that would let us make better progress.

Also @thlorenz who may be interested in the comments on ES6 / cardinal / redeyed thing. My understanding is that Esprima 2 has started adding more ES6 stuff, maybe we just need an upgrade?

@tdd
Copy link
Member Author

tdd commented Feb 22, 2015

Yes, terminal-menu, through its bug in visual-width, makes me irk on most FR menus I get, that incorrectly align the completed right-hand marker. Def due for an upgrade.

I'm absolutely going to address these in the coming weeks, along with pending g11n/L10n's on workshopper-based workshops, adventure, adventure-based workshops and stream-adventure refactoring and bugfixing (it has at least 2 confirmed bugs in its exercise handlers).

Also, FYI: how-to-nom, freshly released by npm Inc, is buggy as hell. 12+ people yesterday got into a lot of different issues. Going to assert these, raise issues, and write as many fixes as I can before translating.

I also slated some time to look deeper into any workshopper / workshopper-exercise open issues, see what can be closed and what warrants further investigation. You might get some new pings on these when I need clarification / context.

Best,

@thlorenz
Copy link

@rvagg very hesitant to switch to ES6 branch of esprima as last time browserify did things got very unstable.
However if you know a stable branch that supports it, we could upgrade redeyed to it which would affect cardinal.

@rvagg
Copy link
Contributor

rvagg commented Feb 23, 2015

@thlorenz perhaps you could --tag a prerelease we could canary for you?

@rvagg
Copy link
Contributor

rvagg commented Mar 4, 2015

@tdd
Copy link
Member Author

tdd commented Mar 4, 2015

Cool! We should then update msee to use the latest version, then update workshopper to use the latest msee. This will only correct part of the issues with msee but that's definitely a step forward; it would be particularly useful for ES6-based workshops such as count-to-6, learn-generators and kick-off-koa, to name only these.

@thlorenz
Copy link

thlorenz commented Mar 4, 2015

BTW not currently documented but redeyed now supports jsx as well.
We just need to create a config-jsx.js file that lists those tokens.

We could add that to cardinal themes to highlight jsx if that is needed.

@thibaudcolas
Copy link

Hey there,

I gave this a try and sent a PR to msee + forked it. This fork includes a few changes from another fork + dependency updates to marked and cardinal. You can try it with npm install --save ThibWeb/msee.

I also looked at marked-terminal but wasn't entirely convinced by its output. My test is available here if someone wants to investigate further (it has support for emojis and Markdown tables, that'd have been nice).

Both modules have small codebases because they rely on marked for the heavy lifting. It wouldn't be too much effort to completely fork and rebuild msee / submit PRs to change the output of marked-terminal. It's still too much (duplication of) effort for it to be worth it for my single little workshopper though, I'd rather see msee's PRs merged.

Edit: The msee PR was merged by the project's maintainer! Here's a PR: #127


I also forked workshopper to use my msee fork (to try it: npm install --save ThibWeb/workshopper#latest-msee). No PR though because I'd rather see this being resolved without having to use git refs in place of proper version numbers, which means resolving the msee dependency question first.

I tried it with lololodash, count-to-6 and my WIP D3 workshopper without any issue. It also makes JSX look sort of ok ✨:

screen shot 2015-10-14 at 10 14 40

I'm very keen to make those changes available as part of this module, let me know what you think.

@martinheidegger
Copy link
Contributor

I just published workshopper-adventure version 4.0.x which does a very different job of rendering markdown code. Particularily with my custom version of msee ( 😈 ) To try it out check this out: workshopper/learnyounode#368

@martinheidegger
Copy link
Contributor

I updated the custom version of msee in the latest workshopper-adventure. and now it is not using visualwidth anymore. Ditched in favor of wcstring.

@thlorenz
Copy link

thlorenz commented Nov 9, 2015

Just wanted to let you know that redeyed is about to switch to latest esprima with ES6 support which will affect cardinal and thus msee as well.

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

No branches or pull requests

5 participants