[prev in list] [next in list] [prev in thread] [next in thread]
List: calligra-devel
Subject: Re: Review Request 123670: Less complicated look up of template locations, fix referencing templates
From: "Boudewijn Rempt" <boud () valdyas ! org>
Date: 2015-05-13 19:16:19
Message-ID: 20150513191619.8613.70624 () mimi ! kde ! org
[Download RAW message or body]
--===============0135133846551510365==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123670/#review80316
-----------------------------------------------------------
Ship it!
Yes, it makes a lot of sense.
- Boudewijn Rempt
On May 7, 2015, 12:50 a.m., Friedrich W. H. Kossebau wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123670/
> -----------------------------------------------------------
>
> (Updated May 7, 2015, 12:50 a.m.)
>
>
> Review request for Calligra, Camilla Boemann, Inge Wallin, Boudewijn Rempt, and \
> Thorsten Zachmann.
>
> Repository: calligra
>
>
> Description
> -------
>
> Template folders are currently found by a more complicated approach:
>
> * part factory adds (and only to the factory's componentdata) a resource type with \
> the respective resource path, e.g. "words_template" and \
> "words/templates/"
> * part itself sets a `templateType`, which actually is the resource type id for its \
> templates
> * code that wants to lookup matching templates first queries the part for the \
> resource type id, only then queries the standard dirs for the templates (assuming \
> that the maincomponent also is the one which has that resource type set)
> Though two spots in `KoApplication` and `KoPart` (and their `Kis` counterparts) \
> ignore that and calculate themselves the matching templates locations, using the \
> appname. Which will fail at least with Author and Krita Gemini/Sketch.
> Attached patch proposes to change that:
> instead of noting the template locations for a document type to the standarddirs of \
> the componentdata of the respecitve part factory, and setting a look-up id with the \
> created parts, the template locations are set directly in the parts and queryable \
> from them.
> Advantages:
>
> * Less complex and will also be better portable to Qt5/KF5.
> * Also fixes the code broken with appnames not matching the prefix of the template \
> locations.
> Not perfectly happy with this yet, but possibly a step in direction of a better \
> solution.
>
> Diffs
> -----
>
> flow/part/FlowFactory.cpp a3b4b14
> flow/part/FlowPart.cpp 22f67f4
> gemini/TemplatesModel.cpp c53a64d
> karbon/ui/KarbonFactory.cpp be55d4c
> karbon/ui/KarbonPart.cpp 6cfc9e3
> krita/ui/KisApplication.cpp af59d90
> krita/ui/KisOpenPane.h 7725227
> krita/ui/KisOpenPane.cpp bf89caa
> krita/ui/KisPart.h 77ccf5f
> krita/ui/KisPart.cpp 5f7d625
> krita/ui/KisTemplateCreateDia.h f6e2874
> krita/ui/KisTemplateCreateDia.cpp 08fb546
> krita/ui/KisTemplateTree.h 7aa7119
> krita/ui/KisTemplateTree.cpp b658330
> krita/ui/kis_factory2.cc 3a85bcf
> libs/main/KoApplication.cpp 189cbe5
> libs/main/KoOpenPane.h 009b38d
> libs/main/KoOpenPane.cpp 752ea15
> libs/main/KoPart.h 491da0a
> libs/main/KoPart.cpp 59f8140
> libs/main/KoTemplateCreateDia.h d20937f
> libs/main/KoTemplateCreateDia.cpp e65a65b6
> libs/main/KoTemplateTree.h 16f3957
> libs/main/KoTemplateTree.cpp c2b1a4d
> plan/kptfactory.cpp 58138b5
> plan/kptpart.cpp fe285a9
> plan/kptview.cpp ad18ccc
> sheets/part/Factory.cpp dc448ac
> sheets/part/Part.cpp 26dbb5e
> sheets/part/View.cpp bf4b239
> stage/part/KPrFactory.cpp be7b861
> stage/part/KPrPart.cpp f8ae251
> words/part/KWFactory.cpp cb1f131
> words/part/KWPart.cpp 3d3013f
> words/part/KWView.cpp 3628cae
> words/part/author/CAuFactory.cpp 07c482a
> words/part/author/CAuPart.cpp 5bd43fb
>
> Diff: https://git.reviewboard.kde.org/r/123670/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Friedrich W. H. Kossebau
>
>
--===============0135133846551510365==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 7bit
<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/123670/">https://git.reviewboard.kde.org/r/123670/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="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;">Yes, \
it makes a lot of sense.</p></pre> <br />
<p>- Boudewijn Rempt</p>
<br />
<p>On May 7th, 2015, 12:50 a.m. UTC, Friedrich W. H. Kossebau wrote:</p>
<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, Camilla Boemann, Inge Wallin, Boudewijn Rempt, and \
Thorsten Zachmann.</div> <div>By Friedrich W. H. Kossebau.</div>
<p style="color: grey;"><i>Updated May 7, 2015, 12:50 a.m.</i></p>
<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;">Template folders are currently found by a more \
complicated approach:</p> <ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 \
1em;line-height: inherit;white-space: normal;"> <li style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: normal;">part factory adds (and \
only to the factory's componentdata) a resource type with the respective resource \
path, e.g. "words_template" and "words/templates/"</li> <li style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">part \
itself sets a <code style="text-rendering: inherit;color: #4444cc;padding: \
0;white-space: normal;margin: 0;line-height: inherit;">templateType</code>, which \
actually is the resource type id for its templates</li> <li style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">code \
that wants to lookup matching templates first queries the part for the resource type \
id, only then queries the standard dirs for the templates (assuming that the \
maincomponent also is the one which has that resource type set)</li> </ul>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Though two spots in <code style="text-rendering: \
inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: \
inherit;">KoApplication</code> and <code style="text-rendering: inherit;color: \
#4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">KoPart</code> \
(and their <code style="text-rendering: inherit;color: #4444cc;padding: \
0;white-space: normal;margin: 0;line-height: inherit;">Kis</code> counterparts) \
ignore that and calculate themselves the matching templates locations, using the \
appname. Which will fail at least with Author and Krita Gemini/Sketch.</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Attached patch proposes to change that: instead of noting the template \
locations for a document type to the standarddirs of the componentdata of the \
respecitve part factory, and setting a look-up id with the created parts, the \
template locations are set directly in the parts and queryable from them.</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Advantages:</p> <ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 \
1em;line-height: inherit;white-space: normal;"> <li style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: normal;">Less complex and will \
also be better portable to Qt5/KF5.</li> <li style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: normal;">Also fixes the code \
broken with appnames not matching the prefix of the template locations.</li> </ul>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Not perfectly happy with this yet, but possibly a step \
in direction of a better solution.</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>flow/part/FlowFactory.cpp <span style="color: grey">(a3b4b14)</span></li>
<li>flow/part/FlowPart.cpp <span style="color: grey">(22f67f4)</span></li>
<li>gemini/TemplatesModel.cpp <span style="color: grey">(c53a64d)</span></li>
<li>karbon/ui/KarbonFactory.cpp <span style="color: grey">(be55d4c)</span></li>
<li>karbon/ui/KarbonPart.cpp <span style="color: grey">(6cfc9e3)</span></li>
<li>krita/ui/KisApplication.cpp <span style="color: grey">(af59d90)</span></li>
<li>krita/ui/KisOpenPane.h <span style="color: grey">(7725227)</span></li>
<li>krita/ui/KisOpenPane.cpp <span style="color: grey">(bf89caa)</span></li>
<li>krita/ui/KisPart.h <span style="color: grey">(77ccf5f)</span></li>
<li>krita/ui/KisPart.cpp <span style="color: grey">(5f7d625)</span></li>
<li>krita/ui/KisTemplateCreateDia.h <span style="color: grey">(f6e2874)</span></li>
<li>krita/ui/KisTemplateCreateDia.cpp <span style="color: \
grey">(08fb546)</span></li>
<li>krita/ui/KisTemplateTree.h <span style="color: grey">(7aa7119)</span></li>
<li>krita/ui/KisTemplateTree.cpp <span style="color: grey">(b658330)</span></li>
<li>krita/ui/kis_factory2.cc <span style="color: grey">(3a85bcf)</span></li>
<li>libs/main/KoApplication.cpp <span style="color: grey">(189cbe5)</span></li>
<li>libs/main/KoOpenPane.h <span style="color: grey">(009b38d)</span></li>
<li>libs/main/KoOpenPane.cpp <span style="color: grey">(752ea15)</span></li>
<li>libs/main/KoPart.h <span style="color: grey">(491da0a)</span></li>
<li>libs/main/KoPart.cpp <span style="color: grey">(59f8140)</span></li>
<li>libs/main/KoTemplateCreateDia.h <span style="color: grey">(d20937f)</span></li>
<li>libs/main/KoTemplateCreateDia.cpp <span style="color: \
grey">(e65a65b6)</span></li>
<li>libs/main/KoTemplateTree.h <span style="color: grey">(16f3957)</span></li>
<li>libs/main/KoTemplateTree.cpp <span style="color: grey">(c2b1a4d)</span></li>
<li>plan/kptfactory.cpp <span style="color: grey">(58138b5)</span></li>
<li>plan/kptpart.cpp <span style="color: grey">(fe285a9)</span></li>
<li>plan/kptview.cpp <span style="color: grey">(ad18ccc)</span></li>
<li>sheets/part/Factory.cpp <span style="color: grey">(dc448ac)</span></li>
<li>sheets/part/Part.cpp <span style="color: grey">(26dbb5e)</span></li>
<li>sheets/part/View.cpp <span style="color: grey">(bf4b239)</span></li>
<li>stage/part/KPrFactory.cpp <span style="color: grey">(be7b861)</span></li>
<li>stage/part/KPrPart.cpp <span style="color: grey">(f8ae251)</span></li>
<li>words/part/KWFactory.cpp <span style="color: grey">(cb1f131)</span></li>
<li>words/part/KWPart.cpp <span style="color: grey">(3d3013f)</span></li>
<li>words/part/KWView.cpp <span style="color: grey">(3628cae)</span></li>
<li>words/part/author/CAuFactory.cpp <span style="color: grey">(07c482a)</span></li>
<li>words/part/author/CAuPart.cpp <span style="color: grey">(5bd43fb)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/123670/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============0135133846551510365==--
[Attachment #3 (text/plain)]
_______________________________________________
calligra-devel mailing list
calligra-devel@kde.org
https://mail.kde.org/mailman/listinfo/calligra-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic