diff options
author | Kyrylo Silin <silin@kyrylo.org> | 2019-03-24 17:15:27 +0200 |
---|---|---|
committer | Kyrylo Silin <silin@kyrylo.org> | 2019-03-30 20:53:06 +0200 |
commit | b92065d465c81a1df33c05025d878a58cd4859c2 (patch) | |
tree | d8809fbdff6cc1d55bdeb9914cba94c1481fd25e | |
parent | f3673d55f148c1004619dcf3fca4906d576d97b5 (diff) | |
download | pry-b92065d465c81a1df33c05025d878a58cd4859c2.tar.gz |
helpers/command_helpers: add tests, refactor variable names
First of all, we move the spec file from `spec` to `spec/helpers`. This is where
it is supposed to be.
Next, we add tests for all the methods that the module defines. During this
process I had to change `module_function` to `extend self`. Rubocop doesn't like
it for some unknown to me reason, so I had to disable the rule. There's no harm
in doing so.
Finally, I refactored some methods (low-hanging fruits only) and discovered that
the `command_error` method is not necessary at all.
All in all, this module is a lot better now but I feel like it shouldn't exist
at all, since almost all methods are very specific to certain Pry commands. It's
hardly a general purpose module for Pry plugins.
-rw-r--r-- | .rubocop.yml | 3 | ||||
-rw-r--r-- | lib/pry/helpers/base_helpers.rb | 2 | ||||
-rw-r--r-- | lib/pry/helpers/command_helpers.rb | 102 | ||||
-rw-r--r-- | lib/pry/helpers/text.rb | 2 | ||||
-rw-r--r-- | spec/command_helpers_spec.rb | 27 | ||||
-rw-r--r-- | spec/helpers/command_helpers_spec.rb | 254 |
6 files changed, 297 insertions, 93 deletions
diff --git a/.rubocop.yml b/.rubocop.yml index ee3c8c06..8c2d4e5b 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -68,3 +68,6 @@ Style/CommentedKeyword: Gemspec/RequiredRubyVersion: Enabled: false + +Style/ModuleFunction: + Enabled: false diff --git a/lib/pry/helpers/base_helpers.rb b/lib/pry/helpers/base_helpers.rb index 6bf74fab..5d75a50a 100644 --- a/lib/pry/helpers/base_helpers.rb +++ b/lib/pry/helpers/base_helpers.rb @@ -1,7 +1,7 @@ class Pry module Helpers module BaseHelpers - extend self # rubocop:disable Style/ModuleFunction + extend self def silence_warnings old_verbose = $VERBOSE diff --git a/lib/pry/helpers/command_helpers.rb b/lib/pry/helpers/command_helpers.rb index efba4cc4..15798c64 100644 --- a/lib/pry/helpers/command_helpers.rb +++ b/lib/pry/helpers/command_helpers.rb @@ -2,94 +2,73 @@ require 'tempfile' class Pry module Helpers - # rubocop:disable Metrics/ModuleLength module CommandHelpers include OptionsHelpers - module_function + extend self # Open a temp file and yield it to the block, closing it after # @return [String] The path of the temp file def temp_file(ext = '.rb') file = Tempfile.new(['pry', ext]) - yield file + yield(file) ensure - file.close(true) if file + file.close(true) end - def internal_binding?(target) - m = target.eval("::Kernel.__method__").to_s + def internal_binding?(context) + method_name = context.eval("::Kernel.__method__").to_s # class_eval is here because of http://jira.codehaus.org/browse/JRUBY-6753 - %w[__binding__ __pry__ class_eval].include?(m) + %w[__binding__ __pry__ class_eval].include?(method_name) + # TODO: codehaus is dead, there was no test for this and the + # description for the commit doesn't exist. Probably a candidate for + # removal so we have a chance to introduce a regression and document it + # properly. end - def get_method_or_raise(name, target, opts = {}, omit_help = false) - meth = Pry::Method.from_str(name, target, opts) - - if name && !meth - command_error( - "The method '#{name}' could not be found.", omit_help, MethodNotFound - ) + def get_method_or_raise(method_name, context, opts = {}) + method = Pry::Method.from_str(method_name, context, opts) + if !method && method_name + raise Pry::MethodNotFound, "method '#{method_name}' could not be found." end (opts[:super] || 0).times do - if meth.super - meth = meth.super + if method.super + method = method.super else - command_error( - "'#{meth.name_with_owner}' has no super method.", - omit_help, - MethodNotFound - ) + raise Pry::MethodNotFound, + "'#{method.name_with_owner}' has no super method" end end - if !meth || (!name && internal_binding?(target)) - command_error( - "No method name given, and context is not a method.", - omit_help, - MethodNotFound - ) + if !method || (!method_name && internal_binding?(context)) + raise Pry::MethodNotFound, + 'no method name given, and context is not a method' end - set_file_and_dir_locals(meth.source_file) - meth - end - - def command_error(message, omit_help, klass = CommandError) - message += " Type `#{command_name} --help` for help." unless omit_help - raise klass, message + set_file_and_dir_locals(method.source_file) + method end - # Remove any common leading whitespace from every line in `text`. - # - # This can be used to make a HEREDOC line up with the left margin, without + # Remove any common leading whitespace from every line in `text`. This + # can be used to make a HEREDOC line up with the left margin, without # sacrificing the indentation level of the source code. # - # e.g. - # opt.banner unindent <<-USAGE + # @example + # opt.banner(unindent(<<-USAGE)) # Lorem ipsum dolor sit amet, consectetur adipisicing elit, # sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. # "Ut enim ad minim veniam." # USAGE # - # Heavily based on textwrap.dedent from Python, which is: - # Copyright (C) 1999-2001 Gregory P. Ward. - # Copyright (C) 2002, 2003 Python Software Foundation. - # Written by Greg Ward <gward@python.net> - # - # Licensed under <http://docs.python.org/license.html> - # From <http://hg.python.org/cpython/file/6b9f0a6efaeb/Lib/textwrap.py> - # - # @param [String] text The text from which to remove indentation - # @return [String] The text with indentation stripped. - def unindent(text, left_padding = 0) - # Empty blank lines - text = text.sub(/^[ \t]+$/, '') - - # Find the longest common whitespace to all indented lines - # Ignore lines containing just -- or ++ as these seem to be used by - # comment authors as delimeters. + # @param [String] dirty_text The text from which to remove indentation + # @return [String] the text with indentation stripped + def unindent(dirty_text, left_padding = 0) + text = dirty_text.sub(/\A[ \t]+\z/, '') # Empty blank lines. + + # Find the longest common whitespace to all indented lines. Ignore lines + # containing just -- or ++ as these seem to be used by comment authors + # as delimeters. scanned_text = text.scan(/^[ \t]*(?!--\n|\+\+\n)(?=[^ \t\n])/) margin = scanned_text.inject do |current_margin, next_indent| if next_indent.start_with?(current_margin) @@ -97,7 +76,7 @@ class Pry elsif current_margin.start_with?(next_indent) next_indent else - "" + '' end end @@ -106,7 +85,7 @@ class Pry # Restrict a string to the given range of lines (1-indexed) # @param [String] content The string. - # @param [Range, Fixnum] lines The line(s) to restrict it to. + # @param [Range, Integer] lines The line(s) to restrict it to. # @return [String] The resulting string. def restrict_to_lines(content, lines) line_range = one_index_range_or_number(lines) @@ -114,11 +93,7 @@ class Pry end def one_index_number(line_number) - if line_number > 0 - line_number - 1 - else - line_number - end + line_number > 0 ? line_number - 1 : line_number end # convert a 1-index range to a 0-indexed one @@ -165,6 +140,5 @@ class Pry pry.inject_local("_dir_", pry.last_dir, ctx) end end - # rubocop:enable Metrics/ModuleLength end end diff --git a/lib/pry/helpers/text.rb b/lib/pry/helpers/text.rb index 01aa70e4..cefeb234 100644 --- a/lib/pry/helpers/text.rb +++ b/lib/pry/helpers/text.rb @@ -3,7 +3,7 @@ class Pry # The methods defined on {Text} are available to custom commands via # {Pry::Command#text}. module Text - extend self # rubocop:disable Style/ModuleFunction + extend self COLORS = { "black" => 0, diff --git a/spec/command_helpers_spec.rb b/spec/command_helpers_spec.rb deleted file mode 100644 index 902dacb8..00000000 --- a/spec/command_helpers_spec.rb +++ /dev/null @@ -1,27 +0,0 @@ -describe Pry::Helpers::CommandHelpers do - before do - @helper = Pry::Helpers::CommandHelpers - end - - describe "unindent" do - it "should remove the same prefix from all lines" do - expect(@helper.unindent(" one\n two\n")).to eq("one\ntwo\n") - end - - it "should not be phased by empty lines" do - expect(@helper.unindent(" one\n\n two\n")).to eq("one\n\ntwo\n") - end - - it "should only remove a common prefix" do - expect(@helper.unindent(" one\n two\n")).to eq(" one\ntwo\n") - end - - it "should also remove tabs if present" do - expect(@helper.unindent("\tone\n\ttwo\n")).to eq("one\ntwo\n") - end - - it "should ignore lines starting with --" do - expect(@helper.unindent(" one\n--\n two\n")).to eq("one\n--\ntwo\n") - end - end -end diff --git a/spec/helpers/command_helpers_spec.rb b/spec/helpers/command_helpers_spec.rb new file mode 100644 index 00000000..0e696cc5 --- /dev/null +++ b/spec/helpers/command_helpers_spec.rb @@ -0,0 +1,254 @@ +RSpec.describe Pry::Helpers::CommandHelpers do + describe "#temp_file" do + it "yields a tempfile" do + expect { |block| subject.temp_file(&block) } + .to yield_with_args(an_instance_of(Tempfile)) + end + + it "creates a tempfile with rb extension" do + subject.temp_file do |file| + expect(file.path).to end_with('.rb') + end + end + + it "allows overwriting file extension" do + subject.temp_file('.clj') do |file| + expect(file.path).to end_with('.clj') + end + end + + it "closes the tempfile" do + tempfile = nil + subject.temp_file('.clj') do |file| + tempfile = file + end + expect(tempfile).to be_closed + end + + it "unlinks the tempfile" do + tempfile = nil + subject.temp_file('.clj') do |file| + tempfile = file + end + expect(tempfile.path).to be_nil + end + end + + describe "#internal_binding?" do + context "when target's __method__ returns __binding__" do + it "returns true" do + expect(subject.internal_binding?(Object.__binding__)).to be_truthy + end + end + + context "when target's __method__ returns __pry__" do + it "returns true" do + expect(subject.internal_binding?(Object.new.__binding__)).to be_truthy + end + end + + context "when target's __method__ returns nil" do + it "returns true" do + expect(subject.internal_binding?(TOPLEVEL_BINDING)).to be_falsey + end + end + end + + describe "#get_method_or_raise" do + subject do + Class.new do + class << self + include Pry::Helpers::CommandHelpers + + def command_name + 'test-command' + end + + def pry_instance + Pry.new + end + + def target + binding + end + end + end + end + + context "when there's name but no corresponding method" do + it "raises MethodNotFound" do + expect { subject.get_method_or_raise('foobar', binding) } + .to raise_error(Pry::MethodNotFound, /method.+could not be found/) + end + end + + context "when super opt is provided but there's no super method" do + let(:test_binding) do + def test_method; end + binding + end + + it "raises MethodNotFound" do + expect { subject.get_method_or_raise('test_method', test_binding, super: 1) } + .to raise_error(Pry::MethodNotFound, /has no super method/) + end + end + + context "when super opt is provided and there's a parent method" do + let(:test_binding) do + ParentClass = Class.new do + def test_method + :parent + end + end + + ChildClass = Class.new(ParentClass) do + def test_method + :child + end + end + + binding + end + + it "gets the parent method" do + method = subject.get_method_or_raise( + 'ChildClass#test_method', test_binding, super: 1 + ) + expect(method.owner).to eq(ParentClass) + end + end + + context "when there's no method name" do + it "raises MethodNotFound" do + expect { subject.get_method_or_raise(nil, binding) } + .to raise_error(Pry::MethodNotFound, /no method name given/) + end + end + end + + describe "#unindent" do + it "removes the same prefix from all lines" do + expect(subject.unindent(" one\n two\n")).to eq("one\ntwo\n") + end + + it "should not be phased by empty lines" do + expect(subject.unindent(" one\n\n two\n")).to eq("one\n\ntwo\n") + end + + it "should only remove a common prefix" do + expect(subject.unindent(" one\n two\n")).to eq(" one\ntwo\n") + end + + it "should also remove tabs if present" do + expect(subject.unindent("\tone\n\ttwo\n")).to eq("one\ntwo\n") + end + + it "should ignore lines starting with --" do + expect(subject.unindent(" one\n--\n two\n")).to eq("one\n--\ntwo\n") + end + end + + describe "#restrict_to_lines" do + context "when lines are specified as an integer" do + it "restricts the given string to the specified line number" do + expect(subject.restrict_to_lines("one\ntwo\n\three\nfour\n", 2)) + .to eq("two\n") + end + end + + context "when lines are specified as a range" do + it "restricts the given string to the specified range" do + expect(subject.restrict_to_lines("one\ntwo\n\three\nfour\n", 2...3)) + .to eq("two\n\three\n") + end + end + end + + describe "#one_index_number" do + context "when line number is more than 0" do + it "decrements the line number" do + expect(subject.one_index_number(2)).to eq(1) + end + end + + context "when line number is 0" do + it "returns the line number" do + expect(subject.one_index_number(0)).to eq(0) + end + end + end + + describe "#one_index_range" do + it "decrements range boundaries" do + expect(subject.one_index_range(3..30)).to eq(2..29) + end + end + + describe "#one_index_range_or_number" do + context "when given an integer" do + it "decrements the line number" do + expect(subject.one_index_range_or_number(2)).to eq(1) + end + end + + context "when given a range" do + it "decrements range boundaries" do + expect(subject.one_index_range_or_number(3..30)).to eq(2..29) + end + end + end + + describe "#absolute_index_number" do + context "when line number is zero" do + it "returns the line number" do + expect(subject.absolute_index_number(0, 1)).to eq(0) + end + end + + context "when line number is less than zero" do + it "returns the absolute sum of line number and array length" do + expect(subject.absolute_index_number(-2, 5)).to eq(3) + end + end + end + + describe "#absolute_index_range" do + context "when given an integer" do + it "returns a range based on the integer and array length" do + expect(subject.absolute_index_range(1, 2)).to eq(1..1) + end + end + + context "when given an integer" do + it "returns an absolute range that was decremented" do + expect(subject.absolute_index_range(-3..-20, 22)).to eq(19..2) + end + end + end + + describe "#set_file_and_dir_locals" do + let(:pry_instance) { Pry.new } + let(:context) { binding } + + it "injects local variable _file_" do + subject.set_file_and_dir_locals('foo/test.rb', pry_instance, context) + expect(context.eval('_file_')).to end_with('test.rb') + end + + it "injects local variable _dir_" do + subject.set_file_and_dir_locals('foo/test.rb', pry_instance, context) + expect(context.eval('_dir_')).to end_with('foo') + end + + it "sets pry instance's last_file to _file_" do + subject.set_file_and_dir_locals('foo/test.rb', pry_instance, context) + expect(pry_instance.last_file).to end_with('test.rb') + end + + it "sets pry instance's last_dir to _dir_" do + subject.set_file_and_dir_locals('foo/test.rb', pry_instance, context) + expect(pry_instance.last_dir).to end_with('foo') + end + end +end |