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

Thorsten Zachmann t.zachmann at zagge.de
Sat Sep 24 07:33:18 BST 2011 

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



karbon/common/commands/KarbonBooleanCommand.cpp
<http://git.reviewboard.kde.org/r/102679/#comment5985>

    I think it would be good if the variable would be renamed too as not it says it is controller but the as the last name of the variable and that will lead to more confuision I guess.
    
    



kexi/doc/dev/CHANGELOG-Kexi-js
<http://git.reviewboard.kde.org/r/102679/#comment5986>

    Is this one of your changes?



kexi/main/KexiMainWindow.cpp
<http://git.reviewboard.kde.org/r/102679/#comment5987>

    These changes seem to be not to belong to your patch



kexi/main/KexiMainWindow.cpp
<http://git.reviewboard.kde.org/r/102679/#comment5988>

    This seems to be a unrelated change. Maybe something wrong with the patch?



krita/plugins/paintops/libpaintop/sensors/kis_dynamic_sensors.cc
<http://git.reviewboard.kde.org/r/102679/#comment5989>

    This seems to be a unrelated change. Maybe something wrong with the patch? 



libs/flake/KoShapeController.cpp
<http://git.reviewboard.kde.org/r/102679/#comment5990>

    Maybe add a comment that the canvas call here is needed for the tests to work without a one.
    
    Maybe it would be food to have one big if for the canvas as now it needs to be tested 2 times.



libs/kotext/KoDocumentRdfBase.h
<http://git.reviewboard.kde.org/r/102679/#comment5991>

    This seems to be a unrelated change



libs/kotext/KoTextCommandBase.h
<http://git.reviewboard.kde.org/r/102679/#comment5992>

    Please update to the new class name to be consistent.



libs/kotext/KoTextCommandBase.h
<http://git.reviewboard.kde.org/r/102679/#comment5993>

    As this is now part of the libs and exported it would be good if the implementation is moved to the cpp file.



libs/kotext/KoTextEditor.h
<http://git.reviewboard.kde.org/r/102679/#comment5995>

    I think it would be nice to descript the paramters here. For the resourceManager you need to make sure to pass the document resource manager and not the canvas one.



libs/kotext/KoTextPaste.h
<http://git.reviewboard.kde.org/r/102679/#comment5994>

    Maybe this should mention it is the document resouce manager (as we also have a canvas resource manager) just to make sure the right one is used.



libs/main/rdf/KoDocumentRdf.h
<http://git.reviewboard.kde.org/r/102679/#comment6003>

    The KoDocumentRdf gets never added the the CanvasResourceManager. Therefor this comment is wrong. Maybe you can cleen this up.



plugins/textshape/TextTool.cpp
<http://git.reviewboard.kde.org/r/102679/#comment5996>

    It makes no sense a resource is either a document resource or a canvas resource. And there should be always a document when there is a canvas. Please only use the document resource manager. I also checked the code and it is added only to the document resource manager.



plugins/textshape/TextTool.cpp
<http://git.reviewboard.kde.org/r/102679/#comment5997>

    This needs to use the document resource manager here. Actually you don't need to pass it to the command as it can be get from the canvas->shapeController() that is passed here by the method resourceManager(). 



plugins/textshape/TextTool.cpp
<http://git.reviewboard.kde.org/r/102679/#comment5998>

    This needs to use the document resource manager here. Actually you don't need to pass it to the command as it can be get from the canvas->shapeController() that is passed here by the method resourceManager(). 



plugins/textshape/TextTool.cpp
<http://git.reviewboard.kde.org/r/102679/#comment5999>

    This needs to use the document resource manager here. Actually you don't need to pass it to the command as it can be get from the canvas->shapeController() that is passed here by the method resourceManager().



plugins/textshape/TextTool.cpp
<http://git.reviewboard.kde.org/r/102679/#comment6000>

    This needs to use the document resource manager here. Actually you don't need to pass it to the command as it can be get from the canvas->shapeController() that is passed here by the method resourceManager().



plugins/textshape/TextTool.cpp
<http://git.reviewboard.kde.org/r/102679/#comment6001>

    This needs to use the document resource manager here. Actually you don't need to pass it to the command as it can be get from the canvas->shapeController() that is passed here by the method resourceManager().



plugins/textshape/commands/TextCutCommand.cpp
<http://git.reviewboard.kde.org/r/102679/#comment6002>

    This needs to use the document resource manager here. Actually you don't need to pass it to the command as it can be get from the canvas->shapeController() that is passed here by the method resourceManager().


- Thorsten


On Sept. 23, 2011, 8:44 a.m., Boudewijn Rempt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102679/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2011, 8:44 a.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 
>   kdgantt/kdgantttimescalezoomdialog.cpp 86fcb32 
>   kexi/doc/dev/CHANGELOG-Kexi-js f99afca 
>   kexi/main/KexiMainWindow.cpp 626ccf6 
>   krita/plugins/formats/odg/kis_odg_import.cc be07b8c 
>   krita/plugins/paintops/libpaintop/sensors/kis_dynamic_sensors.cc 16473f5 
>   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 
>   plan/libs/models/kptschedulemodel.cpp 1241685 
>   plan/libs/ui/reports/reportview.cpp efc6677 
>   plan/plugins/schedulers/tj/taskjuggler/Task.cpp f18cda9 
>   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/20110924/034f0369/attachment.htm>

More information about the calligra-devel mailing list