discourse / mini_mime

minimal mime type library
MIT License
95 stars 34 forks source link

lookups for csv return text/comma-separated-values over text/csv #3

Closed Fryguy closed 7 years ago

Fryguy commented 7 years ago

I see it in the db, so it's working correctly, but I'm curious why text/comma-separated-values was chosen over text/csv. I'm trying to see if I can get capybara updated to use mini_mime but they have a test that's failing and expecting "Content-type: text/csv". I'm not sure it matters, and I can easily change the test on the Capybara side, but I dug in a little further and found a StackOverflow article stating that the RFC suggests to use text/csv: http://stackoverflow.com/questions/7076042/what-mime-type-should-i-use-for-csv . So, what's the "right" mime type, and if we need to change it, how does that affect users of the gem?

SamSaffron commented 7 years ago

I think we should probably fix this, upstream cause

MIME::Types.type_for("x.csv").first.content_type

=> "text/comma-seperated-values" 

The types type_for returns are sorted per:

https://github.com/mime-types/ruby-mime-types/blob/aa499d1ea849584c7e2e63518f10289e76c00ec6/lib/mime/types.rb#L137-L149

perhaps raise this on

https://github.com/mime-types/ruby-mime-types

cc @halostatue

Fryguy commented 7 years ago

@SamSaffron However, they also have an obsolete and registered attribute, where text/comma-separated-values is both obsolete:true and registered:false

MIME::Types.type_for("x.csv").map(&:to_h)
=> [
{"content-type"=>"text/comma-separated-values",
  "encoding"=>"8bit",
  "extensions"=>["csv"],
  "obsolete"=>true,
  "use-instead"=>"text/csv",
  "registered"=>false},
{"content-type"=>"text/csv",
  "friendly"=>{"en"=>"Comma-Separated Values"},
  "encoding"=>"8bit",
  "extensions"=>["csv"],
  "xrefs"=>{"rfc"=>["rfc4180"], "template"=>["text/csv"]},
  "registered"=>true}
]
SamSaffron commented 7 years ago

Looks like a super easy fix then, mind doing a PR to fix the rake task and tests? On Mon, 27 Mar 2017 at 4:45 pm, Jason Frey notifications@github.com wrote:

@SamSaffron https://github.com/SamSaffron However, they also have an obsolete and registered attribute, where text/comma-separated-values is both obsolete:true and registered:false

MIME::Types.type_for("x.csv").map(&:to_h) => [ {"content-type"=>"text/comma-separated-values", "encoding"=>"8bit", "extensions"=>["csv"], "obsolete"=>true, "use-instead"=>"text/csv", "registered"=>false}, {"content-type"=>"text/csv", "friendly"=>{"en"=>"Comma-Separated Values"}, "encoding"=>"8bit", "extensions"=>["csv"], "xrefs"=>{"rfc"=>["rfc4180"], "template"=>["text/csv"]}, "registered"=>true} ]

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/discourse/mini_mime/issues/3#issuecomment-289570294, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAUXccr6pHY8Vv8KOp7VxSCmtmIXyhXks5rqBdWgaJpZM4MqkfO .

SamSaffron commented 7 years ago

what I am having trouble understanding here is why there is both a "obsolete" and "registered" flag... what does it mean to have a mime type that is not registered and not obsolete?

SamSaffron commented 7 years ago

I guess I will just reply here :) access is weird

mda has no registered mime types, but has a non obsolete mime type, so we should prefer "application/x-msaccess" to "application/access" which is obsolete but higher priority

Fryguy commented 7 years ago

@SamSaffron I started to do this last night, but realized that A LOT of changes were going to be needed to the dbs, as there are a bunch of obsoleted values in there. I wrote a script that can regenerate the .dbs, and I should have that pushed up a little later if you want to review it (busy with day job at the moment).

Fryguy commented 7 years ago

mda has no registered mime types, but has a non obsolete mime type, so we should prefer "application/x-msaccess" to "application/access" which is obsolete but higher priority

interesting...So we should prefer the obsolete one over the non-obsolete one?

SamSaffron commented 7 years ago

I just committed a fix

Can you review it? https://github.com/discourse/mini_mime/commit/3cd66be22d5c68882f19a5e3e53a801ac0ae9114

Fryguy commented 7 years ago

Looks good...made a comment on how the Ruby can be cleaned up, but overall it's great. Thanks for fixing this!

Fryguy commented 7 years ago

Closed in 3cd66be