elixir-cldr / cldr_dates_times

Date & times formatting functions for the Common Locale Data Repository (CLDR) package https://github.com/elixir-cldr/cldr
Other
68 stars 13 forks source link

Problems on transliterate, breaking formats #9

Closed ribanez7 closed 5 years ago

ribanez7 commented 5 years ago

Hello, we have detected that when Cldr.Date.to_string() uses the format: :short, and locale: "de", the format starts correctly (dd.MM.yy), until the pipeline reaches the Cldr.Number.Transliterate.transliterate()

At that step, all the dots are changed by commas. So this issue is affecting the formats specified. That also happens if you directly pass a format string with dots: it always switches dots by commas.

The bug is located here, on the transliterate call:

        for format <- Cldr.DateTime.Format.format_list(config) do
          case Compiler.compile(format, backend, Cldr.DateTime.Formatter.Backend) do
            {:ok, transforms} ->
              def format(date, unquote(Macro.escape(format)) = f, locale, options) do
                number_system = if is_map(f), do: f[:number_system], else: options[:number_system]
                formatted = unquote(transforms)

                if error_list = format_errors(formatted) do
                  {:error, Enum.join(error_list, "; ")}
                else
                  formatted =
                    formatted
                    |> Enum.join()
                    |> transliterate(locale, number_system)

                  {:ok, formatted}
                end
              end

            {:error, message} ->
              raise Cldr.FormatCompileError,
                    "#{message} compiling date format: #{inspect(format)}"
          end
        end

Let me know if you need more information for this. I don't know if the transliteration is needed at all, that may be the issue?

Thanks!

kipcole9 commented 5 years ago

Really sorry for the inconvenience. And thanks for taking the time to help my diagnose the issue. I'll work on this now.

The transliterate/3 call is required for two reasons: (a) when the date (or time, date time) format in a given locale specifies a number system and (b) the option :number_system is passed to to_string/3

However clearly what you are seeing is a bug.

kipcole9 commented 5 years ago

Updated to version 2.0.2 which, I believe, fixes this error. Thanks for the report.

mix deps.update ex_cldr_dates_times and you should be good to go.

ribanez7 commented 5 years ago

Hi @kipcole9 , unfortunately the test on your commit is wrong, the correct format in german should be with dots, not with commas. So still an issue. Thanks for the quick response :)

kipcole9 commented 5 years ago

Arrrghhhhh,. I shouldn't do this so late at night. I have re-pushed the same version and fixed the test case.

I need to push a new version of ex_cldr_numbers to remove the warning (and ensure no performance degradation). That will happen in the next 15 minutes.

kipcole9 commented 5 years ago

OK, this time I believe the behaviour is correct. Please:

mix deps.clean ex_cldr_dates_times
mix deps.update ex_cldr_numbers ex_cldr_dates_times
ribanez7 commented 5 years ago

Many thanks! I see it fixed now :)