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

Improve taskwarrior hooks #329

Closed
wants to merge 6 commits into from
Closed

Conversation

xeruf
Copy link
Contributor

@xeruf xeruf commented May 23, 2020

  • A separate hook for the start command
  • Check if active task is the modified one (multiple tasks may be active in taskwarrior)
  • Respect start time

Fixes GothenburgBitFactory/task-timewarrior-hook#13, GothenburgBitFactory/task-timewarrior-hook#16

Copy link
Member

@lauft lauft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution, please address the comments in the code before this PR can be merged.
And please accept the DCO by signing your commits.

ext/on-modify.timewarrior Outdated Show resolved Hide resolved

# Started task.
if 'start' not in old:
subprocess.call(['timew', 'start', start] + new_tags + [':yes', ':adjust'])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self: Do we really need the :adjust hint here? 🤔

Copy link
Contributor Author

@xeruf xeruf Jun 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need a separate hint: The adjust hint bears the problem that changing a start date towards the past might accidentally delete other intervals, at the same time if you have two intervas back to back and you want to move the start time of the latter one back, it should do that without complaining.

Idea:

  • rename "adjust" to "override"
  • create a new hint called "adjust" that will adjust intervals, but will refuse to delete them in the process
    That seems pretty intuitive to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ext/on-modify.timewarrior Outdated Show resolved Hide resolved
ext/on-modify.timewarrior Outdated Show resolved Hide resolved
ext/on-modify.timewarrior Show resolved Hide resolved
@lauft lauft added the bug Something isn't working label Jun 3, 2020
@xeruf
Copy link
Contributor Author

xeruf commented Jun 27, 2020

I'll look into the signing at some point - currently I am happy if I get even the code to a satisfactory level.

@xeruf
Copy link
Contributor Author

xeruf commented Jun 27, 2020

Looking good, I have even written a test script to verify and document expected functionality.

Implementing the alternate adjust hint specification as commented above would make this perfect as far as I have seen :)

@xeruf
Copy link
Contributor Author

xeruf commented Aug 10, 2020

@lauft would appreciate your stance regarding my suggestion :)

@lauft
Copy link
Member

lauft commented Feb 5, 2023

Migrated to GothenburgBitFactory/task-timewarrior-hook#11

Please continue any discussion of this PR there.

@lauft lauft closed this Feb 5, 2023
@GothenburgBitFactory GothenburgBitFactory locked and limited conversation to collaborators Feb 5, 2023
@lauft lauft removed this from the 1.5.0 milestone Feb 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dev-branch on-modify-hook This issue or pull request concerns the on-modify hook
Projects
None yet
Development

Successfully merging this pull request may close these issues.

on-modify hook ignores task start time
2 participants