JSON-safe comma splitting for override options (--fact-override, --enc-override)

doc/advanced-override-facts.md

@@ -18,6 +18,8 @@ To override a fact in the "from" catalog:
 
 You may use as many of these arguments as you wish to adjust as many facts as you wish.
 
+These options support specifying multiple overrides as a comma-separated list. Commas inside JSON values (for example, arrays) will not be treated as separators. You may also repeat the option.
+
 ## Examples
 
 Simulate a change to its IP address in the "to" branch:

doc/optionsref.md

@@ -68,9 +68,9 @@ Usage: octocatalog-diff [command line options]
                                      References to validate
         --[no-]compare-file-text[=force]
                                      Compare text, not source location, of file resources
+        --[no-]storeconfigs          Enable integration with puppetdb for collected resources
         --storeconfigs-backend TERMINUS
                                      Set the terminus used for storeconfigs
-        --[no-]storeconfigs          Enable integration with puppetdb for collected resources
         --retry-failed-catalog N     Retry building a failed catalog N times
         --no-enc                     Disable ENC
         --enc PATH                   Path to ENC script, relative to checkout directory or absolute

lib/octocatalog-diff/cli/options.rb

@@ -178,6 +178,71 @@ def self.option_globally_or_per_branch_array(opts = {})
         end
       end
 
+      def self.split_override_list(input)
+        s = input.to_s
+        parts = []
+        buf = String.new
+        square_depth = 0
+        curly_depth = 0
+        in_quote = nil
+        escaped = false
+
+        s.each_char do |ch|
+          if escaped
+            buf << ch
+            escaped = false
+            next
+          end
+
+          if ch == '\\'
+            buf << ch
+            escaped = true
+            next
+          end
+
+          if in_quote
+            in_quote = nil if ch == in_quote
+            buf << ch
+            next
+          end
+
+          if ch == '"' || ch == "'"
+            in_quote = ch
+            buf << ch
+            next
+          end
+
+          case ch
+          when '['
+            square_depth += 1
+            buf << ch
+          when ']'
+            square_depth -= 1 if square_depth.positive?
+            buf << ch
+          when '{'
+            curly_depth += 1
+            buf << ch
+          when '}'
+            curly_depth -= 1 if curly_depth.positive?
+            buf << ch
+          when ','
+            if square_depth.zero? && curly_depth.zero?
+              part = buf.strip
+              parts << part unless part.empty?
+              buf = String.new
+            else
+              buf << ch
+            end
+          else
+            buf << ch
+          end
+        end
+
+        part = buf.strip
+        parts << part unless part.empty?
+        parts
+      end
+
       # See description of `option_globally_or_per_branch`. This implements the logic for a boolean value.
       # @param :parser [OptionParser object] The OptionParser argument
       # @param :options [Hash] Options hash being constructed; this is modified in this method.

lib/octocatalog-diff/cli/options/enc_override.rb

@@ -7,15 +7,21 @@
   has_weight 322
 
   def parse(parser, options)
-    # Set 'enc_override_in' because more processing is needed, once the command line options
-    # have been parsed, to make this into the final form 'enc_override'.
-    OctocatalogDiff::Cli::Options.option_globally_or_per_branch(
-      parser: parser,
-      options: options,
-      cli_name: 'enc-override',
-      option_name: 'enc_override_in',
-      desc: 'Override parameter from ENC',
-      datatype: []
-    )
+    parser.on('--enc-override STRING1[,STRING2[,...]]', 'Override parameter from ENC globally') do |x|
+      options[:to_enc_override_in] ||= []
+      options[:from_enc_override_in] ||= []
+      OctocatalogDiff::Cli::Options.split_override_list(x).each do |item|
+        options[:to_enc_override_in] << item
+        options[:from_enc_override_in] << item
+      end
+    end
+    parser.on('--to-enc-override STRING1[,STRING2[,...]]', 'Override parameter from ENC for the to branch') do |x|
+      options[:to_enc_override_in] ||= []
+      OctocatalogDiff::Cli::Options.split_override_list(x).each { |item| options[:to_enc_override_in] << item }
+    end
+    parser.on('--from-enc-override STRING1[,STRING2[,...]]', 'Override parameter from ENC for the from branch') do |x|
+      options[:from_enc_override_in] ||= []
+      OctocatalogDiff::Cli::Options.split_override_list(x).each { |item| options[:from_enc_override_in] << item }
+    end
   end
 end

lib/octocatalog-diff/cli/options/fact_override.rb

@@ -9,13 +9,22 @@
   def parse(parser, options)
     # Set 'fact_override_in' because more processing is needed, once the command line options
     # have been parsed, to make this into the final form 'fact_override'.
-    OctocatalogDiff::Cli::Options.option_globally_or_per_branch(
-      parser: parser,
-      options: options,
-      cli_name: 'fact-override',
-      option_name: 'fact_override_in',
-      desc: 'Override fact',
-      datatype: []
-    )
+    # Avoid Array parsing here so JSON values with commas stay intact.
+    parser.on('--fact-override STRING1[,STRING2[,...]]', 'Override fact globally') do |x|
+      options[:to_fact_override_in] ||= []
+      options[:from_fact_override_in] ||= []
+      OctocatalogDiff::Cli::Options.split_override_list(x).each do |item|
+        options[:to_fact_override_in] << item
+        options[:from_fact_override_in] << item
+      end
+    end
+    parser.on('--to-fact-override STRING1[,STRING2[,...]]', 'Override fact for the to branch') do |x|
+      options[:to_fact_override_in] ||= []
+      OctocatalogDiff::Cli::Options.split_override_list(x).each { |item| options[:to_fact_override_in] << item }
+    end
+    parser.on('--from-fact-override STRING1[,STRING2[,...]]', 'Override fact for the from branch') do |x|
+      options[:from_fact_override_in] ||= []
+      OctocatalogDiff::Cli::Options.split_override_list(x).each { |item| options[:from_fact_override_in] << item }
+    end
   end
 end

spec/octocatalog-diff/tests/cli/options/enc_override_spec.rb

@@ -11,5 +11,19 @@
       result = run_optparse(args)
       expect(result[:to_enc_override_in]).to eq(['foo=bar', 'baz=buzz'])
     end
+
+    it 'should accept a comma-separated list of overrides' do
+      args = ['--enc-override', 'foo=bar,baz=buzz']
+      result = run_optparse(args)
+      expect(result[:to_enc_override_in]).to eq(['foo=bar', 'baz=buzz'])
+      expect(result[:from_enc_override_in]).to eq(['foo=bar', 'baz=buzz'])
+    end
+
+    it 'should split comma-separated overrides but not split commas inside JSON' do
+      args = ['--enc-override', 'foo=bar,json_list=(json)["a","b"],baz=buzz']
+      result = run_optparse(args)
+      expect(result[:to_enc_override_in]).to eq(['foo=bar', 'json_list=(json)["a","b"]', 'baz=buzz'])
+      expect(result[:from_enc_override_in]).to eq(['foo=bar', 'json_list=(json)["a","b"]', 'baz=buzz'])
+    end
   end
 end

spec/octocatalog-diff/tests/cli/options/fact_override_spec.rb

@@ -11,5 +11,26 @@
       result = run_optparse(args)
       expect(result[:to_fact_override_in]).to eq(['foo=bar', 'baz=buzz'])
     end
+
+    it 'should not split JSON overrides containing commas' do
+      args = ['--fact-override', 'json_list=(json)["a","b","c"]']
+      result = run_optparse(args)
+      expect(result[:to_fact_override_in]).to eq(['json_list=(json)["a","b","c"]'])
+      expect(result[:from_fact_override_in]).to eq(['json_list=(json)["a","b","c"]'])
+    end
+
+    it 'should accept a comma-separated list of overrides' do
+      args = ['--fact-override', 'foo=bar,baz=buzz']
+      result = run_optparse(args)
+      expect(result[:to_fact_override_in]).to eq(['foo=bar', 'baz=buzz'])
+      expect(result[:from_fact_override_in]).to eq(['foo=bar', 'baz=buzz'])
+    end
+
+    it 'should split comma-separated overrides but not split commas inside JSON' do
+      args = ['--fact-override', 'foo=bar,json_list=(json)["a","b"],baz=buzz']
+      result = run_optparse(args)
+      expect(result[:to_fact_override_in]).to eq(['foo=bar', 'json_list=(json)["a","b"]', 'baz=buzz'])
+      expect(result[:from_fact_override_in]).to eq(['foo=bar', 'json_list=(json)["a","b"]', 'baz=buzz'])
+    end
   end
 end