Add HTML::Pipeline to core #1760

Closed
wants to merge 4 commits into
from

3 participants

@gjtorikian
Jekyll member

HTML::Pipeline is the 🐝's knees. One day @benbalter was like

what are your thoughts on submitting html-pipeline to Jekyll core as a markdown renderer

and I was all

i am very much 👍 for it

So here we go.

Much to my surprise this actually works pretty well. I took a cue from how nanoc accepts filters, wherein you can basically pass in an array of filter names, and The System will figure out what you mean.

I can't figure out how to properly test for failures (they're written in the test), so help on that would be much appreciated.

Todo:

  • Write test for mentions
  • Actually test for failing items
  • Support custom filters Will probably need to be handled by a plugin, not core.
  • Update docs to list html_pipeline defaults

/cc @parkr

@mattr- mattr- and 1 other commented on an outdated diff Nov 27, 2013
lib/jekyll/converters/markdown.rb
@@ -19,7 +21,7 @@ def setup
MarukuParser.new @config
else
STDERR.puts "Invalid Markdown processor: #{@config['markdown']}"
- STDERR.puts " Valid options are [ maruku | rdiscount | kramdown | redcarpet ]"
+ STDERR.puts " Valid options are [ html_pipeline | maruku | rdiscount | kramdown | redcarpet ]"
@mattr-
Jekyll member
mattr- added a line comment Nov 27, 2013

I don't believe there was any sort of ordering in the processor list here, but let's leave maruku as the first one since it's still the default.

@gjtorikian
Jekyll member
gjtorikian added a line comment Nov 27, 2013

This is spat out of an STDERR message, I don't know how it'd affect any processing. But I'll change it.

@mattr-
Jekyll member
mattr- added a line comment Nov 27, 2013

You're right. This doesn't affect any processing. My only concern is if it's listed first then it will be assumed that it's the default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@mattr- mattr- commented on an outdated diff Nov 27, 2013
lib/jekyll/converters/markdown/html_pipeline_parser.rb
+
+ def filter_key(s)
+ s.to_s.downcase.to_sym
+ end
+
+ def is_filter?(f)
+ f < HTML::Pipeline::Filter
+ rescue LoadError, ArgumentError
+ false
+ end
+
+ def ensure_default_opts
+ @config['html_pipeline']['filters'] ||= ['markdownfilter']
+ @config['html_pipeline']['context'] ||= {'gfm' => true}
+ # symbolize strings as keys, which is what HTML::Pipeline wants
+ @config['html_pipeline']['context'] = @config['html_pipeline']['context'].inject({}){|memo,(k,v)| memo[k.to_sym] = v; memo}
@mattr-
Jekyll member
mattr- added a line comment Nov 27, 2013

We have an existing method called symbolize_keys for this. Could you use that instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@mattr-
Jekyll member

Looks great so far. Thanks! ❤️

@parkr parkr and 1 other commented on an outdated diff Nov 27, 2013
jekyll.gemspec
@@ -26,6 +26,7 @@ Gem::Specification.new do |s|
s.add_runtime_dependency('liquid', "~> 2.5.2")
s.add_runtime_dependency('classifier', "~> 1.3")
s.add_runtime_dependency('listen', "~> 1.3")
+ s.add_runtime_dependency('html-pipeline', "~> 1.0.0")
@parkr
Jekyll member
parkr added a line comment Nov 27, 2013

If it's not a default, I'd prefer to keep it as a specify-it-if-you-want-it gem.

@gjtorikian
Jekyll member
gjtorikian added a line comment Nov 27, 2013

Fair enough. Changing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@parkr
Jekyll member

Yay!

@mattr-
Jekyll member

@gjtorikian Were you planning on finishing this out or should we just consider it as it currently is and work on the remaining items in the task list later?

@parkr
Jekyll member

@mattr- We're going to get custom markdown processors working such that this could be a plugin for the future. Thoughts?

@gjtorikian
Jekyll member

@mattr- Yeah, I'm kind of waiting for #1213 to land first. I may just cherry-pick those commits if @envygeeks doesn't get to it first!

@mattr-
Jekyll member

👍 all around for custom markdown processors, etc. 😃

@gjtorikian
Jekyll member

Shut down in favor of #1871

@gjtorikian gjtorikian closed this Dec 25, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment