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

Glob scope path only if configured with a pattern #6692

Merged
merged 6 commits into from Jan 19, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 14 additions & 1 deletion docs/_docs/configuration.md
Expand Up @@ -549,7 +549,9 @@ defaults:
In this example, the `layout` is set to `default` inside the
[collection](../collections/) with the name `my_collection`.

It is also possible to use glob patterns when matching defaults. For example, it is possible to set specific layout for each `special-page.html` in any subfolder of `section` folder. {%- include docs_version_badge.html version="3.7.0" -%}
### Glob patterns in Front Matter defaults

It is also possible to use glob patterns (currently limited to patterns that contain `*`) when matching defaults. For example, it is possible to set specific layout for each `special-page.html` in any subfolder of `section` folder. {%- include docs_version_badge.html version="3.7.0" -%}

```yaml
collections:
Expand All @@ -564,6 +566,17 @@ defaults:
layout: "specific-layout"
```

<div class="note warning">
<h5>Globbing and Performance</h5>
<p>
Please note that globbing a path is a known performance-poison and is
Copy link
Member

Choose a reason for hiding this comment

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

The poison sounds really terrifying! Can we rephrase?

Please note that front matter defaults with glob paths have known performance problems.
Windows users may notice especially high build times by adding globbing. We are tracking [in this issue](let's open an issue).

Copy link
Member Author

Choose a reason for hiding this comment

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

We are tracking [in this issue](let's open an issue).

I believe this is an issue with Ruby for Windows itself as Ruby is not optimized for Windows.. There's not much one can do here IMO..

So.. I'll just re-phrase the "poison" bit to something more mellow..

currently not optimized, especially on Windows. Globbing a path will
increase your build times in proportion to the size of the associated
collection directory.
</p>
</div>


### Precedence

Jekyll will apply all of the configuration settings you specify in the `defaults` section of your `_config.yml` file. However, you can choose to override settings from other scope/values pair by specifying a more specific path for the scope.
Expand Down
15 changes: 10 additions & 5 deletions lib/jekyll/frontmatter_defaults.rb
Expand Up @@ -97,21 +97,26 @@ def applies?(scope, path, type)
applies_path?(scope, path) && applies_type?(scope, type)
end

# rubocop:disable Metrics/AbcSize
def applies_path?(scope, path)
return true if !scope.key?("path") || scope["path"].empty?

sanitized_path = Pathname.new(sanitize_path(path))

site_path = Pathname.new(@site.source)
site_path = Pathname.new(@site.source)
rel_scope_path = Pathname.new(scope["path"])
abs_scope_path = File.join(@site.source, rel_scope_path)
Dir.glob(abs_scope_path).each do |scope_path|
scope_path = Pathname.new(scope_path).relative_path_from site_path
return true if path_is_subpath?(sanitized_path, scope_path)

if scope["path"].to_s.include?("*")
Dir.glob(abs_scope_path).each do |scope_path|
scope_path = Pathname.new(scope_path).relative_path_from site_path
Jekyll.logger.debug "Globbed Scope Path:", scope_path
return true if path_is_subpath?(sanitized_path, scope_path)
end
end

path_is_subpath?(sanitized_path, rel_scope_path)
Copy link
Member

Choose a reason for hiding this comment

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

Could we make this an else for the previous block (if scope["path"].to_s.include?("*"))?
Multiple exits can get confusing.

Copy link
Member Author

@ashmaroli ashmaroli Jan 18, 2018

Choose a reason for hiding this comment

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

We can't make this an else branch because if diff-ln#113 evaluates to false even at the end of looping the globbed array, diff-ln#117 acts as a fallback..

That said, it does not make sense to me why that fallback's needed..

end
# rubocop:enable Metrics/AbcSize

def path_is_subpath?(path, parent_path)
path.ascend do |ascended_path|
Expand Down
12 changes: 10 additions & 2 deletions test/test_front_matter_defaults.rb
Expand Up @@ -16,7 +16,7 @@ class TestFrontMatterDefaults < JekyllUnitTest
},
},],
})
@site.process
@output = capture_output { @site.process }
@affected = @site.pages.find { |page| page.relative_path == "contacts/bar.html" }
@not_affected = @site.pages.find { |page| page.relative_path == "about.html" }
end
Expand All @@ -25,6 +25,10 @@ class TestFrontMatterDefaults < JekyllUnitTest
assert_equal @affected.data["key"], "val"
assert_nil @not_affected.data["key"]
end

should "not call Dir.glob block" do
refute_includes @output, "Globbed Scope Path:"
end
end

context "A site with full front matter defaults (glob)" do
Expand All @@ -40,7 +44,7 @@ class TestFrontMatterDefaults < JekyllUnitTest
},
},],
})
@site.process
@output = capture_output { @site.process }
@affected = @site.pages.find { |page| page.relative_path == "contacts/bar.html" }
@not_affected = @site.pages.find { |page| page.relative_path == "about.html" }
end
Expand All @@ -49,6 +53,10 @@ class TestFrontMatterDefaults < JekyllUnitTest
assert_equal @affected.data["key"], "val"
assert_nil @not_affected.data["key"]
end

should "call Dir.glob block" do
assert_includes @output, "Globbed Scope Path:"
end
end

context "A site with front matter type pages and an extension" do
Expand Down