2013/5/16 Oliver-Rainer Wittmann <orwittm...@googlemail.com> > Hi, > > > On 14.05.2013 10:48, Fan Zheng wrote: > >> Hi, all: >> >> Now I am working on a bugzilla issue 121897, which seems a writer >> undo/redo >> corresponding issue. Inside there are some questions I met, which may need >> help from the experts. >> >> Here is background: >> >> There are some refine work in SW undo/redo mechanism for integrating into >> the SfxUndoManager framework. Inside the implementation, >> SwUndoInserts::UndoImpl, which is the detailed undo method for inserting, >> will drop all the hints inside the current SwTxtNode, and then retrieve >> them back via the SwHistory::TmpRollback. Similar design also exist in >> SwUndoDelete::UndoImpl (Similarly the detailed undo method for deleting), >> but for some unknown reason (is there do exist some), the using hints >> clear >> methods are different inside above 2 undo functions. For SwUndoInserts, >> the SwTxtNode::RstAttr is used, and for SwUndoDelete, >> the SwTxtNode::ClearSwpHintsArr is used. In general, the difference of >> said >> 2 hints clearing method is that, RstAttr will ignore the tox mark index >> and >> ClearSwpHintsArr will ignore the fly content when clearing hints. >> >> And such difference makes the phenomenon of i121897 happens: the former >> tox >> mark index is kept when clearing and a new one inserted later from >> SwHistory; >> >> So my questions are: >> >> 1. Why the undo of inserting use a different clear method from the undo of >> deleting? Should we make them identical or, refine the RstAttr for certain >> usage in undo inserting? >> > > > I am not sure about the existence of two such methods, but I assume that > the corresponding code had evolved over the last decade and nobody took the > responsibility to clean it up. > > I propose unify the functionality in one method. >
I am not sure whether it is workable if we decide to simply drop any of them, for the RstAttr could give more parameters, for more specified conditions of hints clearing, for example the the clear index ranges and the hint attributes type. So such unify you mentioned should be more flexible then they were. > For a release after AOO 4.0 we should investigate in detail all usages of > the two existing methods, define certain test cases for these usages and > then unify the functionality in one method. The most important step here is > to define the test cases. I agree with you. > > > 2. As we see, the whole hints inside the text node will be cleared, even >> if >> the actions should be undone are related to part of the text node. At it >> is >> not a good performance design. Is there any certain reason for doing like >> that? >> > > I do not know. > Do you know, if this came in by the SW undo/redo refactoring? > No, at least the totally hints clearing stuff exist in SwUndoDelete::Undo(...) before the sw undo/redo refactoring. Which exists maybe years, maybe even older than me :) The former OOo code repository is still available at > http://hg.services.openoffice.**org/ <http://hg.services.openoffice.org/>. > May be a short look at its trunk (namely DEV300) and the corresponding log > would give some insight. Even if it reveals that this 'complete clear' had > been made since years. > >> Thanks for your help! >> >> > You are welcome. > > A question from my side: > Does defect 121897 occurs before the SW undo/redo refactoring had been > made? No, it is a regression defect from SW undo/redo refectoring in my personal view. > > > Best regards, Oliver. > > ------------------------------**------------------------------**--------- > To unsubscribe, e-mail: > dev-unsubscribe@openoffice.**apache.org<dev-unsubscr...@openoffice.apache.org> > For additional commands, e-mail: dev-h...@openoffice.apache.org > >
