Re: Re: How to port KIntSpinBox::setSuffix
On Tuesday 03 June 2014 17:59:18 David Faure wrote: PS: Martin: meanwhile Laurent committed the class (which I reviewed) to ktextwidgets, you can use that. Laurent CC-ed me on the commit and I already adjusted the code to use it. Thanks a lot for the fast fix. Cheers Martin signature.asc Description: This is a digitally signed message part. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Is plasma-framework not in tier 3?
On Tue, Jun 3, 2014, at 17:04, Marko Käning wrote: I was wondering why I couldn't find plasma-framework in tier 3 on api.kde.org not below frameworks [1] although it is actually shown as part of tier 3 in the dependency graph in [2] ... The name on the landing page are based on the CMake project name: this cause plasma-framework to be listed as Plasma there. Aurélien ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118458: Fix KDirWatch to act more reliably against various backends.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118458/#review59123 --- Very good to see that it brings the FAM backend to be more in line with the others, behavior-wise. Just a few small issues. autotests/kdirwatch_unittest.cpp https://git.reviewboard.kde.org/r/118458/#comment41151 It seems that the second call should pass created(QString) rather than deleted(QString). autotests/kdirwatch_unittest.cpp https://git.reviewboard.kde.org/r/118458/#comment41152 The commit log doesn't mention any changes related to QFileSystemWatcher, only to the FAM backend. Are you sure you wanted to commit this? If it works then of course I'm happy with it (but maybe in a separate commit then) autotests/kdirwatch_unittest.cpp https://git.reviewboard.kde.org/r/118458/#comment41153 This could actually use QFile::link(), would be more readable. src/lib/io/kdirwatch.cpp https://git.reviewboard.kde.org/r/118458/#comment41154 I'm not 100% sure about the meaning of this boolean variable. (we started an event?) Maybe it can be called something more explicit (waitForInitialFAMEvents? or something that makes sense to you ;)) src/lib/io/kdirwatch.cpp https://git.reviewboard.kde.org/r/118458/#comment41155 Technically this could be an event about a completely unrelated file to the one we're waiting to see, couldn't it? src/lib/io/kdirwatch.cpp https://git.reviewboard.kde.org/r/118458/#comment41156 For performance reasons it was good for this check to be as early as possible. But of course the risk is an infinite loop if we wait to see such a file and we never see it. The solution could be to ensure that we never start watching such a file in the first place... Anyhow, not a blocker. src/lib/io/kdirwatch.cpp https://git.reviewboard.kde.org/r/118458/#comment41157 actual - actually ? - David Faure On June 1, 2014, 9:03 p.m., Matthew Dawson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118458/ --- (Updated June 1, 2014, 9:03 p.m.) Review request for KDE Frameworks. Bugs: 329802 https://bugs.kde.org/show_bug.cgi?id=329802 Repository: kcoreaddons Description --- Fix KDirWatch to act more reliably against various backends. - When KDirWatch used a FAM backend, it wouldn't actually wait for the necessary watches to be put in place before returning, allowing for a race where an application may think a watch is in place when it really isn't. This was easily seen using gamin and running the testDeleteAndRecreateFile test. Fix by delaying useFAM until the watch is in place by waiting until the EndExist FAM event is received. This adds ~100-200ms per watch with gamin. - When a file is deleted and recreated, and scanEntry detects it, the dirty signal would be emitted. Fix to emit a deleted + created signal, as expected. - When a deleted signal was requested, other signals were dropped. Due to the above point, this would stop the created signal from being emitted. Now, all signals are emitted, even when a delete is detected. - When watching recreated files, the created signal could get lost as there was a race between when the created signal would be generated and its signal spy would be created. Fix by making sure the spy is created before the signal could be emitted. - Make sure the created signal isn't emitted twice with both FAM and the polling timer. This occurs as FAM doesn't fix up the fact the entry has been created, and the poller thus assumes it needs to notify the world. Fix by having FAM not emit the event in this case. BUG: 329802 Note that due to a bug in the FAM daemon (which doesn't exist in the gamin daemon), the testDeleteAndRecreateDir unit test fails as FAM doesn't detect the folder deletion. This test previously passed due to the race condition outlined in bug 329802, which was fixed in this RR. I've confirmed this is a bug in FAM using a minimal program talking directly to the daemon, and watching the strace output from FAM. FAM doesn't detect when a folder it is watching directly is deleted. It does detect when a folder inside a folder it is watching is deleted, however. I think this should go in as is, as the unit test failing exposes the actual problem, instead of hiding behind a race when it doesn't really work. If running against FAM is important, I can see about tracking down the issue. Diffs - autotests/kdirwatch_unittest.cpp 4e638e089012276dc8b17b9d960732a161cc3be1 src/lib/io/kdirwatch.cpp cc6feec4d2b2598e50c853172bbc295a8c719bc2 src/lib/io/kdirwatch_p.h
Re: Review Request 118474: Move networkstatus kded module to kf5/kded subdirectory
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118474/#review59125 --- Ship it! Ship It! - David Faure On June 2, 2014, 4:20 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118474/ --- (Updated June 2, 2014, 4:20 p.m.) Review request for KDE Frameworks. Repository: kdelibs4support Description --- Move networkstatus kded module to kf5/kded subdirectory This makes sure the installed file is properly versioned. Diffs - src/solid-networkstatus/kded/CMakeLists.txt ca6bef21d4a8450651eaa8326898b6bfecdbb51a src/solid-networkstatus/kded/networkstatus.desktop b792b3cbe1b4dc95e74426ca358ea7005f8c7998 Diff: https://git.reviewboard.kde.org/r/118474/diff/ Testing --- Module is properly loaded when accessing the org.kde.kded5/modules/networkstatus D-Bus interface. 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 118476: Move kded module to kf5/kded subdirectory
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118476/#review59126 --- Ship it! Ship It! - David Faure On June 2, 2014, 4:39 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118476/ --- (Updated June 2, 2014, 4:39 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Move kded module to kf5/kded subdirectory This makes sure the module is properly versioned. Diffs - src/platformstatus/CMakeLists.txt 676a8be30df41b042758d55b58caa22fc6c89311 src/platformstatus/kded_platformstatus.desktop 9976fa3041a6c12fc4b3ce2146fb481181fa65b8 Diff: https://git.reviewboard.kde.org/r/118476/diff/ Testing --- qdbus org.kde.kded5 /kded loadModule kded_platformstatus succeeds 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 118486: Put transcript plugin in kf5 subdir and simplify search logic
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118486/#review59127 --- Ship it! Ship It! - David Faure On June 2, 2014, 10:03 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118486/ --- (Updated June 2, 2014, 10:03 p.m.) Review request for KDE Frameworks and Chusslove Illich. Repository: ki18n Description --- Put transcript plugin in kf5 subdir and simplify search logic Putting the plugin in a versioned subdirectory ensures there will not be any coinstallability issues with KF6 if it is Qt5-based. Using QPluginLoader to find the plugin makes for simpler, more reliable code. Diffs - src/CMakeLists.txt 3099d2d29e43e40ac761694019f06aec41e54147 src/klocalizedstring.cpp 4a4f6f8c1a79d4b101ae3df5e589cb629f4bd596 Diff: https://git.reviewboard.kde.org/r/118486/diff/ Testing --- Tests pass, but not sure how to test the installed location. Chusslove: do you have a standard way to test the loading and operation of the transcript plugin? Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Is plasma-framework not in tier 3?
Hi Aurélien, On 04 Jun 2014, at 09:15 , Aurélien Gâteau agat...@kde.org wrote: The name on the landing page are based on the CMake project name: this cause plasma-framework to be listed as Plasma there. but why does plasma and plasma-framework appear as separate frameworks in the dependency graph [1]? Is that a glitch of the script which creates the dot graphs then? Greets, Marko [1] http://api.kde.org/frameworks-api/frameworks5-apidocs/plasma-framework/html/plasma-framework-dependencies.html ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
(UNINTENDED?) installation outside the configured DATA_INSTALL_DIR (was Issues for Qt5/KF5/KDE/CI system on OSX/MacPorts)
It turns out that there are indeed a few more KF5 framework trying to install their files in “/Library/Application Support”, here’s the full list of kf5 folders found in the install environment: --- ./frameworkintegration/local-inst/opt/kde/install/darwin/mavericks/clang/kf5-qt5/frameworks/frameworkintegration/inst/Library/Application Support/kf5 ./kconfigwidgets/local-inst/opt/kde/install/darwin/mavericks/clang/kf5-qt5/frameworks/kconfigwidgets/inst/Library/Application Support/kf5 ./kdelibs4support/local-inst/opt/kde/install/darwin/mavericks/clang/kf5-qt5/frameworks/kdelibs4support/inst/Library/Application Support/kf5 ./kdesignerplugin/local-inst/opt/kde/install/darwin/mavericks/clang/kf5-qt5/frameworks/kdesignerplugin/inst/Library/Application Support/kf5 ./kdoctools/local-inst/opt/kde/install/darwin/mavericks/clang/kf5-qt5/frameworks/kdoctools/inst/Library/Application Support/kf5 ./khtml/local-inst/opt/kde/install/darwin/mavericks/clang/kf5-qt5/frameworks/khtml/inst/Library/Application Support/kf5 ./kio/local-inst/opt/kde/install/darwin/mavericks/clang/kf5-qt5/frameworks/kio/inst/Library/Application Support/kf5 ./knewstuff/local-inst/opt/kde/install/darwin/mavericks/clang/kf5-qt5/frameworks/knewstuff/inst/Library/Application Support/kf5 ./kxmlgui/local-inst/opt/kde/install/darwin/mavericks/clang/kf5-qt5/frameworks/kxmlgui/inst/Library/Application Support/kf5 --- Turns out thought that these aren’t all yet, since two frameworks (khtml ktexteditor) actually place their stuff outside of the actually configured kf5 subdirectory: --- $ find . -type d -wholename ./*/local-inst/opt/kde/*/Library/Application Support/* | grep -v /kf5/ | egrep -v /kf5$ ./khtml/local-inst/opt/kde/install/darwin/mavericks/clang/kf5-qt5/frameworks/khtml/inst/Library/Application Support/khtml ./ktexteditor/local-inst/opt/kde/install/darwin/mavericks/clang/kf5-qt5/frameworks/ktexteditor/inst/Library/Application Support/katepart ./ktexteditor/local-inst/opt/kde/install/darwin/mavericks/clang/kf5-qt5/frameworks/ktexteditor/inst/Library/Application Support/katepart5 ./ktexteditor/local-inst/opt/kde/install/darwin/mavericks/clang/kf5-qt5/frameworks/ktexteditor/inst/Library/Application Support/katepart5/script ./ktexteditor/local-inst/opt/kde/install/darwin/mavericks/clang/kf5-qt5/frameworks/ktexteditor/inst/Library/Application Support/katepart5/script/commands ./ktexteditor/local-inst/opt/kde/install/darwin/mavericks/clang/kf5-qt5/frameworks/ktexteditor/inst/Library/Application Support/katepart5/script/files ./ktexteditor/local-inst/opt/kde/install/darwin/mavericks/clang/kf5-qt5/frameworks/ktexteditor/inst/Library/Application Support/katepart5/script/files/quickcoding ./ktexteditor/local-inst/opt/kde/install/darwin/mavericks/clang/kf5-qt5/frameworks/ktexteditor/inst/Library/Application Support/katepart5/script/files/quickcoding/cpp ./ktexteditor/local-inst/opt/kde/install/darwin/mavericks/clang/kf5-qt5/frameworks/ktexteditor/inst/Library/Application Support/katepart5/script/indentation ./ktexteditor/local-inst/opt/kde/install/darwin/mavericks/clang/kf5-qt5/frameworks/ktexteditor/inst/Library/Application Support/katepart5/script/libraries ./ktexteditor/local-inst/opt/kde/install/darwin/mavericks/clang/kf5-qt5/frameworks/ktexteditor/inst/Library/Application Support/katepart5/script/libraries/emmet ./ktexteditor/local-inst/opt/kde/install/darwin/mavericks/clang/kf5-qt5/frameworks/ktexteditor/inst/Library/Application Support/katepart5/syntax --- which is probably unintended! Can this be considered a glitch of their CMake files? Greets, Marko ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Is plasma-framework not in tier 3?
On Wed, Jun 4, 2014, at 1:11, Marko Käning wrote: Hi Aurélien, On 04 Jun 2014, at 09:15 , Aurélien Gâteau agat...@kde.org wrote: The name on the landing page are based on the CMake project name: this cause plasma-framework to be listed as Plasma there. but why does plasma and plasma-framework appear as separate frameworks in the dependency graph [1]? Is that a glitch of the script which creates the dot graphs then? That's definitely a bug. Going to investigate. Thanks for pointing it out. Aurélien ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118269: Fix crash when showing the confirmation dialog for trash/delete operations
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118269/ --- (Updated June 4, 2014, 8:58 a.m.) Status -- This change has been discarded. Review request for KDE Frameworks and David Faure. Bugs: 334648 https://bugs.kde.org/show_bug.cgi?id=334648 Repository: kjobwidgets Description --- Currently Dolphin (and probably anything else that can delete files using KIO) crashes when it's supposed to show the confirmation dialog. The problem is that KonqOperations::askDeleteConfirmation() sets up a KIO::JobUiDelegate object which has no associated job, and calls its setWindow(QWidget*) method before calling the actual askDeleteConfirmation() function. KIO::JobUiDelegate::setWindow(QWidget*) then calls the setWindow function of the base class, KDialogJobUiDelegate::setWindow(QWidget*), which then crashes because it contains the line Q_ASSERT(job()) and there is no job. The problem does not exist in KDE SC 4.x - the code went through a big refactoring in https://git.reviewboard.kde.org/r/111081/ which added the assert. Just removing the assert won't help because the function then calls KJobWidgets::setWindow(KJob *job, QWidget *widget) which dereferences the job, i.e., we get a segfault instead. This patch removes the assert and wraps the function in an if (job()) block instead. I'm not entirely sure if that is the correct solution though - any feedback is welcome. Alternatively, one could move the if-check to the child class, i.e., to KIO::JobUiDelegate::setWindow(QWidget*), if this is the only valid use of a KDialogJobUiDelegate without a job. Or maybe it does not make much sense at all to have the askDeleteConfirmation function, which is probably always called before any job is set up, in a KDialogJobUiDelegate subclass? Changing that would probably require more intrusive changes though. Diffs - src/kdialogjobuidelegate.cpp fb4c99a Diff: https://git.reviewboard.kde.org/r/118269/diff/ Testing --- Fixes the crash for me, and the confirmation dialog works as expected (i.e., the user can choose if the file should really be deleted/trashed or not). Thanks, Frank Reininghaus ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118269: Fix crash when showing the confirmation dialog for trash/delete operations
On June 1, 2014, 3:01 p.m., David Faure wrote: That makes the setWindow call (as called by konq_operations.cpp) useless, though. This is related to the line in jobuidelegate.cpp which says QWidget *widget = job() ? window() : NULL; // ### job is NULL here, most of the time, right? Clearly we need a better way to pass the QWidget* parent to the askDeleteConfirmation() method, it's not working on either end (because no job). But the API (JobUiDelegateExtension) is in KIOCore, so no QWindow nor QWidget there. I think we need to use a member variable in KDialogJobUiDelegate in addition to setting it in KJobWidgets. David Faure wrote: http://www.davidfaure.fr/2014/kjobwidgets.diff http://www.davidfaure.fr/2014/kio_jobuidelegate.diff Seems to work, I get a widget pointer when confirmation dialog pops up. Thanks David! - Frank --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118269/#review58902 --- On May 22, 2014, 9:39 p.m., Frank Reininghaus wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118269/ --- (Updated May 22, 2014, 9:39 p.m.) Review request for KDE Frameworks and David Faure. Bugs: 334648 https://bugs.kde.org/show_bug.cgi?id=334648 Repository: kjobwidgets Description --- Currently Dolphin (and probably anything else that can delete files using KIO) crashes when it's supposed to show the confirmation dialog. The problem is that KonqOperations::askDeleteConfirmation() sets up a KIO::JobUiDelegate object which has no associated job, and calls its setWindow(QWidget*) method before calling the actual askDeleteConfirmation() function. KIO::JobUiDelegate::setWindow(QWidget*) then calls the setWindow function of the base class, KDialogJobUiDelegate::setWindow(QWidget*), which then crashes because it contains the line Q_ASSERT(job()) and there is no job. The problem does not exist in KDE SC 4.x - the code went through a big refactoring in https://git.reviewboard.kde.org/r/111081/ which added the assert. Just removing the assert won't help because the function then calls KJobWidgets::setWindow(KJob *job, QWidget *widget) which dereferences the job, i.e., we get a segfault instead. This patch removes the assert and wraps the function in an if (job()) block instead. I'm not entirely sure if that is the correct solution though - any feedback is welcome. Alternatively, one could move the if-check to the child class, i.e., to KIO::JobUiDelegate::setWindow(QWidget*), if this is the only valid use of a KDialogJobUiDelegate without a job. Or maybe it does not make much sense at all to have the askDeleteConfirmation function, which is probably always called before any job is set up, in a KDialogJobUiDelegate subclass? Changing that would probably require more intrusive changes though. Diffs - src/kdialogjobuidelegate.cpp fb4c99a Diff: https://git.reviewboard.kde.org/r/118269/diff/ Testing --- Fixes the crash for me, and the confirmation dialog works as expected (i.e., the user can choose if the file should really be deleted/trashed or not). Thanks, Frank Reininghaus ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118474: Move networkstatus kded module to kf5/kded subdirectory
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118474/#review59136 --- This review has been submitted with commit eb51d7f228d8b4b611ca36df35a8618afd5a290c by Alex Merry to branch master. - Commit Hook On June 2, 2014, 4:20 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118474/ --- (Updated June 2, 2014, 4:20 p.m.) Review request for KDE Frameworks. Repository: kdelibs4support Description --- Move networkstatus kded module to kf5/kded subdirectory This makes sure the installed file is properly versioned. Diffs - src/solid-networkstatus/kded/CMakeLists.txt ca6bef21d4a8450651eaa8326898b6bfecdbb51a src/solid-networkstatus/kded/networkstatus.desktop b792b3cbe1b4dc95e74426ca358ea7005f8c7998 Diff: https://git.reviewboard.kde.org/r/118474/diff/ Testing --- Module is properly loaded when accessing the org.kde.kded5/modules/networkstatus D-Bus interface. 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 118474: Move networkstatus kded module to kf5/kded subdirectory
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118474/ --- (Updated June 4, 2014, 9:15 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kdelibs4support Description --- Move networkstatus kded module to kf5/kded subdirectory This makes sure the installed file is properly versioned. Diffs - src/solid-networkstatus/kded/CMakeLists.txt ca6bef21d4a8450651eaa8326898b6bfecdbb51a src/solid-networkstatus/kded/networkstatus.desktop b792b3cbe1b4dc95e74426ca358ea7005f8c7998 Diff: https://git.reviewboard.kde.org/r/118474/diff/ Testing --- Module is properly loaded when accessing the org.kde.kded5/modules/networkstatus D-Bus interface. 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 118476: Move kded module to kf5/kded subdirectory
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118476/#review59137 --- This review has been submitted with commit 1a8032610353a592831863774e6e1abc4b6f5189 by Alex Merry to branch master. - Commit Hook On June 2, 2014, 4:39 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118476/ --- (Updated June 2, 2014, 4:39 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Move kded module to kf5/kded subdirectory This makes sure the module is properly versioned. Diffs - src/platformstatus/CMakeLists.txt 676a8be30df41b042758d55b58caa22fc6c89311 src/platformstatus/kded_platformstatus.desktop 9976fa3041a6c12fc4b3ce2146fb481181fa65b8 Diff: https://git.reviewboard.kde.org/r/118476/diff/ Testing --- qdbus org.kde.kded5 /kded loadModule kded_platformstatus succeeds 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 118476: Move kded module to kf5/kded subdirectory
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118476/ --- (Updated June 4, 2014, 9:16 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Move kded module to kf5/kded subdirectory This makes sure the module is properly versioned. Diffs - src/platformstatus/CMakeLists.txt 676a8be30df41b042758d55b58caa22fc6c89311 src/platformstatus/kded_platformstatus.desktop 9976fa3041a6c12fc4b3ce2146fb481181fa65b8 Diff: https://git.reviewboard.kde.org/r/118476/diff/ Testing --- qdbus org.kde.kded5 /kded loadModule kded_platformstatus succeeds 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 118486: Put transcript plugin in kf5 subdir and simplify search logic
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118486/ --- (Updated June 4, 2014, 9:18 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Chusslove Illich. Repository: ki18n Description --- Put transcript plugin in kf5 subdir and simplify search logic Putting the plugin in a versioned subdirectory ensures there will not be any coinstallability issues with KF6 if it is Qt5-based. Using QPluginLoader to find the plugin makes for simpler, more reliable code. Diffs - src/CMakeLists.txt 3099d2d29e43e40ac761694019f06aec41e54147 src/klocalizedstring.cpp 4a4f6f8c1a79d4b101ae3df5e589cb629f4bd596 Diff: https://git.reviewboard.kde.org/r/118486/diff/ Testing --- Tests pass, but not sure how to test the installed location. Chusslove: do you have a standard way to test the loading and operation of the transcript plugin? 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 118486: Put transcript plugin in kf5 subdir and simplify search logic
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118486/#review59138 --- This review has been submitted with commit 8bb821a959fa71c405a1e94bc73aa20920254122 by Alex Merry to branch master. - Commit Hook On June 2, 2014, 10:03 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118486/ --- (Updated June 2, 2014, 10:03 p.m.) Review request for KDE Frameworks and Chusslove Illich. Repository: ki18n Description --- Put transcript plugin in kf5 subdir and simplify search logic Putting the plugin in a versioned subdirectory ensures there will not be any coinstallability issues with KF6 if it is Qt5-based. Using QPluginLoader to find the plugin makes for simpler, more reliable code. Diffs - src/CMakeLists.txt 3099d2d29e43e40ac761694019f06aec41e54147 src/klocalizedstring.cpp 4a4f6f8c1a79d4b101ae3df5e589cb629f4bd596 Diff: https://git.reviewboard.kde.org/r/118486/diff/ Testing --- Tests pass, but not sure how to test the installed location. Chusslove: do you have a standard way to test the loading and operation of the transcript plugin? 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 118490: Change all occurrences of KDE_NO_DEPRECATED to an appropriate define.
On June 3, 2014, 10:16 a.m., Alex Merry wrote:
src/mdnsd-servicetypebrowser.cpp, line 51
https://git.reviewboard.kde.org/r/118490/diff/1/?file=277690#file277690line51
Interesting... any idea which monday?
Matthew Dawson wrote:
I'm assuming that is any Monday, as that used to be the day to break
SIC/BIC on kdelibs. It was there when I inherited the code. I'll just
remove the comment in an updated patch.
Ah, yes, I recall now.
On June 3, 2014, 10:16 a.m., Alex Merry wrote:
src/servicetypebrowser.h, line 93
https://git.reviewboard.kde.org/r/118490/diff/1/?file=277691#file277691line93
Since when?
Matthew Dawson wrote:
At least since KDELibs 4 time. Interestingly, Git shows the @deprecated
came in 2008, from a commit you made :). Looking back further, it seems to
just have been considered unnecessary and should be removed as of 2007, and
for the 4.4 releases effectively. I'll update the comment if the function is
kept.
Heh, so the sparse message is my fault :-)
On June 3, 2014, 10:16 a.m., Alex Merry wrote:
src/servicetypebrowser.h, lines 97-99
https://git.reviewboard.kde.org/r/118490/diff/1/?file=277691#file277691line97
It looks like the mdnsd backend even just returns false, so this is not
even reliable. With that in mind, I'd be tempted to make it an inline
method that just always returns false.
Matthew Dawson wrote:
Is it still late to do a minor SIC change? It appears the function is
useless, and is uneeded. Its only use seems to be for a structure similar to:
if (!isRunning()) {
startBrowse();
}
which can just be reduced to:
startBrowse();
as startBrowse can be called multiple times.
If it is preferred to keep it for SC purposes, then I'll take your
suggestion and just make it inline and return false.
I'd say at this point (after the last beta), and given the negligible cost of
keeping it, it should stay.
- Alex
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118490/#review59066
---
On June 3, 2014, 6:59 a.m., Matthew Dawson wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118490/
---
(Updated June 3, 2014, 6:59 a.m.)
Review request for KDE Frameworks.
Repository: kdnssd
Description
---
Change all occurrences of KDE_NO_DEPRECATED to an appropriate define.
Inline with the new defines used by Frameworks, remove the usage of the
KDE_NO_DEPRECATED define.
Diffs
-
src/avahi-servicetypebrowser.cpp b3c14f84367d5093a6cee5ef3f684edc201a3f96
src/dummy-servicetypebrowser.cpp 39a4b6b660bb47b68b5721535994487dbe2abbf6
src/mdnsd-servicetypebrowser.cpp d16dd1fa1c692cd20e21f1a7d5471ac7956469b3
src/servicetypebrowser.h 3cd10e36bf089e088e8a48bb0cf73daa4fb859c5
Diff: https://git.reviewboard.kde.org/r/118490/diff/
Testing
---
Code still compiles.
Thanks,
Matthew Dawson
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 118514: Add KColumnResizer to KWidgetsAddons
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118514/ --- Review request for KDE Frameworks and Christoph Feck. Repository: kwidgetsaddons Description --- I have been asked to add ColumnResizer to KWidgetAddons. ColumnResizer is a helper class I created long ago to help maintain uniform widths across layouts. You can learn more about it from the blog post I wrote back then: http://agateau.com/2011/clean-up-your-layouts-with-columnresizer/ . I simplified the API a bit, added some documentation, and here it is. Diffs - src/CMakeLists.txt 27b9084 src/kcolumnresizer.h PRE-CREATION src/kcolumnresizer.cpp PRE-CREATION tests/CMakeLists.txt eccf887 tests/kcolumnresizertest.h PRE-CREATION tests/kcolumnresizertest.cpp PRE-CREATION tests/kcolumnresizertest.ui PRE-CREATION Diff: https://git.reviewboard.kde.org/r/118514/diff/ Testing --- Manual test program behaves as expected. Thanks, Aurélien Gâteau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118489: Change all occurrences of KDE_NO_DEPRECATED to an appropriate define.
On June 3, 2014, 10:12 a.m., Alex Merry wrote: Changing the macro name is a no-brainer. Comments below; a lot of them are about binary compatibility issues, and these depend on how we expect this macro to be used. I believe that Qt5 makes all its deprecated functions header-only so that compiling Qt without deprecated symbols produces a version BC with the version with deprecated symbols, and you can also safely set the macro yourself in application code to hide all Qt's deprecated symbols without messing anything up. As you've currently got it, the only way this macro is useful is to produce a separate, binary-incompatible version of the framework that has no deprecated code or symbols. And maybe that's the only thing it's worth supporting. I also highlighted some of the @deprecated apidox things - they really need to be more helpful, and this seemed to be an all the deprecated stuff review. Matthew Dawson wrote: Regarding the binary compatibility, kdelibs4support does a similar thing, which is why I thought it might be appropriate, but I also published the RR to be sure :) . Keeping with my don't rock the boat policy, I'll take whatever the Frameworks community deems to be appropriate. We should decide on a policy for deprecation in frameworks, and then publish that somewhere (I may have a new volunteer that would even be willing to write it!). Regarding the @deprecated apidox, I'll try to write something useful for them and update the patch later tonight. I won't comment on each one invidivually. Do you happen to have a good example of a @deprecated comment? Yeah, I think that policy thing is a good idea. If you have a volunteer for that, that's great! The most productive approach is probably to draft something and send it to the list for feedback. Example: @deprecated since 5.0, use SomeClass::someMethod() instead On June 3, 2014, 10:12 a.m., Alex Merry wrote: src/core/kcoreconfigskeleton.h, lines 1211-1220 https://git.reviewboard.kde.org/r/118489/diff/1/?file=277673#file277673line1211 This wrapping of the apidox in the deprecated macro has been done inconsistently. Matthew Dawson wrote: Is there a standard for how Frameworks code should wrap the declarations? I'll fix any to be consistent with the wider codebase. I don't think there's a standard, no. However, I think it makes sense to wrap the docs - that way, it should be possible to tell doxygen to assume KCONFIGCORE_NO_DEPRECATED is set, and it won't get confused by dox that aren't attached to a declaration. On June 3, 2014, 10:12 a.m., Alex Merry wrote: src/core/kcoreconfigskeleton.h, lines 1475-1487 https://git.reviewboard.kde.org/r/118489/diff/1/?file=277673#file277673line1475 I think this is a bad idea. Binary compatibility is very important in Frameworks, and this breaks it. And messing with virtuals is not just BIC, it's BIC in a way that can cause weird error messages (rather than just linking failures). Matthew Dawson wrote: As mentioned above, I wasn't sure what the policy should be. I'm happy doing it either way, but I think the wider community should decide. Can I suggest sending a separate mail to the list? People tend to brush over RRs, especially ones someone else has already commented on. - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118489/#review59060 --- On June 3, 2014, 6:34 a.m., Matthew Dawson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118489/ --- (Updated June 3, 2014, 6:34 a.m.) Review request for KDE Frameworks. Repository: kconfig Description --- Change all occurrences of KDE_NO_DEPRECATED to an appropriate define. Inline with the new defines used by Frameworks, remove the KDE_NO_DEPRECATED. Also remove some deprecated virtuals when compiling without deprecated features. Make sure there are no deprecated features left when deprecated features are compiled out. When compiling without deprecated features, make sure not to use them. KEmailSettings was using deprecated entries when the enumerations for the features were compiled out, which would fail to compile. Thus, when compiling without deprecated features don't read extra configuration entries. Remove usages of deprecated virtuals, instead use their replacement. Go through and rename uses of usrWriteConfig to usrSave. The change should invoke no behavioural differences. Note that the kconfig_compiler has changed to the new function as well. Note that removing the deprecated virtuals does cause the
Re: Review Request 118514: Add KColumnResizer to KWidgetsAddons
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118514/ --- (Updated June 4, 2014, 11:52 a.m.) Review request for KDE Frameworks and Christoph Feck. Changes --- Fix small issues pointed out by Laurent. Repository: kwidgetsaddons Description --- I have been asked to add ColumnResizer to KWidgetAddons. ColumnResizer is a helper class I created long ago to help maintain uniform widths across layouts. You can learn more about it from the blog post I wrote back then: http://agateau.com/2011/clean-up-your-layouts-with-columnresizer/ . I simplified the API a bit, added some documentation, and here it is. Diffs (updated) - src/CMakeLists.txt 27b9084 src/kcolumnresizer.h PRE-CREATION src/kcolumnresizer.cpp PRE-CREATION tests/CMakeLists.txt eccf887 tests/kcolumnresizertest.h PRE-CREATION tests/kcolumnresizertest.cpp PRE-CREATION tests/kcolumnresizertest.ui PRE-CREATION Diff: https://git.reviewboard.kde.org/r/118514/diff/ Testing --- Manual test program behaves as expected. Thanks, Aurélien Gâteau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118514: Add KColumnResizer to KWidgetsAddons
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118514/#review59145 --- +++ src/kcolumnresizer.h https://git.reviewboard.kde.org/r/118514/#comment41167 We probably need a remove function. src/kcolumnresizer.cpp https://git.reviewboard.kde.org/r/118514/#comment41168 Do we really want it to be critical? Sometimes widgets are loaded dynamically, we probably don't want to explode because some widgets might not line up. Maybe a qWarning + a Q_ASSERT. - David Edmundson On June 4, 2014, 9:52 a.m., Aurélien Gâteau wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118514/ --- (Updated June 4, 2014, 9:52 a.m.) Review request for KDE Frameworks and Christoph Feck. Repository: kwidgetsaddons Description --- I have been asked to add ColumnResizer to KWidgetAddons. ColumnResizer is a helper class I created long ago to help maintain uniform widths across layouts. You can learn more about it from the blog post I wrote back then: http://agateau.com/2011/clean-up-your-layouts-with-columnresizer/ . I simplified the API a bit, added some documentation, and here it is. Diffs - src/CMakeLists.txt 27b9084 src/kcolumnresizer.h PRE-CREATION src/kcolumnresizer.cpp PRE-CREATION tests/CMakeLists.txt eccf887 tests/kcolumnresizertest.h PRE-CREATION tests/kcolumnresizertest.cpp PRE-CREATION tests/kcolumnresizertest.ui PRE-CREATION Diff: https://git.reviewboard.kde.org/r/118514/diff/ Testing --- Manual test program behaves as expected. Thanks, Aurélien Gâteau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Jenkins build became unstable: ktexteditor_master_qt5 #416
See http://build.kde.org/job/ktexteditor_master_qt5/416/changes ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118514: Add KColumnResizer to KWidgetsAddons
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118514/#review59170
---
What about adding something like this to Qt ?
src/kcolumnresizer.h
https://git.reviewboard.kde.org/r/118514/#comment41197
Reading your blog
http://agateau.com/2011/clean-up-your-layouts-with-columnresizer/
I have the impression that the documentation added here is less helpful
than your blog.
That said, can you add one of the example images? Have a look at
kmessagewidget.h how to do this.
src/kcolumnresizer.h
https://git.reviewboard.kde.org/r/118514/#comment41195
Doxygen comments without colon:
@param layout The layout containing the widgets to add
Maybe add, tat the layout must be a QGridLayout or a QFormLayout.
src/kcolumnresizer.cpp
https://git.reviewboard.kde.org/r/118514/#comment41193
The API should probably still be more safe in release mode. What happens if
column == -1?
src/kcolumnresizer.cpp
https://git.reviewboard.kde.org/r/118514/#comment41192
You can use auto here, if wanted (you can use auto much more often in your
patch, btw.):
if (auto gridLayout = qobject_castQGridLayout*(layout)) {
d-addWidgetsFromGridLayout(gridLayout, column);
return;
}
if (auto formLayout = qobject_castQGridLayout*(layout)) {
// ...
}
- Dominik Haumann
On June 4, 2014, 9:52 a.m., Aurélien Gâteau wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118514/
---
(Updated June 4, 2014, 9:52 a.m.)
Review request for KDE Frameworks and Christoph Feck.
Repository: kwidgetsaddons
Description
---
I have been asked to add ColumnResizer to KWidgetAddons. ColumnResizer is a
helper class I created long ago to help maintain uniform widths across
layouts. You can learn more about it from the blog post I wrote back then:
http://agateau.com/2011/clean-up-your-layouts-with-columnresizer/ . I
simplified the API a bit, added some documentation, and here it is.
Diffs
-
src/CMakeLists.txt 27b9084
src/kcolumnresizer.h PRE-CREATION
src/kcolumnresizer.cpp PRE-CREATION
tests/CMakeLists.txt eccf887
tests/kcolumnresizertest.h PRE-CREATION
tests/kcolumnresizertest.cpp PRE-CREATION
tests/kcolumnresizertest.ui PRE-CREATION
Diff: https://git.reviewboard.kde.org/r/118514/diff/
Testing
---
Manual test program behaves as expected.
Thanks,
Aurélien Gâteau
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118514: Add KColumnResizer to KWidgetsAddons
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118514/#review59163 --- Regarding remove type functions, I agree we need them. Please also check if adding a widget multiple times does work. There could also be an addLayout() (and removeLayout()) function, and on each update, it would check which widgets are in there, and handle them. src/kcolumnresizer.h https://git.reviewboard.kde.org/r/118514/#comment41186 Does it make sense to make = 0 the default value for the column? src/kcolumnresizer.cpp https://git.reviewboard.kde.org/r/118514/#comment41188 I think our style guide requires indenting the initializers. src/kcolumnresizer.cpp https://git.reviewboard.kde.org/r/118514/#comment41189 indent src/kcolumnresizer.cpp https://git.reviewboard.kde.org/r/118514/#comment41190 indent src/kcolumnresizer.cpp https://git.reviewboard.kde.org/r/118514/#comment41196 What would be nice is if the widget you add is actually a GroupBox containing a supported layout, then it would internally call addWidgetsFromLayout(). Remains the issue with layouts whose children could change... - Christoph Feck On June 4, 2014, 9:52 a.m., Aurélien Gâteau wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118514/ --- (Updated June 4, 2014, 9:52 a.m.) Review request for KDE Frameworks and Christoph Feck. Repository: kwidgetsaddons Description --- I have been asked to add ColumnResizer to KWidgetAddons. ColumnResizer is a helper class I created long ago to help maintain uniform widths across layouts. You can learn more about it from the blog post I wrote back then: http://agateau.com/2011/clean-up-your-layouts-with-columnresizer/ . I simplified the API a bit, added some documentation, and here it is. Diffs - src/CMakeLists.txt 27b9084 src/kcolumnresizer.h PRE-CREATION src/kcolumnresizer.cpp PRE-CREATION tests/CMakeLists.txt eccf887 tests/kcolumnresizertest.h PRE-CREATION tests/kcolumnresizertest.cpp PRE-CREATION tests/kcolumnresizertest.ui PRE-CREATION Diff: https://git.reviewboard.kde.org/r/118514/diff/ Testing --- Manual test program behaves as expected. Thanks, Aurélien Gâteau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118514: Add KColumnResizer to KWidgetsAddons
On June 4, 2014, 11:41 a.m., Dominik Haumann wrote: What about adding something like this to Qt ? First, I was thinking the same. The efforts with KF5 work was to get as much features as possible into Qt, and only keep the stuff that isn't accepted in our libraries. But here, I am not really sure. Is the alignment requirement specific to KDE's guidelines? - Christoph --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118514/#review59170 --- On June 4, 2014, 9:52 a.m., Aurélien Gâteau wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118514/ --- (Updated June 4, 2014, 9:52 a.m.) Review request for KDE Frameworks and Christoph Feck. Repository: kwidgetsaddons Description --- I have been asked to add ColumnResizer to KWidgetAddons. ColumnResizer is a helper class I created long ago to help maintain uniform widths across layouts. You can learn more about it from the blog post I wrote back then: http://agateau.com/2011/clean-up-your-layouts-with-columnresizer/ . I simplified the API a bit, added some documentation, and here it is. Diffs - src/CMakeLists.txt 27b9084 src/kcolumnresizer.h PRE-CREATION src/kcolumnresizer.cpp PRE-CREATION tests/CMakeLists.txt eccf887 tests/kcolumnresizertest.h PRE-CREATION tests/kcolumnresizertest.cpp PRE-CREATION tests/kcolumnresizertest.ui PRE-CREATION Diff: https://git.reviewboard.kde.org/r/118514/diff/ Testing --- Manual test program behaves as expected. Thanks, Aurélien Gâteau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118514: Add KColumnResizer to KWidgetsAddons
On June 4, 2014, 1:52 p.m., Christoph Feck wrote: src/kcolumnresizer.cpp, line 182 https://git.reviewboard.kde.org/r/118514/diff/2/?file=278585#file278585line182 What would be nice is if the widget you add is actually a GroupBox containing a supported layout, then it would internally call addWidgetsFromLayout(). Remains the issue with layouts whose children could change... I disagree with this as it would make it impossible to add the groupbox itself so that its width is controlled. If you want to align the content of a group box, just do so with addWidgetsFromLayout(groupBox-layout()). - Aurélien --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118514/#review59163 --- On June 4, 2014, 11:52 a.m., Aurélien Gâteau wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118514/ --- (Updated June 4, 2014, 11:52 a.m.) Review request for KDE Frameworks and Christoph Feck. Repository: kwidgetsaddons Description --- I have been asked to add ColumnResizer to KWidgetAddons. ColumnResizer is a helper class I created long ago to help maintain uniform widths across layouts. You can learn more about it from the blog post I wrote back then: http://agateau.com/2011/clean-up-your-layouts-with-columnresizer/ . I simplified the API a bit, added some documentation, and here it is. Diffs - src/CMakeLists.txt 27b9084 src/kcolumnresizer.h PRE-CREATION src/kcolumnresizer.cpp PRE-CREATION tests/CMakeLists.txt eccf887 tests/kcolumnresizertest.h PRE-CREATION tests/kcolumnresizertest.cpp PRE-CREATION tests/kcolumnresizertest.ui PRE-CREATION Diff: https://git.reviewboard.kde.org/r/118514/diff/ Testing --- Manual test program behaves as expected. Thanks, Aurélien Gâteau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118514: Add KColumnResizer to KWidgetsAddons
On June 4, 2014, 11:41 a.m., Dominik Haumann wrote: What about adding something like this to Qt ? Christoph Feck wrote: First, I was thinking the same. The efforts with KF5 work was to get as much features as possible into Qt, and only keep the stuff that isn't accepted in our libraries. But here, I am not really sure. Is the alignment requirement specific to KDE's guidelines? Reading the comments in http://agateau.com/2011/clean-up-your-layouts-with-columnresizer/ there seems to be quite a lot of interest in this, some also saying it should be in Qt. - Dominik --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118514/#review59170 --- On June 4, 2014, 9:52 a.m., Aurélien Gâteau wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118514/ --- (Updated June 4, 2014, 9:52 a.m.) Review request for KDE Frameworks and Christoph Feck. Repository: kwidgetsaddons Description --- I have been asked to add ColumnResizer to KWidgetAddons. ColumnResizer is a helper class I created long ago to help maintain uniform widths across layouts. You can learn more about it from the blog post I wrote back then: http://agateau.com/2011/clean-up-your-layouts-with-columnresizer/ . I simplified the API a bit, added some documentation, and here it is. Diffs - src/CMakeLists.txt 27b9084 src/kcolumnresizer.h PRE-CREATION src/kcolumnresizer.cpp PRE-CREATION tests/CMakeLists.txt eccf887 tests/kcolumnresizertest.h PRE-CREATION tests/kcolumnresizertest.cpp PRE-CREATION tests/kcolumnresizertest.ui PRE-CREATION Diff: https://git.reviewboard.kde.org/r/118514/diff/ Testing --- Manual test program behaves as expected. Thanks, Aurélien Gâteau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118514: Add KColumnResizer to KWidgetsAddons
On June 4, 2014, 12:01 p.m., David Edmundson wrote: src/kcolumnresizer.cpp, line 214 https://git.reviewboard.kde.org/r/118514/diff/2/?file=278585#file278585line214 Do we really want it to be critical? Sometimes widgets are loaded dynamically, we probably don't want to explode because some widgets might not line up. Maybe a qWarning + a Q_ASSERT. Fixed. Note that qCritical() does not abort though, qFatal() does. - Aurélien --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118514/#review59145 --- On June 4, 2014, 11:52 a.m., Aurélien Gâteau wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118514/ --- (Updated June 4, 2014, 11:52 a.m.) Review request for KDE Frameworks and Christoph Feck. Repository: kwidgetsaddons Description --- I have been asked to add ColumnResizer to KWidgetAddons. ColumnResizer is a helper class I created long ago to help maintain uniform widths across layouts. You can learn more about it from the blog post I wrote back then: http://agateau.com/2011/clean-up-your-layouts-with-columnresizer/ . I simplified the API a bit, added some documentation, and here it is. Diffs - src/CMakeLists.txt 27b9084 src/kcolumnresizer.h PRE-CREATION src/kcolumnresizer.cpp PRE-CREATION tests/CMakeLists.txt eccf887 tests/kcolumnresizertest.h PRE-CREATION tests/kcolumnresizertest.cpp PRE-CREATION tests/kcolumnresizertest.ui PRE-CREATION Diff: https://git.reviewboard.kde.org/r/118514/diff/ Testing --- Manual test program behaves as expected. Thanks, Aurélien Gâteau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118514: Add KColumnResizer to KWidgetsAddons
On June 4, 2014, 1:41 p.m., Dominik Haumann wrote:
src/kcolumnresizer.cpp, line 200
https://git.reviewboard.kde.org/r/118514/diff/2/?file=278585#file278585line200
You can use auto here, if wanted (you can use auto much more often in
your patch, btw.):
if (auto gridLayout = qobject_castQGridLayout*(layout)) {
d-addWidgetsFromGridLayout(gridLayout, column);
return;
}
if (auto formLayout = qobject_castQGridLayout*(layout)) {
// ...
}
I am not fond of declaring variables inside the condition part of an if: I find
those harder to read.
- Aurélien
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118514/#review59170
---
On June 4, 2014, 11:52 a.m., Aurélien Gâteau wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118514/
---
(Updated June 4, 2014, 11:52 a.m.)
Review request for KDE Frameworks and Christoph Feck.
Repository: kwidgetsaddons
Description
---
I have been asked to add ColumnResizer to KWidgetAddons. ColumnResizer is a
helper class I created long ago to help maintain uniform widths across
layouts. You can learn more about it from the blog post I wrote back then:
http://agateau.com/2011/clean-up-your-layouts-with-columnresizer/ . I
simplified the API a bit, added some documentation, and here it is.
Diffs
-
src/CMakeLists.txt 27b9084
src/kcolumnresizer.h PRE-CREATION
src/kcolumnresizer.cpp PRE-CREATION
tests/CMakeLists.txt eccf887
tests/kcolumnresizertest.h PRE-CREATION
tests/kcolumnresizertest.cpp PRE-CREATION
tests/kcolumnresizertest.ui PRE-CREATION
Diff: https://git.reviewboard.kde.org/r/118514/diff/
Testing
---
Manual test program behaves as expected.
Thanks,
Aurélien Gâteau
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118514: Add KColumnResizer to KWidgetsAddons
On June 4, 2014, 1:52 p.m., Christoph Feck wrote: src/kcolumnresizer.h, line 93 https://git.reviewboard.kde.org/r/118514/diff/2/?file=278584#file278584line93 Does it make sense to make = 0 the default value for the column? Sounds good to me. Just did so. - Aurélien --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118514/#review59163 --- On June 4, 2014, 11:52 a.m., Aurélien Gâteau wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118514/ --- (Updated June 4, 2014, 11:52 a.m.) Review request for KDE Frameworks and Christoph Feck. Repository: kwidgetsaddons Description --- I have been asked to add ColumnResizer to KWidgetAddons. ColumnResizer is a helper class I created long ago to help maintain uniform widths across layouts. You can learn more about it from the blog post I wrote back then: http://agateau.com/2011/clean-up-your-layouts-with-columnresizer/ . I simplified the API a bit, added some documentation, and here it is. Diffs - src/CMakeLists.txt 27b9084 src/kcolumnresizer.h PRE-CREATION src/kcolumnresizer.cpp PRE-CREATION tests/CMakeLists.txt eccf887 tests/kcolumnresizertest.h PRE-CREATION tests/kcolumnresizertest.cpp PRE-CREATION tests/kcolumnresizertest.ui PRE-CREATION Diff: https://git.reviewboard.kde.org/r/118514/diff/ Testing --- Manual test program behaves as expected. Thanks, Aurélien Gâteau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118514: Add KColumnResizer to KWidgetsAddons
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118514/ --- (Updated June 4, 2014, 2:43 p.m.) Review request for KDE Frameworks and Christoph Feck. Changes --- Fixed many reported issues. Repository: kwidgetsaddons Description --- I have been asked to add ColumnResizer to KWidgetAddons. ColumnResizer is a helper class I created long ago to help maintain uniform widths across layouts. You can learn more about it from the blog post I wrote back then: http://agateau.com/2011/clean-up-your-layouts-with-columnresizer/ . I simplified the API a bit, added some documentation, and here it is. Diffs (updated) - src/kcolumnresizer.cpp PRE-CREATION src/CMakeLists.txt 27b9084 src/kcolumnresizer.h PRE-CREATION tests/CMakeLists.txt eccf887 tests/kcolumnresizertest.h PRE-CREATION tests/kcolumnresizertest.cpp PRE-CREATION tests/kcolumnresizertest.ui PRE-CREATION Diff: https://git.reviewboard.kde.org/r/118514/diff/ Testing --- Manual test program behaves as expected. Thanks, Aurélien Gâteau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 118526: Provide i18nd wrappers in kdeclarative
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118526/ --- Review request for KDE Frameworks, Plasma and Marco Martin. Repository: kdeclarative Description --- Provide i18nd wrappers in kdeclarative As QML might combine multiple modules with different cataloges we need to be able to specify the translation domain explicitly. If there is a need to use a specific domain for all i18n calls (e.g. in a library using QML) there is the possibility to set a global translation domain through KDeclarative. If such a domain is set all i18n calls delegate to the i18nd variant. Due to the nature of KDeclarative we cannot mix i18n calls with different domains. If two modules would require to set the translation domain it's bound to fail. Thus the recommendation is to use the i18nd variants in any QML code which is intended to be used as an import. Diffs - src/kdeclarative/kdeclarative.h b4a274b710f4de7ffbfc275d1e9a0a93be283053 src/kdeclarative/kdeclarative.cpp a35dac5cfbd42e75e892d4ad88c491345be4a1b0 src/kdeclarative/private/kdeclarative_p.h 6b61d123bf74671b413e4e68bf911bb969fdaf53 src/kdeclarative/private/rootcontext.cpp 12309b096495910b83ed1e388989042b45a1 src/kdeclarative/private/rootcontext_p.h 16694b155c668e11cf7a16549a18cdc89b81b3e2 Diff: https://git.reviewboard.kde.org/r/118526/diff/ Testing --- adjusted kwineffects KCM and run it with the x-test language: the strings with i18n from QML side are now picking up the translated strings. Thanks, Martin Gräßlin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Jenkins build is back to stable : ktexteditor_master_qt5 #417
See http://build.kde.org/job/ktexteditor_master_qt5/417/ ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118458: Fix KDirWatch to act more reliably against various backends.
On June 4, 2014, 3:38 a.m., David Faure wrote: autotests/kdirwatch_unittest.cpp, line 523 https://git.reviewboard.kde.org/r/118458/diff/1/?file=277318#file277318line523 The commit log doesn't mention any changes related to QFileSystemWatcher, only to the FAM backend. Are you sure you wanted to commit this? If it works then of course I'm happy with it (but maybe in a separate commit then) I'm not sure how that got fixed, I just seem to remember changing things and in the process of working on that test I discovered that QFSW worked just fine. I don't remember if my changes did that, or if it was fixed sometime ago and I just discovered the fact. Or if Qt5 fixed it? I'll move it to a separate commit to be safe. On June 4, 2014, 3:38 a.m., David Faure wrote: autotests/kdirwatch_unittest.cpp, line 677 https://git.reviewboard.kde.org/r/118458/diff/1/?file=277318#file277318line677 This could actually use QFile::link(), would be more readable. So, here be dragons. QFile::link fails the unit test. But ::link doesn't. I'm going to leave this alone for now. On June 4, 2014, 3:38 a.m., David Faure wrote: src/lib/io/kdirwatch.cpp, line 636 https://git.reviewboard.kde.org/r/118458/diff/1/?file=277319#file277319line636 I'm not 100% sure about the meaning of this boolean variable. (we started an event?) Maybe it can be called something more explicit (waitForInitialFAMEvents? or something that makes sense to you ;)) I'm not sure what this should be. I think startedFAMMonitor is more clear, as it says what the variable is tracking, as opposed to what happens when set. It marks when a FAMMonitor* function is called, which matches. If you prefer the waitForInitialFAMEvents, signalling what will happen, I'll it to that. You did basically guess what the variable meant, but it really speaks more to the FAM Monitor piece, not an event. On June 4, 2014, 3:38 a.m., David Faure wrote: src/lib/io/kdirwatch.cpp, line 1633 https://git.reviewboard.kde.org/r/118458/diff/1/?file=277319#file277319line1633 Technically this could be an event about a completely unrelated file to the one we're waiting to see, couldn't it? As far as I understand the code, no. FAM reports which request the FAM event is associated with, which each KDirWatch event stores. So this FAMEndExists (or similar) will be associated with a single KDW event, and thus only update the correct one. On June 4, 2014, 3:38 a.m., David Faure wrote: src/lib/io/kdirwatch.cpp, line 1719 https://git.reviewboard.kde.org/r/118458/diff/1/?file=277319#file277319line1719 actual - actually ? Whoops, fixed. On June 4, 2014, 3:38 a.m., David Faure wrote: autotests/kdirwatch_unittest.cpp, line 297 https://git.reviewboard.kde.org/r/118458/diff/1/?file=277318#file277318line297 It seems that the second call should pass created(QString) rather than deleted(QString). Copy and paste errors are the worst ... - Matthew --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118458/#review59123 --- On June 1, 2014, 5:03 p.m., Matthew Dawson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118458/ --- (Updated June 1, 2014, 5:03 p.m.) Review request for KDE Frameworks. Bugs: 329802 https://bugs.kde.org/show_bug.cgi?id=329802 Repository: kcoreaddons Description --- Fix KDirWatch to act more reliably against various backends. - When KDirWatch used a FAM backend, it wouldn't actually wait for the necessary watches to be put in place before returning, allowing for a race where an application may think a watch is in place when it really isn't. This was easily seen using gamin and running the testDeleteAndRecreateFile test. Fix by delaying useFAM until the watch is in place by waiting until the EndExist FAM event is received. This adds ~100-200ms per watch with gamin. - When a file is deleted and recreated, and scanEntry detects it, the dirty signal would be emitted. Fix to emit a deleted + created signal, as expected. - When a deleted signal was requested, other signals were dropped. Due to the above point, this would stop the created signal from being emitted. Now, all signals are emitted, even when a delete is detected. - When watching recreated files, the created signal could get lost as there was a race between when the created signal would be generated and its signal spy would be created. Fix by making sure the spy is created before the signal could be emitted. - Make sure the created signal isn't emitted twice with both FAM and the polling timer. This occurs as
Re: Review Request 118458: Fix KDirWatch to act more reliably against various backends.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118458/ --- (Updated June 4, 2014, 1:05 p.m.) Review request for KDE Frameworks. Changes --- Fix patch to resolve issues raised. Bugs: 329802 https://bugs.kde.org/show_bug.cgi?id=329802 Repository: kcoreaddons Description --- Fix KDirWatch to act more reliably against various backends. - When KDirWatch used a FAM backend, it wouldn't actually wait for the necessary watches to be put in place before returning, allowing for a race where an application may think a watch is in place when it really isn't. This was easily seen using gamin and running the testDeleteAndRecreateFile test. Fix by delaying useFAM until the watch is in place by waiting until the EndExist FAM event is received. This adds ~100-200ms per watch with gamin. - When a file is deleted and recreated, and scanEntry detects it, the dirty signal would be emitted. Fix to emit a deleted + created signal, as expected. - When a deleted signal was requested, other signals were dropped. Due to the above point, this would stop the created signal from being emitted. Now, all signals are emitted, even when a delete is detected. - When watching recreated files, the created signal could get lost as there was a race between when the created signal would be generated and its signal spy would be created. Fix by making sure the spy is created before the signal could be emitted. - Make sure the created signal isn't emitted twice with both FAM and the polling timer. This occurs as FAM doesn't fix up the fact the entry has been created, and the poller thus assumes it needs to notify the world. Fix by having FAM not emit the event in this case. BUG: 329802 Note that due to a bug in the FAM daemon (which doesn't exist in the gamin daemon), the testDeleteAndRecreateDir unit test fails as FAM doesn't detect the folder deletion. This test previously passed due to the race condition outlined in bug 329802, which was fixed in this RR. I've confirmed this is a bug in FAM using a minimal program talking directly to the daemon, and watching the strace output from FAM. FAM doesn't detect when a folder it is watching directly is deleted. It does detect when a folder inside a folder it is watching is deleted, however. I think this should go in as is, as the unit test failing exposes the actual problem, instead of hiding behind a race when it doesn't really work. If running against FAM is important, I can see about tracking down the issue. Diffs (updated) - autotests/kdirwatch_unittest.cpp 4e638e089012276dc8b17b9d960732a161cc3be1 src/lib/io/kdirwatch.cpp cc6feec4d2b2598e50c853172bbc295a8c719bc2 src/lib/io/kdirwatch_p.h cf63c41745091cfedcf1b27c9e4aa77d3db6b31a Diff: https://git.reviewboard.kde.org/r/118458/diff/ Testing --- Unit tests all pass now with gamin. Using FAM, one test fails as described above, but I've confirmed the daemon itself is causing the issue. Thanks, Matthew Dawson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Jenkins build is back to stable : kdelibs_stable #1102
See http://build.kde.org/job/kdelibs_stable/1102/changes ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 118538: Add a property containing the real edge a dialog is shown on
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/ --- Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Add a property containing the real edge a dialog is shown on. If a dialog is meant to be shown on the left of an item, if there is not enough space it will be shown on the right. We need to keep track of where we're actually shown in order to show the correct borders and perform the animation in the correct direction. It is exposed as a property in case the dialog itself needs to know which side of the parent item it is, for example if needs to show an arrow to the parent. -- Fixed some serious copy + paste fails in the hitting the bottom edge of the screen check. Diffs - src/plasmaquick/dialog.h 4009222 src/plasmaquick/dialog.cpp 2f8227c Diff: https://git.reviewboard.kde.org/r/118538/diff/ Testing --- Ran dialog_positioning.qml in all 8 combinations of edges + preferred locations. (i.e placed near left edge, showed item with location set to both left and right) Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118538: Add a property containing the real edge a dialog is shown on
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/#review59234 --- what is a valid use case where qml needs to know where the dialog actually is?(not hypothetical please) doing the animation on the proper direction can be tracked completely internally also, plasmoids should never ever do a screen edge animation when in the middle of the desktop, the animation looks bad where it's clipped if it's not against a panel or a screen edge - Marco Martin On June 4, 2014, 7:07 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/ --- (Updated June 4, 2014, 7:07 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Add a property containing the real edge a dialog is shown on. If a dialog is meant to be shown on the left of an item, if there is not enough space it will be shown on the right. We need to keep track of where we're actually shown in order to show the correct borders and perform the animation in the correct direction. It is exposed as a property in case the dialog itself needs to know which side of the parent item it is, for example if needs to show an arrow to the parent. -- Fixed some serious copy + paste fails in the hitting the bottom edge of the screen check. Diffs - src/plasmaquick/dialog.h 4009222 src/plasmaquick/dialog.cpp 2f8227c Diff: https://git.reviewboard.kde.org/r/118538/diff/ Testing --- Ran dialog_positioning.qml in all 8 combinations of edges + preferred locations. (i.e placed near left edge, showed item with location set to both left and right) Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118538: Add a property containing the real edge a dialog is shown on
On June 4, 2014, 7:30 p.m., Marco Martin wrote: what is a valid use case where qml needs to know where the dialog actually is?(not hypothetical please) doing the animation on the proper direction can be tracked completely internally also, plasmoids should never ever do a screen edge animation when in the middle of the desktop, the animation looks bad where it's clipped if it's not against a panel or a screen edge Milou places the most relevant item closest to the launcher. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/#review59234 --- On June 4, 2014, 7:07 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/ --- (Updated June 4, 2014, 7:07 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Add a property containing the real edge a dialog is shown on. If a dialog is meant to be shown on the left of an item, if there is not enough space it will be shown on the right. We need to keep track of where we're actually shown in order to show the correct borders and perform the animation in the correct direction. It is exposed as a property in case the dialog itself needs to know which side of the parent item it is, for example if needs to show an arrow to the parent. -- Fixed some serious copy + paste fails in the hitting the bottom edge of the screen check. Diffs - src/plasmaquick/dialog.h 4009222 src/plasmaquick/dialog.cpp 2f8227c Diff: https://git.reviewboard.kde.org/r/118538/diff/ Testing --- Ran dialog_positioning.qml in all 8 combinations of edges + preferred locations. (i.e placed near left edge, showed item with location set to both left and right) Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118270: [doc] explicitly load external entities (after CVE-2014-0191)
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118270/#review59243 --- This review has been submitted with commit d4fca9ffb31a2383459c89b27f81b10b7ddece1a by Luigi Toscano to branch KDE/4.13. - Commit Hook On June 3, 2014, 1:50 p.m., Luigi Toscano wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118270/ --- (Updated June 3, 2014, 1:50 p.m.) Review request for Documentation, KDE Frameworks, kdelibs, Rohan Garg, Jonathan Riddell, Luc Menut, and Rex Dieter. Bugs: 335001 http://bugs.kde.org/show_bug.cgi?id=335001 Repository: kdelibs Description --- Use the more modern API function for XML loading and enable the flags which load the external entities, so that meinproc4 can work again after the security changes implemented for CVE-2014-0191. Without this change meinproc4 complains (see the referenced bug) The fix (half of the patch, the other half is on code which was removed) applies to KF5 too, hence the group. My tests shows that the documentation cache is properly generated as before, and the patch should work even on the old Packagers (Ubuntu packagers in CC, as Ubuntu is one of the few distributions where libxml2 has been already patched) could you please test it with a fixed libxml and without, and if possible with KF5 as well? Diffs - kdoctools/meinproc.cpp 0894d63 kdoctools/xslt.cpp a7265ca Diff: https://git.reviewboard.kde.org/r/118270/diff/ Testing --- meinproc4 works again Thanks, Luigi Toscano ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118270: [doc] explicitly load external entities (after CVE-2014-0191)
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118270/ --- (Updated June 4, 2014, 8:40 p.m.) Status -- This change has been marked as submitted. Review request for Documentation, KDE Frameworks, kdelibs, Rohan Garg, Jonathan Riddell, Luc Menut, and Rex Dieter. Bugs: 335001 http://bugs.kde.org/show_bug.cgi?id=335001 Repository: kdelibs Description --- Use the more modern API function for XML loading and enable the flags which load the external entities, so that meinproc4 can work again after the security changes implemented for CVE-2014-0191. Without this change meinproc4 complains (see the referenced bug) The fix (half of the patch, the other half is on code which was removed) applies to KF5 too, hence the group. My tests shows that the documentation cache is properly generated as before, and the patch should work even on the old Packagers (Ubuntu packagers in CC, as Ubuntu is one of the few distributions where libxml2 has been already patched) could you please test it with a fixed libxml and without, and if possible with KF5 as well? Diffs - kdoctools/meinproc.cpp 0894d63 kdoctools/xslt.cpp a7265ca Diff: https://git.reviewboard.kde.org/r/118270/diff/ Testing --- meinproc4 works again Thanks, Luigi Toscano ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118538: Add a property containing the real edge a dialog is shown on
On June 4, 2014, 7:30 p.m., Marco Martin wrote: what is a valid use case where qml needs to know where the dialog actually is?(not hypothetical please) doing the animation on the proper direction can be tracked completely internally also, plasmoids should never ever do a screen edge animation when in the middle of the desktop, the animation looks bad where it's clipped if it's not against a panel or a screen edge David Edmundson wrote: Milou places the most relevant item closest to the launcher. Hmm, it would be relevant only in the case milou is iconified and in the desktop, that seems quite a narrow case. api wise it may make sense a Plasma::Types::Direction visualParentDirection, *but* it would have to be exported in plasmoid as well, and I don't want anything like that here. API that is added ad hoc for a single, extremely narrow use case is never a good idea. - Marco --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/#review59234 --- On June 4, 2014, 7:07 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/ --- (Updated June 4, 2014, 7:07 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Add a property containing the real edge a dialog is shown on. If a dialog is meant to be shown on the left of an item, if there is not enough space it will be shown on the right. We need to keep track of where we're actually shown in order to show the correct borders and perform the animation in the correct direction. It is exposed as a property in case the dialog itself needs to know which side of the parent item it is, for example if needs to show an arrow to the parent. -- Fixed some serious copy + paste fails in the hitting the bottom edge of the screen check. Diffs - src/plasmaquick/dialog.h 4009222 src/plasmaquick/dialog.cpp 2f8227c Diff: https://git.reviewboard.kde.org/r/118538/diff/ Testing --- Ran dialog_positioning.qml in all 8 combinations of edges + preferred locations. (i.e placed near left edge, showed item with location set to both left and right) Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 118546: No symlink to common/ directory for documentation common files (broken, now unused, UNIX only)
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118546/ --- Review request for Documentation and KDE Frameworks. Repository: kdoctools Description --- The common/ directory (which contains per-language common files for documentation) has been renamed (kdoctools5-common), and even if it was not, the symlink is not needed anymore to retrieve the documentation (the references to the content in that directory now are accessed through help:/ kioslave). The removal of the need for this symlink solve a long-standing TODO for non-UNIX platform(s). Diffs - KF5DocToolsMacros.cmake 823da90 Diff: https://git.reviewboard.kde.org/r/118546/diff/ Testing --- Documentation generated, help:/ kioslave still working Thanks, Luigi Toscano ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118538: Add a property containing the real edge a dialog is shown on
On June 4, 2014, 7:30 p.m., Marco Martin wrote: what is a valid use case where qml needs to know where the dialog actually is?(not hypothetical please) doing the animation on the proper direction can be tracked completely internally also, plasmoids should never ever do a screen edge animation when in the middle of the desktop, the animation looks bad where it's clipped if it's not against a panel or a screen edge David Edmundson wrote: Milou places the most relevant item closest to the launcher. Marco Martin wrote: Hmm, it would be relevant only in the case milou is iconified and in the desktop, that seems quite a narrow case. api wise it may make sense a Plasma::Types::Direction visualParentDirection, *but* it would have to be exported in plasmoid as well, and I don't want anything like that here. API that is added ad hoc for a single, extremely narrow use case is never a good idea. kickoff changes direction too so the toolbar is closest to the launcher. However, I'm not too fussed about the new property, I can remove the Q_PROPERTY and add it if we decide it's needed. The important part for me is making it show the correct borders and animate in the right direction. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/#review59234 --- On June 4, 2014, 7:07 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/ --- (Updated June 4, 2014, 7:07 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Add a property containing the real edge a dialog is shown on. If a dialog is meant to be shown on the left of an item, if there is not enough space it will be shown on the right. We need to keep track of where we're actually shown in order to show the correct borders and perform the animation in the correct direction. It is exposed as a property in case the dialog itself needs to know which side of the parent item it is, for example if needs to show an arrow to the parent. -- Fixed some serious copy + paste fails in the hitting the bottom edge of the screen check. Diffs - src/plasmaquick/dialog.h 4009222 src/plasmaquick/dialog.cpp 2f8227c Diff: https://git.reviewboard.kde.org/r/118538/diff/ Testing --- Ran dialog_positioning.qml in all 8 combinations of edges + preferred locations. (i.e placed near left edge, showed item with location set to both left and right) Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118538: Add a property containing the real edge a dialog is shown on
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/ --- (Updated June 4, 2014, 10:43 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Add a property containing the real edge a dialog is shown on. If a dialog is meant to be shown on the left of an item, if there is not enough space it will be shown on the right. We need to keep track of where we're actually shown in order to show the correct borders and perform the animation in the correct direction. It is exposed as a property in case the dialog itself needs to know which side of the parent item it is, for example if needs to show an arrow to the parent. -- Fixed some serious copy + paste fails in the hitting the bottom edge of the screen check. Diffs (updated) - src/plasmaquick/dialog.cpp 2f8227c src/plasmaquick/dialog.h 4009222 Diff: https://git.reviewboard.kde.org/r/118538/diff/ Testing --- Ran dialog_positioning.qml in all 8 combinations of edges + preferred locations. (i.e placed near left edge, showed item with location set to both left and right) Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 118547: Expose Formats as singleton
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118547/
---
Review request for KDE Frameworks and Plasma.
Repository: kdeclarative
Description
---
Expose Formats as singleton
Formats is basically just a collection of invokable static methods.
This saves creating a few objects and makes for a a nicer API.
Without it you have to have a bit of code like:
-KCoreAddons.Formats {
-id: formats
-}
then do text:formats.formatData(...) in your labels
this becomes now
KCoreAddons.Format.formatData(...)
Diffs
-
src/qmlcontrols/kcoreaddons/kcoreaddonsplugin.cpp 65dd75f
Diff: https://git.reviewboard.kde.org/r/118547/diff/
Testing
---
Made the two changes needed in plasma-workspace. Everything seemed to be ok.
Thanks,
David Edmundson
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118547: Expose Formats as singleton
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118547/#review59256
---
Ship it!
Makes sense and looks nice!
- Mark Gaiser
On June 5, 2014, 12:03 a.m., David Edmundson wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118547/
---
(Updated June 5, 2014, 12:03 a.m.)
Review request for KDE Frameworks and Plasma.
Repository: kdeclarative
Description
---
Expose Formats as singleton
Formats is basically just a collection of invokable static methods.
This saves creating a few objects and makes for a a nicer API.
Without it you have to have a bit of code like:
-KCoreAddons.Formats {
-id: formats
-}
then do text:formats.formatData(...) in your labels
this becomes now
KCoreAddons.Format.formatData(...)
Diffs
-
src/qmlcontrols/kcoreaddons/kcoreaddonsplugin.cpp 65dd75f
Diff: https://git.reviewboard.kde.org/r/118547/diff/
Testing
---
Made the two changes needed in plasma-workspace. Everything seemed to be ok.
Thanks,
David Edmundson
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118547: Expose Formats as singleton
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118547/#review59261
---
src/qmlcontrols/kcoreaddons/kcoreaddonsplugin.cpp
https://git.reviewboard.kde.org/r/118547/#comment41253
Err no, this will fail when using formatRelativeDate and
formatRelativeDateTime
- Bhushan Shah
On June 5, 2014, 5:33 a.m., David Edmundson wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118547/
---
(Updated June 5, 2014, 5:33 a.m.)
Review request for KDE Frameworks and Plasma.
Repository: kdeclarative
Description
---
Expose Formats as singleton
Formats is basically just a collection of invokable static methods.
This saves creating a few objects and makes for a a nicer API.
Without it you have to have a bit of code like:
-KCoreAddons.Formats {
-id: formats
-}
then do text:formats.formatData(...) in your labels
this becomes now
KCoreAddons.Format.formatData(...)
Diffs
-
src/qmlcontrols/kcoreaddons/kcoreaddonsplugin.cpp 65dd75f
Diff: https://git.reviewboard.kde.org/r/118547/diff/
Testing
---
Made the two changes needed in plasma-workspace. Everything seemed to be ok.
Thanks,
David Edmundson
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118490: Properly mark ServiceBrowser::isRunning as deprecated.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118490/ --- (Updated June 4, 2014, 11:56 p.m.) Review request for KDE Frameworks and Matthew Dawson. Changes --- New version, taking into account all comments. I'd appreciate it if someone can have a quick look over the comment to verify it makes sense. Summary (updated) - Properly mark ServiceBrowser::isRunning as deprecated. Repository: kdnssd Description (updated) --- Properly mark ServiceBrowser::isRunning as deprecated. Fix the @deprecated notation to be correct. Also, make the function a simple inline function that always returns false. This should cause minimal breakage while avoiding overhead from the function. Diffs (updated) - src/mdnsd-servicetypebrowser.cpp d16dd1fa1c692cd20e21f1a7d5471ac7956469b3 src/dummy-servicetypebrowser.cpp 39a4b6b660bb47b68b5721535994487dbe2abbf6 src/avahi-servicetypebrowser.cpp b3c14f84367d5093a6cee5ef3f684edc201a3f96 src/servicetypebrowser.h 3cd10e36bf089e088e8a48bb0cf73daa4fb859c5 Diff: https://git.reviewboard.kde.org/r/118490/diff/ Testing --- Code still compiles. Thanks, Matthew Dawson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
