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

Change liquid converter order #897

Closed

Conversation

jyruzicka
Copy link

Second attempt (see first). Now features less-terrible commits, more unit tests, and Converters::Include's conversion has been separated into its own method.

  • Liquid tags are now parsed after converters are run on files.
  • Liquid tags are encoded as HTML comments (sample:<!--LIQUID-a4ef62-2-->) during conversion, using the new LiquidEncoder class.
  • {% include %} liquid tag will now run included files through converters if any match their extension
  • Docs have been updated to explicitly mention order of parsing for files with YAML front-matter, and to mention that included files are converted if their extension matches.

@jyruzicka
Copy link
Author

Upon installing this on my local machine, maruku started spitting errors involving having raw html on a line with plaintext. Am also experiencing some inconsistencies with {% highlight %} liquid tags. Teach me to think the code is clean based purely on unit tests. I'll see if I can work out some fixes for these issues.

@parkr
Copy link
Member

parkr commented Apr 1, 2013

We'll have to wait until all the tests pass to consider this.

@jyruzicka
Copy link
Author

No worry. I've actually had to back off and approach this from a different angle anyway. Unit tests + travis passing locally, I'll chuck some info into the docs on how to do conversion before liquid tags are parsed, then issue a new (third) pull request.

This time.

@jyruzicka jyruzicka closed this Apr 1, 2013
@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017
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.

None yet

3 participants