Skip to content
This repository was archived by the owner on Jun 27, 2018. It is now read-only.

Add a Pygments lexer for MIR output, and fix HTML escaping. #200

Merged
merged 1 commit into from
Apr 29, 2016

Conversation

birkenfeld
Copy link
Contributor

@birkenfeld birkenfeld commented Apr 27, 2016

Also fixes the problem of insufficient HTML escaping of MIR output
by neither highlighting nor escaping it.

Also fixes the problem of insufficient HTML escaping of MIR output
in the playpen's result pane by neither highlighting nor escaping it.
@birkenfeld birkenfeld changed the title Add a Pygments lexer for MIR output. Add a Pygments lexer for MIR output, and fix HTML escaping. Apr 27, 2016
@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@jonas-schievink
Copy link
Contributor

This is going to be... interesting to rewrite in Rust for #187

@birkenfeld
Copy link
Contributor Author

Well, as it happens I was just beginning to port parts of Pygments to Rust. I could probably have a version with the three lexers in question ready pretty soon.

@birkenfeld
Copy link
Contributor Author

Regardless, could the escaping bug be fixed now at least? It is actively hurting readability of the MIR output.

@jonas-schievink
Copy link
Contributor

Oh, please don't hold back features because of the rewrite. I'm fine with either waiting for your Pygments port or writing a temporary highlighter for #187.

@alexcrichton
Copy link
Member

Thanks @birkenfeld! Sounds reasonable to merge for now and tackle in the rewrite in #187 later.

@jonas-schievink also I still hope to get around to taking a closer look soon, sorry it's taking awhile :(

@alexcrichton alexcrichton merged commit 917eb9f into rust-lang:master Apr 29, 2016
@alexcrichton
Copy link
Member

Ah unfortunately it looks like this breaks the MIR button as it also requires some JS changes, perhaps those could be prep'd as well?

@birkenfeld
Copy link
Contributor Author

Sorry - looks like I'll have to set up a local instance after all. Stand by...

@birkenfeld birkenfeld deleted the mir-highlighting branch April 29, 2016 09:12
@NotaseCretagen
Copy link

NotaseCretagen commented Apr 29, 2016

(Side note: Thanks Alex for restarting play-pen, I've been waiting for that feature when you press Esc key after clicking Run button, to kick in for a while now :D EDIT: confirmed working on both firefox(46.0) and chromium(52.0.2716.0))

@alexcrichton
Copy link
Member

@NotaseCretagen oh oops! Feel free to ping me if it ever needs a restart and I'm not getting around to it

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

Successfully merging this pull request may close these issues.

6 participants