elixir-cldr / cldr

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

Inspect implementation of `%Cldr.LanguageTag{}` is unhelpful and doesn't align with the docs #234

Open nietaki opened 1 week ago

nietaki commented 1 week ago

I used ex_cldr before, which is why I wanted to use it to fetch some information for a number of locales.

When I add it to a project (a real one or a repro one), most of the functions I run return MyApp.Cldr.Locale.new!("en-US") for whatever reason.

$ iex -S mix
Erlang/OTP 25 [erts-13.2] [source] [64-bit] [smp:10:10] [ds:10:10:10] [async-threads:1] [jit]

Compiling 1 file (.ex)
Generating Repro.Cldr for 97 locales named [:af, :am, :ar, :as, :az, ...] with a default locale named :en
Compiling lib/repro/cldr.ex (it's taking more than 10s)
Interactive Elixir (1.14.4) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> Repro.Cldr.validate_locale "en"
{:ok, Repro.Cldr.Locale.new!("en-US")}
iex(2)> Repro.Cldr.default_locale()
Repro.Cldr.Locale.new!("en-US")
iex(3)> Repro.Cldr.Locale.new!("en-US")
Repro.Cldr.Locale.new!("en-US")
iex(4)> Repro.Cldr.Locale.new!(:fr)
Repro.Cldr.Locale.new!("fr-FR")
iex(5)> Enum.count(Repro.Cldr.known_locale_names())
96
iex(6)> Enum.filter(Repro.Cldr.known_locale_names(), fn ln -> String.contains?("#{ln}", "en") end)
[:en]
iex(7)> Repro.Cldr.Locale.new!(:en)
Repro.Cldr.Locale.new!("en-US")

I created a repo with the repro. I tried to keep the configuration minimal:

defmodule Repro.Cldr do
  use Cldr,
    otp_app: :repro,
    default_locale: "en",
    locales:
    ["ja", "br", "be", "hr", "da", "hu", "oc", "ms", "sn", "sk", "sd", "ka", "no", "pa", "yi", "ur",
 "ro", "fi", "el", "es", "hy", "sr", "pt", "kk", "ha", "kn", "eu", "haw", "af", "sa",
 "et", "ln", "bg", "si", "ml", "ne", "fa", "my", "fo", "th", "sq", "pl", "en", "ar", "ca", "bo",
 "cs", "mk", "tg", "it", "as", "tt", "sl", "lv", "gl", "nl", "ko", "cy", "sw", "ba", "sv", "mt",
 "te", "tr", "mg", "lt", "he", "id", "lo", "is", "fr", "am", "vi", "so", "la", "km", "su", "de",
 "bs", "lb", "az", "gu", "zh", "uz", "ta", "tk", "ps", "uk", "mn", "nn", "bn", "mi",
 "ru", "hi", "mr", "yo"],
    providers: []
end
import Config

config :ex_cldr,
  default_backend: Repro.Cldr,
  json_library: Jason

I re-read the installation/configuration docs a couple of times and fiddled with the config and I'm still getting the same results.

Any advice? I assume (hope) this isn't the expected behaviour.

Edit: I updated the issue title to be actionable.

kipcole9 commented 1 week ago

iex calls inspect/2 on the result of any function and that's what is displays in the terminal. As of a ex_cldr 2.38.0 on April 21st, the Inspect protocol implementation for Cldr.LanguageTag.t changed to output executable code when the locale has been resolved to a known/configured locale. So while you see MyApp.Cldr.Locale.new!("en-US") in the terminal, the result is actually a struct.

I made this change to be consistent with how Inspect implementations are evolving in the Elixir code base - more towards inspect producing executable code in more situations.

Does that clear up what you're seeing?

nietaki commented 1 week ago

It seems like an astounding anti-pattern for a number of reasons:

Cldr.Locale.new("en-ES", TestBackend.Cldr)
{:ok, %Cldr.LanguageTag{
  backend: TestBackend.Cldr,
  canonical_locale_name: "en-ES",
  # (lines cut)
  script: :Latn,
  territory: :ES,
  transform: %{},
  language_variants: []
}}

I hoped that worst case scenario this was a ham-handed way of ex_cldr to let me know I should be using the Myapp.Cldr module instead of the Cldr module, so kept on re-checking my config. I'd check the source of Myapp.Cldr to see what's going on, but I skipped that, because it's macro-generated.

I made this change to be consistent with how Inspect implementations are evolving in the Elixir code base - more towards inspect producing executable code in more situations.

Do they really? All of the examples I can find right now have the Inspect protocol implementation return all the relevant/usable information about the underlying term:

iex(7)> ~r/(foo|bar)/u
~r/(foo|bar)/u
iex(8)> spawn fn -> 1 end
#PID<0.110.0>
iex(9)> pid(0, 110, 0)
#PID<0.110.0>
iex(10)> MapSet.new([:foo, :bar, :bar])
MapSet.new([:foo, :bar])
iex(11)> MapSet.new([:foo, :bar])
MapSet.new([:foo, :bar])
iex(12)> URI.new!("https://example.com/en/?foo=bar#header")
%URI{
  scheme: "https",
  userinfo: nil,
  host: "example.com",
  port: 443,
  path: "/en/",
  query: "foo=bar",
  fragment: "header"
}
iex(13)> Repo.get!(PersonGroup, "b9a57dde-52b9-4e71-b300-98d6433b9a79")
%Foo.Models.PersonGroup{
  __meta__: #Ecto.Schema.Metadata<:loaded, "persons_groups">,
  id: "b9a57dde-52b9-4e71-b300-98d6433b9a79",
  person_id: "85d52529-49af-4662-b432-9eec82254adf",
  person: #Ecto.Association.NotLoaded<association :person is not loaded>,
  group_id: "9092bad9-f64a-4bd7-951d-6e38fbdbcbea",
  group: #Ecto.Association.NotLoaded<association :group is not loaded>,
  inserted_at: ~U[2024-03-14 16:57:47.121775Z]
}

Yes, I don't see the whole structure of MapSet or Ecto.Association.NotLoaded, but that's because it's very difficult for me to come up with a reson why I would care about them.

What you did, however, is the equivalent of this:

iex(12)> URI.new!("https://example.com/en/?foo=bar#header")
URI.new!("https://example.com/en/?foo=bar#header")

iex(13)> Repo.get!(PersonGroup, "b9a57dde-52b9-4e71-b300-98d6433b9a79")
Repo.get!(PersonGroup, "b9a57dde-52b9-4e71-b300-98d6433b9a79")

Which in my opinion is incredibly pointless and unhelpful.

Also, please don't get me wrong, it's not like I wasn't aware of the Inspect protocol - it's just that in my decade of working with Elixir I wouldn't come up with someone using it this way. It might sound like I'm annoyed by this situation, and that's only because it's true - this "feature" cost me over an hour of my life, debugging an issue that's not there.


So the answer does clear up what I'm seeing, but makes the intentions before the decision to make it this way even more bizarre.

kipcole9 commented 1 week ago

I have posted in Elixir Forum on this topic to see what community and Elixir core team views are on this.