Review Request 111453: move kacceleratormanager to KWidgetAddons

David Faure faure at kde.org
Tue Jul 9 16:36:10 UTC 2013


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111453/#review35801
-----------------------------------------------------------



kdeui/shortcuts/kacceleratormanager_private.h
<http://git.reviewboard.kde.org/r/111453/#comment26248>

    A forward declaration "class QTabBar;" is enough here, same for qmenubar and qdockwidget.



staging/kwidgets/src/actions/kstandardaction.cpp
<http://git.reviewboard.kde.org/r/111453/#comment26247>

    The QStringList pointer is not great. How about this instead?
    
    KAcceleratorManagerPrivate::setStandardActionNames(stdNames());
    With a function-local static bool so that this is done only once.
    
    Even better would be to export a single function in a namespace, rather than exporting the whole class KAcceleratorManagerPrivate.
    
    
    Note that kaccmp_sns can go away completely then.


- David Faure


On July 8, 2013, 4:39 p.m., Wojciech Kapuscinski wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111453/
> -----------------------------------------------------------
> 
> (Updated July 8, 2013, 4:39 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Description
> -------
> 
> move kacceleratormanager to KWidgetAddons.
> 
> Note:
> This is only for review because this patch contains some ugly hack (export symbol from private class) and this patch is only Proof-on-concept.
> 
> Changes:
> rename kacceleratormanager_private.h -> kacceleratormanager_p.h
> move KAcceleratorManagerPrivate class definition from kacceleratormanager.cpp to kacceleratormanager_p.h
> set translated texts when KStandardAction is constructed for the firs time.
> 
> Open questions:
> - should I add static method for example initStandardNames in KAcceleratorManager namespace that replace appendStandarNames and remove export symbol from KAcceleratorManagerPrivate ?
> - use QMutex in KAcceleratorManager that guarantees thread-safety (lock/unlock mutex in standardNames and initStandardNames)?
> 
> 
> Diffs
> -----
> 
>   kdeui/CMakeLists.txt 35b4019 
>   kdeui/shortcuts/kacceleratormanager.h 68e87d2 
>   kdeui/shortcuts/kacceleratormanager.cpp 990b093 
>   kdeui/shortcuts/kacceleratormanager_private.h ab04d42 
>   staging/kwidgets/src/actions/kstandardaction.cpp 0c3733a 
>   tier1/kwidgetsaddons/src/CMakeLists.txt 82d42db 
> 
> Diff: http://git.reviewboard.kde.org/r/111453/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Wojciech Kapuscinski
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20130709/b1984f6d/attachment.html>


More information about the Kde-frameworks-devel mailing list