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

Support a new relative_include tag #2870

Merged
merged 10 commits into from
Sep 7, 2014
28 changes: 23 additions & 5 deletions lib/jekyll/tags/include.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ class IncludeTag < Liquid::Tag
VALID_SYNTAX = /([\w-]+)\s*=\s*(?:"([^"\\]*(?:\\.[^"\\]*)*)"|'([^'\\]*(?:\\.[^'\\]*)*)'|([\w\.-]+))/
VARIABLE_SYNTAX = /(?<variable>[^{]*\{\{\s*(?<name>[\w\-\.]+)\s*(\|.*)?\}\}[^\s}]*)(?<params>.*)/

INCLUDES_DIR = '_includes'

def initialize(tag_name, markup, tokens)
super
matched = markup.strip.match(VARIABLE_SYNTAX)
Expand Down Expand Up @@ -96,8 +94,12 @@ def render_variable(context)
end
end

def includes_dir
'_includes'
end

def render(context)
dir = File.join(File.realpath(context.registers[:site].source), INCLUDES_DIR)
dir = dir_to_include(context)
Copy link
Member

Choose a reason for hiding this comment

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

dir_to_include? Are we including the whole dir? Or is it resolved_includes_dir?

Copy link
Member Author

Choose a reason for hiding this comment

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

The second one. 😊


file = render_variable(context) || @file
validate_file_name(file)
Expand All @@ -113,10 +115,14 @@ def render(context)
partial.render!(context)
end
rescue => e
raise IncludeTagError.new e.message, File.join(INCLUDES_DIR, @file)
raise IncludeTagError.new e.message, File.join(includes_dir, @file)
end
end

def dir_to_include(context)
File.join(File.realpath(context.registers[:site].source), includes_dir)
end

def validate_path(path, dir, safe)
if safe && !realpath_prefixed_with?(path, dir)
raise IOError.new "The included file '#{path}' should exist and should not be a symlink"
Expand All @@ -126,7 +132,7 @@ def validate_path(path, dir, safe)
end

def path_relative_to_source(dir, path)
File.join(INCLUDES_DIR, path.sub(Regexp.new("^#{dir}"), ""))
File.join(includes_dir, path.sub(Regexp.new("^#{dir}"), ""))
end

def realpath_prefixed_with?(path, dir)
Expand All @@ -138,7 +144,19 @@ def source(file, context)
File.read(file, file_read_opts(context))
end
end

class IncludeRelativeTag < IncludeTag
def includes_dir
'.'
end

def dir_to_include(context)
page_path = context.registers[:page].nil? ? includes_dir : File.dirname(context.registers[:page]["path"])
File.join(File.realpath(context.registers[:site].source), page_path)
Copy link
Member

Choose a reason for hiding this comment

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

We should ensure this path is sanitized properly. Would you please change this line to

Jekyll.sanitized_path(context.registers[:site].source, page_path)

? The realpath stuff is resolved later on in the including process so we don't need that here.

end
end
Copy link
Member

Choose a reason for hiding this comment

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

Why use case statements instead of just overwrite certain methods based on the tag class? If you have a separate class, you may as well separate the class-specific code from each other. 😉

Copy link
Member Author

Choose a reason for hiding this comment

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

true.dat

Copy link
Member

Choose a reason for hiding this comment

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

If you want me to do that, I am happy to submit a PR to your PR. Or something? #howtogithub

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, I was planing on just killing this and relying more on tag_name to do the weight. There's only two spots that need to change. Does that make sense? #howdoiprogram

end
end

Liquid::Template.register_tag('include', Jekyll::Tags::IncludeTag)
Liquid::Template.register_tag('include_relative', Jekyll::Tags::IncludeRelativeTag)
29 changes: 29 additions & 0 deletions test/source/_posts/2014-09-02-relative-includes.markdown
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
title: Post
layout: post
include1: rel_include.html
include2: include_relative/rel_include
include3: rel_INCLUDE
include4: params
include5: clude
---

Liquid tests
- 1 {% include_relative include_relative/{{ page.include1 }} %}
- 2 {% include_relative {{ page.include2 | append: '.html' }} %}
- 3 {% include_relative include_relative/{{ page.include3 | downcase | append: '.html' }} %}

Whitespace tests
- 4 {% include_relative include_relative/{{page.include1}} %}
- 5 {% include_relative include_relative/{{ page.include1}} %}
- 6 {% include_relative include_relative/{{ page.include3 | downcase | append: '.html'}} %}

Parameters test
- 7 {% include_relative include_relative/{{ page.include4 | append: '.html' }} var1='foo' var2='bar' %}

Partial variable test
- 8 {% include_relative include_relative/rel_in{{ page.include5 }}.html %}

Relative to self test:

- 9 {% include_relative 2014-03-03-yaml-with-dots.md %}
7 changes: 7 additions & 0 deletions test/source/_posts/include_relative/params.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<span id='include-param'>{{include.param}}</span>

<ul id='param-list'>
{% for param in include %}
<li>{{param[0]}} = {{param[1]}}</li>
{% endfor %}
</ul>
1 change: 1 addition & 0 deletions test/source/_posts/include_relative/rel_include.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
relative_included
2 changes: 1 addition & 1 deletion test/test_generated_site.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class TestGeneratedSite < Test::Unit::TestCase
end

should "ensure post count is as expected" do
assert_equal 42, @site.posts.size
assert_equal 43, @site.posts.size
end

should "insert site.posts into the index" do
Expand Down
2 changes: 1 addition & 1 deletion test/test_site.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class TestSite < Test::Unit::TestCase
Jekyll::Configuration::DEFAULTS.merge({'source' => source_dir, 'destination' => dest_dir})
end
@site = Site.new(Jekyll.configuration)
@num_invalid_posts = 2
@num_invalid_posts = 4
end

should "have an empty tag hash by default" do
Expand Down
94 changes: 94 additions & 0 deletions test/test_tags.rb
Original file line number Diff line number Diff line change
Expand Up @@ -510,4 +510,98 @@ def fill_post(code, override = {})
end
end
end

context "relative include tag with variable and liquid filters" do
setup do
stub(Jekyll).configuration do
site_configuration({'pygments' => true})
end

site = Site.new(Jekyll.configuration)
post = Post.new(site, source_dir, '', "2014-09-02-relative-includes.markdown")
layouts = { "default" => Layout.new(site, source_dir('_layouts'), "simple.html")}
post.render(layouts, {"site" => {"posts" => []}})
@content = post.content
end

should "include file as variable with liquid filters" do
assert_match %r{1 relative_include}, @content
assert_match %r{2 relative_include}, @content
assert_match %r{3 relative_include}, @content
end

should "include file as variable and liquid filters with arbitrary whitespace" do
assert_match %r{4 relative_include}, @content
assert_match %r{5 relative_include}, @content
assert_match %r{6 relative_include}, @content
end

should "include file as variable and filters with additional parameters" do
assert_match '<li>var1 = foo</li>', @content
assert_match '<li>var2 = bar</li>', @content
end

should "include file as partial variable" do
assert_match %r{8 relative_include}, @content
end

should "include files relative to self" do
assert_match %r{9 —\ntitle: Test Post Where YAML}, @content
end

context "trying to do bad stuff" do
context "include missing file" do
setup do
@content = <<CONTENT
---
title: missing file
---

{% include_relative missing.html %}
CONTENT
end

should "raise error relative to source directory" do
exception = assert_raise IOError do
create_post(@content, {'permalink' => 'pretty', 'source' => source_dir, 'destination' => dest_dir, 'read_posts' => true})
end
assert_equal 'Included file \'./missing.html\' not found', exception.message
end
end
end

context "with symlink'd include" do

should "not allow symlink includes" do
File.open("/tmp/pages-test", 'w') { |file| file.write("SYMLINK TEST") }
assert_raise IOError do
content = <<CONTENT
---
title: Include symlink
---

{% include_relative tmp/pages-test %}

CONTENT
create_post(content, {'permalink' => 'pretty', 'source' => source_dir, 'destination' => dest_dir, 'read_posts' => true, 'safe' => true })
end
assert_no_match /SYMLINK TEST/, @result
end

should "not expose the existence of symlinked files" do
ex = assert_raise IOError do
content = <<CONTENT
---
title: Include symlink
---

{% include_relative tmp/pages-test-does-not-exist %}

CONTENT
create_post(content, {'permalink' => 'pretty', 'source' => source_dir, 'destination' => dest_dir, 'read_posts' => true, 'safe' => true })
end
assert_match /should exist and should not be a symlink/, ex.message
end
end
end
end