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

Add date argument to log command #86

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

teddywing
Copy link
Contributor

@teddywing teddywing commented Jan 28, 2018

Adds an optional date argument to the log command that can be
specified just before the task alias.

Some examples from @skoppelmanCC:

hcl log yesterday @documentation +2 Ran spellcheck
hcl log 3 days ago @wireframes +3.5
hcl log last friday @admin +1 filed timesheets

command_test.rb(#test_start):
Update expected method call arguments with spent_at. Now that we're
passing in spent_at, the expected arguments should include it.

utility_test.rb: Add tests for #get_date

Fixes #83.

@teddywing
Copy link
Contributor Author

Huh, just realised this doesn't actually work correctly. I had been testing with 'yesterday', but that was just a fluke because the stop command handles both today and yesterday (

entry = DayEntry.with_timer(http) || DayEntry.with_timer(http, Date.today - 1)
).

Assuming I need to get DayEntry.with_timer to use the provided date in order to get this to work correctly.

Adds an optional date argument to the `log` command that can be
specified just before the task alias.

Some examples from @skoppelmanCC:

    hcl log yesterday @documentation +2 Ran spellcheck
    hcl log 3 days ago @wireframes +3.5
    hcl log last friday @admin +1 filed timesheets

Commands#log:
Copy over the contents of `Commands#start` and replace `task.start` with
`task.add`. This allows us to log the time without actually starting a
timer (and thus having to subsequently stop that timer).

command_test.rb(#test_start):
Update expected method call arguments with `spent_at`. Now that we're
passing in `spent_at`, the expected arguments should include it.

command_test.rb(#test_log_failure):
Delete this test because I've rewritten the `Commands#log` method to no
longer use timers.

utility_test.rb: Add tests for `#get_date`

Fixes zenhob#83.
@teddywing teddywing force-pushed the log-for-past-days--squashed branch from 5045d9a to 873325e Compare January 31, 2018 19:53
@teddywing
Copy link
Contributor Author

Came up with a new mechanism for doing logging and handling past dates. This duplicates the work of Commands#start while introducing a way to parse a date as an optional first argument, and instead of calling task.start, it calls task.add. This enables the time to be logged on the specified date without ever having to deal with stopping timers.

With this latest change, I've duplicated most of the contents of Commands#start, and haven't figured out a good strategy to eliminate that duplication. If that's a concern, I'd appreciate any ideas to simplify the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant