<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/107973/">http://git.reviewboard.kde.org/r/107973/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 28th, 2012, 5:17 p.m., <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/107973/diff/1/?file=102093#file102093line460" style="color: black; font-weight: bold; text-decoration: underline;">kdeui/actions/kaction.h</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; ">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">460</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">KShapeGesture</span> <span class="n">shapeGesture</span><span class="p">(</span><span class="n">ShortcutTypes</span> <span class="n">type</span> <span class="o">=</span> <span class="n">ActiveShortcut</span><span class="p">)</span> <span class="k">const</span><span class="p">;</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></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;">Since the end game is to move KAction in kde4support, we should try to keep its API unchanged.
Please keep those methods in its API, but internally the implementation should just forward to the new implementation you're working on in KGestureMap.</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;">Done</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 28th, 2012, 5:17 p.m., <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/107973/diff/1/?file=102098#file102098line44" style="color: black; font-weight: bold; text-decoration: underline;">kdeui/shortcuts/kgesturemap.h</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; ">class QEvent;</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">41</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">addGesture</span><span class="p">(</span><span class="k">const</span> <span class="n">KShapeGesture</span> <span class="o">&</span><span class="n">gesture</span><span class="p">,</span> <span class="n">KAction</span> <span class="o">*</span><span class="n">kact</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">44</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">addGesture</span><span class="p">(</span><span class="k">const</span> <span class="n">KShapeGesture</span> <span class="o">&</span><span class="n">gesture</span><span class="p">,</span> <span class="n">KAction</span> <span class="o">*</span><span class="n">kact</span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="n"><span class="hl">bool</span></span><span class="hl"> </span><span class="n"><span class="hl">replace</span></span><span class="hl"> </span><span class="o"><span class="hl">=</span></span><span class="nb"><span class="hl">false</span></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;">I don't really like the extra boolean which flips the semantic of addGesture... Looking at lxr.kde.org it turns out that addGesture and removeGesture are never called (except by KAction of course).
That's good news as it means we can more freely modify KGestureMap API. :-)
Seeing the existing calls in KAction basically always do a removeGesture followed by an addGesture, and that KAction itself exposed a setter API, what about simply removing them from the API and just have a setGesture + setDefaultGesture in the public API?</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;">Done</pre>
<br />
<p>- Valentin</p>
<br />
<p>On December 28th, 2012, 11:32 a.m., Valentin Rusu wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Frameworks.</div>
<div>By Valentin Rusu.</div>
<p style="color: grey;"><i>Updated Dec. 28, 2012, 11:32 a.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;">This changeset removes all gesture facilities from KAction and define them in KGestureMap. Thanks for the feedback.</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 builds OK. KGestureMap is not actually used, though, as KGestureMap::eventFilter method return false right away.</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>KDE5PORTING.html <span style="color: grey">(86dfc1a)</span></li>
<li>kdeui/actions/kaction.h <span style="color: grey">(f45c94d)</span></li>
<li>kdeui/actions/kaction.cpp <span style="color: grey">(b78d1d6)</span></li>
<li>kdeui/actions/kaction_p.h <span style="color: grey">(f87272c)</span></li>
<li>kdeui/dialogs/kshortcutseditor.cpp <span style="color: grey">(5653984)</span></li>
<li>kdeui/dialogs/kshortcutseditoritem.cpp <span style="color: grey">(e85a203)</span></li>
<li>kdeui/shortcuts/kgesturemap.h <span style="color: grey">(56b42b6)</span></li>
<li>kdeui/shortcuts/kgesturemap.cpp <span style="color: grey">(1350dbe)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/107973/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>