On Tuesday 07 June 2011 16.10.55 Aaron J. Seigo wrote: > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/101515/#review3736 > ----------------------------------------------------------- > > > both of the classes that need this are in the kdeui library, so there is no > reason to export this symbol. it is not imho API that needs to or should > be public. i'd instead recommend moving this to a private header, such as > kglobalaccel_p.h and including that header in both of the .cpp classes > that need it. > > - Aaron J. >
Aaron, I forgot that there's a kglobalaccel in kdelibs when I wrote the summary.. therefore I didn't specify that this function is needed by the daemon kglobalaccel from kde-runtime. I went ahead and commited today, in retrospect I should probably have written this email first... Let me know if you still have concerns regarding this! Simon > On June 6, 2011, 8:46 a.m., Simon Persson wrote: > > ----------------------------------------------------------- > > This is an automatically generated e-mail. To reply, visit: > > http://git.reviewboard.kde.org/r/101515/ > > ----------------------------------------------------------- > > > > (Updated June 6, 2011, 8:46 a.m.) > > > > > > Review request for kdelibs and Michael Jansen. > > > > > > Summary > > ------- > > > > First of two patches to fix a set of bugs for global shortcuts involving shift key. Not sure if this can go in 4.7 because of the freeze? It adds a new function to the API for the sake of a bugfix. Prepared the following commit message: > > Export the function isShiftAsModifierAllowed() > > > > This function inside KKeySequnceWidgetPrivate is needed by > > kglobalaccel, export here rather than duplicating code and risk > > falling out of sync. Also add Qt::Key_Backtab to the list, this is > > not needed internally since there is a special case for > > Alt+Shift+Tab to not be recorded as Alt+Backtab but other users of > > this function will need it. > > > > This addresses bugs 179504, 197548 and 215030. > > > > http://bugs.kde.org/show_bug.cgi?id=179504 > > http://bugs.kde.org/show_bug.cgi?id=197548 > > http://bugs.kde.org/show_bug.cgi?id=215030 > > > > Diffs > > ----- > > > > kdeui/widgets/kkeysequencewidget.h b9aafdc > > kdeui/widgets/kkeysequencewidget.cpp a35c2b4 > > > > Diff: http://git.reviewboard.kde.org/r/101515/diff > > > > > > Testing > > ------- > > > > > > Thanks, > > > > Simon