Review Request: Move several commands from TextTool to the KoTextEditor interface
Pierre Stirnweiss
pstirnweiss at googlemail.com
Fri Sep 23 09:10:13 BST 2011
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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20110923/6913af27/attachment.htm>
More information about the calligra-devel
mailing list