From e6ac84e99c0e32dc2641f85945594feeaf68051b Mon Sep 17 00:00:00 2001 From: Kyrylo Silin Date: Sat, 4 May 2019 18:09:57 +0300 Subject: control_d_handler: don't mutate eval_string within the handler This is a preparational step for #1824 (Enabling `# frozen_string_literal: true` in `~/.pryc` crashes most operations) Alternative to https://github.com/pry/pry/pull/2030 (config: delete the `control_d_handler` option) We had to jump a few hoops to change how the handler works. The problem is that mutation is the default expected behaviour. Therefore, we had to change its API. There's no need to pass `eval_string` because `pry_instance` already has it as an attribute. `config.control_d_handler` is a proxy proc, to preserve backwards compatibility with users of old signature (one known user is Pry Byebug). The handler will emit a warning if the old signature is used. --- lib/pry/config.rb | 27 ++++++++++++++++ lib/pry/control_d_handler.rb | 7 ++-- lib/pry/pry_instance.rb | 2 +- spec/commands/cd_spec.rb | 2 +- spec/config_spec.rb | 72 +++++++++++++++++++++++++++++++++++++++++- spec/control_d_handler_spec.rb | 33 ++++++++++++------- 6 files changed, 125 insertions(+), 18 deletions(-) diff --git a/lib/pry/config.rb b/lib/pry/config.rb index 55718200..feeb4b40 100644 --- a/lib/pry/config.rb +++ b/lib/pry/config.rb @@ -261,6 +261,33 @@ class Pry @custom_attrs = @custom_attrs.dup end + def control_d_handler=(value) + proxy_proc = + if value.arity == 2 + Pry::Warning.warn( + "control_d_handler's arity of 2 parameters was deprecated " \ + '(eval_string, pry_instance). Now it gets passed just 1 ' \ + 'parameter (pry_instance)' + ) + proc do |*args| + if args.size == 2 + value.call(args.first, args[1]) + else + value.call(args.first.eval_string, args.first) + end + end + else + proc do |*args| + if args.size == 2 + value.call(args[1]) + else + value.call(args.first) + end + end + end + @control_d_handler = proxy_proc + end + private def lazy_readline diff --git a/lib/pry/control_d_handler.rb b/lib/pry/control_d_handler.rb index 1021c767..f9b93003 100644 --- a/lib/pry/control_d_handler.rb +++ b/lib/pry/control_d_handler.rb @@ -7,9 +7,10 @@ class Pry # 1. In an expression behave like `!` command. # 2. At top-level session behave like `exit` command. # 3. In a nested session behave like `cd ..`. - def self.default(eval_string, pry_instance) - if !eval_string.empty? - eval_string.replace('') # Clear input buffer. + def self.default(pry_instance) + if !pry_instance.eval_string.empty? + # Clear input buffer. + pry_instance.eval_string = '' elsif pry_instance.binding_stack.one? pry_instance.binding_stack.clear throw(:breakout) diff --git a/lib/pry/pry_instance.rb b/lib/pry/pry_instance.rb index 2aebedc2..41eddb78 100644 --- a/lib/pry/pry_instance.rb +++ b/lib/pry/pry_instance.rb @@ -595,7 +595,7 @@ class Pry def handle_line(line, options) if line.nil? - config.control_d_handler.call(@eval_string, self) + config.control_d_handler.call(self) return end diff --git a/spec/commands/cd_spec.rb b/spec/commands/cd_spec.rb index ae715a3a..5ea64861 100644 --- a/spec/commands/cd_spec.rb +++ b/spec/commands/cd_spec.rb @@ -129,7 +129,7 @@ describe 'cd' do describe 'when using ^D (Control-D) key press' do it 'should keep correct old binding' do @t.eval 'cd :john_dogg', 'cd :mon_dogg', 'cd :kyr_dogg', - 'Pry.config.control_d_handler.call("", pry_instance)' + 'Pry.config.control_d_handler.call(pry_instance)' expect(@t.mapped_binding_stack).to eq [@o, :john_dogg, :mon_dogg] @t.eval 'cd -' diff --git a/spec/config_spec.rb b/spec/config_spec.rb index a3d38043..13aa52aa 100644 --- a/spec/config_spec.rb +++ b/spec/config_spec.rb @@ -28,7 +28,7 @@ RSpec.describe Pry::Config do specify { expect(subject.should_load_requires).to be(true).or be(false) } specify { expect(subject.should_load_plugins).to be(true).or be(false) } specify { expect(subject.windows_console_warning).to be(true).or be(false) } - specify { expect(subject.control_d_handler).to be_a(Method) } + specify { expect(subject.control_d_handler).to respond_to(:call) } specify { expect(subject.memory_size).to be_a(Numeric) } specify { expect(subject.extra_sticky_locals).to be_a(Hash) } specify { expect(subject.command_completions).to be_a(Proc) } @@ -171,4 +171,74 @@ RSpec.describe Pry::Config do expect(subject[:foo]).to eq(1) end end + + describe "#control_d_handler=" do + context "when the handler expects multiple arguments" do + it "prints a warning" do + expect(Pry::Warning).to receive(:warn).with( + "control_d_handler's arity of 2 parameters was deprecated " \ + '(eval_string, pry_instance). Now it gets passed just 1 ' \ + 'parameter (pry_instance)' + ) + subject.control_d_handler = proc { |_arg1, _arg2| } + end + end + + context "when the handler expects just one argument" do + it "doesn't print a warning" do + expect(Pry::Warning).not_to receive(:warn) + subject.control_d_handler = proc { |_arg1| } + end + end + end + + describe "#control_d_handler" do + let(:pry_instance) { Pry.new } + + context "when it returns a callable accepting one argument" do + context "and when it is called with one argument" do + it "calls the handler with a pry instance" do + subject.control_d_handler = proc do |arg| + expect(arg).to eql(pry_instance) + end + subject.control_d_handler.call(pry_instance) + end + end + + context "and when it is called with multiple arguments" do + before { allow(Pry::Warning).to receive(:warn) } + + it "calls the handler with a pry instance" do + subject.control_d_handler = proc do |arg| + expect(arg).to eql(pry_instance) + end + subject.control_d_handler.call('', pry_instance) + end + end + end + + context "when it returns a callabale with two arguments" do + before { allow(Pry::Warning).to receive(:warn) } + + context "and when it's called with one argument" do + it "calls the handler with a eval_string and a pry instance" do + subject.control_d_handler = proc do |arg1, arg2| + expect(arg1).to eq('') + expect(arg2).to eql(pry_instance) + end + subject.control_d_handler.call(pry_instance) + end + end + + context "and when it's called with multiple arguments" do + it "calls the handler with a eval_string and a pry instance" do + subject.control_d_handler = proc do |arg1, arg2| + expect(arg1).to eq('') + expect(arg2).to eql(pry_instance) + end + subject.control_d_handler.call('', pry_instance) + end + end + end + end end diff --git a/spec/control_d_handler_spec.rb b/spec/control_d_handler_spec.rb index 5e21ccbe..030cefd6 100644 --- a/spec/control_d_handler_spec.rb +++ b/spec/control_d_handler_spec.rb @@ -1,48 +1,57 @@ RSpec.describe Pry::ControlDHandler do context "when given eval string is non-empty" do - let(:eval_string) { 'hello' } - let(:pry_instance) { Pry.new } + let(:pry_instance) do + Pry.new.tap do |p| + p.eval_string = 'hello' + end + end it "clears input buffer" do - described_class.default(eval_string, pry_instance) - expect(eval_string).to be_empty + described_class.default(pry_instance) + expect(pry_instance.eval_string).to be_empty end end context "when given eval string is empty & pry instance has one binding" do - let(:eval_string) { '' } - let(:pry_instance) { Pry.new.tap { |p| p.binding_stack = [binding] } } + let(:pry_instance) do + Pry.new.tap do |p| + p.eval_string = '' + p.binding_stack = [binding] + end + end it "throws :breakout" do - expect { described_class.default(eval_string, pry_instance) } + expect { described_class.default(pry_instance) } .to throw_symbol(:breakout) end it "clears binding stack" do - expect { described_class.default(eval_string, pry_instance) } + expect { described_class.default(pry_instance) } .to throw_symbol expect(pry_instance.binding_stack).to be_empty end end context "when given eval string is empty & pry instance has 2+ bindings" do - let(:eval_string) { '' } let(:binding1) { binding } let(:binding2) { binding } let(:binding_stack) { [binding1, binding2] } let(:pry_instance) do - Pry.new.tap { |p| p.binding_stack = binding_stack } + Pry.new.tap do |p| + p.eval_string = '' + p.binding_stack = binding_stack + end end it "saves a dup of the current binding stack in the 'cd' command" do - described_class.default(eval_string, pry_instance) + described_class.default(pry_instance) cd_state = pry_instance.commands['cd'].state expect(cd_state.old_stack).to eq([binding1, binding2]) end it "pops the binding off the stack" do - described_class.default(eval_string, pry_instance) + described_class.default(pry_instance) expect(pry_instance.binding_stack).to eq([binding1]) end end -- cgit v1.2.1