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