Re: Regarding my work on grouping delete undo actions with Track Changes set

2017-08-18 Thread Michael Stahl
On 06.08.2017 09:19, Rosemary Sebastian wrote:
> Currently we have a separate class SwUndoRedlineDelete to take care of
> undo delete actions with Track Changes set. But the CanGrouping function
> in the class does not serve any purpose in reality.
>  
> From what I understand making the function work again would mean
> reimplementing the undo-redo mechanisms that are already implemented in
> SwUndoDelete class. So I propose that we drop the SwUndoRedlineDelete
> class and instead rework the SwUndoDelete class to handle redline delete
> undo-redo actions as well.

hello Rosemary,

first, i applaud your courage at looking at Writer's redlining code.

looking at commit b2fa5f3f5ee68a0aa2225d7b5aa80ccef2cb70ac i am not
convinced that using SwUndoDelete for redline deletions is the way to go.

the changes are incomplete, in that they only prevent deleting the text
in the start/end nodes of the selection; you'd also have to disable the
SaveContent() that copies everything to the undo-nodes-array, and the
bMoveNds case that moves complete nodes to the undo-nodes-array, and the
DelContentIndex() that deletes various things that are anchored in the
selection but which should be kept in the redline, at which point there
isn't much left...

and of course, as you found, you can't combine a UndoDelete with
redlining enabled with one that had redlining disabled anyway.

you'll end up with a class that has even more complicated conditional
code than SwUndoDelete already has.

i guess it would be much simpler to figure out why
SwUndoRedlineDelete::CanGrouping() doesn't work and fix that; apparently
the redline itself is combined just fine, only the Undo action isn't.

some testing indicates it worked in OOo 3.3 but broke in OOo 3.4 beta so
it's quite possible that the bug is my fault, cf. CWS undoapi ...

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice


Regarding my work on grouping delete undo actions with Track Changes set

2017-08-06 Thread Rosemary Sebastian
Hello everyone

Currently we have a separate class SwUndoRedlineDelete to take care of undo
delete actions with Track Changes set. But the CanGrouping function in the
class does not serve any purpose in reality.

>From what I understand making the function work again would mean
reimplementing the undo-redo mechanisms that are already implemented in
SwUndoDelete class. So I propose that we drop the SwUndoRedlineDelete class
and instead rework the SwUndoDelete class to handle redline delete
undo-redo actions as well.

Due to lack of feedback on gerrit, I created a remote feature branch
feature/sw-delete-undo-rework for this task. You can track the progress
here:
https://cgit.freedesktop.org/libreoffice/core/log/?h=feature/sw-delete-undo-rework
.


If you have any concerns regarding my work, please write to the mailing
list. I would love to hear any feedback or comments.

Regards
Rosemary
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice