<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/120099/">https://git.reviewboard.kde.org/r/120099/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 5th, 2014, 5:58 p.m. UTC, <b>Christoph Feck</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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;">Sorry for chiming in late, just noticed the commit.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Any reason why the resource is checked first, not last?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If the programmer decides to use the resource, the "locateAll" effectively turns into a "locateNone". Not really friendly to administrators.</p></pre>
</blockquote>
<p>On October 5th, 2014, 6:10 p.m. UTC, <b>Christoph Cullmann</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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;">Actually that is a big benefit in my eyes, if you ship software with compiled in resources, you don't want people mingling around with them, IMHO.
E.g. in ktexteditor I could use this and be sure that no actions or stuff is missing only because some "fixed" ui file got placed in the right location.</p></pre>
</blockquote>
<p>On October 5th, 2014, 6:42 p.m. UTC, <b>Albert Astals Cid</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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;">I agree with Christoph Feck, we should not disable such an important feature as locateAll, if you don't want locateAll don't use it, but don't break it with strange rules as "LocateAll won't do locate all if your xmlgui is in your .rc file instead of the filesystem".</p></pre>
</blockquote>
<p>On October 5th, 2014, 7:25 p.m. UTC, <b>Christoph Cullmann</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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;">? I don't get that point, I don't break "locateAll", the patch just changes the default lookup of setXMLFile, if the application/library author decides to put the ui file in a resource.</p></pre>
</blockquote>
<p>On October 6th, 2014, 5:30 p.m. UTC, <b>Albert Astals Cid</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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;">It does break setXMLFile behaviour, the code has a locateAll but now we're checking for the rc file before, so setXMLFile behaves differently on depending how you install/package your application. That's unexpected and i would say undesired, and even if it is desired, it is undocumented.</p></pre>
</blockquote>
<p>On October 6th, 2014, 5:31 p.m. UTC, <b>Albert Astals Cid</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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;">And it also probably breaks all kind of editing of toolbar by the user.</p></pre>
</blockquote>
</blockquote>
<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;">That is true, you would need to call setLocalXMLFile, in addition, thats true :(
I agree that this needs to be either fixed or documented, still I don't see the problem with the fact that the application/library author can enforce that the resource file is used.
But as two people think it is an issue, I should fix it the way you say.
Should I reverse the direction of the lookup then, to have it behind the current "kxmlgui5/" but in front of the deprecated locations that warn?</p></pre>
<br />
<p>- Christoph</p>
<br />
<p>On October 5th, 2014, 5:32 p.m. UTC, Christoph Cullmann 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 KDE Frameworks and David Faure.</div>
<div>By Christoph Cullmann.</div>
<p style="color: grey;"><i>Updated Oct. 5, 2014, 5:32 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kxmlgui
</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;">Instead of installing them in the kxmlgui5 share prefix, install them in a kxmlgui5 prefix in resources.
That will allow to test stuff without installing it first, as the resources will be found anyway.
Still the "I can edit it and have a copy in my writable datadir" should work.
Question is: is it possible with one "kxmlgui" prefix for the resource or is it better to swap that around like "componentname/kxmlgui5/.."?</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;">Compiles & if I package ui file in kate into resource, works.</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>src/kxmlguiclient.cpp <span style="color: grey">(e8170ad)</span></li>
<li>src/kxmlguifactory.cpp <span style="color: grey">(c4ad97b)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/120099/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>