Review Request 111210: Move KHelpMenu and friends to XmlGui
David Faure
faure at kde.org
Wed Jul 3 10:56:38 UTC 2013
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111210/#review35498
-----------------------------------------------------------
Impressive piece of work, well done.
The commit log is a bit confusing. KHelpMenu is really a small piece of the puzzle. This in fact most of the XMLGUI technology to the xmlgui framework, including KToolBar and KMainWindow.
kdeui/dialogs/kaboutapplicationdialog.cpp
<http://git.reviewboard.kde.org/r/111210/#comment26044>
please add a
#pragma message("Use QFontDatabase::systemFont(QFontDatabase::FixedFont) once https://codereview.qt-project.org/59808 is in")
(so you can also clarify that part of the message log - the final decision is clear now)
kdeui/dialogs/kaboutapplicationpersonmodel_p.cpp
<http://git.reviewboard.kde.org/r/111210/#comment26045>
That's quite verbose. QLatin1String("Delicious") or better QStringLiteral("Delicious") would be enough. The conversion to QString will happen implicitly since that's the return type of the method.
kdeui/tests/kbugreporttest.cpp
<http://git.reviewboard.kde.org/r/111210/#comment26047>
QLatin1String(0) hurts my eyes. Please make it just QString().
kdeui/tests/ktoolbar_unittest.cpp
<http://git.reviewboard.kde.org/r/111210/#comment26052>
strange test indeed... we notify of the change, but don't check anything afterwards. Weird.
Let's leave it for now, then.
kdeui/xmlgui/kmainwindow.cpp
<http://git.reviewboard.kde.org/r/111210/#comment26053>
something strange happened here. A newline got removed?
kdeui/xmlgui/kxmlguibuilder.cpp
<http://git.reviewboard.kde.org/r/111210/#comment26059>
ok, if every use of "name" below is to convert it back to QString, why not change the type of this variable to a QString and remove the toUtf8() ?
kdeui/xmlgui/kxmlguiclient.cpp
<http://git.reviewboard.kde.org/r/111210/#comment26061>
qPrintable converts QString to char*.
QLatin1String converts char* to QString.
It's like matter and anti-matter :-)
Please remove both.
kdeui/xmlgui/kxmlguiclient.cpp
<http://git.reviewboard.kde.org/r/111210/#comment26070>
same here
kdeui/xmlgui/kxmlguiclient.cpp
<http://git.reviewboard.kde.org/r/111210/#comment26071>
same here
staging/xmlgui/src/CMakeLists.txt
<http://git.reviewboard.kde.org/r/111210/#comment26077>
Sonnet? I'm surprised. What's the relation between kmainwindow/kxmlgui* and sonnet?
- David Faure
On July 2, 2013, 4:31 p.m., Andrea Scarpino wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111210/
> -----------------------------------------------------------
>
> (Updated July 2, 2013, 4:31 p.m.)
>
>
> Review request for KDE Frameworks.
>
>
> Description
> -------
>
> Move:
> * KAboutApplicationDialog
> * KBugReport
> * KHelpMenu
> * KShortcutsEditor
> * KShortcutsDialog
> * KEditToolBar
> * KShortcutWidget
> * KKeySequenceWidget
> * KXmlGuiClient
> * KXmlGuiFactory
> * KXmlGuiBuilder
> * KMainWindow
> * KToolBar
> * KActionCollection
> * KToggleBarAction
> * KActionCategory
> to XmlGui.
>
> Notes:
> * the code that uses KGestureMap has been commented. What to do?
> * the code that uses KGlobalSettings has been commented. Waiting for a decision to be taken in the thread http://lists.kde.org/?l=kde-frameworks-devel&m=137149104802300&w=2
>
> Needs both:
> * https://git.reviewboard.kde.org/r/111183/
> * https://git.reviewboard.kde.org/r/111191/
>
>
> Diffs
> -----
>
> kdeui/CMakeLists.txt d8b9b53
> kdeui/actions/kactioncategory.h 25f3be8
> kdeui/actions/kactioncategory.cpp
> kdeui/actions/kactioncollection.h 8ca6845
> kdeui/actions/kactioncollection.cpp 7dc2987
> kdeui/actions/ktoggletoolbaraction.h e937ba3
> kdeui/actions/ktoggletoolbaraction.cpp 5db4192
> kdeui/dialogs/kaboutapplicationconfigattica_p.h.cmake
> kdeui/dialogs/kaboutapplicationdialog.h 7459f7e
> kdeui/dialogs/kaboutapplicationdialog.cpp d656fa2
> kdeui/dialogs/kaboutapplicationpersonlistdelegate_p.h
> kdeui/dialogs/kaboutapplicationpersonlistdelegate_p.cpp 8933202
> kdeui/dialogs/kaboutapplicationpersonlistview_p.h
> kdeui/dialogs/kaboutapplicationpersonlistview_p.cpp
> kdeui/dialogs/kaboutapplicationpersonmodel_p.h 5001da8
> kdeui/dialogs/kaboutapplicationpersonmodel_p.cpp 25d2765
> kdeui/dialogs/kaboutkdedialog_p.h
> kdeui/dialogs/kaboutkdedialog_p.cpp 35061f6
> kdeui/dialogs/kbugreport.h c3ace5e
> kdeui/dialogs/kbugreport.cpp cf56aa3
> kdeui/dialogs/kedittoolbar.h 217c3c6
> kdeui/dialogs/kedittoolbar.cpp b4fb8cb
> kdeui/dialogs/kedittoolbar_p.h c6436d2
> kdeui/dialogs/kshortcuteditwidget.cpp 228a401
> kdeui/dialogs/kshortcutschemeseditor.cpp 095aeeb
> kdeui/dialogs/kshortcutsdialog.h 54082f9
> kdeui/dialogs/kshortcutsdialog.cpp 1a187b3
> kdeui/dialogs/kshortcutsdialog.ui
> kdeui/dialogs/kshortcutsdialog_p.h 78067fa
> kdeui/dialogs/kshortcutseditor.h 5b7e24a
> kdeui/dialogs/kshortcutseditor.cpp 5fad58b
> kdeui/dialogs/kshortcutseditordelegate.cpp 53c619c
> kdeui/dialogs/kshortcutseditoritem.cpp 3be65ef
> kdeui/dialogs/kswitchlanguagedialog_p.h
> kdeui/dialogs/kswitchlanguagedialog_p.cpp 466d8dc
> kdeui/kdepackages.h
> kdeui/make_kdepackages_updated.py
> kdeui/shortcuts/kshortcutschemeshelper.cpp 1ea1631
> kdeui/shortcuts/kshortcutschemeshelper_p.h
> kdeui/tests/CMakeLists.txt 6ef234a
> kdeui/tests/kactioncategorytest.h
> kdeui/tests/kactioncategorytest.cpp
> kdeui/tests/kbugreporttest.cpp cd71173
> kdeui/tests/kmainwindow_unittest.h
> kdeui/tests/kmainwindow_unittest.cpp
> kdeui/tests/kmainwindowrestoretest.h
> kdeui/tests/kmainwindowrestoretest.cpp 95146a2
> kdeui/tests/kmainwindowtest.h
> kdeui/tests/kmainwindowtest.cpp 3deac97
> kdeui/tests/ktoolbar_unittest.cpp 4e1431b
> kdeui/tests/ktoolbartest.cpp 25c0598
> kdeui/tests/kxmlgui_unittest.h
> kdeui/tests/kxmlgui_unittest.cpp cf99a0a
> kdeui/tests/kxmlguitest.h
> kdeui/tests/kxmlguitest.cpp cc8d9d3
> kdeui/tests/kxmlguitest_part.rc
> kdeui/tests/kxmlguitest_shell.rc
> kdeui/tests/kxmlguiwindowtest.cpp ac484e2
> kdeui/tests/kxmlguiwindowtestui.rc
> kdeui/tests/testguiclient.h f10207e
> kdeui/tests/testxmlguiwindow.h 0ac78fe
> kdeui/widgets/khelpmenu.h 9210810
> kdeui/widgets/khelpmenu.cpp 24e44db
> kdeui/widgets/kkeysequencewidget.h 6ea1b5a
> kdeui/widgets/kkeysequencewidget.cpp 235353f
> kdeui/widgets/kkeysequencewidget_p.h
> kdeui/widgets/kshortcutwidget.h 18ec960
> kdeui/widgets/kshortcutwidget.cpp
> kdeui/widgets/kshortcutwidget.ui
> kdeui/xmlgui/kmainwindow.h 32e37cf
> kdeui/xmlgui/kmainwindow.cpp e7f2d31
> kdeui/xmlgui/kmainwindow_p.h 92646f3
> kdeui/xmlgui/kmainwindowiface.cpp 6c77d93
> kdeui/xmlgui/kmainwindowiface_p.h b59b2b1
> kdeui/xmlgui/kmenumenuhandler_p.h
> kdeui/xmlgui/kmenumenuhandler_p.cpp 27cd5c3
> kdeui/xmlgui/ktoolbar.h 94f5c81
> kdeui/xmlgui/ktoolbar.cpp 8157bb4
> kdeui/xmlgui/ktoolbarhandler.cpp 1ad8da6
> kdeui/xmlgui/ktoolbarhandler_p.h
> kdeui/xmlgui/kxmlguibuilder.h e841384
> kdeui/xmlgui/kxmlguibuilder.cpp 04d760c
> kdeui/xmlgui/kxmlguiclient.h 3b06b50
> kdeui/xmlgui/kxmlguiclient.cpp 96ea35b
> kdeui/xmlgui/kxmlguifactory.h c6e853b
> kdeui/xmlgui/kxmlguifactory.cpp 272b010
> kdeui/xmlgui/kxmlguifactory_p.h
> kdeui/xmlgui/kxmlguifactory_p.cpp 42b97d4
> kdeui/xmlgui/kxmlguiversionhandler.cpp 3e6227b
> kdeui/xmlgui/kxmlguiversionhandler_p.h
> kdeui/xmlgui/kxmlguiwindow.h 218cd77
> kdeui/xmlgui/kxmlguiwindow.cpp a74eece
> staging/xmlgui/CMakeLists.txt 6320a19
> staging/xmlgui/autotests/CMakeLists.txt 83c2b11
> staging/xmlgui/src/CMakeLists.txt a506ab0
> staging/xmlgui/src/config-xmlgui.h.cmake 58949d8
> staging/xmlgui/tests/CMakeLists.txt dce637f
> staging/xmlgui/tests/kxmlguitest.cpp PRE-CREATION
>
> Diff: http://git.reviewboard.kde.org/r/111210/diff/
>
>
> Testing
> -------
>
> Builds.
> Several {auto,}tests segfaults.
>
>
> Thanks,
>
> Andrea Scarpino
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20130703/51407b20/attachment-0001.html>
More information about the Kde-frameworks-devel
mailing list