Tk Source Code

View Ticket
Ticket UUID: 554763
Title: TIP #104 Reimplement undo stack + minor fixes
Type: Patch Version: TIP Implementation
Submitter: lgac Created on: 2002-05-11 05:46:52
Subsystem: 18. [text] Assigned To: das
Priority: 4 Severity:
Status: Closed Last Modified: 2003-11-13 03:16:19
Resolution: Fixed Closed By: hobbs
    Closed on: 2002-09-02 21:06:30
This is a Tk 8.4a4 patch.

The purpose is a reimplemtation of the undo stack.
An item on the undo stack is now a list of items
containing two scripts (Tc_Obj's): the apply action
(for redo) and the revert action (for undo).
This opens the path to undoing more than only text
inserts and deletions, such as applying or removing
tags (this is not part of the patch however).

The undo stack implementation has been separated from
the text widget, and is in fact usable by other
widgets as well now.

Some typos in the text.n doc are corrected as well,
and a problem with the tk::TextPaste function (pasting
twice) was also corrected.

This patch has only been verfied on Red Hat Linux 7.2

The Windows was adapted to accomodate the
new tkUndo.c and tkUndo.h files.

The Mac section was NOT adapted yet for these two new
User Comments: hobbs added on 2002-06-22 06:04:22:

File Added - 25594: tip104_hobbs.txt

Logged In: YES 

I'm attaching the actual patch I used for 8.4cvs on 2002-06-
21, that has minor changes to tkUndo.h for EXTERN handling 
on Windows.  It is based on lgac's latest, with the -maxundo 
and stack limit.

I'm also assigning to DAS so he can correct the Mac build 
files.  Otherwise this is commited for Win/Unix.

lgac added on 2002-06-22 01:55:11:

File Added - 25571: tk8.4a4_patch4.txt

lgac added on 2002-06-22 01:55:10:
Logged In: YES 

In an additional attempt to comply to the discussion 
regarding TIP#104 I created yet another patch. This one 
implements stack depth control. To use it a new option 
was added to the text width -maxundo which sets the 
maximum number of compound actions to keep on the 
Jeff, as the maintainer, you can select whatever patch 
you feel comfortable with, or which has been accepted.
The doc has been adapted to reflect this text widget 
change, the tests have been extended.
The MAC section has still NOT been adapted to account 
the 2 new files.

hobbs added on 2002-06-21 23:48:27:
Logged In: YES 

While the Tcl_Obj* vs. Tcl_DString interface can make for 
good discussion, as this is still a private set of APIs based on 
existing code in the core, I do not think it should inhibit the 
current patch going in.  These can be changed later, when 
the undo stack is more exposed.

vincentdarley added on 2002-06-21 22:06:40:
Logged In: YES 

The patch looks good, but I would strongly suggest 
using a Tcl_Obj* rather than a Tcl_DString interface.  
There are two reasons I can see for that, and one against:

Against: the text widget (and perhaps other widgets) in 
Tk uses Tcl_DString extensively, because it hasn't really 
been objectified ever, so the undo stack will fit more 
easily with those widgets.

For: Tcl uses Tcl_Obj almost everywhere, Tk is gradually 
being objectified, and so looking towards the future, we 
really should have all _new_ interfaces to Tk use 
Tcl_Obj if at all possible.  Since widgets will need to be 
updated to use the undo stack, it may well be sensible 
to objectify them at the same time anyway.

For: If the undo stack is exposed to Tcl, then 
presumably somewhere the equivalent of 'eval 
$userCmd $undoInformation' will be called. If the 
$undoInformation is in a Tcl_Obj, that evaluation can be 
more efficient than if it had to use a Tcl_DString.  
(Because of uses of Tcl_EvalObj, with Tcl list objects etc)

anyway, those are my thoughts...

lgac added on 2002-06-21 14:09:12:

File Added - 25543: tk8.4a4_patch3.txt

Logged In: YES 

based on the discussions about TIP 104 (dealing with this
patch) I have made two modifications:

1) The interpreter in which to evaluate the apply and 
revert scripts is now stored in the undo/redo stack, so the
TkUndoRevert and TkUndoApply functions loose their 
interp argument.

2) TK_UNDO_COMMAND was changed to 
TK_UNDO_ACTION for consistency reasons.

the complete patch is attached (tk8.4a4_patch3.txt)

lgac added on 2002-05-24 04:13:12:

File Added - 23614: tk8.4a4_patch2.txt

lgac added on 2002-05-24 04:10:03:
Logged In: YES 

I attached a new patch on tk8.4a4 which takes the 
generalization of the undo one step further. The text 
widget is no longer aware of the internals of the undo 
mechanism at all with this patch.
I noticed that a TIP is required, although generalizing the 
undo stack has already been discussed when I submitted 
TIP #26. I hope that this TIP can indeed be processed 
fast, since I did not have very positive experiences with 
TIP #26 from a perspective of processing speed :^)

dkf added on 2002-05-23 17:51:16:
Logged In: YES 

It does need a TIP, but not a big one (it just needs to say
that this exposes the internal interfaces induced by the
earlier TIP to allow whole-application undo stacks. Or
something like that.)  I don't expect it to be controversial
and think it could be fast-tracked in in very little time.

vincentdarley added on 2002-05-23 16:19:43:
Logged In: YES 

I think this is an excellent idea.  It would be good to go
further and rip out more 'generic' code from the Text widget
(all that Btree stuff, for example).

Does this need a TIP or can it be applied as simply a more
useful implementation of the TIP which added 'undo' to the
text widget in the first place?

lgac added on 2002-05-11 12:46:52:

File Added - 22861: tk8.4a4.patch