> On Jan. 4, 2013, 7:56 a.m., Kevin Ottens wrote: > > kdeui/shortcuts/kgesturemap.h, line 44 > > <http://git.reviewboard.kde.org/r/107973/diff/2/?file=103127#file103127line44> > > > > I'd expect to see the action as first parameter since we're kind of > > moving from "action->setGesture(gesture)" seeing it become > > "map->setGesture(action, gesture)" feels more natural to me (the assignment > > order is respected). > > > > Of course this comment holds for all the other setters in that class, I > > won't open separate issues.
Totally agree with that. In fact I did thought to this by myself, but hesitated to change the signature. > On Jan. 4, 2013, 7:56 a.m., Kevin Ottens wrote: > > kdeui/shortcuts/kgesturemap.h, line 55 > > <http://git.reviewboard.kde.org/r/107973/diff/2/?file=103127#file103127line55> > > > > This one looks like a nice convenience to keep though as there's > > several gesture types, etc. I'd rename it to removeAllGestures though. > On Jan. 4, 2013, 7:56 a.m., Kevin Ottens wrote: > > kdeui/shortcuts/kgesturemap.h, line 65 > > <http://git.reviewboard.kde.org/r/107973/diff/2/?file=103127#file103127line65> > > > > Instead of that one we should have two methods instead: shapeGesture > > and defaultShapeGesture. This way we avoid the QPair completely from the > > API (again, which one would be default? .first? or .second?). Well, that one is a matter of taste, as I planned to create a ActionShapeGestures class, inheriting the QPair, providing two getters named defaultGesture and activeGesture. But I'll stick with your suggestion, as it avoids introducing another symbol (ActionShapeGestures). - Valentin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107973/#review24635 ----------------------------------------------------------- On Jan. 4, 2013, 9:18 p.m., Valentin Rusu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/107973/ > ----------------------------------------------------------- > > (Updated Jan. 4, 2013, 9:18 p.m.) > > > Review request for KDE Frameworks and Kevin Ottens. > > > Description > ------- > > This changeset removes all gesture facilities from KAction and define them in > KGestureMap. Thanks for the feedback. > > > Diffs > ----- > > KDE5PORTING.html 86dfc1a > kdeui/actions/kaction.h f45c94d > kdeui/actions/kaction.cpp b78d1d6 > kdeui/actions/kaction_p.h f87272c > kdeui/dialogs/kshortcutseditor.cpp 5653984 > kdeui/dialogs/kshortcutseditoritem.cpp e85a203 > kdeui/shortcuts/kgesturemap.h 56b42b6 > kdeui/shortcuts/kgesturemap.cpp 1350dbe > > Diff: http://git.reviewboard.kde.org/r/107973/diff/ > > > Testing > ------- > > Code builds OK. KGestureMap is not actually used, though, as > KGestureMap::eventFilter method return false right away. > > > Thanks, > > Valentin Rusu > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
