<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/112443/">http://git.reviewboard.kde.org/r/112443/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 23rd, 2013, 12:07 p.m. CEST, <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;">Tested the patch in my tree, works for caps lock too.
Now it highlights a dependency problem... We don't want a dependency on QX11Extras from KGuiAddons. So maybe we should move KModifierKeyInfo to your proposed KX11Extras?</pre>
</blockquote>
<p>On September 23rd, 2013, 12:59 p.m. CEST, <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;">Then I would suggest to move this class to KWindowSystem for the moment. For the module KX11Extras I still need some code changes first (for some reason netwm is using KWindowSystem - it's wrong IMHO and needs fixing)</pre>
</blockquote>
<p>On September 23rd, 2013, 1:22 p.m. CEST, <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;">Well, if you remember it was my first choice to put KModifierKeyInfo in KWindowSystem, but you pushed back. :-)
I'm ok if you move it there then, looking forward to the patches. ;-)</pre>
</blockquote>
<p>On September 29th, 2013, 6:37 p.m. CEST, <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;">You know, if the dependency on QX11Extras is only for QX11Info::display(), that's really easy to write "by hand" in kmodifierkeyinfo_x11.cpp
(see the 5 lines in QX11Info::display).
Although I suspect after porting to XCB it would rather be about QX11Info::connection(), same thing though (accessible via QPlatformNativeInterface).
QX11Info mostly exists for Qt4 source compat and for convenience, it's not mandatory to go through it.
I'm not convinced about this going to KWindowSystem.
An application developer who would want to display a "Caps Lock" indicator in the statusbar of his wordprocessor (say kile, which does exactly this) would first look at Qt, then notice you can't query it for the status of CapsLock, and would then look at kguiaddons and find it. But why would they look into KWindowSystem for this? This has nothing to do with window management. It's a key handling feature, complementing what's in QtGui.
I certainly expect it to be implemented on Windows and/or Mac at some point in the future, it definitely makes sense there too (unlike, say, NETWM).</pre>
</blockquote>
<p>On September 30th, 2013, 9:26 a.m. CEST, <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, I can see how to get away from QX11Extras, but Kevin also complained about XLib and XCB. And I do not see how I can interact with X11 without using either of those two. So either I'm allowed to use X-specific code in this module or the code cannot go into this module.</pre>
</blockquote>
<p>On October 9th, 2013, 6:49 p.m. CEST, <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;">XCB is less of an issue if that's an optional dependency.</pre>
</blockquote>
<p>On October 21st, 2013, 1:18 p.m. CEST, <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;">Dropped some of the issues to reflect my previous comment.</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;">I just tried to get rid of the QX11Extras dependency. But I'm failing trying to access the QPA. I'm getting a compile error that qplatformnativeinterface.h cannot be found. Any ideas?</pre>
<br />
<p>- Martin</p>
<br />
<p>On September 4th, 2013, 8:37 a.m. CEST, Martin Gräßlin 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.</div>
<div>By Martin Gräßlin.</div>
<p style="color: grey;"><i>Updated Sept. 4, 2013, 8:37 a.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;">Ported to native event filter and to xcb-xkb by reimplementing the events. Most parts are kept on xlib though as we don't have xkb.h to get proper defines.</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;">used kmodifierkeyinfotest application. Would appreciate if someone else could run it as I don't have a caps lock.</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/kguiaddons/CMakeLists.txt <span style="color: grey">(3124c4d)</span></li>
<li>tier1/kguiaddons/src/lib/CMakeLists.txt <span style="color: grey">(dc6aafa)</span></li>
<li>tier1/kguiaddons/src/lib/util/kmodifierkeyinfoprovider_dummy.cpp <span style="color: grey">(7913d29)</span></li>
<li>tier1/kguiaddons/src/lib/util/kmodifierkeyinfoprovider_p.h <span style="color: grey">(ee8e82e)</span></li>
<li>tier1/kguiaddons/src/lib/util/kmodifierkeyinfoprovider_x11.cpp <span style="color: grey">(2f28d41)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/112443/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>