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

Control how numbers on their own are treated #205

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions lib/chronic/handlers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,25 @@ def handle_sd_rmn(tokens, options)
handle_m_d(month, day, tokens[2..tokens.size], options)
end

def handle_od(tokens, options)
t = Chronic.time_class.parse(options[:text])
t = Chronic.time_class.new(self.now.year, self.now.month, t.day)

Span.new(t, t + 1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

BTW, t + 1 is just till next second, but in this case time is not known, thus meaning span actually should be whole day, but if you'll set it so then chronic will return time as 12:00 because of how :guess works. I just hate how fucked up is this lib. Everything passed to handle_generic will be with time 00:00 if no time is specified, but for all other cases/handlers it will be 12:00.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, so for now I should just change the 1 to 60_60_24?

Copy link
Collaborator

Choose a reason for hiding this comment

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

can just leave as it is, but correct way would be Date::DAY_SECONDS, take a look at my PR #206

rescue ArgumentError => e
raise e unless e.message =~ /out of range/
end

# Handle scalar-day
def handle_sd(tokens, options)
day = tokens[0].get_tag(ScalarDay)
t = Chronic.time_class.new(self.now.year, self.now.month, day.type)

Span.new(t, t + 1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

here same...

rescue ArgumentError => e
raise e unless e.message =~ /out of range/
end

# Handle repeater-month-name/ordinal-day with separator-on
def handle_rmn_od_on(tokens, options)
if tokens.size > 3
Expand Down
14 changes: 12 additions & 2 deletions lib/chronic/parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ class Parser
:guess => true,
:ambiguous_time_range => 6,
:endian_precedence => [:middle, :little],
:ambiguous_year_future_bias => 50
:ambiguous_year_future_bias => 50,
:ambiguous_number_priority => :time
}

attr_accessor :now
Expand Down Expand Up @@ -47,6 +48,9 @@ class Parser
# look x amount of years into the future and past. If the
# two digit year is `now + x years` it's assumed to be the
# future, `now - x years` is assumed to be the past.
# :ambiguous_number_priority - When parsing a number on its own (e.g. "1"),
# should it be treated as a time (1pm) or a date (1st of
# the current month)? Valid values are :time (default) or :date
def initialize(options = {})
@options = DEFAULT_OPTIONS.merge(options)
@now = options.delete(:now) || Chronic.time_class.now
Expand Down Expand Up @@ -162,7 +166,7 @@ def definitions(options = {})
Handler.new([:repeater_day_name, :repeater_month_name, :ordinal_day, :separator_at?, 'time?'], :handle_rdn_rmn_od),
Handler.new([:repeater_day_name, :ordinal_day, :separator_at?, 'time?'], :handle_rdn_od),
Handler.new([:scalar_year, [:separator_slash, :separator_dash], :scalar_month, [:separator_slash, :separator_dash], :scalar_day, :repeater_time, :time_zone], :handle_generic),
Handler.new([:ordinal_day], :handle_generic),
Handler.new([:ordinal_day], :handle_od),
Handler.new([:repeater_month_name, :scalar_day, :scalar_year], :handle_rmn_sd_sy),
Handler.new([:repeater_month_name, :ordinal_day, :scalar_year], :handle_rmn_od_sy),
Handler.new([:repeater_month_name, :scalar_day, :scalar_year, :separator_at?, 'time?'], :handle_rmn_sd_sy),
Expand Down Expand Up @@ -204,6 +208,12 @@ def definitions(options = {})
]
}

if options[:ambiguous_number_priority] == :date
@@definitions[:date] << Handler.new([:scalar_day], :handle_sd)
else
@@definitions[:date] = @@definitions[:date].reject { |l| l.pattern == [:scalar_day] }
Copy link
Collaborator

Choose a reason for hiding this comment

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

this 214 line does nothing, there's no such pattern with just [:scalar_day]

Copy link
Author

Choose a reason for hiding this comment

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

Well, this same commit adds it if : ambiguous_number_priority is :date. I was finding that these definitions seemed to persist between calls, so if you called it once with : ambiguous_number_priority => :date and then later with : ambiguous_number_priority => :time then it would break

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh right, it caches. Actually I think it's not good idea to change @@definitions, it's not thread safe and better would be just parse that option in some handler or something like that.

Copy link
Author

Choose a reason for hiding this comment

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

So maybe have a handle_lonely_int that decides for itself whether to treat it as a day or an hour based upon options?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah that would work I think.

end

endians = [
Handler.new([:scalar_month, [:separator_slash, :separator_dash], :scalar_day, [:separator_slash, :separator_dash], :scalar_year, :separator_at?, 'time?'], :handle_sm_sd_sy),
Handler.new([:scalar_month, [:separator_slash, :separator_dash], :scalar_day, :separator_at?, 'time?'], :handle_sm_sd),
Expand Down
1 change: 1 addition & 0 deletions test/helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
require 'chronic'
end

require 'minitest'
require 'minitest/autorun'

class TestCase < MiniTest::Test
Expand Down
21 changes: 19 additions & 2 deletions test/test_parsing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,26 @@ def test_handle_generic
assert_in_delta time, time2, 0.000001

assert_nil Chronic.parse("1/1/32.1")
end

def test_handle_od
now = Time.new(2013, 8, 1)
time = Chronic.parse("28th", :now => now)
assert_equal Time.new(2013, 8, 28), time

now = Time.new(2013, 9, 1)
time = Chronic.parse("28th", :now => now)
assert_equal Time.new(2013, 9, 28), time
end

def test_handle_sd
now = Time.new(2013, 8, 1)
time = Chronic.parse("28", :now => now, :ambiguous_number_priority => :date)
assert_equal Time.new(2013, 8, 28), time

time = Chronic.parse("28th")
assert_equal Time.new(Time.now.year, Time.now.month, 28), time
now = Time.new(2013, 9, 1)
time = Chronic.parse("28", :now => now, :ambiguous_number_priority => :date)
assert_equal Time.new(2013, 9, 28), time
end

def test_handle_rmn_sd
Expand Down