> On Feb. 5, 2015, 8:20 p.m., David Edmundson wrote: > > ksmserver/screenlocker/lockwindow.h, line 41 > > <https://git.reviewboard.kde.org/r/122419/diff/2/?file=347074#file347074line41> > > > > Personally I'd pass the GlobalAccel in the ctor. You have it available, > > then you don't need to have the setter and more importantly you can rely > > m_globalBlah existing so you don't need to check in the if() in > > lockwindow.cpp; > > > > But whatever, this works too.
I decided against it for multiple reasons: first of all I don't like the ever getting longer ctors and think dedicated setter methods are better. Second: the argument with the not needed if is wrong ;-) There might be cases where GlobalAccel will be null, so it's still needed. E.g. the unit tests don't construct it and I assume in the Wayland case we will not use it (GlobalAccel needs to move into kwin, ksld needs to move into kwin, input passes through kwin - we can do better then) - Martin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122419/#review75492 ----------------------------------------------------------- On Feb. 5, 2015, 8:58 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. 5, 2015, 8:58 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
