Refactor command hooks so they could use Pry::Hooks (newer API)

Fixes #651 ({after,before}_command hooks should be unified with
Pry::Hooks system)

* The support for the `Pry.commands.{after,before}_command` API was kept
* Hooks defined via the older API use random names
* The order of hooks execution was changed from LIFO to FIFO

lib/pry/command.rb

@@ -167,11 +167,12 @@ class Pry
         end
       end
 
+      # @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
-        @hooks ||= {:before => [], :after => []}
+        Pry.hooks
       end
 
       def command_regex
@@ -223,6 +224,7 @@ class Pry
     attr_accessor :arg_string
     attr_accessor :context
     attr_accessor :command_set
+    attr_accessor :hooks
     attr_accessor :_pry_
 
     # The block we pass *into* a command so long as `:takes_block` is
@@ -271,6 +273,7 @@ class Pry
       self.output       = context[:output]
       self.eval_string  = context[:eval_string]
       self.command_set  = context[:command_set]
+      self.hooks        = context[:hooks]
       self._pry_        = context[:pry_instance]
     end
 
@@ -445,17 +448,28 @@ class Pry
 
     private
 
+    def find_hooks(event)
+      event_name = "#{event}_#{command_name}"
+      (self.hooks || self.class.hooks).get_hooks(event_name).values
+    end
+
+    def before_hooks
+      find_hooks('before')
+    end
+
+    def after_hooks
+      find_hooks('after')
+    end
+
     # Run the `#call` method and all the registered hooks.
     # @param [Array<String>] args The arguments to `#call`
     # @return [Object] The return value from `#call`
     def call_with_hooks(*args)
-      self.class.hooks[:before].each do |block|
-        instance_exec(*args, &block)
-      end
+      before_hooks.each { |block| instance_exec(*args, &block) }
 
       ret = call(*args)
 
-      self.class.hooks[:after].each do |block|
+      after_hooks.each do |block|
         ret = instance_exec(*args, &block)
       end
 

lib/pry/command_set.rb

@@ -125,9 +125,10 @@ class Pry
     #   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[:before].unshift block
+      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
@@ -139,11 +140,17 @@ class Pry
     #   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[:after] << block
+      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

lib/pry/pry_instance.rb

@@ -406,7 +406,8 @@ class Pry
       :target => current_binding,
       :output => output,
       :eval_string => @eval_string,
-      :pry_instance => self
+      :pry_instance => self,
+      :hooks => hooks
     )
 
     # set a temporary (just so we can inject the value we want into eval_string)

spec/command_set_spec.rb

@@ -428,7 +428,7 @@ describe Pry::CommandSet do
         @set.before_command('foo') { foo << 4 }
         @set.run_command(@ctx, 'foo')
 
-        expect(foo).to eq [4, 3, 2, 1]
+        expect(foo).to eq [2, 3, 4, 1]
       end
 
     end

spec/command_spec.rb

@@ -45,29 +45,66 @@ describe "Pry::Command" do
       expect(mock_command(cmd).return).to eq 5
     end
 
-    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 3 + args[0].to_i
+    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
+          def process
+            output.puts 1 + args[0].to_i
+          end
         end
       end
 
-      @set.before_command 'marvin' do |i|
-        output.puts 2 + i.to_i
-      end
-      @set.before_command 'marvin' do |i|
-        output.puts 1 + i.to_i
-      end
+      let(:hooks) {
+        h = Pry::Hooks.new
+        h.add_hook('before_jamaica', 'name1') do |i|
+          output.puts 3 - i.to_i
+        end
 
-      @set.after_command 'marvin' do |i|
-        output.puts 4 + i.to_i
-      end
+        h.add_hook('before_jamaica', 'name2') do |i|
+          output.puts 4 - i.to_i
+        end
 
-      @set.after_command 'marvin' do |i|
-        output.puts 5 + i.to_i
-      end
+        h.add_hook('after_jamaica', 'name3') do |i|
+          output.puts 2 + i.to_i
+        end
 
-      expect(mock_command(cmd, %w(2)).output).to eq "3\n4\n5\n6\n7\n"
+        h.add_hook('after_jamaica', 'name4') do |i|
+          output.puts 3 + i.to_i
+        end
+      }
+
+      it "should call hooks in the right order" do
+        out = pry_tester(hooks: hooks, commands: @set).process_command('jamaica 2')
+        expect(out).to eq("1\n2\n3\n4\n5\n")
+      end
     end
 
     # TODO: This strikes me as rather silly...