ngrunwald / ring-middleware-format

Ring middleware for parsing parameters and emitting responses in JSON or other formats
163 stars 49 forks source link

Remove icu4j dependency, add optional namespace with charset detection code #75

Closed Deraen closed 5 years ago

Deraen commented 5 years ago

Fixes #74

If :charset get-or-default-charset option is provided, r-m-f will work even without icu4j dependency.

Ping @jimpil

During testing these changes, I found out there there wasn't a single test testing charset detection, and that it looks like charset detection has been broken previously. available-charsets lists the charset names in lower-case while icu4j returns name upper-case, so detection has always returned nil...) Because of this, I'm now considering if we could just default to no-detection and no-icu and provide separate namespace with guess-charset function. I need to investigate how long the detection has been broken.

Deraen commented 5 years ago

Okay, charset detection works with 0.3.2 but was broken in 0.4.0 in this commit where available-charsets is changed to use lower-case names: https://github.com/ngrunwald/ring-middleware-format/commit/f15ec0f413d42fb7ba0abdb2117047a29912f207

The version was released 2014-08-12 which is 4.5 years ago... Okay, I now think we can safely default to no charset detection and it won't break any projects.

jimpil commented 5 years ago

Excuse my ignorance, but who is this PR waiting approval from? Can I help in any way? I'm just asking because I see another two PRs hanging there for more than 2 years. I hope this doesn't get stuck like those. Thanks in advance...

Kind regards

Deraen commented 5 years ago

Merged & released now.

jimpil commented 5 years ago

Awesome - thanks again :+1: