search

twitter / twitter-cldr-rb

Ruby implementation of the ICU (International Components for Unicode) that uses the Common Locale Data Repository to format dates, plurals, and more.
Apache License 2.0
672 stars 93 forks source link

Respect locale when formatting currencies #194

Closed muirdm closed 8 years ago

muirdm commented 8 years ago

My goal was to make 1337.localize(:"en-AU").to_currency.to_s(:currency => "USD", :use_cldr_symbol => true) yield "USD1337.00", not "$1337.00" (Australia uses "$" for AUD, not USD). I had to do two things:

  1. Update CLDR database to include proper currency symbols (e.g. old version had "US$" instead of "USD" for en-AU's USD symbol).
  2. Tweak DataReader#format_number to pass number's locale to the formatter so the currency formatter could fetch the localized currency (was defaulting to "en").

I see other issues/PRs related to trying to update to CLDR v29, but I'm not clear on what exactly is blocking the upgrade in terms of breaking existing functionality. I updated to v29 at least for the "update" rake tasks that worked, and I blindly updated failing tests assuming they were just from improved entries in the CLDR database. I expect this PR can't be accepted for whatever reason, but I at least wanted to get the currency formatting fix out there.

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 96.988% when pulling 976d4b5766d0e81b6d3f64c22fe146e96504a686 on retailnext:import-new-currencies into d1b7d645ba465c076bcab3e63ae27435e348a896 on twitter:master.

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 96.988% when pulling 976d4b5766d0e81b6d3f64c22fe146e96504a686 on retailnext:import-new-currencies into d1b7d645ba465c076bcab3e63ae27435e348a896 on twitter:master.

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 96.988% when pulling 976d4b5766d0e81b6d3f64c22fe146e96504a686 on retailnext:import-new-currencies into d1b7d645ba465c076bcab3e63ae27435e348a896 on twitter:master.

camertron commented 8 years ago

Hey @muirrn, thanks for the pull request. As you noted, I probably won't be able to accept this PR, but I do have one of my own in-flight for updating to CLDR v29. In other words, currency symbols should be updated in the near future :)

muirdm commented 8 years ago

Cool, thanks. I'll open a separate PR for the localized currency formatting fix (#2 in the PR comment).