-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add support for ISO8601 calendar and others #667
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -126,7 +126,7 @@ CONST_STRING_DECL(kCFCalendarIdentifierChinese, "chinese"); | |
CONST_STRING_DECL(kCFCalendarIdentifierRepublicOfChina, "roc"); | ||
CONST_STRING_DECL(kCFCalendarIdentifierPersian, "persian"); | ||
CONST_STRING_DECL(kCFCalendarIdentifierIndian, "indian"); | ||
CONST_STRING_DECL(kCFCalendarIdentifierISO8601, ""); | ||
CONST_STRING_DECL(kCFCalendarIdentifierISO8601, "iso8601"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without this line change, the lookup for ISO8601 failed. |
||
CONST_STRING_DECL(kCFCalendarIdentifierCoptic, "coptic"); | ||
CONST_STRING_DECL(kCFCalendarIdentifierEthiopicAmeteMihret, "ethiopic"); | ||
CONST_STRING_DECL(kCFCalendarIdentifierEthiopicAmeteAlem, "ethiopic-amete-alem"); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,15 +20,41 @@ class TestNSCalendar: XCTestCase { | |
|
||
static var allTests: [(String, (TestNSCalendar) -> () throws -> Void)] { | ||
return [ | ||
("test_allCalendars", test_allCalendars), | ||
("test_gettingDatesOnGregorianCalendar", test_gettingDatesOnGregorianCalendar ), | ||
("test_gettingDatesOnHebrewCalendar", test_gettingDatesOnHebrewCalendar ), | ||
("test_gettingDatesOnChineseCalendar", test_gettingDatesOnChineseCalendar), | ||
("test_gettingDatesOnISO8601Calendar", test_gettingDatesOnISO8601Calendar), | ||
("test_copy",test_copy), | ||
// Disabled because this fails on linux https://bugs.swift.org/browse/SR-320 | ||
// ("test_currentCalendarRRstability", test_currentCalendarRRstability), | ||
] | ||
} | ||
|
||
func test_allCalendars() { | ||
for identifier in [ | ||
Calendar.Identifier.buddhist, | ||
Calendar.Identifier.chinese, | ||
Calendar.Identifier.coptic, | ||
Calendar.Identifier.ethiopicAmeteAlem, | ||
Calendar.Identifier.ethiopicAmeteMihret, | ||
Calendar.Identifier.gregorian, | ||
Calendar.Identifier.hebrew, | ||
Calendar.Identifier.indian, | ||
Calendar.Identifier.islamic, | ||
Calendar.Identifier.islamicCivil, | ||
Calendar.Identifier.islamicTabular, | ||
Calendar.Identifier.islamicUmmAlQura, | ||
Calendar.Identifier.iso8601, | ||
Calendar.Identifier.japanese, | ||
Calendar.Identifier.persian, | ||
Calendar.Identifier.republicOfChina | ||
] { | ||
let calendar = Calendar(identifier: identifier) | ||
XCTAssertEqual(identifier,calendar.identifier) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if there's a generic property I should be looking up here for all calendars to test that they are created appropriately? At the moment it's just checking the identifier and initialiser. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can have tests that create calendars for all types and then verify the year, month and day (something like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not all calendars have the same concept of year, though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At the moment, the identifier is the most correct property to be checking for equality. The other properties of the calendar object don't necessarily indicate whether or not the calendar is of a certain kind (meaning "gregorian" "buddist" etc.). Since this test doesn't care about checking the equality of two calendar instances and only cares about the calendar instance having the correct identity, I think this is sufficient for now. |
||
} | ||
} | ||
|
||
func test_gettingDatesOnGregorianCalendar() { | ||
let date = Date(timeIntervalSince1970: 1449332351) | ||
|
||
|
@@ -40,6 +66,19 @@ class TestNSCalendar: XCTestCase { | |
XCTAssertEqual(components.month, 12) | ||
XCTAssertEqual(components.day, 5) | ||
} | ||
|
||
func test_gettingDatesOnISO8601Calendar() { | ||
let date = Date(timeIntervalSince1970: 1449332351) | ||
|
||
var calendar = Calendar(identifier: .iso8601) | ||
calendar.timeZone = TimeZone(identifier: "UTC")! | ||
let components = calendar.dateComponents([.year, .month, .day], from: date) | ||
|
||
XCTAssertEqual(components.year, 2015) | ||
XCTAssertEqual(components.month, 12) | ||
XCTAssertEqual(components.day, 5) | ||
} | ||
|
||
|
||
func test_gettingDatesOnHebrewCalendar() { | ||
let date = Date(timeIntervalSince1970: 1552580351) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems there was a name change from kCFxxxCalendar to kCFCalendarXXxx, but I tried to add the older one here. However, I couldn't find the kCFCopticCalendar, so I didn't use that. I wonder if there's benefit in normalising all of these to kCFCalendarXxxx instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you could define them here: https://github.com/apple/swift-corelibs-foundation/blob/master/CoreFoundation/Locale.subproj/CFLocaleKeys.c#L149
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm open to that idea, but will wait for the recommended solution before I make any further changes here.