Review Request: Move several commands from TextTool to the KoTextEditor interface

Boudewijn Rempt boud at valdyas.org
Fri Sep 23 09:42:45 BST 2011



> On Sept. 22, 2011, 8:08 p.m., C. Boemann wrote:
> > In general quite nice. A few remarks, and then i just assumed you moved the commands with maybe a little rename but i din't exactly read all of them. Please advice if i need to.
> > 
> > 
> > For the kotexteditor::paste i think we should irc a bit

I also had to remove the dependency on TextTool in the commands, but for the rest, yeah, not much changed. I'm on irc whenever you like, although  I'll be in bed part of the day. Bit under the weather.


> On Sept. 22, 2011, 8:08 p.m., C. Boemann wrote:
> > libs/flake/KoShapeController.cpp, line 160
> > <http://git.reviewboard.kde.org/r/102679/diff/1/?file=36769#file36769line160>
> >
> >     do we loose funtionality here
> >     
> >     i later saw it being done in calligratables but are we sure of the other apps?

Yes, this method and this line was inserted there (by me) only for tables.


> On Sept. 22, 2011, 8:08 p.m., C. Boemann wrote:
> > libs/kotext/KoTextEditor.h, line 231
> > <http://git.reviewboard.kde.org/r/102679/diff/1/?file=36803#file36803line231>
> >
> >     please improve this description.

Any suggestions? the total text is now

     * Insert the selection from the given KoTextEditor. If there is no selection, the entire
     * content of the document behind the editor is used. This changes the cursor position of
     * the editor instance.


> On Sept. 22, 2011, 8:08 p.m., C. Boemann wrote:
> > libs/kotext/KoTextEditor.cpp, line 978
> > <http://git.reviewboard.kde.org/r/102679/diff/1/?file=36804#file36804line978>
> >
> >     so you only want to copy if the source selects part of a table??

ah, fixed.


> On Sept. 22, 2011, 8:08 p.m., C. Boemann wrote:
> > libs/kotext/KoTextEditor.cpp, line 996
> > <http://git.reviewboard.kde.org/r/102679/diff/1/?file=36804#file36804line996>
> >
> >     editor-> ?? are you sure

Thanks, fixed.


> On Sept. 22, 2011, 8:08 p.m., C. Boemann wrote:
> > libs/kotext/commands/TextPasteCommand.cpp, line 100
> > <http://git.reviewboard.kde.org/r/102679/diff/1/?file=36818#file36818line100>
> >
> >     can this be fixed

Yes -- it probably was there already though, and you're the one who made me switch off cleanup whitespace in qtcreator :P


> On Sept. 22, 2011, 8:08 p.m., C. Boemann wrote:
> > plugins/textshape/dialogs/ParagraphSettingsDialog.cpp, line 33
> > <http://git.reviewboard.kde.org/r/102679/diff/1/?file=36854#file36854line33>
> >
> >     do we still need cursor here then?

Unfortunately, yes -- this is one place where the refactoring isn't completely done yet, just as much as was needed after moving the commands.


- Boudewijn


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


On Sept. 22, 2011, 7:14 p.m., Boudewijn Rempt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102679/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2011, 7:14 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Summary
> -------
> 
> KoTextEditor is meant to be the one and only interface we allow to editing a QTextDocument. TextTool breaks this encapsulation in many ways. This patch improves the situation but doesn't solve it completely yet. Several commands have been moved to kotext and encapsulated in KoTextEditor. This simplifies the code in the textshape quite a bit. The other bits will follow later on.
> 
> In order to make it possible to test this code, I wanted to be able to create a KoShapeController without a canvas, so KoShapeController was refactored a bit as well. Because KoShapeControllerBase was confusingly named, I renamed that class after irc discussion.
> 
> 
> Diffs
> -----
> 
>   braindump/src/SectionContainer.h ec00b6f 
>   braindump/src/ViewManager.h bf4e89c 
>   flow/plugins/dockers/stencilboxdocker/StencilShapeFactory.h dffa96e 
>   flow/plugins/dockers/stencilboxdocker/StencilShapeFactory.cpp 318e483 
>   karbon/common/commands/KarbonBooleanCommand.h 9c49a74 
>   karbon/common/commands/KarbonBooleanCommand.cpp 947ba83 
>   karbon/ui/KarbonPart.h 2dc5f85 
>   karbon/ui/dockers/KarbonLayerDocker.h 55ee45d 
>   karbon/ui/dockers/KarbonLayerDocker.cpp 30e1ab2 
>   karbon/ui/dockers/KarbonLayerModel.cpp f26666f 
>   krita/plugins/formats/odg/kis_odg_import.cc be07b8c 
>   krita/ui/canvas/kis_canvas2.h cf1d633 
>   krita/ui/canvas/kis_canvas2.cpp 6dc2b95 
>   krita/ui/flake/kis_shape_controller.h 4e0540a 
>   krita/ui/flake/kis_shape_layer.h 2ec5170 
>   krita/ui/flake/kis_shape_layer.cc 8004eae 
>   krita/ui/flake/kis_shape_layer_paste.h 096896f 
>   krita/ui/kis_doc2.h faa2e1f 
>   krita/ui/kis_doc2.cc ed11964 
>   libs/flake/CMakeLists.txt 4311bd0 
>   libs/flake/KoCanvasBase.h 5f8f0ab 
>   libs/flake/KoCanvasBase.cpp 2361bc1 
>   libs/flake/KoCanvasController.h 3fc370e 
>   libs/flake/KoDataCenterBase.h de447fa 
>   libs/flake/KoShape.h 84bdbfc 
>   libs/flake/KoShapeBasedDocumentBase.h PRE-CREATION 
>   libs/flake/KoShapeBasedDocumentBase.cpp PRE-CREATION 
>   libs/flake/KoShapeController.h e3c65ab 
>   libs/flake/KoShapeController.cpp 27722b3 
>   libs/flake/KoShapeControllerBase.h 5d1db1f 
>   libs/flake/KoShapeControllerBase.cpp 3f9d9a2 
>   libs/flake/KoShapeLoadingContext.h b0e0358 
>   libs/flake/KoShapeLoadingContext.cpp 50d50bb 
>   libs/flake/KoShapePaste.cpp d94dc11 
>   libs/flake/KoToolBase.h 13dc3c7 
>   libs/flake/KoToolBase.cpp 627591e 
>   libs/flake/KoToolManager.h 48cbaa8 
>   libs/flake/KoToolManager.cpp 8c612b0 
>   libs/flake/commands/KoPathCombineCommand.h a34edb5 
>   libs/flake/commands/KoPathCombineCommand.cpp 72aaed4 
>   libs/flake/commands/KoPathPointRemoveCommand.cpp fa748a6 
>   libs/flake/commands/KoPathSeparateCommand.h 4b52a60 
>   libs/flake/commands/KoPathSeparateCommand.cpp a7b0ab7 
>   libs/flake/commands/KoShapeClipCommand.h 883402c 
>   libs/flake/commands/KoShapeClipCommand.cpp 7159c91 
>   libs/flake/commands/KoShapeCreateCommand.h 4a9f3b8 
>   libs/flake/commands/KoShapeCreateCommand.cpp 6322c8d 
>   libs/flake/commands/KoShapeDeleteCommand.h 1d1eac4 
>   libs/flake/commands/KoShapeDeleteCommand.cpp 452ffea 
>   libs/flake/commands/KoShapeUnclipCommand.h 8903e33 
>   libs/flake/commands/KoShapeUnclipCommand.cpp 94d2308 
>   libs/flake/tests/MockShapes.h 3f7ba47 
>   libs/flake/tests/TestSnapStrategy.cpp 7f2b63b 
>   libs/kopageapp/KoPADocument.h cfc9822 
>   libs/kopageapp/KoPADocumentModel.cpp b968182 
>   libs/kopageapp/KoPADocumentStructureDocker.cpp 15c3224 
>   libs/kotext/CMakeLists.txt b174f5d 
>   libs/kotext/KoDocumentRdfBase.h 165f8fd 
>   libs/kotext/KoDocumentRdfBase.cpp f6445e6 
>   libs/kotext/KoTextCommandBase.h PRE-CREATION 
>   libs/kotext/KoTextCommandBase.cpp PRE-CREATION 
>   libs/kotext/KoTextDocument.cpp 1265e2c 
>   libs/kotext/KoTextEditor.h 4947db8 
>   libs/kotext/KoTextEditor.cpp d6d6738 
>   libs/kotext/KoTextOdfSaveHelper.h 4e247c2 
>   libs/kotext/KoTextOdfSaveHelper.cpp 9d5add8 
>   libs/kotext/KoTextPaste.h 61321d9 
>   libs/kotext/KoTextPaste.cpp 067d238 
>   libs/kotext/commands/ChangeListCommand.h PRE-CREATION 
>   libs/kotext/commands/ChangeListCommand.cpp PRE-CREATION 
>   libs/kotext/commands/ChangeTrackedDeleteCommand.h PRE-CREATION 
>   libs/kotext/commands/ChangeTrackedDeleteCommand.cpp PRE-CREATION 
>   libs/kotext/commands/DeleteCommand.h PRE-CREATION 
>   libs/kotext/commands/DeleteCommand.cpp PRE-CREATION 
>   libs/kotext/commands/ListItemNumberingCommand.h PRE-CREATION 
>   libs/kotext/commands/ListItemNumberingCommand.cpp PRE-CREATION 
>   libs/kotext/commands/TextPasteCommand.h PRE-CREATION 
>   libs/kotext/commands/TextPasteCommand.cpp PRE-CREATION 
>   libs/kotext/opendocument/KoTextWriter.h 04ea489 
>   libs/kotext/opendocument/KoTextWriter.cpp f5d9d77 
>   libs/kotext/tests/TestKoTextEditor.h 8013086 
>   libs/kotext/tests/TestKoTextEditor.cpp 85fab42 
>   libs/main/rdf/KoDocumentRdf.h 219ff42 
>   libs/main/rdf/KoDocumentRdf.cpp 42b557d 
>   plugins/dockers/shapecollection/CollectionShapeFactory.h 870fe2e 
>   plugins/dockers/shapecollection/CollectionShapeFactory.cpp 020e1af 
>   plugins/pictureshape/PictureShapeFactory.cpp 9ae730c 
>   plugins/pluginshape/PluginShapeFactory.cpp ee5f508 
>   plugins/textshape/CMakeLists.txt 3df7aa5 
>   plugins/textshape/TextShapeFactory.cpp 4100b47 
>   plugins/textshape/TextTool.h 6aaef61 
>   plugins/textshape/TextTool.cpp 142d934 
>   plugins/textshape/commands/AcceptChangeCommand.h 2945d9e 
>   plugins/textshape/commands/AcceptChangeCommand.cpp 66b121c 
>   plugins/textshape/commands/ChangeListCommand.h a7c2f7e 
>   plugins/textshape/commands/ChangeListCommand.cpp 8981a1b 
>   plugins/textshape/commands/ChangeListLevelCommand.h f657ee1 
>   plugins/textshape/commands/ChangeListLevelCommand.cpp bad0b68 
>   plugins/textshape/commands/ChangeTrackedDeleteCommand.h 6acf4bd 
>   plugins/textshape/commands/ChangeTrackedDeleteCommand.cpp f155681 
>   plugins/textshape/commands/DeleteCommand.h b85bbb9 
>   plugins/textshape/commands/DeleteCommand.cpp cd741dc 
>   plugins/textshape/commands/ListItemNumberingCommand.h 4457d84 
>   plugins/textshape/commands/ListItemNumberingCommand.cpp f00162b 
>   plugins/textshape/commands/RejectChangeCommand.h 925138d 
>   plugins/textshape/commands/RejectChangeCommand.cpp 3338875 
>   plugins/textshape/commands/ShowChangesCommand.h cacd86a 
>   plugins/textshape/commands/ShowChangesCommand.cpp e61f883 
>   plugins/textshape/commands/TextCommandBase.h d6306db 
>   plugins/textshape/commands/TextCommandBase.cpp be52032 
>   plugins/textshape/commands/TextCutCommand.cpp 31776f9 
>   plugins/textshape/commands/TextPasteCommand.h 90f4c3d 
>   plugins/textshape/commands/TextPasteCommand.cpp 36a1f76 
>   plugins/textshape/dialogs/ParagraphSettingsDialog.cpp cdddbe3 
>   plugins/textshape/dialogs/SimpleParagraphWidget.cpp 5b843ba 
>   plugins/textshape/tests/CMakeLists.txt e6ab42a 
>   plugins/videoshape/VideoShapeFactory.cpp f1c8d79 
>   stage/part/KPrPlaceholderStrategy.h e5ea2cb 
>   tables/Sheet.h 0e7285a 
>   tables/Sheet.cpp 6c15a2e 
>   tables/part/CanvasItem.cpp 1ba27a1 
>   tables/part/View.cpp 1384ec6 
>   words/part/KWDocument.h 4143803 
>   words/part/commands/KWFrameCreateCommand.h 354486d 
>   words/part/commands/KWFrameCreateCommand.cpp b4a4fb1 
>   words/part/commands/KWFrameDeleteCommand.h 1bc79b8 
>   words/part/commands/KWFrameDeleteCommand.cpp c5d3c4b 
> 
> Diff: http://git.reviewboard.kde.org/r/102679/diff
> 
> 
> Testing
> -------
> 
> manual gui test + ran the unittests. Added more testing.
> 
> 
> Thanks,
> 
> Boudewijn
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20110923/6335b5b6/attachment.htm>


More information about the calligra-devel mailing list