-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: updates for Basque (eu) #3749
base: main
Are you sure you want to change the base?
Conversation
As a reference you can take a look at moment.js, which was updated years ago by me: https://github.com/moment/moment/blob/develop/locale/eu.js For example, "en {result}" was still in Spanish...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't speak the language, but the changes look good to me!
@fturmel snapshot looks good. There is only one thing that it is not perfect, but I don't think it is possible with the current implementation:
But suffix is added after result is calculated, and the 'about' (gutxi gorabehera) is added beforehand to the result, so I guess it needs to stay like this. Thanks! |
Keep this on hold for a moment... I only checked |
Great! Here's the unicode CLDR reference if needed: https://www.unicode.org/cldr/charts/44/summary/eu.html If you change anything else, you can update the snapshot markdown file yourself by running I'm sure it's possible to reshape the formatDistance function to support the "about" form you're describing. How does this look? if (options?.addSuffix) {
if (options.comparison && options.comparison > 0) {
if (token.startsWith("about")) {
return result.replace("gutxi", "barru gutxi");
}
return result + " barru";
} else {
return "duela " + result;
}
} |
@fturmel Thank you for your patience. I finally found a moment to go through all files yesterday. I added several small changes to conform to the official Basque rules. Snapshot has been updated too and looks OK. |
formatDistance
for Basque (eu)a9cfd31
to
2e12e33
Compare
This fixes things like repeated years in long format and required dots in abbreviations.
medium: "y MMM d", | ||
short: "yy/MM/dd", | ||
short: "y-MM-dd", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CLDR shows both y/MM/dd
and y-MM-dd
:
y/MM/dd
: https://www.unicode.org/cldr/charts/44/summary/eu.html#793cd78298fbed73y-MM-dd
: https://www.unicode.org/cldr/charts/44/summary/eu.html#439e81cebcf40890
and both are valid in Basque: https://www.euskaltzaindia.eus/dok/arauak/Araua_0037.pdf
but the dashed version is more common.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://www.unicode.org/cldr/charts/44/summary/eu.html#57dac0d1b36c1261 is probably the CLDR field to look at for this value (gregorian date format short yy/M/d
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair. It's hard for a non-speaker to take a stance when things deviate from the CLDR. Maybe @date-fns/I18n-eu can chime in (and @eillarra should probably be added to this team).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks! Up to @kossnocorp to approve and merge, provided there are no additional comments from the i18n-eu
team.
As a reference you can take a look at moment.js, which was updated years ago by me: https://github.com/moment/moment/blob/develop/locale/eu.js
For example, "en {result}" was still in Spanish...