CLI undo edit?

[Neil Williams]

On Wednesday 10 August 2005 2:34 pm, David Hampton wrote:
> On Wed, 2005-08-10 at 12:44 +0100, Neil Williams wrote:
> > In order to do redo properly, the undo operation needs to add itself to
> > the list as an event that includes the state of affected parameter values
> > before the user clicked undo because the *current* state of any entity
> > would not exist in the list, only the previous state.
>
> Not necessarily.  You could code the system such that each item in the
> list contains both the before and after values for the entity.  Undo
> then becomes walking the list backward applying "before" state, and redo
> is walking the list forward applying "after" state.

Let's work from this plan: 
(Some changes from what I might have said previously)
:-)

Change one of the structs slightly:

typedef struct qof_undo_event_t
{
        GNCEngineEventType type;
        Timespec ts;
        GList *entity_list; /**< GList of qof_undo_entity* */
}qof_undo_event;

/** @brief the undo list itself */
// definition within the QofBook opaque struct
        GList *book_undo; /**< GList of qof_undo_event* */
        gint index_position; /**< the current position within the undo list 
within this book*/

//qof_begin_edit modified to accept a parameter name.
//gnc-event.c modified to make an IMPORT event.

1. Open a book, undo list is empty. Calling qof_book_can_undo(book) or 
qof_book_can_redo(book) returns FALSE in both cases.

2. Edit one entity, in qof_begin_edit the value originally read from the book 
is now read from the entity and cached. (in case there is no event).

3. The event is received, the data has been changed, the entry in the undo 
list can now be made, correlating one event to the undo data previously 
cached. The book->index_position variable is checked to be zero and if not, 
the newest undo data is freed (decrementing index_position as it goes) until 
index_position is zero. The event and it's data is now prepended to the undo 
list. 
qof_book_can_undo would now return TRUE. 
qof_book_can_redo still false.

4. At the next qof_begin_edit, the data entered by the user in the first edit 
is cached by qof_begin_edit. When the second event is generated, the cached 
data is prepended to the undo list, book->index_position is again checked.

5. User clicks Undo and QOF increments book->index_position, reads the undo 
data at that position and sets that in the book. 
qof_book_can_undo remains true, 
qof_book_can_redo now returns TRUE also. 
qof_book_can_undo returns false if length == 0 or index_position == length. 
qof_book_can_redo returns false if index_position == 0 or length == 0.

6. User clicks Redo and QOF decrements book->index_position and sets the data 
for that position in the book.

That eliminates the need to set redo data explicitly. The value from the 
latest committed event is always assumed to be the redo value, the NEXT value 
is the undo value.

undo: set position[1], redo set position[0].

If book->index_position is non-zero when the next event is received, undo data 
between book->index_position and zero is freed and book->index_position set 
to zero .

Equally, there will be a check on the total size of the undo list and a 
curtailment of the list when it exceeds the maximum. All undo data for the 
oldest event is freed when the next event is received, if the maximum is 
exceeded.

I can also add a qof_book_clear_undo(QofBook *book); prototype for non-SQL 
backends that need to explicitly clear the undo list upon save?

> > I'd like to check my logic of how this would work - when a user clicks
> > "undo" / presses Ctrl-Z 3 times and then restarts editing, the last two
> > positions in the list need to be freed - but only if the next event is
> > NOT a redo. Yes?
>
> Wouldn't it be the last three positions?

Yes, sorry. Undoing three times would make index_position == 3. Re-editing at 
that point would mean positions 0, 1 and 2 would be invalid and could be 
freed before setting index_position back to zero. qof_book_can_redo would 
therefore return FALSE.

> Undo/redo shouldn't affect the 
> event list.  

OK. So the book keeps track of book->index_position and undo/redo are direct 
calls from the UI to the book, not using events themselves, using the 
prototypes I mentioned before:
void qof_book_undo(QofBook *book);
void qof_book_redo(QofBook *book);

Undo/Redo still need to issue an event in case the undone / redone data is 
actually in a different window - so that would compliment whichever event 
generated the undo data in the first place.

> Any other change discards all events in the list that have 
> been undone but not redone.

Yes, discarding ALL events in the list means that other entities elsewhere in 
the book lose their undo data too if it occurs after the value of 
index_location - I'm pretty sure that is common undo behaviour as those 
entities may be referencing data that has now been undone.

Should the UI be able to detect if the next undo operation would be trivial or 
non-trivial? e.g. if the next undo is an import event, it might be sensible 
to warn the user that a lot of data could change on a single click.

-- 

Neil Williams
=============
http://www.data-freedom.org/
http://www.nosoftwarepatents.com/
http://www.linux.codehelp.co.uk/

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.gnucash.org/pipermail/gnucash-devel/attachments/20050810/08db6993/attachment.bin