<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/122981/">https://git.reviewboard.kde.org/r/122981/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 4th, 2015, 9:14 a.m. UTC, <b>Gregor Mi</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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/122981/diff/4/?file=359983#file359983line372" style="color: black; font-weight: bold; text-decoration: underline;">src/kglobalaccel.h</a>
<span style="font-weight: normal;">
(Diff revision 4)
</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; ">Q_SIGNALS:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">353</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">void</span> <span class="n">globalShortcutChanged</span><span class="p">(</span><span class="n">QAction</span> <span class="o">*</span><span class="n">action</span><span class="p">,</span> <span class="k">const</span> <span class="n">QKeySequence</span> <span class="o">&</span><span class="n">seq</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">372</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">void</span> <span class="n">globalShortcutChanged</span><span class="p">(</span><span class="k"><span class="hl">const</span></span><span class="hl"> </span><span class="n">QAction</span> <span class="o">*</span><span class="n">action</span><span class="p">,</span> <span class="k">const</span> <span class="n">QKeySequence</span> <span class="o">&</span><span class="n">seq</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">NOTE that this causes compile error in libKF5XmlGui
/home/gregor/dev/kf5/usr/lib64/libKF5XmlGui.so.5.8.0: undefined reference to `KGlobalAccel::globalShortcutChanged(QAction*, QKeySequence const&)'
collect2: error: ld returned 1 exit status</pre>
</blockquote>
<p>On April 4th, 2015, 9:40 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;">"Of course not" - that's an ABI <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">and</em> API incompatible change (big no-go, not gonna happen)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">a) introduce a flag to ::updateGlobalShortcut() whether the shortcut should actually be updated (or just somehow initialized) by shortcut() (defaulting true)
b) if that flag is false (set by shortcut() on invocation)
1. exit early if there's a known shortcut to skip the expensive stuff
2. NEVER emit the changed signal
c) const_cast<QAction*> the action parameter from ::shortcut()
d) add comments that constness is to be (weakly) guaranteed if "onlyInitial" (or so) is true
e) add a TODO for KF/6 to remove constness from the ::shortcut() parameter</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; 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;">I removed the const (since it is not needed anymore for this RR) and left comments instead for future improvements.</p></pre>
<br />
<p>- Gregor</p>
<br />
<p>On April 10th, 2015, 10:53 a.m. UTC, Gregor Mi 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 Gregor Mi.</div>
<p style="color: grey;"><i>Updated April 10, 2015, 10:53 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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In some cases you need to call loadShortcutFromGlobalSettings() in order not to get a an empty list when calling shortcut() (which is const).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">See discussion in https://git.reviewboard.kde.org/r/122249/ ("libksysguard: add Kill Window to End Process button and show correct keyboard shortcut").</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>autotests/kglobalshortcuttest.cpp <span style="color: grey">(3b661bbb612807a3bbbe34835d4ae712c2ec74da)</span></li>
<li>src/kglobalaccel.h <span style="color: grey">(3fe20ca8e4ec6ceb0bb9e54235aef7f1aeeb8c16)</span></li>
<li>src/kglobalaccel.cpp <span style="color: grey">(1b6b3f5cb6d42401d684e6a491d12a6e57248fd1)</span></li>
<li>src/kglobalaccel_p.h <span style="color: grey">(eca7c52378ad60d0d5806561214b9788dd46a11e)</span></li>
<li>autotests/kglobalshortcuttest.h <span style="color: grey">(b1122a8f5ca2f3f7afbe78f8edba87325426c1a6)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/122981/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>