Skip to content

Allow whitespaces after parsed content in DateFormatter.date(from:) #2785

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
May 12, 2020

Conversation

lxbndr
Copy link
Contributor

@lxbndr lxbndr commented May 7, 2020

Noticed the difference between macOS and Windows test results in our codebase (we are heavily dependent on date parsing). I have not found any docs about this, but actual results and deep debugging left no doubt about the mechanics.

Apple implementation checks for unparsed content in source string.
Any non-whitespace leftover characters are considered as failure.
On the other hand, whitespaces are permitted and should not affect the result.

Related to #1822 (SR-9322).
This patch alters verification logic to be closer to Apple implementation.

Not sure about preconditionFailure check, but that's really unexpected condition.

cc @compnerd @spevans

@spevans
Copy link
Contributor

spevans commented May 7, 2020

@swift-ci test linux

@millenomi
Copy link
Contributor

@swift-ci please test

@compnerd
Copy link
Member

Apple implementation checks for unparsed content in source string.
Any non-whitespace leftover characters are considered as failure.
On the other hand, whitespaces are permitted and should not affect the result.

The check was partially implemented in swiftlang#1822 (SR-9322).
This patch alters verification logic to be closer to Apple implementation.
@lxbndr lxbndr force-pushed the date-formatter-whitespaces branch from fd2f66c to dd22d7d Compare May 11, 2020 12:29
@lxbndr lxbndr requested a review from spevans May 11, 2020 12:30
@spevans
Copy link
Contributor

spevans commented May 11, 2020

@swift-ci test linux

@millenomi
Copy link
Contributor

@swift-ci please test linux

@millenomi
Copy link
Contributor

Gotta say please. 😇

@millenomi
Copy link
Contributor

By the way, macOS works now.

@compnerd compnerd merged commit ab75dee into swiftlang:master May 12, 2020
@lxbndr lxbndr deleted the date-formatter-whitespaces branch May 13, 2020 06:10
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.

4 participants