From 5f1caf3ada2436c84666f9d01d70f3a43f4f7ab4 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 26 Feb 2014 08:01:08 -0800 Subject: [PATCH] core: re-ask if confirmation bad input [GH-3027] --- CHANGELOG.md | 1 + lib/vagrant/action/builtin/confirm.rb | 15 ++++++++++++--- lib/vagrant/action/builtin/destroy_confirm.rb | 2 +- .../unit/vagrant/action/builtin/confirm_test.rb | 17 +++++++++++++++++ 4 files changed, 31 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b39e5c17..a8b74ab63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ IMPROVEMENTS: - core: Added "error-exit" type to machine-readable output which contains error information that caused a non-zero exit status. [GH-2999] + - command/destroy: confirmation will re-ask question if bad input. [GH-3027] - guests/solaris: More accurate Solaris >= 11, < 11 detection. [GH-2824] - provisioners/chef-solo: New config `synced_folder_type` replaces the `nfs` option. This can be used to set the synced folders the provisioner diff --git a/lib/vagrant/action/builtin/confirm.rb b/lib/vagrant/action/builtin/confirm.rb index 7fc6aabbf..210c9c88a 100644 --- a/lib/vagrant/action/builtin/confirm.rb +++ b/lib/vagrant/action/builtin/confirm.rb @@ -11,10 +11,11 @@ module Vagrant # @param [String] message The message to ask the user. # @param [Symbol] force_key The key that if present and true in # the environment hash will skip the confirmation question. - def initialize(app, env, message, force_key=nil) + def initialize(app, env, message, force_key=nil, **opts) @app = app @message = message @force_key = force_key + @allowed = opts[:allowed] end def call(env) @@ -24,8 +25,16 @@ module Vagrant # the result to "Y" choice = "Y" if @force_key && env[@force_key] - # If we haven't chosen yes, then ask the user via TTY - choice = env[:ui].ask(@message) if !choice + if !choice + while true + # If we haven't chosen yes, then ask the user via TTY + choice = env[:ui].ask(@message) + + # If we don't have an allowed set just exit + break if !@allowed + break if @allowed.include?(choice) + end + end # The result is only true if the user said "Y" env[:result] = choice && choice.upcase == "Y" diff --git a/lib/vagrant/action/builtin/destroy_confirm.rb b/lib/vagrant/action/builtin/destroy_confirm.rb index 2e76737c6..3a2272c3d 100644 --- a/lib/vagrant/action/builtin/destroy_confirm.rb +++ b/lib/vagrant/action/builtin/destroy_confirm.rb @@ -13,7 +13,7 @@ module Vagrant message = I18n.t("vagrant.commands.destroy.confirmation", :name => env[:machine].name) - super(app, env, message, force_key) + super(app, env, message, force_key, allowed: ["y", "n", "Y", "N"]) end end end diff --git a/test/unit/vagrant/action/builtin/confirm_test.rb b/test/unit/vagrant/action/builtin/confirm_test.rb index 7f689582f..9cd201042 100644 --- a/test/unit/vagrant/action/builtin/confirm_test.rb +++ b/test/unit/vagrant/action/builtin/confirm_test.rb @@ -33,4 +33,21 @@ describe Vagrant::Action::Builtin::Confirm do described_class.new(app, env, message).call(env) env[:result].should_not be end + + it "should ask multiple times if an allowed set is given and response isn't in that set" do + times = 0 + env[:ui].stub(:ask) do |arg| + expect(arg).to eql(message) + times += 1 + + if times <= 3 + "nope" + else + "y" + end + end + described_class.new(app, env, message, allowed: ["y", "N"]).call(env) + expect(env[:result]).to be_true + expect(times).to eq(4) + end end