<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="http://git.reviewboard.kde.org/r/113798/">http://git.reviewboard.kde.org/r/113798/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 11th, 2013, 5:45 p.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;">Thanks for implementing that. Indeed the TODO didn't mean adding a method to QApplication, but it was about the member that was there in qt4, and that is now in QXcbConnection.
The optional dependency on QtGui breaks the rules for tier1 addons, though. Maybe we need to move KDBusService up to... the KService framework maybe?
(Note that there are more todos about startupId stuff, in KDBusService::Activate etc. This would probably be easier to implement in a higher framework too.</pre>
</blockquote>
<p>On November 11th, 2013, 6:15 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;">Oh? I can't find a rule on the wiki that precludes it.
Regardless, the other TODOs would be a lot easier from tier2 or above, where we could use KStartupInfo from KWindowSystem. KService probably does make the most sense out of what we have, although it's still a bit mismatched (I wouldn't expect to need KService if my application didn't use plugins). Is there maybe a use for a framework that provides things to help make apps behave well in free desktop environments?</pre>
</blockquote>
<p>On November 13th, 2013, 8:38 p.m. UTC, <b>Kevin Ottens</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 still prefer having KDBusService stay there... What about doing the xcb connection dance in our platform theme plugin instead? And then attaching a dynamic property on the qapp instance?
This way we can channel the startup id, and from KDBusService POV it only needs to know about QCoreApplication.</pre>
</blockquote>
<p>On November 14th, 2013, 12:30 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;">How about this for an idea: platform_data is an inherently platformy thing (as its name suggests). Perhaps KDBusService could have a (global-static-ish) way of registering a platform interface that contained virtual methods
QVariantMap platformData() const;
void activated(QVariantMap platform_data);
(as a minimum; more fine-grained hooks could also be provided) that the platform plugin could implement and register. The constructor would use the first method to get the necessary data (such as sn id) at one end, and the second to use it in the main instance (and when called via D-Bus).</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;">Doesn't make a big difference with what I'm proposing at the end of the day... I'd say I still prefer the dynamic property, less coupling and less symbols exported (even though it's more fragile or error prone, but it won't be touched often).</pre>
<br />
<p>- Kevin</p>
<br />
<p>On November 11th, 2013, 4:37 p.m. UTC, Alex Merry wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://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, David Faure and Kevin Ottens.</div>
<div>By Alex Merry.</div>
<p style="color: grey;"><i>Updated Nov. 11, 2013, 4:37 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdelibs
</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;">KDBusService: pass the desktop startup ID when calling Activate
We use a bit of private API to ask the xcb platform plugin for it
directly.
The TODO in the original code suggested getting a method in QApplication (or one of its ancestors) to get the startup id, but I think that's very unlikely to be accepted.
There's still the hooks to make use of the value on the other side to sort out, though.</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;">Builds, tests pass. A quick-hack modification of the autotest, along with some hacked-in debug statements, show the value is getting passed properly.</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>tier1/kdbusaddons/CMakeLists.txt <span style="color: grey">(78cc44333574355ff8504481fcb9c88cfc90daf5)</span></li>
<li>tier1/kdbusaddons/src/CMakeLists.txt <span style="color: grey">(0509015afd2d24d34f85a7d6fd786092820814bf)</span></li>
<li>tier1/kdbusaddons/src/config-kdbusaddons.h.cmake <span style="color: grey">(PRE-CREATION)</span></li>
<li>tier1/kdbusaddons/src/kdbusservice.cpp <span style="color: grey">(b773c80b30c6ee39d6d8b4d8c962b83dbd87f7d4)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/113798/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>