<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/123440/">https://git.reviewboard.kde.org/r/123440/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Calligra and Thorsten Zachmann.</div>
<div>By Friedrich W. H. Kossebau.</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
calligra
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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 ) );</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The resource type "styles" is used nowhere else.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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 ;)</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The default markers are still loaded in all apps, tested with Karbon, Words & Stage.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>libs/flake/KoMarkerCollection.cpp <span style="color: grey">(06c8f98)</span></li>
<li>libs/widgets/KoGlobal.cpp <span style="color: grey">(80c3b2e)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/123440/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>