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

CLI: undo argument #505

Open
azureorangexyz opened this issue Oct 6, 2022 · 13 comments
Open

CLI: undo argument #505

azureorangexyz opened this issue Oct 6, 2022 · 13 comments
Labels
contributions welcome Further design discussions and PRs are welcome.

Comments

@azureorangexyz
Copy link

The Documentation sais something about a undo argument for todo:

To operate on a todo, the id is what’s used to reference it. For example, to edit the Buy soy milk task from the example above, the proper command is todo edit 3, or todo undo 3 to un-mark the task as done.

but there seems to be no such options in V4.1.0, and I did not find any in the source code either.

Was this option removed, or is it planned to be implemented?
I would love to have the option to un-mark done tasks (but I do not know any python to do a pull request).

@WhyNotHugo
Copy link
Member

Yeah, apparently it was removed in 2015. I've no idea why it was deprecated, but that happened in 9b863d5.

I honestly haven't needed to use it since then, but I don't think we deliberately planned to drop this command.

@WhyNotHugo
Copy link
Member

You can edit the todo to undo this manually as a workaround.

@azureorangexyz
Copy link
Author

Yeah, apparently it was removed in 2015. I've no idea why it was deprecated, but that happened in 9b863d5.

I honestly haven't needed to use it since then, but I don't think we deliberately planned to drop this command.

Okay, It would have been great to have this feature, as I use todoman with a dmenu script. :)

You can edit the todo to undo this manually as a workaround.

Yes I'll use this feature for now.

If the undo feature isn't coming back, maybe remove the corresponding paragraph from the documentation.

@WhyNotHugo WhyNotHugo added the contributions welcome Further design discussions and PRs are welcome. label Oct 7, 2022
@WhyNotHugo
Copy link
Member

I'll take a PR that implements this. I think we need to update both PROGRESS and STATUS.

@r4ulill0
Copy link
Contributor

Is there any progress on this issue? I have stumbled with the paragraph and undo seems to keep being deprecated.

If there is still interest in implement it, i think i can try it. Seems as an easy task to learn a bit about the project code and the to-do standard.

Is there any dev oriented documentation? It could be handy as I have only found the lifecycle and some tips.

@WhyNotHugo
Copy link
Member

If there is still interest in implement it, i think i can try it. Seems as an easy task to learn a bit about the project code and the to-do standard.

Sure, contributions for this are welcome.

Is there any dev oriented documentation? It could be handy as I have only found the lifecycle and some tips.

todoman/cli.py is the main entry point. You can check def done as a very simple example (that's the function that gets called when someone runs todo done).

I think that instead of todo.complete() this function should do something like todo.status = "IN-PROCESS, todo.percent_complete = 0.

tests/test_cli.py contains tests for these methods, I suggest looking at a few of them (including the ones that call done) as a reference for tests.

@r4ulill0
Copy link
Contributor

I think undo should restore the status to "NEEDS-ACTION" because if I undo a task, it should go back to its initial state. Todo.__init__() doesn't initialize as "IN-PROCESS" and undo shouldn't have any effect on a recently created todo.

My initial approach was to undo everything todo.complete() does even the series management. But I'm not completely sure if deleting the last related todo and restoring its todo.rrule is a good idea because it's not guaranteed that the last entry in todo.related is going to be the recursive one.

Can the user add new related task to an already completed todo?
If so, that one could be after the one created for the series. In that case I guess it is better to keep the series copy without restoring the rrule (because it was already duplicated).

@WhyNotHugo
Copy link
Member

Yup, "NEEDS-ACTION" is actually correct.

We should be able to determine if another todo is the next in the series when all of the following are true:

  • The todo being undone has a related todo.
  • The related todo is not competed.
  • The related todo has an rrule.
  • Applying the rrule to the current todo produces one identical to the related todo (except for the UID).

In this case, it should be safe to delete the related todo (and the relationship to it), since it's clearly the next in the series and has not been touched and we know it will be re-created when completing the current one.

Leaving it untouched should not be terrible though; it's just that both instances will show up now, which will likely be reported as a bug.

Dealing with the recurrence situation can be dealt in a follow-up.

Can the user add new related task to an already completed todo?

Not via todoman, but yes via other tools (or manually editing one).

r4ulill0 added a commit to r4ulill0/todoman that referenced this issue Feb 19, 2023
@r4ulill0
Copy link
Contributor

I have made an implementation but trying to make tests for it I stumbled upon a problem with how todo_factory works. After debugging with pdb and pytest seems like todo_factory doesn't create the related list inside my todo and that mess up all asserts afterward.

I have tried to update the cache but seems like is not an issue on that side.
I suspect that it could be something with how the data is stored in the ics file but I haven't seen any clue in that direction.

https://github.com/r4ulill0/todoman/blob/45444ef8aa49af2b329e580c3f4a9f4231a664c7/tests/test_cli.py#L767-L798

@r4ulill0
Copy link
Contributor

r4ulill0 commented Mar 16, 2023

It has taken a while but solving some problems with my editor, I have found the issue. We were not saving related Todos in our sqlite cache. (So when I try to retrieve information about the related Todo, I can't because all I have is an empty array)

Edit: I think modifying just the cache could not be enough as related Todos will be wiped out when cache refreshes. I'm checking other issues because it could be solved with #288 but i have to check how Akonadi saves recurring todos first.

Moved by curiosity I have checked if any other attribute is missing but seems like it's not the case 🎉.

I have thought about a table (relations) similar to categories to track related Todos. It could be useful in the future if there are more features that depend on how a Todo relates to others.

Having to create a new table, tests and modifications on how we create, modify and retrieve Todos; should I create another issue for it and a separate PR ❓?

I'm pretty bad estimating the scope of it so I would appreciate any thoughts about this.

r4ulill0 added a commit to r4ulill0/todoman that referenced this issue Apr 7, 2023
@r4ulill0
Copy link
Contributor

I'm no longer interested in contributing to this project. If anyone want to continue this issue, feel free to do so.

@WhyNotHugo
Copy link
Member

Sorry to hear that. My sincerest apologies for the frequent delays in reviews/feedback. I'm well aware of how discouraging that can be.

Thank you for your contributions thus far, and best of luck in future.

1 similar comment
@WhyNotHugo
Copy link
Member

Sorry to hear that. My sincerest apologies for the frequent delays in reviews/feedback. I'm well aware of how discouraging that can be.

Thank you for your contributions thus far, and best of luck in future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributions welcome Further design discussions and PRs are welcome.
Projects
None yet
Development

No branches or pull requests

3 participants