Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: wrong ordinal for fr local #1932

Merged
merged 2 commits into from Jun 11, 2022
Merged

fix: wrong ordinal for fr local #1932

merged 2 commits into from Jun 11, 2022

Conversation

R3tep
Copy link
Contributor

@R3tep R3tep commented Jun 7, 2022

This pull request fix a wrong ordinal definition for fr local.

@codecov
Copy link

codecov bot commented Jun 7, 2022 •
edited

Codecov Report

Merging #1932 (ef0561d) into dev (7b4a99c) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##               dev     #1932   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          181       181           
  Lines         2064      2064           
  Branches       538       538           
=========================================
  Hits          2064      2064
Impacted Files Coverage Δ
src/locale/fr.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7b4a99c...ef0561d. Read the comment docs.

@iamkun
Copy link
Owner

iamkun commented Jun 7, 2022 •
edited

Is fr locale's ordinal different under different format tokens?

ref https://github.com/moment/moment/blob/develop/src/locale/fr.js#L82

@R3tep
Copy link
Contributor Author

R3tep commented Jun 8, 2022

The fr locale can change whether the grammatical gender is masculine or feminine.

This only affects numbers equal to 1.

The masculine grammatical gender is 1er.
The feminine grammatical gender is 1re.

But for a number greater than 1, it is always suffixed by e like 11e

@iamkun
Copy link
Owner

iamkun commented Jun 11, 2022

Thanks, let's get this merged

@iamkun iamkun merged commit 8f09834 into iamkun:dev Jun 11, 2022
iamkun pushed a commit that referenced this pull request Jul 19, 2022
@semantic-release-bot
chore(release): 1.11.4 [skip ci]
fdf648b 
## [1.11.4](v1.11.3...v1.11.4) (2022-07-19)

### Bug Fixes

* correct past property in ku (kurdish) locale ([#1916](#1916)) ([74e82b9](74e82b9))
* fix French [fr] local ordinal ([#1932](#1932)) ([8f09834](8f09834))
* fix objectSupport plugin ConfigTypeMap type ([#1441](#1441)) ([#1990](#1990)) ([fd51fe4](fd51fe4))
* fix type error to add ordianl property in InstanceLocaleDataReturn and GlobalLocaleDataReturn types ([#1931](#1931)) ([526f0ae](526f0ae))
* update locale ar-* meridiem function ([#1954](#1954)) ([3d31611](3d31611))
* zh-tw / zh-hk locale ordinal error ([#1976](#1976)) ([0a1bd08](0a1bd08))
@iamkun
Copy link
Owner

iamkun commented Jul 19, 2022

🎉 This PR is included in version 1.11.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

@xavier-villelegier
Copy link

xavier-villelegier commented Jul 25, 2022 •
edited

Hey @R3tep, why this change ? In french when it's the 1st of July for exemple we say "1er juillet 2022", but for the 2nd, we never write "2e juillet 2022" or "3e juillet 2022". This might be correct mathematically speaking, but not in term of dates ? What was your use case ?

cc @iamkun

@xavier-villelegier xavier-villelegier mentioned this pull request Aug 5, 2022
Merged
BePo65 pushed a commit to BePo65/dayjs that referenced this pull request Aug 6, 2022
@semantic-release-bot @BePo65
chore(release): 1.11.4 [skip ci]
f7096c6 
## [1.11.4](iamkun/dayjs@v1.11.3...v1.11.4) (2022-07-19)

### Bug Fixes

* correct past property in ku (kurdish) locale ([iamkun#1916](iamkun#1916)) ([74e82b9](iamkun@74e82b9))
* fix French [fr] local ordinal ([iamkun#1932](iamkun#1932)) ([8f09834](iamkun@8f09834))
* fix objectSupport plugin ConfigTypeMap type ([iamkun#1441](iamkun#1441)) ([iamkun#1990](iamkun#1990)) ([fd51fe4](iamkun@fd51fe4))
* fix type error to add ordianl property in InstanceLocaleDataReturn and GlobalLocaleDataReturn types ([iamkun#1931](iamkun#1931)) ([526f0ae](iamkun@526f0ae))
* update locale ar-* meridiem function ([iamkun#1954](iamkun#1954)) ([3d31611](iamkun@3d31611))
* zh-tw / zh-hk locale ordinal error ([iamkun#1976](iamkun#1976)) ([0a1bd08](iamkun@0a1bd08))
@CBD-iBiZa
Copy link

CBD-iBiZa commented Nov 29, 2022 •
edited

@xavier-villelegier

Hey @R3tep, why this change ? In french when it's the 1st of July for exemple we say "1er juillet 2022", but for the 2nd, we never write "2e juillet 2022" or "3e juillet 2022". This might be correct mathematically speaking, but not in term of dates ? What was your use case ?

cc @iamkun

For week of year, with ordinal (wo format), the output in french is not correct.

  • 1st (week) => 1re (semaine), not 1er (semaine)
  • 2nd (week) => 2e (semaine), not 2 (semaine)
  • 3rd (week) => 3e (semaine), not 3 (semaine)
  • ...

It was perfectly ok in Moment, not with DayJS+WeekOfYear plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@R3tep @iamkun @xavier-villelegier @CBD-iBiZa