-
-
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 serious errors in locale/is #3314
base: main
Are you sure you want to change the base?
Conversation
- In icelandic day and month names are not capitalized. - Remove "." from abbr. names to closer match the "en-US" form.
Also use non-capturing groups for better `.match` performance ...especially on the more complex (more lenient) RegExps
bb6470a
to
e313164
Compare
e313164
to
e278eea
Compare
e278eea
to
c8fc60f
Compare
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.
@maranomynet Just catching up here - as the original author of the v1 Icelandic locale, thank you for making these updates for v2 and beyond. I've just left a note about not passing {{count}}
to the formatDistance
singular params. IMO this would break the convention with other locales where the grammatical declension is hardcoded into the string. Otherwise, this looks good.
any: new RegExp( | ||
'^(?:' + | ||
[ | ||
/ja(?:n(?:\.|úar)?)?/.source, | ||
/f(?:eb(?:\.|rúar)?)?/.source, | ||
/m(?:r|ar[.s]?)/.source, | ||
/a(?:pr(?:\.|íl)?)?/.source, | ||
/m(?:í|aí\.?)/.source, | ||
/j(?:n|ún[.í]?)/.source, | ||
/j(?:l|úl[.í]?)/.source, | ||
/á(?:gú(?:\.|st)?)?/.source, | ||
/s(?:ept?(?:\.|ember)?)?/.source, | ||
/o(?:kt(?:\.|óber)?)?/.source, | ||
/n(?:óv(?:\.|ember)?)?/.source, | ||
/d(?:es(?:\.|ember)?)?/.source, | ||
].join('|') + | ||
')', | ||
'i' | ||
), |
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.
Thank you.
@derekblank I wouldn't mind fixing it also, but that will take a bit more work, as it involves adding declensions. I can create a separate PR for that fix, later, or if you don't mind waiting a bit, I'll add it to this one. |
I rabbit-holed into fixing the As you see the grammar is hairy enough already, so I'd prefer not adding "one"-form exceptions all over the place. What we have now is a grammatically coherent and correct localization for Icelandic — something I've been dying to see released for 8 months now. |
I'm nost sure why the CI fails to detect @derekblank is there something I need to change in the PR? |
5a885fa
to
48b41b7
Compare
See commit messages for more info.
I added a start of a test.ts file for the icelandic locale, focusing on the parsing patterns, which were badly broken.
In addition to fixing typos in the matching and parsing patterns, I also made the matchers way (!) more forgiving/lenient in terms of the "width" of the mathced pattern, to better adhere to the Robustness Principle.
Also: Added proper pluralizations and declensions in formatDistance strings.