Merge pull request #4 from jkeiser-oc/master

Fix --no-option boolean form and required options

2 files changed, 36 insertions, 16 deletions

diff --git a/lib/mixlib/cli.rb b/lib/mixlib/cli.rb
index 31eb3d9..39edd7f 100644
--- a/lib/mixlib/cli.rb
+++ b/lib/mixlib/cli.rb
@@ -138,19 +138,11 @@ module Mixlib
               raise ArgumentError, "You must pass :on, :tail, or :head to :on"
             end
                       
-          parse_block = case opt_val[:boolean]
-            when true
-              Proc.new() do
-                config[opt_key] = (opt_val[:proc] && opt_val[:proc].call(true)) || true
-                puts opts if opt_val[:show_options]
-                exit opt_val[:exit] if opt_val[:exit]
-              end
-            when false
-              Proc.new() do |c|
-                config[opt_key] = (opt_val[:proc] && opt_val[:proc].call(c)) || c
-                puts opts if opt_val[:show_options]
-                exit opt_val[:exit] if opt_val[:exit]
-              end
+          parse_block =
+            Proc.new() do |c|
+              config[opt_key] = (opt_val[:proc] && opt_val[:proc].call(c)) || c
+              puts opts if opt_val[:show_options]
+              exit opt_val[:exit] if opt_val[:exit]
             end
                     
           full_opt = [ opt_method ]
@@ -163,7 +155,7 @@ module Mixlib
       
       # Deal with any required values
       options.each do |opt_key, opt_value|
-        if opt_value[:required]
+        if opt_value[:required] && !config.has_key?(opt_key)
           reqarg = opt_value[:short] || opt_value[:long]
           puts "You must supply #{reqarg}!"
           puts @opt_parser
diff --git a/spec/mixlib/cli_spec.rb b/spec/mixlib/cli_spec.rb
index 05c6491..9eed069 100644
--- a/spec/mixlib/cli_spec.rb
+++ b/spec/mixlib/cli_spec.rb
@@ -156,18 +156,46 @@ describe Mixlib::CLI do
         @cli.config[:i_am_boolean].should == true
       end
       
+      it "should set the corresponding config value to false when a boolean is prefixed with --no" do
+        TestCLI.option(:i_am_boolean, :long => "--[no-]bool", :boolean => true)
+        @cli = TestCLI.new
+        @cli.parse_options([ '--no-bool' ])
+        @cli.config[:i_am_boolean].should == false
+      end
+
       it "should exit if a config option has :exit set" do
         TestCLI.option(:i_am_exit, :short => "-x", :boolean => true, :exit => 0)
         @cli = TestCLI.new
         lambda { @cli.parse_options(["-x"]) }.should raise_error(SystemExit)
       end
-      
+
       it "should exit if a required option is missing" do
         TestCLI.option(:require_me, :short => "-r", :boolean => true, :required => true)
         @cli = TestCLI.new
         lambda { @cli.parse_options([]) }.should raise_error(SystemExit)
       end
-      
+
+      it "should not exit if a required option is specified" do
+        TestCLI.option(:require_me, :short => "-r", :boolean => true, :required => true)
+        @cli = TestCLI.new
+        @cli.parse_options(["-r"])
+        @cli.config[:require_me].should == true
+      end
+
+      it "should not exit if a required boolean option is specified and false" do
+        TestCLI.option(:require_me, :long => "--[no-]req", :boolean => true, :required => true)
+        @cli = TestCLI.new
+        @cli.parse_options(["--no-req"])
+        @cli.config[:require_me].should == false
+      end
+
+      it "should not exit if a required option is specified and empty" do
+        TestCLI.option(:require_me, :short => "-r VALUE", :required => true)
+        @cli = TestCLI.new
+        @cli.parse_options(["-r", ""])
+        @cli.config[:require_me].should == ""
+      end
+
       it "should preserve all of the commandline arguments, ARGV" do
         TestCLI.option(:config_file, :short => "-c CONFIG")
         @cli = TestCLI.new