search

phosphorjs / phosphor

The PhosphorJS Library
BSD 3-Clause "New" or "Revised" License
1.04k stars 166 forks source link

Undo redo #422

Closed ian-r-rose closed 4 years ago

ian-r-rose commented 4 years ago

This implements undo/redo for the datastore.

  1. The message passing interface has been replaced with an IServerAdapter interface, which can receive transactions, as well as requests for undo and redo events. Please let me know if the interfaec seems reasonable.
  2. The datastore now has its own cemetery to keep track of which transactions should be applied. If the undo count is greater than the redo count, they are not applied.
  3. Tables and fields now have the ability to unapply patches as well as apply them. The unapply versions are mostly the reverse of the apply functions.
ian-r-rose commented 4 years ago

The example heroku app here shows the undo/redo in action. Users only undo transactions that they have added themselves (that is, they don't undo things that collaborators have done). The "read only" check box is included in the undo/redo stack.

sccolbert commented 4 years ago

One thing to keep in mind, it's technically impossible to undo a transaction before it is created, since by definition the transaction id would not yet exist. So really, the cemetery is only taking care of concurrent undo of a particular transaction.

vidartf commented 4 years ago

One thing to keep in mind, it's technically impossible to undo a transaction before it is created, since by definition the transaction id would not yet exist. So really, the cemetery is only taking care of concurrent undo of a particular transaction.

What do we do in the case where a network snag causes an undo transaction to arrive before the original transaction message? I don't think it is very likely, but we should at least fail loudly if we are not going to handle that case.

sccolbert commented 4 years ago

What do we do in the case where a network snag causes an undo transaction to arrive before the original transaction message? I don't think it is very likely, but we should at least fail loudly if we are not going to handle that case.

The transaction cemetery will handle that case.

vidartf commented 4 years ago

The transaction cemetery will handle that case.

Then I have no concerns 👍

ian-r-rose commented 4 years ago

Ready for another look-through.