Re: Review Request 115097: KParts: Move each class into its own header/source file pair
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115097/#review47626 --- Ship it! Good work, thanks. I agree with the small SC break for the sake of consistency. - David Faure On Jan. 18, 2014, 3:47 a.m., Friedrich W. H. Kossebau wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115097/ --- (Updated Jan. 18, 2014, 3:47 a.m.) Review request for KDE Frameworks and David Faure. Repository: kparts Description --- As discussed in http://lists.kde.org/?l=kde-frameworks-develm=138994749530457w=2 this patch ensures that all (exported) classes are declared in header files which match the class name. And while includes had to be touched the patch also does some fixup/cleanup of includes, so the headers only include what is needed and have kparts/part.h in the installed headers and use part.h in the sources. These headers have been split out as new ones: From browserextension.h: * browserarguments.h * windowargs.h * openurlevent.h * browserhostextension.h * liveconnectextension.h From event.h: * guiactivateevent.h * partactivateevent.h * partselectevent.h From part.h: * openurlarguments.h * partbase.h * readonlypart.h * readwritepart.h From htmlextension.h: * selectorinterface.h * htmlsettingsinterface.h listingextension.h has been split up even, as there is no class ListingExtension, into: * listingfilterextension.h * listingnotificationextension.h Matching adaption for KDE4Support: https://git.reviewboard.kde.org/r/115098/ There is one issue: KDE4 code relies on getting all the now moved-out classes when including browserextension.h, event.h, part.h, htmlextension.h. While for CamelCase-forwarding headers, like KParts/Part or KParts/Event in KDE4Support this can be catched by just including all the now needed headers, I found no way yet to also have proper backward support for normal includes like kparts/part.h. There is lots of code which includes that to use KParts::ReadOnlyPart. Because kparts/part.h is now also the correct path for the now-only KParts::Part declaring header from the KParts modul, I could not simply install a kde4-compat part.h from KDE4Support into KDE4Support/kparts/ and then forward include the current headers, because it will fail at least with #include kparts/part.h Would it be acceptable to simply break SC for old code including the lowercase headers (like kparts/part.h) and using classes declared in there not matching the filename (like KParts::ReadOnlyPart)? Or is there a smart workaround for the problem? One workaround I proposed in the email: The real headers would just have the fallback support directly. E.g the new part.h would have // KDE4 compat #if KDE4COMPATIBLE #include kparts/guiactivateevent.h #include kparts/partactivateevent.h #include kparts/partselectevent.h #endif to include the other headers at the places where their classes had been defined before. This would mean having still KDE4 mentioned in KF5 code :) But KDE4COMPATIBLE (or a better name) should be only defined when KDE4Support is linked as well. No strong opinion myself. I would opt for the small SC-break :) Looking at all the stuff build with kdesrc-build/kf5-qt5-build-include there are small adaptions needed for * khtml * kross * kmediaplayer * kdewebkit * ktexteditor * kprintutils * okteta Mainly stuff like -#include kparts/part.h +#include kparts/readonlypart.h Locally all fixed, would commit the adaptions directly to those modules. Diffs - src/windowargs.cpp PRE-CREATION tests/normalktm.h f3bc291 tests/notepad.h 73a6fa2 tests/parts.h 1207c2c tests/parts.cpp 872bdb8 tests/partviewer.h bfe3158 tests/terminal_test.cpp eda318a tests/testmainwindow.h 7ba44e0 src/browserinterface.cpp a008e84 src/browseropenorsavequestion.h cb8d3ed src/browseropenorsavequestion.cpp dd52608 src/browserrun.h 5a0b996 src/browserrun.cpp bcf50df src/browserrun_p.h fbfbea6 src/event.h 056d735 src/event.cpp 1d88c82 src/fileinfoextension.h e1efbc1 src/fileinfoextension.cpp 8ecb234 src/guiactivateevent.h PRE-CREATION src/guiactivateevent.cpp PRE-CREATION src/historyprovider.h f167f30 src/historyprovider.cpp 1e4733a src/htmlextension.h 66966e4 src/htmlextension.cpp 3a6df16 src/htmlsettingsinterface.h PRE-CREATION src/htmlsettingsinterface.cpp PRE-CREATION src/kde_terminal_interface.h d029e02 src/listingextension.h 0b2e1dd src/listingextension.cpp d380584 src/listingfilterextension.h PRE-CREATION src/listingfilterextension.cpp
Re: Review Request 115098: Adapt KParts compat-forwarding headers to new one-header-per-class policy in KParts
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115098/#review47627 --- Why does this remove some forwarding headers? - David Faure On Jan. 18, 2014, 3:48 a.m., Friedrich W. H. Kossebau wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115098/ --- (Updated Jan. 18, 2014, 3:48 a.m.) Review request for KDE Frameworks and David Faure. Repository: kde4support Description --- Counterpart to: https://git.reviewboard.kde.org/r/115097/ Main discussion over there, please. Diffs - src/includes/KParts/OpenUrlEvent c3e9e59 src/includes/KParts/Part bcb6c5e src/includes/KParts/PartActivateEvent dabdd2f src/includes/KParts/PartBase bcb6c5e src/includes/KParts/PartManager 2dcfeb3 src/includes/KParts/PartSelectEvent dabdd2f src/includes/KParts/Plugin f73168d src/includes/KParts/ReadOnlyPart bcb6c5e src/includes/KParts/ReadWritePart bcb6c5e src/includes/KParts/StatusBarExtension 8c8a481 src/includes/KParts/TextExtension cb73ab5 src/includes/KParts/WindowArgs c3e9e59 src/kparts/listingextension.h PRE-CREATION src/CMakeLists.txt 23b0d6d src/includes/CMakeLists.txt 2b2130f src/includes/KParts/BrowserExtension c3e9e59 src/includes/KParts/BrowserHostExtension c3e9e59 src/includes/KParts/BrowserInterface 640f47b src/includes/KParts/BrowserRun b63479b src/includes/KParts/Event dabdd2f src/includes/KParts/FileInfoExtension 13c7c41 src/includes/KParts/GUIActivateEvent dabdd2f src/includes/KParts/HistoryProvider b8c3fc9 src/includes/KParts/HtmlExtension aae280e src/includes/KParts/ListingExtension 38598f9 src/includes/KParts/LiveConnectExtension c3e9e59 src/includes/KParts/MainWindow 452e115 Diff: https://git.reviewboard.kde.org/r/115098/diff/ Testing --- Thanks, Friedrich W. H. Kossebau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115024: Remove check for X11
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115024/#review47628 --- Ship it! Windows says Ctrl Alt too, so this seems correct to me. - David Faure On Jan. 15, 2014, 12:21 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115024/ --- (Updated Jan. 15, 2014, 12:21 p.m.) Review request for KDE Frameworks and Andreas Hartmetz. Repository: kxmlgui Description --- Remove check for X11 The only thing that was using it was a preprocessor branch in kkeysequencewidget.cpp, which only had branches for Mac and X11. It appears to be intended to control the order of modifiers in a key sequence description, but there is no explanation anywhere in the logs for the fact that it checks for X11. (Added Andreas as the original author of this code back in the transition to KDE 4). Update: actually, it looks like this originally used KKeyServer to get the modifier descriptions, which was only implemented for X11 and Mac. So that explains that... Diffs - CMakeLists.txt 11a5af110a101a18e4b5a36f1d7e91a34c1b09c5 src/CMakeLists.txt 29e7dfe4aa89c03778bf4137840727fb54c1332b src/config-xmlgui.h.cmake bde7885db30843a0cb241d1ee0ac9e22c762d7b3 src/kkeysequencewidget.cpp 65ff05eec6b99cdcf7db5505475f11918b76a767 Diff: https://git.reviewboard.kde.org/r/115024/diff/ Testing --- Configure, build, run tests, install. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115024: Remove check for X11
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115024/ --- (Updated Jan. 18, 2014, 9:40 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Andreas Hartmetz. Repository: kxmlgui Description --- Remove check for X11 The only thing that was using it was a preprocessor branch in kkeysequencewidget.cpp, which only had branches for Mac and X11. It appears to be intended to control the order of modifiers in a key sequence description, but there is no explanation anywhere in the logs for the fact that it checks for X11. (Added Andreas as the original author of this code back in the transition to KDE 4). Update: actually, it looks like this originally used KKeyServer to get the modifier descriptions, which was only implemented for X11 and Mac. So that explains that... Diffs - CMakeLists.txt 11a5af110a101a18e4b5a36f1d7e91a34c1b09c5 src/CMakeLists.txt 29e7dfe4aa89c03778bf4137840727fb54c1332b src/config-xmlgui.h.cmake bde7885db30843a0cb241d1ee0ac9e22c762d7b3 src/kkeysequencewidget.cpp 65ff05eec6b99cdcf7db5505475f11918b76a767 Diff: https://git.reviewboard.kde.org/r/115024/diff/ Testing --- Configure, build, run tests, install. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 115099: Add function ecm_generate_pri_file() to provide qmake support. Make ECMSetupVersion set PROJECT_VERSION_*
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115099/
---
Review request for Build System, Extra Cmake Modules and KDE Frameworks.
Repository: extra-cmake-modules
Description
---
Two commits:
1) Make ECMSetupVersion set PROJECT_VERSION_*
This makes it easier for other functions to access the project version,
for instance my upcoming ECM_GENERATE_PRI_FILE()
2) This file provides the function ecm_generate_pri_file().
ECM_GENERATE_PRI_FILE() creates a .pri file for a library so that qmake-based
apps can more easily use the library.
It also sets ECM_MKSPECS_INSTALL_DIR as the directory to install the .pri file
to.
Diffs
-
modules/ECMGeneratePriFile.cmake PRE-CREATION
modules/ECMSetupVersion.cmake 6c3a9959be31ee186cf173bb28585dfc52860a55
Diff: https://git.reviewboard.kde.org/r/115099/diff/
Testing
---
Adding these lines to kwidgetaddons/src/CMakeLists.txt:
include(ECMGeneratePriFile)
ecm_generate_pri_file(BASE_NAME KWidgetsAddons LIB_NAME KF5WidgetsAddons DEPS
widgets FILENAME_VAR PRI_FILENAME)
install(FILES ${PRI_FILENAME} DESTINATION ${ECM_MKSPECS_INSTALL_DIR})
And these to kjobwidgets:
include(ECMGeneratePriFile)
ecm_generate_pri_file(BASE_NAME KJobWidgets LIB_NAME KF5JobWidgets DEPS
KCoreAddons widgets FILENAME_VAR PRI_FILENAME)
install(FILES ${PRI_FILENAME} DESTINATION ${ECM_MKSPECS_INSTALL_DIR})
And I added a qmake_test subdir in kf5umbrella with qmake_test.pro saying:
QT += KArchive KJobWidgets KWidgetsAddons
SOURCES += main.cpp
- links to all the mentionned libs, including KCoreAddons (via KJobWidgets).
This requires $QMAKEPATH set to the kf5 install prefix.
Thanks,
David Faure
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115077: Rename Macros in KF5DocTools to KDE5_
On Jan. 17, 2014, 6:51 p.m., Alex Merry wrote: KF5DocToolsMacros.cmake, lines 166-172 https://git.reviewboard.kde.org/r/115077/diff/1/?file=234284#file234284line166 These should *not* be renamed, as they are compatibility macros. However, they should probably be moved to kde4support. I wouldn't rename it indeed. Moving it to KDE4Support can be good indeed, although it kind of defeats the purpose, as you'll need to actually depend on KDE4Support to get the warning. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115077/#review47592 --- On Jan. 17, 2014, 3:14 p.m., David Narváez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115077/ --- (Updated Jan. 17, 2014, 3:14 p.m.) Review request for Documentation and KDE Frameworks. Repository: kdoctools Description --- Part of the overall task of removing mentions of KDE4 from the code. Diffs - KF5DocToolsMacros.cmake 191a2c5 Diff: https://git.reviewboard.kde.org/r/115077/diff/ Testing --- Thanks, David Narváez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115077: Rename Macros in KF5DocTools to KDE5_
On Jan. 17, 2014, 6:51 p.m., Alex Merry wrote: KF5DocToolsMacros.cmake, lines 166-172 https://git.reviewboard.kde.org/r/115077/diff/1/?file=234284#file234284line166 These should *not* be renamed, as they are compatibility macros. However, they should probably be moved to kde4support. Aleix Pol Gonzalez wrote: I wouldn't rename it indeed. Moving it to KDE4Support can be good indeed, although it kind of defeats the purpose, as you'll need to actually depend on KDE4Support to get the warning. Luigi, your call. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115077/#review47592 --- On Jan. 17, 2014, 3:14 p.m., David Narváez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115077/ --- (Updated Jan. 17, 2014, 3:14 p.m.) Review request for Documentation and KDE Frameworks. Repository: kdoctools Description --- Part of the overall task of removing mentions of KDE4 from the code. Diffs - KF5DocToolsMacros.cmake 191a2c5 Diff: https://git.reviewboard.kde.org/r/115077/diff/ Testing --- Thanks, David Narváez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115077: Rename Macros in KF5DocTools to KDE5_
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115077/ --- (Updated Jan. 18, 2014, 1:16 p.m.) Review request for Documentation, KDE Frameworks and Luigi Toscano. Changes --- Adding maintainer explicitly Repository: kdoctools Description --- Part of the overall task of removing mentions of KDE4 from the code. Diffs - KF5DocToolsMacros.cmake 191a2c5 Diff: https://git.reviewboard.kde.org/r/115077/diff/ Testing --- Thanks, David Narváez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115077: Rename Macros in KF5DocTools to KDE5_
On Jan. 17, 2014, 6:51 p.m., Alex Merry wrote: KF5DocToolsMacros.cmake, lines 166-172 https://git.reviewboard.kde.org/r/115077/diff/1/?file=234284#file234284line166 These should *not* be renamed, as they are compatibility macros. However, they should probably be moved to kde4support. Aleix Pol Gonzalez wrote: I wouldn't rename it indeed. Moving it to KDE4Support can be good indeed, although it kind of defeats the purpose, as you'll need to actually depend on KDE4Support to get the warning. David Narváez wrote: Luigi, your call. Well, most projects will start off depending on KDE4Support anyway, and if you don't use it I think you should just get an error about the macro not being found. - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115077/#review47592 --- On Jan. 18, 2014, 1:16 p.m., David Narváez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115077/ --- (Updated Jan. 18, 2014, 1:16 p.m.) Review request for Documentation, KDE Frameworks and Luigi Toscano. Repository: kdoctools Description --- Part of the overall task of removing mentions of KDE4 from the code. Diffs - KF5DocToolsMacros.cmake 191a2c5 Diff: https://git.reviewboard.kde.org/r/115077/diff/ Testing --- Thanks, David Narváez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115099: Add function ecm_generate_pri_file() to provide qmake support. Make ECMSetupVersion set PROJECT_VERSION_*
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115099/#review47642
---
modules/ECMGeneratePriFile.cmake
https://git.reviewboard.kde.org/r/115099/#comment33808
What if this was TARGET target and you extracted the interface defines
etc. from that target?
That, at least, was the idea I had for generating pkg-config files.
- Alex Merry
On Jan. 18, 2014, 11:02 a.m., David Faure wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115099/
---
(Updated Jan. 18, 2014, 11:02 a.m.)
Review request for Build System, Extra Cmake Modules and KDE Frameworks.
Repository: extra-cmake-modules
Description
---
Two commits:
1) Make ECMSetupVersion set PROJECT_VERSION_*
This makes it easier for other functions to access the project version,
for instance my upcoming ECM_GENERATE_PRI_FILE()
2) This file provides the function ecm_generate_pri_file().
ECM_GENERATE_PRI_FILE() creates a .pri file for a library so that qmake-based
apps can more easily use the library.
It also sets ECM_MKSPECS_INSTALL_DIR as the directory to install the .pri
file to.
Diffs
-
modules/ECMGeneratePriFile.cmake PRE-CREATION
modules/ECMSetupVersion.cmake 6c3a9959be31ee186cf173bb28585dfc52860a55
Diff: https://git.reviewboard.kde.org/r/115099/diff/
Testing
---
Adding these lines to kwidgetaddons/src/CMakeLists.txt:
include(ECMGeneratePriFile)
ecm_generate_pri_file(BASE_NAME KWidgetsAddons LIB_NAME KF5WidgetsAddons DEPS
widgets FILENAME_VAR PRI_FILENAME)
install(FILES ${PRI_FILENAME} DESTINATION ${ECM_MKSPECS_INSTALL_DIR})
And these to kjobwidgets:
include(ECMGeneratePriFile)
ecm_generate_pri_file(BASE_NAME KJobWidgets LIB_NAME KF5JobWidgets DEPS
KCoreAddons widgets FILENAME_VAR PRI_FILENAME)
install(FILES ${PRI_FILENAME} DESTINATION ${ECM_MKSPECS_INSTALL_DIR})
And I added a qmake_test subdir in kf5umbrella with qmake_test.pro saying:
QT += KArchive KJobWidgets KWidgetsAddons
SOURCES += main.cpp
- links to all the mentionned libs, including KCoreAddons (via KJobWidgets).
This requires $QMAKEPATH set to the kf5 install prefix.
Thanks,
David Faure
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115098: Adapt KParts compat-forwarding headers to new one-header-per-class policy in KParts
On Jan. 18, 2014, 8:58 a.m., David Faure wrote: Why does this remove some forwarding headers? Because they are installed from the KParts module itself already, with the same forwarding include path. So no need to duplicate them in KDE4Support, or? (Only difference is those from KParts have '' around the path, not '' and ''. This is a small error in ecm_generate_headers still, it needs to add ../ in front of the path in case of a prefixed path. Other option would be '' as well '', but a relative link seems safer to me) - Friedrich W. H. --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115098/#review47627 --- On Jan. 18, 2014, 3:48 a.m., Friedrich W. H. Kossebau wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115098/ --- (Updated Jan. 18, 2014, 3:48 a.m.) Review request for KDE Frameworks and David Faure. Repository: kde4support Description --- Counterpart to: https://git.reviewboard.kde.org/r/115097/ Main discussion over there, please. Diffs - src/includes/KParts/OpenUrlEvent c3e9e59 src/includes/KParts/Part bcb6c5e src/includes/KParts/PartActivateEvent dabdd2f src/includes/KParts/PartBase bcb6c5e src/includes/KParts/PartManager 2dcfeb3 src/includes/KParts/PartSelectEvent dabdd2f src/includes/KParts/Plugin f73168d src/includes/KParts/ReadOnlyPart bcb6c5e src/includes/KParts/ReadWritePart bcb6c5e src/includes/KParts/StatusBarExtension 8c8a481 src/includes/KParts/TextExtension cb73ab5 src/includes/KParts/WindowArgs c3e9e59 src/kparts/listingextension.h PRE-CREATION src/CMakeLists.txt 23b0d6d src/includes/CMakeLists.txt 2b2130f src/includes/KParts/BrowserExtension c3e9e59 src/includes/KParts/BrowserHostExtension c3e9e59 src/includes/KParts/BrowserInterface 640f47b src/includes/KParts/BrowserRun b63479b src/includes/KParts/Event dabdd2f src/includes/KParts/FileInfoExtension 13c7c41 src/includes/KParts/GUIActivateEvent dabdd2f src/includes/KParts/HistoryProvider b8c3fc9 src/includes/KParts/HtmlExtension aae280e src/includes/KParts/ListingExtension 38598f9 src/includes/KParts/LiveConnectExtension c3e9e59 src/includes/KParts/MainWindow 452e115 Diff: https://git.reviewboard.kde.org/r/115098/diff/ Testing --- Thanks, Friedrich W. H. Kossebau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 115100: Rename kmailservice binary to kmailservice5
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115100/ --- Review request for KDE Frameworks and David Faure. Repository: kio Description --- As was discussed on release-team/kde-packager lists, this patch renames the binary to kmailservice5, for allowing kio to co-exist in the same prefix as kdelibs4. There is also ktelnetservice, i'll create a patch for it today possibly. Diffs - autotests/krununittest.cpp 95a8b6c src/ioslaves/mailto/CMakeLists.txt 433eaa7 src/ioslaves/mailto/kmailservice.desktop 76139e9 Diff: https://git.reviewboard.kde.org/r/115100/diff/ Testing --- Builds + autotests build. Thanks, Hrvoje Senjan ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 115101: add signal KMessageWidget::hideAnimationFinished()
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115101/ --- Review request for KDE Frameworks, Albert Astals Cid and Aurélien Gâteau. Repository: kwidgetsaddons Description --- This patch adds the signal KMessageWidget::hideAnimationFinished(), indicating that the hide animation is finished. This is required by Kate Part's passive notification system, since the next pending message is shown only after the hide animation is finished. Right now, Kate Part launches a timer of 500ms (these 500 ms are used by kmessagewidget's animatedHide()) to estimate when the animation is finished. This signal cannot be replaced by e.g. deriving KMessageWidget, since QHideEvent is also sent in case of the parent widget is hidden. Diffs - src/kmessagewidget.h a17bccf src/kmessagewidget.cpp deb82a3 Diff: https://git.reviewboard.kde.org/r/115101/diff/ Testing --- In Kate Part, not commited so far, though. Thanks, Dominik Haumann ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115101: add signal KMessageWidget::hideAnimationFinished()
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115101/ --- (Updated Jan. 18, 2014, 5:47 p.m.) Review request for KDE Frameworks, Albert Astals Cid and Aurélien Gâteau. Changes --- add showAnimationFinished(); Repository: kwidgetsaddons Description (updated) --- This patch adds the signal KMessageWidget::hideAnimationFinished(), indicating that the hide animation is finished. This is required by Kate Part's passive notification system, since the next pending message is shown only after the hide animation is finished. Right now, Kate Part launches a timer of 500ms (these 500 ms are used by kmessagewidget's animatedHide()) to estimate when the animation is finished. This signal cannot be replaced by e.g. deriving KMessageWidget, since QHideEvent is also sent in case of the parent widget is hidden. The same holds for animatedShow(): Emit signal showAnimationFinished() when the show animation is finished. Diffs (updated) - src/kmessagewidget.h a17bccf src/kmessagewidget.cpp deb82a3 Diff: https://git.reviewboard.kde.org/r/115101/diff/ Testing --- In Kate Part, not commited so far, though. Thanks, Dominik Haumann ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115100: Rename kmailservice/ktelnetservice binaries to kmailservice5/ktelnetservice5
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115100/ --- (Updated Jan. 18, 2014, 5:48 p.m.) Review request for KDE Frameworks and David Faure. Changes --- Added ktelnetservice part Summary (updated) - Rename kmailservice/ktelnetservice binaries to kmailservice5/ktelnetservice5 Repository: kio Description --- As was discussed on release-team/kde-packager lists, this patch renames the binary to kmailservice5, for allowing kio to co-exist in the same prefix as kdelibs4. There is also ktelnetservice, i'll create a patch for it today possibly. Diffs (updated) - autotests/krununittest.cpp 95a8b6c src/ioslaves/mailto/CMakeLists.txt 433eaa7 src/ioslaves/mailto/kmailservice.desktop 76139e9 Diff: https://git.reviewboard.kde.org/r/115100/diff/ Testing --- Builds + autotests build. Thanks, Hrvoje Senjan ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115100: Rename kmailservice/ktelnetservice binaries to kmailservice5/ktelnetservice5
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115100/ --- (Updated Jan. 18, 2014, 5:49 p.m.) Review request for KDE Frameworks and David Faure. Repository: kio Description --- As was discussed on release-team/kde-packager lists, this patch renames the binary to kmailservice5, for allowing kio to co-exist in the same prefix as kdelibs4. There is also ktelnetservice, i'll create a patch for it today possibly. Diffs (updated) - autotests/krununittest.cpp 95a8b6c src/ioslaves/mailto/CMakeLists.txt 433eaa7 src/ioslaves/mailto/kmailservice.cpp 5fc8688 src/ioslaves/mailto/kmailservice.desktop 76139e9 src/ioslaves/telnet/CMakeLists.txt 24fa8cc src/ioslaves/telnet/ktelnetservice.cpp 6bd35fc src/ioslaves/telnet/ktelnetservice.desktop 23e84fe Diff: https://git.reviewboard.kde.org/r/115100/diff/ Testing --- Builds + autotests build. Thanks, Hrvoje Senjan ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115101: add signal KMessageWidget::hideAnimationFinished()
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115101/ --- (Updated Jan. 18, 2014, 6:13 p.m.) Review request for KDE Frameworks, Albert Astals Cid and Aurélien Gâteau. Changes --- Another update, adding two bool functions for checking whether animations are running. Repository: kwidgetsaddons Description (updated) --- This patch adds the signal KMessageWidget::hideAnimationFinished(), indicating that the hide animation is finished. This is required by Kate Part's passive notification system, since the next pending message is shown only after the hide animation is finished. Right now, Kate Part launches a timer of 500ms (these 500 ms are used by kmessagewidget's animatedHide()) to estimate when the animation is finished. This signal cannot be replaced by e.g. deriving KMessageWidget, since QHideEvent is also sent in case of the parent widget is hidden. The same holds for animatedShow(): Emit signal showAnimationFinished() when the show animation is finished. Besides that, the two convenience functions bool hideAnimationRunning() const; bool showAnimationRunning() const; are also added to check the current state of the animations. This is also required by Kate Part in order to wait until an animation is finished before starting a new one. Diffs (updated) - src/kmessagewidget.h a17bccf src/kmessagewidget.cpp deb82a3 Diff: https://git.reviewboard.kde.org/r/115101/diff/ Testing --- In Kate Part, not commited so far, though. Thanks, Dominik Haumann ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115078: don't install dbus interface files in kglobalaccel
On Jan. 17, 2014, 8:43 p.m., Martin Gräßlin wrote: this would break my workflow given that I install kde4 and kf5 to different prefixes. I think this needs a different approach, but please don't ask me for it. I'm lacking ideas how we could solve this upstream. What about just renaming xml files? Interfaces would be left compatible, and the files would not conflict. Only it would look a bit ugly, and possibly confusing... (To my knowledge, there is no rule/spec that xml must match interface name, but that could be false ;-) - Hrvoje --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115078/#review47614 --- On Jan. 17, 2014, 4:07 p.m., Jonathan Riddell wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115078/ --- (Updated Jan. 17, 2014, 4:07 p.m.) Review request for KDE Frameworks and Martin Gräßlin. Repository: kglobalaccel Description --- don't install dbus interface files in kglobalaccel, these overlap with the equivalent files in kdelibs4 which causes problems for some distributions Diffs - KF5GlobalAccelConfig.cmake.in 5f069d3 src/CMakeLists.txt d48e92e Diff: https://git.reviewboard.kde.org/r/115078/diff/ Testing --- Thanks, Jonathan Riddell ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115077: Rename Macros in KF5DocTools to KDE5_
On Jan. 17, 2014, 6:51 p.m., Alex Merry wrote: KF5DocToolsMacros.cmake, lines 166-172 https://git.reviewboard.kde.org/r/115077/diff/1/?file=234284#file234284line166 These should *not* be renamed, as they are compatibility macros. However, they should probably be moved to kde4support. Aleix Pol Gonzalez wrote: I wouldn't rename it indeed. Moving it to KDE4Support can be good indeed, although it kind of defeats the purpose, as you'll need to actually depend on KDE4Support to get the warning. David Narváez wrote: Luigi, your call. Alex Merry wrote: Well, most projects will start off depending on KDE4Support anyway, and if you don't use it I think you should just get an error about the macro not being found. My thoughts: - is the dependency on KDE4Support the first thing a ported application should do? If yes, then the macros can be kept and moved; but please read on; - KDE4_CREATE_HTML_HANDBOOK can be completely removed. It has been deprecated on Wed Aug 15 03:55:48 2007 +, 82f7b6ba0f8be7d314ac780b9a873e98b6be39b2, and it is not used (apart from the definition in kdelibs and kf5/kdoctools). - KDE4_CREATE_HANDBOOK should not be renamed, and it can be moved to KDE4Support (I see that it was already renamed as KDOCTOOLS_CREATE_HANDBOOK in the frameworks-based code) - I see KDE4_SERIALIZE_TOOL has been renamed as well; but it is defined in KDE4Support: http://lxr.kde.org/source/frameworks/kde4support/cmake/modules/FindKDE4Internal.cmake#455 I'm not sure how to deal with this: probably it would be better to define it here and rename it as KDOCTOOLS_blabla. Does it make sense? - Luigi --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115077/#review47592 --- On Jan. 18, 2014, 1:16 p.m., David Narváez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115077/ --- (Updated Jan. 18, 2014, 1:16 p.m.) Review request for Documentation, KDE Frameworks and Luigi Toscano. Repository: kdoctools Description --- Part of the overall task of removing mentions of KDE4 from the code. Diffs - KF5DocToolsMacros.cmake 191a2c5 Diff: https://git.reviewboard.kde.org/r/115077/diff/ Testing --- Thanks, David Narváez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115101: add signal KMessageWidget::hideAnimationFinished()
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115101/#review47655 --- Hm, how does KMessageWidget behave, if you post the next message without waiting for the animation to finish? I somehow feel that the queuing should be done inside KMessageWidget. - Christoph Feck On Jan. 18, 2014, 6:13 p.m., Dominik Haumann wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115101/ --- (Updated Jan. 18, 2014, 6:13 p.m.) Review request for KDE Frameworks, Albert Astals Cid and Aurélien Gâteau. Repository: kwidgetsaddons Description --- This patch adds the signal KMessageWidget::hideAnimationFinished(), indicating that the hide animation is finished. This is required by Kate Part's passive notification system, since the next pending message is shown only after the hide animation is finished. Right now, Kate Part launches a timer of 500ms (these 500 ms are used by kmessagewidget's animatedHide()) to estimate when the animation is finished. This signal cannot be replaced by e.g. deriving KMessageWidget, since QHideEvent is also sent in case of the parent widget is hidden. The same holds for animatedShow(): Emit signal showAnimationFinished() when the show animation is finished. Besides that, the two convenience functions bool hideAnimationRunning() const; bool showAnimationRunning() const; are also added to check the current state of the animations. This is also required by Kate Part in order to wait until an animation is finished before starting a new one. Diffs - src/kmessagewidget.h a17bccf src/kmessagewidget.cpp deb82a3 Diff: https://git.reviewboard.kde.org/r/115101/diff/ Testing --- In Kate Part, not commited so far, though. Thanks, Dominik Haumann ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115101: add signal KMessageWidget::hideAnimationFinished()
On Jan. 18, 2014, 7:46 p.m., Christoph Feck wrote: Hm, how does KMessageWidget behave, if you post the next message without waiting for the animation to finish? I somehow feel that the queuing should be done inside KMessageWidget. KMessageWidget has no post function at all. It is just a widget that displays text and actions. Queuing is what Kate Part builds around it, and that is exactly why these functions the patch adds are required. - Dominik --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115101/#review47655 --- On Jan. 18, 2014, 6:13 p.m., Dominik Haumann wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115101/ --- (Updated Jan. 18, 2014, 6:13 p.m.) Review request for KDE Frameworks, Albert Astals Cid and Aurélien Gâteau. Repository: kwidgetsaddons Description --- This patch adds the signal KMessageWidget::hideAnimationFinished(), indicating that the hide animation is finished. This is required by Kate Part's passive notification system, since the next pending message is shown only after the hide animation is finished. Right now, Kate Part launches a timer of 500ms (these 500 ms are used by kmessagewidget's animatedHide()) to estimate when the animation is finished. This signal cannot be replaced by e.g. deriving KMessageWidget, since QHideEvent is also sent in case of the parent widget is hidden. The same holds for animatedShow(): Emit signal showAnimationFinished() when the show animation is finished. Besides that, the two convenience functions bool hideAnimationRunning() const; bool showAnimationRunning() const; are also added to check the current state of the animations. This is also required by Kate Part in order to wait until an animation is finished before starting a new one. Diffs - src/kmessagewidget.h a17bccf src/kmessagewidget.cpp deb82a3 Diff: https://git.reviewboard.kde.org/r/115101/diff/ Testing --- In Kate Part, not commited so far, though. Thanks, Dominik Haumann ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115065: kdoctools renames to add 5 namespace to prevent clashes with kdelibs4
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115065/#review47660
---
The patch looks ago, I have two remarks:
- I think that kde5options should be kf5options (as it happened with
kde4-config - kf5-config)
- I'm not sure if the content of those manpages still applies. The special
options described into those files were provided (and still provided through
KDE4Support) by KCmdLineArgs, now replaced by the QCommandLineParser. I don't
see those options to be defined into Frameworks, which means that
{kde|kf}5options should be removed; are some of them provided by Qt directly?
If not, also qt5config should disappear. Can someone from the
kde-frameworks-devel list shed some light on this?
- Luigi Toscano
On Jan. 17, 2014, 4:48 p.m., Jonathan Riddell wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115065/
---
(Updated Jan. 17, 2014, 4:48 p.m.)
Review request for Documentation, KDE Frameworks, Luigi Toscano, and Scarlett
Clark.
Repository: kdoctools
Description
---
Rename man pages and checkXML tool to prevent clashes with kdelibs4
credit should go to scarlett
Diffs
-
CMakeLists.txt 74c7af5
checkXML.in.cmake d7a57c7
checkXML5.in.cmake PRE-CREATION
docs/CMakeLists.txt 7e9612f
docs/checkXML/CMakeLists.txt 7f8226c
docs/checkXML/man-checkXML.1.docbook 2bfb3f3
docs/checkXML5/CMakeLists.txt PRE-CREATION
docs/checkXML5/man-checkXML5.1.docbook PRE-CREATION
docs/kde5options/CMakeLists.txt PRE-CREATION
docs/kde5options/man-kde5options.7.docbook PRE-CREATION
docs/kdeoptions/CMakeLists.txt a91f451
docs/kdeoptions/man-kdeoptions.7.docbook 7e62f41
docs/qt5options/CMakeLists.txt PRE-CREATION
docs/qt5options/man-qt5options.7.docbook PRE-CREATION
docs/qtoptions/CMakeLists.txt f1dbb6c
docs/qtoptions/man-qtoptions.7.docbook a00677a
Diff: https://git.reviewboard.kde.org/r/115065/diff/
Testing
---
Thanks,
Jonathan Riddell
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
