Skip to content

Parity: NSCoding: DateIntervalFormatter #2154

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

Merged
merged 1 commit into from
Apr 23, 2019
Merged

Parity: NSCoding: DateIntervalFormatter #2154

merged 1 commit into from
Apr 23, 2019

Conversation

millenomi
Copy link
Contributor

  • Implement NSCoding for DateIntervalFormatter through new hooks in CFDateIntervalFormatter. This requires fixing several bugs uncovered in the process:

  • Correctly initialize CFDateIntervalFormatter’s date and time style.

  • Fix __NSSwiftData to never try to encode itself, matching what the Darwin counterpart does.

  • NSCalendar needs to check if a coder contains values for a specific key before trying to fetch that key.

  • Add tests with fixtures. Since time zone data can come from different ICU versions, only compare time zone names.

  • Prevent fixtures with duplicate identifiers.

@millenomi
Copy link
Contributor Author

@millenomi
Copy link
Contributor Author

@swift-ci please test

 - Implement NSCoding for DateIntervalFormatter through new hooks in CFDateIntervalFormatter. This requires fixing several bugs uncovered in the process:

 - Correctly initialize CFDateIntervalFormatter’s date and time style.

 - Fix __NSSwiftData to never try to encode itself, matching what the Darwin counterpart does.

 - NSCalendar needs to check if a coder contains values for a specific key before trying to fetch that key.

 - Add tests with fixtures. Since time zone data can come from different ICU versions, only compare time zone names.

 - Prevent fixtures with duplicate identifiers.

/* From https://developer.apple.com/documentation/foundation/nstimezone/1387250-init:
"Discussion
As of macOS 10.6, the underlying implementation of this method has been changed to ignore the specified data parameter."
Copy link
Contributor Author

Choose a reason for hiding this comment

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

cc @compnerd Is this going to be a problem for your implementation?

Copy link
Member

Choose a reason for hiding this comment

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

Actually for Windows, it is the inverse - that is more inline with the current implementation.

@millenomi
Copy link
Contributor Author

@swift-ci please test

1 similar comment
@millenomi
Copy link
Contributor Author

@swift-ci please test

@millenomi millenomi merged commit 91abe2a into swiftlang:master Apr 23, 2019
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.

2 participants