Refactor massive execute_supervised_task_for_all_guards method.

- Renamed some Guard methods to be shorter and more consistent.
- Extract methods from execute_supervised_task_for_all_guards for less complexity.
- Added more specs for extracted methods.
- Added more docs on how marking of deleted/moved files works.
- Refactor Guard to be unaware of the :watch_all_modifications options for simplicity.

lib/guard.rb

@@ -33,10 +33,10 @@ module Guard
       @interactor = Interactor.new
       @listener   = Listener.select_and_init(@options[:watchdir] ? File.expand_path(@options[:watchdir]) : Dir.pwd, options)
 
-      @watch_all_modifications = options[:watch_all_modifications]
       @options[:notify] && ENV['GUARD_NOTIFY'] != 'false' ? Notifier.turn_on : Notifier.turn_off
 
       UI.clear if @options[:clear]
+
       debug_command_execution if @options[:debug]
 
       self
@@ -112,7 +112,7 @@ module Guard
 
       UI.info "Guard is now watching at '#{ listener.directory }'"
 
-      execute_supervised_task_for_all_guards(:start)
+      run_guard_task(:start)
 
       interactor.start
       listener.start
@@ -123,7 +123,7 @@ module Guard
     def stop
       UI.info 'Bye bye...', :reset => true
       listener.stop
-      execute_supervised_task_for_all_guards(:stop)
+      run_guard_task(:stop)
       abort
     end
 
@@ -131,7 +131,7 @@ module Guard
     #
     def reload
       run do
-        execute_supervised_task_for_all_guards(:reload)
+        run_guard_task(:reload)
       end
     end
 
@@ -139,7 +139,7 @@ module Guard
     #
     def run_all
       run do
-        execute_supervised_task_for_all_guards(:run_all)
+        run_guard_task(:run_all)
       end
     end
 
@@ -160,7 +160,7 @@ module Guard
     #
     def run_on_change(paths)
       run do
-        execute_supervised_task_for_all_guards(:run_on_change, paths)
+        run_guard_task(:run_on_change, paths)
       end
     end
 
@@ -172,68 +172,104 @@ module Guard
     def run
       listener.lock
       interactor.lock
+
       UI.clear if options[:clear]
+
       begin
         yield
       rescue Interrupt
       end
+
       interactor.unlock
       listener.unlock
     end
 
-    # Loop through all groups and execute the given task for each Guard in it,
+    # Loop through all groups and run the given task for each Guard.
-    # but halt the task execution for the all Guards within a group if one Guard
+    #
+    # Stop the task run for the all Guards within a group if one Guard
     # throws `:task_has_failed` and the group has its `:halt_on_fail` option to `true`.
     #
     # @param [Symbol] task the task to run
-    # @param [Array] files the list of files to pass to the task
+    # @param [Array<String>] files the list of files to pass to the task
     #
-    def execute_supervised_task_for_all_guards(task, files = nil)
+    def run_guard_task(task, files = nil)
       groups.each do |group|
         catch group.options[:halt_on_fail] == true ? :task_has_failed : :no_catch do
           guards(:group => group.name).each do |guard|
             if task == :run_on_change
-              paths = Watcher.match_files(guard, files)
+              run_on_change_task(files, guard, task)
-              unless paths.empty?
-                if @watch_all_modifications
-                  UI.debug "#{guard.class.name}##{task} with #{paths.inspect}"
-                  supervised_task(guard, task, paths.select { |f| !f.start_with?('!') })
-                  deletions = paths.collect { |f| f.slice(1..-1) if f.start_with?('!') }.compact
-                  unless deletions.empty?
-                    UI.debug "#{guard.class.name}#run_on_deletion with #{deletions.inspect}"
-                    supervised_task(guard, :run_on_deletion, deletions)
-                  end
-                else
-                  UI.debug "#{guard.class.name}##{task} with #{paths.inspect}"
-                  supervised_task(guard, task, paths)
-                end
-              end
             else
-              supervised_task(guard, task)
+              run_supervised_task(guard, task)
             end
           end
         end
       end
     end
 
-    # Let a Guard execute its task, but fire it
+    # Run the `:run_on_change` task. When the option `watch_all_modifications` is set,
-    # if his work leads to a system failure.
+    # the task is split to run changed paths on {Guard::Guard#run_on_change}, whereas
+    # deleted paths run on {Guard::Guard#run_on_deletion}.
+    #
+    # @param [Array<String>] files the list of files to pass to the task
+    # @param [Guard::Guard] guard the guard to run
+    # @param [Symbol] task the task to run
+    #
+    def run_on_change_task(files, guard, task)
+      paths = Watcher.match_files(guard, files)
+      changes = changed_paths(paths)
+      deletions = deleted_paths(paths)
+
+      unless changes.empty?
+        UI.debug "#{ guard.class.name }##{ task } with #{ changes.inspect }"
+        run_supervised_task(guard, task, changes)
+      end
+
+      unless deletions.empty?
+        UI.debug "#{ guard.class.name }#run_on_deletion with #{ deletions.inspect }"
+        run_supervised_task(guard, :run_on_deletion, deletions)
+      end
+    end
+
+    # Detects the paths that have changed.
+    #
+    # Deleted paths are prefixed by an exclamation point, @see Guard::Listener#modified_files
+    #
+    # @param [Array<String>] paths the watched paths
+    # @return [Array<String>] the changed paths
+    #
+    def changed_paths(paths)
+      paths.select { |f| !f.start_with?('!') }
+    end
+
+    # Detects the paths that have been deleted.
+    #
+    # Deleted paths are prefixed by an exclamation point, @see Guard::Listener#modified_files
+    #
+    # @param [Array<String>] paths the watched paths
+    # @return [Array<String>] the deleted paths
+    #
+    def deleted_paths(paths)
+      paths.select { |f| f.start_with?('!') }.map { |f| f.slice(1..-1) }
+    end
+
+    # Run a Guard task, but remove the Guard when his work leads to a system failure.
     #
     # @param [Guard::Guard] guard the Guard to execute
-    # @param [Symbol] task_to_supervise the task to run
+    # @param [Symbol] task the task to run
     # @param [Array] args the arguments for the task
     # @return [Boolean, Exception] the result of the Guard
     #
-    def supervised_task(guard, task_to_supervise, *args)
+    def run_supervised_task(guard, task, *args)
-      guard.hook("#{ task_to_supervise }_begin", *args)
+      guard.hook("#{ task }_begin", *args)
-      result = guard.send(task_to_supervise, *args)
+      result = guard.send(task, *args)
-      guard.hook("#{ task_to_supervise }_end", result)
+      guard.hook("#{ task }_end", result)
 
       result
 
     rescue Exception => ex
-      UI.error("#{ guard.class.name } failed to achieve its <#{ task_to_supervise.to_s }>, exception was:" +
+      UI.error("#{ guard.class.name } failed to achieve its <#{ task.to_s }>, exception was:" +
                "\n#{ ex.class }: #{ ex.message }\n#{ ex.backtrace.join("\n") }")
+
       guards.delete guard
       UI.info("\n#{ guard.class.name } has just been fired")
 

lib/guard/listener.rb

@@ -125,8 +125,11 @@ module Guard
 
     # Get the modified files.
     #
-    # If watch_all_modifications is true then moved and deleted files are also appended
+    # If the `:watch_all_modifications` option is true, then moved and
-    # to the returned array prefixed so !/home/user/dir/file.rb
+    # deleted files are also reported, but prefixed by an exclamation point.
+    #
+    # @example Deleted or moved file
+    #   !/home/user/dir/file.rb
     #
     # @param [Array<String>] dirs the watched directories
     # @param [Hash] options the listener options

spec/guard_spec.rb

@@ -342,7 +342,7 @@ describe Guard do
     end
   end
 
-  describe ".execute_supervised_task_for_all_guards" do
+  describe ".run_guard_task" do
     subject { ::Guard.setup }
 
     before do
@@ -363,7 +363,7 @@ describe Guard do
       end
 
       it "executes the task for each guard in each group" do
-        subject.execute_supervised_task_for_all_guards(:task)
+        subject.run_guard_task(:task)
 
         @sum.all? { |k, v| v == 2 }.should be_true
       end
@@ -384,7 +384,7 @@ describe Guard do
       end
 
       it "executes the task only for guards that didn't fail for group with :halt_on_fail == true" do
-        subject.execute_supervised_task_for_all_guards(:task)
+        subject.run_guard_task(:task)
 
         @sum[:foo].should eql 1
         @sum[:bar].should eql 7
@@ -392,7 +392,45 @@ describe Guard do
     end
   end
 
-  describe ".supervised_task" do
+  describe ".run_on_change_task" do
+    let(:guard) do
+      class Guard::Dummy < Guard::Guard
+        def watchers
+          [Guard::Watcher.new(/.+\.rb/)]
+        end
+      end
+
+      Guard::Dummy.new
+    end
+
+    it 'runs the :run_on_change task with the watched file changes' do
+      Guard.should_receive(:run_supervised_task).with(guard, :run_on_change, ['a.rb', 'b.rb'])
+      Guard.run_on_change_task(['a.rb', 'b.rb', 'templates/d.haml'], guard, :run_on_change)
+    end
+
+    it 'runs the :run_on_deletion task with the watched file deletions' do
+      Guard.should_receive(:run_supervised_task).with(guard, :run_on_deletion, ['c.rb'])
+      Guard.run_on_change_task(['!c.rb', '!templates/e.haml'], guard, :run_on_change)
+    end
+  end
+
+  describe ".changed_paths" do
+    let(:paths) { ['a.rb', 'b.rb', '!c.rb', 'templates/d.haml', '!templates/e.haml'] }
+
+    it 'returns the changed paths' do
+      Guard.changed_paths(paths).should =~ ['a.rb', 'b.rb', 'templates/d.haml']
+    end
+  end
+
+  describe ".deleted_paths" do
+    let(:paths) { ['a.rb', 'b.rb', '!c.rb', 'templates/d.haml', '!templates/e.haml'] }
+
+    it 'returns the deleted paths' do
+      Guard.deleted_paths(paths).should =~ ['c.rb', 'templates/e.haml']
+    end
+  end
+
+  describe ".run_supervised_task" do
     subject { ::Guard.setup }
 
     before do
@@ -408,22 +446,22 @@ describe Guard do
         end
 
         it "doesn't fire the Guard" do
-          lambda { subject.supervised_task(@g, :regular_without_arg) }.should_not change(subject.guards, :size)
+          lambda { subject.run_supervised_task(@g, :regular_without_arg) }.should_not change(subject.guards, :size)
         end
 
         it "returns the result of the task" do
-          ::Guard.supervised_task(@g, :regular_without_arg).should be_true
+          ::Guard.run_supervised_task(@g, :regular_without_arg).should be_true
         end
 
         it "passes the args to the :begin hook" do
           @g.should_receive(:hook).with("regular_without_arg_begin", "given_path")
-          ::Guard.supervised_task(@g, :regular_without_arg, "given_path")
+          ::Guard.run_supervised_task(@g, :regular_without_arg, "given_path")
         end
 
         it "passes the result of the supervised method to the :end hook" do
           @g.should_receive(:hook).with("regular_without_arg_begin", "given_path")
           @g.should_receive(:hook).with("regular_without_arg_end", true)
-          ::Guard.supervised_task(@g, :regular_without_arg, "given_path")
+          ::Guard.run_supervised_task(@g, :regular_without_arg, "given_path")
         end
       end
 
@@ -433,17 +471,17 @@ describe Guard do
         end
 
         it "doesn't fire the Guard" do
-          lambda { subject.supervised_task(@g, :regular_with_arg, "given_path") }.should_not change(subject.guards, :size)
+          lambda { subject.run_supervised_task(@g, :regular_with_arg, "given_path") }.should_not change(subject.guards, :size)
         end
 
         it "returns the result of the task" do
-          ::Guard.supervised_task(@g, :regular_with_arg, "given_path").should eql "I'm a success"
+          ::Guard.run_supervised_task(@g, :regular_with_arg, "given_path").should eql "I'm a success"
         end
 
         it "calls the default begin hook but not the default end hook" do
           @g.should_receive(:hook).with("failing_begin")
           @g.should_not_receive(:hook).with("failing_end")
-          ::Guard.supervised_task(@g, :failing)
+          ::Guard.run_supervised_task(@g, :failing)
         end
       end
     end
@@ -452,7 +490,7 @@ describe Guard do
       before(:each) { @g.stub!(:group) { :foo }; @g.stub!(:failing) { throw :task_has_failed } }
 
       it "throws :task_has_failed" do
-        expect { subject.supervised_task(@g, :failing) }.to throw_symbol(:task_has_failed)
+        expect { subject.run_supervised_task(@g, :failing) }.to throw_symbol(:task_has_failed)
       end
     end
 
@@ -460,12 +498,12 @@ describe Guard do
       before(:each) { @g.stub!(:failing) { raise "I break your system" } }
 
       it "fires the Guard" do
-        lambda { subject.supervised_task(@g, :failing) }.should change(subject.guards, :size).by(-1)
+        lambda { subject.run_supervised_task(@g, :failing) }.should change(subject.guards, :size).by(-1)
         subject.guards.should_not include(@g)
       end
 
       it "returns the exception" do
-        failing_result = ::Guard.supervised_task(@g, :failing)
+        failing_result = ::Guard.run_supervised_task(@g, :failing)
         failing_result.should be_kind_of(Exception)
         failing_result.message.should == 'I break your system'
       end