dominikbraun / timetrace

A simple CLI for tracking your working time.
Apache License 2.0
700 stars 76 forks source link

Proposal: Add `notify` command #91

Open FelixTheodor opened 3 years ago

FelixTheodor commented 3 years ago

I just stumbled across this project: https://github.com/gen2brain/beeep It makes sending notifications to the user very easy. Then I had an idea - my main concern with time-tracking software is that I always forget the tracking when I'm in the middle of something. Therefore, it could be a useful feature for timetrace if the user can set a timer to get a notification when the current task he is tracking is running for x minutes.

I already implemented a first version of this and opened a WIP-Pull-Request for it.

You can start a record and then run timetrace notify 5m & (the &-symbol makes the process run in background). As soon as the task is running for 5 minutes, a notification should pop up and the notify process finishes. If the tracking is stopped before 5 minutes are over, no notification should be send. On my linux system, this already works fine.

What do you think of this idea and the first implementation, @dominikbraun ?

dominikbraun commented 3 years ago

I really like the idea! Didn't have the time to view the implementation yet, but it sounds pretty good.

There are at least some requested features that make sense but shouldn't go into the timetrace binary directly, for example #41. This is why I'm planning to support extensions for timetrace (though I'm not yet sure which mechanism I'm going to choose for this). Maybe the notify feature would be a candidate for such an extension as well?

dominikbraun commented 3 years ago

@aligator Any updates on the extensions architecture?

FelixTheodor commented 3 years ago

Glad that you like the idea!

This feature can of course be a candidate for an extension, so if you think that's the most clear way to do it we should go for it :) However, I'm curios about your reasoning - #41 seems to change a lot of things under the hood for already existing commands, so I can see why it works better as an extension. But the notify command would just add functionality "outside" of the existing code. Is it because of the dependency to another library?

dominikbraun commented 3 years ago

However, I'm curios about your reasoning - #41 seems to change a lot of things under the hood for already existing commands, so I can see why it works better as an extension. But the notify command would just add functionality "outside" of the existing code. Is it because of the dependency to another library?

In general, I'm not against including the notification feature in the timetrace binary. The idea more was that if there's an easy way to provide extensions, all functionality that goes beyond the stateless, fire-and-forget timetrace command could be provided as such an extension. But the notify command indeed is a feature relevant for a majority of users, so whether it should be directly included in the binary is rather a matter of taste.

FelixTheodor commented 3 years ago

Thanks for the explanation!

Since I'm curious how extensions would look like on a technical level, I won't mind to provide this feature as one. I agree that its a matter of taste. On the long run, one could even take a look at download statistics of the extension and decide based on that if something is popular or important enough to go into the main binary :)

aligator commented 3 years ago

@aligator Any updates on the extensions architecture?

As I have currently broken internet, I did not do much.
What I did so far is using timetrace as lib and implement the filesystem interface by a fake-clockify filesystem. Some parts I had to alter in timetrace to make this work.

But this approach has the downside, that you have to create a new timetrace binary for each combination of plugins. I don't think thats feasible when we want to support several plugins. (BTW. Caddy did it this way but there is also a very complex build system around it.)

Another possibility is to create a small API (+maybe a lib to call it easily), create an extra binary for each plugin which calls that API and then let timetrace automatically start all binaries from the plugin folder. These would have to communicate with timetrace over the api.
As timetrace is only a short running program, I am not sure if this would start fast enough to work nice as there has to be started a webserver somehow (or similar). Is there any easy to use IPC (Inter Process Communication) for Go out there?

Also possible would be to call these binaries and pass information by piping it to the stdin of it and getting information by reading the stdout of it.

The latter seems to be the easiest way. But I haven't tried something like this yet. The commands could just be sent end received using json strings. Maybe I do some experiments in this area soon :-)

What do other Go projects use? Is there maybe already a lib for Go IPC?

aligator commented 3 years ago

Just found https://github.com/hashicorp/go-plugin Seems to be stable and widely used. But I am not sure how good it works with a one-shot comand such as timetrace.

The idea with stdin / stdout will only work with plugins which live only as long as the timetrace comand runs. So in case of the notification timer it's maybe not the best way. Edit: Just looked over your initial implementation of the notification. It could work with the stdin / stdout approach, if we allow plugins to prevent timetrace from stopping execution...

dominikbraun commented 3 years ago

BTW, kubectl plugins could be an interesting take as well: https://kubernetes.io/docs/tasks/extend-kubectl/kubectl-plugins/

aligator commented 3 years ago

I am playing around now with creating a POC plugin framework based on the stdin / stdout.

aligator commented 3 years ago

Here the POC https://github.com/aligator/goplug Currently you can provide commands the Plugins can call and also send events, the plugins can listen to.

To use goplug you basically have to create a struct which inherits goplug.Plugin and add Methods which just use RegisterCommand or Send to provide custom commands or events.

Then the plugin just has to create an instance of this struct and call these methods. It's very basic for now, but my concept should be clear.

aligator commented 3 years ago

@dominikbraun What do you think about #104?

dominikbraun commented 3 years ago

@aligator Thanks for your effort, I'm going to take a look at it.

sfrique commented 3 years ago

@FelixTheodor what do you think of adding also a notification if no tracking is happening?

So we would have a flag or something to also notify after a period of nothing been tracked. I am saying this cause I often forgot to START to track something rather than stoping it.

So we could have 3 ways:

I am not sure if this should be implemented inside the binary or a third party/plugin that could also have systray for visual information as well. But initially, having a simple notify can help =]

What do you think?

FelixTheodor commented 3 years ago

@sfrique I do like the idea! But I think it would then probably require a lot more work, especially if we want a systray and probably automatic start on start-up (I guess, otherwise the feature would not make that much sense if you still need to remember to start the reminder^^)? And the extension would need to run in background all the time. At the current state, the notify command is still running completely in foreground and it's just hidden by the &. I will first focus on getting this simple notify to work via Plug-Ins, but in the future it could be a fun side-project to build a full notification application for timetrace :)