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

Full support for options and extensions in HtmlRenderer #48

Merged
merged 12 commits into from
Jun 27, 2017

Conversation

kivikakk
Copy link
Collaborator

There was some incomplete support, this is just rounding it off and enabling it on all the same tests we put the normal renderer through.

Most of it doesn't actually need to be switched on or off specially — we rely on the fact that e.g. we'll only get a strikethrough or table_cell node if the parser had that enabled in the first place. The only switch we do have which is renderer-specific is tagfilter.

/cc @gjtorikian for review

@kivikakk kivikakk changed the title Full support for extensions in HtmlRenderer Full support for options and extensions in HtmlRenderer Jun 26, 2017
Copy link
Owner

@gjtorikian gjtorikian left a comment

Choose a reason for hiding this comment

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

Left a couple of comments but nothing terribly blocking. I think the only thing I'd really like to see is a method to parse the @opts inclusion.

@stream = StringIO.new("".force_encoding("utf-8"))
@need_blocksep = false
@warnings = Set.new []
@in_tight = false
@in_plain = false
@buffer = ''
@tagfilter = extensions.include?(:tagfilter)
Copy link
Owner

Choose a reason for hiding this comment

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

What's tagfilter? Is that from LIBERAL_HTML_TAG ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's the name of the tagfilter extension as bundled with cmark-gfm; we just detect the name of it here and emulate its behaviour.

end

def sourcepos(node)
if @opts & CommonMarker::Config::Render::SOURCEPOS != 0
Copy link
Owner

Choose a reason for hiding this comment

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

Rather than the if indentation I'd rather this bail early:

return "" unless @opts & CommonMarker::Config::Render::SOURCEPOS.zero?
...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍 That unless example is pretty messed up (logic backwards, missing parens) but I take your point :p

out('<pre><code')
if node.fence_info && !node.fence_info.empty?
out(' class="language-', node.fence_info.split(/\s+/)[0], '">')
if @opts & CommonMarker::Config::Render::GITHUB_PRE_LANG != 0
Copy link
Owner

Choose a reason for hiding this comment

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

Actually now that I see this pattern used a couple of times (one more below!), could we pull it out into a separate method? Like...

def option_included?(option)
  @opts & CommonMarker::Config::Render.const_get(option)
end

...

if  option_included?(GITHUB_PRE_LANG)
....

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

const_get gives me the heeby-jeebies but ruby-enum defines a value method which does the same thing.

@kivikakk kivikakk merged commit 94132e4 into master Jun 27, 2017
@kivikakk kivikakk deleted the kivikakk/extensions branch June 27, 2017 02:56
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.

2 participants