I like it, thanks @rebornix!

@envygeeks Your suggestion is awesome! This function turns out to be a simple if/else block

Guys, I just gather some stats for this enhancement. To make the stats data more intuitive, I separate parsing time from total rendering time when build site with --profile option.

Take Jekyll official docs for example, before caching include files, includes are parsed hundred times and part of them take about 100ms.

After this patch is applied, include files parsing time decrease significantly like below,

Even though the total build time of Jekyll docs only decrease about 5%, but sites heavily using _includes in their layout may benefit a lot 😃

I'm 👍 one this, just waiting for others to review.

/cc @jekyll/core

Thanks!

Spotted an issue using this when combined with jekyll-assets (and perhaps other plugins/examples where liquid is parsed within liquid). Example, when I use the following code:

{% for item in items %}
    {{ item.title | slugify }}
    {% asset_source "c-item__logo--{{ item.title | slugify }}.svg" %}
{% endfor %}

The following is produced:

24_ways
<svg class="24ways" xmlns="http://www.w3.org/2000/svg" width="48" height="48" viewBox="0 0 960 960">... </svg>

unicef_uk
<svg class="24ways" xmlns="http://www.w3.org/2000/svg" width="48" height="48" viewBox="0 0 960 960">... </svg>

bradshaw_s_guide
<svg class="24ways" xmlns="http://www.w3.org/2000/svg" width="48" height="48" viewBox="0 0 960 960">... </svg>

(the generated asset is the same asset). For comparison, the desired output should be:

24_ways
<svg class="24ways" xmlns="http://www.w3.org/2000/svg" width="48" height="48" viewBox="0 0 960 960">... </svg>

unicef_uk
<svg class="unicef" xmlns="http://www.w3.org/2000/svg" width="48" height="48" viewBox="0 0 960 960">... </svg>

bradshaw_s_guide
<svg class="bradshaws" xmlns="http://www.w3.org/2000/svg" width="48" height="48" viewBox="0 0 960 960">... </svg>

Should I now file this as a separate bug? In Jekyll, or Jekyll Assets?

Parsing and rendering should not interact in this way... @envygeeks how does asset_source work?

{{ item.title | slugify }}

works as charm, but assets_source didn't handle inside liquid rendering correctly. Seems it's related to issue Parse Liquid output within asset_source tag.

@rebornix before you go blaming me explain yourself. We handle rendering directly through Jekyll: https://github.com/jekyll/jekyll-assets/blob/master/lib/jekyll/assets/liquid/tag/parser.rb#L172 so I expect you to be very careful and explicit about how it's my fault especially if this is happening after your pull is merged.

@parkr @rebornix @paulrobertlloyd we take in the source they give us, we parse as normal and then we run the value through Jekyll's own renderer with the context (and only the context) the user is in so they get those variables. We use the raw args as the filename so that if a user error pops up they get the raw args spot as the place the error happened.

@envygeeks It's my mistake to make arbitrary judgement without any solid proof. I apologise.

When I say some code doesn't handle some logic correctly (my judgement might be correct or wrong), I'm not blaming people who wrote the code. If I make you feel that way, I apologise.

Let me take a deep look into this issue and give it a turnaround. Sorry for bringing you bad feelings, sincerely.

@rebornix I'm already looking into it, and I don't know if it's either one of our faults. I'm using the context being passed to me to render but after doing a deep test pre-your commit I don't ever get an updated context so I always end up with the same n for some reason.

This is my bug, I don't know where but for some reason our contexts are being called dozens of times and it leads to a race. I don't know how that is happening since we only hit the caller once but I'll have to track down where.

I take that back I don't know where the bug is. I don't even get it. I'm gonna need a few hours.

I know the problem. It's because we expect tag to be called for every tag but it's actually called multiple times with multiple contexts and our own caching and destructive parsing is pushing out the new context. I had no idea Liquid works this way so I'll need to rework our parser to be less desctructive with our hashes on the assumption we are a fire and reload fire instead of a fire fire fire.

@envygeeks Great to hear you find the root cause 👍 😄

Putting more info here for this PR: It caches include parsed templates for saving liquid parsing time and that's all. If you feel this pr may lead to some bugs, a quick way to filter it out is to create a layout without any include block then test again.

envygeeks/jekyll-assets@1eb580f