Re: [api-dev] Re: Attempt for an UNO Undo API
On 10/18/2010 05:16 PM, Malte Timmermann wrote: 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. Maybe using an own Undo manager for the EditEngine is wrong. When we implemented Notes2, I finally ended up with letting the container (Writer) tracking the changes, I didn't create an additional undo manager for the note(s). That means, each time when an undo entry shall be created, the Writer just copies the content of the outliner object inside the note. For more complicated cases this could be improved by a better encapsulation of what needs to be copied by providing suitable methods in the Outliner class. This way we got an Undo that is not prone to crashes and that doesn't depend on whether the user is editing the note or the text of the document. It also appeared that giving access to all entries of the undo stack while editing in a note doesn't create a problem if the implementation of the notes view is aware of sudden destruction (something that IMHO is necessary anyway). I think that we could apply this idea to drawing objects also. For OLE objects OTOH this probably wouldn't help as copying the whole object each time it changes surely would be too much. Regards, Mathias -- Mathias Bauer (mba) - Project Lead OpenOffice.org Writer OpenOffice.org Engineering at Oracle: http://blogs.sun.com/GullFOSS Please don't reply to nospamfor...@gmx.de. I use it for the OOo lists and only rarely read other mails sent to it. - To unsubscribe, e-mail: dev-unsubscr...@api.openoffice.org For additional commands, e-mail: dev-h...@api.openoffice.org
Re: [api-dev] Re: Attempt for an UNO Undo API
Hi Malte, 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. Hmm. This would mean silent removal of Undo actions from the undo action list - not ideal, from an UX POV. Also, the Undo manager has to care for undo actions which fail when invoked, anyway (think of a broken bridge to the UNO component providing the action, or something like this). So, for the moment I think it is safe to assume that Undo actions simply throw an DisposedException if they cannot provide the functionality anymore. Still, an interesting question would be how to react on this. It might not always be safe to assume that the action can be removed from the stack - subsequent actions might depend on the manipulations done by the now-failing action. The safest solution would be to clear to stack completely in such a case, but that might be too offensive. Not sure. 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. addUndo, in fact, is where my interest in the complete topic originates from, since we have components (the property browser for form controls, for instance) modifying a document per UNO API only. Currently, there is no way to make those actions UNDOable - which is an UX problem for years now. So, unless I am overruled, I will *not* implement an API which does not have addUndo( XUndo ) :) Ciao Frank -- ORACLE Frank Schönheit | Software Engineer | frank.schoenh...@oracle.com Oracle Office Productivity: http://www.oracle.com/office - To unsubscribe, e-mail: dev-unsubscr...@api.openoffice.org For additional commands, e-mail: dev-h...@api.openoffice.org
Re: [api-dev] Re: Attempt for an UNO Undo API
Hi Bjoern, No need for that german pig. However, what is desperately needed is a basic general concept not only for the interface, but also for the implementation of undo, including some design patterns (like for example the simple undo guard proposed in i114888). Otherwise we will end up with conflicting designs, which is hard to clean up. To clarify: I dont want to rewrite all of undo at once -- but at least we should have a shared and documented vision how it is _supposed_ to work. As wrote in my other reply to Malte, the current proposal is not intended to replace all Undo implementations, instead, the current goal is to provide Undo access from outside the core implementations. If you see things in this design which you think will hinder a future more generic use of it, please let's discuss it, I'm most willing to make the API as future-proof as possible, even if we will not immediately use it as complete internal replacement. Ciao Frank -- ORACLE Frank Schönheit | Software Engineer | frank.schoenh...@oracle.com Oracle Office Productivity: http://www.oracle.com/office - To unsubscribe, e-mail: dev-unsubscr...@api.openoffice.org For additional commands, e-mail: dev-h...@api.openoffice.org
[api-dev] Re: Attempt for an UNO Undo API
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. 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. making copies of data is an acceptable overhead of an Undo implementation that actually _works_. [oh, and what you said is not actually true for paragraphs, but lots of other stuff] -- That's what C++ is all about: the development of your ethical judgment. Everybody can tell good from bad; choosing between the ugly and the disgusting takes real wisdom. -- Yossi Kreinin - To unsubscribe, e-mail: dev-unsubscr...@api.openoffice.org For additional commands, e-mail: dev-h...@api.openoffice.org
[api-dev] Re: Attempt for an UNO Undo API
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
Re: [api-dev] Re: Attempt for an UNO Undo API
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
Re: [api-dev] Re: Attempt for an UNO Undo API
On Mon, 18 Oct 2010 17:16:32 +0200 Malte Timmermann malte.timmerm...@oracle.com wrote: 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. It is still broken by design. Either you provide stable undo operations for a scenario or you do not provide undo operations for the scenario at all. Because you can also be certain that the user you speak of would prefer no undo for tricky operations rather than an undo stack than sometimes crahes the application. 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. No need for that german pig. However, what is desperately needed is a basic general concept not only for the interface, but also for the implementation of undo, including some design patterns (like for example the simple undo guard proposed in i114888). Otherwise we will end up with conflicting designs, which is hard to clean up. To clarify: I dont want to rewrite all of undo at once -- but at least we should have a shared and documented vision how it is _supposed_ to work. BR, Bjoern - To unsubscribe, e-mail: dev-unsubscr...@api.openoffice.org For additional commands, e-mail: dev-h...@api.openoffice.org
