bogdan / datagrid

Gem to create tables grids with sortable columns and filters
MIT License
1.02k stars 116 forks source link

Added raw_csv_headers config option #196

Closed smoyte closed 8 years ago

smoyte commented 8 years ago

This PR adds a raw_csv_headers config option that when set to true, uses the raw column names instead of the translated ones. These seem better suited to CSV in some cases where the data are being input to a statistical program, etc.

bogdan commented 8 years ago

I see your point: I sometimes do CSV export that suppose to be consumed by other software... which is the case to have a computer readable headers instead of human readable...

But I don't like it as global setting. For example, some projects would want to have both - export using human headers and computer headers in certain parts or even develop it as an export option.

I don't see the right way of doing it now.... I'll think more and maybe will have an idea in several days.

smoyte commented 8 years ago

Could it be both? What if I added the ability to pass raw_csv_headers to to_csv (stripping that option out before they get passed onto the Ruby CSV library)?

I like the global setting simply because the project I'm using it for now is an admin panel and I know with pretty good confidence that all tables will need it. If that changes in one case I could use the per-call version to say raw_csv_headers: false.

Happy to make this happen if you agree.

On Wed, Sep 28, 2016 at 8:37 AM, Bogdan Gusiev notifications@github.com wrote:

I see your point: I sometimes do CSV export that suppose to be consumed by other software... which is the case to have a computer readable headers instead of human readable...

But I don't like it as global setting. For example, some projects would want to have both - export using human headers and computer headers in certain parts or even develop it as an export option.

I don't see the right way of doing it now.... I'll think more and maybe will have an idea in several days.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bogdan/datagrid/pull/196#issuecomment-250154354, or mute the thread https://github.com/notifications/unsubscribe-auth/AAq3cBCLrw9EiDUoLNRr4yTUofx-ik7Cks5qul-JgaJpZM4KIBH7 .

Tom Smyth

Worker-Owner, Sassafras Tech Collective Specializing in innovative, usable tech for social change sassafras.coop · @sassafrastech

bogdan commented 8 years ago

Are you fine with the following solution?

module CsvRawHeaders

  def to_csv(*args, **options)
    options[:headers] ||= columns.map(&:name)
    super(*args, options)
  end
end

Than you can include this module to every grid class that needs such export or to Datagrid itself:

Datagrid.send(:include, CsvRawHeaders)
smoyte commented 8 years ago

Absolutely! Much simpler than my solution! Thanks!

Should the module be namespace under Datagrid though, perhaps?

On Wed, Sep 28, 2016 at 9:52 AM, Bogdan Gusiev notifications@github.com wrote:

Are you fine with the following solution?

module CsvRawHeaders

def to_csv(_args, _options) options[:headers] ||= columns.map(&:name) super(args, options) endend

Than you can include this module to every grid class that needs such export or to Datagrid itself:

Datagrid.send(:include, CsvRawHeaders)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bogdan/datagrid/pull/196#issuecomment-250172690, or mute the thread https://github.com/notifications/unsubscribe-auth/AAq3cF5nSFClAOieeOsLdKgwwaWMaDIuks5qunE0gaJpZM4KIBH7 .

Tom Smyth

Worker-Owner, Sassafras Tech Collective Specializing in innovative, usable tech for social change sassafras.coop · @sassafrastech

bogdan commented 8 years ago

It is up to you to decide.

2016-09-28 17:17 GMT+03:00 Tom Smyth notifications@github.com:

Absolutely! Much simpler than my solution! Thanks!

Should the module be namespace under Datagrid though, perhaps?

On Wed, Sep 28, 2016 at 9:52 AM, Bogdan Gusiev notifications@github.com wrote:

Are you fine with the following solution?

module CsvRawHeaders

def to_csv(_args, _options) options[:headers] ||= columns.map(&:name) super(args, options) endend

Than you can include this module to every grid class that needs such export or to Datagrid itself:

Datagrid.send(:include, CsvRawHeaders)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bogdan/datagrid/pull/196#issuecomment-250172690, or mute the thread https://github.com/notifications/unsubscribe-auth/ AAq3cF5nSFClAOieeOsLdKgwwaWMaDIuks5qunE0gaJpZM4KIBH7

.

Tom Smyth

Worker-Owner, Sassafras Tech Collective Specializing in innovative, usable tech for social change sassafras.coop · @sassafrastech

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/bogdan/datagrid/pull/196#issuecomment-250179593, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHeRPK6o3SPkRg4pPkhxmuHQ_j9aIgIks5qunb1gaJpZM4KIBH7 .

Bogdan Gusiev. agresso@gmail.com

smoyte commented 8 years ago

Ok. So just to be clear, I will update my PR to reflect this change, right? That is your wish?

On Wed, Sep 28, 2016 at 10:33 AM, Bogdan Gusiev notifications@github.com wrote:

It is up to you to decide.

2016-09-28 17:17 GMT+03:00 Tom Smyth notifications@github.com:

Absolutely! Much simpler than my solution! Thanks!

Should the module be namespace under Datagrid though, perhaps?

On Wed, Sep 28, 2016 at 9:52 AM, Bogdan Gusiev <notifications@github.com

wrote:

Are you fine with the following solution?

module CsvRawHeaders

def to_csv(_args, _options) options[:headers] ||= columns.map(&:name) super(args, options) endend

Than you can include this module to every grid class that needs such export or to Datagrid itself:

Datagrid.send(:include, CsvRawHeaders)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bogdan/datagrid/pull/196#issuecomment-250172690, or mute the thread https://github.com/notifications/unsubscribe-auth/ AAq3cF5nSFClAOieeOsLdKgwwaWMaDIuks5qunE0gaJpZM4KIBH7

.

Tom Smyth

Worker-Owner, Sassafras Tech Collective Specializing in innovative, usable tech for social change sassafras.coop · @sassafrastech

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/bogdan/datagrid/pull/196#issuecomment-250179593, or mute the thread https://github.com/notifications/unsubscribe-auth/ AAHeRPK6o3SPkRg4pPkhxmuHQ_j9aIgIks5qunb1gaJpZM4KIBH7 .

Bogdan Gusiev. agresso@gmail.com

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bogdan/datagrid/pull/196#issuecomment-250184356, or mute the thread https://github.com/notifications/unsubscribe-auth/AAq3cMpmFDKGp8ja74v0hLZ9WgvgKkS-ks5qunqvgaJpZM4KIBH7 .

Tom Smyth

Worker-Owner, Sassafras Tech Collective Specializing in innovative, usable tech for social change sassafras.coop · @sassafrastech

bogdan commented 8 years ago

I don't wish to do it in gem at all... why? Because it is too rare request to add a config option. You should do it in your own project in some way. Either as I proposed or differently.

smoyte commented 8 years ago

Oh I see what you're saying now. Fair enough.

On Wed, Sep 28, 2016 at 11:19 AM, Bogdan Gusiev notifications@github.com wrote:

I don't wish to do it in gem at all... why? Because it is too rare request to add a config option. You should do it in your own project in some way. Either as I proposed or differently.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bogdan/datagrid/pull/196#issuecomment-250199332, or mute the thread https://github.com/notifications/unsubscribe-auth/AAq3cDfyyqYe6AUyzq8ikvh16V4mImS1ks5quoV7gaJpZM4KIBH7 .

Tom Smyth

Worker-Owner, Sassafras Tech Collective Specializing in innovative, usable tech for social change sassafras.coop · @sassafrastech