refactor `args.after` and add `args.before`

New implementation allows for any number of chained commands.
The main command executes between "before" and "after" execution.

Commands can be simple strings, argument arrays or procs.

lib/hub/args.rb

@@ -14,27 +14,31 @@ module Hub
       @after = nil
       @skip = false
       @original_args = args.first
+      @chain = [nil]
     end
 
-    # With no arguments, returns the `after` callback.
-    #
-    # With a single argument, sets the `after` callback.
-    # Can be set to a string or a proc.
-    #
-    # If proc:
-    #   The proc is executed after the git command is executed. For
-    #   example, the `hub version` command sets the following proc to
-    #   print its information after running `git version`:
-    #
-    #     after { puts "hub version #{version_number}" }
-    #
-    # If string:
-    #   The string is assumed to be a command and executed after the
-    #   git command is executed:
-    #
-    #     after "echo 'hub version #{version_number}'"
-    def after(command = nil, &block)
-      @after ||= block ? block : command
+    # Adds an `after` callback.
+    # A callback can be a command or a proc.
+    def after(cmd_or_args = nil, args = nil, &block)
+      @chain.insert(-1, normalize_callback(cmd_or_args, args, block))
+    end
+
+    # Adds a `before` callback.
+    # A callback can be a command or a proc.
+    def before(cmd_or_args = nil, args = nil, &block)
+      @chain.insert(@chain.index(nil), normalize_callback(cmd_or_args, args, block))
+    end
+
+    # Tells if there are multiple (chained) commands or not.
+    def chained?
+      @chain.size > 1
+    end
+
+    # Returns an array of all commands.
+    def commands
+      chain = @chain.dup
+      chain[chain.index(nil)] = self.to_exec
+      chain
     end
 
     # Skip running this command.
@@ -47,15 +51,10 @@ module Hub
       @skip
     end
 
-    # Boolean indicating whether an `after` callback has been set.
-    def after?
-      !!@after
-    end
-
     # Array of `executable` followed by all args suitable as arguments
     # for `exec` or `system` calls.
-    def to_exec
-      [executable].concat self
+    def to_exec(args = self)
+      [executable].concat args
     end
 
     # All the words (as opposed to flags) contained in this argument
@@ -78,7 +77,23 @@ module Hub
 
     # Tests if arguments were modified since instantiation
     def changed?
-      self != @original_args
+      chained? or self != @original_args
+    end
+
+    private
+
+    def normalize_callback(cmd_or_args, args, block)
+      if block
+        block
+      elsif args
+        [cmd_or_args].concat args
+      elsif Array === cmd_or_args
+        self.to_exec cmd_or_args
+      elsif cmd_or_args
+        cmd_or_args
+      else
+        raise ArgumentError, "command or block required"
+      end
     end
   end
 end

lib/hub/commands.rb

@@ -170,11 +170,9 @@ module Hub
       }
 
       if names.any?
-        commands = names.map { |name| "git remote add #{name} #{github_url(:user => name)}" }
-        commands << args.to_exec.join(' ')
-        args.replace commands.shift.split(' ')
-        args.shift # don't want "git"
-        args.after commands.join('; ')
+        names.each do |name|
+          args.before ['remote', 'add', name, github_url(:user => name)]
+        end
       end
     end
 
@@ -203,18 +201,15 @@ module Hub
         end
 
         if user
-          # cherry-pick comes after the fetch
-          args.after args.to_exec.join(' ')
-
           if user == repo_owner
             # fetch from origin if the repo belongs to the user
-            args.replace ['fetch', default_remote]
+            args.before ['fetch', default_remote]
           elsif remotes.include?(user)
-            args.replace ['fetch', user]
+            args.before ['fetch', user]
           else
             secure = scheme == 'https:'
             remote_url = github_url(:user => user, :repo => repo, :private => secure)
-            args.replace ['remote', 'add', '-f', user, remote_url]
+            args.before ['remote', 'add', '-f', user, remote_url]
           end
         end
       end
@@ -306,13 +301,9 @@ module Hub
       remotes = args[1].split(',')
       args[1] = remotes.shift
 
-      after = "git push #{remotes.shift} #{branch}"
-
-      while remotes.length > 0
-        after += "; git push #{remotes.shift} #{branch}"
+      remotes.each do |name|
+        args.after ['push', name, branch]
       end
-
-      args.after after
     end
 
     # $ hub browse

lib/hub/runner.rb

@@ -23,22 +23,19 @@ module Hub
       new(*args).execute
     end
 
-    # Returns the current after callback, which (if set) is run after
-    # the target git command.
-    #
-    # See the `Hub::Args` class for more information on the `after`
-    # callback.
-    def after
-      args.after.to_s
-    end
-
-    # A string representation of the git command we would run if
-    # #execute were called.
+    # A string representation of the command that would run.
     def command
       if args.skip?
         ''
       else
-        args.to_exec.join(' ')
+        commands.join('; ')
+      end
+    end
+
+    # An array of all commands as strings.
+    def commands
+      args.commands.map do |cmd|
+        cmd.respond_to?(:join) ? cmd.join(' ') : cmd.to_s
       end
     end
 
@@ -50,25 +47,25 @@ module Hub
     # execution if they don't make sense.
     def execute
       unless args.skip?
-        if args.after?
-          execute_with_after_callback
+        if args.chained?
+          execute_command_chain
         else
           exec(*args.to_exec)
         end
       end
     end
 
-    # Runs the target git command then executes the `after` callback.
-    #
-    # See the `Hub::Args` class for more information on the `after`
-    # callback.
-    def execute_with_after_callback
-      after = args.after
-      if system(*args.to_exec)
-        after.respond_to?(:call) ? after.call : exec(after)
-        exit
-      else
-        exit 1
+    # Runs multiple commands in succession; exits at first failure.
+    def execute_command_chain
+      commands = args.commands
+      commands.each_with_index do |cmd, i|
+        if cmd.respond_to?(:call) then cmd.call
+        elsif i == commands.length - 1
+          # last command in chain
+          exec(*cmd)
+        else
+          exit($?.exitstatus) unless system(*cmd)
+        end
       end
     end
   end

test/helper.rb

@@ -70,6 +70,11 @@ class Test::Unit::TestCase
     assert_equal expected, Hub(input).command, "$ git #{input}"
   end
 
+  def assert_commands(*expected)
+    input = expected.pop
+    assert_equal expected, Hub(input).commands
+  end
+
   # Asserts that the command will be forwarded to git without changes
   def assert_forwarded(input)
     cmd = Hub(input)

test/hub_test.rb

@@ -240,18 +240,18 @@ class HubTest < Test::Unit::TestCase
     stub_remotes_group('xoebus', nil)
     stub_existing_fork('xoebus')
 
-    h = Hub("fetch xoebus")
-    assert_equal "git remote add xoebus git://github.com/xoebus/hub.git", h.command
-    assert_equal "git fetch xoebus", h.after
+    assert_commands "git remote add xoebus git://github.com/xoebus/hub.git",
+                    "git fetch xoebus",
+                    "fetch xoebus"
   end
 
   def test_fetch_new_remote_with_options
     stub_remotes_group('xoebus', nil)
     stub_existing_fork('xoebus')
 
-    h = Hub("fetch --depth=1 --prune xoebus")
-    assert_equal "git remote add xoebus git://github.com/xoebus/hub.git", h.command
-    assert_equal "git fetch --depth=1 --prune xoebus", h.after
+    assert_commands "git remote add xoebus git://github.com/xoebus/hub.git",
+                    "git fetch --depth=1 --prune xoebus",
+                    "fetch --depth=1 --prune xoebus"
   end
 
   def test_fetch_multiple_new_remotes
@@ -260,12 +260,10 @@ class HubTest < Test::Unit::TestCase
     stub_existing_fork('xoebus')
     stub_existing_fork('rtomayko')
 
-    h = Hub("fetch --multiple xoebus rtomayko")
-
-    assert_equal "git remote add xoebus git://github.com/xoebus/hub.git", h.command
-    expected = ["git remote add rtomayko git://github.com/rtomayko/hub.git"] <<
-                "git fetch --multiple xoebus rtomayko"
-    assert_equal expected.join('; '), h.after
+    assert_commands "git remote add xoebus git://github.com/xoebus/hub.git",
+                    "git remote add rtomayko git://github.com/rtomayko/hub.git",
+                    "git fetch --multiple xoebus rtomayko",
+                    "fetch --multiple xoebus rtomayko"
   end
 
   def test_fetch_multiple_comma_separated_remotes
@@ -274,12 +272,10 @@ class HubTest < Test::Unit::TestCase
     stub_existing_fork('xoebus')
     stub_existing_fork('rtomayko')
 
-    h = Hub("fetch xoebus,rtomayko")
-
-    assert_equal "git remote add xoebus git://github.com/xoebus/hub.git", h.command
-    expected = ["git remote add rtomayko git://github.com/rtomayko/hub.git"] <<
-                "git fetch --multiple xoebus rtomayko"
-    assert_equal expected.join('; '), h.after
+    assert_commands "git remote add xoebus git://github.com/xoebus/hub.git",
+                    "git remote add rtomayko git://github.com/rtomayko/hub.git",
+                    "git fetch --multiple xoebus rtomayko",
+                    "fetch xoebus,rtomayko"
   end
 
   def test_fetch_multiple_new_remotes_with_filtering
@@ -294,83 +290,65 @@ class HubTest < Test::Unit::TestCase
     # mygrp:  a remotes group; skipped
     # URL:    can't be a username; skipped
     # typo:   fork doesn't exist; skipped
-    h = Hub("fetch --multiple mislav xoebus mygrp git://example.com typo")
-
-    assert_equal "git remote add xoebus git://github.com/xoebus/hub.git", h.command
-    expected = "git fetch --multiple mislav xoebus mygrp git://example.com typo"
-    assert_equal expected, h.after
+    assert_commands "git remote add xoebus git://github.com/xoebus/hub.git",
+                    "git fetch --multiple mislav xoebus mygrp git://example.com typo",
+                    "fetch --multiple mislav xoebus mygrp git://example.com typo"
   end
 
   def test_cherry_pick
-    h = Hub("cherry-pick a319d88")
-    assert_equal "git cherry-pick a319d88", h.command
-    assert !h.args.after?
+    assert_forwarded "cherry-pick a319d88"
   end
 
   def test_cherry_pick_url
     url = 'http://github.com/mislav/hub/commit/a319d88'
-    h = Hub("cherry-pick #{url}")
-    assert_equal "git fetch mislav", h.command
-    assert_equal "git cherry-pick a319d88", h.after
+    assert_commands "git fetch mislav", "git cherry-pick a319d88", "cherry-pick #{url}"
   end
 
   def test_cherry_pick_url_with_fragment
     url = 'http://github.com/mislav/hub/commit/abcdef0123456789#comments'
-    h = Hub("cherry-pick #{url}")
-    assert_equal "git fetch mislav", h.command
-    assert_equal "git cherry-pick abcdef0123456789", h.after
+    assert_commands "git fetch mislav", "git cherry-pick abcdef0123456789", "cherry-pick #{url}"
   end
 
   def test_cherry_pick_url_with_remote_add
     url = 'http://github.com/xoebus/hub/commit/a319d88'
-    h = Hub("cherry-pick #{url}")
-    assert_equal "git remote add -f xoebus git://github.com/xoebus/hub.git", h.command
-    assert_equal "git cherry-pick a319d88", h.after
+    assert_commands "git remote add -f xoebus git://github.com/xoebus/hub.git",
+                    "git cherry-pick a319d88",
+                    "cherry-pick #{url}"
   end
 
   def test_cherry_pick_private_url_with_remote_add
     url = 'https://github.com/xoebus/hub/commit/a319d88'
-    h = Hub("cherry-pick #{url}")
-    assert_equal "git remote add -f xoebus git@github.com:xoebus/hub.git", h.command
-    assert_equal "git cherry-pick a319d88", h.after
+    assert_commands "git remote add -f xoebus git@github.com:xoebus/hub.git",
+                    "git cherry-pick a319d88",
+                    "cherry-pick #{url}"
   end
 
   def test_cherry_pick_origin_url
     url = 'https://github.com/defunkt/hub/commit/a319d88'
-    h = Hub("cherry-pick #{url}")
-    assert_equal "git fetch origin", h.command
-    assert_equal "git cherry-pick a319d88", h.after
+    assert_commands "git fetch origin", "git cherry-pick a319d88", "cherry-pick #{url}"
   end
 
   def test_cherry_pick_github_user_notation
-    h = Hub("cherry-pick mislav@a319d88")
-    assert_equal "git fetch mislav", h.command
-    assert_equal "git cherry-pick a319d88", h.after
+    assert_commands "git fetch mislav", "git cherry-pick a319d88", "cherry-pick mislav@a319d88"
   end
 
   def test_cherry_pick_github_user_repo_notation
     # not supported
-    h = Hub("cherry-pick mislav/hubbub@a319d88")
-    assert_equal "git cherry-pick mislav/hubbub@a319d88", h.command
-    assert !h.args.after?
+    assert_forwarded "cherry-pick mislav/hubbub@a319d88"
   end
 
   def test_cherry_pick_github_notation_too_short
-    h = Hub("cherry-pick mislav@a319")
-    assert_equal "git cherry-pick mislav@a319", h.command
-    assert !h.args.after?
+    assert_forwarded "cherry-pick mislav@a319"
   end
 
   def test_cherry_pick_github_notation_with_remote_add
-    h = Hub("cherry-pick xoebus@a319d88")
-    assert_equal "git remote add -f xoebus git://github.com/xoebus/hub.git", h.command
-    assert_equal "git cherry-pick a319d88", h.after
+    assert_commands "git remote add -f xoebus git://github.com/xoebus/hub.git",
+                    "git cherry-pick a319d88",
+                    "cherry-pick xoebus@a319d88"
   end
 
   def test_init
-    h = Hub("init -g")
-    assert_equal "git init", h.command
-    assert_equal "git remote add origin git@github.com:tpw/hub.git", h.after
+    assert_commands "git init", "git remote add origin git@github.com:tpw/hub.git", "init -g"
   end
 
   def test_init_no_login
@@ -382,15 +360,15 @@ class HubTest < Test::Unit::TestCase
   end
 
   def test_push_two
-    h = Hub("push origin,staging cool-feature")
-    assert_equal "git push origin cool-feature", h.command
-    assert_equal "git push staging cool-feature", h.after
+    assert_commands "git push origin cool-feature", "git push staging cool-feature",
+                    "push origin,staging cool-feature"
   end
 
   def test_push_more
-    h = Hub("push origin,staging,qa cool-feature")
-    assert_equal "git push origin cool-feature", h.command
-    assert_equal "git push staging cool-feature; git push qa cool-feature", h.after
+    assert_commands "git push origin cool-feature",
+                    "git push staging cool-feature",
+                    "git push qa cool-feature",
+                    "push origin,staging,qa cool-feature"
   end
 
   def test_create