> On mar. 22, 2015, 3:56 p.m., Albert Astals Cid wrote: > > src/kglobalaccel.h, line 260 > > <https://git.reviewboard.kde.org/r/122981/diff/2/?file=356138#file356138line260> > > > > typo in following > > Gregor Mi wrote: > About why it is needed: see > KGlobalShortcutTest::testLoadShortcutFromGlobalSettings: shortcut() does not > behave as expected when the new method loadShortcutFromGlobalSettings() is > not called > > I don't know if always calling loadShortcutFromGlobalSettings() before > shortcut() would break existing code. E.g. in > kwin/effects/desktopgrid/desktopgrid_config.cpp shortcut() is used without > prior call of loadShortcutFromGlobalSettings(). > > On the other hand, a call to loadShortcutFromGlobalSettings() might be > harmless but I don't have a good overview of the KGlobalAccel usage patterns. > > Albert Astals Cid wrote: > Honestly i have no idea on how this kglobalaccel thing works, but it > seems to me you're just adding a way to workaround a bug instead of fixing it. > > Gregor Mi wrote: > Me neither :-) > > I was hoping that someone with more experience with kglobalaccel can say > more about the proposed solution. > > Like you Thomas suggested to put the loading method into shortcut() > (which is essentially a one-liner: d->updateGlobalShortcut(action, > KGlobalAccelPrivate::ActiveShortcut, KGlobalAccel::Autoloading);). > > Martin Gräßlin wrote: > well we could try it: add the one-line change and try whether the unit > tests pass and then try whether the case in DesktopGridConfig works. > > Unfortunately I'm also not that into KGloalAccel code yet to be 100 % > whether it's a valid change :-( > > Gregor Mi wrote: > I tried to call loadShortcutFromGlobalSettings() from within shortcut() > and found that this violates const constraints: > > shortcut() is const > d->updateGlobalShortcut is not const > > So this is not possible. > > How to proceed now? > > Albert Astals Cid wrote: > make mutable the members that updateGlobalShortcut modifies > > Gregor Mi wrote: > I tried again and the call was even possible with only introducing some > more consts. But the call of loadShortcutFromGlobalSettings() from within > shortcut() makes more unit tests fail than without. > > => the general call of loadShortcutFromGlobalSettings() from within > shortcut() seems to harm more than it helps. > > Currently, I have a problem with getting the unit tests to all pass > (which is independet of this change; it happens also in master branch). See > my analysis below and comment of the unit test itself: > > /* These tests could be better. They don't include actually triggering > actions, > and we just choose very improbable shortcuts to avoid conflicts with > real > applications' shortcuts. */ > > It would be nice to document some instructions of how to reset the global > shortcuts to a defined state.
Use strace to see which files are being read. - Albert ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122981/#review77912 ----------------------------------------------------------- On abr. 3, 2015, 5:50 p.m., Gregor Mi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122981/ > ----------------------------------------------------------- > > (Updated abr. 3, 2015, 5:50 p.m.) > > > Review request for KDE Frameworks, Martin Gräßlin and Thomas Lübking. > > > Repository: kglobalaccel > > > Description > ------- > > In some cases you need to call loadShortcutFromGlobalSettings() in order not > to get a an empty list when calling shortcut() (which is const). > > See discussion in https://git.reviewboard.kde.org/r/122249/ ("libksysguard: > add Kill Window to End Process button and show correct keyboard shortcut"). > > > Diffs > ----- > > autotests/kglobalshortcuttest.h b1122a8f5ca2f3f7afbe78f8edba87325426c1a6 > autotests/kglobalshortcuttest.cpp 3b661bbb612807a3bbbe34835d4ae712c2ec74da > src/kglobalaccel.h 3fe20ca8e4ec6ceb0bb9e54235aef7f1aeeb8c16 > src/kglobalaccel.cpp 1b6b3f5cb6d42401d684e6a491d12a6e57248fd1 > src/kglobalaccel_p.h eca7c52378ad60d0d5806561214b9788dd46a11e > > Diff: https://git.reviewboard.kde.org/r/122981/diff/ > > > Testing > ------- > > > Thanks, > > Gregor Mi > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
