Centralize CSV/style option parsing

.rubocop_todo.yml

@@ -122,6 +122,7 @@ Style/Documentation:
     - 'lib/comma/header_extractor.rb'
     - 'lib/comma/mongoid.rb'
     - 'lib/comma/object.rb'
+    - 'lib/comma/options.rb'
     - 'lib/comma/relation.rb'
 
 # Offense count: 2

lib/comma.rb

@@ -23,6 +23,7 @@ module Comma
 
 require 'comma/data_mapper_collection' if defined? DataMapper
 
+require 'comma/options'
 require 'comma/generator'
 require 'comma/array'
 require 'comma/object'
@@ -42,15 +43,6 @@ module Comma
 
       # Capture any CSV optional settings passed to comma or comma specific options
       csv_options = options.slice(*CSV_HANDLER::DEFAULT_OPTIONS.merge(Comma::DEFAULT_OPTIONS).keys)
-      csv_options = csv_options.each_with_object({}) do |(k, v), h|
-        # XXX: Convert string to boolean
-        h[k] = case k
-               when :write_headers
-                 (v != 'false') if v.is_a?(String)
-               else
-                 v
-               end
-      end
       data = obj.to_comma(csv_options)
       data = "\xEF\xBB\xBF#{data}" if with_bom
       disposition = "attachment; filename=\"#{filename}.#{extension}\""

lib/comma/generator.rb

@@ -4,14 +4,10 @@ module Comma
   class Generator
     def initialize(instance, style)
       @instance = instance
-      @style    = style
-      @options  = {}
-
-      return unless @style.is_a?(Hash)
-
-      @options                  = @style.clone
-      @style                    = @options.delete(:style) || Comma::DEFAULT_OPTIONS[:style]
-      @filename                 = @options.delete(:filename)
+      parsed = Comma::Options.parse(style)
+      @style = parsed[:style]
+      @filename = parsed[:filename]
+      @options = parsed[:csv]
     end
 
     def run(iterator_method)

lib/comma/options.rb

@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+module Comma
+  module Options
+    module_function
+
+    def parse(input)
+      return { style: input, filename: nil, csv: {} } unless input.is_a?(Hash)
+
+      csv = input.dup
+      style = csv.delete(:style) || Comma::DEFAULT_OPTIONS[:style]
+      filename = csv.delete(:filename)
+      csv[:write_headers] = csv[:write_headers] != 'false' if csv[:write_headers].is_a?(String)
+
+      { style: style, filename: filename, csv: csv }
+    end
+  end
+end

spec/comma/options_spec.rb

@@ -0,0 +1,63 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Comma::Options do # rubocop:disable Metrics/BlockLength
+  describe '.parse' do # rubocop:disable Metrics/BlockLength
+    it 'treats a bare symbol as the style, with no filename or csv options' do
+      expect(described_class.parse(:brief)).to eq(style: :brief, filename: nil, csv: {})
+    end
+
+    it 'passes a bare nil through unchanged rather than defaulting it' do
+      expect(described_class.parse(nil)).to eq(style: nil, filename: nil, csv: {})
+    end
+
+    it 'passes a bare false through unchanged rather than defaulting it' do
+      expect(described_class.parse(false)).to eq(style: false, filename: nil, csv: {})
+    end
+
+    it 'extracts :style from a hash, defaulting to :default when absent' do
+      result = described_class.parse(col_sep: ';')
+      expect(result[:style]).to eq(:default)
+      expect(result[:csv]).to eq(col_sep: ';')
+    end
+
+    it 'extracts an explicit :style from a hash' do
+      result = described_class.parse(style: :brief, col_sep: ';')
+      expect(result[:style]).to eq(:brief)
+      expect(result[:csv]).to eq(col_sep: ';')
+    end
+
+    it 'extracts :filename from a hash and excludes it from :csv' do
+      result = described_class.parse(filename: 'export.csv', col_sep: ';')
+      expect(result[:filename]).to eq('export.csv')
+      expect(result[:csv]).to eq(col_sep: ';')
+    end
+
+    it 'passes through a real boolean write_headers unchanged' do
+      result = described_class.parse(write_headers: false)
+      expect(result[:csv]).to eq(write_headers: false)
+    end
+
+    it 'coerces the string "false" for write_headers to boolean false' do
+      result = described_class.parse(write_headers: 'false')
+      expect(result[:csv]).to eq(write_headers: false)
+    end
+
+    it 'coerces any other string for write_headers to boolean true' do
+      result = described_class.parse(write_headers: 'true')
+      expect(result[:csv]).to eq(write_headers: true)
+    end
+
+    it 'does not mutate the input hash' do
+      input = { style: :brief, filename: 'f.csv', col_sep: ';' }
+      described_class.parse(input)
+      expect(input).to eq(style: :brief, filename: 'f.csv', col_sep: ';')
+    end
+
+    it 'does not raise when given a frozen hash' do
+      input = { style: :brief, col_sep: ';' }.freeze
+      expect { described_class.parse(input) }.not_to raise_error
+    end
+  end
+end