Delete Pry::CommandSet#{before,after}_command

These commands have been deprecated for years. It's high time to get rid of them.

4 files changed, 83 insertions, 166 deletions

diff --git a/lib/pry/command.rb b/lib/pry/command.rb
index 40abbd99..257e20c2 100644
--- a/lib/pry/command.rb
+++ b/lib/pry/command.rb
@@ -170,8 +170,6 @@ class Pry
 
       # @deprecated Replaced with {Pry::Hooks#add_hook}. Left for compatibility.
       # Store hooks to be run before or after the command body.
-      # @see Pry::CommandSet#before_command
-      # @see Pry::CommandSet#after_command
       def hooks
         Pry.hooks
       end
diff --git a/lib/pry/command_set.rb b/lib/pry/command_set.rb
index 3d3e313c..0fb7ef0d 100644
--- a/lib/pry/command_set.rb
+++ b/lib/pry/command_set.rb
@@ -116,41 +116,6 @@ class Pry
       @commands[match]
     end
 
-    # Execute a block of code before a command is invoked. The block also
-    # gets access to parameters that will be passed to the command and
-    # is evaluated in the same context.
-    # @param [String, Regexp] search The match or listing of the command.
-    # @yield The block to be run before the command.
-    # @example Display parameter before invoking command
-    #   Pry.config.commands.before_command("whereami") do |n|
-    #     output.puts "parameter passed was #{n}"
-    #   end
-    # @deprecated Use {Pry::Hooks#add_hook} instead.
-    def before_command(search, &block)
-      cmd = find_command_by_match_or_listing(search)
-      cmd.hooks.add_hook("before_#{cmd.command_name}", random_hook_name, block)
-    end
-
-    # Execute a block of code after a command is invoked. The block also
-    # gets access to parameters that will be passed to the command and
-    # is evaluated in the same context.
-    # @param [String, Regexp] search The match or listing of the command.
-    # @yield The block to be run after the command.
-    # @example Display text 'command complete' after invoking command
-    #   Pry.config.commands.after_command("whereami") do |n|
-    #     output.puts "command complete!"
-    #   end
-    # @deprecated Use {Pry::Hooks#add_hook} instead.
-    def after_command(search, &block)
-      cmd = find_command_by_match_or_listing(search)
-      cmd.hooks.add_hook("after_#{cmd.command_name}", random_hook_name, block)
-    end
-
-    def random_hook_name
-      (0...8).map { ('a'..'z').to_a[rand(26)] }.join
-    end
-    private :random_hook_name
-
     def each(&block)
       @commands.each(&block)
     end
diff --git a/spec/command_set_spec.rb b/spec/command_set_spec.rb
index 9fefcb30..4cd8b676 100644
--- a/spec/command_set_spec.rb
+++ b/spec/command_set_spec.rb
@@ -388,113 +388,110 @@ describe Pry::CommandSet do
     end
   end
 
-  describe "command decorators - before_command and after_command" do
-    describe "before_command" do
-      it 'should be called before the original command' do
-        foo = []
-        @set.command('foo') { foo << 1 }
-        @set.before_command('foo') { foo << 2 }
-        @set.run_command(@ctx, 'foo')
-
-        expect(foo).to eq [2, 1]
-      end
+  describe "before_* hook" do
+    it 'should be called before the original command' do
+      foo = []
+      @set.command('foo') { foo << 1 }
+      @set['foo'].hooks.add_hook('before_foo', 'name') { foo << 2 }
+      @set.run_command(@ctx, 'foo')
 
-      it 'should be called before the original command, using listing name' do
-        foo = []
-        @set.command(/^foo1/, '', listing: 'foo') { foo << 1 }
-        @set.before_command('foo') { foo << 2 }
-        @set.run_command(@ctx, /^foo1/)
+      expect(foo).to eq [2, 1]
+    end
 
-        expect(foo).to eq [2, 1]
-      end
+    it 'should be called before the original command, using listing name' do
+      foo = []
+      @set.command(/^foo1/, '', listing: 'foo') { foo << 1 }
+      cmd = @set.find_command_by_match_or_listing('foo')
+      cmd.hooks.add_hook('before_foo', 'name') { foo << 2 }
+      @set.run_command(@ctx, /^foo1/)
 
-      it 'should share the context with the original command' do
-        @ctx[:target] = "test target string".__binding__
-        before_val  = nil
-        orig_val    = nil
-        @set.command('foo') { orig_val = target }
-        @set.before_command('foo') { before_val = target }
-        @set.run_command(@ctx, 'foo')
+      expect(foo).to eq [2, 1]
+    end
 
-        expect(before_val).to eq @ctx[:target]
-        expect(orig_val).to eq @ctx[:target]
-      end
+    it 'should share the context with the original command' do
+      @ctx[:target] = "test target string".__binding__
+      before_val  = nil
+      orig_val    = nil
+      @set.command('foo') { orig_val = target }
+      @set['foo'].hooks.add_hook('before_foo', 'name') { before_val = target }
+      @set.run_command(@ctx, 'foo')
 
-      it 'should work when applied multiple times' do
-        foo = []
-        @set.command('foo') { foo << 1 }
-        @set.before_command('foo') { foo << 2 }
-        @set.before_command('foo') { foo << 3 }
-        @set.before_command('foo') { foo << 4 }
-        @set.run_command(@ctx, 'foo')
+      expect(before_val).to eq @ctx[:target]
+      expect(orig_val).to eq @ctx[:target]
+    end
 
-        expect(foo).to eq [2, 3, 4, 1]
-      end
+    it 'should work when applied multiple times' do
+      foo = []
+      @set.command('foo') { foo << 1 }
+      @set['foo'].hooks.add_hook('before_foo', 'name1') { foo << 2 }
+      @set['foo'].hooks.add_hook('before_foo', 'name2') { foo << 3 }
+      @set['foo'].hooks.add_hook('before_foo', 'name3') { foo << 4 }
+      @set.run_command(@ctx, 'foo')
 
+      expect(foo).to eq [2, 3, 4, 1]
     end
+  end
 
-    describe "after_command" do
-      it 'should be called after the original command' do
-        foo = []
-        @set.command('foo') { foo << 1 }
-        @set.after_command('foo') { foo << 2 }
-        @set.run_command(@ctx, 'foo')
+  describe "after_* hooks" do
+    it 'should be called after the original command' do
+      foo = []
+      @set.command('foo') { foo << 1 }
+      @set['foo'].hooks.add_hook('after_foo', 'name') { foo << 2 }
+      @set.run_command(@ctx, 'foo')
 
-        expect(foo).to eq [1, 2]
-      end
+      expect(foo).to eq [1, 2]
+    end
 
-      it 'should be called after the original command, using listing name' do
-        foo = []
-        @set.command(/^foo1/, '', listing: 'foo') { foo << 1 }
-        @set.after_command('foo') { foo << 2 }
-        @set.run_command(@ctx, /^foo1/)
+    it 'should be called after the original command, using listing name' do
+      foo = []
+      @set.command(/^foo1/, '', listing: 'foo') { foo << 1 }
+      cmd = @set.find_command_by_match_or_listing('foo')
+      cmd.hooks.add_hook('after_foo', 'name') { foo << 2 }
+      @set.run_command(@ctx, /^foo1/)
 
-        expect(foo).to eq [1, 2]
-      end
+      expect(foo).to eq [1, 2]
+    end
 
-      it 'should share the context with the original command' do
-        @ctx[:target] = "test target string".__binding__
-        after_val   = nil
-        orig_val    = nil
-        @set.command('foo') { orig_val = target }
-        @set.after_command('foo') { after_val = target }
-        @set.run_command(@ctx, 'foo')
+    it 'should share the context with the original command' do
+      @ctx[:target] = "test target string".__binding__
+      after_val   = nil
+      orig_val    = nil
+      @set.command('foo') { orig_val = target }
+      @set['foo'].hooks.add_hook('after_foo', 'name') { after_val = target }
+      @set.run_command(@ctx, 'foo')
 
-        expect(after_val).to eq @ctx[:target]
-        expect(orig_val).to eq @ctx[:target]
-      end
+      expect(after_val).to eq @ctx[:target]
+      expect(orig_val).to eq @ctx[:target]
+    end
 
-      it 'should determine the return value for the command' do
-        @set.command('foo', 'bar', keep_retval: true) { 1 }
-        @set.after_command('foo') { 2 }
-        expect(@set.run_command(@ctx, 'foo')).to eq 2
-      end
+    it 'should determine the return value for the command' do
+      @set.command('foo', 'bar', keep_retval: true) { 1 }
+      @set['foo'].hooks.add_hook('after_foo', 'name') { 2 }
+      expect(@set.run_command(@ctx, 'foo')).to eq 2
+    end
 
-      it 'should work when applied multiple times' do
-        foo = []
-        @set.command('foo') { foo << 1 }
-        @set.after_command('foo') { foo << 2 }
-        @set.after_command('foo') { foo << 3 }
-        @set.after_command('foo') { foo << 4 }
-        @set.run_command(@ctx, 'foo')
+    it 'should work when applied multiple times' do
+      foo = []
+      @set.command('foo') { foo << 1 }
+      @set['foo'].hooks.add_hook('after_foo', 'name1') { foo << 2 }
+      @set['foo'].hooks.add_hook('after_foo', 'name2') { foo << 3 }
+      @set['foo'].hooks.add_hook('after_foo', 'name3') { foo << 4 }
+      @set.run_command(@ctx, 'foo')
 
-        expect(foo).to eq [1, 2, 3, 4]
-      end
+      expect(foo).to eq [1, 2, 3, 4]
     end
+  end
 
-    describe "before_command and after_command" do
-      it 'should work when combining both before_command and after_command' do
-        foo = []
-        @set.command('foo') { foo << 1 }
-        @set.after_command('foo') { foo << 2 }
-        @set.before_command('foo') { foo << 3 }
-        @set.run_command(@ctx, 'foo')
-
-        expect(foo).to eq [3, 1, 2]
-      end
+  describe "before_command and after_command" do
+    it 'should work when combining both before_command and after_command' do
+      foo = []
+      @set.command('foo') { foo << 1 }
+      @set['foo'].hooks.add_hook('after_foo', 'name') { foo << 2 }
+      @set['foo'].hooks.add_hook('before_foo', 'name') { foo << 3 }
+      @set.run_command(@ctx, 'foo')
 
+      expect(foo).to eq [3, 1, 2]
     end
-
   end
 
   describe 'find_command' do
diff --git a/spec/command_spec.rb b/spec/command_spec.rb
index d7c1d406..071474eb 100644
--- a/spec/command_spec.rb
+++ b/spec/command_spec.rb
@@ -45,34 +45,6 @@ describe "Pry::Command" do
       expect(mock_command(cmd).return).to eq 5
     end
 
-    context "deprecated API" do
-      it "should call hooks in the right order" do
-        cmd = @set.create_command 'marvin', "Pained by the diodes in his left side" do
-          def process
-            output.puts 1 + args[0].to_i
-          end
-        end
-
-        @set.before_command 'marvin' do |i|
-          output.puts 3 - i.to_i
-        end
-
-        @set.before_command 'marvin' do |i|
-          output.puts 4 - i.to_i
-        end
-
-        @set.after_command 'marvin' do |i|
-          output.puts 2 + i.to_i
-        end
-
-        @set.after_command 'marvin' do |i|
-          output.puts 3 + i.to_i
-        end
-
-        expect(mock_command(cmd, %w(2)).output).to eq "1\n2\n3\n4\n5\n"
-      end
-    end
-
     context "hooks API" do
       before do
         @set.create_command 'jamaica', 'Out of Many, One People' do
@@ -106,21 +78,6 @@ describe "Pry::Command" do
         expect(out).to eq("1\n2\n3\n4\n5\n")
       end
     end
-
-    # TODO: This strikes me as rather silly...
-    it 'should return the value from the last hook with keep_retval' do
-      cmd = @set.create_command 'slartibartfast', "Designs Fjords", keep_retval: true do
-        def process
-          22
-        end
-      end
-
-      @set.after_command 'slartibartfast' do
-        10
-      end
-
-      expect(mock_command(cmd).return).to eq 10
-    end
   end
 
   describe 'help' do