Merge branch 'prettier-role-bulk-delete'

4 files changed, 42 insertions, 27 deletions

diff --git a/chef/lib/chef/knife.rb b/chef/lib/chef/knife.rb
index 6b62516adc..1b28f5382e 100644
--- a/chef/lib/chef/knife.rb
+++ b/chef/lib/chef/knife.rb
@@ -477,7 +477,7 @@ class Chef
       output(format_for_display(object)) if config[:print_after]
 
       obj_name = delete_name ? "#{delete_name}[#{name}]" : object
-      self.msg("Deleted #{obj_name}!")
+      self.msg("Deleted #{obj_name}")
     end
 
     def bulk_delete(klass, fancy_name, delete_name=nil, list=nil, regex=nil, &block)
diff --git a/chef/lib/chef/knife/client_create.rb b/chef/lib/chef/knife/client_create.rb
index 6fb078b7d4..5e6af55bc4 100644
--- a/chef/lib/chef/knife/client_create.rb
+++ b/chef/lib/chef/knife/client_create.rb
@@ -57,7 +57,7 @@ class Chef
 
         key = output.save
 
-        ui.info("Created (or updated) #{output}")
+        ui.info("Created #{output}")
 
         if config[:file]
           File.open(config[:file], "w") do |f|
diff --git a/chef/lib/chef/knife/role_bulk_delete.rb b/chef/lib/chef/knife/role_bulk_delete.rb
index b9d4a06347..8b24f55846 100644
--- a/chef/lib/chef/knife/role_bulk_delete.rb
+++ b/chef/lib/chef/knife/role_bulk_delete.rb
@@ -31,10 +31,33 @@ class Chef
 
       def run
         if @name_args.length < 1
-          ui.fatal("You must supply a regular expression to match the results against")
-          exit 42
-        else
-          bulk_delete(Chef::Role, "role", nil, nil, @name_args[0])
+          ui.error("You must supply a regular expression to match the results against")
+          exit 1
+        end
+
+        all_roles = Chef::Role.list(true)
+
+        matcher = /#{@name_args[0]}/
+        roles_to_delete = {}
+        all_roles.each do |name, role|
+          next unless name =~ matcher
+          roles_to_delete[role.name] = role
+        end
+
+        if roles_to_delete.empty?
+          ui.info "No roles match the expression /#{@name_args[0]}/"
+          exit 0
+        end
+
+        ui.msg("The following roles will be deleted:")
+        ui.msg("")
+        ui.msg(ui.list(roles_to_delete.keys.sort, :columns_down))
+        ui.msg("")
+        ui.confirm("Are you sure you want to delete these roles")
+
+        roles_to_delete.sort.each do |name, role|
+          role.destroy
+          ui.msg("Deleted role #{name}")
         end
       end
     end
diff --git a/chef/spec/unit/knife/role_bulk_delete_spec.rb b/chef/spec/unit/knife/role_bulk_delete_spec.rb
index 22410d1f88..867320911b 100644
--- a/chef/spec/unit/knife/role_bulk_delete_spec.rb
+++ b/chef/spec/unit/knife/role_bulk_delete_spec.rb
@@ -6,9 +6,9 @@
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
 # You may obtain a copy of the License at
-# 
+#
 #     http://www.apache.org/licenses/LICENSE-2.0
-# 
+#
 # Unless required by applicable law or agreed to in writing, software
 # distributed under the License is distributed on an "AS IS" BASIS,
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -26,8 +26,9 @@ describe Chef::Knife::RoleBulkDelete do
       :print_after => nil
     }
     @knife.name_args = ["."]
-    @knife.stub!(:output).and_return(:true)
-    @knife.stub!(:confirm).and_return(true)
+    @stdout = StringIO.new
+    @knife.ui.stub!(:stdout).and_return(@stdout)
+    @knife.ui.stub!(:confirm).and_return(true)
     @roles = Hash.new
     %w{dev staging production}.each do |role_name|
       role = Chef::Role.new()
@@ -37,31 +38,31 @@ describe Chef::Knife::RoleBulkDelete do
     end
     Chef::Role.stub!(:list).and_return(@roles)
   end
-  
+
   describe "run" do
-    
+
     it "should get the list of the roles" do
       Chef::Role.should_receive(:list).and_return(@roles)
       @knife.run
     end
-    
+
     it "should print the roles you are about to delete" do
-      @knife.should_receive(:output).with(@knife.format_list_for_display(@roles))
       @knife.run
+      @stdout.string.should match(/#{@knife.ui.list(@roles.keys.sort, :columns_down)}/)
     end
-    
+
     it "should confirm you really want to delete them" do
-      @knife.should_receive(:confirm)
+      @knife.ui.should_receive(:confirm)
       @knife.run
     end
-    
+
     it "should delete each role" do
       @roles.each_value do |r|
         r.should_receive(:destroy)
       end
       @knife.run
     end
-    
+
     it "should only delete roles that match the regex" do
       @knife.name_args = ["dev"]
       @roles["dev"].should_receive(:destroy)
@@ -75,14 +76,5 @@ describe Chef::Knife::RoleBulkDelete do
       lambda { @knife.run }.should raise_error(SystemExit)
     end
 
-    describe "with -p or --print_after" do
-      it "should pretty_print the roles, formatted for display" do
-        @knife.config[:print_after] = true
-        @roles.each_value do |n|
-          @knife.should_receive(:output).with(@knife.format_for_display(n))
-        end
-        @knife.run
-      end
-    end
   end
 end