Skip to content

Commit

Permalink
Merge pull request #395 from RobinDaugherty/fix/393-pry-uninitialized…
Browse files Browse the repository at this point in the history
…-constant

Fix Pry `uninitialized constant` exception
  • Loading branch information
RobinDaugherty committed Aug 3, 2017
2 parents 43694c4 + b272b11 commit a351d4f
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 21 deletions.
18 changes: 3 additions & 15 deletions lib/better_errors/error_page.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ def do_eval(opts)
return { error: "REPL unavailable in this stack frame" }
end

@repls[index] ||= get_repl(index, binding)
@repls[index] ||= REPL.provider.new(binding, exception)

send_input(index, code)
eval_and_respond(index, code)
end

def backtrace_frames
Expand Down Expand Up @@ -111,19 +111,7 @@ def inspect_value(obj)
"<span class='unsupported'>(exception was raised in inspect)</span>"
end

def get_repl(index, binding)
REPL.provider.new(binding).tap do |repl|
if repl.is_a?(REPL::Pry)
pry = repl.instance_variable_get(:@pry)
pry.instance_variable_set(
:@last_exception,
::Pry::LastException.new(@exception.exception)
)
end
end
end

def send_input(index, code)
def eval_and_respond(index, code)
result, prompt, prefilled_input = @repls[index].send_input(code)

{
Expand Down
4 changes: 3 additions & 1 deletion lib/better_errors/repl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ def self.detect
end

def self.test_provider(provider)
require provider[:impl]
# We must load this file instead of `require`ing it, since during our tests we want the file
# to be reloaded. In practice, this will only be called once, so `require` is not necessary.
load "#{provider[:impl]}.rb"
true
rescue LoadError
false
Expand Down
2 changes: 1 addition & 1 deletion lib/better_errors/repl/basic.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module BetterErrors
module REPL
class Basic
def initialize(binding)
def initialize(binding, _exception)
@binding = binding
end

Expand Down
8 changes: 7 additions & 1 deletion lib/better_errors/repl/pry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,23 @@ def print(*args)
end
end

def initialize(binding)
def initialize(binding, exception)
@fiber = Fiber.new do
@pry.repl binding
end
@input = BetterErrors::REPL::Pry::Input.new
@output = BetterErrors::REPL::Pry::Output.new
@pry = ::Pry.new input: @input, output: @output
@pry.hooks.clear_all if defined?(@pry.hooks.clear_all)
store_last_exception exception
@fiber.resume
end

def store_last_exception(exception)
return unless defined? ::Pry::LastException
@pry.instance_variable_set(:@last_exception, ::Pry::LastException.new(exception.exception))
end

def send_input(str)
local ::Pry.config, color: false, pager: false do
@fiber.resume "#{str}\n"
Expand Down
57 changes: 57 additions & 0 deletions spec/better_errors/error_page_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,5 +88,62 @@ def initialize(message = nil)
expect(error_page.exception_message).not_to match(/\A\n\n/)
end
end

describe '#do_eval' do
let(:exception) { empty_binding.eval("raise") rescue $! }
subject(:do_eval) { error_page.do_eval("index" => 0, "source" => command) }
let(:command) { 'EvalTester.stuff_was_done(:yep)' }
before do
stub_const('EvalTester', eval_tester)
end
let(:eval_tester) { double('EvalTester', stuff_was_done: 'response') }

context 'with Pry disabled' do
it "evaluates the code" do
do_eval
expect(eval_tester).to have_received(:stuff_was_done).with(:yep)
end

it 'returns a hash of the code and its result' do
expect(do_eval).to include(
highlighted_input: /stuff_was_done/,
prefilled_input: '',
prompt: '>>',
result: "=> \"response\"\n",
)
end
end

context 'with Pry enabled' do
before do
BetterErrors.use_pry!
# Cause the provider to be unselected, so that it will be re-detected.
BetterErrors::REPL.provider = nil
end
after do
BetterErrors::REPL::PROVIDERS.shift
BetterErrors::REPL.provider = nil

# Ensure the Pry REPL file has not been included. If this is not done,
# the constant leaks into other examples.
BetterErrors::REPL.send(:remove_const, :Pry)
end

it "evaluates the code" do
BetterErrors::REPL.provider
do_eval
expect(eval_tester).to have_received(:stuff_was_done).with(:yep)
end

it 'returns a hash of the code and its result' do
expect(do_eval).to include(
highlighted_input: /stuff_was_done/,
prefilled_input: '',
prompt: '>>',
result: "=> \"response\"\n",
)
end
end
end
end
end
4 changes: 3 additions & 1 deletion spec/better_errors/repl/basic_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ module REPL
binding
}

let(:repl) { Basic.new fresh_binding }
let!(:exception) { raise ZeroDivisionError, "you divided by zero you silly goose!" rescue $! }

let(:repl) { Basic.new(fresh_binding, exception) }

it_behaves_like "a REPL provider"
end
Expand Down
16 changes: 14 additions & 2 deletions spec/better_errors/repl/pry_spec.rb
Original file line number Diff line number Diff line change
@@ -1,17 +1,29 @@
require "spec_helper"
require "pry"
require "better_errors/repl/pry"
require "better_errors/repl/shared_examples"

module BetterErrors
module REPL
describe Pry do
before(:all) do
load "better_errors/repl/pry.rb"
end
after(:all) do
# Ensure the Pry REPL file has not been included. If this is not done,
# the constant leaks into other examples.
# In practice, this constant is only defined if `use_pry!` is called and then the
# REPL is used, causing BetterErrors::REPL to require the file.
BetterErrors::REPL.send(:remove_const, :Pry)
end

let(:fresh_binding) {
local_a = 123
binding
}

let(:repl) { Pry.new fresh_binding }
let!(:exception) { raise ZeroDivisionError, "you divided by zero you silly goose!" rescue $! }

let(:repl) { Pry.new(fresh_binding, exception) }

it "does line continuation" do
output, prompt, filled = repl.send_input ""
Expand Down

0 comments on commit a351d4f

Please sign in to comment.