<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/104801/">http://git.reviewboard.kde.org/r/104801/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hate to break the news to you after all that work. But the exact same thing you have achieved with this patch could be achieved by using a QAction for the missing Shortcut. Or a KAction for that matter as long as it is not part of the KActionCollection.
There is only one reason to use KAction. The Shortcuts Editor. And KXMLwhatever. And global shortcuts. So make that there are only three reasons to use KAction.
None of them know how to handle a KAction with three Shortcuts set. So your patch has not (yet?) achieved anything you could not gain by just using a hidden unconfigurable second Q(K)Action. So i would say it does not make sense to apply it in its current form.
Unless you are willing to go all the way which you only should do after finding out what the frameworks branchs does to kaction. So you effort is not thrown away in the near / middle future.
Mike</pre>
<br />
<p>- Michael</p>
<br />
<p>On May 9th, 2012, 6:21 p.m., Mark Gaiser 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 kdelibs.</div>
<div>By Mark Gaiser.</div>
<p style="color: grey;"><i>Updated May 9, 2012, 6:21 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;">So i was trying to fix this bug: https://bugs.kde.org/show_bug.cgi?id=181531 That only asked for one more shortcut. That issue seems to be a little more complicated than it looks. Till this point KActions could only have a "Primary" and a "Alternate" shortcut. 2 in total which is - in some situations - not enough.
I fixed this by roughly restructuring nearly all of the KShortcut cpp file.
The only thing i'm not quite sure about is how "KShortcut KAction::shortcut(ShortcutTypes type) const" looks right now.. If anyone has some clarification on that one..?</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;">I tested this by adding the missing bindings for Dolphin's back/forward and it seems to be working just fine. I can use all available shortcuts.</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/actions/kaction.h <span style="color: grey">(d877554)</span></li>
<li>kdeui/actions/kaction.cpp <span style="color: grey">(309cf82)</span></li>
<li>kdeui/shortcuts/kshortcut.h <span style="color: grey">(c720830)</span></li>
<li>kdeui/shortcuts/kshortcut.cpp <span style="color: grey">(e307ab0)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/104801/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>