projecthamster / hamster

GNOME time tracker
http://projecthamster.org
GNU General Public License v3.0
1.08k stars 249 forks source link

feat: add resume and continue commands #720

Open xai opened 1 year ago

xai commented 1 year ago

Support resuming the last tracked activity.

When hamster resume is called, the last tracked activity is started again with the current start_time.

If hamster resume --continuous (or short with -c) is called, the last tracked activity continues to be tracked, i.e., its end time is removed.

GeraldJansen commented 1 year ago

In the case of --continuous, it might make more sense to simply update the last task removing the end time.

xai commented 1 year ago

In the case of --continuous, it might make more sense to simply update the last task removing the end time.

Great idea, I adjusted my commit accordingly.

DirkHoffmann commented 1 year ago

Linguistically, I would rather go for --continue (or --pick-up in a more common way) than --continuous.

Some native speaker's view on this?

aquaherd commented 1 year ago

Not a native speaker either but how about hamster resume --no-gap?

xai commented 1 year ago

Sue, I can rebase and change the naming. I like hamster resume --no-gap, as this is exactly what it does. Would that be ok with you?

DirkHoffmann commented 1 year ago

Sue, I can rebase and change the naming. I like hamster resume --no-gap, as this is exactly what it does. Would that be ok with you?

I would still be in favour of --continue, even more because we decided to continue the previous entry by removing its end time rather than making a new one. For me,

for an ongoing task.

But we can also wait, until I asked a native speaker today.

xai commented 1 year ago

Thanks, sounds good! Let me know when you have decided and I should rebase!

GeraldJansen commented 1 year ago

I think separate actions, hamster continue and hamster resume would be more convenient and would be easier to document.

The command line documentation, ie. hamster help, should be updated.

Also, the list of options in hamster.bash should be updated, i.e. `opts="activities categories current export list search start stop ".

matthijskooijman commented 1 year ago

I think separate actions, hamster continue and hamster resume would be more convenient and would be easier to document.

Sounds reasonable, though I would not be in favor of "continue" and "resume", since they really sound like they are the same thing.

Maybe "restart" is an option for the with-gap case?

DirkHoffmann commented 1 year ago

After consultation, resume --continue would be best choice (semantically).

But I am actually more with @GeraldJansen's proposal for separate hamster continue and hamster resume commands instead of the optional parameter. I don't think that they are very different and the CLI is probably (?) not used directly by a majority of users. So it is rather a matter of documenting it correctly for those who write other interfaces.

The only argument to keep hamster resume --continue rather than hamster continue might be that they are pretty much too similar to appear as two distinct commands. (I don't know, if there are similar precedences for other commands.)

Anyway, I think the discussion went long enough. (Thanks for all the input!) And the person who actually implements the feature should be able now to make a sensible choice.

xai commented 12 months ago

Thanks for all that valuable feedback! I understand the different viewpoints and would like to make everyone happy because changes to the command line interface should indeed be thought through well ;)

So I figure that continue and resume should be separate commands because they do different things, but that a resume --no-gap might still be interesting to start the most recent activity from its previous end time.

I will further revise my PR to incorporate all your feedback!

xai commented 12 months ago

(Sorry, I misclicked and closed accidentally)

Now, we should have all three behaviors implemented and documented:

I'm looking forward to further suggestions!

xai commented 12 months ago

Amended again to make flake8 happy as well.

github-actions[bot] commented 11 months ago

Automatically generated build artifacts for commit 43f03c19a5e63a8f541cc5d772f2b6c4fb8550bc (note: these links will expire after some time):

To test this PR, download and unzip the flatpak application above and then install with:

flatpak install Hamster.flatpak
aquaherd commented 8 months ago

@xai Sorry to be a bother but could you please add your new commands to the bash completion?

xai commented 8 months ago

@xai Sorry to be a bother but could you please add your new commands to the bash completion?

No problem at all! I now also complete the --no-gap of the resume command (the commands themselves were already being completed).

After rebasing to the current master, I re-tested my commands (i.e., continue, resume, and resume --no-gap) and found that I had to add a fix for hamster continue to actually set the current activity by setting the end_time to null explicitly. Just calling __touch_fact() with a None end_time resulted in an updated end_time but not in active tracking. Therefore, I now added __continue_fact() in db.py and call that instead of __touch_fact().

Let me know if there is something else I can improve, @aquaherd!