Make the "no matching servers" error more sane (closes #8828)

git-svn-id: http://svn.rubyonrails.org/rails/tools/capistrano@7205 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

CHANGELOG

@@ -1,5 +1,7 @@
 *SVN*
 
+* Make the "no matching servers" error more sane [halorgium]
+
 * Make sure the invoke task gives a sane error when the COMMAND value is omitted [halorgium]
 
 * Make sure variables are conditionally set in the deploy recipes, so as not to clobber values set elsewhere [Jamis Buck]

lib/capistrano/configuration/connections.rb

@@ -105,7 +105,7 @@ module Capistrano
           servers = find_servers_for_task(task, options)
 
           if servers.empty?
-            raise ScriptError, "`#{task.fully_qualified_name}' is only run for servers matching #{task.options.inspect}, but no servers matched"
+            raise Capistrano::NoMatchingServersError, "`#{task.fully_qualified_name}' is only run for servers matching #{task.options.inspect}, but no servers matched"
           end
 
           if task.continue_on_error?
@@ -114,7 +114,7 @@ module Capistrano
           end
         else
           servers = find_servers(options)
-          raise ScriptError, "no servers found to match #{options.inspect}" if servers.empty?
+          raise Capistrano::NoMatchingServersError, "no servers found to match #{options.inspect}" if servers.empty?
         end
 
         servers = [servers.first] if options[:once]

lib/capistrano/errors.rb

@@ -3,6 +3,7 @@ module Capistrano
 
   class CaptureError < Error; end
   class NoSuchTaskError < Error; end
+  class NoMatchingServersError < Error; end
   
   class RemoteError < Error
     attr_accessor :hosts

lib/capistrano/shell.rb

@@ -160,7 +160,7 @@ HELP
           connect(task)
           configuration.execute_task(task)
         end
-      rescue Capistrano::NoSuchTaskError => error
+      rescue Capistrano::NoMatchingServersError, Capistrano::NoSuchTaskError => error
         warn "error: #{error.message}"
       end
 

lib/capistrano/task_definition.rb

@@ -3,10 +3,12 @@ require 'capistrano/server_definition'
 module Capistrano
   # Represents the definition of a single task.
   class TaskDefinition
-    attr_reader :name, :namespace, :options, :body
+    attr_reader :name, :namespace, :options, :body, :desc, :on_error
 
     def initialize(name, namespace, options={}, &block)
       @name, @namespace, @options = name, namespace, options
+      @desc = @options.delete(:desc)
+      @on_error = options.delete(:on_error)
       @body = block or raise ArgumentError, "a task requires a block"
       @servers = nil
     end
@@ -28,7 +30,7 @@ module Capistrano
     def description(rebuild=false)
       @description = nil if rebuild
       @description ||= begin
-        description = options[:desc] || ""
+        description = @desc || ""
 
         indentation = description[/\A\s+/]
         if indentation
@@ -61,7 +63,7 @@ module Capistrano
     # Indicates whether the task wants to continue, even if a server has failed
     # previously
     def continue_on_error?
-      options[:on_error] == :continue
+      @on_error == :continue
     end
   end
 end

test/configuration/connections_test.rb

@@ -138,13 +138,13 @@ class ConfigurationConnectionsTest < Test::Unit::TestCase
 
   def test_execute_on_servers_without_current_task_should_raise_error_if_no_matching_servers
     @config.expects(:find_servers).with(:a => :b, :c => :d).returns([])
-    assert_raises(ScriptError) { @config.execute_on_servers(:a => :b, :c => :d) { |list| } }
+    assert_raises(Capistrano::NoMatchingServersError) { @config.execute_on_servers(:a => :b, :c => :d) { |list| } }
   end
 
   def test_execute_on_servers_should_raise_an_error_if_the_current_task_has_no_matching_servers_by_default
     @config.current_task = mock_task
     @config.expects(:find_servers_for_task).with(@config.current_task, {}).returns([])
-    assert_raises(ScriptError) do
+    assert_raises(Capistrano::NoMatchingServersError) do
       @config.execute_on_servers do
         flunk "should not get here"
       end

test/configuration/namespace_dsl_test.rb

@@ -58,14 +58,14 @@ class ConfigurationNamespacesDSLTest < Test::Unit::TestCase
     @config.desc "A description"
     @config.task(:testing) { puts "foo" }
     @config.task(:another) { puts "bar" }
-    assert_equal "A description", @config.tasks[:testing].options[:desc]
-    assert_nil @config.tasks[:another].options[:desc]
+    assert_equal "A description", @config.tasks[:testing].desc
+    assert_nil @config.tasks[:another].desc
   end
 
   def test_pending_desc_should_apply_only_to_next_task_in_any_namespace
     @config.desc "A description"
     @config.namespace(:outer) { task(:testing) { puts "foo" } }
-    assert_equal "A description", @config.namespaces[:outer].tasks[:testing].options[:desc]
+    assert_equal "A description", @config.namespaces[:outer].tasks[:testing].desc
   end
 
   def test_defining_task_without_block_should_raise_error