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

Switch from a generator to a converter. #29

Closed
wants to merge 1 commit into from
Closed

Switch from a generator to a converter. #29

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 31, 2015

This allows us to process HTML rather than just markdown. This removes the need for #28 as it happens after the markdown conversion.

I took the code to deal with code blocks from gjtorikian/html-pipeline#163

Fixes #12
Fixes #5
Fixes #4

This allows us to process HTML rather than just markdown.

Fixes #12
Fixes #5
Fixes #4
@perlun
Copy link

perlun commented Nov 20, 2015

Does this work with Jekyll 3.0 also? I get warnings w/ 3.0, and the generated html is completely messed up...

@ghost
Copy link
Author

ghost commented Nov 20, 2015

@perlun What warnings and markup are you getting? We're using this here https://github.com/kinesisptyltd/tech.kinesis.org without any problems.

@perlun
Copy link

perlun commented Nov 21, 2015

This is what I'm seeing:

 Regenerating: 1 file(s) changed at 2015-11-21 14:46:48        Deprecation: Collection#each should be called on the #docs array directly.

The emoji replacement (so that e.g. :smile: is replaced by 😄) is also failing. By downgrading to jekyll 2.5, everything started working again - no warnings, and the emoji replacements are now there.

@perlun
Copy link

perlun commented Nov 21, 2015

(using jemoji 0.5.0)

@perlun
Copy link

perlun commented Nov 21, 2015

Confirmed that 0.5.1 works correctly with Jekyll 3.

@ghost
Copy link
Author

ghost commented Nov 21, 2015

Sorry thought you meant this PR. 0.5.1 definitely works with 3 and 2.

site.pages.each { |doc| emojify doc }
site.docs_to_write.each { |doc| emojify doc }
def matches(ext)
ext == ".html"
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't match .md or .textile or any other formats that convert into HTML. :/

Copy link
Author

Choose a reason for hiding this comment

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

Is there a way to control the order of conveters in Jekyll? Because afacit this happens completely after all other processing and is performed directly on the HTML.

@ghost
Copy link
Author

ghost commented Nov 22, 2015

Hrm this is trickier than I thought. Only processing the HTML at the end still doesn't fix #12. Keeping this as a generator and going the #28 route will solve #4 (just call the filter in includes) and #5. But #12 will still be an issue and I can't think of a way to solve that without doing what is suggested in that issue and that's creating a MarkdownEmojiConverter.

@parkr
Copy link
Member

parkr commented Mar 8, 2016

We fixed this in #33 by using Hooks! Thanks 😄

@parkr parkr closed this Mar 8, 2016
@jekyll jekyll locked and limited conversation to collaborators Apr 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

emoji shouldn't be compiled in code block Filter? Verify include support
4 participants