From 8598a180bc5505aadaf616f2333fc60865fbd452 Mon Sep 17 00:00:00 2001
From: Skye Shaw <whatitdowhatitdontdowhatitizzz@yahoo.com>
Date: Wed, 2 Nov 2011 22:37:50 -0700
Subject: [PATCH] fixed quoting, more tests...

---
 README.md           |   8 ++--
 lib/optout.rb       |  71 +++++++++++++++++-------------
 spec/optout_spec.rb | 102 +++++++++++++++++++++++---------------------
 3 files changed, 99 insertions(+), 82 deletions(-)

diff --git a/README.md b/README.md
index 7102b4f..5a77ddf 100644
--- a/README.md
+++ b/README.md
@@ -21,15 +21,15 @@ Validate an option hash and turn it into something suitable to pass to `exec()`
       :user => true
     }
 
-    `gem #{optout.shell(options)}`
-    # Returns: "install rake --platform mswin -v 0.9.2 --user-install"
-
     exec "gem", *optout.argv(options)
     # Returns: ["install", "rake", "--platform", "mswin", "-v", "0.9.2", "--user-install"]
 
+    `gem #{optout.shell(options)}`
+    # Returns: "'install' 'rake' --platform 'mswin' -v '0.9.2' --user-install"
+
 ## Install
 
-Sorry, no gemspec yet... in process of extracting from a larger project.
+Sorry, no gem yet... in process of extracting (and enhancing) from a larger project.
 
 ## Author
 
diff --git a/lib/optout.rb b/lib/optout.rb
index c64755b..8f7ad78 100644
--- a/lib/optout.rb
+++ b/lib/optout.rb
@@ -1,6 +1,11 @@
 require "rbconfig"
 require "pathname"
 
+
+require "pp"
+
+
+
 class Optout
   VERSION = "0.01"
 
@@ -66,7 +71,6 @@ class << self
     #    # Creates: ["--prefix=/sshaw/lib", "libssl2"]
     #
 
-    # block req
     def options(config = {}, &block)
       optout = new(config)
       optout.instance_eval(&block) if block_given?
@@ -74,15 +78,6 @@ def options(config = {}, &block)
     end
 
     alias :keys :options
-
-    def const_missing(name)
-      if name == "File" || name == "Dir"
-        p "Missing #{name}"
-        const_get("Validator::#{name}").new
-      else
-        super
-      end
-    end
   end
 
   def initialize(args = {})
@@ -185,8 +180,6 @@ class Option
     attr :value
     attr :index
 
-    QUOTE = RbConfig::CONFIG["host_os"] =~ /mswin|mingw/i ? "\"" : "'"
-
     def self.create(key, *args)
       options = Hash === args.last ? args.pop : {}
       switch  = args.shift
@@ -198,18 +191,12 @@ def self.create(key, *args)
           @value  = v.shift || options[:default]
           @joinon = String === options[:multiple] ? options[:multiple] : ","
           @index  = options[:index].to_i
-
-          # Check for: "--[without]-feature-x"
-
-          # If a switch ends with "=" we require an arg and make sure
-          # that there's no space between the switch and its arg.
-          eq_switch  = !!(@switch =~ /\w=\z/)
-          @separator = eq_switch ? "" : options[:arg_separator] || " "
-          required   = options.include?(:required) ? options[:required] : eq_switch
+          @separator = options[:arg_separator] || " "
 
           @validators = []
-          @validators << Validator::Required.new(required)
+          @validators << Validator::Required.new(options[:required])
           @validators << Validator::Multiple.new(options[:multiple])
+
           # Could be an Array..?
           @validators << Validator.for(options[:validator]) if options[:validator]
         end
@@ -217,21 +204,25 @@ def self.create(key, *args)
     end
 
     def to_s
-      opt = to_a
-      opt[1] = quote(opt[1]) if opt[1] =~ /\s/
+      opt = create_opt_array
+      if opt.any?
+        if opt.size == 1
+          opt[0] = quote(opt[0]) unless @switch
+        else
+          opt[1] = quote(opt[1])
+        end
+      end
       opt.join(@separator)
     end
 
     def to_a
-      opt = []
-      # if eq_switch or arg_sep is blank we want a 1 element Array -right?
-      opt << @switch if @switch && @value
-      opt << normalize(@value) if @value && @value != true       # Only include @value for non-boolean options
+      opt = create_opt_array
+      opt = [ opt.join(@separator) ] if blank_separator?
       opt
     end
 
     def empty?
-      @value.to_s.empty?
+      !@value || @value.to_s.empty?
     end
 
     def validate!
@@ -239,10 +230,30 @@ def validate!
     end
 
     private
+    def create_opt_array
+      opt = []
+      opt << @switch if @switch && @value
+      opt << normalize(@value) if !empty? && @value != true       # Only include @value for non-boolean options
+      opt
+    end
+
+    def blank_separator?
+      @separator.gsub(/\s+/, "").empty?
+    end
+
     # bob's     = bob\'s
     # bob's big = 'bob'\''s big'
     def quote(value)
-      sprintf "%s%s%s", QUOTE, value.gsub(QUOTE) { "\\#{QUOTE}" }, QUOTE
+      if unix?
+        # For --opt=n we dont always want to quote!
+        sprintf "'%s'", value.gsub("'") { "'\\''" }
+      else
+        %|"#{value}"|
+      end
+    end
+
+    def unix?
+      RbConfig::CONFIG["host_os"] !~ /mswin|mingw/i
     end
 
     def normalize(value)
@@ -257,7 +268,7 @@ def self.for(setting)
       else
         # Load based on setting's name or the name of its class
         validator = setting.class.name
-        if validator == "Class" &&
+        if validator == "Class"
           name = setting.name.split("::", 2)
           validator = name[1] if name[1] && name[0] == "Optout"
         end
diff --git a/spec/optout_spec.rb b/spec/optout_spec.rb
index f2e6441..3fee424 100644
--- a/spec/optout_spec.rb
+++ b/spec/optout_spec.rb
@@ -2,8 +2,13 @@
 require "tempfile"
 require "fileutils"
 
-def options_class
-  Optout::Option.create(:x, "-x")
+def option_class(*args)
+  options = Hash === args.last ? args.pop : {}
+  switch = args.shift
+  klass = Optout::Option.create(:x, switch, options)
+  # Need mocha to stub this?
+  klass.class_eval { def unix?; true; end }
+  klass
 end
 
 describe Optout do
@@ -68,83 +73,84 @@ def options_class
     end
   end
 
-  context "creating the option string" do
+  context "creating options as a String" do
     it "should only output the switch if its value is true" do
       klass = Optout::Option.create(:x, "-x")
       opt = klass.new
-      opt.to_s.should == ""
+      opt.to_s.should eql("")
       opt = klass.new(false)
-      opt.to_s.should == ""
+      opt.to_s.should eql("")
       #opt = @klass.new(99)
       #opt.to_s.should == ""
       opt = klass.new(true)
-      opt.to_s.should == "-x"
+      opt.to_s.should eql("-x")
     end
 
     it "should only output the switch with its argument if the value is not a boolean" do
-      klass = Optout::Option.create(:x, "-x")
-      klass.new(123).to_s.should == "-x 123"
+      klass = option_class("-x")
+      klass.new(123).to_s.should eql("-x '123'")
     end
 
     it "should only output the value if there's no switch" do
-      klass = Optout::Option.create(:x)
-      klass.new("123").to_s.should == "123"
+      klass = option_class
+      klass.new("123").to_s.should == "'123'"
     end
 
     it "should use the default if no value is given" do
-      klass = Optout::Option.create(:x, "-x", :default => 69)
-      klass.new.to_s.should == "-x 69"
-      klass.new(123).to_s.should == "-x 123"
+      klass = option_class("-x", :default => 69)
+      klass.new.to_s.should eql("-x '69'")
+      klass.new(123).to_s.should eql("-x '123'")
     end
 
     it "should separate the option from its value with an alternate character" do
-      klass = Optout::Option.create(:x, "-x", :arg_separator => ":")
-      klass.new(123).to_s.should eql("-x:123")
+      klass = option_class("-x", :arg_separator => ":")
+      klass.new(123).to_s.should eql("-x:'123'")
     end
 
-    it "should concatinate the value if it's an array" do
-      klass = Optout::Option.create(:x, "-x")
-      klass.new(%w|A B C|).to_s.should eql("-x A,B,C")
-      klass = Optout::Option.create(:x, "-x", :multiple => ":")
-      klass.new(%w|A B C|).to_s.should eql("-x A:B:C")
+    it "should concatenate the value if it's an array" do
+      klass = option_class("-x")
+      klass.new(%w|A B C|).to_s.should eql("-x 'A,B,C'")
+      klass = option_class("-x", :multiple => ":")
+      klass.new(%w|A B C|).to_s.should eql("-x 'A:B:C'")
     end
 
-    it "should quote a value with spaces that aren't leading/trailing" do
-      klass = Optout::Option.create(:x, "-x")
-      klass.new(" a ").to_s.should eql("-x a")
-      klass.new("a b c").to_s.should eql("-x 'a b c'")
+    context "on a machine running a Unix based OS" do
+      it "should escape single quotes in a value" do
+        klass = option_class("-x")
+        klass.new("' a'b'c '").to_s.should == %q|-x ''\'' a'\''b'\''c '\'''|
+      end
+
+      it "should always use single quotes to quote the value" do
+        klass = option_class("-x")
+        klass.new(" a ").to_s.should eql("-x 'a'")
+        klass.new("a b c").to_s.should eql("-x 'a b c'")
+      end
     end
 
-    # This is not correct.. see bash
-    it "should escape quotes in a value" do
-      klass = Optout::Option.create(:x, "-x")
-      klass.new("'a'b'c'").to_s.should == %|-x \'a\'b\'c\'|
+    context "on a machine running Windows" do
+      it "should always use double quotes to quote the value" do
+        klass = option_class("-x")
+        klass.class_eval {  def unix?; false; end }
+        klass.new("a b c").to_s.should eql('-x "a b c"')
+      end
+
     end
   end
 
   context "validating the option" do
     it "should raise an OptionRequired error if there's no value and one's required" do
-      klass = Optout::Option.create(:x)
+      klass = option_class
       proc { klass.new.validate! }.should_not raise_exception
-      klass = Optout::Option.create(:x, :required => true)
-      proc { klass.new(123).validate! }.should_not raise_exception
-      proc { klass.new.validate! }.should raise_exception(Optout::OptionRequired)
-    end
-
-    it "should raise an OptionRequired error if there's no value and the switch ends with a '='" do
-      klass = Optout::Option.create(:x, "--x=")
+      klass = option_class(:required => true)
       proc { klass.new(123).validate! }.should_not raise_exception
       proc { klass.new.validate! }.should raise_exception(Optout::OptionRequired)
-      # Separate spec for this?
-      klass = Optout::Option.create(:x, "--x=", :required => false)
-      proc { klass.new.validate! }.should_not raise_exception
     end
 
     # need to check multiple's default value
     it "should raise an OptionInvalid error if multiple values are given when they're not allowed" do
-      klass = Optout::Option.create(:x)
+      klass = option_class
       proc { klass.new.validate! }.should_not raise_exception
-      klass = Optout::Option.create(:x, :multiple => false)
+      klass = option_class(:multiple => false)
       proc { klass.new(123).validate! }.should_not raise_exception
       proc { klass.new(%w|A B|).validate! }.should raise_exception(Optout::OptionInvalid)
     end
@@ -155,7 +161,7 @@ def validate!(opt)
           raise "raise up!"
         end
       end
-      klass = Optout::Option.create(:x, :validator => v.new)
+      klass = option_class(:validator => v.new)
       proc { klass.new.validate! }.should raise_exception(RuntimeError, "raise up!")
     end
   end
@@ -327,24 +333,24 @@ def validate!(opt)
 
 describe Optout::Validator::Class do
   it "should raise an exception if the value is not of the specified Class" do
-    opt = options_class.new("x")
+    opt = option_class.new("x")
     v = Optout::Validator::Class.new(Float)
     proc { v.validate!(opt) }.should raise_exception(Optout::OptionInvalid)
-    opt = options_class.new(1.12)
+    opt = option_class.new(1.12)
     proc { v.validate!(opt) }.should_not raise_exception
   end
 end
 
 describe Optout::Validator::Boolean do
   it "should raise an exception if the value is not boolean or nil" do
-    opt = options_class.new("x")
+    opt = option_class.new("x")
     v = Optout::Validator::Boolean.new
     proc { v.validate!(opt) }.should raise_exception(Optout::OptionInvalid)
-    opt = options_class.new(nil)
+    opt = option_class.new(nil)
     proc { v.validate!(opt) }.should_not raise_exception
-    opt = options_class.new(true)
+    opt = option_class.new(true)
     proc { v.validate!(opt) }.should_not raise_exception
-    opt = options_class.new(false)
+    opt = option_class.new(false)
   end
 end