> On Feb. 4, 2015, 11:28 a.m., David Edmundson wrote: > > ksmserver/screenlocker/globalaccel.cpp, line 273 > > <https://git.reviewboard.kde.org/r/122419/diff/1/?file=346752#file346752line273> > > > > you're not handling emacs style shortcuts here.
AFAIK emacs style shortcuts are not supported as global shortcuts. If I am wrong in that, please show me an example, that I can look at it. > On Feb. 4, 2015, 11:28 a.m., David Edmundson wrote: > > ksmserver/screenlocker/globalaccel.cpp, line 66 > > <https://git.reviewboard.kde.org/r/122419/diff/1/?file=346752#file346752line66> > > > > why bother doing this and the check in init? It's not like kglobal > > accel is going to reset whilst the locker is active, and even if it does > > close we don't need to react differently. > > > > It's DBus activated so the right thing to do (TM) is to call ListNames > > regardless, and to call Invoke regardless. The DBus server will start up > > kglobalaccel5 if it's not running; and if it can't run you get an error > > which you handle anyway. > > > > you can just remove m_connected, and the entirity of init(). > > > > > > Also as-is you have a race if lock is called before init's ListName > > finishes which would make shortcuts not work in that instance I do not think that the lock screen should start kglobalaccel (should not change state of session). That's why I only want to interact with it if it's available and why I did the checks. I think they are useful. > Also as-is you have a race if lock is called before init's ListName finishes > which would make shortcuts not work in that instance doesn't matter, could only happen during session startup - it's ksmserver after all. > On Feb. 4, 2015, 11:28 a.m., David Edmundson wrote: > > ksmserver/screenlocker/globalaccel.cpp, line 113 > > <https://git.reviewboard.kde.org/r/122419/diff/1/?file=346752#file346752line113> > > > > if we're treating this as a bool, why not just make it a bool? > > > > The only person that increments this is this method, and that will only > > do it when it's m_updatingInformation is 0. there are multiple places increasing m_updatingInformation. - Martin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122419/#review75369 ----------------------------------------------------------- On Feb. 4, 2015, 10:02 a.m., Martin Gräßlin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122419/ > ----------------------------------------------------------- > > (Updated Feb. 4, 2015, 10:02 a.m.) > > > Review request for Plasma. > > > Bugs: 198097 > https://bugs.kde.org/show_bug.cgi?id=198097 > > > Repository: plasma-workspace > > > Description > ------- > > This change implements support for white listed global shortcuts in > the lock screen. It interacts with KGlobalAccel to fetch shortcuts > and checks them when a key is pressed. For more detailed information > on how this functions, please see the documentation added to the new > file globalacel.h. > > So far only shortcuts from kmix are white listed. This allows to > mute and change volume while the screen is locked. > > CCBUG: 148228 > CCBUG: 104353 > FEATURE: 198097 > FIXED-IN: 5.3.0 > > > Diffs > ----- > > ksmserver/screenlocker/CMakeLists.txt > f73ec98bdc05d5cea7802c5ccb1354b6a3efa2f5 > ksmserver/screenlocker/autotests/CMakeLists.txt > 9c940a8fe97ae488aeea53d1f1abb3c38c2e13de > ksmserver/screenlocker/globalaccel.h PRE-CREATION > ksmserver/screenlocker/globalaccel.cpp PRE-CREATION > ksmserver/screenlocker/ksldapp.h 2e2e5dcc721d3854fad4ae4019e767a7d1a33718 > ksmserver/screenlocker/ksldapp.cpp 8c8607d86d700ade3e1e5b34cbf5c0233897d9ce > ksmserver/screenlocker/lockwindow.h > cad62ed0f3583f78b0bdb2d96990c8441b8d3b9d > ksmserver/screenlocker/lockwindow.cpp > 0d5afa879051e0802cf1b676ec6024783d3da959 > > Diff: https://git.reviewboard.kde.org/r/122419/diff/ > > > Testing > ------- > > So far only with the test application > > > Thanks, > > Martin Gräßlin > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
