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

jemoji as a hook: better guarding against accidents #33

Merged
merged 7 commits into from
Mar 8, 2016
Merged

Conversation

parkr
Copy link
Member

@parkr parkr commented Mar 8, 2016

Fixes #12. /cc @jingweno
Fixes #4. /cc @benbalter
Replaces #29.

  • Register a Jekyll::Hook for pages & documents (which includes posts)
  • Use the whole HTML::Pipeline to guard against emojifying to broadly (for emoji shouldn't be compiled in code block #12)
  • Update specs
  • Lock to Jekyll 3 or greater
  • Only run on HTML docs

@benbalter
Copy link
Contributor

LGTM. Nice work. Should we add a test to ensure it doesn't emojify, e.g., in a code block?

@parkr
Copy link
Member Author

parkr commented Mar 8, 2016

Should we add a test to ensure it doesn't emojify, e.g., in a code block?

Yes, added in 0372356.

@benbalter, should we only emojify HTML documents?

@benbalter
Copy link
Contributor

should we only emojify HTML documents?

I'd think so (meaning documents/pages with a .html output extension), especially since the jemoji output is HTML 😄.

@parkr
Copy link
Member Author

parkr commented Mar 8, 2016

@jekyllbot: merge

jekyllbot added a commit that referenced this pull request Mar 8, 2016
@jekyllbot jekyllbot merged commit 0dc54bb into master Mar 8, 2016
@jekyllbot jekyllbot deleted the HOOKIFY branch March 8, 2016 21:13
jekyllbot added a commit that referenced this pull request Mar 8, 2016
parkr added a commit that referenced this pull request Mar 8, 2016
parkr added a commit that referenced this pull request 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 Verify include support
3 participants