> On June 16, 2014, 7:10 p.m., Martin Gräßlin wrote: > > I would like to get the opinion from people who designed kglobalaccel. > > Personally I do not understand what the default shortcuts are for and why > > they are needed. > > > > From reading the documentation my understanding is that your patch is wrong > > and instead the applications need to be fixed (it's just the default as > > what one can click in the config interface, but not the loaded global > > shortcut). > > Vishesh Handa wrote: > The difference between an "active" and "default" shortcut is that the > user cannot configure a default shortcut. Have a look at the systemsettings > for global shortcuts. You'll get a better idea. > > About the application being wrong, I disagree. Most people would think > that calling 'setDefaultShortcut(..)' would actually set the shortcut and not > just write it to a config file. The way I see it, we either except this > patch, or we change kglobalaccel (the daemon) to actually utilize the default > shortcut. If you want, I can submit a patch for that. It's just about 4-5 > lines. > > Martin Gräßlin wrote: > I don't know. Given the documentation I understand it as "this is just > the default shortcut", if one wants to have the global shortcut, the > setShortcut method needs to be used. Whether it's a valid use case to only > have the default shortcut, I'm not sure. But I assume that this was intended > behavior by the people who implemented it. > > Overall my feeling is that default is not intended to be used at all. But > as said: we need input here from people more experienced with kglobalaccel > > Vishesh Handa wrote: > I've been running this patch for a day now, and it is buggy. Custom > shortcuts seem to get overwritten with the default one. I'm not bothering to > fix it, as it would be best to figure out what course of action we want to > take.
As mentioned during the meeting today: if should set the default and active shortcut. - Kevin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118783/#review60209 ----------------------------------------------------------- On June 16, 2014, 4:49 p.m., Vishesh Handa wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/118783/ > ----------------------------------------------------------- > > (Updated June 16, 2014, 4:49 p.m.) > > > Review request for KDE Frameworks and Martin Gräßlin. > > > Repository: kglobalaccel > > > Description > ------- > > When a client calls setDefaultShortcut, only the default shortcut is > set. This makes sense, however kglobalaccel doesn't actually do anything > with the global shortcut, it just writes it to the configuration and > reads it back. > > All the actual logic is implemented behind "active" or "normal" shortcuts. > In kdelibs4, most applications would call KAction::setGlobalShorcut which > had a default of setting BOTH the active and the default shortcut. Again, > I'm not sure what they point of this was if the default shortcut does not > actually do anything. > > This fixes bugs such as the brightness key not working because > Powerdevil only sets the "default" shortcut. > > > Diffs > ----- > > src/kglobalaccel.cpp 54d18ec > > Diff: https://git.reviewboard.kde.org/r/118783/diff/ > > > Testing > ------- > > > Thanks, > > Vishesh Handa > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
