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

C. Boemann cbo at boemann.dk
Fri Sep 23 09:32:46 BST 2011 

ah the irc suggestion was on the specifics of a paste method, and not related 
to architecture

have fun

On Friday 23 September 2011 10:10:13 Pierre Stirnweiss wrote:
> I didn't mean to make an assessement on the patch actually. I was answering
> to the IRC suggestion actually. I indeed personnaly think that the
> textshape is not the place for the commands.
> However, we should also try to keep an open mind for the exercise at the
> sprint. Otherwise we might format ourselves in merely tweaking the current
> broken design of the textshape/kotext relationship.
> 
> Pierre (on his way to the Oktoberfest)
> 
> On Fri, Sep 23, 2011 at 10:01 AM, C. Boemann <cbo at boemann.dk> wrote:
> > Well yes but i thought we agreed on this. I mean, do you think this is
> > not a
> > step in the right direction?
> > 
> > On Friday 23 September 2011 09:56:00 Pierre Stirnweiss wrote:
> > > I thought we had planned to discuss the kotext/textlayout/textshape
> > > architecture at the sprint.
> > > 
> > > Pierre
> > > 
> > > On Thu, Sep 22, 2011 at 10:08 PM, C. Boemann <cbr at boemann.dk> wrote:
> > > >    This is an automatically generated e-mail. To reply, visit:
> > > > http://git.reviewboard.kde.org/r/102679/
> > > > 
> > > > 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
> > > > 
> > > >    libs/flake/KoShapeController.cpp<
> > 
> > http://git.reviewboard.kde.org/r/1026
> > 
> > > >    79/diff/1/?file=36769#file36769line160> (Diff
> > > > 
> > > > revision 1)
> > > > 
> > > > KUndo2Command* KoShapeController::addShapeDirect(KoShape *shape,
> > > > KUndo2Command *parent)
> > > > 
> > > >   160
> > 
> > KoToolManager::instance()->updateShapeControllerBase(shapeControllerB
> > 
> > > >     ase, canvas->canvasController());
> > > >  
> > > >  158
> > > >  
> > > >     d->shapeBasedDocument = shapeBasedDocument;
> > > >   
> > > >   do we loose funtionality here
> > > > 
> > > > i later saw it being done in calligratables but are we sure of the
> > 
> > other
> > 
> > > > apps?
> > > > 
> > > >    libs/kotext/KoTextEditor.h<
> > 
> > http://git.reviewboard.kde.org/r/102679/dif
> > 
> > > >    f/1/?file=36803#file36803line231> (Diff
> > > > 
> > > > revision 1)
> > > > 
> > > > public slots:
> > > >    231
> > > >    
> > > >      * Insert the selection from the given KoTextEditor
> > > >   
> > > >   please improve this description.
> > > >   
> > > >    libs/kotext/KoTextEditor.cpp<
> > 
> > http://git.reviewboard.kde.org/r/102679/d
> > 
> > > >    iff/1/?file=36804#file36804line978> (Diff
> > > > 
> > > > revision 1)
> > > > 
> > > > bool KoTextEditor::paste(KoTextEditor *editor,
> > > > 
> > > >    978
> > > >    
> > > >     if (!editor->hasComplexSelection()) return;
> > > >   
> > > >   so you only want to copy if the source selects part of a table??
> > > >   
> > > >    libs/kotext/KoTextEditor.cpp<
> > 
> > http://git.reviewboard.kde.org/r/102679/d
> > 
> > > >    iff/1/?file=36804#file36804line996> (Diff
> > > > 
> > > > revision 1)
> > > > 
> > > > bool KoTextEditor::paste(KoTextEditor *editor,
> > > > 
> > > >    996
> > > >    
> > > >         editor->addCommand(new DeleteCommand(DeleteCommand::NextChar,
> > > >         d->document, shapeController));
> > > >   
> > > >   editor-> ?? are you sure
> > > >   
> > > >    libs/kotext/commands/TextPasteCommand.cpp<
> > 
> > http://git.reviewboard.kde.o
> > 
> > > >    rg/r/102679/diff/1/?file=36818#file36818line100> (Diff
> > > > 
> > > > revision 1)
> > > > 
> > > > void TextPasteCommand::redo()
> > > > 
> > > >    100
> > > >    
> > > >               can this be fixed
> > > >    
> > > >    plugins/textshape/dialogs/ParagraphSettingsDialog.cpp<
> > 
> > http://git.revie
> > 
> > > >    wboard.kde.org/r/102679/diff/1/?file=36854#file36854line33> (Diff
> > > > 
> > > > revision 1)
> > > > 
> > > >   32
> > > > 
> > > > ParagraphSettingsDialog::ParagraphSettingsDialog(TextTool *tool,
> > > > QTextCursor *cursor, QWidget* parent)
> > > > 
> > > >  32
> > > > 
> > > > ParagraphSettingsDialog::ParagraphSettingsDialog(TextTool *tool,
> > > > QTextCursor *cursor, QWidget* parent)
> > > > 
> > > >   do we still need cursor here then?
> > > > 
> > > > - C.
> > > > 
> > > > On September 22nd, 2011, 7:14 p.m., Boudewijn Rempt wrote:
> > > >   Review request for Calligra.
> > > > 
> > > > By Boudewijn Rempt.
> > > > 
> > > > *Updated Sept. 22, 2011, 7:14 p.m.*
> > > > Description
> > > > 
> > > > 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.
> > > > 
> > > >   Testing
> > > > 
> > > > manual gui test + ran the unittests. Added more testing.
> > > > 
> > > >   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)
> > > > 
> > > > View Diff <http://git.reviewboard.kde.org/r/102679/diff/>
> > > > 
> > > > _______________________________________________
> > > > calligra-devel mailing list
> > > > calligra-devel at kde.org
> > > > https://mail.kde.org/mailman/listinfo/calligra-devel
> > 
> > _______________________________________________
> > calligra-devel mailing list
> > calligra-devel at kde.org
> > https://mail.kde.org/mailman/listinfo/calligra-devel

More information about the calligra-devel mailing list