Partial variable in include tag #2693

Merged
merged 2 commits into from Aug 6, 2014

Conversation

Projects
None yet
3 participants
@ivantsepp
Contributor

ivantsepp commented Aug 5, 2014

Implements #2142

The fix is simply adding to the VARIABLE_SYNTAX regex. I changed it so that the "variable" capture group would also capture any characters before {{ and after }}.

The regex bit that I added to the beginning is [^{]* which selects anything except for { and the bit that I ended to the end is [^\s}]* to match anything except for } or whitespace (in case it tries to greedily match the params part).

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Aug 5, 2014

Member

Wow, great work @ivantsepp! You even covered the tests twice.

Do you think we should expand/rework our docs for this feature to include this fix?

/cc @mattr- / @benbalter for your 👀

Member

parkr commented Aug 5, 2014

Wow, great work @ivantsepp! You even covered the tests twice.

Do you think we should expand/rework our docs for this feature to include this fix?

/cc @mattr- / @benbalter for your 👀

@parkr parkr added the :shipit: label Aug 5, 2014

@ivantsepp

This comment has been minimized.

Show comment
Hide comment
@ivantsepp

ivantsepp Aug 6, 2014

Contributor

I was just thinking about this some more, would we want to allow the following to work as well?

{% include in{{ page.include5 }}.{{ page.ext }} %}

if page.include5 is "clude" and page.ext is "html". Basically, should we allow multiple variable tags?

Contributor

ivantsepp commented Aug 6, 2014

I was just thinking about this some more, would we want to allow the following to work as well?

{% include in{{ page.include5 }}.{{ page.ext }} %}

if page.include5 is "clude" and page.ext is "html". Basically, should we allow multiple variable tags?

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Aug 6, 2014

Member

@ivantsepp We could hypothetically accept anything. The safety concern is in access to the file system, which comes later.

Member

parkr commented Aug 6, 2014

@ivantsepp We could hypothetically accept anything. The safety concern is in access to the file system, which comes later.

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

@parkr parkr merged commit 21b6a01 into jekyll:master Aug 6, 2014

1 check passed

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

parkr added a commit that referenced this pull request Aug 6, 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.