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.

Sign up for GitHub

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

Jump to bottom

Replace prettify.js with highlight.js #22

Merged
merged 1 commit into from
Nov 6, 2020
Merged

Replace prettify.js with highlight.js #22

merged 1 commit into from
Nov 6, 2020

Conversation

zakame
Copy link
Contributor

@zakame zakame commented Oct 24, 2020 •
edited
Loading

Summary

This replaces the old prettify.js highlighter with highlight.js.

Motivation

prettify.js has long been abandoned by Google so we need to replace it.

References

mojolicious/mojo#1544
mojolicious/mojo#1578

@zakame zakame force-pushed the highlightjs branch 2 times, most recently from ec6fdd5 to 43c1e46 Compare October 24, 2020 19:03
@marcusramberg
Copy link
Member

Will this change how things look at all? Any chance of a screenshot? The markup looks cleaner to me.

@zakame
Copy link
Contributor Author

zakame commented Oct 29, 2020

Will this change how things look at all? Any chance of a screenshot? The markup looks cleaner to me.

There's a few here:

I can add a few more here, hold on.

@zakame
Copy link
Contributor Author

zakame commented Oct 29, 2020

From Mojo::Template:

image

From Mojolicious::Guides::Cookbook:

image

From Mojo::UserAgent:

image

@kraih
Copy link
Member

kraih commented Oct 29, 2020

So the diagrams are broken now, that's unfortunate.

@zakame
Copy link
Contributor Author

zakame commented Oct 29, 2020

Let me check on fixing those diagrams. Here's the (unstyled) current prettify version of https://docs.mojolicious.org/Mojolicious/Guides/Cookbook for reference:

image

@zakame zakame mentioned this pull request Oct 29, 2020
Merged
@zakame zakame requested review from kraih and jhthorsen October 29, 2020 19:20
@zakame
Copy link
Contributor Author

zakame commented Oct 29, 2020

Updated, this also bumps highlight.js to 10.3.2 per https://github.com/highlightjs/highlight.js/releases/tag/10.3.2 and also adding some plugins for Cookbook listings (e.g. Apache/Nginx/Systemd/YAML configs.)

kraih
kraih previously approved these changes Oct 29, 2020
View reviewed changes
@kraih kraih requested a review from a team October 29, 2020 19:34
@kraih kraih dismissed their stale review October 29, 2020 20:12

Missed issue.

@zakame zakame requested review from Grinnz and kraih and removed request for a team October 30, 2020 05:45
@kraih kraih requested a review from a team October 30, 2020 10:28
kraih
kraih previously approved these changes Oct 30, 2020
View reviewed changes
@kraih kraih requested a review from a team October 30, 2020 18:42
jhthorsen
jhthorsen reviewed Oct 30, 2020
View reviewed changes
templates/layouts/mojolicious.html.ep Outdated Show resolved Hide resolved
templates/layouts/mojolicious.html.ep
%= javascript '/mojolicious/highlight.js/nginx.min.js'
%= javascript '/mojolicious/highlight.js/yaml.min.js'
%= stylesheet '/mojolicious/highlight.js/highlight-mojo-light.css'
<script>hljs.initHighlightingOnLoad();</script>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it’s better to start highlighting right before instead of onload()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just following what's on the tin: https://highlightjs.org/usage/

There's also option for customizing the load, but it still attaches to DOMContentLoaded; let me know if you have a better suggestion.

@kraih kraih dismissed their stale review October 30, 2020 21:57

Outdated.

@zakame zakame requested review from kraih and jhthorsen October 31, 2020 08:27
@zakame
Use highlight.js for syntax coloring
9605ae4 
* Add highlight.js, highlight-mojo-light.css and plugins

Start of try replacing prettify.js with highlight.js

* Update templates for highlight.js

* Mojolicious::Plugin::MojoDocs: Revise highlighting logic for code blocks

Filter out code blocks we want highlighted, then apply "nohighlight" to
those we don't, while preserving existing CSS classes (if any.)

* Remove prettify.js and css

Thanks for the service!
@zakame
Copy link
Contributor Author

zakame commented Oct 31, 2020

Updated to rebundle highlight + plugins used here as one file and rebased to latest.

@kraih kraih requested a review from a team October 31, 2020 13:45
@kraih kraih requested a review from a team November 4, 2020 18:41
Grinnz
Grinnz approved these changes Nov 6, 2020
View reviewed changes
Copy link
Contributor

@Grinnz Grinnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mergify mergify bot merged commit 3cec09d into mojolicious:master Nov 6, 2020
@zakame
Copy link
Contributor Author

zakame commented Nov 6, 2020

Thanks again everyone! 🎉

@zakame zakame deleted the highlightjs branch November 6, 2020 20:11
@kraih
Copy link
Member

kraih commented Nov 6, 2020

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Grinnz Grinnz Grinnz approved these changes

@kraih kraih kraih approved these changes

@jhthorsen jhthorsen Awaiting requested review from jhthorsen

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@zakame @marcusramberg @kraih @jhthorsen @Grinnz