Skip to content

Centralize CSV/style option parsing #163

Description

@eitoball

Summary

Centralize parsing of to_comma style vs. CSV options so Generator and the Rails render csv: renderer share one code path.

Problem

Callers can pass either a style symbol or a hash of options:

users.to_comma(:short)
users.to_comma(style: :short, col_sep: ';', write_headers: false)
users.to_comma(filename: 'export.csv')

Today this is handled in two places with slightly different logic:

  1. Comma::Generator#initialize (lib/comma/generator.rb) — treats style as either a Symbol or Hash, pulls out :style, :filename, passes remainder to Ruby's CSV library

  2. Rails renderer (lib/comma.rb) — slices options from render kwargs, includes a string-to-boolean hack for write_headers:

    # XXX: Convert string to boolean
    h[k] = case k
           when :write_headers
             (v != 'false') if v.is_a?(String)
           else
             v
           end

This duplication is easy to drift (e.g. new Comma-specific option added in one place only).

Proposed approach

Add Comma::Options (or similar) with a single entry point:

Comma::Options.parse(input)
# => { style: :default, filename: nil, csv: { col_sep: ',', write_headers: true, ... } }

Use it from:

  • Comma::Generator#initialize
  • ActionController :csv renderer registration in lib/comma.rb

Document supported keys in one module (align with Comma::DEFAULT_OPTIONS in lib/comma.rb).

Files likely involved

  • lib/comma/generator.rb
  • lib/comma.rb
  • New: lib/comma/options.rb (name TBD)
  • spec/comma/comma_spec.rb (options / filename / write_headers examples)
  • spec/controllers/users_controller_spec.rb (renderer option passthrough)

Acceptance criteria

  • Comma::Options.parse (or equivalent) handles symbol, hash, and default cases
  • String "false" for write_headers still works in controller specs
  • filename, col_sep, force_quotes, style combinations unchanged (existing specs pass)
  • Generator and renderer both delegate to the shared parser
  • No public API change

Labels (suggested)

refactor

Depends on

None strictly; pairs well with #4 (Rails extraction).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions