ActiveSupport::TimeWithZone compatbility

[tatey]

This patch relaxes the type checking when parsing an explicit time object to Nickel. If that time object happens to be an instance of ActiveSupport::TimeWithZone then nickel parses the query string instead of blowing up.

Without the patch.

require 'nickel'
require 'active_support/time'
Time.zone = 'Sydney'
Nickel::InvalidDateTimeError: Nickel::InvalidDateTimeError
  from /Users/Tate/Desktop/nickel-0.1.5/lib/nickel/nlp.rb:15:in `initialize'
  from /Users/Tate/Desktop/nickel-0.1.5/lib/nickel.rb:13:in `new'
  from /Users/Tate/Desktop/nickel-0.1.5/lib/nickel.rb:13:in `parse'
  from (irb):11
  from /Users/Tate/.homebrew/opt/rbenv/versions/2.1.2/bin/irb:11:in `<main>'

With the patch.

require 'nickel'
require 'active_support/time'
Time.zone = 'Sydney'
Nickel.parse('6pm tomorrow', Time.current) # => message: "", occurrences: [#<Occurrence type: single, start_date: 20140716, start_time: 180000>]

We need this patch because our client posts to the server with a UNIX timestamp (Eg: 1405065600) and timezone (Eg: "Sydney") for each request.

Any feedback is welcome.

[iainbeeston]

Hey thanks for the pull request. That's an annoying bug.

But can I suggest something even more dramatic? Could you remove the fail statement completely? Type detection is an enormous code-smell, I hadn't noticed that in the codebase

[iainbeeston]

(Also, raising that error doesn't seem to do anything useful - it'd fail on the strftime call anyway if it's not defined. Nickel should be duck typing, not pretending to be statically typed)

[tatey]

Thanks for the fast response.

I wasn't sure how the patch would go down so I opted to preserve the existing behaviour. Very happy to remove the fail altogether.

Would you like me to rebase into a single commit?

[iainbeeston]

Sure, rebasing would be great, thanks.

I'll update and publish the gem once I get the new version of the pull request.

[iainbeeston]

0.1.6 of the gem has your change

[tatey]

Thanks a tonne!

On 15 Jul 2014, at 8:48 pm, Iain Beeston notifications@github.com wrote:

0.1.6 of the gem has your change

— Reply to this email directly or view it on GitHub.