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

Add parsing negative components in durations when ISO 8601 #2955

Closed
wants to merge 2 commits into
base: develop
from

Conversation

Projects
None yet
4 participants
@stephenreddek
Contributor

stephenreddek commented Feb 7, 2016

This is to resolve issue #2408 where formats such as 'P-6D' were not supported for durations. It supports negating each component of the duration. If the whole duration has already been negated (-P6D -> -6 days), then it will make the component positive ('-P-6D' -> 6 days). The issue showed "-P-6H+3M" as an example, but the lack of a 'T' does not fit with the rest of the standard. In light of this, the changes allow for "-PT-6H+3M" instead.

@@ -45,14 +45,21 @@ export function createDuration (input, key) {
};
} else if (!!(match = isoRegex.exec(input))) {
sign = (match[1] === '-') ? -1 : 1;
var ySign = (match[2] === '-') ? -sign : sign;

This comment has been minimized.

@icambron

icambron Feb 22, 2016

Member

Can you simplify this into a local localSign function?

This comment has been minimized.

@stephenreddek

stephenreddek Feb 24, 2016

Contributor

I think it can be simplified by making the parseFloat handle the sign. If the isoRegex includes (?:-)? inside each of the elements before the character class, then it can be part of the capture group and will be parsed as negative by parseFloat. I can update the pull request to include that.

This comment has been minimized.

@icambron
@ichernev

This comment has been minimized.

Contributor

ichernev commented Mar 6, 2016

Currently duration objects are not smart enough to handle mixed sign units. Also its not clear how such durations are to be added to moments -- order from big to small (year to second) or small to big.

Is there any mention in the ISO 8601 about this?

@stephenreddek

This comment has been minimized.

Contributor

stephenreddek commented Mar 17, 2016

I'm not sure I understand what you mean by how the durations should be added in this context. Are you referring to the order the components are added to a moment so that proper bubbling occurs?

Are there other, larger changes that must be made before this issue can be addressed?

@ichernev

This comment has been minimized.

Contributor

ichernev commented Apr 11, 2016

@stephenreddek I think we can merge this. Its a good idea to make adding subtracting a duration from a moment -- we should go from big to small units (months, days, milliseconds), now we do the opposite (for some reason...).

@ichernev ichernev added this to the 2.13.0 milestone Apr 11, 2016

@ichernev

This comment has been minimized.

Contributor

ichernev commented Apr 16, 2016

Merged in 6592ad9

@ichernev ichernev closed this Apr 16, 2016

ichernev added a commit that referenced this pull request Apr 16, 2016

Merge pull request #2955 from stephenreddek:develop
Add parsing negative components in durations when ISO 8601
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment