> On May 9, 2012, 8:53 p.m., Michael Jansen wrote:
> > 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

Well.. that sucks!

Anyway, this patch is the first part. It doesn't break any backwards 
compatibility and simply allows apps that want to use more shortcuts to freely 
use them.
The second patch would be against Dolphin to allow some more shortcuts. I 
already have the dolphin patch ready.

After that the next patch would be to get the Shortcut Editor to support this 
as well. I don't have a patch for that, yet!

>From a quick look at KShortcut in frameworks it seems to be just the same as 
>the current 4.8 branch. Just a bunch of TODO items for the constructors to use 
>QShortcut. KShortcut doesn't seem to be going away.

I would like to push this one and fix dolphin and the shortcut editor if i'm 
allowed to.


If that's not oke, then please do tell me how to fix this the proper way. This 
is what i would like to start doing if it's not oke to push:
Step 1. KAction to inherit QAction and add the global shortcut stuff. Perhaps 
some more stuff.
Step 2. Rewrite KShortcut to inherit QShortcut and ONLY have the additional 
option for global shortcuts.
Step 3. Adjust the Shortcut Editor to use the new structure.
Step 4. Deprecate most of KAction in the 4.8 kdelibs branch.
Step 5. Deprecate all of KShortcut except the global related stuff

Your input would be welcome.

-- little braindump --
It would be very nice to have a KDE "KInputShortcut" class in which you can mix 
shortcuts from various input devices. For example: KInputShortcut(Qt::CTRL + 
Qt::LeftButton) .. That would really add something useful!


- Mark


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104801/#review13624
-----------------------------------------------------------


On May 9, 2012, 6:21 p.m., Mark Gaiser wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104801/
> -----------------------------------------------------------
> 
> (Updated May 9, 2012, 6:21 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Description
> -------
> 
> 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..?
> 
> 
> Diffs
> -----
> 
>   kdeui/actions/kaction.h d877554 
>   kdeui/actions/kaction.cpp 309cf82 
>   kdeui/shortcuts/kshortcut.h c720830 
>   kdeui/shortcuts/kshortcut.cpp e307ab0 
> 
> Diff: http://git.reviewboard.kde.org/r/104801/diff/
> 
> 
> Testing
> -------
> 
> 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.
> 
> 
> Thanks,
> 
> Mark Gaiser
> 
>

Reply via email to