Review Request 123440: Load defaults markers.xml directly via "data" resource dirs, not "styles"

[Thorsten Zachmann]

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

Ship it!


Looks good.

- Thorsten Zachmann


On April 20, 2015, 10:11 p.m., Friedrich W. H. Kossebau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123440/
> -----------------------------------------------------------
> 
> (Updated April 20, 2015, 10:11 p.m.)
> 
> 
> Review request for Calligra and Thorsten Zachmann.
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> Currently for marker styles (i.e. line ends, e.g. arrows) KoMarkerCollection loads a file "markers.xml". It finds a file with the name by the resource type "styles". Calligra itself installs only one such file, i.e. the library of KoMarkerCollection itself does that. The path where that one file is installed is registered for resource type "styles" by KoGlobal (while kowidgets actually depends on flake, not the other way round).
>     libs/widgets/KoGlobal.cpp:    KGlobal::dirs()->addResourceType("styles", "data", "calligra/styles/");
>     libs/flake/KoMarkerCollection.cpp:    QString filePath(KStandardDirs::locate("styles", "markers.xml"));
>     libs/flake/styles/CMakeLists.txt: install(FILES markers.xml DESTINATION ${DATA_INSTALL_DIR}/calligra/styles)
> 
> The resource type "styles" is also used by Words, Stage & Flow, registering "words/styles/" etc. They use the type to tell KoOdfLoadingContext where to find the app-specific defaultstyles.xml:
>     words/part/KWFactory.cpp:  s_instance->dirs()->addResourceType("styles", "data", "words/styles/");
>     stage/part/KPrFactory.cpp: s_instance->dirs()->addResourceType("styles", "data", "stage/styles/");
>     flow/part/FlowFactory.cpp: s_instance->dirs()->addResourceType("styles", "data", "flow/styles/");
>     libs/odf/KoOdfLoadingContext.cpp:    QString fileName( KStandardDirs::locate("styles", "defaultstyles.xml", componentData ) );
> 
> The resource type "styles" is used nowhere else.
> 
> Was it ever planned that Words, Stage, Flow (or perhaps another app) could overwrite the basic default markers.xml file, and is that the reason why the shared resource type is used?
> 
> If not, what I assume, then I propose to remove that indirect access, and have KoMarkerCollection search the file directly in calligra/styles, instead of any other paths registered for "styles".
> With the registration of "calligra/styles" no longer needed, removing it should also speed up minimally loading the defaultstyles.xml, as there are two paths less to look inside now.
> And porting to Qt5 QStandardPaths has one issue less in the end ;)
> 
> 
> Diffs
> -----
> 
>   libs/flake/KoMarkerCollection.cpp 06c8f98 
>   libs/widgets/KoGlobal.cpp 80c3b2e 
> 
> Diff: https://git.reviewboard.kde.org/r/123440/diff/
> 
> 
> Testing
> -------
> 
> The default markers are still loaded in all apps, tested with Karbon, Words & Stage.
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

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