Allow ISO-8601 to be passed as a format. #1693
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sbstp
changed the title
This commit allows the ISO-8601 format to be passed as a format in the moment(string, array) constructor. This is a preliminary commit as this addition as yielded a few issues.
I've added a useFallback parameter to makeDateFromString. When not supplied, the old behavior remains. When supplied and true, it does not call the createFromInputCallback and instead returns an empty config. This also seems to fix the first issue, it now plays much nicer with scoring and precendence.
I've added much more relevant tests, checking the _f property for the format that was used for parsing and _pf.iso in the case of ISO.
|
This seems to be working properly, now. It also seems like it didn't break anything else. Let me know if you want me to add more tests or if you think that something won't work properly. |
| var i, l, | ||
| string = config._i, | ||
| match = isoRegex.exec(string); | ||
|
|
||
| useFallback = typeof useFallback === 'undefined' ? true : useFallback; |
There was a problem hiding this comment.
useFallback == null should be enough here (check for null or undefined).
|
I I think its good in general. Just please extract the parsing iso part into a separate function. |
`moment.js` now has a separate parseISO function which only handles the parsing. `makeDateFromString` keeps the same behavior and uses parseISO. The tests have also been refactored and moved into their own function.
|
Should I also update the docs to document this? |
|
Thanks, that looks great. About docs you can make a pr at the momentjs.com repo. I do docs changes on release in general but you're welcome to help. |
ichernev
added a commit
that referenced
this pull request
Jun 12, 2014
Allow ISO-8601 to be passed as a format. @changelog @section features @description support moment.ISO_8601 as a format in constructor, that would use ISO time format parsing
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit allows the ISO-8601 format to be passed as a format in the moment(string, array) constructor. This is an attempt to make #1680 possible.This is a preliminary commit as this addition as yielded a few issues. This is not ready for merge.
Here are some of the issues:
makeDateFromStringdoes not play nice with scoring (might be due to the way I implemented it)makeDateFromStringshoves what it doesn't know into intocreateFromInputFallbackwhich has been modified to throw always throw an error in the test suite.I will probably try to add an optional parameter to
makeDateFromStringwhich tells it to not usecreateFromInputFallbackto fix the second issue.