Feature #9549
closedImprovements to Time::strptime
Description
I opened a pull request on GitHub a few days ago but wanted to open a parallel issue here, since I think this is the preferred tracker.
This patch includes three commits:
-
Add default arguments to
Time::strptime
to matchDate::strptime
. After this patch,Time::strptime
may be invoked with 0, 1, or 2 arguments. When invoked with 0 or 1, it produces a result equivalent toDate::strptime
invoked with the same arguments. -
Raise ArgumentError if time passed to
Time::strptime
is invalid. This fixes a Ruby bug and adds a test to ensure it will not regress. Before this commit:require 'date'
Date::strptime('31/02/2014', '%d/%m/%Y') # ArgumentError: invalid date
require 'time'
Time::strptime('31/02/2014', '%d/%m/%Y') # 2014-03-03 00:00:00 +0000 -
I have also renamed variables in
Time::parse
to be consistent with the changes I made toTime::strptime
. Specifically, renamingd
tohash
, since it is not aDate
and renaming thedate
parameter totime
. I believe both of these changes make the code clearer.
Thank you for considering this patch.
Files
Updated by nagachika (Tomoyuki Chikanaga) over 10 years ago
- Status changed from Open to Assigned
- Assignee set to akr (Akira Tanaka)
Updated by akr (Akira Tanaka) over 10 years ago
- Status changed from Assigned to Feedback
Erik Michaels-Ober wrote:
- Add default arguments to
Time::strptime
to matchDate::strptime
. After this patch,Time::strptime
may be invoked with 0, 1, or 2 arguments. When invoked with 0 or 1, it produces a result equivalent toDate::strptime
invoked with the same arguments.
I'm not sure that is a good idea.
For example, -4712-01-01 is not a special date in Time.
Raise ArgumentError if time passed to
Time::strptime
is invalid. This fixes a Ruby bug and adds a test to ensure it will not regress. Before this commit:require 'date'
Date::strptime('31/02/2014', '%d/%m/%Y') # ArgumentError: invalid date
require 'time'
Time::strptime('31/02/2014', '%d/%m/%Y') # 2014-03-03 00:00:00 +0000
I'm reluctant to such a restriction.
I tried to verify invalid time using round trip test once.
(ruby-core:14517, ruby-dev:33058, r14765, r15203)
But it caused bigger problems over benefits.
Updated by akr (Akira Tanaka) almost 10 years ago
- Related to Bug #10588: Invalid Dates added
Updated by akr (Akira Tanaka) about 7 years ago
- Status changed from Feedback to Rejected