-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
feat: Add fr-CA locale #1511
feat: Add fr-CA locale #1511
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1511 +/- ##
=======================================
Coverage 99.92% 99.92%
=======================================
Files 96 96
Lines 1277 1278 +1
=======================================
+ Hits 1276 1277 +1
Misses 1 1
Continue to review full report at Codecov.
|
Related: #15101
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 for your contrib! 🎉
Please fix the merge conflicts. /cc. @ezkemboi -- kindly review. |
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 for your contrib! Can you check my comments concerning the decimal separator and the regex
@@ -106,7 +108,7 @@ for (let locale, i = 0; i < farsiLocales.length; i++) { | |||
} | |||
|
|||
// Source: https://en.wikipedia.org/wiki/Decimal_mark | |||
export const dotDecimal = ['ar-EG', 'ar-LB', 'ar-LY']; | |||
export const dotDecimal = ['ar-EG', 'ar-LB', 'ar-LY', 'fr-CA']; |
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.
According to many sources (even the link in the code comment) francophone area of Canada (fr-CA) are using decimal comma separator. Can you check this?
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.
Your're right I made a mistake. I'm in Quebec (canada-french) and most people i work with hate decimals for numbers. I was sure we had the same standard as English.
So yes, in french this is the decimal
and not the point
. Sorry
@@ -9,6 +9,7 @@ export const alpha = { | |||
'es-ES': /^[A-ZÁÉÍÑÓÚÜ]+$/i, | |||
'fa-IR': /^[ابپتثجچحخدذرزژسشصضطظعغفقکگلمنوهی]+$/i, | |||
'fr-FR': /^[A-ZÀÂÆÇÉÈÊËÏÎÔŒÙÛÜŸ]+$/i, | |||
'fr-CA': /^[A-ZÀÂÆÇÉÈÊËÏÎÔŒÙÛÜŸ]+$/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.
Since it's the same regex for fr-FR
and fr-CA
may be we should just assign the value like it was done for pt-BR
and all arabic locales
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.
Of course.
@@ -42,6 +43,7 @@ export const alphanumeric = { | |||
'el-GR': /^[0-9Α-ω]+$/i, | |||
'es-ES': /^[0-9A-ZÁÉÍÑÓÚÜ]+$/i, | |||
'fr-FR': /^[0-9A-ZÀÂÆÇÉÈÊËÏÎÔŒÙÛÜŸ]+$/i, | |||
'fr-CA': /^[0-9A-ZÀÂÆÇÉÈÊËÏÎÔŒÙÛÜŸ]+$/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.
Same here, doing alphanumeric['fr-CA'] = alphanumeric['fr-FR']
would be easier/better since it's the same regex
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.
Of course²
However, I am on preventive leave due to COVID cases in my family, so I will not be able to correct this in a few days. Can you make the changes? Also, thanks for the review and the suggestions. |
Sorry, i don't have write access on master in order to make changes to your Pull request. I guess it can wait few days. Stay safe and take care! |
Actually you can create a remote pointing to them and then pull, make
changes and create a PR. Remember to attribute them in the commit so as to
track contribution.
On Fri, Nov 20, 2020 at 6:50 PM Sarhan Aissi ***@***.***> wrote:
Sorry, i don't have write access on master in order to make changes to
your Pull request. I guess it can wait few days. Stay safe and take care!
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1511 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAB7ZENC2KOFYJ5BIJ4PFZTSQ2F5ZANCNFSM4TEFUNVQ>
.
--
Sent from a tiny device while on the move.
|
I didn't know that, thank you for the info @profnandaa |
@tux-tn -- however, you will need to make a separate PR |
@profnandaa already did |
closing in favor of #1528 |
Related: #1510
{{ briefly describe what you have done in this PR }}
Checklist