elixir-cldr / cldr

Elixir implementation of CLDR/ICU
Other
440 stars 33 forks source link

Misleading documentation for "gettext" option. #192

Closed eriknaslund closed 1 year ago

eriknaslund commented 1 year ago

Background

After reading https://hexdocs.pm/ex_cldr/readme.html I tried my best configuring ex_cldr and ex_cldr_plugs (the PutLocale plug). I spent an embarrassingly long time trying to figure out how to make Gettext and Cldr work together, since no matter what I did I couldn't manage to make the plug set the current locale for Gettext.

It would be over the top nice to have a full fledged tutorial on how to configure ex_cldr + gettext + phoenix, but the major stumbling block for me was this piece of the documentation:

:gettext: configures Cldr to use a Gettext module as an additional source of locales you want to configure. Since Gettext uses the Posix locale name format (locales with an '_' in them) and Cldr uses the Unicode format (a '-' as the subtag separator), Cldr will transliterate locale names from Gettext into the Cldr canonical form.

I had naively configured my Cldr backend like this (some non-essentials omitted):

use Cldr,
    default_locale: "en",
    locales: ["en", "sv"],
    providers: []

I was just getting [warning] Locale "sv" does not have a known Gettext locale. No Gettext locale has been set. I couldn't understand why the PutLocale plug refused to acknowledge that I had the "en" and "sv" locales for Gettext.

The trick was to add gettext: AppWeb.Gettext as an option to the use statement. Very simple in hindsight. However I got confused by the docs since they said "configures Cldr to use a Gettext module as an additional source of locales ". Since I had already defined my locales in the Cldr backend I assumed that would be enough.

Request

Do you think there would be a room for a rewording in the docs stating that you need to set the gettext option in case you want to make use of Gettext together with ex_cldr (via the PutLocale plug)?

Another option would be to change the warning message of the plug to say something like "make sure you have set the gettext option for your Cldr backend".

kipcole9 commented 1 year ago

Erik, thanks very much for the report and I am so sorry for the inconvenience. I will most certainly update the documentation today as you suggest. You can usually reach me in the elixir-lang slack if you hit a roadblock and need a faster response.

kipcole9 commented 1 year ago

@eKIK I have pushed a commit to improve that documentation section. Feedback most welcome. I'll publish a docs update when you're comfortable its clearer. I will review the error message in Cldr.PutLocale as you suggest (and will open an issue there).

kipcole9 commented 1 year ago

I have also pushed a commit to ex_cldr_plugs to improve the warnings when attempting to set a Gettext locale.

There are now two possible warnings:

kipcole9 commented 1 year ago

I've published ex_cldr_plugs version 1.2.1 with the following changelog entry:

Bug Fixes

Let me know if the updated README.md is clear enough so that no-one else needs to go through what you went through and I'll publish the docs and close the issue.

eriknaslund commented 1 year ago

These are some sweet changes @kipcole9! I'm especially loving the new PutLocale warning message, that's super clear to me! These changes will definitely address the issue I experienced.

And btw, no need to say sorry for the inconvenience. I got a good excuse to dive into reading some of the Cldr code when troubleshooting, and I found it very neat and easy to follow. Keep up the great work! :)

kipcole9 commented 1 year ago

Thanks for the kind words @eKIK. I'll push the docs update now. Don't hesitate to raise more issues - developer experience is something I work hard at (clearly not always successfully!!!).

kipcole9 commented 1 year ago

Docs updated: https://hexdocs.pm/ex_cldr/readme.html#backend-configuration-keys