I agree that the pointer stuff is bad, but I don't fully agree with "broken by design".
>From a user's perspective, it would look more broken when deleting a selection with 50+ pages would take a reasonable amount of time, only because some expensive Undo information is being created, which probably will never be used. But if you insist, you can also find other examples where resources might not be available anymore. Or implementations w/o support for ref counting: For example, each EditEngine Undo action needs to have an EditEngine*. And EditEngines in OOo are destroyed and re-created quite frequently. Currently there is no problem, because the EditEngine also has it's own UndoManager, which simply will vanish when the EditEngine is being destroyed. When the EditEngine would put it's UndoAction in SW's UndoManager, it would have a problem.... Work around would be to make every undo action a component listener, so it could invalidate the EditEngine pointer or reference. This way, you avoid the crashes, but you still have the Undo Actions on the Stack. When the user now wants to perform Undo, nothing will happen, which might be supprsing. Maybe the Undo needs some "valid" state, or also being able to dispose itself when it needs to do, and the the UndoManager needs to listen for that. I don't know what the best solution is, but I know that it's not easy to get undo right... I am also not sure if we really need to create the "Eierlegende Wollmilchsau" with a new Undo API, or if most people wouldn't already be happy with some API at the document level where the can perform StartUndoContext/EndUndoContext/Undo/Redo/Repeat/EnableUndo. Could be some "XUndo" API very similar to XUndoManager, but w/o addUndo() and the hidden stuff. Then the "modifiable" UndoManager (addUndo and what ever else is needed) and XUndoAction stuff could be an extra step / optional implementation. Malte. Björn Michaelsen wrote, On 10/18/10 16:27: > Hi Malte, Michael, > > Am Mon, 18 Oct 2010 15:46:00 +0200 > schrieb Michael Stahl <michael.x.st...@oracle.com>: > >> On 18/10/2010 14:55, Malte Timmermann wrote: >>> The Undo actions could have pointers to some data which doesn't >>> exist anymore, and the extension can't remove the actions from the >>> UndoManager. > > That is broken by design. > >>> Avoiding pointers could mean making the Undo actions >>> more "expensive" (memory, time). For example, Writer and EditEngine >>> simply keep Paragraph* when paragraphs are being deleted completely. >> >> this is probably one of the reasons why using Undo/Redo in writer is >> the fastest way to crash OOo... >> using bare pointers to objects that are not owned by the Undo action >> is a horrible idea. > > +1 > >> making copies of data is an acceptable overhead of an Undo >> implementation that actually _works_. > > +1 > >> [oh, and what you said is not actually true for paragraphs, but lots >> of other stuff] > > - Create page style > - Delete newly created page style > - undo twice => crash (issue i92304, fixed in cws swbookmarkfixes01) > > BR, > > Bjoern > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@api.openoffice.org > For additional commands, e-mail: dev-h...@api.openoffice.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@api.openoffice.org For additional commands, e-mail: dev-h...@api.openoffice.org
