CLI undo edit?

[Derek Atkins]

Do you really need to keep track of the operations individually?
I would think a much better API would be:

  QofOperation qof_book_start_operation(QofBook* book, const char *op_name);
  void qof_book_end_operation(QofOperation oper);

This lets the operation itself define the title of the operation,
let's the application define as many operations as you want...
And the op_name can even be a translatable string!

I see no need for a enumeration of event types like you have.  Why
do you think you need that?  These aren't really events, these
are begin/end wrappers around UI operations, which is only used
by the undo/redo subsystem.

At least that's my take on it all.

-derek

Neil Williams <linux at codehelp.co.uk> writes:

> On Wednesday 10 August 2005 8:49 am, Derek Atkins wrote:
>> Neil Williams <linux at codehelp.co.uk> writes:
>> > For now, I'm afraid, I have to step back and keep to what I can
>> > realistically hope to complete.
>
> Just revisiting this for the limited CLI version:
>
>> Also, I think the right way to go about it is not a list of changed
>> entities, but a list of events/operations.
>
> The existing events, I feel, are largely inappropriate for undo. What a user 
> would consider a single operation can consist of many engine events. So in 
> the CLI I'm experimenting with a wrapper that UI developers can use to mark 
> the extent of a 'user operation'. A kind of superclass of begin_edit and 
> commit_edit, it would be 
>
> qof_book_operation(book, QOF_ENTITY_MODIFY)
> /* code that generates engine events or modifies entities */
> qof_book_operation(book, QOF_OP_END);
> for a small operation (like editing the name of an Account) to:
>
> qof_book_operation(book, QOF_BOOK_IMPORT)
> /* code */
> qof_book_operation(book, QOF_OP_END);
> for an entire import.
>
> These calls would be almost exclusively within the UI code, rather than the 
> objects which currently generate a lot of the events.
>
> This could work when engine events are suspended too. It would, however, 
> require additions to each part of the UI that wants to support undo for it's 
> activity. A simple two line addition, but I see no other way of QOF 
> determining what the *user* would understand as a single operation compared 
> to what the engine considers as an event. As far as QOF is concerned, merging 
> two books is the same sequence of events as individually altering each entity 
> in turn - just a whole lot faster.
>
> This also gives UI developers fine control over how the undo list works for 
> their contexts.
>
> It could even give David the chance to explain how many changes have not been 
> saved and what kind of changes those are, in addition to how long ago the 
> first change was made.
>
> Provisional operation types:
> 	QOF_OP_NONE = 0,   /**< uninitialized value */
> 	QOF_OP_END,        /**< Mark the end of the operation. */
> 	QOF_ENTITY_CREATE, /**< A new entity is to be created in the book. */
> 	QOF_ENTITY_DELETE, /**< An existing entity is to be removed from the book. */
> 	QOF_ENTITY_MODIFY, /**< An existing entity is to be modified. */
> 	QOF_BOOK_MERGE,    /**< A book is to be merged into this book. */
> 	QOF_BOOK_IMPORT,   /**< A book is to be imported into this book. */
>
> What others?
>
> (and are merge and import that different in terms of undo ?)
>
> The operations would always be considered from the user's perspective. i.e. a 
> create operation is where the user adds a new entity. Naturally, on undo, 
> this would be reversed and the entity freed but it is still a CREATE 
> operation.
>
> In line with the rest of the undo, this is all centred in the book. (Well, it 
> will be, in this experimental stage it's only based in the CLI context or in 
> statics!)
>
>> Then you can undo/redo per 
>> operation.  Each operation can have a list of changed entities.
>> Therefore, an "import" is a single operation (so it's a single
>> undo/redo entry in the undo/list) but it can have as many entries as
>> necessary in the changed-entries list.
>
> That will be supported because the operation itself is a GList of entity 
> changes.
>
>>  Import - [obj1, obj2, obj3, obj4, obj4]
>
> Does any process need to know *which* objects are involved in any one 
> operation like that (beyond the undo code)?
>
>>  New
>>  Acct  -- [ acct ]
>
> I'm considering implementing this as:
>
> QofOperationType  QOF_ENTITY_CREATE
> 	GList entity_changes
> 		{ QofParam, GUID, parameter_value_as_string, ent->e_type }
>
> I'd use the operation type to determine, e.g. if the entity should be deleted 
> on undo and created on redo (or vice versa). A simple QOF_ENTITY_MODIFY would 
> simply set the parameter (or fail if no entity exists but that would indicate 
> a more severe failure elsewhere).
>
> Initially, I won't be supporting full-scale import undo because there are 
> simply too many entity changes possible.
>
> Are we sure we want to support entire import operations in the full sized QOF 
> undo? Should the undo list size be flexible according to the type / size of 
> the operations recorded? After all, a sequence of imports, merges and other 
> "large workload" operations could tie up a significant amount of memory for 
> the undo list.
>
> 300 entity level operations, 30 book level (3?). 
>
> If we support 300 QOF_ENTITY_MODIFY operations that is radically different to 
> supporting 300 QOF_BOOK_IMPORT or QOF_BOOK_MERGE operations (each of which 
> could include lists of hundreds of entity changes). I'm not sure if we want 
> to see processes carrying an undo list larger than the original book!
>
> Should some operations be specifically excluded from undo?
>
> (I'll certainly be doing that in the CLI, at least in early versions.)
>
>> I don't expect this anytime soon; I'd much rather see g2 get done and
>> out the door first ;)
>
>> > I will use some low-level form of this in the CLI - a single 
>> > entity undo list in a CLI sub-shell that should form the basis of the full 
>> > QOF version. Probably just an implementation of qof_undo_entity that 
>> > bypasses  
>> > the event system and works solely within the CLI shell code.
>
> The CLI does need some rudimentary undo capability to be user-friendly and to 
> help me with it's original purpose - testing and debugging the code I've 
> already put into QOF.
>
> -- 
>
> Neil Williams
> =============
> http://www.data-freedom.org/
> http://www.nosoftwarepatents.com/
> http://www.linux.codehelp.co.uk/
>
> _______________________________________________
> gnucash-devel mailing list
> gnucash-devel at gnucash.org
> https://lists.gnucash.org/mailman/listinfo/gnucash-devel

-- 
       Derek Atkins, SB '93 MIT EE, SM '95 MIT Media Laboratory
       Member, MIT Student Information Processing Board  (SIPB)
       URL: http://web.mit.edu/warlord/    PP-ASEL-IA     N1NWH
       warlord at MIT.EDU                        PGP key available