samg / timetrap

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

Run custom ruby scripts as post-command hooks #92

Closed logankoester closed 8 years ago

logankoester commented 10 years ago

Inspired by git-hooks, I've added the concept of callback hooks so that timetrap commands can be extended by the end user or through third-party gems.

After any command is finished, if there is a Ruby file at ~/.timetrap/hooks/#{current_sheet}/#{command}.rb, it will be required before the process exits.

I needed this functionality to support my timetrap-hipchat gem, which notifies a chat room when I am billing time and what I'm working on.

hipchat

Would love to see this merged in! Let's discuss :smile_cat:

a-b commented 10 years ago

+1 for hooks just want to make sure that ~/.timetrap/ - based on timetrap configuration and not hardcoded..

samg commented 10 years ago

I like this idea a lot and have considered this kind of feature a few times in the past. I'm wondering why the hooks would be sheet specific? It seems easier to me to have one hook per command which could handle any sheet specific logic internally (e.g. if sheet == 'foo' then ...)

I haven't had a chance yet to review the code or investigate why the build is failing, but I'm interested in pursuing this direction.

Thanks!

logankoester commented 10 years ago

It's been a couple weeks now and my memory is hazy but I believe the build is only failing because some erroneous text is now being written to STDOUT in certain circumstances, and that's breaking the test case matchers.

Should be quick to fix, I'll try to find a minute to take care of.

logankoester commented 10 years ago

@samg As for whether hooks should be sheet-specific, I think it comes down to what kind of use cases we are interested in.

When I use timetrap, I create a sheet for each client or project. I might want hooks to notify a chat room (as above), or to push entries into that client's next invoice.

In such a scenario, hooks seem naturally sheet-specific. That is to say, each sheet has a file tree containing whatever logic and integrations are relevant to that project. Rather than one file with a big hash of settings for lots of unrelated sheets.

If you're more interested in, say, adding growl notification triggers to your timetrap, then yeah, sheet-specific hooks would be awkward and redundant.

Probably the best solution is to implement both. Timetrap could reserve a sheet name like "global", and always trigger hooks under ~/.timetrap/hooks/global/ as well as any hooks specific to the active sheet.

logankoester commented 10 years ago

@a-b To answer your question, yes indeed ~/.timetrap/hooks is based on a new configuration variable ('hooks_path') and is not hardcoded.

See https://github.com/samg/timetrap/pull/92/files#diff-9c13f48a9bf75e206e3e3c5897fc9a4dR44

logankoester commented 10 years ago

Hey @samg, I have now fixed the problems that were causing the build to fail. Good tests! Caught a bug that would have otherwise gone unnoticed for a while.

Please review again at your convenience.