samg / timetrap

Simple command line timetracker
http://rubygems.org/gems/timetrap
Other
1.49k stars 116 forks source link

--at switch not synching to my timezone #47

Closed wulftone closed 12 years ago

wulftone commented 12 years ago

Heya, I love this gem!

However, I'm having an issue when I clock in, then clock out with an --at. It oftentimes adds an extra 24 hours because it thinks that it's at GMT, so I have to specify that it is still today where I am by putting the date into the command This is, as you might imagine, a bit cumbersome.

It may also occur during editing.

I'm running ubuntu and have my timezone set to Pacific.

Thanks!

samg commented 12 years ago

@wulftone - yeah that does sound annoying. All the natural language time parsing is done by the chronic gem, but it does have an option (:context) for whether it should assume that (ambiguous) times are in the past or future (http://rubydoc.info/gems/chronic/0.6.6/Chronic.parse). It appears to default to future, which in timetrap's case may not make a lot of sense.

I wonder if this should be made configurable through timetrap, or if it timetrap should just set the :context to past.

samg commented 12 years ago

I might have spoken too soon. Playing around with Chronic in irb it seems that option only impacts ambiguous dates, (e.g. "Jan 1") and not times ("11am").

>> Time.now
=> Wed Dec 28 22:44:16 -0800 2011
>> Chronic.parse("10pm")
=> Wed Dec 28 22:00:00 -0800 2011
>> Chronic.parse("1pm")
=> Wed Dec 28 13:00:00 -0800 2011
>> Chronic.parse("1am")
=> Wed Dec 28 01:00:00 -0800 2011
>> Chronic.parse("1am", :context => :future)
=> Wed Dec 28 01:00:00 -0800 2011
>> Chronic.parse("12am", :context => :future)
=> Wed Dec 28 00:00:00 -0800 2011
>> Chronic.parse("12am today", :context => :future)
=> nil
>> Chronic.parse("today 12am", :context => :future)
=> Wed Dec 28 00:00:00 -0800 2011
>> Chronic.parse("tomorrow 12am", :context => :future)
=> Thu Dec 29 00:00:00 -0800 2011
>> Chronic.parse("1/1", :context => :future)
=> Tue Jan 16 12:00:00 -0800 2001
>> Chronic.parse("Jan 1", :context => :future)
=> Sun Jan 01 12:00:00 -0800 2012
>> Chronic.parse("Jan 1", :context => :past)
=> Sat Jan 01 12:00:00 -0800 2011
>> Chronic.parse("Jan 1", :context => :future)
=> Sun Jan 01 12:00:00 -0800 2012
>> 

So a past context probably still makes sense for timetrap, but I'm not sure it will solve your problem. I'm also not sure why it's interpreting your times as GMT.

wulftone commented 12 years ago

Just for documentation purposes, here's a real example with a new sheet of what happens. I clocked in at 11:52 and it got the time and date correct:

trevor@wulf ~ $ t d
Timesheet: timeTest
    Day                Start      End        Duration   Notes
    Thu Dec 29, 2011   11:52:37 -            1:10:45    
                                             1:10:45
    ---------------------------------------------------------
    Total                                    1:10:45
trevor@wulf ~ $ t o --at 13:00
Checked out of sheet "timeTest".
trevor@wulf ~ $ t d
Timesheet: timeTest
    Day                Start      End        Duration   Notes
    Thu Dec 29, 2011   11:52:37 - 13:00:00  25:07:23    
                                            25:07:23
    ---------------------------------------------------------
    Total                                   25:07:23
trevor@wulf ~ $ date
Thu Dec 29 13:05:26 PST 2011
trevor@wulf ~ $ irb
ruby-1.9.2-p290 :001 > Time.now
2011-12-29 13:05:56 -0800
ruby-1.9.2-p290 :003 > require 'chronic'
true
ruby-1.9.2-p290 :004 > Chronic.parse('now')
2011-12-29 13:06:15 -0800
ruby-1.9.2-p290 :005 > Chronic.parse('today')
2011-12-29 18:30:00 -0800
ruby-1.9.2-p290 :006 > Chronic.parse('13:00')
2011-12-30 13:00:00 -0800
ruby-1.9.2-p290 :007 > Chronic.parse('13:00 today')
2011-12-29 13:00:00 -0800

It clocked in at the right time, but clocked out 24 hours ahead.

Interestingly, using non-military time works just fine:

trevor@wulf ~ $ t i --at 2pm
Checked into sheet "timeTest".
trevor@wulf ~ $ t d
Timesheet: timeTest
    Day                Start      End        Duration   Notes
    Thu Dec 29, 2011   11:52:37 - 13:00:00  25:07:23    
                       14:00:00 -            1:39:51    
                                            26:47:14
    ---------------------------------------------------------
    Total                                   26:47:14
trevor@wulf ~ $ t o --at 3pm
Checked out of sheet "timeTest".
trevor@wulf ~ $ t d
Timesheet: timeTest
    Day                Start      End        Duration   Notes
    Thu Dec 29, 2011   11:52:37 - 13:00:00  25:07:23    
                       14:00:00 - 15:00:00   1:00:00    
                                            26:07:23
    ---------------------------------------------------------
    Total                                   26:07:23
trevor@wulf ~ $ t i --at 15:01
Checked into sheet "timeTest".
trevor@wulf ~ $ t d
Timesheet: timeTest
    Day                Start      End        Duration   Notes
    Thu Dec 29, 2011   11:52:37 - 13:00:00  25:07:23    
                       14:00:00 - 15:00:00   1:00:00    
                                            26:07:23
    Fri Dec 30, 2011   15:01:00 -          -24:40:19    
                                           -24:40:19
    ---------------------------------------------------------
    Total                                    2:47:42

I opened an issue with the chronic gem: https://github.com/mojombo/chronic/issues/76

samg commented 12 years ago

I just pushed gem version 1.7.8, which I think will improve this behavior for you.

Now when you modify an entry's end time (via t out or t edit) it will use the entry's start time as the :now context instead of the current time. This will have the effect that "ambiguous" times like 13:30 will assume you mean the first 13:30 after the start time, instead of after the current time. This means it will be much more difficult to accidentally create a >24 hour entry.

It won't solve the issue with t i --at 15:01. I'll have to think a bit more about what the correct behavior is there. Setting the context to :past will make it difficult to check in at times in the near future. It might make sense to run both contexts and pick the one that is closest to Time.now.

Anyway hopefully this release improves things a bit.

samg commented 12 years ago

I just pushed another patch version (1.7.9) which improves that start time handling. When you specify a time like "16:30" it will now assume you mean the closest time to now, not the next one in the future.

Cheers

wulftone commented 12 years ago

awesome! thanks. It appears to work just fine now.