Skip to content

remove highlight.js#1021

Closed
thaJeztah wants to merge 1 commit intodocker:masterfrom
thaJeztah:highlight-only-once
Closed

remove highlight.js#1021
thaJeztah wants to merge 1 commit intodocker:masterfrom
thaJeztah:highlight-only-once

Conversation

@thaJeztah
Copy link
Member

Code is already highlighted through "rouge", so
enabling highlight.js only resulted in code
being parsed/highlighted twice.

This patch removes highlight.js and updates
the "github" stylesheet to be compatible
with rouge.

/cc @joaofnfernandes @mstanleyjones @johndmulhausen

Code is already highlighted through "rouge", so
enabling highlight.js only resulted in code
being parsed/highlighted _twice_.

This patch removes highlight.js and updates
the "github" stylesheet to be compatible
with rouge.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
mdlinville
mdlinville previously approved these changes Jan 4, 2017
Copy link

@mdlinville mdlinville left a comment

Choose a reason for hiding this comment

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

LGTM from a quick perusal of a few pages I know have highlighted code...

@joaofnfernandes
Copy link
Contributor

Datacenter looks good. @sanscontext where's that page with all possible markdown permutations you've created? Is that page OK?

@sanscontext
Copy link
Contributor

sanscontext commented Jan 4, 2017

@joaofnfernandes
Copy link
Contributor

It seems that Rouge has support for Docker files in their latest version (2.0.7), but
we're importing Rouge through github-pages, which in turn imports Rouge 1.11.1.
https://github.com/jneen/rouge/blob/master/lib/rouge/lexers/docker.rb

@johndmulhausen
Copy link
Contributor

While I'd love to do this, Jekyll does not support the Rouge version where Dockerfile support was added (which was 2.0.4). As you see here, it's dependency is set to ~>1.7: https://rubygems.org/gems/jekyll

$ bundle update
Fetching gem metadata from https://rubygems.org/...........
Fetching version metadata from https://rubygems.org/..
Fetching dependency metadata from https://rubygems.org/.
Resolving dependencies...
Bundler could not find compatible versions for gem "rouge":
  In Gemfile:
    rouge (~> 2.0)

    jekyll (~> 3.3) was resolved to 3.3.0, which depends on
      rouge (~> 1.7)

It's not a GitHub Pages thing; no idea when Jekyll will get their act together.

@johndmulhausen
Copy link
Contributor

johndmulhausen commented Jan 4, 2017

Trying this Gemfile:

source "https://rubygems.org"

gem "rouge","2.0.7"
gem "jekyll-seo-tag"
gem "kramdown"
gem "jekyll-redirect-from"
gem "jekyll","3.3.1"

You get the incompatibility error when running bundle update

Trying this:

source "https://rubygems.org"

gem "rouge","2.0.7"
gem "jekyll-seo-tag"
gem "kramdown"
gem "jekyll-redirect-from"
gem "jekyll"

Succeeds but bundler reverts way back to Jekyll 2.5.3, probably because that was before it had dependency checking for Rouge: https://rubygems.org/gems/jekyll/versions/2.5.3

The very next version past 2.5.3, which was 3.0.0, was when Jekyll pinned its Rouge dependency at ~>1.7 and it has stayed there ever since:
https://rubygems.org/gems/jekyll/versions/3.0.0

@sanscontext
Copy link
Contributor

sanscontext commented Jan 5, 2017

In the mean time we can all watch this issue: jekyll/jekyll#5556

@mdlinville mdlinville dismissed their stale review February 1, 2017 00:39

Removing my +1 on this one so it doesn't get merged accidentally.

@thaJeztah thaJeztah mentioned this pull request Feb 8, 2017
@johndmulhausen
Copy link
Contributor

Closing in favor of #2757. We can use conf instead of native Dockerfile support for now.

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.

5 participants