Closed Geekfish closed 7 years ago
Had a quick look, looks good :+1:, going to have allocate a few hours to go over it carefully, probably wont get to itbefore end of next week at moment (at least 10 days).
I switched to elm-format myself a while back so I'm surprised indenting changed on some files, but its fine.
No worries at all, thank you for your time! All feedback would be greatly appreciated, I'm fairly new to Elm-world so might have taken a less than optimal approach π€ Regarding elm-format, I think all the files that were reformatted were locale contributions, so maybe they came from people who didn't use it.
Nice work, the only thing you left out was exposing the greek config and i18n file, which I have to admit I've forgotten a few times in the past myself.
Your generalisation of the AM/PM stuff was not a surprise in how it was done - high praise in my book. Thanks for the input.
Good spot, thanks again!
Hi!
This PR is to adding i18n support for am/pm π as well as support for Greek π¬π·
I wanted to just add a Greek configuration to this library, but I very soon realised that the short time format in Greek is using a 12-hour clock format (aka AM/PM). Even though the library currently supports AM/PM as strings, it's only kind of valid for languages that use the latin format.
The rationale behind some of the changes:
I added a separate file for 12-hour clock related stuff, which I tried to avoid by initially adding everything to
Date.Extra.TimeUnit
or other logically compatible modules instead but in many cases I ran into circular import problems (because of calls toDate.Extra.Format
and some defaults coming explicitly from the English language).One of the things I added was the idea of "default" formats, (
I_default
). If you like this idea then I think it could be a good base for subsequent work, as some of the English settings could in theory move there, so that explicit calls to English fromFormat
are no longer needed. But if you really don't like it I could removeI_default
and add a function in every single language file that returns "AM" and "PM".elm-format
(which I run by default) caused some indentation fixes in some of the files I touched. Again let me know if this is ok.As a subsequent test to the AM/PM I also added the Russian π·πΊ abbreviations (though as far as I know they are not very widely used). Some other uses of this feature would be for languages that might use "AM" and "PM" as well, but maybe in a slightly different default format (ex. by requiring dots like "5 a.m.", which I believe is standard for Spanish).
PS. Another thing that I uncovered was that certain languages, like Greek and Russian, might require months with different suffixes/formats. I have added a TODO in the Greek config, but I think solving that issue might be more complex, as the formatting has to become context aware (e.g. is the month name next to a day?). You can see those types in the ICU references below under month names "Formatting" and "Standalone" subsections. Another issue for another PR...
Refs: ICU Greek ICU Russian ICU Spanish