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

Alias Drop#invoke_drop to Drop#[] #6338

Merged
merged 7 commits into from Sep 6, 2017
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
7 changes: 7 additions & 0 deletions lib/jekyll/drops/drop.rb
Expand Up @@ -55,6 +55,7 @@ def [](key)
fallback_data[key]
end
end
alias_method :invoke_drop, :[]

# Set a field in the Drop. If mutable, sets in the mutations and
# returns. If not mutable, checks first if it's trying to override a
Expand Down Expand Up @@ -211,6 +212,12 @@ def fetch(key, default = nil, &block)
return yield(key) unless block.nil?
return default unless default.nil?
end

private

def fallback_data
@fallback_data ||= {}
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The base drop class (in #[], #keys and #key?) assumes fallback data to exist and return a hash. Stubbed out the default value so that child classes don't each have to implement it if they don't have fallback data (including our test fixture).

Copy link
Member

Choose a reason for hiding this comment

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

I intentionally left this method out so all drops had to implement this – it was part of the design of Jekyll Drops, that some fallback data had to be provided by the developer of a drop. I'm not sure I like that going away. :/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was part of the design of Jekyll Drops, that some fallback data had to be provided by the developer of a drop

Admittedly out of scope, so glad to remove from this PR. Curious if that rationale still holds true when Jekyll internally often implements it as an empty hash?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed via 847ed7f

end
end
end
5 changes: 0 additions & 5 deletions lib/jekyll/drops/unified_payload_drop.rb
Expand Up @@ -16,11 +16,6 @@ def jekyll
def site
@site_drop ||= SiteDrop.new(@obj)
end

private
def fallback_data
@fallback_data ||= {}
end
end
end
end
5 changes: 0 additions & 5 deletions lib/jekyll/drops/url_drop.rb
Expand Up @@ -79,11 +79,6 @@ def short_year
def y_day
@obj.date.strftime("%j")
end

private
def fallback_data
{}
end
end
end
end
65 changes: 48 additions & 17 deletions test/test_drop.rb
Expand Up @@ -2,6 +2,14 @@

require "helper"

class DropFixture < Jekyll::Drops::Drop
mutable true

def foo
"bar"
end
end

class TestDrop < JekyllUnitTest
context "a document drop" do
setup do
Expand All @@ -12,37 +20,60 @@ class TestDrop < JekyllUnitTest
@document = @site.collections["methods"].docs.detect do |d|
d.relative_path == "_methods/configuration.md"
end
@drop = @document.to_liquid
@document_drop = @document.to_liquid
@drop = DropFixture.new({})
end

should "reject 'nil' key" do
refute @drop.key?(nil)
end

should "raise KeyError if key is not found and no default provided" do
assert_raises KeyError do
@drop.fetch("not_existing_key")
end
should "return values for #[]" do
assert_equal "bar", @drop["foo"]
end

should "fetch value without default" do
assert_equal "Jekyll.configuration", @drop.fetch("title")
should "return values for #invoke_drop" do
assert_equal "bar", @drop.invoke_drop("foo")
end

should "fetch default if key is not found" do
assert_equal "default", @drop.fetch("not_existing_key", "default")
end
context "mutations" do
should "return mutations for #[]" do
@drop["foo"] = "baz"
assert_equal "baz", @drop["foo"]
end

should "fetch default boolean value correctly" do
assert_equal false, @drop.fetch("bar", false)
should "return mutations for #invoke_drop" do
@drop["foo"] = "baz"
assert_equal "baz", @drop.invoke_drop("foo")
end
end

should "fetch default value from block if key is not found" do
assert_equal "default bar", @drop.fetch("bar") { |el| "default #{el}" }
end
context "fetch" do
should "raise KeyError if key is not found and no default provided" do
assert_raises KeyError do
@document_drop.fetch("not_existing_key")
end
end

should "fetch default value from block first if both argument and block given" do
assert_equal "baz", @drop.fetch("bar", "default") { "baz" }
should "fetch value without default" do
assert_equal "Jekyll.configuration", @document_drop.fetch("title")
end

should "fetch default if key is not found" do
assert_equal "default", @document_drop.fetch("not_existing_key", "default")
end

should "fetch default boolean value correctly" do
assert_equal false, @document_drop.fetch("bar", false)
end

should "fetch default value from block if key is not found" do
assert_equal "default bar", @document_drop.fetch("bar") { |el| "default #{el}" }
end

should "fetch default value from block first if both argument and block given" do
assert_equal "baz", @document_drop.fetch("bar", "default") { "baz" }
end
end
end
end