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

Added documentation about out-of-range components to DateTime.parse. #44521

Closed
wants to merge 3 commits into from

Conversation

hoylen
Copy link
Contributor

@hoylen hoylen commented Dec 21, 2020

The DateTime.parse method accepts out-of-range components. For example, parsing the string "2020-01-42" produces "2020-02-11".

This was raised as a bug in #11189, but after more than 7 years that issue was closed with the solution being "use parseStrict from the intl package" instead.

This pull request simply updates the documentation of DateTime's parse method so that users know:

  1. This is how the method behaves. Otherwise they might be surprised when it unexpectedly happens in their code.
  2. This behaviour is a feature and not a bug. Otherwise they might raise another issue for it.
  3. The parseStrict method in the intl package exists. Otherwise they might reinvent the wheel.

@google-cla google-cla bot added the cla: yes label Dec 21, 2020
@hoylen hoylen changed the title Added comment about out-of-range components to DateTime.parse. Added documentation about out-of-range components to DateTime.parse. Dec 21, 2020
@kevmoo
Copy link
Member

kevmoo commented Dec 21, 2020

@mraleph
Copy link
Member

mraleph commented Jan 11, 2021

@hoylen see review comments on https://dart-review.googlesource.com/c/sdk/+/176680

once they are addressed just push a new commit into your branch and it will be picked up automatically.

* them as overflows into the next larger component.
* For example, "2020-01-42" will be parsed as 2020-02-11, because
* the last valid date in that month is 2020-01-31, so 42 days is
* interprted as 31 days of that month plus 11 days into the next month.
Copy link

Choose a reason for hiding this comment

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

nit: interpreted*

@Jonas-Sander
Copy link

Would really love to see this. Is this still being discussed?

@mraleph
Copy link
Member

mraleph commented Jun 10, 2021

This is ready to land by requires rebase.

@hoylen
Copy link
Contributor Author

hoylen commented Jun 27, 2021

Rebase done. Ready "to land".

@kevmoo
Copy link
Member

kevmoo commented Jun 27, 2021

@mraleph @lrhn – can you take a look and hit the button in gerrit?

@dart-bot dart-bot closed this in 4b82a5f Jun 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants