Review Request: Undo redo framework for text refactored

Thorsten Zachmann t.zachmann at zagge.de
Fri Feb 24 11:58:16 GMT 2012


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104047/#review10863
-----------------------------------------------------------


The works looks quite good. Some comments. I did not test the code yet.


libs/flake/commands/KoShapeDeleteCommand.cpp
<http://git.reviewboard.kde.org/r/104047/#comment8795>

    This seems not to be needed please remvoe.



libs/kotext/KoTextEditor.cpp
<http://git.reviewboard.kde.org/r/104047/#comment8796>

    This code block is duplicated quite often. I think it makes sense to re-factor this so it can be reused e.g.
    
    startCommand(i18nc(...));
    
    The same can be done for endCommand
    
    However this change can also work to a later state



libs/kotext/KoTextEditor_undo.cpp
<http://git.reviewboard.kde.org/r/104047/#comment8798>

    there should be a blank after the while



words/part/KWDocument.h
<http://git.reviewboard.kde.org/r/104047/#comment8799>

    Is it enough to do that only for Words?



words/part/dialogs/KWAnchoringProperties.cpp
<http://git.reviewboard.kde.org/r/104047/#comment8800>

    I think it would make to code more clear when the ChangeAnchorPropertiesCommand would be moved here



words/part/dialogs/KWFrameDialog.cpp
<http://git.reviewboard.kde.org/r/104047/#comment8801>

    Indention is wrong


- Thorsten Zachmann


On Feb. 23, 2012, 12:10 p.m., C. Boemann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104047/
> -----------------------------------------------------------
> 
> (Updated Feb. 23, 2012, 12:10 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> We have made changes to the undo framework in kotexteditor to make things work.
> The DeleteCommand has become much more capable as it handles complex selections and bookmarks now
> The KoTextEditor.cpp file has been split out into 3 files
> The StyleChange commands and shapeAnchoring commands have been adapted to the new frame work
> 
> 
> Diffs
> -----
> 
>   libs/flake/KoShapeController.h abc4358 
>   libs/flake/commands/KoShapeDeleteCommand.cpp 519f047 
>   libs/kotext/CMakeLists.txt 567649b 
>   libs/kotext/KoTextDocument.h 56f55e6 
>   libs/kotext/KoTextDocument.cpp 5608860 
>   libs/kotext/KoTextEditor.h 689051b 
>   libs/kotext/KoTextEditor.cpp 3c141c7 
>   libs/kotext/KoTextEditor_format.cpp PRE-CREATION 
>   libs/kotext/KoTextEditor_p.h 77725f3 
>   libs/kotext/KoTextEditor_undo.cpp PRE-CREATION 
>   libs/kotext/commands/ChangeAnchorPropertiesCommand.h 4206bb8 
>   libs/kotext/commands/ChangeAnchorPropertiesCommand.cpp 7d4b912 
>   libs/kotext/commands/ChangeStylesCommand.h accfd31 
>   libs/kotext/commands/ChangeStylesCommand.cpp b9fa725 
>   libs/kotext/commands/ChangeStylesMacroCommand.cpp 34c3822 
>   libs/kotext/commands/ChangeTrackedDeleteCommand.cpp 89edde1 
>   libs/kotext/commands/DeleteAnchorsCommand.cpp 621829f 
>   libs/kotext/commands/DeleteCommand.h 89b448a 
>   libs/kotext/commands/DeleteCommand.cpp 2a4527d 
>   libs/kotext/commands/TextPasteCommand.cpp 6b9de68 
>   libs/kotext/tests/TestKoTextEditor.cpp bd15b3a 
>   libs/kundo2/kundo2stack.h c0539fe 
>   libs/kundo2/kundo2stack.cpp 48d6625 
>   libs/main/rdf/KoSemanticStylesheet.cpp 7853dd0 
>   libs/main/tests/rdf_test.cpp fb57a5e 
>   plugins/textshape/TextTool.cpp ddfe47f 
>   plugins/textshape/commands/TextCutCommand.cpp bdb8eea 
>   words/part/KWDocument.h 0005bc6 
>   words/part/KWDocument.cpp 8e00cb6 
>   words/part/dialogs/KWAnchoringProperties.cpp c742e26 
>   words/part/dialogs/KWFrameDialog.cpp 0fcc0b7 
>   words/part/frames/KWTextFrameSet.cpp ebd532f 
> 
> Diff: http://git.reviewboard.kde.org/r/104047/diff/
> 
> 
> Testing
> -------
> 
> We have played around with undo redo in all the cases we knew were failing and quite a lot of other tests.
> Undo of many things are still not implemented, but that is not really what this is about. It's about the basic undo, and getting the framework right. Fixing undo of individual stuff can always be done as bug fixes
> One issue we have noted is insertion of table, whic we believe is a qt bug. We have one last workaround in our minds, but other than that we will just have to wait for a qt fix
> 
> 
> Thanks,
> 
> C. Boemann
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20120224/2f8bf186/attachment.htm>


More information about the calligra-devel mailing list