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

Convert StaticFile liquid representation to a Drop & add front matter defaults support to StaticFiles #5871

Merged
merged 1 commit into from Feb 11, 2017

Conversation

Projects
None yet
3 participants
@benbalter
Contributor

benbalter commented Feb 8, 2017

Per jekyll/jekyll-sitemap#157 (comment), this PR implements a new StaticFileDrop such that static files can inherit front matter defaults.

I believe this is implemented as a non-breaking change. The resulting hash should be identical, with a new collection field added (to more closely mimic the DocumentDrop).

I also tried to obtain as much parity with how Documents implement front matter defaults, by spiking out a new data method, rather than simply delegating to the existing defaults method for fallback data (which we could do if we don't want static files to have user-modifiable metadata beyond defaults).

@benbalter benbalter requested a review from parkr Feb 8, 2017

@parkr

Wonderful! Just 2 comments. We should ensure that the collection item is either a string (its label) or the value of Collection#to_liquid. Thank you so much, Ben!

class StaticFileDrop < Drop
extend Forwardable
def_delegators :@obj, :name, :extname, :modified_time, :basename
def_delegator :@obj, :relative_path, :path

This comment has been minimized.

@parkr

parkr Feb 8, 2017

Member

This may not be 100% the same as below. We have to ensure this has the forward slash at the start!

@parkr

parkr Feb 8, 2017

Member

This may not be 100% the same as below. We have to ensure this has the forward slash at the start!

This comment has been minimized.

@@ -148,8 +148,9 @@ def setup_static_file_with_defaults(base, dir, name, defaults)
"extname" => ".txt",
"modified_time" => @static_file.modified_time,
"path" => "/static_file.txt",
"collection" => nil

This comment has been minimized.

@parkr

parkr Feb 8, 2017

Member

Can we add a test for a static file in a collection, please? That would help clarify this item's behavior.

@parkr

parkr Feb 8, 2017

Member

Can we add a test for a static file in a collection, please? That would help clarify this item's behavior.

@benbalter

This comment has been minimized.

Show comment
Hide comment
@benbalter

benbalter Feb 9, 2017

Contributor

We should ensure that the collection item is either a string (its label) or the value of Collection#to_liquid

Collection is being called via type which is either nil or the sym label

Contributor

benbalter commented Feb 9, 2017

We should ensure that the collection item is either a string (its label) or the value of Collection#to_liquid

Collection is being called via type which is either nil or the sym label

@parkr

parkr approved these changes Feb 11, 2017

@parkr parkr changed the title from add StaticFileDrop to Convert StaticFile liquid representation to a Drop & add front matter defaults support Feb 11, 2017

@parkr parkr changed the title from Convert StaticFile liquid representation to a Drop & add front matter defaults support to Convert StaticFile liquid representation to a Drop & add front matter defaults support to StaticFiles Feb 11, 2017

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Feb 11, 2017

Member

@jekyllbot: merge +minor

Member

parkr commented Feb 11, 2017

@jekyllbot: merge +minor

@jekyllbot jekyllbot merged commit 7ea53e0 into master Feb 11, 2017

0 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details

@jekyllbot jekyllbot deleted the static-file-drop branch Feb 11, 2017

jekyllbot added a commit that referenced this pull request Feb 11, 2017

parkr added a commit that referenced this pull request Mar 8, 2017

parkr added a commit that referenced this pull request Mar 9, 2017

Merge pull request #5940 from jekyll/3.4-stable-backport-5871
Backport #5871 for v3.4.x: Convert StaticFile liquid representation to a Drop & add front matter defaults support to StaticFiles
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment