From c8f23b3464d596c08922dc923c64bb57488e6227 Mon Sep 17 00:00:00 2001 From: robert Date: Tue, 20 Nov 2018 14:10:21 +0100 Subject: [PATCH] automatically call Pry::Config::Lazy objects before returning them. The pry-rails prompt uses `pry.config.prompt_name` but assumes it will always be a string and never tries to `.call` the object, which means we can end up with a prompt like this: [2] [vagrant][development] #(main)> This commit solves the problem for pry-rails and other prompts who make the same mistake by calling `.call` for them, automatically, upon read from a config object. The commit also removes the issue where every prompt has to deal with `prompt_name` being a string / Proc-like object then acting accordingly. --- lib/pry/config/behavior.rb | 3 +- lib/pry/config/lazy.rb | 3 ++ lib/pry/prompt.rb | 14 ++---- spec/config_spec.rb | 99 +++++++++++++++++++++++--------------- 4 files changed, 67 insertions(+), 52 deletions(-) diff --git a/lib/pry/config/behavior.rb b/lib/pry/config/behavior.rb index c6336763..a28921f7 100644 --- a/lib/pry/config/behavior.rb +++ b/lib/pry/config/behavior.rb @@ -76,7 +76,8 @@ class Pry # def [](key) key = key.to_s - key?(key) ? @lookup[key] : (@default && @default[key]) + obj = key?(key) ? @lookup[key] : (@default && @default[key]) + Pry::Config::Lazy === obj ? obj.call : obj end # diff --git a/lib/pry/config/lazy.rb b/lib/pry/config/lazy.rb index 6de5f953..a521f918 100644 --- a/lib/pry/config/lazy.rb +++ b/lib/pry/config/lazy.rb @@ -1,6 +1,9 @@ class Pry class Config < Pry::BasicObject # Wraps a block so it can have a name. + # The primary purpose for instances of this class is to be used as a + # configuration value that is computed upon each access, see {Pry.lazy} + # for more information. # # @example # proc1 = proc {} diff --git a/lib/pry/prompt.rb b/lib/pry/prompt.rb index e4e46354..232fd61c 100644 --- a/lib/pry/prompt.rb +++ b/lib/pry/prompt.rb @@ -102,14 +102,6 @@ class Pry nil end - - private - - def prompt_name(name) - return name unless name.is_a?(Pry::Config::Lazy) - - name.call - end end # @return [String] @@ -172,7 +164,7 @@ class Pry format( "[%s] %s(%s)%s%s ", in_count: _pry_.input_ring.count, - name: prompt_name(_pry_.config.prompt_name), + name: _pry_.config.prompt_name, context: Pry.view_clip(context), nesting: (nesting > 0 ? ":#{nesting}" : ''), separator: sep @@ -197,7 +189,7 @@ class Pry format( "[%s] (%s) %s: %s%s ", in_count: _pry_.input_ring.count, - name: prompt_name(_pry_.config.prompt_name), + name: _pry_.config.prompt_name, tree: tree.join(' / '), stack_size: _pry_.binding_stack.size - 1, separator: sep @@ -211,7 +203,7 @@ class Pry ) do |context, _nesting, _pry_, sep| format( "%s %s:%s %s ", - name: prompt_name(_pry_.config.prompt_name), + name: _pry_.config.prompt_name, context: Pry.view_clip(context), pwd: Dir.pwd, separator: sep diff --git a/spec/config_spec.rb b/spec/config_spec.rb index 8f2861f1..bd2b63b0 100644 --- a/spec/config_spec.rb +++ b/spec/config_spec.rb @@ -1,7 +1,7 @@ describe Pry::Config do describe "bug #1552" do specify "a local key has precendence over its default when the stored value is false" do - local = Pry::Config.from_hash({}, Pry::Config.from_hash('color' => true)) + local = described_class.from_hash({}, described_class.from_hash('color' => true)) local.color = false expect(local.color).to eq(false) end @@ -9,44 +9,44 @@ describe Pry::Config do describe "bug #1277" do specify "a local key has precendence over an inherited method of the same name" do - local = Pry::Config.from_hash(output: 'foobar') + local = described_class.from_hash(output: 'foobar') local.extend(Module.new { def output(); 'broken'; end }) expect(local.output).to eq('foobar') end end describe "reserved keys" do - it "raises Pry::Config::ReservedKeyError on assignment of a reserved key" do - local = Pry::Config.new + it "raises ReservedKeyError on assignment of a reserved key" do + local = described_class.new local.instance_variable_get(:@reserved_keys).each do |key| - expect { local[key] = 1 }.to raise_error(Pry::Config::ReservedKeyError) + expect { local[key] = 1 }.to raise_error(described_class::ReservedKeyError) end end end describe "traversal to parent" do it "traverses back to the parent when a local key is not found" do - local = Pry::Config.new Pry::Config.from_hash(foo: 1) + local = described_class.new described_class.from_hash(foo: 1) expect(local.foo).to eq(1) end it "stores a local key and prevents traversal to the parent" do - local = Pry::Config.new Pry::Config.from_hash(foo: 1) + local = described_class.new described_class.from_hash(foo: 1) local.foo = 2 expect(local.foo).to eq(2) end it "traverses through a chain of parents" do - root = Pry::Config.from_hash({foo: 21}) - local1 = Pry::Config.new(root) - local2 = Pry::Config.new(local1) - local3 = Pry::Config.new(local2) + root = described_class.from_hash({foo: 21}) + local1 = described_class.new(root) + local2 = described_class.new(local1) + local3 = described_class.new(local2) expect(local3.foo).to eq(21) end it "stores a local copy of the parents hooks upon accessing them" do - parent = Pry::Config.from_hash(hooks: "parent_hooks") - local = Pry::Config.new parent + parent = described_class.from_hash(hooks: "parent_hooks") + local = described_class.new parent local.hooks.gsub! 'parent', 'local' expect(local.hooks).to eq 'local_hooks' expect(parent.hooks).to eq('parent_hooks') @@ -55,19 +55,19 @@ describe Pry::Config do describe ".from_hash" do it "returns an object without a default" do - local = Pry::Config.from_hash({}) + local = described_class.from_hash({}) expect(local.default).to eq(nil) end it "returns an object with a default" do - default = Pry::Config.new(nil) - local = Pry::Config.from_hash({}, default) + default = described_class.new(nil) + local = described_class.from_hash({}, default) expect(local.default).to eq(local) end - it "recursively builds Pry::Config objects from a Hash" do + it "recursively creates Pry::Config objects from a Hash" do h = {'foo1' => {'foo2' => {'foo3' => 'foobar'}}} - default = Pry::Config.from_hash(h) + default = described_class.from_hash(h) expect(default.foo1).to be_instance_of(described_class) expect(default.foo1.foo2).to be_instance_of(described_class) end @@ -75,7 +75,7 @@ describe Pry::Config do describe "#respond_to_missing?" do before do - @config = Pry::Config.new(nil) + @config = described_class.new(nil) end it "returns a Method object for a dynamic key" do @@ -93,7 +93,7 @@ describe Pry::Config do describe "#respond_to?" do before do - @config = Pry::Config.new(nil) + @config = described_class.new(nil) end it "returns true for a local key" do @@ -108,43 +108,43 @@ describe Pry::Config do describe "#default" do it "returns nil" do - local = Pry::Config.new(nil) + local = described_class.new(nil) expect(local.default).to eq(nil) end it "returns the default" do - default = Pry::Config.new(nil) - local = Pry::Config.new(default) + default = described_class.new(nil) + local = described_class.new(default) expect(local.default).to eq(default) end end describe "#keys" do it "returns an array of local keys" do - root = Pry::Config.from_hash({zoo: "boo"}, nil) - local = Pry::Config.from_hash({foo: "bar"}, root) + root = described_class.from_hash({zoo: "boo"}, nil) + local = described_class.from_hash({foo: "bar"}, root) expect(local.keys).to eq(["foo"]) end end describe "#==" do it "compares equality through the underlying lookup table" do - local1 = Pry::Config.new(nil) - local2 = Pry::Config.new(nil) + local1 = described_class.new(nil) + local2 = described_class.new(nil) local1.foo = "hi" local2.foo = "hi" expect(local1).to eq(local2) end it "compares equality against an object who does not implement #to_hash" do - local1 = Pry::Config.new(nil) + local1 = described_class.new(nil) expect(local1).not_to eq(Object.new) end end describe "#forget" do it "forgets a local key" do - local = Pry::Config.new Pry::Config.from_hash(foo: 1) + local = described_class.new described_class.from_hash(foo: 1) local.foo = 2 expect(local.foo).to eq 2 local.forget(:foo) @@ -154,13 +154,13 @@ describe Pry::Config do describe "#to_hash" do it "provides a copy of local key & value pairs as a Hash" do - local = Pry::Config.new Pry::Config.from_hash(bar: true) + local = described_class.new described_class.from_hash(bar: true) local.foo = "21" expect(local.to_hash).to eq({ "foo" => "21" }) end it "returns a duplicate of the lookup table" do - local = Pry::Config.new(nil) + local = described_class.new(nil) local.to_hash.merge!("foo" => 42) expect(local.foo).not_to eq(42) end @@ -168,7 +168,7 @@ describe Pry::Config do describe "#merge!" do before do - @config = Pry::Config.new(nil) + @config = described_class.new(nil) end it "merges an object who returns a Hash through #to_hash" do @@ -195,7 +195,7 @@ describe Pry::Config do describe "#clear" do before do - @local = Pry::Config.new(nil) + @local = described_class.new(nil) end it "returns true" do @@ -211,7 +211,7 @@ describe Pry::Config do describe "#[]=" do it "stores keys as strings" do - local = Pry::Config.from_hash({}) + local = described_class.from_hash({}) local[:zoo] = "hello" expect(local.to_hash).to eq({ "zoo" => "hello" }) end @@ -219,21 +219,40 @@ describe Pry::Config do describe "#[]" do it "traverses back to a default" do - default = Pry::Config.from_hash({k: 1}) - local = Pry::Config.new(default) + default = described_class.from_hash({k: 1}) + local = described_class.new(default) expect(local['k']).to eq(1) end it "traverses back to a default (2 deep)" do - default1 = Pry::Config.from_hash({k: 1}) - default2 = Pry::Config.from_hash({}, default1) - local = Pry::Config.new(default2) + default1 = described_class.from_hash({k: 1}) + default2 = described_class.from_hash({}, default1) + local = described_class.new(default2) expect(local['k']).to eq(1) end it "traverses back to a default that doesn't exist, and returns nil" do - local = Pry::Config.from_hash({}, nil) + local = described_class.from_hash({}, nil) expect(local['output']).to eq(nil) end + + context "when returning a Pry::Config::Lazy object" do + it "invokes #call on it" do + c = described_class.from_hash foo: Pry.lazy { 10 } + expect(c['foo']).to eq(10) + end + + it "invokes #call upon each access" do + c = described_class.from_hash foo: Pry.lazy { 'foo' } + expect(c['foo']).to_not equal(c['foo']) + end + end + + context "when returning an instance of BasicObject" do + it "returns without raising an error" do + c = described_class.from_hash(foo: BasicObject.new) + expect(BasicObject === c['foo']).to be(true) + end + end end end