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.<br>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.<br>

<br>Pierre (on his way to the Oktoberfest)<br><br><div class="gmail_quote">On Fri, Sep 23, 2011 at 10:01 AM, C. Boemann <span dir="ltr"><<a href="mailto:cbo@boemann.dk">cbo@boemann.dk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">

Well yes but i thought we agreed on this. I mean, do you think this is not a<br>
step in the right direction?<br>
<div class="im"><br>
<br>
<br>
On Friday 23 September 2011 09:56:00 Pierre Stirnweiss wrote:<br>
> I thought we had planned to discuss the kotext/textlayout/textshape<br>
> architecture at the sprint.<br>
><br>
> Pierre<br>
><br>
> On Thu, Sep 22, 2011 at 10:08 PM, C. Boemann <<a href="mailto:cbr@boemann.dk">cbr@boemann.dk</a>> wrote:<br>
> >    This is an automatically generated e-mail. To reply, visit:<br>
> > <a href="http://git.reviewboard.kde.org/r/102679/" target="_blank">http://git.reviewboard.kde.org/r/102679/</a><br>
> ><br>
> > In general quite nice. A few remarks, and then i just assumed you moved<br>
> > the commands with maybe a little rename but i din't exactly read all of<br>
> > them. Please advice if i need to.<br>
> ><br>
> ><br>
> > For the kotexteditor::paste i think we should irc a bit<br>
> ><br>
</div>> >    libs/flake/KoShapeController.cpp<<a href="http://git.reviewboard.kde.org/r/1026" target="_blank">http://git.reviewboard.kde.org/r/1026</a><br>
> >    79/diff/1/?file=36769#file36769line160> (Diff<br>
<div class="im">> ><br>
> > revision 1)<br>
> ><br>
> > KUndo2Command* KoShapeController::addShapeDirect(KoShape *shape,<br>
> > KUndo2Command *parent)<br>
> ><br>
> >   160<br>
> ><br>
> >     KoToolManager::instance()->updateShapeControllerBase(shapeControllerB<br>
> >     ase, canvas->canvasController());<br>
> ><br>
> >  158<br>
> ><br>
> >     d->shapeBasedDocument = shapeBasedDocument;<br>
> ><br>
> >   do we loose funtionality here<br>
> ><br>
> > i later saw it being done in calligratables but are we sure of the other<br>
> > apps?<br>
> ><br>
</div>> >    libs/kotext/KoTextEditor.h<<a href="http://git.reviewboard.kde.org/r/102679/dif" target="_blank">http://git.reviewboard.kde.org/r/102679/dif</a><br>
> >    f/1/?file=36803#file36803line231> (Diff<br>
<div class="im">> ><br>
> > revision 1)<br>
> ><br>
> > public slots:<br>
> >    231<br>
> ><br>
> >      * Insert the selection from the given KoTextEditor<br>
> ><br>
> >   please improve this description.<br>
> ><br>
</div>> >    libs/kotext/KoTextEditor.cpp<<a href="http://git.reviewboard.kde.org/r/102679/d" target="_blank">http://git.reviewboard.kde.org/r/102679/d</a><br>
> >    iff/1/?file=36804#file36804line978> (Diff<br>
<div class="im">> ><br>
> > revision 1)<br>
> ><br>
> > bool KoTextEditor::paste(KoTextEditor *editor,<br>
> ><br>
> >    978<br>
> ><br>
> >     if (!editor->hasComplexSelection()) return;<br>
> ><br>
> >   so you only want to copy if the source selects part of a table??<br>
> ><br>
</div>> >    libs/kotext/KoTextEditor.cpp<<a href="http://git.reviewboard.kde.org/r/102679/d" target="_blank">http://git.reviewboard.kde.org/r/102679/d</a><br>
> >    iff/1/?file=36804#file36804line996> (Diff<br>
<div class="im">> ><br>
> > revision 1)<br>
> ><br>
> > bool KoTextEditor::paste(KoTextEditor *editor,<br>
> ><br>
> >    996<br>
> ><br>
> >         editor->addCommand(new DeleteCommand(DeleteCommand::NextChar,<br>
> >         d->document, shapeController));<br>
> ><br>
> >   editor-> ?? are you sure<br>
> ><br>
</div>> >    libs/kotext/commands/TextPasteCommand.cpp<<a href="http://git.reviewboard.kde.o" target="_blank">http://git.reviewboard.kde.o</a><br>
> >    rg/r/102679/diff/1/?file=36818#file36818line100> (Diff<br>
<div class="im">> ><br>
> > revision 1)<br>
> ><br>
> > void TextPasteCommand::redo()<br>
> ><br>
> >    100<br>
> ><br>
> >               can this be fixed<br>
> ><br>
</div>> >    plugins/textshape/dialogs/ParagraphSettingsDialog.cpp<<a href="http://git.revie" target="_blank">http://git.revie</a><br>
> >    <a href="http://wboard.kde.org/r/102679/diff/1/?file=36854#file36854line33" target="_blank">wboard.kde.org/r/102679/diff/1/?file=36854#file36854line33</a>> (Diff<br>
<div><div></div><div class="h5">> ><br>
> > revision 1)<br>
> ><br>
> >   32<br>
> ><br>
> > ParagraphSettingsDialog::ParagraphSettingsDialog(TextTool *tool,<br>
> > QTextCursor *cursor, QWidget* parent)<br>
> ><br>
> >  32<br>
> ><br>
> > ParagraphSettingsDialog::ParagraphSettingsDialog(TextTool *tool,<br>
> > QTextCursor *cursor, QWidget* parent)<br>
> ><br>
> >   do we still need cursor here then?<br>
> ><br>
> > - C.<br>
> ><br>
> > On September 22nd, 2011, 7:14 p.m., Boudewijn Rempt wrote:<br>
> >   Review request for Calligra.<br>
> ><br>
> > By Boudewijn Rempt.<br>
> ><br>
> > *Updated Sept. 22, 2011, 7:14 p.m.*<br>
> > Description<br>
> ><br>
> > KoTextEditor is meant to be the one and only interface we allow to<br>
> > editing a QTextDocument. TextTool breaks this encapsulation in many<br>
> > ways. This patch improves the situation but doesn't solve it completely<br>
> > yet. Several commands have been moved to kotext and encapsulated in<br>
> > KoTextEditor. This simplifies the code in the textshape quite a bit. The<br>
> > other bits will follow later on.<br>
> ><br>
> > In order to make it possible to test this code, I wanted to be able to<br>
> > create a KoShapeController without a canvas, so KoShapeController was<br>
> > refactored a bit as well. Because KoShapeControllerBase was confusingly<br>
> > named, I renamed that class after irc discussion.<br>
> ><br>
> >   Testing<br>
> ><br>
> > manual gui test + ran the unittests. Added more testing.<br>
> ><br>
> >   Diffs<br>
> ><br>
> >    - braindump/src/SectionContainer.h (ec00b6f)<br>
> >    - braindump/src/ViewManager.h (bf4e89c)<br>
> >    - flow/plugins/dockers/stencilboxdocker/StencilShapeFactory.h<br>
> >    (dffa96e) -<br>
> >    flow/plugins/dockers/stencilboxdocker/StencilShapeFactory.cpp<br>
> >    (318e483)<br>
> >    - karbon/common/commands/KarbonBooleanCommand.h (9c49a74)<br>
> >    - karbon/common/commands/KarbonBooleanCommand.cpp (947ba83)<br>
> >    - karbon/ui/KarbonPart.h (2dc5f85)<br>
> >    - karbon/ui/dockers/KarbonLayerDocker.h (55ee45d)<br>
> >    - karbon/ui/dockers/KarbonLayerDocker.cpp (30e1ab2)<br>
> >    - karbon/ui/dockers/KarbonLayerModel.cpp (f26666f)<br>
> >    - krita/plugins/formats/odg/kis_odg_import.cc (be07b8c)<br>
> >    - krita/ui/canvas/kis_canvas2.h (cf1d633)<br>
> >    - krita/ui/canvas/kis_canvas2.cpp (6dc2b95)<br>
> >    - krita/ui/flake/kis_shape_controller.h (4e0540a)<br>
> >    - krita/ui/flake/kis_shape_layer.h (2ec5170)<br>
> >    - krita/ui/flake/kis_shape_layer.cc (8004eae)<br>
> >    - krita/ui/flake/kis_shape_layer_paste.h (096896f)<br>
> >    - krita/ui/kis_doc2.h (faa2e1f)<br>
> >    - krita/ui/kis_doc2.cc (ed11964)<br>
> >    - libs/flake/CMakeLists.txt (4311bd0)<br>
> >    - libs/flake/KoCanvasBase.h (5f8f0ab)<br>
> >    - libs/flake/KoCanvasBase.cpp (2361bc1)<br>
> >    - libs/flake/KoCanvasController.h (3fc370e)<br>
> >    - libs/flake/KoDataCenterBase.h (de447fa)<br>
> >    - libs/flake/KoShape.h (84bdbfc)<br>
> >    - libs/flake/KoShapeBasedDocumentBase.h (PRE-CREATION)<br>
> >    - libs/flake/KoShapeBasedDocumentBase.cpp (PRE-CREATION)<br>
> >    - libs/flake/KoShapeController.h (e3c65ab)<br>
> >    - libs/flake/KoShapeController.cpp (27722b3)<br>
> >    - libs/flake/KoShapeControllerBase.h (5d1db1f)<br>
> >    - libs/flake/KoShapeControllerBase.cpp (3f9d9a2)<br>
> >    - libs/flake/KoShapeLoadingContext.h (b0e0358)<br>
> >    - libs/flake/KoShapeLoadingContext.cpp (50d50bb)<br>
> >    - libs/flake/KoShapePaste.cpp (d94dc11)<br>
> >    - libs/flake/KoToolBase.h (13dc3c7)<br>
> >    - libs/flake/KoToolBase.cpp (627591e)<br>
> >    - libs/flake/KoToolManager.h (48cbaa8)<br>
> >    - libs/flake/KoToolManager.cpp (8c612b0)<br>
> >    - libs/flake/commands/KoPathCombineCommand.h (a34edb5)<br>
> >    - libs/flake/commands/KoPathCombineCommand.cpp (72aaed4)<br>
> >    - libs/flake/commands/KoPathPointRemoveCommand.cpp (fa748a6)<br>
> >    - libs/flake/commands/KoPathSeparateCommand.h (4b52a60)<br>
> >    - libs/flake/commands/KoPathSeparateCommand.cpp (a7b0ab7)<br>
> >    - libs/flake/commands/KoShapeClipCommand.h (883402c)<br>
> >    - libs/flake/commands/KoShapeClipCommand.cpp (7159c91)<br>
> >    - libs/flake/commands/KoShapeCreateCommand.h (4a9f3b8)<br>
> >    - libs/flake/commands/KoShapeCreateCommand.cpp (6322c8d)<br>
> >    - libs/flake/commands/KoShapeDeleteCommand.h (1d1eac4)<br>
> >    - libs/flake/commands/KoShapeDeleteCommand.cpp (452ffea)<br>
> >    - libs/flake/commands/KoShapeUnclipCommand.h (8903e33)<br>
> >    - libs/flake/commands/KoShapeUnclipCommand.cpp (94d2308)<br>
> >    - libs/flake/tests/MockShapes.h (3f7ba47)<br>
> >    - libs/flake/tests/TestSnapStrategy.cpp (7f2b63b)<br>
> >    - libs/kopageapp/KoPADocument.h (cfc9822)<br>
> >    - libs/kopageapp/KoPADocumentModel.cpp (b968182)<br>
> >    - libs/kopageapp/KoPADocumentStructureDocker.cpp (15c3224)<br>
> >    - libs/kotext/CMakeLists.txt (b174f5d)<br>
> >    - libs/kotext/KoDocumentRdfBase.h (165f8fd)<br>
> >    - libs/kotext/KoDocumentRdfBase.cpp (f6445e6)<br>
> >    - libs/kotext/KoTextCommandBase.h (PRE-CREATION)<br>
> >    - libs/kotext/KoTextCommandBase.cpp (PRE-CREATION)<br>
> >    - libs/kotext/KoTextDocument.cpp (1265e2c)<br>
> >    - libs/kotext/KoTextEditor.h (4947db8)<br>
> >    - libs/kotext/KoTextEditor.cpp (d6d6738)<br>
> >    - libs/kotext/KoTextOdfSaveHelper.h (4e247c2)<br>
> >    - libs/kotext/KoTextOdfSaveHelper.cpp (9d5add8)<br>
> >    - libs/kotext/KoTextPaste.h (61321d9)<br>
> >    - libs/kotext/KoTextPaste.cpp (067d238)<br>
> >    - libs/kotext/commands/ChangeListCommand.h (PRE-CREATION)<br>
> >    - libs/kotext/commands/ChangeListCommand.cpp (PRE-CREATION)<br>
> >    - libs/kotext/commands/ChangeTrackedDeleteCommand.h (PRE-CREATION)<br>
> >    - libs/kotext/commands/ChangeTrackedDeleteCommand.cpp (PRE-CREATION)<br>
> >    - libs/kotext/commands/DeleteCommand.h (PRE-CREATION)<br>
> >    - libs/kotext/commands/DeleteCommand.cpp (PRE-CREATION)<br>
> >    - libs/kotext/commands/ListItemNumberingCommand.h (PRE-CREATION)<br>
> >    - libs/kotext/commands/ListItemNumberingCommand.cpp (PRE-CREATION)<br>
> >    - libs/kotext/commands/TextPasteCommand.h (PRE-CREATION)<br>
> >    - libs/kotext/commands/TextPasteCommand.cpp (PRE-CREATION)<br>
> >    - libs/kotext/opendocument/KoTextWriter.h (04ea489)<br>
> >    - libs/kotext/opendocument/KoTextWriter.cpp (f5d9d77)<br>
> >    - libs/kotext/tests/TestKoTextEditor.h (8013086)<br>
> >    - libs/kotext/tests/TestKoTextEditor.cpp (85fab42)<br>
> >    - libs/main/rdf/KoDocumentRdf.h (219ff42)<br>
> >    - libs/main/rdf/KoDocumentRdf.cpp (42b557d)<br>
> >    - plugins/dockers/shapecollection/CollectionShapeFactory.h (870fe2e)<br>
> >    - plugins/dockers/shapecollection/CollectionShapeFactory.cpp (020e1af)<br>
> >    - plugins/pictureshape/PictureShapeFactory.cpp (9ae730c)<br>
> >    - plugins/pluginshape/PluginShapeFactory.cpp (ee5f508)<br>
> >    - plugins/textshape/CMakeLists.txt (3df7aa5)<br>
> >    - plugins/textshape/TextShapeFactory.cpp (4100b47)<br>
> >    - plugins/textshape/TextTool.h (6aaef61)<br>
> >    - plugins/textshape/TextTool.cpp (142d934)<br>
> >    - plugins/textshape/commands/AcceptChangeCommand.h (2945d9e)<br>
> >    - plugins/textshape/commands/AcceptChangeCommand.cpp (66b121c)<br>
> >    - plugins/textshape/commands/ChangeListCommand.h (a7c2f7e)<br>
> >    - plugins/textshape/commands/ChangeListCommand.cpp (8981a1b)<br>
> >    - plugins/textshape/commands/ChangeListLevelCommand.h (f657ee1)<br>
> >    - plugins/textshape/commands/ChangeListLevelCommand.cpp (bad0b68)<br>
> >    - plugins/textshape/commands/ChangeTrackedDeleteCommand.h (6acf4bd)<br>
> >    - plugins/textshape/commands/ChangeTrackedDeleteCommand.cpp (f155681)<br>
> >    - plugins/textshape/commands/DeleteCommand.h (b85bbb9)<br>
> >    - plugins/textshape/commands/DeleteCommand.cpp (cd741dc)<br>
> >    - plugins/textshape/commands/ListItemNumberingCommand.h (4457d84)<br>
> >    - plugins/textshape/commands/ListItemNumberingCommand.cpp (f00162b)<br>
> >    - plugins/textshape/commands/RejectChangeCommand.h (925138d)<br>
> >    - plugins/textshape/commands/RejectChangeCommand.cpp (3338875)<br>
> >    - plugins/textshape/commands/ShowChangesCommand.h (cacd86a)<br>
> >    - plugins/textshape/commands/ShowChangesCommand.cpp (e61f883)<br>
> >    - plugins/textshape/commands/TextCommandBase.h (d6306db)<br>
> >    - plugins/textshape/commands/TextCommandBase.cpp (be52032)<br>
> >    - plugins/textshape/commands/TextCutCommand.cpp (31776f9)<br>
> >    - plugins/textshape/commands/TextPasteCommand.h (90f4c3d)<br>
> >    - plugins/textshape/commands/TextPasteCommand.cpp (36a1f76)<br>
> >    - plugins/textshape/dialogs/ParagraphSettingsDialog.cpp (cdddbe3)<br>
> >    - plugins/textshape/dialogs/SimpleParagraphWidget.cpp (5b843ba)<br>
> >    - plugins/textshape/tests/CMakeLists.txt (e6ab42a)<br>
> >    - plugins/videoshape/VideoShapeFactory.cpp (f1c8d79)<br>
> >    - stage/part/KPrPlaceholderStrategy.h (e5ea2cb)<br>
> >    - tables/Sheet.h (0e7285a)<br>
> >    - tables/Sheet.cpp (6c15a2e)<br>
> >    - tables/part/CanvasItem.cpp (1ba27a1)<br>
> >    - tables/part/View.cpp (1384ec6)<br>
> >    - words/part/KWDocument.h (4143803)<br>
> >    - words/part/commands/KWFrameCreateCommand.h (354486d)<br>
> >    - words/part/commands/KWFrameCreateCommand.cpp (b4a4fb1)<br>
> >    - words/part/commands/KWFrameDeleteCommand.h (1bc79b8)<br>
> >    - words/part/commands/KWFrameDeleteCommand.cpp (c5d3c4b)<br>
> ><br>
</div></div>> > View Diff <<a href="http://git.reviewboard.kde.org/r/102679/diff/" target="_blank">http://git.reviewboard.kde.org/r/102679/diff/</a>><br>
<div><div></div><div class="h5">> ><br>
> > _______________________________________________<br>
> > calligra-devel mailing list<br>
> > <a href="mailto:calligra-devel@kde.org">calligra-devel@kde.org</a><br>
> > <a href="https://mail.kde.org/mailman/listinfo/calligra-devel" target="_blank">https://mail.kde.org/mailman/listinfo/calligra-devel</a><br>
_______________________________________________<br>
calligra-devel mailing list<br>
<a href="mailto:calligra-devel@kde.org">calligra-devel@kde.org</a><br>
<a href="https://mail.kde.org/mailman/listinfo/calligra-devel" target="_blank">https://mail.kde.org/mailman/listinfo/calligra-devel</a><br>
</div></div></blockquote></div><br>