<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/124066/">https://git.reviewboard.kde.org/r/124066/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 17th, 2015, 7:24 a.m. UTC, <b>David Faure</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 really have the overview anymore, but the kdelibs4 solution was fully extensible, a servicetype desktop file could define new keys and their type. It looks like desktopfileparser.cpp doesn't have the same flexibility, if each and every app needs to add their keys to the code :(</p></pre>
</blockquote>
<p>On June 17th, 2015, 3:12 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;">Possibly we should let desktopfileparser read a servicetype file? Was also suggested here: https://git.reviewboard.kde.org/r/121672/</p></pre>
</blockquote>
<p>On June 17th, 2015, 3:51 p.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;">So it has to look in standardpaths and somewhere in the buildpath for a service file, then use that to generate the json file? I guess the servicefile could be fairly simple, as it just has to note unknown key/type combinations. The KDE4-style service files do have that information, and as long as we don't have to parse that at runtime, we should be fine. On the other hand, we'll get build-time dependencies on these servicetype files, as it's now not good enough anymore to install them at some point, the resulting json information is going to differ then.</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;">Observation: I'm looking into parsing the proper servicetype, and I think it could be quite straightforward, if we're OK with going over all servicetypes installed at. Would incur some cost, but it would mean that apps can -- yet again -- add their own non-bool, non-int and non-string keys.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">But ... as to this patch, I'd still like to see it go in because the formfactor key makes sense for pretty much all services, plugins and application, so the usecase is not simply per app, but we want to limit the plugins and apps per formfactor (this would also allow us starting with a different commandline option on a different formfactor (by providing more than one desktop file), and it's IMO the most flexible solution we can provide). It also means, that it should probably be elevated to premier API, so a new getter for KPluginMetaData makes sense. I'll add that and will update this patch.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">At the same time, I'll work on parsing the servicetypes from desktoptojson, that should work just fine, as long as servicetypes are available at compile time (would not cause regressions anyway, since we don't support it right now, so would be a mere addition).</p></pre>
<br />
<p>- Sebastian</p>
<br />
<p>On June 11th, 2015, 4:16 a.m. UTC, Sebastian Kügler 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, Alex Richardson and David Faure.</div>
<div>By Sebastian Kügler.</div>
<p style="color: grey;"><i>Updated June 11, 2015, 4:16 a.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;">This patch adds X-KDE-FormFactor to the keys recognized as stringlists.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">We would like to see this to go in to allow us to filter plugins (KCMs for example) by form factor, so we only display UI plugins that are suitable for a given target device.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The idea is that plugins indicate which form factor (for example media center, tablet, desktop, etc...) they're suitable for, and the "host application" filters based on these.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If this approach is deemed valid, I'd be happy to add convenience API to KPluginMetaData, i.e. QStringList KPluginMetaData::formFactor(). This patch would be the minimal implementation we'd need.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The naming of the key is of course open to better suggestions.</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;">added autotest, also implemented using this in the Plasma Active settings app as proof-of-concept, works like a charm.</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>autotests/data/fakeplugin.desktop <span style="color: grey">(95152f6)</span></li>
<li>autotests/kpluginmetadatatest.cpp <span style="color: grey">(231ac36)</span></li>
<li>src/lib/plugin/desktopfileparser.cpp <span style="color: grey">(b19da6b)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/124066/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>