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

Allow passing :strict_variables and :strict_filters options to Liquid's renderer #6726

Merged
merged 7 commits into from Mar 14, 2018

Conversation

Projects
None yet
5 participants
@ashmaroli
Member

ashmaroli commented Jan 30, 2018

Resolves #6725

Allow users to enable detecting non-existent variables and filters specified in a template by setting ["liquid"]["strict_variables"] and ["liquid"]["strict_filters"] to true respectively in their config file.

  • initial implementation
  • unit test / cucumber feature
  • documentation

/cc @begincalendar, @iddan, @ianhinder

pass strict_* options to Liquid's renderer
allow users to enable detecting non-existent variables and filters
specified in a template by setting ["liquid"]["strict_variables"] and
["liquid"]["strict_filters"] to `true` respectively in their config file.

@ashmaroli ashmaroli requested a review from pathawks Jan 30, 2018

@DirtyF

DirtyF approved these changes Jan 30, 2018

@ashmaroli

This comment has been minimized.

Member

ashmaroli commented Jan 30, 2018

@DirtyF Don't merge this till I push a documentation commit..

@DirtyF

This comment has been minimized.

Member

DirtyF commented Jan 30, 2018

@ashmaroli feel free to put a WIP to prevent merging too soon 😉

Anyways it's better if @jekyll/build review this. Nice addition ❤️

@ashmaroli ashmaroli changed the title from Allow passing :strict_variables and :strict_filters options to Liquid's renderer to WIP: Allow passing :strict_variables and :strict_filters options to Liquid's renderer Jan 30, 2018

@parkr

I love this.

"""
When I run jekyll build
Then I should get a non-zero exit-status
And I should see "undefined filter foobar in index.html" in the build output

This comment has been minimized.

@parkr

parkr Jan 30, 2018

Member

Is a line number printed? Could we test for that?

"""
When I run jekyll build
Then I should get a non-zero exit-status
And I should see "undefined variable author in index.html" in the build output

This comment has been minimized.

@parkr

parkr Jan 30, 2018

Member

Is a line number printed? Could we test for that?

This comment has been minimized.

@ashmaroli
private
def liquid_options
@liquid_options ||= site.config["liquid"]

This comment has been minimized.

@parkr

parkr Jan 30, 2018

Member

These could be in lib/jekyll/liquid_renderer/file.rb and be merged into the info hash in that class to isolate liquid concerns from here to there? What do you think?

This comment has been minimized.

@ashmaroli

ashmaroli Jan 30, 2018

Member

Both LiquidRenderer and LiquidRenderer::File have no info hash.. Are you suggesting, I add it..?

This comment has been minimized.

@ashmaroli

ashmaroli Jan 30, 2018

Member

merge could slow down rendering especially when each call to renderer.run is going to call a merge.. so deferring that for the future..

@ashmaroli ashmaroli changed the title from WIP: Allow passing :strict_variables and :strict_filters options to Liquid's renderer to Allow passing :strict_variables and :strict_filters options to Liquid's renderer Jan 30, 2018

@oe

oe approved these changes Jan 30, 2018

to `true` respectively. {% include docs_version_badge.html version="3.8.0" %}
Do note that while `error_mode` configures Liquid's parser, the `strict_variables`
and `strict_filters` options configures Liquid's renderer and are consequently,

This comment has been minimized.

@oe

oe Jan 30, 2018

Member

configures -> configure

This comment has been minimized.

@ashmaroli

ashmaroli Jan 31, 2018

Member

Nice catch..!! 😃

@ashmaroli ashmaroli added this to the v3.8.0 milestone Jan 31, 2018

@oe oe referenced this pull request Feb 17, 2018

Closed

Release 3.8.0 #6783

12 of 13 tasks complete
@oe

This comment has been minimized.

Member

oe commented Mar 14, 2018

@jekyllbot: merge +minor

@jekyllbot jekyllbot merged commit 51bdea1 into jekyll:master Mar 14, 2018

3 checks passed

WIP ready for review
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ashmaroli ashmaroli deleted the ashmaroli:liquid-render-options branch Mar 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment