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

variables in include tag with liquid filters #1841

Merged
merged 4 commits into from
Dec 26, 2013

Conversation

jens-na
Copy link

@jens-na jens-na commented Dec 17, 2013

Use variable {% include %} with filters
This pull request adds support for filters when using the include tag with a variable.
To be more precise:

Since PR #1495 it is possible to use variables inside an include tag, like that:

    {% for item in page.dependencies %}
        {% include {{item}} %}
    {% endfor %}

but unfortunately you cannot use filters.

With this pull reuqest you can use something like this:

    {% for item in page.dependencies %}
        {% include {{ item | append: '.html' }} %}
    {% endfor %}

This is especially useful when you want to modify the file/filename which should get included.

Possible use cases

Append .html to the file name

{% include {{ item | append: '.html' }} %}

Make sure the file name is lowercase

{% include {{ item | downcase | append: '.html' }} %}

Prepend a directory to choose another directory inside the _includes/ directory

{% include {{ item | downcase | prepend: '/dependencies/' | append: '.html' }} %}
==> ./_includes/dependencies/<item>.html

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 43ef9a2 on jens-na:include-variable-liquid-filters into 12a55b8 on jekyll:master.

@parkr
Copy link
Member

parkr commented Dec 17, 2013

/cc @maul-esel

elsif File.symlink?(file) && safe
raise IOError.new "The included file '#{INCLUDES_DIR}/#{@file}' should not be a symlink"
raise IOError.new "The included file '#{INCLUDES_DIR}/#{file}' should not be a symlink"

Choose a reason for hiding this comment

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

If you want to use file to output the real path and not the variable etc. (in case it is one), that's ok. But as file already contains the full path in this method, you might wanna remove the #{INCLUDES_DIR} part in this message and the one above 😉

@maul-esel
Copy link

Looks great! Just the one comment above.

@maul-esel
Copy link

Oh and also, as we discussed in #1789, this fixes cases like {% include {{ var }} %} (spaces inside brackets), so you might wanna remove the note concerning this from the docs (here or in a separate PR / commit once this is merged).

@coveralls
Copy link

Coverage Status

Coverage increased (+0.42%) when pulling 412fe8b on jens-na:include-variable-liquid-filters into 12a55b8 on jekyll:master.

@maul-esel
Copy link

👍

@coveralls
Copy link

Coverage Status

Coverage increased (+0.24%) when pulling 412fe8b on jens-na:include-variable-liquid-filters into 12a55b8 on jekyll:master.

@parkr
Copy link
Member

parkr commented Dec 19, 2013

Looking great! Would you please add a test for multiple filters so we know that one works, too? :)

@jens-na
Copy link
Author

jens-na commented Dec 19, 2013

Do you mean a cucumber test?
I have already added several tests in test/test_tags.rb (including a test for multiple filters)

Specifically I test something like this:

---
include3: INCLUDE
---
{% include {{ page.include3 | downcase | append: '.html' }} %}

@parkr
Copy link
Member

parkr commented Dec 19, 2013

Indeed! Thanks.

This LGTM. @mattr-?

@parkr parkr merged commit 6e8f31f into jekyll:master Dec 26, 2013
parkr added a commit that referenced this pull request Dec 26, 2013
@jens-na jens-na mentioned this pull request Mar 15, 2014
4 tasks
@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

5 participants