<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/115653/">https://git.reviewboard.kde.org/r/115653/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 11th, 2014, 10:53 a.m. UTC, <b>Alex Merry</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;">Two (related) things concern me about this:
You're putting stuff that references the particular Qt platform plugin name into desktop files, and you're doing it with something not prefixed with X-KDE-
I'm not even sure "platform" is really the right term, since you are talking about the window system.
I would suggest, at a minimum, translating between "xcb" (the Qt platform name that refers to the libraries used to implement it) and "X11" (the actual window system).</pre>
</blockquote>
<p>On February 11th, 2014, 11:20 a.m. UTC, <b>Dominik Haumann</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;">I'd also be interested as to why we need to extend KService and add a getter for this. Can't we achieve the same by adding a X-KDE-... flag that is read by the host application? For instance, in KF5, Kate and KDevelop may share plugins, therefore, we introduced a X-KTextEditor-Load-Default={kate, kdevelop, ...}, which looks like a similar use case to me.</pre>
</blockquote>
<p>On February 11th, 2014, 12:26 p.m. UTC, <b>Martin Gräßlin</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;">> I would suggest, at a minimum, translating between "xcb" (the Qt platform name that refers to the libraries used to implement it) and "X11" (the actual window system).
no, I really want it to be the platform name as it's specified by QGuiPlatform::platormName
> I'd also be interested as to why we need to extend KService and add a getter for this.
I had talked to Ben (systemsettings maintainer) how to do it and he pointed me to KService::noDisplay() :-) Let's say I consider this as an RFC, that's why I haven't extended KServiceTypeTrader like it's done for showInKDE.
I'm not sure whether just do it in the host application will be enough or will lead to lots of code duplication. We at least need this in:
* Systemsettings (hide modules which are X11 specific, add modules which are Wayland specific)
* KInfoCenter (same thing)
* kcmshell (would be nice not to crash)
* KWin (awesome solution to get rid of screenshot effect on Wayland)
And probably more - Plasma is a clear candidate where we might also want to make some plasmoids not available on one or the other platform (e.g. Pager looks pretty broken on Wayland at the moment).</pre>
</blockquote>
<p>On February 11th, 2014, 12:38 p.m. UTC, <b>Alex Merry</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;">If you want it to be the Qt platform name, I *strongly* encourage the use of an X-KDE- prefix.</pre>
</blockquote>
<p>On February 11th, 2014, 12:44 p.m. UTC, <b>Martin Gräßlin</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;">ok :-) What would you say to X-KDE-OnlyShowOnQtPlatforms and X-KDE-NotShowOnQtPlatforms
or maybe even QtGui instead of Qt?</pre>
</blockquote>
<p>On February 11th, 2014, 2:56 p.m. UTC, <b>Alex Merry</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;">I'd be fine with that, as it does exactly what it says on the tin.</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;">Martin: what do you mean by extending KServiceTypeTrader? showInKDE seems to just use the OnlyShowIn key in kservice.cpp, I see nothing specific to that in the trader code.
Anyhow, the X-KDE-OnlyShowOnQtPlatforms idea sounds fine to me.
</pre>
<br />
<p>- David</p>
<br />
<p>On February 11th, 2014, 10:40 a.m. UTC, Martin Gräßlin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Frameworks.</div>
<div>By Martin Gräßlin.</div>
<p style="color: grey;"><i>Updated Feb. 11, 2014, 10:40 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kservice
</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;">Introduce a showOnCurrentPlatform method to KService
This is inspired by showInKDE to easily exclude modules which doesn't
make any sense on the current platform. E.g. in systemsettings we
do not want services which are X11 specific if systemsettings is
executed on Wayland.</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;">Added "OnlyShowOnPlatforms=xcb;" to kcmbell's desktop file. Shown in systemsettings on X, hidden on Wayland</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/services/kservice.h <span style="color: grey">(6bc1bb988b273c9b2e6593f5f517535701b3854d)</span></li>
<li>src/services/kservice.cpp <span style="color: grey">(1da29e2629c09a150acee977237a25924056e3bc)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/115653/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>