From 118377e6f0de6121eb1fa9f737d729344a742533 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 27 Jul 2012 19:29:40 -0700 Subject: [PATCH] Destroy sequence asks the user for confirmation. --- lib/vagrant/action/builder.rb | 9 ++++ lib/vagrant/action/builtin/call.rb | 2 +- lib/vagrant/action/runner.rb | 2 +- plugins/commands/destroy/command.rb | 43 +------------------ plugins/providers/virtualbox/action.rb | 26 ++++++++--- test/unit/vagrant/action/builder_test.rb | 12 ++++++ test/unit/vagrant/action/builtin/call_test.rb | 21 ++++++--- 7 files changed, 59 insertions(+), 56 deletions(-) diff --git a/lib/vagrant/action/builder.rb b/lib/vagrant/action/builder.rb index 00743e8b5..a13912403 100644 --- a/lib/vagrant/action/builder.rb +++ b/lib/vagrant/action/builder.rb @@ -17,6 +17,15 @@ module Vagrant # Vagrant::Action.run(app) # class Builder + # This is a shortcut for a middleware sequence with only one item + # in it. For a description of the arguments and the documentation, please + # see {#use} instead. + # + # @return [Builder] + def self.build(middleware, *args, &block) + new.use(middleware, *args, &block) + end + # Returns a mergeable version of the builder. If `use` is called with # the return value of this method, then the stack will merge, instead # of being treated as a separate single middleware. diff --git a/lib/vagrant/action/builtin/call.rb b/lib/vagrant/action/builtin/call.rb index 1b7065be7..f12c43954 100644 --- a/lib/vagrant/action/builtin/call.rb +++ b/lib/vagrant/action/builtin/call.rb @@ -40,7 +40,7 @@ module Vagrant # Build our new builder based on the result builder = Builder.new - @block.call(new_env[:result], builder) + @block.call(new_env, new_env[:result], builder) # Run the result with our new environment final_env = runner.run(builder, new_env) diff --git a/lib/vagrant/action/runner.rb b/lib/vagrant/action/runner.rb index b25796b38..b92d2726c 100644 --- a/lib/vagrant/action/runner.rb +++ b/lib/vagrant/action/runner.rb @@ -18,7 +18,7 @@ module Vagrant def run(callable_id, options=nil) callable = callable_id - callable = Builder.new.use(callable_id) if callable_id.kind_of?(Class) + callable = Builder.build(callable_id) if callable_id.kind_of?(Class) raise ArgumentError, "Argument to run must be a callable object or registered action." if !callable || !callable.respond_to?(:call) # Create the initial environment with the options given diff --git a/plugins/commands/destroy/command.rb b/plugins/commands/destroy/command.rb index 1fcd14f98..278e3bda3 100644 --- a/plugins/commands/destroy/command.rb +++ b/plugins/commands/destroy/command.rb @@ -20,52 +20,11 @@ module VagrantPlugins @logger.debug("'Destroy' each target VM...") with_target_vms(argv, :reverse => true) do |vm| vm.action(:destroy) - next - - if vm.created? - # Boolean whether we should actually go through with the destroy - # or not. This is true only if the "--force" flag is set or if the - # user confirms it. - do_destroy = false - - if options[:force] - do_destroy = true - else - choice = nil - - begin - choice = @env.ui.ask(I18n.t("vagrant.commands.destroy.confirmation", - :name => vm.name)) - rescue Interrupt - # Ctrl-C was pressed (or SIGINT). We just exit immediately - # with a non-zero exit status. - return 1 - rescue Vagrant::Errors::UIExpectsTTY - # We raise a more specific error but one which basically - # means the same thing. - raise Vagrant::Errors::DestroyRequiresForce - end - - do_destroy = choice.upcase == "Y" - end - - if do_destroy - @logger.info("Destroying: #{vm.name}") - vm.destroy - else - @logger.info("Not destroying #{vm.name} since confirmation was declined.") - @env.ui.success(I18n.t("vagrant.commands.destroy.will_not_destroy", - :name => vm.name), :prefix => false) - end - else - @logger.info("Not destroying #{vm.name}, since it isn't created.") - vm.ui.info I18n.t("vagrant.commands.common.vm_not_created") - end end # Success, exit status 0 0 - end + end end end end diff --git a/plugins/providers/virtualbox/action.rb b/plugins/providers/virtualbox/action.rb index 701868529..bab80fd4a 100644 --- a/plugins/providers/virtualbox/action.rb +++ b/plugins/providers/virtualbox/action.rb @@ -16,13 +16,25 @@ module VagrantPlugins def self.action_destroy Vagrant::Action::Builder.new.tap do |b| b.use CheckVirtualbox - b.use Call, Created do |result, b2| - # `result` is a boolean true/false of whether the VM is created or - # not. So if the VM _is_ created, then we continue with the - # destruction. - if result - b2.use Vagrant::Action::General::Validate - b2.use CheckAccessible + b.use Call, Created do |env, created, b2| + if created + # If the VM is created, then we confirm that we want to + # destroy it. + message = I18n.t("vagrant.commands.destroy.confirmation", + :name => env[:machine].name) + confirm = Vagrant::Action::Builder.build(Confirm, message) + + b2.use Call, confirm do |_env, confirmed, b3| + if confirmed + b3.use Vagrant::Action::General::Validate + b3.use CheckAccessible + else + env[:ui].info I18n.t("vagrant.commands.destroy.will_not_destroy", + :name => env[:machine.name]) + end + end + else + env[:ui].info I18n.t("vagrant.commands.common.vm_not_created") end end end diff --git a/test/unit/vagrant/action/builder_test.rb b/test/unit/vagrant/action/builder_test.rb index 411f22b99..ee00e0818 100644 --- a/test/unit/vagrant/action/builder_test.rb +++ b/test/unit/vagrant/action/builder_test.rb @@ -10,6 +10,18 @@ describe Vagrant::Action::Builder do Proc.new { |env| env[:data] << data } end + context "build" do + it "should provide build as a shortcut for basic sequences" do + data = {} + proc = Proc.new { |env| env[:data] = true } + + instance = described_class.build(proc) + instance.call(data) + + data[:data].should == true + end + end + context "basic `use`" do it "should add items to the stack and make them callable" do data = {} diff --git a/test/unit/vagrant/action/builtin/call_test.rb b/test/unit/vagrant/action/builtin/call_test.rb index 1dd8ea1cd..1618773df 100644 --- a/test/unit/vagrant/action/builtin/call_test.rb +++ b/test/unit/vagrant/action/builtin/call_test.rb @@ -11,7 +11,7 @@ describe Vagrant::Action::Builtin::Call do env[:result] = "value" end - described_class.new(app, env, callable) do |result, builder| + described_class.new(app, env, callable) do |_env, result, builder| received = result end.call({}) @@ -22,7 +22,7 @@ describe Vagrant::Action::Builtin::Call do received = 42 callable = lambda { |env| } - described_class.new(app, env, callable) do |result, builder| + described_class.new(app, env, callable) do |_env, result, builder| received = result end.call({}) @@ -33,7 +33,7 @@ describe Vagrant::Action::Builtin::Call do received = nil callable = lambda { |env| received = env[:foo] } - described_class.new(app, env, callable) do |result, builder| + described_class.new(app, env, callable) do |_env, result, builder| # Nothing. end.call({ :foo => :bar }) @@ -45,7 +45,7 @@ describe Vagrant::Action::Builtin::Call do callable = lambda { |env| } next_step = lambda { |env| received = "value" } - described_class.new(app, env, callable) do |result, builder| + described_class.new(app, env, callable) do |_env, result, builder| builder.use next_step end.call({}) @@ -57,10 +57,21 @@ describe Vagrant::Action::Builtin::Call do callable = lambda { |env| } next_step = lambda { |env| received = env[:foo] } - described_class.new(app, env, callable) do |result, builder| + described_class.new(app, env, callable) do |_env, result, builder| builder.use next_step end.call({ :foo => :bar }) received.should == :bar end + + it "should yield the environment" do + received = nil + callable = lambda { |env| env[:foo] = "value" } + + described_class.new(app, env, callable) do |env, _result, _builder| + received = env[:foo] + end.call({}) + + received.should == "value" + end end