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

Replace prettify.js with something maintained #1544

Closed
kraih opened this issue Aug 1, 2020 · 9 comments · Fixed by #1578
Closed

Replace prettify.js with something maintained #1544

kraih opened this issue Aug 1, 2020 · 9 comments · Fixed by #1578

Comments

@kraih
Copy link
Member

kraih commented Aug 1, 2020

pretty

For syntax highlighting on the exception page we still use prettify.js, which like most Google projects has been abandoned. So we should really replace it with something better, like highlight.js.

There was actually an attempt before, but it failed because of interdependencies with mojolicious.org. That is no longer a problem, because assets are not shared anymore.

For testing you can use the included example app examples/responses.pl (perl -Ilib examples/responses.pl daemon). The screenshot above was taken of a request for http://127.0.0.1:3000/res5.

@zakame
Copy link
Contributor

zakame commented Oct 17, 2020

Let me try working on this 💪 Is highlight.js still preferred, or should we be also looking for other libraries?

@kraih
Copy link
Member Author

kraih commented Oct 17, 2020

No preference really, it just should be maintained and work well.

@zakame
Copy link
Contributor

zakame commented Oct 17, 2020

Cool, I just pulled in @dotandimet's old PR, pruning out the old PODRenderer-related changes and updated with master, and it seems ok:

image

Since its a 5-year-old PR though I'll check on the highlight.js itself for library upgrades and other changes...

@kraih
Copy link
Member Author

kraih commented Oct 17, 2020

Don't forget to check template/HTML highlighting too.

@zakame
Copy link
Contributor

zakame commented Oct 17, 2020

Yep, was just about to check in on that:

image

This is on latest highlight.js 10.2.1 a bit of tweaking on the CSS and debug.html.ep to follow the bare minimum instruction in https://highlightjs.org/usage . It seems smart enough to handle both template HTML and raw Perl code now without explicitly stating class in <pre><code> tags:

image

@kraih
Copy link
Member Author

kraih commented Oct 18, 2020

Would be nice to get mojolicious.org updated too.

@zakame
Copy link
Contributor

zakame commented Oct 24, 2020

Got around to trying this with mojolicious.org now, unforuntately it looks like the similar situation (or worse) in #737 (comment):

image

Types/package names and <script>stuff in</script> no longer highlight right, as compared to the original:

image

Has to do with the engine itself, not recognizing the right tokens:

image

vs

image

Guess I'll block #1578 for now and look at the other aforementioned alternative (prism.js) to check...

@kraih
Copy link
Member Author

kraih commented Oct 24, 2020

That does look quite a bit better than #737 (comment) at least.

@zakame
Copy link
Contributor

zakame commented Oct 24, 2020

Yeah, on second thought... checking on mojodocs now, it does seem that highlightjs is more consistent for the embedded templates, e.g. for https://docs.mojolicious.org/Mojolicious/Guides/Rendering#Template-inheritance:

image

vs

image

Even with the lack of highlights for types, for highlightjs at least it looks a lot less Angry Fruit Salad.

Let me post my current mojolicious.org changes now for further discussion 👍

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

Successfully merging a pull request may close this issue.

2 participants