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

Scope path glob #6268

Merged
merged 3 commits into from Oct 9, 2017
Merged
Show file tree
Hide file tree
Changes from all 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: 15 additions & 0 deletions docs/_docs/configuration.md
Expand Up @@ -549,6 +549,21 @@ 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.

```yaml
collections:
my_collection:
output: true

defaults:
-
scope:
path: "section/*/special-page.html"
values:
layout: "specific-layout"
```

### 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
21 changes: 18 additions & 3 deletions lib/jekyll/frontmatter_defaults.rb
Expand Up @@ -100,12 +100,27 @@ def applies?(scope, path, type)
def applies_path?(scope, path)
return true if !scope.key?("path") || scope["path"].empty?

scope_path = Pathname.new(scope["path"])
Pathname.new(sanitize_path(path)).ascend do |ascended_path|
if ascended_path.to_s == scope_path.to_s
sanitized_path = Pathname.new(sanitize_path(path))

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|
Copy link
Member

Choose a reason for hiding this comment

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

We have some code in EntryFilter#glob_include? we might be able to use here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looked through that code, it relies on File#fnmatch? that uses the same rules as Dir#glob, though EntryFilter#glob_include? itself is not super-useful in this code, it would take more lines of code to reuse glob_include?

scope_path = Pathname.new(scope_path).relative_path_from site_path
return true if path_is_subpath?(sanitized_path, scope_path)
end

path_is_subpath?(sanitized_path, rel_scope_path)
end

def path_is_subpath?(path, parent_path)
Copy link
Member

Choose a reason for hiding this comment

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

do you want to expose this method as a public method? or would it rather be a private helper method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ashmaroli Isn't there a private scope starting right before applies?

Copy link
Member

Choose a reason for hiding this comment

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

right.. my bad.. ignore..

path.ascend do |ascended_path|
if ascended_path.to_s == parent_path.to_s
return true
end
end

false
end

# Determines whether the scope applies to type.
Expand Down
24 changes: 24 additions & 0 deletions test/test_front_matter_defaults.rb
Expand Up @@ -27,6 +27,30 @@ class TestFrontMatterDefaults < JekyllUnitTest
end
end

context "A site with full front matter defaults (glob)" do
setup do
@site = fixture_site({
"defaults" => [{
"scope" => {
"path" => "contacts/*.html",
Copy link
Member

Choose a reason for hiding this comment

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

What happens if I have *.html here? Should it apply to about.html and contacts/index.html?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we're using Dir.glob, according to docs, *.html will match only files on top directory, to match any HTML file, a **/*.html pattern should we used

Copy link
Member

Choose a reason for hiding this comment

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

That's what would happen with the current code, yeah. I'm wondering if it makes sense to have *.html apply to all files in my entire source that end in .html. If I were using globs, that's what I would expect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems to me that EntryFilter#glob_include behaves in a similar way since File.fnmatch uses same rules of matching. Making *.html to match all files recursively will limit usability of the feature since if I want to set values only for root html files for whatever reason and not for any other - it will be impossible to achieve. Anyhow, this behaviour should be documented somewhere?

Copy link
Member

Choose a reason for hiding this comment

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

It should definitely be documented! If you wanted to apply to just root, would /*.html work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, it would, though it would definitely take more custom logic that using Dir#glob and then there would be question why EntryFilter treats *.html differently

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regarding the documentation - where should I write what?

"type" => "page",
},
"values" => {
"key" => "val",
},
},],
})
@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

should "affect only the specified path and type" do
assert_equal @affected.data["key"], "val"
assert_nil @not_affected.data["key"]
end
end

context "A site with front matter type pages and an extension" do
setup do
@site = fixture_site({
Expand Down