Implement "undo"

[djmitche]

This needs some additional thought. It seems like operations would make this easy, but there are a few complicating factors;

• Operations as defined are not reversible (for example an Update operation only gives the new value of the attribute, not the old value)
• A user's notion of "one command" is probably several operations, such as a sequence of Create and Update operations for task add
• Things are a lot more complicated once task sync has occurred

A few thoughts from a walk in the woods a few days ago:

• Let's just not support undoing things that have been sync'd (I believe TaskWarrior doesn't either, but I'm not sure)
• Locally, each replica can store a fuller form of an Operation that includes enough information to reverse it. This information can be discarded during sync and the operations replaced with irreversible operations. This matters for the case where a sync partially completes, rebasing local operations on top of new versions from the server but then failing to add a new version to the server.
• Add "undo points" into the operation stream to mark each point that a user might want to undo back to. So a task add command might add [UndoPoint, Create(..), Update(..), Update(..), Update(..)]. UndoPoints, too, would be discarded during a sync and not sent to the server.

Then task undo just means reversing the most recent operation and deleting it from the list of operations, repeating until an UndoPoint is seen, an irreversible operation is found, or the list of operations is exhausted.

The above might make no sense by the time I or someone else reads this issue, in which case feel free to just ignore.

[djmitche]

I think Undo is typically implemented by creating "inverse" operations and adding those to the list of operations, rather than by removing operations. The relevant paper is here.

After a sync, the "inverse" may not be correct. Imagine, for example, that we create an Update{ property: "description', value: "do the thing", old_value: "do stuff"}, getting "do stuff" from the local task database at the time the update is made. Next, a synchronization occurs and this operation is rebased on top of an operation that sets the description to some other value. Now the reverse operation will set the description back to "do stuff", which is not desired. Fixing the old_value property of updates as they are rebased is difficult, I think.

So, more evidence that we should disallow undos after a sync.

[dbr]

"Undo" as "apply the operation in reverse" matches my experience with undo in things like emacs, git This has the nice effect of undo being an undoable operation, in quite a logical manner:

For example with emacs, you have operations "A, B, C". If you press undo, the operations become "A, B, C, undo to state of B". If you press undo again, another operation is added and merged with the last so you have "A, B, C, undo to state of A". If you perform some other operation "D", you end up with "A, B, C, undo to state of A, D".

It also combines logical groups of operations (e.g inserting 20 characters of text) into one operation (similar to as you describe "task add" maybe being 5 low-level operations)

After a sync, the "inverse" may not be correct

From a users point of view, I would expect task sync && task undo would essentially revert the local replica to the state as it was before the sync. E.g if I edit the description of a task, then sync where there are no other changes occur - the undo wouldn't revert the description change, it would undo the sync operation (even if that is a no-op)

Not sure that is necessarily easy to implement - and disallowing undo after a sync also seems pretty reasonable (although may not play well with sync being run in a cron)

[djmitche]

I wrote up some docs for what I've been thinking of at https://github.com/djmitche/taskchampion/pull/new/issue92

In thinking about how task undo interacts with task sync,

• ta 123 modify +sometag
• (ta sync runs in a crontask)
• ta undo - "oops, I didn't want to add that tag"

The user's expectation is that this undoes the last thing that happened on that replica. But depending on what happened in the sync, that may be very ambiguous. Did the sync add +sometag to the task as well? Did the sync add and then remove that tag? Did the sync delete the task entirely?

To have task undo undo the entire sync operation is also a possibility, but I think a user would find that kind of surprising in this case. And, it's also quite difficult to implement!

I think the least surprising thing to do in this case is to show an error:

$ ta undo
All operations have been synchronized.  There are no local operations to undo.

this also leaves open the possibility of adding functionality there later, if we figure out what to do.

[djmitche]

Neat:

24|{"Update":{"uuid":"662dcaec-260c-459b-912c-036e5c57c91a","property":"modified","old_value":"1639950116","value":"1639950259","timestamp":"2021-12-19T21:44:19.424543015Z"}}
25|{"Update":{"uuid":"662dcaec-260c-459b-912c-036e5c57c91a","property":"tag_eee","old_value":null,"value":"","timestamp":"2021-12-19T21:44:19.434380339Z"}}
26|{"Update":{"uuid":"662dcaec-260c-459b-912c-036e5c57c91a","property":"tag_dddd","old_value":null,"value":"","timestamp":"2021-12-19T21:44:19.444195370Z"}}
27|{"Update":{"uuid":"662dcaec-260c-459b-912c-036e5c57c91a","property":"tag_ddd","old_value":null,"value":"","timestamp":"2021-12-19T21:44:19.457492659Z"}}
28|"UndoPoint"
29|{"Update":{"uuid":"662dcaec-260c-459b-912c-036e5c57c91a","property":"modified","old_value":"1639950259","value":"1639950272","timestamp":"2021-12-19T21:44:32.226777442Z"}}
30|{"Update":{"uuid":"662dcaec-260c-459b-912c-036e5c57c91a","property":"tag_dddd","old_value":"","value":null,"timestamp":"2021-12-19T21:44:32.238414977Z"}}
31|"UndoPoint"
32|{"Update":{"uuid":"662dcaec-260c-459b-912c-036e5c57c91a","property":"modified","old_value":"1639950272","value":"1639950304","timestamp":"2021-12-19T21:45:04.765380077Z"}}
33|{"Update":{"uuid":"662dcaec-260c-459b-912c-036e5c57c91a","property":"tag_dddd","old_value":null,"value":null,"timestamp":"2021-12-19T21:45:04.774410873Z"}}