D8983: Properly parse dates in cookies when running in non-English locale
anthonyfieroni added inline comments.
INLINE COMMENTS
> kcookiejar.cpp:78-84
> +static const char *const day_name[] = {
> + "Mon", "Tue", "Wed", "Thu", "Fri", "Sat", "Sun"
> +};
> +for (int i = 0; i < 7; ++i) {
> +// No need to check for long names since the short names are
> +// prefixes of the long names
> +if (weekday.startsWith(QL1S(day_name[i]), Qt::CaseInsensitive)) {
You can use not predefined days name
QLocale locale = QLocale::c();
for (int i = 1; i < 8; ++i) {
if (weekday.startsWith(locale.dayName(i, QLocale::ShortFormat),
Qt::CaseInsensitive)) {
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8983
To: schwab, dfaure, #frameworks
Cc: anthonyfieroni, #frameworks
KDE CI: Frameworks kio kf5-qt5 SUSEQt5.7 - Build # 2 - Fixed!
BUILD SUCCESS Build URL https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.7/2/ Project: Frameworks kio kf5-qt5 SUSEQt5.7 Date of build: Sun, 26 Nov 2017 03:52:56 + Build duration: 15 min and counting JUnit Tests Name: (root) Failed: 2 test(s), Passed: 53 test(s), Skipped: 0 test(s), Total: 55 test(s)Failed: TestSuite.kiowidgets-kurifiltersearchprovideractionstestFailed: TestSuite.kiowidgets-kurifiltertest Cobertura Report Project Coverage Summary Name PackagesFilesClassesLinesConditionalsCobertura Coverage Report65% (22/34)65% (278/425)65% (278/425)51% (29695/57972)37% (17309/47122)Coverage Breakdown by Package Name FilesClassesLinesConditionalsautotests97% (66/68)97% (66/68)92% (7738/8415)50% (4812/9694)autotests.http100% (9/9)100% (9/9)100% (586/587)59% (217/368)autotests.kcookiejar100% (1/1)100% (1/1)91% (180/198)67% (63/94)src.core83% (98/118)83% (98/118)57% (8063/14105)49% (4744/9584)src.core.kssl100% (1/1)100% (1/1)40% (35/88)50% (3/6)src.filewidgets78% (29/37)78% (29/37)48% (3683/7613)31% (1470/4785)src.gui100% (2/2)100% (2/2)95% (104/110)77% (57/74)src.ioslaves.file100% (2/2)100% (2/2)53% (434/819)44% (324/736)src.ioslaves.ftp0% (0/2)0% (0/2)0% (0/1364)0% (0/1513)src.ioslaves.help0% (0/5)0% (0/5)0% (0/247)0% (0/184)src.ioslaves.http89% (8/9)89% (8/9)41% (1788/4338)35% (1373/3979)src.ioslaves.http.kcookiejar33% (2/6)33% (2/6)47% (630/1332)55% (656/1184)src.ioslaves.remote100% (2/2)100% (2/2)28% (71/258)8% (17/220)src.ioslaves.remote.kdedmodule0% (0/4)0% (0/4)0% (0/14)100% (0/0)src.ioslaves.telnet0% (0/1)0% (0/1)0% (0/43)0% (0/30)src.ioslaves.trash67% (8/12)67% (8/12)52% (725/1385)46% (449/968)src.ioslaves.trash.tests67% (2/3)67% (2/3)88% (709/802)47% (515/1089)src.kcms.kio0%
KDE CI: Frameworks kio kf5-qt5 FreeBSDQt5.7 - Build # 142 - Fixed!
BUILD SUCCESS Build URL https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20FreeBSDQt5.7/142/ Project: Frameworks kio kf5-qt5 FreeBSDQt5.7 Date of build: Sun, 26 Nov 2017 03:52:56 + Build duration: 10 min and counting JUnit Tests Name: (root) Failed: 0 test(s), Passed: 54 test(s), Skipped: 0 test(s), Total: 54 test(s)
KDE CI: Frameworks kio kf5-qt5 SUSEQt5.10 - Build # 2 - Fixed!
BUILD SUCCESS Build URL https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.10/2/ Project: Frameworks kio kf5-qt5 SUSEQt5.10 Date of build: Sun, 26 Nov 2017 03:52:56 + Build duration: 6 min 44 sec and counting JUnit Tests Name: (root) Failed: 2 test(s), Passed: 53 test(s), Skipped: 0 test(s), Total: 55 test(s)Failed: TestSuite.kiowidgets-kurifiltersearchprovideractionstestFailed: TestSuite.kiowidgets-kurifiltertest Cobertura Report Project Coverage Summary Name PackagesFilesClassesLinesConditionalsCobertura Coverage Report62% (21/34)65% (275/425)65% (275/425)50% (29100/57947)36% (16888/47046)Coverage Breakdown by Package Name FilesClassesLinesConditionalsautotests97% (66/68)97% (66/68)90% (7613/8414)49% (4720/9694)autotests.http100% (9/9)100% (9/9)100% (586/587)59% (217/368)autotests.kcookiejar100% (1/1)100% (1/1)91% (180/198)67% (63/94)src.core82% (97/118)82% (97/118)55% (7774/14086)48% (4586/9580)src.core.kssl100% (1/1)100% (1/1)40% (35/88)50% (3/6)src.filewidgets78% (29/37)78% (29/37)48% (3683/7613)31% (1470/4785)src.gui0% (0/2)0% (0/2)0% (0/105)0% (0/74)src.ioslaves.file100% (2/2)100% (2/2)53% (434/819)44% (324/736)src.ioslaves.ftp0% (0/2)0% (0/2)0% (0/1364)0% (0/1513)src.ioslaves.help0% (0/5)0% (0/5)0% (0/247)0% (0/184)src.ioslaves.http89% (8/9)89% (8/9)39% (1705/4337)32% (1261/3979)src.ioslaves.http.kcookiejar33% (2/6)33% (2/6)47% (630/1332)55% (656/1184)src.ioslaves.remote100% (2/2)100% (2/2)28% (71/258)8% (17/220)src.ioslaves.remote.kdedmodule0% (0/4)0% (0/4)0% (0/14)100% (0/0)src.ioslaves.telnet0% (0/1)0% (0/1)0% (0/43)0% (0/30)src.ioslaves.trash67% (8/12)67% (8/12)52% (715/1385)45% (437/968)src.ioslaves.trash.tests67% (2/3)67% (2/3)88% (709/802)47% (515/1089)src.kcms.kio0%
D8366: Factoring out lists of url data within KFilePlacesModelTest
ngraham added a comment. Is this ready to go in, or do we need some more time for review? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8366 To: franckarrecot, renatoo, ervin, mlaurent Cc: ervin, ngraham, mlaurent, #frameworks
D8348: Add a section for removable devices
ngraham added a comment. @renatoo, now that https://phabricator.kde.org/D8332 is in, this no patch longer applies cleanly. Can you update so I can land it? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8348 To: renatoo, #dolphin, #frameworks, #vdg, ervin, ngraham, mwolff Cc: mwolff, abetts, mlaurent, anthonyfieroni, ngraham, #frameworks
D8332: Added baloo urls into places model
This revision was automatically updated to reflect the committed changes. Closed by commit R241:7eb6333bdb48: Added baloo urls into places model (authored by Renato Araujo Oliveira Filho renato.ara...@kdab.com, committed by ngraham). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8332?vs=22810=22942 REVISION DETAIL https://phabricator.kde.org/D8332 AFFECTED FILES autotests/CMakeLists.txt autotests/kfileplacesmodeltest.cpp autotests/kfileplacesviewtest.cpp src/filewidgets/kfileplacesmodel.cpp src/filewidgets/kfileplacesview.cpp To: renatoo, #frameworks, #dolphin, #kde_applications, dvratil, #vdg, ngraham, ervin, mlaurent, dfaure, mwolff Cc: mwolff, dfaure, ervin, usta, mlaurent, dvratil, ngraham, #frameworks
D8098: Strip down and re-write the baloo tags KIO slave
smithjd updated this revision to Diff 22940. smithjd added a comment. - Fix for slash-less tag urls. REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8098?vs=22938=22940 BRANCH master-nestedTags (branched from master) REVISION DETAIL https://phabricator.kde.org/D8098 AFFECTED FILES src/kioslaves/tags/kio_tags.cpp src/kioslaves/tags/kio_tags.h src/kioslaves/tags/tags.protocol To: smithjd, #frameworks, vhanda, #dolphin, ngraham, dfaure Cc: anthonyfieroni, dfaure, nicolasfella, ngraham
KDE CI: Frameworks kwidgetsaddons kf5-qt5 WindowsMSVCQt5.9 - Build # 31 - Fixed!
BUILD SUCCESS Build URL https://build.kde.org/job/Frameworks%20kwidgetsaddons%20kf5-qt5%20WindowsMSVCQt5.9/31/ Project: Frameworks kwidgetsaddons kf5-qt5 WindowsMSVCQt5.9 Date of build: Sun, 26 Nov 2017 02:45:51 + Build duration: 11 min and counting JUnit Tests Name: (root) Failed: 1 test(s), Passed: 17 test(s), Skipped: 0 test(s), Total: 18 test(s)Failed: TestSuite.kcolorbuttontest
D8791: Avoid inconsistent passworddialog
This revision was automatically updated to reflect the committed changes. Closed by commit R236:0919a1b8ba52: Avoid inconsistent passworddialog (authored by cryptodude, committed by ngraham). REPOSITORY R236 KWidgetsAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8791?vs=22268=22939 REVISION DETAIL https://phabricator.kde.org/D8791 AFFECTED FILES src/kpassworddialog.cpp To: cryptodude, dfaure, cfeck, ngraham Cc: ngraham, #frameworks
D8999: KJob: add start(int delay) method
rjvbb abandoned this revision. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D8999 To: rjvbb, dfaure Cc: apol, anthonyfieroni, #frameworks
D8098: Strip down and re-write the baloo tags KIO slave
smithjd added a comment. Currently known issues: Entering a tagged folder works, listing it from within the tags: protocol in the slave results in a folder cannot be entered error. Attempting to copy a tag tree with such a populated folder results in a file not found error for each file inside the folder. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D8098 To: smithjd, #frameworks, vhanda, #dolphin, ngraham, dfaure Cc: anthonyfieroni, dfaure, nicolasfella, ngraham
D8098: Strip down and re-write the baloo tags KIO slave
smithjd updated this revision to Diff 22938. smithjd added a comment. - Fix valid tag calculation. REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8098?vs=22857=22938 BRANCH master-nestedTags (branched from master) REVISION DETAIL https://phabricator.kde.org/D8098 AFFECTED FILES src/kioslaves/tags/kio_tags.cpp src/kioslaves/tags/kio_tags.h src/kioslaves/tags/tags.protocol To: smithjd, #frameworks, vhanda, #dolphin, ngraham, dfaure Cc: anthonyfieroni, dfaure, nicolasfella, ngraham
KDE CI: Frameworks plasma-framework kf5-qt5 FreeBSDQt5.7 - Build # 152 - Fixed!
BUILD SUCCESS Build URL https://build.kde.org/job/Frameworks%20plasma-framework%20kf5-qt5%20FreeBSDQt5.7/152/ Project: Frameworks plasma-framework kf5-qt5 FreeBSDQt5.7 Date of build: Sat, 25 Nov 2017 22:20:05 + Build duration: 1 hr 1 min and counting JUnit Tests Name: (root) Failed: 1 test(s), Passed: 13 test(s), Skipped: 0 test(s), Total: 14 test(s)Failed: TestSuite.plasma-packagestructuretest
what happens when calling KDirWatch::addDir() multiple times with the same path?
Hi, KDirWatch is mostly rather old code, does anyone really know why it does what it does? For instance, watching a directory multiple times with KDW::addDir() results in as many "Added already watched Entry" debug messages but I can't get my head around the underlying goal. It's also not clear (as in undocumented) whether you'd need to call KDW::removeDir() the same number of times in order to stop watching the directory in question (I'm assuming you have to). As a result I feel I need to wrap each call to KDW::addDir(foo) with a check of KDW::contains(foo). Thanks! René
D8983: Properly parse dates in cookies when running in non-English locale
schwab added a reviewer: Frameworks. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8983 To: schwab, dfaure, #frameworks Cc: #frameworks
KDE CI: Frameworks plasma-framework kf5-qt5 WindowsMSVCQt5.9 - Build # 67 - Fixed!
BUILD SUCCESS Build URL https://build.kde.org/job/Frameworks%20plasma-framework%20kf5-qt5%20WindowsMSVCQt5.9/67/ Project: Frameworks plasma-framework kf5-qt5 WindowsMSVCQt5.9 Date of build: Sat, 25 Nov 2017 22:20:05 + Build duration: 46 min and counting JUnit Tests Name: (root) Failed: 8 test(s), Passed: 4 test(s), Skipped: 0 test(s), Total: 12 test(s)Failed: TestSuite.coronatestFailed: TestSuite.plasma-configmodeltestFailed: TestSuite.plasma-dialogstatetestFailed: TestSuite.plasma-fallbackpackagetestFailed: TestSuite.plasma-iconitemtestFailed: TestSuite.plasma-packagestructuretestFailed: TestSuite.plasma-storagetestFailed: TestSuite.plasma-themetest
KDE CI: Frameworks plasma-framework kf5-qt5 SUSEQt5.10 - Build # 6 - Fixed!
BUILD SUCCESS Build URL https://build.kde.org/job/Frameworks%20plasma-framework%20kf5-qt5%20SUSEQt5.10/6/ Project: Frameworks plasma-framework kf5-qt5 SUSEQt5.10 Date of build: Sat, 25 Nov 2017 22:20:05 + Build duration: 10 min and counting JUnit Tests Name: (root) Failed: 7 test(s), Passed: 8 test(s), Skipped: 0 test(s), Total: 15 test(s)Failed: TestSuite.dialognativetestFailed: TestSuite.plasma-configmodeltestFailed: TestSuite.plasma-dialogqmltestFailed: TestSuite.plasma-fallbackpackagetestFailed: TestSuite.plasma-iconitemtestFailed: TestSuite.plasma-packagestructuretestFailed: TestSuite.plasma-storagetest Cobertura Report Project Coverage Summary Name PackagesFilesClassesLinesConditionalsCobertura Coverage Report33% (6/18)35% (56/159)35% (56/159)27% (3562/13213)19% (1971/10379)Coverage Breakdown by Package Name FilesClassesLinesConditionalsautotests85% (22/26)85% (22/26)54% (609/1130)28% (419/1474)src.declarativeimports.calendar0% (0/11)0% (0/11)0% (0/448)0% (0/237)src.declarativeimports.core22% (4/18)22% (4/18)12% (251/2130)7% (96/1416)src.declarativeimports.plasmacomponents0% (0/9)0% (0/9)0% (0/522)0% (0/214)src.declarativeimports.plasmaextracomponents0% (0/5)0% (0/5)0% (0/44)0% (0/27)src.declarativeimports.platformcomponents0% (0/4)0% (0/4)0% (0/60)0% (0/14)src.declarativeimports.platformcomponents.utils0% (0/2)0% (0/2)0% (0/15)0% (0/4)src.plasma55% (12/22)55% (12/22)41% (1441/3486)28% (826/2899)src.plasma.packagestructure0% (0/7)0% (0/7)0% (0/141)0% (0/14)src.plasma.private46% (11/24)46% (11/24)42% (673/1614)28% (316/1115)src.plasma.scripting0% (0/3)0% (0/3)0% (0/161)0% (0/130)src.plasmapkg0% (0/1)0% (0/1)0% (0/45)0% (0/40)src.plasmaquick50% (6/12)50% (6/12)28% (557/1997)18% (309/1703)src.plasmaquick.private33% (1/3)33% (1/3)28% (31/110)36% (5/14)src.scriptengines.qml.plasmoid0% (0/6)0% (0/6)0% (0/1152)0% (0/1054)tests.dpi0% (0/2)0% (0/2)0% (0/22)0% (0/2)tests.kplugins0%
KDE CI: Frameworks plasma-framework kf5-qt5 SUSEQt5.7 - Build # 9 - Fixed!
BUILD SUCCESS Build URL https://build.kde.org/job/Frameworks%20plasma-framework%20kf5-qt5%20SUSEQt5.7/9/ Project: Frameworks plasma-framework kf5-qt5 SUSEQt5.7 Date of build: Sat, 25 Nov 2017 22:20:05 + Build duration: 5 min 56 sec and counting JUnit Tests Name: (root) Failed: 7 test(s), Passed: 8 test(s), Skipped: 0 test(s), Total: 15 test(s)Failed: TestSuite.dialognativetestFailed: TestSuite.plasma-configmodeltestFailed: TestSuite.plasma-dialogqmltestFailed: TestSuite.plasma-fallbackpackagetestFailed: TestSuite.plasma-iconitemtestFailed: TestSuite.plasma-packagestructuretestFailed: TestSuite.plasma-storagetest Cobertura Report Project Coverage Summary Name PackagesFilesClassesLinesConditionalsCobertura Coverage Report33% (6/18)35% (56/159)35% (56/159)27% (3561/13209)19% (1970/10379)Coverage Breakdown by Package Name FilesClassesLinesConditionalsautotests85% (22/26)85% (22/26)54% (609/1130)28% (419/1474)src.declarativeimports.calendar0% (0/11)0% (0/11)0% (0/448)0% (0/237)src.declarativeimports.core22% (4/18)22% (4/18)12% (250/2126)7% (95/1412)src.declarativeimports.plasmacomponents0% (0/9)0% (0/9)0% (0/522)0% (0/214)src.declarativeimports.plasmaextracomponents0% (0/5)0% (0/5)0% (0/44)0% (0/27)src.declarativeimports.platformcomponents0% (0/4)0% (0/4)0% (0/60)0% (0/14)src.declarativeimports.platformcomponents.utils0% (0/2)0% (0/2)0% (0/15)0% (0/4)src.plasma55% (12/22)55% (12/22)41% (1441/3486)28% (826/2903)src.plasma.packagestructure0% (0/7)0% (0/7)0% (0/141)0% (0/14)src.plasma.private46% (11/24)46% (11/24)42% (673/1614)28% (316/1115)src.plasma.scripting0% (0/3)0% (0/3)0% (0/161)0% (0/130)src.plasmapkg0% (0/1)0% (0/1)0% (0/45)0% (0/40)src.plasmaquick50% (6/12)50% (6/12)28% (557/1997)18% (309/1703)src.plasmaquick.private33% (1/3)33% (1/3)28% (31/110)36% (5/14)src.scriptengines.qml.plasmoid0% (0/6)0% (0/6)0% (0/1152)0% (0/1054)tests.dpi0% (0/2)0% (0/2)0% (0/22)0% (0/2)tests.kplugins0%
D8992: Warn about errors when parsing json files
mpyne accepted this revision. This revision is now accepted and ready to land. REPOSITORY R244 KCoreAddons BRANCH master REVISION DETAIL https://phabricator.kde.org/D8992 To: apol, #frameworks, mpyne
D8999: KJob: add start(int delay) method
dfaure added a comment. -2 There *is* already API for this: `QTimer::singleShot(delay, job, ::start)` REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D8999 To: rjvbb, dfaure Cc: apol, anthonyfieroni, #frameworks
D9001: Better handle of subjobs
anthonyfieroni added a comment. It should be two different patches, one in KIO and one in KCoreAddons. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D9001 To: jtamate, #frameworks, dfaure Cc: anthonyfieroni, ltoscano
D8999: KJob: add start(int delay) method
rjvbb added a comment. I'm not convinced that there's much that can be done in the example I gave to make it smarter and more capable to predict random events, but that's not the point here. It's just an explicit example I had at hand. I proposed this addition because it's a tiny convenience that I think can be useful in other situations as well. In fact it surprised me a bit that there wasn't already an API for spawning an async job after a delay. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D8999 To: rjvbb, dfaure Cc: apol, anthonyfieroni, #frameworks
D9001: Better handle of subjobs
dfaure added a comment. Phabricator usage: you can't upload one patch for two git repos into a single change request. This makes the "repo" field wrong, and it makes it impossible to get context. Use `arc diff` and create two review requests (unfortunately). Commit log: something like "fix crash during file copy after message box warning" would already be better than either just a bug number (v1) or a very generic sentence (v2). The bug looks like a nested event loop issue, which is always nasty. I don't have time right now for full debugging, but if you could paste a backtrace of where a messagebox is shown that would be helpful. I suppose it all comes from the slave calling warning? Warnings have never been really handled well... Maybe we should collect warnings and show them at the end of the job? This might help with these issues... KCompositeJob patch: that one looks ok, it reads like basic input validation, even though I would have never thought this could happen (again, nested event loops is most certainly the reason...) KIO patch: If there are still any subjobs running, we should kill them rather than patiently wait for them to finish (it's pointless, if the whole job is being aborted). Which subjobs are still there, in fact? This code already kills the put job if the get job has an error, and vice-versa. Are there more jobs that should be killed in that code? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D9001 To: jtamate, #frameworks, dfaure Cc: ltoscano
D8999: KJob: add start(int delay) method
anthonyfieroni added inline comments. INLINE COMMENTS > rjvbb wrote in kjob.cpp:98 > In fact, are you sure that KJob::start won't be ambiguous? Trying to compile > this with clang++ I get > > src/lib/jobs/kjob.cpp:98:5: error: no matching function for call to > 'singleShot' > QTimer::singleShot(delay, this, ::start); > ^~ > > /opt/local/libexec/qt5/Library/Frameworks/QtCore.framework/Headers/qtimer.h:83:17: > note: candidate function not viable: no overload of 'start' matching 'const > char *' for 3rd argument > static void singleShot(int msec, const QObject *receiver, const char > *member); > ^ > > /opt/local/libexec/qt5/Library/Frameworks/QtCore.framework/Headers/qtimer.h:187:17: > note: candidate function not viable: no known conversion from 'int' to > 'std::chrono::milliseconds' (aka 'duration') for 1st > argument > static void singleShot(std::chrono::milliseconds value, const QObject > *receiver, const char *member) > ^ > > /opt/local/libexec/qt5/Library/Frameworks/QtCore.framework/Headers/qtimer.h:102:24: > note: candidate template ignored: couldn't infer template argument 'Func1' > static inline void singleShot(Duration interval, const typename > QtPrivate::FunctionPointer::Object *receiver, Func1 slot) > ^ > > /opt/local/libexec/qt5/Library/Frameworks/QtCore.framework/Headers/qtimer.h:130:13: > note: candidate template ignored: couldn't infer template argument 'Func1' > singleShot(Duration interval, Qt::TimerType timerType, Func1 > slot) > ^ > > /opt/local/libexec/qt5/Library/Frameworks/QtCore.framework/Headers/qtimer.h:138:13: > note: candidate template ignored: couldn't infer template argument 'Func1' > singleShot(Duration interval, QObject *context, Func1 slot) > ^ > > /opt/local/libexec/qt5/Library/Frameworks/QtCore.framework/Headers/qtimer.h:107:24: > note: candidate function template not viable: requires 4 arguments, but 3 > were provided > static inline void singleShot(Duration interval, Qt::TimerType > timerType, const typename QtPrivate::FunctionPointer::Object *receiver, > ^ > > /opt/local/libexec/qt5/Library/Frameworks/QtCore.framework/Headers/qtimer.h:123:13: > note: candidate function template not viable: requires 2 arguments, but 3 > were provided > singleShot(Duration interval, Func1 slot) > ^ > > /opt/local/libexec/qt5/Library/Frameworks/QtCore.framework/Headers/qtimer.h:145:13: > note: candidate function template not viable: requires 4 arguments, but 3 > were provided > singleShot(Duration interval, Qt::TimerType timerType, QObject > *context, Func1 slot) > ^ > > /opt/local/libexec/qt5/Library/Frameworks/QtCore.framework/Headers/qtimer.h:84:17: > note: candidate function not viable: requires 4 arguments, but 3 were > provided > static void singleShot(int msec, Qt::TimerType timerType, const QObject > *receiver, const char *member); > ^ > > /opt/local/libexec/qt5/Library/Frameworks/QtCore.framework/Headers/qtimer.h:193:17: > note: candidate function not viable: requires 4 arguments, but 3 were > provided > static void singleShot(std::chrono::milliseconds value, Qt::TimerType > timerType, const QObject *receiver, const char *member) > ^ > 1 error generated. Oh yes, it ambiguous :) It should use static_cast, if patch is accepted :) REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D8999 To: rjvbb, dfaure Cc: apol, anthonyfieroni, #frameworks
D8999: KJob: add start(int delay) method
apol added a comment. I don't really see the value here. Can't the application developer also do the QTimer gimmick? Something I find myself doing often is the singleShot with 0, maybe simply adding a `startLater()` (à la `deleteLater()`) method would work and be slightly more elegant API. I'm not sure when is it that we'd use a delay != 0. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D8999 To: rjvbb, dfaure Cc: apol, anthonyfieroni, #frameworks
D8999: KJob: add start(int delay) method
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. I don't think it's KJob's job (haha) to be responsible for compressing change notifications, which is basically the use case you're mentioning. This can be done with a QTimer and an associate container of pending changes, on the application side. Such a solution is much more flexible because you can add the data you need (for fast lookups, for more precise decision making etc.) in that container, rather than having to introspect not-started-yet jobs. Not to mention the issue with in-progress jobs. The pipeline here is KDirWatch ---> notification compression ---> actual operations that *should* be triggered = KJob. Misusing KJob as a holder for notification compression seems wrong to me, there are better data structures for that. -1 from me. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D8999 To: rjvbb, dfaure Cc: apol, anthonyfieroni, #frameworks
D8999: KJob: add start(int delay) method
rjvbb set the repository for this revision to R244 KCoreAddons. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D8999 To: rjvbb, dfaure Cc: anthonyfieroni, #frameworks
D8999: KJob: add start(int delay) method
rjvbb updated this revision to Diff 22934. rjvbb added a comment. `int delay` can be negative; prevent warnings from QTimer by calling start() directly when `delay<=0`. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8999?vs=22932=22934 REVISION DETAIL https://phabricator.kde.org/D8999 AFFECTED FILES src/lib/jobs/kjob.cpp src/lib/jobs/kjob.h To: rjvbb, dfaure Cc: anthonyfieroni, #frameworks
D9002: add PKGUILD to bash syntax
jvanderwaa created this revision. jvanderwaa added a reviewer: Framework: Syntax Highlighting. jvanderwaa added a project: Framework: Syntax Highlighting. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY Add pacman's PKGBUILD files as bash syntax highlighted files. PKGBUILDs are used by Arch Linux. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D9002 AFFECTED FILES data/syntax/bash.xml To: jvanderwaa, #framework_syntax_highlighting Cc: #frameworks, genethomas, cullmann, vkrause, dhaumann
D8999: KJob: add start(int delay) method
rjvbb added inline comments. INLINE COMMENTS > rjvbb wrote in kjob.cpp:98 > So KJob::emitSpeed() doesn't use the "old" syntax for compatibility reasons? In fact, are you sure that KJob::start won't be ambiguous? Trying to compile this with clang++ I get src/lib/jobs/kjob.cpp:98:5: error: no matching function for call to 'singleShot' QTimer::singleShot(delay, this, ::start); ^~ /opt/local/libexec/qt5/Library/Frameworks/QtCore.framework/Headers/qtimer.h:83:17: note: candidate function not viable: no overload of 'start' matching 'const char *' for 3rd argument static void singleShot(int msec, const QObject *receiver, const char *member); ^ /opt/local/libexec/qt5/Library/Frameworks/QtCore.framework/Headers/qtimer.h:187:17: note: candidate function not viable: no known conversion from 'int' to 'std::chrono::milliseconds' (aka 'duration') for 1st argument static void singleShot(std::chrono::milliseconds value, const QObject *receiver, const char *member) ^ /opt/local/libexec/qt5/Library/Frameworks/QtCore.framework/Headers/qtimer.h:102:24: note: candidate template ignored: couldn't infer template argument 'Func1' static inline void singleShot(Duration interval, const typename QtPrivate::FunctionPointer::Object *receiver, Func1 slot) ^ /opt/local/libexec/qt5/Library/Frameworks/QtCore.framework/Headers/qtimer.h:130:13: note: candidate template ignored: couldn't infer template argument 'Func1' singleShot(Duration interval, Qt::TimerType timerType, Func1 slot) ^ /opt/local/libexec/qt5/Library/Frameworks/QtCore.framework/Headers/qtimer.h:138:13: note: candidate template ignored: couldn't infer template argument 'Func1' singleShot(Duration interval, QObject *context, Func1 slot) ^ /opt/local/libexec/qt5/Library/Frameworks/QtCore.framework/Headers/qtimer.h:107:24: note: candidate function template not viable: requires 4 arguments, but 3 were provided static inline void singleShot(Duration interval, Qt::TimerType timerType, const typename QtPrivate::FunctionPointer::Object *receiver, ^ /opt/local/libexec/qt5/Library/Frameworks/QtCore.framework/Headers/qtimer.h:123:13: note: candidate function template not viable: requires 2 arguments, but 3 were provided singleShot(Duration interval, Func1 slot) ^ /opt/local/libexec/qt5/Library/Frameworks/QtCore.framework/Headers/qtimer.h:145:13: note: candidate function template not viable: requires 4 arguments, but 3 were provided singleShot(Duration interval, Qt::TimerType timerType, QObject *context, Func1 slot) ^ /opt/local/libexec/qt5/Library/Frameworks/QtCore.framework/Headers/qtimer.h:84:17: note: candidate function not viable: requires 4 arguments, but 3 were provided static void singleShot(int msec, Qt::TimerType timerType, const QObject *receiver, const char *member); ^ /opt/local/libexec/qt5/Library/Frameworks/QtCore.framework/Headers/qtimer.h:193:17: note: candidate function not viable: requires 4 arguments, but 3 were provided static void singleShot(std::chrono::milliseconds value, Qt::TimerType timerType, const QObject *receiver, const char *member) ^ 1 error generated. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D8999 To: rjvbb, dfaure Cc: anthonyfieroni, #frameworks
D8999: KJob: add start(int delay) method
rjvbb set the repository for this revision to R244 KCoreAddons. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D8999 To: rjvbb, dfaure Cc: anthonyfieroni, #frameworks
D8999: KJob: add start(int delay) method
rjvbb updated this revision to Diff 22932. rjvbb marked 2 inline comments as done. rjvbb edited the summary of this revision. rjvbb added a comment. Updated as requested. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8999?vs=22925=22932 REVISION DETAIL https://phabricator.kde.org/D8999 AFFECTED FILES src/lib/jobs/kjob.cpp src/lib/jobs/kjob.h To: rjvbb, dfaure Cc: anthonyfieroni, #frameworks
D8999: KJob: add start(int delay) method
rjvbb marked 2 inline comments as done. rjvbb added inline comments. INLINE COMMENTS > anthonyfieroni wrote in kjob.cpp:98 > Use new syntax > > QTimer::singleShot(delay, this, ::start); So KJob::emitSpeed() doesn't use the "old" syntax for compatibility reasons? > anthonyfieroni wrote in kjob.h:182 > You cannot add a virtual function it will break ABI compatibility. Duh, that s*kcs ... (I knew about making existing methods virtual). I'm not entirely certain if this has to be a virtual method but I guess there's no other choice. At least this removes any doubts as to whether I should provide a default implementation =) REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D8999 To: rjvbb, dfaure Cc: anthonyfieroni, #frameworks
D9001: Better handle of subjobs
jtamate retitled this revision from "Don't remove a subjob if is not in the list of subjobs and dnoe emitResult if the job still has subjobs.." to "Better handle of subjobs". jtamate edited the summary of this revision. jtamate edited the test plan for this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D9001 To: jtamate, #frameworks, dfaure Cc: ltoscano
D9001: Don't remove a subjob if is not in the list of subjobs and dnoe emitResult if the job still has subjobs..
ltoscano added a comment. Almost :) The line should have only BUG: 364039 and the title should be a bit shorter (think about the usual rules for git commit messages) REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D9001 To: jtamate, #frameworks, dfaure Cc: ltoscano
D9001: Don't remove a subjob if is not in the list of subjobs and dnoe emitResult if the job still has subjobs..
jtamate retitled this revision from "Possible solution to bug 364039" to "Don't remove a subjob if is not in the list of subjobs and dnoe emitResult if the job still has subjobs..". jtamate edited the summary of this revision. jtamate edited the test plan for this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D9001 To: jtamate, #frameworks, dfaure Cc: ltoscano
D9001: Possible solution to bug 364039
ltoscano added a comment. I can't comment on the content of the review, but remember that what you write in the title, the summary and the test plan will become the git commit message. So please change the description to something more descriptive. You can refer to the bug with a single individual line inside the summary like BUG: 364039 REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D9001 To: jtamate, #frameworks, dfaure Cc: ltoscano
D9001: Possible solution to bug 364039
jtamate created this revision. jtamate added reviewers: Frameworks, dfaure. Restricted Application added a project: Frameworks. REVISION SUMMARY This is my first phabricator revision, I hope I'm doing it right. First part of the patch in kcoreaddons. Don't remove a child if it is not in the list of children. Second part in kio. Don't emitResult, that will delete the job, if the job has subjobs. It fixes the crash for me as I was able to reproduce it and I can't. I hope it doesn't introduce any leak. TEST PLAN Using a filesystem where you can not change file rights (for example ntfs), move a file from a ext4 folder to that filesystem. The "can not change permissions" dialog appears, with options to retry or cancel. Press retry, a new dialog saying it can not move the original file because it does not exists appears, press retry more than 3 times, then cancel. Before I got a crash, now I can do this several times without crash. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D9001 AFFECTED FILES src/core/filecopyjob.cpp src/lib/jobs/kcompositejob.cpp To: jtamate, #frameworks, dfaure
D8998: Add FindSeccomp to find-modules
davidk marked 3 inline comments as done. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D8998 To: davidk, graesslin Cc: #frameworks, #build_system
D8998: Add FindSeccomp to find-modules
davidk updated this revision to Diff 22931. davidk added a comment. Remove apparently unneeded version check REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8998?vs=22922=22931 BRANCH master REVISION DETAIL https://phabricator.kde.org/D8998 AFFECTED FILES find-modules/FindSeccomp.cmake To: davidk, graesslin Cc: #frameworks, #build_system
D8999: KJob: add start(int delay) method
anthonyfieroni added inline comments.
INLINE COMMENTS
> kjob.cpp:98
> +{
> +QTimer::singleShot(delay, this, SLOT(start()));
> +}
Use new syntax
QTimer::singleShot(delay, this, ::start);
> kjob.h:182
> + */
> +Q_SCRIPTABLE virtual void start(int delay);
> +
You cannot add a virtual function it will break ABI compatibility.
REPOSITORY
R244 KCoreAddons
REVISION DETAIL
https://phabricator.kde.org/D8999
To: rjvbb, dfaure
Cc: anthonyfieroni, #frameworks
D8998: Add FindSeccomp to find-modules
davidk added inline comments. INLINE COMMENTS > graesslin wrote in FindSeccomp.cmake:50-55 > No idea, that's copy pasted from some other cmake modules. Well, based on the fact that no other find-module includes such a check, I will remove it. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D8998 To: davidk, graesslin Cc: #frameworks, #build_system
D8919: Add explicit AppMenu protocol
graesslin added inline comments. Restricted Application edited projects, added Plasma on Wayland; removed Plasma. INLINE COMMENTS > appmenu.xml:37-38 > + > + > + > + Could you please add "The string must be encoded in latin-1". REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D8919 To: davidedmundson, #plasma Cc: broulik, graesslin, plasma-devel, #frameworks, leezu, ZrenBot, alexeymin, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein
D8998: Add FindSeccomp to find-modules
graesslin added inline comments. INLINE COMMENTS > davidk wrote in FindSeccomp.cmake:50-55 > I'm not sure about this. @graesslin is this nessecarry? Most find-modules > don't check for the CMake version, and we don't do anything special here. No idea, that's copy pasted from some other cmake modules. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D8998 To: davidk, graesslin Cc: #frameworks, #build_system
D8999: KJob: add start(int delay) method
rjvbb created this revision. rjvbb added a reviewer: dfaure. rjvbb added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY I'd like to propose a KJob method that provides a simple API to start asynchronous jobs after a delay. The default implementation would simply call `KJob::start()` via a single shot QTimer. My justification (correct me if I'm wrong...): Consider a cross-platform filemanager that provides a graphical directory representation which is to be kept in sync with the actual contents using a KDirWatch instance. Being cross-platform it has to contend with limitations on its use of KDirWatch and thus watches only directories. If it handles directory (re)loads with individual KJobs it will likely be starting multiple jobs for (re)loading the same folder(s) during disk-intensive operations in the parent directory (e.g. checking out a different git branch). In order to limit the cost and potential side-effects of such concurrent reloads it can keep track of the current job list and cancel all jobs already loading a given folder when a new reload request comes in for that directory. Starting the jobs with a delay should help reducing this cost even more: load requests that are superseded by a new request within the delay will not have cost a significant number of CPU cycles and will not have had the chance to cause side-effects. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D8999 AFFECTED FILES src/lib/jobs/kjob.cpp src/lib/jobs/kjob.h To: rjvbb, dfaure Cc: #frameworks
D8998: Add FindSeccomp to find-modules
davidk added inline comments. INLINE COMMENTS > FindSeccomp.cmake:50-55 > +if(CMAKE_VERSION VERSION_LESS 2.8.12) > +message(FATAL_ERROR "CMake 2.8.12 is required by FindSeccomp.cmake") > +endif() > +if(CMAKE_MINIMUM_REQUIRED_VERSION VERSION_LESS 2.8.12) > +message(AUTHOR_WARNING "Your project should require at least CMake > 2.8.12 to use FindSeccomp.cmake") > +endif() I'm not sure about this. @graesslin is this nessecarry? Most find-modules don't check for the CMake version, and we don't do anything special here. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D8998 To: davidk, graesslin Cc: #frameworks, #build_system
D8998: Add FindSeccomp to find-modules
davidk added a reviewer: graesslin. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D8998 To: davidk, graesslin Cc: #frameworks, #build_system
D8998: Add FindSeccomp to find-modules
davidk created this revision. Restricted Application added projects: Frameworks, Build System. Restricted Application added subscribers: Build System, Frameworks. REVISION SUMMARY This is copied from KScreenlocker, but will be utilized in Baloo too. TEST PLAN - Autotests are working - KScreenlocker and Baloo build with seccomp enabled REPOSITORY R240 Extra CMake Modules BRANCH master REVISION DETAIL https://phabricator.kde.org/D8998 AFFECTED FILES find-modules/FindSeccomp.cmake To: davidk Cc: #frameworks, #build_system
