Review Request 120890: Supporting changes for Calligra Gemini

Dan Leinir Turthra Jensen admin at leinir.dk
Mon Nov 3 10:26:10 GMT 2014 


> On Nov. 1, 2014, 8:28 p.m., Friedrich W. H. Kossebau wrote:
> > And another round of mostly code-style nitpicks.
> > 
> > From my little testing of the branch, there were not visible regressions, and Calligra Gemini at least for the Words part seemed usable. For the 2.9 release perhaps the Stage part should be disabled I guess, given that the Touch mode is still only in demo state. Though ideally someone (won't be me sadly) should get excited about that and perhaps also just make the still-only-demo-mockup things disabled for the release, so StageTouch could at least be used for giving presentations (I assume that with a second screen attached the second screen is hardcoded for the actual slide output?).
> > 
> > So my vote as non-maintainer of Stage&Words (thus no Ship it) is that this works good enough to live already in master and gets further polishing and improvements there.

The presenter DJ thing does lack support for actually doing presentations, but we could easily swap the button's action for the actual presentation launcher for a release with 2.9.


> On Nov. 1, 2014, 8:28 p.m., Friedrich W. H. Kossebau wrote:
> > active/src/MainWindow.cpp, line 68
> > <https://git.reviewboard.kde.org/r/120890/diff/4/?file=324293#file324293line68>
> >
> >     Why this addition? From what I can see Active does not yet use the QtQuick plugins, so this is not (yet) needed.

This is the bit which in a previous RR had hard coded homedir paths ;) It will also (if Calligra Active gets installed) contain a CalligraActive subdir, but this search will still find them, as they're installed in the same base location as the Calligra QtQuick components :)


> On Nov. 1, 2014, 8:28 p.m., Friedrich W. H. Kossebau wrote:
> > libs/kopageapp/KoPADocumentModel.cpp, line 67
> > <https://git.reviewboard.kde.org/r/120890/diff/4/?file=324304#file324304line67>
> >
> >     stamp("please add brackets")

also fixed a load of other similar issues throughout the file, because why not


> On Nov. 1, 2014, 8:28 p.m., Friedrich W. H. Kossebau wrote:
> > libs/main/KoView.cpp, line 368
> > <https://git.reviewboard.kde.org/r/120890/diff/4/?file=324316#file324316line368>
> >
> >     why the "this->"? left-over when `parent` was named `parentWidget`?

it was actually deliberate, because it was then explicitly the same as what's inside the loop... but no, you're right, that only makes people go "huh?", so just parentWidget() now :)


> On Nov. 1, 2014, 8:28 p.m., Friedrich W. H. Kossebau wrote:
> > qtquick/CMakeLists.txt, line 47
> > <https://git.reviewboard.kde.org/r/120890/diff/4/?file=324322#file324322line47>
> >
> >     Why is CMAKE_INSTALL_LIBDIR used here? (And where is it actually from? Could not find a doc about it yet). See also https://git.reviewboard.kde.org/r/120933/ , which proposes to use LIB_INSTALL_DIR instead for the Krita counter parts.

damn... ok, so the problem here is windows again... CMAKE_INSTALL_LIBDIR is defined by GNUInstallDirs (which we removed earlier, which we should not have), because LIB_INSTALL_DIR is broken on Windows... So, going to have to put that include back again, because without it things break on Windows (it kept working for me because i still had my cmakecache)


> On Nov. 1, 2014, 8:28 p.m., Friedrich W. H. Kossebau wrote:
> > qtquick/CQTextDocumentNotesModel.h, line 54
> > <https://git.reviewboard.kde.org/r/120890/diff/4/?file=324349#file324349line54>
> >
> >     Think the usual
> >     ```
> >     Q_SIGNALS:
> >         void countChanged();
> >     ```
> >     makes this signal easier to see. Not only for consistency.

Agreed - convenient for quick test type things, but kind of dangerous for readability :)


> On Nov. 1, 2014, 8:28 p.m., Friedrich W. H. Kossebau wrote:
> > words/part/dockers/KWStatisticsWidget.cpp, line 294
> > <https://git.reviewboard.kde.org/r/120890/diff/4/?file=324420#file324420line294>
> >
> >     Don't be so generous with trailing whitespaces ;)

i see wat u did thar ;)


> On Nov. 1, 2014, 8:28 p.m., Friedrich W. H. Kossebau wrote:
> > libs/main/gemini/ViewModeSwitchEvent.h, line 61
> > <https://git.reviewboard.kde.org/r/120890/diff/4/?file=324317#file324317line61>
> >
> >     Please add an explanation why the magic "+10" is used, not obvious to me.

Hmm... This was actually due to the Kis tablet events, but given they don't exist outside Krita, and Krita Gemini doesn't use this event anyway, it's superfluous and we can un-weird it. If we manage to merge the two later on, we'll need to make the Kis tablet events the ones which work at a higher event number, but we can get to that then. This is now ;)


- Dan Leinir Turthra


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120890/#review69655
-----------------------------------------------------------


On Nov. 1, 2014, 5:48 p.m., Dan Leinir Turthra Jensen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120890/
> -----------------------------------------------------------
> 
> (Updated Nov. 1, 2014, 5:48 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> The many changes in this patch are all the ones found in the Calligra Gemini branch. It includes the addition of the Qt Quick compinents created a while ago by Arjen and Shantanu, as well as a number of changes which improves the writing performance and experience in Words, especially when dealing with very large documents. It further extends the template system with support for variations.
> 
> It does not include the gemini subdirectory itself (and as such to compile this you would need to check out the branch itself, rather than apply this patch), the reason being the large amount of data found in this subdirectory took the patch from the current 233KiB to a nice, solid 55MiB. So, i thought this would be more useful.
> 
> 
> Diffs
> -----
> 
>   words/part/dockers/KWStatisticsWidget.cpp 63d0384 
>   words/part/frames/KWCopyShape.cpp 744f292 
>   words/part/pagetool/KWPageTool.cpp 7935390 
>   words/templates/Wordprocessing/A4.desktop 8a35eaf 
>   words/templates/Wordprocessing/CMakeLists.txt 88b73fa 
>   words/templates/Wordprocessing/ColorfulA4.desktop 04caf8e 
>   words/templates/Wordprocessing/ColorfulLetter.desktop ce38ab5 
>   words/templates/Wordprocessing/FaxA4.desktop 2b43998 
>   words/templates/Wordprocessing/FaxLetter.desktop fc1c161 
>   words/templates/Wordprocessing/ProfessionalA4.desktop 7c84f34 
>   words/templates/Wordprocessing/ProfessionalLetter.desktop 5f8b94f 
>   words/templates/Wordprocessing/a4.png PRE-CREATION 
>   words/templates/Wordprocessing/colorful.png PRE-CREATION 
>   words/templates/Wordprocessing/fax.png PRE-CREATION 
>   words/templates/Wordprocessing/professional.png PRE-CREATION 
>   stage/templates/odf/strange_far_hills.png PRE-CREATION 
>   words/part/KWCanvasBase.cpp 0d2d28f 
>   words/part/KWCanvasItem.cpp cfdae98 
>   words/part/KWGui.cpp 65aedf7 
>   words/part/KWPageStyle.h a3bf58b 
>   words/part/KWView.h f21f8e3 
>   words/part/KWView.cpp 1285d21 
>   words/part/commands/KWShapeCreateCommand.h eb268f8 
>   words/part/dockers/KWNavigationWidget.cpp 1bb54e0 
>   words/part/dockers/KWStatisticsWidget.h eac4d44 
>   stage/templates/odf/skyline_morning.desktop ec4140d 
>   stage/templates/odf/skyline_morning.png PRE-CREATION 
>   stage/templates/odf/skyline_morning_wide.desktop d3337ae 
>   stage/templates/odf/skyline_morning_wide.png PRE-CREATION 
>   stage/templates/odf/skyline_night.desktop 28e3a18 
>   stage/templates/odf/skyline_night.png PRE-CREATION 
>   stage/templates/odf/skyline_night_wide.desktop e188171 
>   stage/templates/odf/skyline_night_wide.png PRE-CREATION 
>   stage/templates/odf/strange_far_hills.desktop 1a17807 
>   stage/templates/odf/hisc-action-cohere-magenta.svgz PRE-CREATION 
>   stage/templates/odf/hisc-action-cohere-red.svgz PRE-CREATION 
>   stage/templates/odf/kde_events.desktop afa4534 
>   stage/templates/odf/kde_events.png PRE-CREATION 
>   stage/templates/odf/rounded_square.desktop 85015a2 
>   stage/templates/odf/rounded_square.png PRE-CREATION 
>   stage/templates/odf/simple_waves.desktop dd2bd96 
>   stage/templates/odf/simple_waves.png PRE-CREATION 
>   stage/templates/odf/skyline_monotone.desktop 0c9dd68 
>   stage/templates/odf/skyline_monotone.png PRE-CREATION 
>   stage/templates/odf/skyline_monotone_wide.desktop 152c719 
>   stage/templates/odf/skyline_monotone_wide.png PRE-CREATION 
>   stage/templates/odf/hisc-action-cohere-gold.svgz PRE-CREATION 
>   stage/templates/odf/flood_light.desktop 23adbb2 
>   stage/templates/odf/flood_light.png PRE-CREATION 
>   stage/templates/odf/hisc-action-cohere-blue.svgz PRE-CREATION 
>   qtquick/CQCanvasController.cpp PRE-CREATION 
>   qtquick/CQCanvasControllerItem.h PRE-CREATION 
>   qtquick/CQCanvasControllerItem.cpp PRE-CREATION 
>   qtquick/CQTextDocumentModel.cpp PRE-CREATION 
>   qtquick/CQTextDocumentNotesModel.h PRE-CREATION 
>   qtquick/CQTextDocumentNotesModel.cpp PRE-CREATION 
>   qtquick/CQTextToCModel.h PRE-CREATION 
>   qtquick/CQTextToCModel.cpp PRE-CREATION 
>   qtquick/CQThumbnailItem.h PRE-CREATION 
>   qtquick/CQThumbnailItem.cpp PRE-CREATION 
>   qtquick/CalligraQtQuickComponentsPlugin.h PRE-CREATION 
>   qtquick/CalligraQtQuickComponentsPlugin.cpp PRE-CREATION 
>   qtquick/qmldir PRE-CREATION 
>   stage/part/KPrView.h b4bab24 
>   stage/part/KPrView.cpp 1edd8e8 
>   stage/part/animations/KPrAnimTransitionFilter.cpp 3347fe1 
>   stage/part/animations/KPrAnimateColor.cpp 59a5e0e 
>   stage/part/animations/KPrAnimateTransform.cpp 914047e 
>   stage/templates/common_desktop/emptyLandscapeWide.desktop c5f8f8a 
>   stage/templates/odf/CMakeLists.txt f7db7d0 
>   stage/templates/odf/blue_orange_vector.desktop 31dae09 
>   stage/templates/odf/blue_orange_vector.png PRE-CREATION 
>   stage/templates/odf/burning_desire.desktop ac79022 
>   stage/templates/odf/burning_desire.png PRE-CREATION 
>   stage/templates/odf/business.desktop d552db3 
>   stage/templates/odf/business.png PRE-CREATION 
>   stage/templates/odf/cohere-blue.desktop PRE-CREATION 
>   stage/templates/odf/cohere-blue.otp PRE-CREATION 
>   stage/templates/odf/cohere-blue.png PRE-CREATION 
>   stage/templates/odf/cohere-gold.desktop PRE-CREATION 
>   stage/templates/odf/cohere-gold.otp PRE-CREATION 
>   stage/templates/odf/cohere-gold.png PRE-CREATION 
>   stage/templates/odf/cohere-magenta.desktop PRE-CREATION 
>   stage/templates/odf/cohere-magenta.otp PRE-CREATION 
>   stage/templates/odf/cohere-magenta.png PRE-CREATION 
>   stage/templates/odf/cohere-red.desktop PRE-CREATION 
>   stage/templates/odf/cohere-red.otp PRE-CREATION 
>   stage/templates/odf/cohere-red.png PRE-CREATION 
>   stage/templates/odf/curious_penguin.desktop 429e7c9 
>   stage/templates/odf/curious_penguin.png PRE-CREATION 
>   qtquick/CQImageProvider.h PRE-CREATION 
>   qtquick/CQImageProvider.cpp PRE-CREATION 
>   qtquick/CQLinkArea.h PRE-CREATION 
>   qtquick/CQLinkArea.cpp PRE-CREATION 
>   qtquick/CQPluginLoaderProxy.h PRE-CREATION 
>   qtquick/CQPluginLoaderProxy.cpp PRE-CREATION 
>   qtquick/CQPresentationCanvas.h PRE-CREATION 
>   qtquick/CQPresentationCanvas.cpp PRE-CREATION 
>   qtquick/CQPresentationModel.h PRE-CREATION 
>   qtquick/CQPresentationModel.cpp PRE-CREATION 
>   qtquick/CQPresentationView.h PRE-CREATION 
>   qtquick/CQPresentationView.cpp PRE-CREATION 
>   qtquick/CQSpreadsheetCanvas.h PRE-CREATION 
>   qtquick/CQSpreadsheetCanvas.cpp PRE-CREATION 
>   qtquick/CQSpreadsheetListModel.h PRE-CREATION 
>   qtquick/CQSpreadsheetListModel.cpp PRE-CREATION 
>   qtquick/CQTextDocumentCanvas.h PRE-CREATION 
>   qtquick/CQTextDocumentCanvas.cpp PRE-CREATION 
>   qtquick/CQTextDocumentModel.h PRE-CREATION 
>   CMakeLists.txt fa00fdd 
>   CalligraProducts.cmake c2ef8ac 
>   active/main.cpp 7e4c888 
>   active/qml/Test.qml PRE-CREATION 
>   active/src/CASpreadsheetHandler.cpp e8cebff 
>   active/src/CATextDocumentModel.cpp 4e72581 
>   active/src/MainWindow.cpp c378881 
>   cmake/modules/FindLibgit2.cmake PRE-CREATION 
>   cmake/modules/FindLibqgit2.cmake PRE-CREATION 
>   filters/words/docx/export/DocxFile.cpp d67e41f 
>   filters/words/msword-odf/wv2/src/utilities.cpp 4be8c38 
>   krita/sketch/KisSketchView.cpp 5cb9172 
>   krita/sketch/SketchDeclarativeView.cpp e352fc9 
>   libs/flake/KoCanvasController.h 804a1e4 
>   libs/flake/KoCanvasController.cpp 5190f62 
>   libs/flake/KoToolManager.h ac17dd0 
>   libs/flake/KoToolManager.cpp 5d4da91 
>   libs/kopageapp/KoPADocumentModel.cpp 22bd75a 
>   libs/kopageapp/tools/backgroundTool/KoPABackgroundToolFactory.cpp f442b84 
>   libs/main/KoApplication.cpp b20c0e9 
>   libs/main/KoDocument.h c3936a9 
>   libs/main/KoDocumentSectionDelegate.cpp 81c53c0 
>   libs/main/KoMainWindow.cpp 8db43f8 
>   libs/main/KoPart.cpp 04b8d25 
>   libs/main/KoTemplate.h f96099c 
>   libs/main/KoTemplate.cpp f1d3bd9 
>   libs/main/KoTemplateCreateDia.cpp 72573cc 
>   libs/main/KoTemplateGroup.cpp ab35dd1 
>   libs/main/KoTemplateTree.cpp 8f864b4 
>   libs/main/KoView.cpp 4770dcc 
>   libs/main/gemini/ViewModeSwitchEvent.h PRE-CREATION 
>   libs/odf/KoEncryptedStore.cpp 53c75b6 
>   libs/widgets/KoZoomAction.h c5f69b0 
>   plugins/defaultTools/defaulttool/SelectionDecorator.h 83e715e 
>   plugins/defaultTools/defaulttool/SelectionDecorator.cpp 3e16cfa 
>   qtquick/CMakeLists.txt PRE-CREATION 
>   qtquick/CQCanvasBase.h PRE-CREATION 
>   qtquick/CQCanvasBase.cpp PRE-CREATION 
>   qtquick/CQCanvasController.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/120890/diff/
> 
> 
> Testing
> -------
> 
> * Compiles and shipped to users on Windows (through the Calligra Gemini packages)
> * Used every day for writing in a production environment (for producing my novel and recent short story)
> 
> 
> Thanks,
> 
> Dan Leinir Turthra Jensen
> 
>

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

More information about the calligra-devel mailing list