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

feat: add resume and continue commands #720

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

Conversation

xai
Copy link

@xai xai commented Apr 28, 2023

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
Copy link
Contributor

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

@xai
Copy link
Author

xai commented Apr 28, 2023

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
Copy link
Member

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
Copy link
Member

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

@xai
Copy link
Author

xai commented Nov 20, 2023

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
Copy link
Member

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,

  • continue evokes something like "11:00-12:00" → "11:00-now" and
  • no gap evokes rather "11:00-12:00" → "11:00-12:00 + 12:00-now"

for an ongoing task.

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

@xai
Copy link
Author

xai commented Nov 21, 2023

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

@GeraldJansen
Copy link
Contributor

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
Copy link
Member

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
Copy link
Member

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 xai force-pushed the feat/resume branch 3 times, most recently from ea53132 to 0180b08 Compare November 28, 2023 23:28
@xai
Copy link
Author

xai commented Nov 28, 2023

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
Copy link
Author

xai commented Nov 28, 2023

(Sorry, I misclicked and closed accidentally)

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

  • continue removes the end time of the most recent activity
  • resume starts the most recent activity with the current time
  • resume --no-gap starts the most recent activity with its previous end time

I'm looking forward to further suggestions!

@xai
Copy link
Author

xai commented Nov 29, 2023

Amended again to make flake8 happy as well.

Copy link

github-actions bot commented Dec 3, 2023

Automatically generated build artifacts for commit 43f03c1 (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
Copy link
Member

aquaherd commented Mar 3, 2024

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

Support resuming/continuing the last tracked activity.

When `hamster resume` is called, the last tracked activity is
started, i.e., the same activity is started again with the current
start_time. If the `resume` command is called with the option
`--no-gap`, the activity is started again with its previous end_time.

If `hamster continue` is called, the last tracked activity continues to
be tracked, i.e., its end_time is removed and there is no gap in the
time tracking.

Signed-off-by: Olaf Lessenich <[email protected]>
@xai
Copy link
Author

xai commented Mar 20, 2024

@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!

@xai xai changed the title feat: add resume command feat: add resume and continue commands Mar 20, 2024
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.

5 participants