Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

month/day format with time returns nil #114

Merged
merged 3 commits into from

2 participants

@grantovich

This is related to #59, which I commented on but I guess it wasn't noticed. The issue is that month/day parsing (without a year) seems to always return nil when a time is given. For example:

irb> Chronic.parse('5/6')
=> 2012-05-06 12:00:00 -0400
irb> Chronic.parse('5/6/2012 at 12pm')
=> 2012-05-06 12:00:00 -0400
irb> Chronic.parse('5/6 at 12pm')
=> nil
@grantovich

The attached pull request fixes this issue and includes test cases for it.

@grantovich grantovich commented on the diff
test/test_chronic.rb
@@ -67,6 +67,8 @@ def test_endian_definitions
# middle, little
endians = [
Chronic::Handler.new([:scalar_month, :separator_slash_or_dash, :scalar_day, :separator_slash_or_dash, :scalar_year, :separator_at?, 'time?'], :handle_sm_sd_sy),
+ Chronic::Handler.new([:scalar_month, :separator_slash_or_dash, :scalar_day, :separator_at?, 'time?'], :handle_sm_sd),
+ Chronic::Handler.new([:scalar_day, :separator_slash_or_dash, :scalar_month, :separator_at?, 'time?'], :handle_sd_sm),
Chronic::Handler.new([:scalar_day, :separator_slash_or_dash, :scalar_month, :separator_slash_or_dash, :scalar_year, :separator_at?, 'time?'], :handle_sd_sm_sy)
]

I left this test as-is in the interest of separating changes, but I'm not sure why these lines and the following assert_equal are present. It's just testing that the endians array is equal to a copy-pasted copy of itself. Am I missing something here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@leejarvis
Collaborator

Yeah I must have missed your comment on that other issue. This looks great, thanks for the pull will merge this straight away.

@leejarvis leejarvis merged commit 201e2c3 into mojombo:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
3  lib/chronic/chronic.rb
@@ -188,7 +188,6 @@ def definitions(options={})
Handler.new([:scalar_day, :repeater_month_name, :scalar_year, :separator_at?, 'time?'], :handle_sd_rmn_sy),
Handler.new([:scalar_day, :repeater_month_name, :separator_at?, 'time?'], :handle_sd_rmn),
Handler.new([:scalar_year, :separator_slash_or_dash, :scalar_month, :separator_slash_or_dash, :scalar_day, :separator_at?, 'time?'], :handle_sy_sm_sd),
- Handler.new([:scalar_month, :separator_slash_or_dash, :scalar_day], :handle_sm_sd),
Handler.new([:scalar_month, :separator_slash_or_dash, :scalar_year], :handle_sm_sy),
Handler.new([:scalar_day, :separator_slash_or_dash, :repeater_month_name, :separator_slash_or_dash, :scalar_year], :handle_sm_rmn_sy)
],
@@ -216,6 +215,8 @@ def definitions(options={})
endians = [
Handler.new([:scalar_month, :separator_slash_or_dash, :scalar_day, :separator_slash_or_dash, :scalar_year, :separator_at?, 'time?'], :handle_sm_sd_sy),
+ Handler.new([:scalar_month, :separator_slash_or_dash, :scalar_day, :separator_at?, 'time?'], :handle_sm_sd),
+ Handler.new([:scalar_day, :separator_slash_or_dash, :scalar_month, :separator_at?, 'time?'], :handle_sd_sm),
Handler.new([:scalar_day, :separator_slash_or_dash, :scalar_month, :separator_slash_or_dash, :scalar_year, :separator_at?, 'time?'], :handle_sd_sm_sy)
]
View
19 lib/chronic/handlers.rb
@@ -224,27 +224,30 @@ def handle_sy_sm_sd(tokens, options)
handle_sm_sd_sy(new_tokens + time_tokens, options)
end
- # Handle scalar-day/scalar-month AND scalar-month/scalar-day
+ # Handle scalar-month/scalar-day
def handle_sm_sd(tokens, options)
month = tokens[0].get_tag(ScalarMonth).type
day = tokens[1].get_tag(ScalarDay).type
year = Chronic.now.year
-
- if Array(options[:endian_precedence]).first == :little
- day, month = month, day
- end
+ time_tokens = tokens.last(tokens.size - 2)
return if month_overflow?(year, month, day)
begin
- start_time = Chronic.time_class.local(year, month, day)
- end_time = Chronic.time_class.local(year, month, day + 1)
- Span.new(start_time, end_time)
+ day_start = Chronic.time_class.local(year, month, day)
+ day_or_time(day_start, time_tokens, options)
rescue ArgumentError
nil
end
end
+ # Handle scalar-day/scalar-month
+ def handle_sd_sm(tokens, options)
+ new_tokens = [tokens[1], tokens[0]]
+ time_tokens = tokens.last(tokens.size - 2)
+ handle_sm_sd(new_tokens + time_tokens, options)
+ end
+
# Handle scalar-month/scalar-year
def handle_sm_sy(tokens, options)
month = tokens[0].get_tag(ScalarMonth).type
View
2  test/test_chronic.rb
@@ -67,6 +67,8 @@ def test_endian_definitions
# middle, little
endians = [
Chronic::Handler.new([:scalar_month, :separator_slash_or_dash, :scalar_day, :separator_slash_or_dash, :scalar_year, :separator_at?, 'time?'], :handle_sm_sd_sy),
+ Chronic::Handler.new([:scalar_month, :separator_slash_or_dash, :scalar_day, :separator_at?, 'time?'], :handle_sm_sd),
+ Chronic::Handler.new([:scalar_day, :separator_slash_or_dash, :scalar_month, :separator_at?, 'time?'], :handle_sd_sm),
Chronic::Handler.new([:scalar_day, :separator_slash_or_dash, :scalar_month, :separator_slash_or_dash, :scalar_year, :separator_at?, 'time?'], :handle_sd_sm_sy)
]

I left this test as-is in the interest of separating changes, but I'm not sure why these lines and the following assert_equal are present. It's just testing that the endians array is equal to a copy-pasted copy of itself. Am I missing something here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
View
8 test/test_parsing.rb
@@ -294,8 +294,14 @@ def test_handle_sm_sd
time = parse_now("05/06", :endian_precedence => [:little, :medium])
assert_equal Time.local(2006, 6, 5, 12), time
+ time = parse_now("05/06 6:05:57 PM")
+ assert_equal Time.local(2006, 5, 6, 18, 05, 57), time
+
+ time = parse_now("05/06 6:05:57 PM", :endian_precedence => [:little, :medium])
+ assert_equal Time.local(2006, 6, 5, 18, 05, 57), time
+
time = parse_now("13/01")
- assert_nil time
+ assert_equal Time.local(2006, 1, 13, 12), time
end
# def test_handle_sm_sy
Something went wrong with that request. Please try again.