<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/123376/">https://git.reviewboard.kde.org/r/123376/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 1st, 2015, 7:51 a.m. UTC, <b>Thomas Lübking</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;">Two things.
a) "1" is a magic number (stupid xcb, should be xcppb ;-) - please add a comment that this picks the 1st level shift
b) the overall code looks a bit inefficient. No idea how smart xcb is under the hood, but
1. we fetch the keysym via xcb_key_press_lookup_keysym
2. we check the mod state to be numlock
3. we fetch the keysym again (xcb_key_press_lookup_keysym just wraps xcb_key_symbols_get_keysym on ev->detail)
4. we map keysymX to keysymQt
5. we check the (mapped to Qt) modifier for the SHIFT flag
6. this patch fetches and maps keysymX once more
.... do you want to do a general cleanup with this or another patch?</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;">Hi, sorry for the later reply. I've been really busy later.
I can do a code cleanup (will have some free time 2 weeks from now). What do you want me to do.</pre>
<br />
<p>- Yuxuan</p>
<br />
<p>On April 25th, 2015, 7:54 p.m. UTC, Yuxuan Shui 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, Martin Gräßlin and Thomas Lübking.</div>
<div>By Yuxuan Shui.</div>
<p style="color: grey;"><i>Updated April 25, 2015, 7:54 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=341959">341959</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kglobalaccel
</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;">For details, see discussion in corresponding bug report.</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/runtime/kglobalaccel_x11.cpp <span style="color: grey">(abee5bc)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/123376/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>