Review Request 111545: Port komain away from KParts

Friedrich W. H. Kossebau kossebau at kde.org
Tue Jul 23 19:19:07 BST 2013 

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


Still not completely reviewed. At least a set of new comments, includes work for you already :P

And patch still includes the deletion of krita/sdk, which should be done in a separate commit (not only to make this one smaller and easier to review).


CMakeLists.txt
<http://git.reviewboard.kde.org/r/111545/#comment26873>

    in a recent discussion people agreed to not use the "ko" prefix in the dir name of the libs. Patch to normalize all folders in libs/ needs yet to be done. But "libs/komvc" could already be "libs/mvc" in your patch.
    
    But then libs/komvc is not yet part of this patch. Any reason to have it included already?



karbon/plugins/flattenpath/CMakeLists.txt
<http://git.reviewboard.kde.org/r/111545/#comment26874>

    karbon/karbonplugins seems a little redundant to me. Any reason for the prefix "karbon" to "plugins"?



plan/kptview.cpp
<http://git.reviewboard.kde.org/r/111545/#comment26884>

    No idea on that, but that is fundamental when changing the views. So needs proper replacement :/
    Not familiar enough with Plan (yet) to tell what and how



sheets/part/View.cpp
<http://git.reviewboard.kde.org/r/111545/#comment26882>

    With one view this calcStatusBarOp() no longer needs to be called?
    What about MDI and switching the view? Then that calcLabel might need updating still.
    So something equivalent to trigger this will be needed.
    At least add a TODO.



sheets/ui/MapViewModel.cpp
<http://git.reviewboard.kde.org/r/111545/#comment26883>

    with one view (or MDI) this action plugging is still needed. Just load a document with multiple sheets and see that the actions are not there.
    Please add a TODO or add to some proper place (view creation?).


- Friedrich W. H. Kossebau


On July 22, 2013, 7:10 a.m., Boudewijn Rempt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111545/
> -----------------------------------------------------------
> 
> (Updated July 22, 2013, 7:10 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> This is a continuation of the KoDocument/KoPart split that Camilla and I worked on last year, and it is a step towards the MVC refactoring discussed in Essen.
> 
> We actually did not use any of the kparts functionality anymore: not the embedding of Calligra parts in other applications, not document embedding, and the various plugins that were kparts actually did not do anything beyond what a kxmlguiclient plugin did.
> 
> This patch makes KoView and KoPart a KXMLGuiClient and lets KoMainWindow manage the KoParts. It is the first step in further refactoring:
> 
> * Make KoMainWindow a generic window for Calligra apps
> * Make KoDocument responsible for all loading and saving
> * Make KoPart a factory class that can create documents, views and canvaes
> * Make KoView more properly an application-specific mainwindow implementation for an application that can show one or more documents
> 
> Current issues:
> 
> * In CalligraPlan I have commented out some code that didn't seem to actually do anything but was dependent on KParts. That needs to be checked by Friedrich for regressions
> * KoMainWindow has way too many local KoPart pointers... That needs to be cleaned up.
> 
> Then I want to move the load/save code from KoPart to KoDocument and start the bigger refactoring... That will hopefully give us split view/mdi/tabbed view of documents again. And make the flipbook and animation plugins for krita easier to handle.
> 
> For easy testing I recommend checking out the krita-mvc-rempt branch.
> 
> 
> This addresses bugs 310310, 313482 and 313483.
>     http://bugs.kde.org/show_bug.cgi?id=310310
>     http://bugs.kde.org/show_bug.cgi?id=313482
>     http://bugs.kde.org/show_bug.cgi?id=313483
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 38c3f62 
>   braindump/plugins/quickstates/BraindumpQuickStatesPlugin.h b9625b0 
>   braindump/plugins/quickstates/BraindumpQuickStatesPlugin.cpp bc27946 
>   braindump/plugins/quickstates/CMakeLists.txt ecadaea 
>   braindump/src/CMakeLists.txt 7bfe50b 
>   braindump/src/View.cpp ef19171 
>   cmake/modules/FindCalligraLibs.cmake.in 1e49d17 
>   devtools/cstester/cstester.cpp b88619b 
>   extras/okularodpgenerator/OkularOdpGenerator.cpp db1d5e5 
>   flow/part/FlowPart.cpp 5fedf83 
>   karbon/data/CMakeLists.txt 6b1e329 
>   karbon/data/karbon_module.desktop 7acc680 
>   karbon/data/karbon_viewplugin.desktop PRE-CREATION 
>   karbon/plugins/flattenpath/CMakeLists.txt f83620b 
>   karbon/plugins/flattenpath/FlattenPathPlugin.h 622973c 
>   karbon/plugins/flattenpath/FlattenPathPlugin.cpp d03ccca 
>   karbon/plugins/flattenpath/karbonflattenpathplugin.desktop PRE-CREATION 
>   karbon/plugins/refinepath/CMakeLists.txt caf6aa7 
>   karbon/plugins/refinepath/RefinePathPlugin.h be16e29 
>   karbon/plugins/refinepath/RefinePathPlugin.cpp b951adc 
>   karbon/plugins/refinepath/karbonrefinepath.desktop PRE-CREATION 
>   karbon/plugins/roundcorners/CMakeLists.txt 82b7159 
>   karbon/plugins/roundcorners/RoundCornersPlugin.h 5cd610a 
>   karbon/plugins/roundcorners/RoundCornersPlugin.cpp 7ac63bf 
>   karbon/plugins/roundcorners/karbonroundcornersplugin.desktop PRE-CREATION 
>   karbon/plugins/whirlpinch/CMakeLists.txt 2e920e2 
>   karbon/plugins/whirlpinch/WhirlPinchPlugin.h 51280a1 
>   karbon/plugins/whirlpinch/WhirlPinchPlugin.cpp 94a3527 
>   karbon/plugins/whirlpinch/karbonwhirlpinchplugin.desktop PRE-CREATION 
>   karbon/ui/KarbonFactory.cpp 8bbabbe 
>   karbon/ui/KarbonPart.cpp be13dfe 
>   karbon/ui/KarbonView.cpp 7e54ede 
>   karbon/ui/dockers/KarbonLayerDocker.cpp 1cf8395 
>   krita/plugins/assistants/RulerAssistant/CMakeLists.txt 957fb4f 
>   krita/plugins/extensions/shiva/shivagenerator.h 55894f4 
>   krita/plugins/paintops/libbrush/kis_brush_registry.cpp f54a619 
>   krita/plugins/paintops/particle/particle_paintop_plugin.h bd6b245 
>   krita/sdk/createplugin.rb 1f52c38 
>   krita/sdk/createtest.py 4b7843f 
>   krita/sdk/pop_extract.rb 0d88588 
>   krita/sdk/templates/docker/APPNAMELC.h 56c8d19 
>   krita/sdk/templates/docker/APPNAMELC.cc 740de47 
>   krita/sdk/templates/docker/APPNAMELC_dock.h 919d5d5 
>   krita/sdk/templates/docker/APPNAMELC_dock.cc 6379bfb 
>   krita/sdk/templates/docker/CMakeLists.txt f671812 
>   krita/sdk/templates/docker/krita_APPNAMELC.desktop f287069 
>   krita/sdk/templates/filefilter/APPNAMELC_converter.h ddbd5d0 
>   krita/sdk/templates/filefilter/APPNAMELC_converter.cc 2da0b2f 
>   krita/sdk/templates/filefilter/APPNAMELC_export.h 4e8a612 
>   krita/sdk/templates/filefilter/APPNAMELC_export.cc 9798937 
>   krita/sdk/templates/filefilter/APPNAMELC_import.h e2672a7 
>   krita/sdk/templates/filefilter/APPNAMELC_import.cc b063ec3 
>   krita/sdk/templates/filefilter/CMakeLists.txt bc7afdc 
>   krita/sdk/templates/filefilter/krita_APPNAMELC.desktop 736f9a5 
>   krita/sdk/templates/filefilter/krita_APPNAMELC_export.desktop 81ac62e 
>   krita/sdk/templates/filefilter/krita_APPNAMELC_import.desktop 0542ae3 
>   krita/sdk/templates/filter/APPNAMELC.h 141d813 
>   krita/sdk/templates/filter/APPNAMELC.cc 1ff583b 
>   krita/sdk/templates/filter/CMakeLists.txt f5e94e9 
>   krita/sdk/templates/filter/kritaAPPNAMELC.desktop b5b86af 
>   krita/sdk/templates/tool/CMakeLists.txt f4ec68c 
>   krita/sdk/templates/tool/kis_tool_APPNAMELC.h 01e49fa 
>   krita/sdk/templates/tool/kis_tool_APPNAMELC.cc 9ab759b 
>   krita/sdk/templates/tool/kritatoolAPPNAMELC.desktop af9ea01 
>   krita/sdk/templates/tool/tool_APPNAMELC.h ce9a416 
>   krita/sdk/templates/tool/tool_APPNAMELC.cc 1a162fa 
>   krita/sdk/templates/view/APPNAMELC.h 960576f 
>   krita/sdk/templates/view/APPNAMELC.cc cc761f6 
>   krita/sdk/templates/view/APPNAMELC.rc 3c9f9aa 
>   krita/sdk/templates/view/CMakeLists.txt 65be2c7 
>   krita/sdk/templates/view/kritaAPPNAMELC.desktop f287069 
>   krita/ui/kis_factory2.h 888b742 
>   krita/ui/kis_factory2.cc b7896e5 
>   krita/ui/kis_paintop_box.cc e97a400 
>   krita/ui/kis_part2.cpp 83d5f34 
>   krita/ui/kis_view2.cpp 832a893 
>   krita/ui/kis_view_plugin.h ff19b2a 
>   krita/ui/kis_view_plugin.cpp 69f5b9d 
>   libs/kokross/KoScriptingPart.h 4f4c933 
>   libs/kokross/KoScriptingPart.cpp 09b8cbb 
>   libs/kopageapp/KoPAView.h 80b9f78 
>   libs/kopageapp/KoPAView.cpp 58b4940 
>   libs/kopageapp/tests/CMakeLists.txt 0d44ee3 
>   libs/main/CMakeLists.txt 9b8503b 
>   libs/main/KoDocument.h 3372060 
>   libs/main/KoDocument.cpp 1738778 
>   libs/main/KoDocumentEntry.cpp e1b03f7 
>   libs/main/KoDocument_p.h 6f3931f 
>   libs/main/KoFactory.h dc4ada0 
>   libs/main/KoFactory.cpp c361a11 
>   libs/main/KoFilterEntry.cpp 8e23eac 
>   libs/main/KoMainWindow.h 1b3503f 
>   libs/main/KoMainWindow.cpp 12423a7 
>   libs/main/KoPart.h d4ecf5f 
>   libs/main/KoPart.cpp 56e27b5 
>   libs/main/KoServiceProvider.h 35d603a 
>   libs/main/KoServiceProvider.cpp f063401 
>   libs/main/KoView.h 10b6403 
>   libs/main/KoView.cpp 1b4a722 
>   libs/rdf/CMakeLists.txt bd29ead 
>   plan/CMakeLists.txt 4ff06eb 
>   plan/kptmaindocument.cpp 333310c 
>   plan/kptpart.cpp 7674480 
>   plan/kptview.h 02acb77 
>   plan/kptview.cpp 537e047 
>   plan/plan_viewplugin.desktop PRE-CREATION 
>   plan/plugins/scripting/CMakeLists.txt 836176d 
>   plan/plugins/scripting/ScriptingPart.cpp e6d6004 
>   plan/plugins/scripting/planscripting.desktop PRE-CREATION 
>   plan/workpackage/CMakeLists.txt a1fbb87 
>   plugins/chartshape/ChartShape.cpp 55d5eb8 
>   plugins/colorengines/lcms2/CMakeLists.txt faa07a9 
>   plugins/colorengines/lcms2/LcmsEnginePlugin.h 36a5497 
>   plugins/kexi/spreadsheet/CMakeLists.txt 78fbe7e 
>   plugins/reporting/barcode/CMakeLists.txt 7423872 
>   plugins/reporting/chart/CMakeLists.txt a0d6378 
>   plugins/reporting/maps/CMakeLists.txt 6bc29a0 
>   plugins/reporting/web/CMakeLists.txt 03a9aeb 
>   plugins/staging/CMakeLists.txt 754362f 
>   plugins/staging/googledocs/onlinedocument.h d4141f3 
>   plugins/staging/googledocs/onlinedocument.cpp aca9fa7 
>   plugins/staging/pivottables/CMakeLists.txt efb2753 
>   plugins/staging/pivottables/pivotplugin.h 17fc384 
>   plugins/staging/pivottables/pivotplugin.cpp 1bef7d2 
>   plugins/staging/pivottables/sheetspivottables.desktop PRE-CREATION 
>   sheets/CMakeLists.txt e0f4a02 
>   sheets/part/Part.cpp 7fd573e 
>   sheets/part/View.h 49bcdf6 
>   sheets/part/View.cpp 0dc055a 
>   sheets/plugins/scripting/CMakeLists.txt 27e3861 
>   sheets/plugins/scripting/ScriptingPart.h ac9e614 
>   sheets/plugins/scripting/ScriptingPart.cpp 26431b0 
>   sheets/plugins/scripting/sheetsscripting.desktop PRE-CREATION 
>   sheets/plugins/solver/CMakeLists.txt ad813e6 
>   sheets/plugins/solver/Solver.h 21ee006 
>   sheets/plugins/solver/Solver.cpp c87639e 
>   sheets/plugins/solver/sheetssolver.desktop PRE-CREATION 
>   sheets/sheets_viewplugin.desktop PRE-CREATION 
>   sheets/ui/MapViewModel.h ef3cf55 
>   sheets/ui/MapViewModel.cpp 070ee29 
>   stage/part/KPrPart.cpp b79da91 
>   words/part/CMakeLists.txt 4e2650c 
>   words/part/KWPart.cpp 9e3a818 
>   words/part/author/CAuPart.cpp 0863305 
> 
> Diff: http://git.reviewboard.kde.org/r/111545/diff/
> 
> 
> Testing
> -------
> 
> Ran all apps, loaded and saved documents and ran unittests.
> 
> 
> Thanks,
> 
> Boudewijn Rempt
> 
>

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

More information about the calligra-devel mailing list