<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/119053/">https://git.reviewboard.kde.org/r/119053/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Push it, after looking at my small optimization suggestion.</pre>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/119053/diff/1/?file=285795#file285795line205" style="color: black; font-weight: bold; text-decoration: underline;">autotests/kglobalshortcuttest.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void KGlobalShortcutTest::testStealShortcut()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">201</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QList</span><span class="o"><</span><span class="n">QKeySequence</span><span class="o">></span> <span class="n">shortcuts</span> <span class="o">=</span> <span class="n">KGlobalAccel</span><span class="o">::</span><span class="n">self</span><span class="p">()</span><span class="o">-></span><span class="n">shortcut</span><span class="p">(</span><span class="n">m_action<span class="hl">B</span></span><span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">205</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QList</span><span class="o"><</span><span class="n">QKeySequence</span><span class="o">></span> <span class="n">shortcuts</span> <span class="o">=</span> <span class="n">KGlobalAccel</span><span class="o">::</span><span class="n">self</span><span class="p">()</span><span class="o">-></span><span class="n">shortcut</span><span class="p">(</span><span class="n">m_action<span class="hl">A</span></span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">(I can confirm that the kdelibs4 unittest passes with m_actionA here instead of m_actionB -- well, it obviously passes for both actions)</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/119053/diff/1/?file=285796#file285796line435" style="color: black; font-weight: bold; text-decoration: underline;">src/kglobalaccel.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void KGlobalAccelPrivate::_k_shortcutGotChanged(const QStringList &actionId,</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">435</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">actionShortcuts</span><span class="p">.</span><span class="n">insert</span><span class="p">(</span><span class="n">action</span><span class="p">,</span> <span class="n">shortcutFromIntList</span><span class="p">(</span><span class="n">keys</span><span class="p">));</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">sounds like shortcutFromInList(keys) should be put in a local variable, for use in this line and the next.</pre>
</div>
<br />
<p>- David Faure</p>
<br />
<p>On July 1st, 2014, 7:59 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, Plasma and David Faure.</div>
<div>By Martin Gräßlin.</div>
<p style="color: grey;"><i>Updated July 1, 2014, 7:59 a.m.</i></p>
<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;">Sorry for the last minute changes. I expected the bugs to be in the KCM and not in kglobalaccel and unfortunately the autotests cannot be executed on build.kde.org thus I never noticed that they don't pass :-(
Update actionShortcuts on auto loading shortcuts
When using auto-loading the shortcut needs to be updated from what the
daemon returns. This had not been done yet and KGlobalAccel still kept
the shortcut passed in.
With this change the autotests pass and the global shortcut KCM shows
the used shortcuts properly.
Update actionShortcuts when daemon emits yourShortcutGotChanged
Without updating the shortcut kept in actionShortcuts KGlobalAccel
still returns the old shortcut when invoking the shortcut() method.
The unit test for this was unfortunately broken as it checked the
wrong QAction. This is also fixed with this change.
Set componentName and DisplayName on QActions in KGlobalShortcutTest
This fixes the testFindActionByKey.</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;">unit tests pass now.</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/kglobalshortcuttest.cpp <span style="color: grey">(3f76760cb9e06a19d636a81546706cee548e1869)</span></li>
<li>src/kglobalaccel.cpp <span style="color: grey">(a98891c806764132b4f02b828d59584a85745fb3)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/119053/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>