CLI undo edit?

classic Classic list List threaded Threaded
24 messages Options
12
Reply | Threaded
Open this post in threaded view
|

CLI undo edit?

Neil Williams-2
When editing an instance in the CLI, there is the option to print the instance
(to the terminal) at any point - to confirm what has and has not been
changed.

Once the edit is complete, any other instance can also be selected and
printed, re-edited or deleted.

There is also the option to merge an existing file into the current book.

Adding a new instance automatically moves into an edit sub-shell using the new
instance.

Once edited, the user still has the option to write the data out to a file
(either new or the original) or to quit without saving (with a prompt if the
book is dirty). Once GnuCash v2 XML is replaced, this would only apply to QSF
data.

Is it necessary to offer an undo at the edit stage or should edits take place
immediately (as in the GUI)?

Undo isn't a trivial addition so I wanted to check it was worthwhile - I can
see how to implement it for QSF but the GnuCash v2 XML is a touch more
awkward.

(The CLI commands, options and shells will also be included in PilotQOF and
any other QOF CLI programs.)

--

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


_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

attachment0 (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: CLI undo edit?

Derek Atkins
If QOF supported a native "undo" that would be REALLY COOL..  In fact,
supported a "list" of undo operations would be even cooler!

:)

-derek

Neil Williams <[hidden email]> writes:

> Is it necessary to offer an undo at the edit stage or should edits take place
> immediately (as in the GUI)?
>
> Undo isn't a trivial addition so I wanted to check it was worthwhile - I can
> see how to implement it for QSF but the GnuCash v2 XML is a touch more
> awkward.
>
> (The CLI commands, options and shells will also be included in PilotQOF and
> any other QOF CLI programs.)

--
       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
       [hidden email]                        PGP key available
_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: CLI undo edit?

Neil Williams-2
On Saturday 06 August 2005 9:36 pm, Derek Atkins wrote:
> If QOF supported a native "undo" that would be REALLY COOL..  In fact,
> supported a "list" of undo operations would be even cooler!
>
> :)

(well, I did ask!)

My ideas on how that might be best achieved: (as expected, quite long!)

If it's limited to one undo operation per entity then a hashtable indexed on
the GUID could work. That's the first option, not perfect but probably quite
fast. A second change in the same entity would overwrite any previous and
undoing changes would need to be done as a block. That's more of a confirm
and commit model - like the merge, keep all changes back until a commit is
executed. Not particularly good for undo, me thinks.

A list would be more flexible - undo lists have to be sequential so a generic
qof_undo() would need a single GList of all changes and rolling such a list
in strict sequential order back and forth (i.e. a redo as well as undo) is
far faster than iterating / searching.

I'm presuming the list would be freed only when the book is saved? Therefore a
job for the backend? I think QSF could do that, not sure about the GnuCash
XML v2. (What happens with a SQL backend?)

The question is whether to store the entire entity or just parameter values.
If only changes, is the overhead of identifying which bits have changed more
than the overhead of storing the entity itself?

Isn't this tying back into the whole SQL identification_of_changes idea? Can
those be combined? QOF creates the undo list via the instance dirty routines
and the SQL backend reads that to identify what changes need to be committed?
(and then clears it at some point or keeps a configurable number of events on
the list - more like a stack?)

It may be handy to only store changes, but that means passing the parameter
name into the instance_dirty routines (which will retrieve the value as a
string).

void qof_instance_make_dirty(QofInstance *inst, const char *param_name);

(QOF can't undo / redo anything unless the data is accessible as a parameter
with a QofAccessFunc *and* a QofSetterFunc so passing the value as a string
is pointless. The combination of param_name and the object e_type from the
instance do the rest.)

I could do a qof_undo struct that contains the QofParam and a gpointer of the
previous value of that parameter along with the type and GUID of the entity
concerned?

struct qof_undo
{
        QofParam *param; /**< static anyway so only store a pointer */
        gpointer value  /**< Duplicated? (and freed later) */
        GUID guid;
        QofIdType type; /**< ditto param, static. */
};

Then make a GList of qof_undo* 's.

The gpointer value might just as easily be a string as QOF is already used to
converting each type to and from strings. The guid may be more easily stored
as a string too. So then the entire undo list consists of repeated structs
consisting only of two pointers and two strings. Would those go into the
cache?

A Timespec in qof_undo would solve David's request for a time since first
change too. Simply read the first value in the GList and return ts or
ts.tv_sec.

/** @brief Built into a GList for qof_undo() and qof_redo() */
typedef struct qof_undo_t
{
        QofParam *param; /**< static anyway so only store a pointer */
        char *value; /**< cached string? */
        char *guid_as_string; /**< cached string? */
        QofIdType type; /**< ditto param, static. */
        Timespec ts;
}qof_undo;

/** @brief rolls back one change at a time */
void qof_undo(void);
/** @brief re-does one undo at a time */
void qof_redo(void);

Each would return silently if the end of the list is reached in the
appropriate direction. Maybe they should return gboolean so that the UI can
tell if more steps are available? Or should that be a separate function:
gboolean qof_can_undo(); gboolean qof_can_redo();
Both would return FALSE after a save operation.

What about created / deleted entities? Some kind of enum as to the state of
the entity perhaps, store each value and then store a value which denotes
that the entity must be recreated. This would be read before the parameter
values and would be interpreted as CREATE for undo (later in the list so read
first) and DELETE for redo (reverse direction, parameters stored first then
delete). Dirty instance calls would simply append their data to the list. Too
much overhead? (ISTR prepend is faster so maybe those would be reversed.)

qof_undo would interpret a CREATE as a special case that re-instated the
entity plus all it's parameters in one undo operation - maybe by using the
Timespec to identify how far back to roll the list, all parameter values of
the entity to be deleted being given the same Timespec.

Currently, replaying the logs is used for a crash situation (amongst others) -
are we looking for some form of storage for the undo list? That, AFAICT,
would be unusual for an application - most accept a loss of data upon a crash
- and would be a worthy feather in the cap for GnuCash data integrity. Would
the logs simply use the qof_undo data as well as / instead of their current
data source, essentially providing an offline storage for the qof_undo data
(written in the same as-we-go-until-we-crash manner as the current logs)? By
using only strings, the log procedure could be relatively simple and possibly
easier to implement as a data recovery method.

(I said it was non-trivial!)

Oh, and for this to work, gncCommodity is really going to have to be made
available to QOF! (along with all other data sources that users may need to
use in undo - if it isn't a QofObject with QofClass set and get parameters,
it cannot be in a QOF undo/redo list).

This is well beyond what can be achieved (and tested) before G2 is released.
Something for QOF 0.7 & G2.1 perhaps.

--

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


_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

attachment0 (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: CLI undo edit?

Neil Williams-2
On Sunday 07 August 2005 12:04 am, Neil Williams wrote:

> It may be handy to only store changes, but that means passing the parameter
> name into the instance_dirty routines (which will retrieve the value as a
> string).
>
> void qof_instance_make_dirty(QofInstance *inst, const char *param_name);
>
> (QOF can't undo / redo anything unless the data is accessible as a
> parameter with a QofAccessFunc *and* a QofSetterFunc so passing the value
> as a string is pointless. The combination of param_name and the object
> e_type from the instance do the rest.)
>
> I could do a qof_undo struct that contains the QofParam and a gpointer of
> the previous value of that parameter along with the type and GUID of the
> entity concerned?
Hmm, problem there. If the undo is recorded where the dirty flag is normally
set (after the edit), the previous value won't be available. Probably better
to put this into qof_begin_edit and get the parameter value there - before
the value is changed. Only changes that use begin_edit will therefore end up
in the undo / redo list - could provide some measure of control.

Maybe incorporate dirty instance flags into qof_commit_edit and have one less
call - it would set the QofCollection as dirty at the same time.

Are there times when a successful commit_edit should NOT lead to a dirty
instance flag?

(BTW. I'm using lowercase because the macros in qof-be-utils.h currently have
problems with the opaque structs when used from an external library and I'm
testing function versions - at least until the problem is solved.)

--

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


_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

attachment0 (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: CLI undo edit?

David Hampton-2
In reply to this post by Neil Williams-2
On Sun, 2005-08-07 at 00:04 +0100, Neil Williams wrote:

> A list would be more flexible - undo lists have to be sequential so a generic
> qof_undo() would need a single GList of all changes and rolling such a list
> in strict sequential order back and forth (i.e. a redo as well as undo) is
> far faster than iterating / searching.

My vote would be for a single unro/redo list.  I think a list per entity
would be very confusing, and it would require exposing the internals of
gnucash (the entity) to all users so they can understand how to use the
undo functionality.

> A Timespec in qof_undo would solve David's request for a time since first
> change too. Simply read the first value in the GList and return ts or
> ts.tv_sec.

:-)

> Each would return silently if the end of the list is reached in the
> appropriate direction. Maybe they should return gboolean so that the UI can
> tell if more steps are available? Or should that be a separate function:
> gboolean qof_can_undo(); gboolean qof_can_redo();
> Both would return FALSE after a save operation.

The UI must be able to find out if there are items available for
undo/redo.  It needs to provide this information to the user by
correctly dimming the Undo and Redo menu items.

David



_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: CLI undo edit?

Neil Williams-2
On Sunday 07 August 2005 4:58 pm, you wrote:
> On Sun, 2005-08-07 at 00:04 +0100, Neil Williams wrote:
> > A list would be more flexible - undo lists have to be sequential so a
> > generic qof_undo() would need a single GList of all changes and rolling
> > such a list in strict sequential order back and forth (i.e. a redo as
> > well as undo) is far faster than iterating / searching.
>
> My vote would be for a single unro/redo list.  

I think you might misunderstood that. There can only be one list per
application using QOF. All undo operations within that application would use
the same list. If it lives in the book, all undo operations on that book
would share the same undo/redo list and those programs that support multiple
books can have multiple lists, accessed according to whichever session or
book is set as current. GnuCash doesn't come into that category yet, it only
concerns itself with a single book outside import/export.

I discounted the idea of a hashtable of entities but even that would be a
single table of all affected entities, not thousands of hashtables each
within an entity.

> I think a list per entity
> would be very confusing,

?? Where did I say per entity? At no point is an undo list a list per entity!
A list OF entities, yes.

It's a single list of entities - or more specifically entity changes. What I
was considering was whether to store these changes as the entire entity or
only as the modified parameter data, ignoring parameters that have not been
changed. My proposed qof_undo opaque struct would simply store the data from
a single parameter, the GUID and the type of the object. Repeated structs
would then be built into a list.

The undo list would be a single sequential list that can be read forwards or
backwards and holds the parameter data changes in ALL entities since the last
save.

One list for all entities in the book. The natural place for the list may
actually BE the QofBook - I was thinking of the backend but the book is the
data-set so the data-set undo list should probably go there.

> and it would require exposing the internals of
> gnucash (the entity) to all users so they can understand how to use the
> undo functionality.

?? Not at all, the undo would be native to QOF, it would all be
behind-the-scenes. None of the internals need bother any user. Click a button
in the toolbar and QOF takes the qof_undo struct from the start or end of the
list as appropriate and sets the previous version of the parameter data in
the entity indicated by the undo data.

As noted, this may well be a reverse list, prepending the most recent changes
so that undo uses list->next and redo uses list->prev.

The entity will remain exposed to the developers, even when QOF is an external
library, but the undo list will not - that will reside elsewhere in an opaque
struct. qof_undo() and qof_redo() functions would provide access.

If it's in the book, that would be:
void qof_undo(QofBook *book);
void qof_redo(QofBook *book);

Each call only moves one step back in the list, subject to the problems of
deleted/created entities.

Click it once and one change is undone, no matter where that change occurred
within the book, no matter which entity is involved. Click redo and it's
redone. The user needs to know absolutely nothing about entities. Even the
developer using the API only needs to know the function prototypes above,
together with the boolean functions below.

Setting the data via QOF would use the same functions as already exist in the
objects, so this even allows the UI to track which window contains the data
changed by the undo/redo - the object would send the same event messages as
if the user had undone the changes manually.

So if the user makes a change in an Account, views a report and then clicks
undo, the UI should be able to pick up the EVENT_MODIFY and realise that
maybe the Account window should be given focus.

What *cannot* be done with a native QOF undo/redo is anything related to
non-QOF data, like the reports, configuration data, settings, options etc.
unless these also exist as genuine QOF objects. e.g. changing the
smallest-commodity-unit in an Account can be undone. Changing the date fields
of a report cannot.

> > Each would return silently if the end of the list is reached in the
> > appropriate direction. Maybe they should return gboolean so that the UI
> > can tell if more steps are available? Or should that be a separate
> > function: gboolean qof_can_undo(); gboolean qof_can_redo();
> > Both would return FALSE after a save operation.

gboolean qof_can_undo(QofBook *book);

gboolean qof_can_redo(QofBook *book);

> The UI must be able to find out if there are items available for
> undo/redo.  It needs to provide this information to the user by
> correctly dimming the Undo and Redo menu items.

That's why I mentioned qof_can_undo and qof_can_redo. Those would return a
simple boolean. The UI can handle that, can't it? (I know I don't do a lot of
GUI programming, but I do understand active and disabled toggles.)
:-)

I can't see what you're getting at, I know the UI needs to be able to detect
if there are changes available to undo or redo - what I was asking was is it
preferable to have that functionality as a pair of separate functions rather
than confusing things by calling undo when all you wanted was to know if you
can undo. I guess I've answered my own question!

That's how I work, I set out an idea at the top of the email and argue it out
in the text - that way I cover the pros and the cons.

Anyway, it's ages away yet.

--

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


_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

attachment0 (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: CLI undo edit?

David Hampton-2
On Sun, 2005-08-07 at 17:46 +0100, Neil Williams wrote:

> On Sunday 07 August 2005 4:58 pm, you wrote:
> > On Sun, 2005-08-07 at 00:04 +0100, Neil Williams wrote:
> > > A list would be more flexible - undo lists have to be sequential so a
> > > generic qof_undo() would need a single GList of all changes and rolling
> > > such a list in strict sequential order back and forth (i.e. a redo as
> > > well as undo) is far faster than iterating / searching.
> >
> > My vote would be for a single unro/redo list.  
>
> I think you might misunderstood that. There can only be one list per
> application using QOF. All undo operations within that application would use
> the same list. If it lives in the book, all undo operations on that book
> would share the same undo/redo list and those programs that support multiple
> books can have multiple lists, accessed according to whichever session or
> book is set as current.

I can see multiple undo lists within one application working, but it
will need to be well documented.  I've been trying to keep the gui
organized so it can support multiple open books.  The idea is that any
given window will only have pages from a single book.  If you open a
second book, you must open a second window.  I need to figure out how to
integrate the account hierarchy druid code with this, because it creates
a temporary book for the life of the import process.

> > I think a list per entity
> > would be very confusing,
>
> ?? Where did I say per entity?

Your first paragraph.

        If it's limited to one undo operation per entity then a
        hashtable indexed on the GUID could work. That's the first
        option, not perfect but probably quite fast. A second change
        in the same entity would overwrite any previous ...

I misread this to imply that you were keeping changes on a per-entity
basis.

> At no point is an undo list a list per entity!

O.K.

> > and it would require exposing the internals of
> > gnucash (the entity) to all users so they can understand how to use the
> > undo functionality.
>
> ?? Not at all, the undo would be native to QOF, it would all be
> behind-the-scenes.

That's good. My statement was based un a misunderstanding of what you
wrote.

> I can't see what you're getting at, I know the UI needs to be able to detect
> if there are changes available to undo or redo - what I was asking was is it
> preferable to have that functionality as a pair of separate functions rather
> than confusing things by calling undo when all you wanted was to know if you
> can undo.

Separate functions.

David



_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: CLI undo edit?

Neil Williams-2
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

David Hampton wrote:
| I can see multiple undo lists within one application working, but it
| will need to be well documented.

If the undo list is retained within the book and only referenced from
that book, this would provide sufficient segregation.

|  I've been trying to keep the gui
| organized so it can support multiple open books.  The idea is that any
| given window will only have pages from a single book.

I can see problems there: an entity cannot access another entity from
another book directly, it needs to store a reference, especially if any
one book can be written out without any of the others.

Periods of time fit more easily into multiple books.

This is all going to take time to develop and test.

What do you seek to achieve by this use of multiple books? Lookup speeds?

If period closing is sorted out, a standard book will be fine.

Conceptually, from a user point of view, I don't see how a book for a
single Account would be useful. Queries also concentrate on searching
within a single book - such small books may be fast but it's hard to see
how data could be aggregated to provide summaries and overviews.

Or are you talking about multiple *main* windows of GnuCash?

|  If you open a
| second book, you must open a second window.  I need to figure out how to
| integrate the account hierarchy druid code with this, because it creates
| a temporary book for the life of the import process.

I'm not sure it can, yet. I'm also not sure about such books, except for
import/export as with QSF. References between entities need careful
handling - look how long it has taken to get partial books and
references sorted out in QSF.

I think what you are looking at is similar to QOF applications like GnoTime.

| If it's limited to one undo operation per entity then a
| hashtable indexed on the GUID could work. That's the first
| option, not perfect but probably quite fast. A second change
| in the same entity would overwrite any previous ...
|
| I misread this to imply that you were keeping changes on a per-entity
| basis.

I was actually hinting at the problems with a GHashTable approach which
overwrites the value (the changed data) of any existing key (the GUID).
One hashtable indexed by GUID's would only allow one undo operation for
each entity listed in the hashtable.


- --

Neil Williams
=============
http://www.codehelp.co.uk/
http://www.dclug.org.uk/
http://www.isbn.org.uk/
http://sourceforge.net/projects/isbnsearch/

http://www.biglumber.com/x/web?qs=0x8801094A28BCB3E3
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (Darwin)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFC9nmik7DVr6iX/QIRAl6fAJwK5GWlbjpHxbgzNgRVtdE2TJTB1wCeP4lP
OAPhygoM+RHb7qf/jwGiYI0=
=dyRG
-----END PGP SIGNATURE-----
_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: CLI undo edit?

David Hampton-2
On Sun, 2005-08-07 at 22:14 +0100, Neil Williams wrote:

> Or are you talking about multiple *main* windows of GnuCash?

Yes.  Gnucash 1.8 has this capability today and it is retained in g2.
You will not be able to display data from two books in the same main
window of gnucash.  The file (book) name is part of the window title so
it will be clear with a glance which data file is being edited.

David


_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: CLI undo edit?

Neil Williams-2
On Sunday 07 August 2005 10:22 pm, David Hampton wrote:
> On Sun, 2005-08-07 at 22:14 +0100, Neil Williams wrote:
> > Or are you talking about multiple *main* windows of GnuCash?
>
> Yes.  Gnucash 1.8 has this capability today and it is retained in g2.
> You will not be able to display data from two books in the same main
> window of gnucash.  The file (book) name is part of the window title so
> it will be clear with a glance which data file is being edited.

Then each book will maintain it's own undo list, that's not a problem at all.
The second main window is, in effect, just another process using the QOF
library. Spinning QOF out as an external library can only aid the support for
multiple main windows of GnuCash or any other QOF process.

Now we are very definitely on the same songsheet!!
:-)

--

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


_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

attachment0 (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: CLI undo edit?

Derek Atkins
In reply to this post by Neil Williams-2
Neil Williams <[hidden email]> writes:

> David Hampton wrote:
> | I can see multiple undo lists within one application working, but it
> | will need to be well documented.
>
> If the undo list is retained within the book and only referenced from
> that book, this would provide sufficient segregation.

I presume by "retained" you mean "in core" and not "in storage"...

IMHO the undo/redo list should be size-bounded, not unlimited.

Also, it should not necessarily depend on the dirty flag.  It should
be independent of the individual dirty flags.  The reason: SQL.  In a
SQL backend the data is necessarily never dirty (because it's saved on
every Commit operation.  e.g. every time you commit a txn your changes
are saved).  BUT if I make changes I might want to back out those
changes..  Hense the "undo".

-derek

--
       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
       [hidden email]                        PGP key available
_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: CLI undo edit?

Neil Williams-2
On Monday 08 August 2005 9:47 am, Derek Atkins wrote:
> Neil Williams <[hidden email]> writes:
> > If the undo list is retained within the book and only referenced from
> > that book, this would provide sufficient segregation.
>
> I presume by "retained" you mean "in core" and not "in storage"...

Yes, the list itself would be part of the QofBook struct and accessed via an
API exposed via qofbook.h - the undo list itself could be stored in a similar
manner to the current logs if that was useful. Depends on the SQL backend -
if we're all using immediate commit in SQL then would there be any need for
the logs at all?

> IMHO the undo/redo list should be size-bounded, not unlimited.

I was wondering about that too. So a hard-coded limit of, say 300 - should
that be configurable? or an absolute limit and configurable only downwards?

> Also, it should not necessarily depend on the dirty flag.

Maybe it would use the event interface instead - every modify or create event
would be added to the undo list?

Should the name reflect that?
void qof_book_undo_event(QofBook *book);
?
or just
void qof_undo(QofBook *book);
?

--

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


_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

attachment0 (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: CLI undo edit?

Derek Atkins
Neil Williams <[hidden email]> writes:

> On Monday 08 August 2005 9:47 am, Derek Atkins wrote:

>> I presume by "retained" you mean "in core" and not "in storage"...
>
> Yes, the list itself would be part of the QofBook struct and
> accessed via an API exposed via qofbook.h - the undo list itself
> could be stored in a similar manner to the current logs if that was
> useful. Depends on the SQL backend - if we're all using immediate
> commit in SQL then would there be any need for the logs at all?

IMHO, no.

Also keep in mind that the logs are not complete and don't store
enough information.  It would be much better if QOF had an in-core
"log" that it could use instead of depending on the existing file-log
interface.  Personally I'd like to rip out the file logs...  They
cause way too many problems.  :(

>> IMHO the undo/redo list should be size-bounded, not unlimited.
>
> I was wondering about that too. So a hard-coded limit of, say 300 - should
> that be configurable? or an absolute limit and configurable only downwards?

I suspect if you keep the last 300 _operations_ (where an import is
one operation) then yes, that would probably be sufficient.  :) If,
however, it's only 300 changed entities, that might NOT be sufficient.

>> Also, it should not necessarily depend on the dirty flag.
>
> Maybe it would use the event interface instead - every modify or create event
> would be added to the undo list?

No, I don't think so.  I think it should still use the begin/commit
edit hooks.  OTOH, I'd also like some way to combine multiple
begin/commit blocks into single events (e.g. "import") so that you can
undo the full import all at once.

> Should the name reflect that?
> void qof_book_undo_event(QofBook *book);
> ?
> or just
> void qof_undo(QofBook *book);

I think "qof_book_undo()" is probably the best name for the function,
but the name is relatively easy to change later ;)

-derek
--
       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
       [hidden email]                        PGP key available
_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: CLI undo edit?

Bugzilla from bock@step.polymtl.ca
On August 9, 2005 04:10 am, Derek Atkins wrote:

> Neil Williams <[hidden email]> writes:
> > On Monday 08 August 2005 9:47 am, Derek Atkins wrote:
> >> I presume by "retained" you mean "in core" and not "in storage"...
> >
> > Yes, the list itself would be part of the QofBook struct and
> > accessed via an API exposed via qofbook.h - the undo list itself
> > could be stored in a similar manner to the current logs if that was
> > useful. Depends on the SQL backend - if we're all using immediate
> > commit in SQL then would there be any need for the logs at all?
>
> IMHO, no.
>
> Also keep in mind that the logs are not complete and don't store
> enough information.  It would be much better if QOF had an in-core
> "log" that it could use instead of depending on the existing file-log
> interface.  Personally I'd like to rip out the file logs...  They
> cause way too many problems.  :(
We should rip them out in the head branch...


--
Benoit Grégoire, http://benoitg.coeus.ca/

_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

attachment0 (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: CLI undo edit?

Neil Williams-2
In reply to this post by Derek Atkins
On Tuesday 09 August 2005 9:10 am, Derek Atkins wrote:

> Neil Williams <[hidden email]> writes:
> > the undo list itself
> > could be stored in a similar manner to the current logs if that was
> > useful. Depends on the SQL backend - if we're all using immediate
> > commit in SQL then would there be any need for the logs at all?
>
> IMHO, no.
>
> Also keep in mind that the logs are not complete and don't store
> enough information.
It's becoming clear that my original QOF undo idea would be similarly
incomplete, albeit in different areas.

> It would be much better if QOF had an in-core
> "log" that it could use instead of depending on the existing file-log
> interface.  Personally I'd like to rip out the file logs...  They
> cause way too many problems.  :(

I wish I'd never mentioned undo . . . . - this is going to take time.

> >> IMHO the undo/redo list should be size-bounded, not unlimited.
> >
> > I was wondering about that too. So a hard-coded limit of, say 300 -
> > should that be configurable? or an absolute limit and configurable only
> > downwards?
>
> I suspect if you keep the last 300 _operations_ (where an import is
> one operation) then yes, that would probably be sufficient.  :) If,
> however, it's only 300 changed entities, that might NOT be sufficient.

(I wasn't even thinking of storing an entire import in undo at this stage!)
:-(

That would probably need a branch off the list - one pointer on the list to
either a second list from the import or some other collation of the import
changes.

(And yes, I was thinking of a simple 300 changes - but without an undo for
imports then importing data would have probably required the deletion of the
previous undo list (as it may reference entities changed in the import) upon
completion of the import commit. That, I accept, would not be workable.)

> >> Also, it should not necessarily depend on the dirty flag.
> >
> > Maybe it would use the event interface instead - every modify or create
> > event would be added to the undo list?
>
> No, I don't think so.  I think it should still use the begin/commit
> edit hooks.  OTOH, I'd also like some way to combine multiple
> begin/commit blocks into single events (e.g. "import") so that you can
> undo the full import all at once.

This is *definitely* getting too much for me at this time. I'm afraid I cannot
promise to meet those requirements / expectations. Undo may have to go on the
back burner until next year. Sorry.

Ah well, at least I asked. QOF undo is, if I may quote, a "simple matter of
programming". Unfortunately, it is programming that I do not have time to do
myself, yet.

I *might* investigate a very limited undo strictly within CashUtil, just as a
framework, that allows only the current entity edits to be undone during the
operation of the current sub-shell. It may throw more light on how the QOF
version could operate and lay some foundations that I can work on later
within QOF and thereby GnuCash.

For now, I'm afraid, I have to step back and keep to what I can realistically
hope to complete.

--

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


_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

attachment0 (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: CLI undo edit?

Derek Atkins
In reply to this post by Bugzilla from bock@step.polymtl.ca
Benoit Grégoire <[hidden email]> writes:

> We should rip them out in the head branch...

Well, not until we have full SQL support.. ;)

-derek
--
       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
       [hidden email]                        PGP key available

_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: CLI undo edit?

Derek Atkins
In reply to this post by Neil Williams-2
Neil Williams <[hidden email]> writes:

> For now, I'm afraid, I have to step back and keep to what I can realistically
> hope to complete.

Oh, that's fine.  I honestly didn't expect Undo in the near term.
Besides, we don't quite have SQL done, yet, so it's not a major deal
to me.  :)

I think "undo" will be much more important once we get SQL, but it's
not AS important now.

Also, I think the right way to go about it is not a list of changed
entities, but a list of events/operations.  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.

Something like this:

  |
  |
 Import - [obj1, obj2, obj3, obj4, obj4]
  |
 New
 Acct  -- [ acct ]
  |
 New
 Txn   -- [ txn ]
  |
 Mod
 Txn   -- [ txn ]
  |
  |

Please don't feel inadiquate; you've done a tremendous amount of work.
Undo/Redo is a HARD problem in general unless we have the framework in
place to support it.  Unfortunately I suspect that framework will
require changes to QOF, but we'll have to see.

I don't expect this anytime soon; I'd much rather see g2 get done and
out the door first ;)

-derek
--
       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
       [hidden email]                        PGP key available
_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: CLI undo edit?

Neil Williams-2
On Wednesday 10 August 2005 8:49 am, Derek Atkins wrote:
> Also, I think the right way to go about it is not a list of changed
> entities, but a list of events/operations.

So each event would generate a list of the parameter values of each affected
entity as they existed prior to the event.

Undo List
        -> List of affected entities (just the type and GUID) per event.
                -> list of affected parameter values before the event.

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.

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?

I'd use modified begin_edit/commit_edit calls that pass the char*
param->parameter_name to identify the entity changes and use the type of
event to determine how the values are stored and reset.

Values would need to be obtained at the begin edit stage and added to the undo
list (or what is becoming more of a tree) only when an event was received -
making storing the undo data into a two stage process, invisible to the
application concerned. If there is no event before commit_edit, the undo data
is freed (or should that if no event is received before the NEXT begin_edit?)

Changes that don't use BOTH a begin_edit/commit_edit pair AND generate an
event won't be available for undo or redo. What happens in the numerous
places where engine events are suspended?

Outline so far:

/** @brief The parameter changes, >=1 per affected entity

One per parameter change - the bottom level of any undo. A single click of
Undo could use the data from one or many parameter changes - as determined by
the event. Each parameter change can be for any entity of any registered type
in the book and parameter changes can be repeated for the multiple changes to
different parameters of the same entity. The combination of param, guid and
type will be unique per event. (i.e. no event will ever set the same
parameter of the same entity twice (with or without different data) in one
undo operation.)
*/
typedef struct qof_undo_entity_t
{
        QofParam *param; /**< static anyway so only store a pointer */
/* or maybe just use the param_name */
        const GUID *guid; /**< or use a cached guid_as_string? */
        QofIdType type; /**< ditto param, static. */
        char *value; /**< cached string? */
}qof_undo_entity;

/** @brief The affected entities, >=1 per event

The top level of any undo. Contains a GList that keeps the type of event and
the GList of qof_undo_entity* instances relating to that event. Some form of
index / counter probably too in order to speed up freeing unwanted events and
undo data upon resumption of editing and in controlling the total number of
events that can be undone.

Each qof_undo_event.entity_list can contain data about >1 type of entity.
*/
typedef struct qof_undo_event_t
{
        GNCEngineEventType type;
        Timespec ts;
        gint index_position;
        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* */

Also in qofbook.c:
#define MAX_UNDO_LENGTH     300

The Timespec would probably be better in this top level struct and it would be
the earliest valid Timespec from all qof_undo instances in the list for that
event.

The book would report the first valid Timespec in the undo list as the time of
the first change of the book data.

The entire undo list would have to be accessible only from a single book -
multiple books, multiple undo lists.

With such a structure, an undo list that could hold data for all parameters of
all entities affected by up to 300 individual events should be more than
adequate. I plucked 300 from examples of other applications - there's no
particular reason behind it - we could use 100 or 500. I doubt there's much
point in going beyond 500 and, personally, I'd see much less than 100 as a
wasted opportunity.

A simple #define should suffice for extensibility.

We can look at configuration options for reducing it from the default later.

> Then you can undo/redo per
> operation.

OK.

> 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.

Agreed. Other edits may also affect values of more than one parameter so there
may be a lot of these sub-lists around. e.g. entity deletion.

It would need begin_edit() to pass the name of the parameter that is about to
be edited - not a big problem. (To save storing the entire entity everytime.)

> Please don't feel inadiquate; you've done a tremendous amount of work.

Thank you, Derek. Overwhelmed is more the feeling! There's simply so much left
to do! I admit, I do feel swamped by it at times.

Your encouragement and support is much appreciated. It's a vital component of
the development process - volunteer projects cannot survive without
motivation and nothing saps morale more than a lack of leadership and
encouragement. It can't be easy to keep GnuCash on track when you are a
volunteer yourself.

It's just a shame that Linas isn't around to be hands-on with QOF. It takes
time for new developers to be able to contribute and an experienced lead
developer is so important.

> Undo/Redo is a HARD problem in general unless we have the framework in
> place to support it.  Unfortunately I suspect that framework will
> require changes to QOF, but we'll have to see.

No more than the changes needed for book merging or QSF, I suspect.

> I don't expect this anytime soon; I'd much rather see g2 get done and
> out the door first ;)

Definitely. 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.

--

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


_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

attachment0 (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: CLI undo edit?

David Hampton-2
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.

> 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?  Undo/redo shouldn't affect the
event list.  Any other change discards all events in the list that have
been undone but not redone.

David


_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: CLI undo edit?

Neil Williams-2
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/


_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

attachment0 (196 bytes) Download Attachment
12