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

Fixes for Date Manipulations #668

Merged
merged 4 commits into from Oct 5, 2016
Merged

Conversation

UberJason
Copy link
Contributor

This PR has fixes for DateComponents and Calendar which were causing date manipulations to fail, resolving SR-2846:

  • DateComponents no longer returns isLeapMonth = false when isLeapMonth wasn't set (in situations where isLeapMonth is irrelevant, for example using DateComponents to add 1 day to another date).
  • NSCalendar's _convert() is fixed for the character set for isLeapMonth to align with expected value from UCalendarDateFields ('l' instead of 'L').
  • NSCalendar's date(byAdding:to:options:) is fixed to do math on the passed-in date, rather than the reference time 0.0.

A test for this is included.

Jason Ji added 3 commits October 4, 2016 16:40
date(byAdding:to:options:) was always trying to add the components to CFAbsoluteTime 0.0, rather than the passed-in date.
- DateComponent should only return isLeapMonth if the underlying leapMonthSet variable is true. Previously, DateComponent was setting isLeapMonth to false in situations where there is no concept of isLeapMonth.
- _convert was using 'L' for isLeapMonth, when the corresponding UCalendarDateFields enum value for isLeapMonth is 'l'. This was causing _CFCalendarAddComponentsV to return error status.
@UberJason
Copy link
Contributor Author

@swift-ci Please smoke test

@@ -89,6 +90,18 @@ class TestNSCalendar: XCTestCase {
XCTAssertEqual(copy.firstWeekday, 2)
XCTAssertEqual(copy.minimumDaysInFirstWeek, 2)
}

func test_addingDates() {
let calendar = Calendar.current
Copy link
Member

Choose a reason for hiding this comment

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

Let's use the gregorian calendar specifically here, to avoid assuming anything about the environment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in latest commit, thanks!

@parkera
Copy link
Member

parkera commented Oct 4, 2016

cc @lifeissweetgood

@parkera
Copy link
Member

parkera commented Oct 4, 2016

@swift-ci please test

@lifeissweetgood lifeissweetgood merged commit 1681a38 into apple:master Oct 5, 2016
@UberJason
Copy link
Contributor Author

Thanks very much, @parkera and @lifeissweetgood, for quickly merging this PR. Do either of you know if this patch will make it into Swift 3.0.1 preview 2? Our team is in somewhat dire need of this patch, as it's preventing us from doing date manipulations on our Swift-based server.

@parkera
Copy link
Member

parkera commented Oct 6, 2016

@UberJason Can you put up a PR against the swift-3.0 branch branch?

UberJason pushed a commit to UberJason/swift-corelibs-foundation that referenced this pull request Oct 6, 2016
Fixes for Date Manipulations
@UberJason
Copy link
Contributor Author

@parkera Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants