Review Request: Export the function isShiftAsModifierAllowed() from KKeySequenceWidget

Simon Persson simonpersson1 at gmail.com
Wed Jun 29 16:13:14 BST 2011


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




More information about the kde-core-devel mailing list