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

Save modifications #14

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Save modifications #14

wants to merge 4 commits into from

Conversation

xeruf
Copy link
Contributor

@xeruf xeruf commented May 25, 2020

Created by @Fongshway - I don't actually know what it does, but I wanted to keep track of modifications others have done in case I pick it up :)

@Fongshway
Copy link

@xerus2000 so from a code perspective, this was more of a POC than anything else, but I cleaned up the spacing to make it clearer what I was trying to play around with. There was some weirdness around spacing vs tabs in the editor I was using, which I addressed in a1b6003.

The two things I was trying to incorporate are:

  • Removing the totalactivetime UDA from taskwarrior-time-tracking-hook.
  • Incorporating handling of scheduled, since I use that as part of my Taskwarrior workflow more broadly, in way that is consistent with how relativeRecurDue and relativeRecurWait are implemented.

@JensErat
Copy link
Owner

I was more like "better dump it on GitHub" when I put the hook together (not much more than a hack originally either), I'm really surprised (and glad) this got quite some attention (and shows where task warrior lacks...).

I'm not sure about the totalactivetime field -- I guess the idea is that if the relative recur hook creates a copy, reset the time calculated by the other hook? The code seems safe even for non-users of the other hook, so why not go for it. I would add some reference though. Did I get it right regarding my comment proposal?

I didn't really know about scheduled so far (just using wait all the time), but the use case and implementation seems very legit. Status waiting is correct anyway?

@JensErat
Copy link
Owner

Especially @xerus2000 your thumbs-up on #12 makes me feel that issue is pretty related to this, does the PR actually resolve #12?

@Fongshway
Copy link

Fongshway commented May 25, 2020

@JensErat well I'm glad you shared it! Definitely useful given the lack of support for this functionality in taskw.

For the totalactivetime field, yes that was the idea and you did get it right in your comment proposal, but I don't actually use that time tracking hook anymore... so I'm okay leaving that out of any patches to this repo. I guess the broader question would be how to support deleting arbitrary task fields on new tasks this hook creates.

As for supporting the scheduled field, I opened #15. Also, the status waiting was incorrect in this branch, so I updated the scheduled behavior to match due in #15.

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.

3 participants