<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/109019/">http://git.reviewboard.kde.org/r/109019/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 27th, 2013, 7:16 a.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;">There's been a small misunderstanding I think. You introduced a slot which keep emitting actionGlobalShortcutChanged for actions alright *but* you introduced it in the wrong class. This slot should be in KAction and the connect for KGlobalAccel signal be done in KAction constructor, this way it is completely transparent to KGlobalAccel what KAction is doing.
Otherwise that's inserting an extra KGlobalAccel -> KAction dependency which will be in the way of later having a completely QAction based KGlobalAccel.</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;">Damn, you're right, the QAction move should not be forgotten in the way :-)</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 27th, 2013, 7:16 a.m. UTC, <b>Kevin Ottens</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="http://git.reviewboard.kde.org/r/109019/diff/5/?file=116035#file116035line481" style="color: black; font-weight: bold; text-decoration: underline;">kdeui/shortcuts/kglobalaccel.cpp</a>
<span style="font-weight: normal;">
(Diff revision 5)
</span>
</th>
</tr>
</thead>
<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">480</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">void</span> <span class="n">KGlobalAccelPrivate</span><span class="o">::</span><span class="n">_k_emitActionGlobalShortcutChanged</span><span class="p">(</span><span class="n">KAction</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;">It should go in kaction.cpp. Of course it will become KAction::_k_emitActionGlobalShortcutChanged and the emit will occur only if action == this.</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;">Well, I rather put it to KActionPrivate ;-)</pre>
<br />
<p>- Valentin</p>
<br />
<p>On February 26th, 2013, 7:28 p.m. UTC, Valentin Rusu 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 Valentin Rusu.</div>
<p style="color: grey;"><i>Updated Feb. 26, 2013, 7:28 p.m.</i></p>
<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;">KDELibs cleanup task: move global shortcut facilities from KAction to KGlobalAccel. The second step, registering QActions instead of KActions, will be done after completing this review.</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;">Code compiles</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>kdeui/shortcuts/kglobalaccel_p.h <span style="color: grey">(04feaba)</span></li>
<li>kdeui/shortcuts/kglobalaccel.cpp <span style="color: grey">(36dbab7)</span></li>
<li>kdeui/actions/kaction_p.h <span style="color: grey">(b50e678)</span></li>
<li>kdeui/shortcuts/kglobalaccel.h <span style="color: grey">(ed68bba)</span></li>
<li>kdeui/actions/kaction.cpp <span style="color: grey">(ec63a72)</span></li>
<li>KDE5PORTING.html <span style="color: grey">(e1b41d1)</span></li>
<li>kdeui/actions/kaction.h <span style="color: grey">(ddaa4de)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/109019/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>