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

Variable {% include %} #1495

Merged
merged 5 commits into from
Oct 4, 2013
Merged

Variable {% include %} #1495

merged 5 commits into from
Oct 4, 2013

Conversation

maul-esel
Copy link

This PR adds support for including files by giving a variable to the include tag. It uses liquid-like syntax:

{% include {{variable}} %}

This fixes #1464.

Todo:

  • implement
  • add cucumber feature
  • add documentation
  • rebase on current master

@maul-esel
Copy link
Author

Note that spaces inside brackets are not allowed!

@parkr
Copy link
Member

parkr commented Sep 2, 2013

Interesting! This is really cool.

def render(context)
includes_dir = File.join(context.registers[:site].source, '_includes')

return error if error = validate_file(includes_dir)
if error = retrieve_variable(context) || validate_file(includes_dir)
Copy link
Member

Choose a reason for hiding this comment

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

What if we had the syntax

{% include site.sidebar_include %}

and just checked for the filename, fell back to the variable and then finally failed if both failed?

Copy link
Author

Choose a reason for hiding this comment

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

I thought about that too but I'd personally prefer the clarity of a different syntax for variables and literal file names - to differentiate them to the user, or anyone trying to understand some site's code (maintainability?).

I don't know, it would just feel kinda wrong to me... 😄 And it would also add complexity to the code, when differentiating between a file that can't be found (fallback) or there is a symlink involved (error) etc.

If you would like me to change it nevertheless, just tell me.

@mattr-
Copy link
Member

mattr- commented Sep 3, 2013

What's the use case for this? In particular, under what circumstance would I want a variable instead of a filename for an include tag?

@parkr
Copy link
Member

parkr commented Sep 3, 2013

@mattr- One example is this:

{% for post in site.posts %}
  {% include {{post.sidebar}} %}
{% endfor %}

Another is:

{% for year in (1983..2013) %}
  {% capture filename %}alumni/{{year}}.html{% endcapture %}
  {% include {{filename}} %}
{% endfor %}

@mattr-
Copy link
Member

mattr- commented Sep 3, 2013

Examples are one thing. Use cases are
another. 😝

In this case, I can extract the use case(s) from the examples, which is the
ability to have parameterized includes. Thanks. 😃

On Tue, Sep 3, 2013 at 2:10 PM, Parker Moore notifications@github.comwrote:

@mattr- https://github.com/mattr- One example is this:

{% for post in site.posts %}
{% include {{post.sidebar}} %}
{% endfor %}

Another is:

{% for year in (1983..2013) %}
{% capture filename %}alumni/{{year}}.html{% endcapture %}
{% include {{filename}} %}
{% endfor %}


Reply to this email directly or view it on GitHubhttps://github.com//pull/1495#issuecomment-23738752
.

@parkr
Copy link
Member

parkr commented Sep 4, 2013

So are you 👍?

@doktorbro
Copy link
Member

This feature will turn includes to beasts. Looks very promising.

@mattr-
Copy link
Member

mattr- commented Sep 7, 2013

@parkr Yeah. I'm 👍 Was just trying to give you crap. 😝

@mattr-
Copy link
Member

mattr- commented Sep 9, 2013

@maul-esel Would you mind rebasing this on top of current master? Thanks!

@maul-esel
Copy link
Author

@mattr-: Rebased!

def retrieve_variable(context)
if /\{\{([\w\-\.]+)\}\}/ =~ @file
return "No variable #{$1} was found in include tag" if context[$1].nil?
@file = context[$1]
Copy link
Member

Choose a reason for hiding this comment

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

what if the context doesn't contain $1?

Copy link
Author

Choose a reason for hiding this comment

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

See the line before?

Copy link
Member

Choose a reason for hiding this comment

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

Nope! Haha, that's what I get for reviewing code tired and distracted. Sorry!

@maul-esel
Copy link
Author

Should be ready to 🚢!

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 3d3295b29e965637fab0d2ee1942af541988c2fa on maul-esel:variable-include into * on mojombo:master*.

@robwierzbowski
Copy link

+1! Was just asking if this was possible in IRC.

My use case is building a section of content dynamically based on yml schema. The yml contains references to includes for each part of the piece of content, and then a template does (would do)

{% for part in content %}
  {% include {{part.type}} content="{{ part.content }}" %}
{% endfor %}

@parkr
Copy link
Member

parkr commented Sep 17, 2013

@maul-esel I'll take a look at this and we'll get it shipped soon! I'll probably merge @penibelst's PR first then add this in after. Thanks for doing this!!

@robwierzbowski Yep! The only thing you'll have to change is the way you assign content:

{% for part in content %}
  {% include {{part.type}} content=part.content %}
{% endfor %}

Quotes denote a string. No quotes means it'll try for a variable first.

@robwierzbowski
Copy link

Perfect [?].

@migreyes
Copy link

Really excited about this one. 👏 👏 👏

@parkr
Copy link
Member

parkr commented Sep 29, 2013

@mattr- I'd love to see this in v1.3. What do you think?

@maul-esel Would you mind rebasing? Seems there is a conflict.

@maul-esel
Copy link
Author

I had hoped to rebase this on top of #1514. Anyway, I can rebase it the other way round. When are you going to release 1.3? I'm not on my usual development machine right now, and it might take me a few days to get everything running and so on.

@parkr
Copy link
Member

parkr commented Sep 29, 2013

@maul-esel If you'd prefer we make a call on #1514 first, we can do that. We probably won't release v1.3.0 for a few weeks.

@mattr-
Copy link
Member

mattr- commented Oct 1, 2013

Last three weeks have been a blur. Once it's been rebased, let's merge it. 😃

@mattr-
Copy link
Member

mattr- commented Oct 1, 2013

If you want to rebase on top of #1514 now, you can. It just landed in master. 😃

@maul-esel maul-esel deleted the variable-include branch October 4, 2013 12:59
@parkr
Copy link
Member

parkr commented Oct 4, 2013

🎉 YAY

@sferik
Copy link
Contributor

sferik commented Oct 4, 2013

This pull request appears to have broken the build with the following error:

(::) failed steps (::)
The directory "_site" does not exist (MiniTest::Assertion)
./features/step_definitions/jekyll_steps.rb:147:in `/^the (.*) directory should +exist$/'
features/include_tag.feature:47:in `Then the _site directory should exist'
Failing Scenarios:
cucumber -p travis features/include_tag.feature:37 # Scenario: Include a file from a variable

@parkr
Copy link
Member

parkr commented Oct 4, 2013

Houston, we've got a problem:

features/include_tag.feature:37 is failing due to this error:

Configuration file: /private/tmp/jekyll/_config.yml
            Source: /private/tmp/jekyll
       Destination: /private/tmp/jekyll/_site
      Generating...  Liquid Exception: Invalid syntax for include tag.
error: Invalid syntax for include tag. File contains invalid characters or sequences:

    {{site.include_file1}}

Valid syntax:

    {% include file.ext param='value' param2='value' %}

Use --trace to view backtrace

@AJ-Acevedo
Copy link
Contributor

This feature and the _data feature are my two most anticipated features in v1.3!

@jedfoster
Copy link

This is exactly what I wanted 2 years ago, when I first started playing with Jekyll. The lack of this feature was a deal breaker. Thank you!

However... I'm running into an issue where the first included file is included for all iterations in a loop.

This:

{% for project in site.data.work %}
  <section id="{{ project.id }}">
    {% include {{project.file}} %}
  </section>
{% endfor %}

Generates:

<section id="roughdraft">
<img src="/img/portfolio/sites/roughdraft-2-sm.png" alt="" />
<h1><a href="http://www.roughdraft.io">Roughdraft.io</a></h1>
<p>A micro-publishing platform backed by GitHub Gists. Written in Ruby, using Sinatra. Deployed to Heroku.</p>
</section>
<section id="sassmeister">
<img src="/img/portfolio/sites/roughdraft-2-sm.png" alt="" />
<h1><a href="http://www.roughdraft.io">Roughdraft.io</a></h1>
<p>A micro-publishing platform backed by GitHub Gists. Written in Ruby, using Sinatra. Deployed to Heroku.</p>
</section>
<section id="la-amistad-dental">
<img src="/img/portfolio/sites/roughdraft-2-sm.png" alt="" />
<h1><a href="http://www.roughdraft.io">Roughdraft.io</a></h1>
<p>A micro-publishing platform backed by GitHub Gists. Written in Ruby, using Sinatra. Deployed to Heroku.</p>
</section>

It's as though project.file isn't being reset on subsequent iterations. I'm using Jekyll 1.3.0

@floatingboxes
Copy link

I'm having the same issue as @jedfoster ...

This bit is in my front matter:

 ---
 sidebars:
  - about-the-blog
  - recent-posts
  - helpful-links
 ---

Then in my layout I have something like:

 {% for sidebar in page.sidebars %}
   {% assign path = '' %}
   {% capture path %}sidebars/{{sidebar}}.html{% endcapture %}
   {% include {{path}} %}
 {% endfor %}

However I just get sidebars/about-the-blog.html included three times.

If I just output the path (sans include) like this...

 {% for sidebar in page.sidebars %}
   {% assign path = '' %}
   {% capture path %}sidebars/{{sidebar}}.html{% endcapture %}
   {{path}} <br/>
 {% endfor %}

Then I get...

 sidebars/about-the-blog.html
 sidebars/recent-posts.html
 sidebars/helpful-links.html

@maul-esel
Copy link
Author

That's because the include tag retrieves the variables value the first time, and then forgets it came from a variable and not from an explicit path. I'll look into this.

maul-esel pushed a commit to maul-esel/jekyll that referenced this pull request Nov 14, 2013
This fixes the bug reported in jekyll#1495 (comments).
csim pushed a commit to csim/jekyll that referenced this pull request Nov 22, 2013
This fixes the bug reported in jekyll#1495 (comments).
@yevon-cn
Copy link

It seems Filters can't be use in the variable as follow ?

{% include {{ _temp | prepend: 'analysis/' }} %}

@yevon-cn
Copy link

I have the same issue as @floatingboxes @jedfoster , it is really a big bug. Hope the new version come soon.

@maul-esel
Copy link
Author

@yevon-cn: no, filters can't be used, you'll have to use an assign tag to implement sth. like this.

@yevon-cn
Copy link

@maul-esel Thank you, I see.

@exoton
Copy link

exoton commented Mar 15, 2014

The documentation at http://jekyllrb.com/docs/templates/ has spaces inside brackets around variable name in ProTip: {% include {{ my_variable }} %}
Just spent some time trying to figure out why it does not work until found this page.

@mattr-
Copy link
Member

mattr- commented Mar 15, 2014

@Yuraki Are you saying that the documentation is wrong because it has the spaces?

@exoton
Copy link

exoton commented Mar 15, 2014

Yes, I am. I was following the documentation and could not understand why I was getting an error:

Liquid Exception: Invalid syntax for include tag: f }} Valid syntax: {% include file.ext param='value' param2='value' %}

until I've found this page with a comment by maul-esel: Note that spaces inside brackets are not allowed!

@jens-na
Copy link

jens-na commented Mar 15, 2014

The documentation is not wrong. Spaces are allowed since pull request #1841.

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

PS:
Here is a file which shows the possibilities.

@exoton
Copy link

exoton commented Mar 15, 2014

I see. Thanks. I guess it's not a part of the latest release 1.4.3. So technically... it will become correct soon.

@jens-na
Copy link

jens-na commented Mar 15, 2014

You're right, but as far as I know it should already be included in the 2.0.0.alpha.1 release which you can install with gem install jekyll --pre.

@mattgreenfield mattgreenfield mentioned this pull request Jan 3, 2016
@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.

Inclusion using assignment