<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/126618/">https://git.reviewboard.kde.org/r/126618/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 3rd, 2016, 3:22 p.m. UTC, <b>Alex Richardson</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;">According to the documentation AppLocalDataLocation is the following: "C:/Users/<USER>/AppData/Local/<APPNAME>", "C:/ProgramData/<APPNAME>", "<APPDIR>", "<APPDIR>/data"</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In which directory are the desktop files? Unfortunately I can't check as my Windows machine broke a while back and I haven't compiled KF5 on Windows since.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Patch looks good to me otherwise as it will still check the same directories change behaviour on Linux. Only minor issue is that the error message is a little bit confusing now.</p></pre>
</blockquote>
<p>On January 3rd, 2016, 3:30 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;">hi, same problem occurs on mac, too, i think a better fallback would be something install prefix relative, that would work on win + mac.</p></pre>
</blockquote>
<p>On January 3rd, 2016, 4:18 p.m. UTC, <b>Alex Richardson</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;">Would checking <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">${KDE_INSTALL_FULL_KSERVICETYPES5DIR} + path</code> first and then fall back to GenericDataLocation work on Windows and Mac? Or do we still need the AppDataLocation for runtime detection of the paths?</p></pre>
</blockquote>
<p>On January 3rd, 2016, 5:30 p.m. UTC, <b>Ralf Habacker</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;">From https://build.opensuse.org/build/home:rhabacker:branches:windows:mingw:win32:KF516/openSUSE_13.2/x86_64/mingw32-umbrello5/_log
...
Installing: /home/abuild/rpmbuild/BUILDROOT/mingw32-umbrello5-2.18.99.6f6891a-2.25.x86_64/usr/i686-w64-mingw32/sys-root/mingw/share/applications/org.kde.umbrello.desktop</p></pre>
</blockquote>
<p>On January 3rd, 2016, 6:11 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;">KDE_INSTALL_FULL_KSERVICETYPES5DIR should be good enough to have something working to be able to build KF5 on win/mac without patching. If the desktoptojson program should work even after packed into some installer/application bundle I guess we would need the appdata fallback, too. The question is: is that a use case needed. For me it is enough to be able to have it working in a devel setup.</p></pre>
</blockquote>
<p>On January 3rd, 2016, 6:48 p.m. UTC, <b>Alex Richardson</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;">This code can also be used at runtime through <a href="http://api.kde.org/frameworks-api/frameworks5-apidocs/kcoreaddons/html/classKPluginMetaData.html#ac6e87c110b4743ce1b16044c649838ba" style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">KPluginMetaData::fromDesktopFile()</a> although any users should probably be using JSON files directly.</p></pre>
</blockquote>
<p>On January 3rd, 2016, 7:59 p.m. UTC, <b>Kåre Särs</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;">The kpart.desktop file that KTextEditor was looking for is in <installdir>/share/kservicetypes5/.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I added AppLocalData to get the <APPDI> path which is normally "<installdir>/bin/" for KDE aplications. I could also live with KDE_INSTALL_FULL_KSERVICETYPES5DIR, but generally I would try to avoid absolute paths hardcoded into binaries...</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'll update the error printout.</p></pre>
</blockquote>
<p>On January 4th, 2016, 11:53 a.m. UTC, <b>Sebastian Kügler</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;">Plasma packages use KPluginMetaData::fromDesktopFile(), this cannot be changed easily, as almost all packages around (our own, and third party) are using .desktop files. We're slowly transitioning, but it will take time. Removing that would mean a lot of our stuff would break, and even more 3rd party plasmoids, kwin scripts, etc..</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;">We won't remove what works atm and I guess it will not matter that much if plasmoids/kwin stuff works "better" on win/mac or like they do atm.</p></pre>
<br />
<p>- Christoph</p>
<br />
<p>On January 3rd, 2016, 1:22 p.m. UTC, Kåre Särs 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.</div>
<div>By Kåre Särs.</div>
<p style="color: grey;"><i>Updated Jan. 3, 2016, 1:22 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kcoreaddons
</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;">Without this patch kcoreaddons_desktop_to_json() will not find the destop file.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">On windows GenericDataLocation returns "C:/Users/<USER>/AppData/Local" or "C:/ProgramData". That is not a path that contains any destip files ;)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This patch adds AppLocalDataLocation to the seach if the previous search does not return a match. </p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Another option would be to hardcode the absolute path to all places that uses kcoreaddons_desktop_to_json(), but that does not feel too nice...</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What other options do we have?</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;">KTextEditor compiles again on windows</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/lib/plugin/desktopfileparser.cpp <span style="color: grey">(1122af8)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/126618/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>