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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Render Liquid in CoffeeScript files #2830

Merged
merged 6 commits into from Aug 29, 2014

Conversation

Projects
None yet
4 participants
@kansaichris

Closes #2792. Is it really this simple?

Incidentally, it seems a bit odd for render_with_liquid? to always return true, but I guess it's nice to keep the function around just in case we need to disable Liquid for some other file type in the future.

/cc @gjtorikian and @parkr for review 馃檱

@kansaichris

This comment has been minimized.

Show comment
Hide comment
@kansaichris

kansaichris Aug 26, 2014

Whoops, forgot to fix the Cucumber tests. I'll get to that as soon as I can. 馃槉

Whoops, forgot to fix the Cucumber tests. I'll get to that as soon as I can. 馃槉

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Aug 26, 2014

Member

LGTM 馃槃 @gjtorikian, can I get a 馃憤 or 馃憥 ?

Member

parkr commented Aug 26, 2014

LGTM 馃槃 @gjtorikian, can I get a 馃憤 or 馃憥 ?

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Aug 26, 2014

Member

@kansaichris would you also please add docs to site/_docs/assets.md? A big warning note about liquid and template languages (like mustache) would be great. There may also be a note about coffeescript not being rendered w/Liquid that can be removed.

Thank you!

Member

parkr commented Aug 26, 2014

@kansaichris would you also please add docs to site/_docs/assets.md? A big warning note about liquid and template languages (like mustache) would be great. There may also be a note about coffeescript not being rendered w/Liquid that can be removed.

Thank you!

@gjtorikian

This comment has been minimized.

Show comment
Hide comment
@gjtorikian

gjtorikian Aug 26, 2014

Member

Yeah, adding docs would be huge.

Member

gjtorikian commented Aug 26, 2014

Yeah, adding docs would be huge.

@kansaichris

This comment has been minimized.

Show comment
Hide comment
@kansaichris

kansaichris Aug 27, 2014

Here's the CI build error, for what it's worth:

features/drafts.feature:16  Scenario: Don't preview a draft ......There was an error highlighting your code: 
---
---
// start content
.my-definition
  font-size: 1.2em 
While attempting to convert the above code, Pygments.rb returned an unacceptable value. 
This is usually a timeout problem solved by running `jekyll build` again. 
  Liquid Exception: Pygments.rb returned an unacceptable value when attempting to highlight some code. in _docs/assets.md

Here's the CI build error, for what it's worth:

features/drafts.feature:16  Scenario: Don't preview a draft ......There was an error highlighting your code: 
---
---
// start content
.my-definition
  font-size: 1.2em 
While attempting to convert the above code, Pygments.rb returned an unacceptable value. 
This is usually a timeout problem solved by running `jekyll build` again. 
  Liquid Exception: Pygments.rb returned an unacceptable value when attempting to highlight some code. in _docs/assets.md
@kansaichris

This comment has been minimized.

Show comment
Hide comment
@kansaichris

kansaichris Aug 27, 2014

would you also please add docs to site/_docs/assets.md?

Done. 馃槃

@parkr For future reference, do you prefer documentation updates to be submitted in the same pull request as the affected code or separately?

would you also please add docs to site/_docs/assets.md?

Done. 馃槃

@parkr For future reference, do you prefer documentation updates to be submitted in the same pull request as the affected code or separately?

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Aug 27, 2014

Member

@parkr For future reference, do you prefer documentation updates to be submitted in the same pull request as the affected code or separately?

I prefer documentation updates to be submitted in the same pull request. 馃槃 Check out the first line of CONTRIBUTING.markdown.

Member

parkr commented Aug 27, 2014

@parkr For future reference, do you prefer documentation updates to be submitted in the same pull request as the affected code or separately?

I prefer documentation updates to be submitted in the same pull request. 馃槃 Check out the first line of CONTRIBUTING.markdown.

the <a href="/docs/templates/">Liquid template syntax</a>, you
will need to place <code>{&#37; raw &#37;}</code> and
<code>{&#37; endraw &#37;}</code> tags around your code.</p>
</div>

This comment has been minimized.

@parkr

parkr Aug 27, 2014

Member

@kansaichris

This comment has been minimized.

Show comment
Hide comment
@kansaichris

kansaichris Aug 27, 2014

Check out the first line of CONTRIBUTING.markdown.

How could I have missed that. 馃槱

Should I add anything else here, or does this look like it's good to go?

Check out the first line of CONTRIBUTING.markdown.

How could I have missed that. 馃槱

Should I add anything else here, or does this look like it's good to go?

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Aug 28, 2014

Member

Should I add anything else here, or does this look like it's good to go?

Looks good to me! 馃槃

Member

parkr commented Aug 28, 2014

Should I add anything else here, or does this look like it's good to go?

Looks good to me! 馃槃

@kansaichris

This comment has been minimized.

Show comment
Hide comment
@kansaichris

kansaichris Aug 29, 2014

@parkr Could I get a merge, or are we waiting on @gjtorikian? 馃槈

@parkr Could I get a merge, or are we waiting on @gjtorikian? 馃槈

@gjtorikian

This comment has been minimized.

Show comment
Hide comment
@gjtorikian

gjtorikian Aug 29, 2014

Member

I have nothing to do! I can't merge it.

Member

gjtorikian commented Aug 29, 2014

I have nothing to do! I can't merge it.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Aug 29, 2014

Member

I just wanted a 馃憤 or 馃憥 from Garen haha it's cool man, I'll merge tonight.

Member

parkr commented Aug 29, 2014

I just wanted a 馃憤 or 馃憥 from Garen haha it's cool man, I'll merge tonight.

@gjtorikian

This comment has been minimized.

Show comment
Hide comment
@gjtorikian

gjtorikian Aug 29, 2014

Member

馃憤 馃榿 all this process!

Member

gjtorikian commented Aug 29, 2014

馃憤 馃榿 all this process!

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Aug 29, 2014

Member

Sorrryyyyy!!

Member

parkr commented Aug 29, 2014

Sorrryyyyy!!

@parkr parkr merged commit 190ab7f into jekyll:master Aug 29, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@kansaichris kansaichris deleted the kansaichris:render-liquid-in-coffeescript branch Aug 29, 2014

@kansaichris

This comment has been minimized.

Show comment
Hide comment
@kansaichris

kansaichris Aug 29, 2014

Thanks @parkr! 鉂わ笍

Thanks @parkr! 鉂わ笍

parkr added a commit that referenced this pull request Aug 29, 2014

@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.