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.

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

Parse month from string in customParseFormat #457

Merged
merged 17 commits into from
Feb 23, 2019

Conversation

Kreozot
Copy link
Contributor

@Kreozot Kreozot commented Jan 16, 2019

to handle 'MMM' and 'MMMM' formats (February, Feb, etc)

@@ -64,15 +64,16 @@ const parseDate = (date) => {

class Dayjs {
constructor(cfg) {
this.$L = this.$L || parseLocale(cfg.locale, null, true) || L
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to make locale available in parse() function

@codecov-io
Copy link

codecov-io commented Jan 16, 2019

Codecov Report

Merging #457 into dev will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@         Coverage Diff         @@
##            dev   #457   +/-   ##
===================================
  Coverage   100%   100%           
===================================
  Files        55     58    +3     
  Lines       484    525   +41     
  Branches     75     88   +13     
===================================
+ Hits        484    525   +41
Impacted Files Coverage Δ
src/index.js 100% <100%> (ø) ⬆️
src/plugin/customParseFormat/index.js 100% <100%> (ø) ⬆️
src/locale/et.js 100% <0%> (ø) ⬆️
src/locale/nn.js 100% <0%> (ø) ⬆️
src/locale/es-do.js 100% <0%> (ø) ⬆️
src/locale/ar.js 100% <0%> (ø) ⬆️
src/locale/bg.js 100% <0%> (ø) ⬆️
src/locale/nl.js 100% <0%> (ø) ⬆️
src/locale/el.js 100% <0%> (ø) ⬆️
src/locale/fi.js 100% <0%> (ø) ⬆️
... and 33 more

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 3bd06f2...56dba7c. Read the comment docs.

@Kreozot
Copy link
Contributor Author

Kreozot commented Jan 20, 2019

@iamkun Is it ok now?

@iamkun
Copy link
Owner

iamkun commented Jan 20, 2019

I'll test babel.config.js change if I got some time first.

@iamkun iamkun added the next release Will merge into next release label Feb 11, 2019
@iamkun
Copy link
Owner

iamkun commented Feb 18, 2019

babel.config.js tested on windows, works good.

MM: [match2, addInput('month')],
MMM: [matchWord, function (input) {
const locale = instance.$locale()
const { months } = locale
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also consider monthsShort in some locales

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why change locale before date parsing?

? monthsShort.findIndex(month => month === input)
: months.findIndex(month => month.substr(0, 3) === input)
if (matchIndex < 0) {
throw new Error(`Failed to parse "${input}" as MMM`)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no error, just skip it like others

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But suddenly it broke the tests on "Invalid Date" after parse corrupted string

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, am I should to bring errors back?

const inputUk = '2018 трав 03'
const format = 'YYYY MMM DD'
dayjs.locale(uk)
expect(dayjs(inputUk, format).valueOf()).toBe(moment(input, format).valueOf())
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not a good test .

we should compare with

moment.locale('uk')

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.
I also write test with passing locale in config (btw, it is undocumented for now)

})

it('return Invalid Date when parse corrupt string', () => {
const input = '2018 Februaru 03'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why moment('2018 Februaru 03', 'YYYY MMMM DD') returns a valid date.

But we might should change Februaru to another error string

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@Kreozot
Copy link
Contributor Author

Kreozot commented Feb 19, 2019

@iamkun Take a look, please!
And I also added case with passing locale in config to docs.

@iamkun iamkun merged commit f343206 into iamkun:dev Feb 23, 2019
iamkun pushed a commit that referenced this pull request Feb 24, 2019
## [1.8.7](v1.8.6...v1.8.7) (2019-02-24)

### Bug Fixes

* Add plugin type definitions ([#418](#418)) ([361d437](361d437))
* Add Swahili locale ([#508](#508)) ([b9cee84](b9cee84))
* Parse month string 'MMMM MMM (February, Feb)' in customParseFormat ([#457](#457)) ([f343206](f343206))
* Update declaration file .diff .isBefore .isSame .isAfter ([#496](#496)) ([4523275](4523275))
* Word orders corrections for locale 'fa' ([#491](#491)) ([56050c2](56050c2))
@iamkun
Copy link
Owner

iamkun commented Feb 24, 2019

🎉 This PR is included in version 1.8.7 🎉

The release is available on:

Your semantic-release bot 📦🚀

andrewhood125ruhuc added a commit to andrewhood125ruhuc/SidRH2 that referenced this pull request May 10, 2022
## [1.8.7](iamkun/dayjs@v1.8.6...v1.8.7) (2019-02-24)

### Bug Fixes

* Add plugin type definitions ([#418](iamkun/dayjs#418)) ([361d437](iamkun/dayjs@361d437))
* Add Swahili locale ([#508](iamkun/dayjs#508)) ([b9cee84](iamkun/dayjs@b9cee84))
* Parse month string 'MMMM MMM (February, Feb)' in customParseFormat ([#457](iamkun/dayjs#457)) ([f343206](iamkun/dayjs@f343206))
* Update declaration file .diff .isBefore .isSame .isAfter ([#496](iamkun/dayjs#496)) ([4523275](iamkun/dayjs@4523275))
* Word orders corrections for locale 'fa' ([#491](iamkun/dayjs#491)) ([56050c2](iamkun/dayjs@56050c2))
andrewhood125ruhuc added a commit to andrewhood125ruhuc/SidRH2 that referenced this pull request May 10, 2022
## [1.8.7](iamkun/dayjs@v1.8.6...v1.8.7) (2019-02-24)

### Bug Fixes

* Add plugin type definitions ([#418](iamkun/dayjs#418)) ([361d437](iamkun/dayjs@361d437))
* Add Swahili locale ([#508](iamkun/dayjs#508)) ([b9cee84](iamkun/dayjs@b9cee84))
* Parse month string 'MMMM MMM (February, Feb)' in customParseFormat ([#457](iamkun/dayjs#457)) ([f343206](iamkun/dayjs@f343206))
* Update declaration file .diff .isBefore .isSame .isAfter ([#496](iamkun/dayjs#496)) ([4523275](iamkun/dayjs@4523275))
* Word orders corrections for locale 'fa' ([#491](iamkun/dayjs#491)) ([56050c2](iamkun/dayjs@56050c2))
splashwizard pushed a commit to splashwizard/tracking-time that referenced this pull request Oct 21, 2024
## [1.8.7](iamkun/dayjs@v1.8.6...v1.8.7) (2019-02-24)

### Bug Fixes

* Add plugin type definitions ([#418](iamkun/dayjs#418)) ([361d437](iamkun/dayjs@361d437))
* Add Swahili locale ([#508](iamkun/dayjs#508)) ([b9cee84](iamkun/dayjs@b9cee84))
* Parse month string 'MMMM MMM (February, Feb)' in customParseFormat ([#457](iamkun/dayjs#457)) ([f343206](iamkun/dayjs@f343206))
* Update declaration file .diff .isBefore .isSame .isAfter ([#496](iamkun/dayjs#496)) ([4523275](iamkun/dayjs@4523275))
* Word orders corrections for locale 'fa' ([#491](iamkun/dayjs#491)) ([56050c2](iamkun/dayjs@56050c2))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next release Will merge into next release released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants