Skip to content

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion CoreFoundation/Locale.subproj/CFCalendar.c
Original file line number Diff line number Diff line change
Expand Up @@ -236,14 +236,23 @@ CFCalendarRef CFCalendarCopyCurrent(void) {
}

Boolean _CFCalendarInitWithIdentifier(CFCalendarRef calendar, CFStringRef identifier) {
if (identifier != kCFGregorianCalendar && identifier != kCFBuddhistCalendar && identifier != kCFJapaneseCalendar && identifier != kCFIslamicCalendar && identifier != kCFIslamicCivilCalendar && identifier != kCFHebrewCalendar && identifier != kCFChineseCalendar) {
if (identifier != kCFGregorianCalendar && identifier != kCFBuddhistCalendar && identifier != kCFJapaneseCalendar && identifier != kCFIslamicCalendar && identifier != kCFIslamicCivilCalendar && identifier != kCFHebrewCalendar && identifier != kCFRepublicOfChinaCalendar && identifier != kCFPersianCalendar && identifier != kCFCalendarIdentifierCoptic && identifier != kCFCalendarIdentifierEthiopicAmeteMihret && identifier != kCFCalendarIdentifierEthiopicAmeteAlem && identifier != kCFChineseCalendar && identifier != kCFISO8601Calendar && identifier != kCFIslamicTabularCalendar && identifier != kCFIslamicUmmAlQuraCalendar) {
if (CFEqual(kCFGregorianCalendar, identifier)) identifier = kCFGregorianCalendar;
else if (CFEqual(kCFBuddhistCalendar, identifier)) identifier = kCFBuddhistCalendar;
else if (CFEqual(kCFJapaneseCalendar, identifier)) identifier = kCFJapaneseCalendar;
else if (CFEqual(kCFIslamicCalendar, identifier)) identifier = kCFIslamicCalendar;
else if (CFEqual(kCFIslamicCivilCalendar, identifier)) identifier = kCFIslamicCivilCalendar;
else if (CFEqual(kCFHebrewCalendar, identifier)) identifier = kCFHebrewCalendar;
else if (CFEqual(kCFRepublicOfChinaCalendar, identifier)) identifier = kCFRepublicOfChinaCalendar;
else if (CFEqual(kCFPersianCalendar, identifier)) identifier = kCFPersianCalendar;
else if (CFEqual(kCFIndianCalendar, identifier)) identifier = kCFIndianCalendar;
else if (CFEqual(kCFCalendarIdentifierCoptic, identifier)) identifier = kCFCalendarIdentifierCoptic;
Copy link
Contributor Author

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?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

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.

else if (CFEqual(kCFCalendarIdentifierEthiopicAmeteMihret, identifier)) identifier = kCFCalendarIdentifierEthiopicAmeteMihret;
else if (CFEqual(kCFCalendarIdentifierEthiopicAmeteAlem, identifier)) identifier = kCFCalendarIdentifierEthiopicAmeteAlem;
else if (CFEqual(kCFChineseCalendar, identifier)) identifier = kCFChineseCalendar;
else if (CFEqual(kCFISO8601Calendar, identifier)) identifier = kCFISO8601Calendar;
else if (CFEqual(kCFIslamicTabularCalendar, identifier)) identifier = kCFIslamicTabularCalendar;
else if (CFEqual(kCFIslamicUmmAlQuraCalendar, identifier)) identifier = kCFIslamicUmmAlQuraCalendar;
else return false;
}

Expand Down
2 changes: 1 addition & 1 deletion CoreFoundation/Locale.subproj/CFLocaleKeys.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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");
Expand Down
2 changes: 1 addition & 1 deletion Foundation/NSCalendar.swift
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ extension NSCalendar {
public static let ethiopicAmeteMihret = NSCalendar.Identifier("ethiopic")
public static let ethiopicAmeteAlem = NSCalendar.Identifier("ethiopic-amete-alem")
public static let hebrew = NSCalendar.Identifier("hebrew")
public static let ISO8601 = NSCalendar.Identifier("")
public static let ISO8601 = NSCalendar.Identifier("iso8601")
public static let indian = NSCalendar.Identifier("indian")
public static let islamic = NSCalendar.Identifier("islamic")
public static let islamicCivil = NSCalendar.Identifier("islamic-civil")
Expand Down
39 changes: 39 additions & 0 deletions TestFoundation/TestNSCalendar.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Member

@pushkarnk pushkarnk Oct 4, 2016

Choose a reason for hiding this comment

The 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 gettingDatesOnGregorianCalendar). That should be enough to make sure the underlying ICU is supporting all the types.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not all calendars have the same concept of year, though.

Choose a reason for hiding this comment

The 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)

Expand All @@ -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)
Expand Down