D28302: [KNewFileMenu] Add extension to proposed filename

2020-03-28 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH l-template-proposed-file-name-ext (branched from master) REVISION DETAIL https://phabricator.kde.org/D28302 To: ahmadsamir, #frameworks, dfaure Cc: kde-frameworks-devel, LeGast00n, c

D28371: [KOpenWithDialog] Add generic name from .desktop files as a tooltip

2020-03-28 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH l-kopenwith-tooltip (branched from master) REVISION DETAIL https://phabricator.kde.org/D28371 To: ahmadsamir, #frameworks, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, m

D28302: [KNewFileMenu] Add extension to proposed filename

2020-03-28 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > knewfilemenu.cpp:467 > text = text.trimmed(); // In some languages, there is a space in front > of "...", see bug 268895 > +// add the extension but also use the translations from "Name=" entry, > +// should work with .txt, .html and w

D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

2020-03-28 Thread David Faure
dfaure added a comment. In D28201#635026 , @broulik wrote: > I think we should also have a replacement job for `new KRun(url)` Yes, these launcher jobs are the building blocks for finally being able to turn KRun into a job (KIO::OpenUrlJo

D28302: [KNewFileMenu] Add extension to proposed filename

2020-03-28 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. Thanks for all your work, I'm impressed! INLINE COMMENTS > knewfilemenu.cpp:464 > +// Prompt the user to set the destination filename, and initially display > +// the file

D27951: Allow users to change dropAction to MoveAction through kdeglobals

2020-03-28 Thread David Faure
dfaure added a comment. Note sure how you're testing UDS_* but you need to pass KIO::StatInode to KIO::statDetails for inode and device ID to be filled in. Testcase: apply http://www.davidfaure.fr/2020/uds_device_test.diff then `kioclient5 openProperties ~/.bashrc` REPOSITORY R241 KIO

D28295: Introduce KNotificationJobUiDelegate

2020-03-28 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. Indeed missing @since 5.69 To get the description, just connect to the `description` signal and store it, until the time you need to show the error? INLINE COMMENTS > knotif

D25315: KDirModel: implement showing a root node for the requested URL

2020-03-28 Thread David Faure
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit R241:78329e2eb60d: KDirModel: implement showing a root node for the requested URL (authored by dfaure). CHANGED PRIOR TO CO

D28367: KServiceAction: store parent service

2020-03-27 Thread David Faure
dfaure added a dependent revision: D28268: KDesktopFileActions: port from KRun::run to KIO::ApplicationLauncherJob. REPOSITORY R309 KService REVISION DETAIL https://phabricator.kde.org/D28367 To: dfaure, broulik, davidedmundson Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ng

D28268: KDesktopFileActions: port from KRun::run to KIO::ApplicationLauncherJob

2020-03-27 Thread David Faure
dfaure added a dependency: D28367: KServiceAction: store parent service. REPOSITORY R241 KIO BRANCH kdesktopfileactions REVISION DETAIL https://phabricator.kde.org/D28268 To: dfaure, davidedmundson, apol, broulik Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28268: KDesktopFileActions: port from KRun::run to KIO::ApplicationLauncherJob

2020-03-27 Thread David Faure
dfaure updated this revision to Diff 78710. dfaure edited the summary of this revision. dfaure added a comment. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. Rebase on D28367 , remove KService::Ptr arg REPOSITORY R24

D28367: KServiceAction: store parent service

2020-03-27 Thread David Faure
dfaure created this revision. dfaure added reviewers: broulik, davidedmundson. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY This allows KIO::ApplicationLauncherJob to find the icon and name of th

D28266: KPropertiesDialog: port KRun::run to ApplicationLauncherJob

2020-03-27 Thread David Faure
dfaure added a reviewer: broulik. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D28266 To: dfaure, davidedmundson, broulik Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28285: [ApplicationLauncherJob] Add way to launch KServiceAction

2020-03-27 Thread David Faure
dfaure added a comment. Makes sense. But in many places, having to provide both is problematic. E.g. when the KServiceAction is the value<>() of a QAction. I'll look into storing the KService::Ptr inside the KServiceAction. If this works out, we can remove the KService::Ptr argument fr

D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

2020-03-27 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > commandlauncherjobtest.cpp:99 > + > +void CommandLauncherJobTest::doesNotFailOnNonExistingExecutable() > +{ See this unittest ;-) As the comment says, QProcess works since it manages to start /bin/sh. We don't want to wait until the application/p

D25660: Decouple KBookmarksMenu from KActionCollection

2020-03-27 Thread David Faure
dfaure added a subscriber: mlaurent. dfaure added a comment. @mlaurent AFAICS you removed the deprecation macro completely in master, and left it (with 0x06) in release/20.04? I thought we wanted - to remove it in release/20.04 (or set it to a tested value like 0x053A00) - to s

D25660: Decouple KBookmarksMenu from KActionCollection

2020-03-27 Thread David Faure
dfaure added a comment. As usual, it's the too strong "no deprecated API usage" define in the apps. Fixed. REPOSITORY R294 KBookmarks REVISION DETAIL https://phabricator.kde.org/D25660 To: nicolasfella, #frameworks, dfaure Cc: bcooksley, kossebau, dfaure, apol, kde-frameworks-devel,

D27951: Allow users to change dropAction to MoveAction through kdeglobals

2020-03-27 Thread David Faure
dfaure added a comment. Symlinks are a bit special, we could add a way to ask kio_file not to follow them. But at least it should work for regular files and directories, right? Anyhow, we don't have a kio_file result here so I guess my point is moot. REPOSITORY R241 KIO REVISION

D28285: [ApplicationLauncherJob] Add way to launch KServiceAction

2020-03-26 Thread David Faure
dfaure added a comment. Interestingly, D28268 is also about executing a KServiceAction, but I cannot port it to this API, because the KService::Ptr is not known in that (public) method. The caller, KFileItemActions, knows it so it could pass it to a new

D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

2020-03-26 Thread David Faure
dfaure added a task: T11549: KIO: remove unneeded QWidget dependencies to set parent windows or display errors. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D28201 To: dfaure, apol, davidedmundson, nicolasfella, vkrause, broulik Cc: ahmadsamir, kde-frameworks-devel, LeGas

D28128: Add force save behavior to KEntryMap

2020-03-25 Thread David Faure
dfaure added a comment. I did some debugging. e.bForceSave is set to true at the time of doing the writeEntry("testRestore", "restore") -- or if I split this and create another KConfig instance (I thought this would fail...) it is then set while loading the file (good). The naming

D28285: [ApplicationLauncherJob] Add way to launch KServiceAction

2020-03-25 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D28285 To: broulik, #frameworks, dfaure, davidedmundson Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28285: [ApplicationLauncherJob] Add way to launch KServiceAction

2020-03-25 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. Excellent idea! I'm glad to see that the class design allows these kinds of things. INLINE COMMENTS > applicationlauncherjob.cpp:57 > +d->m_service.detach(); > +if (d

D28266: KPropertiesDialog: port KRun::run to ApplicationLauncherJob

2020-03-24 Thread David Faure
dfaure created this revision. dfaure added a reviewer: davidedmundson. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY KRun::run(exec, urls) is somewhere in between the two jobs. Since we know the n

D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

2020-03-24 Thread David Faure
dfaure closed this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D28201 To: dfaure, apol, davidedmundson, nicolasfella, vkrause, broulik Cc: ahmadsamir, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28264: KIO: rename ProcessLauncherJob to ApplicationLauncherJob

2020-03-24 Thread David Faure
dfaure closed this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D28264 To: dfaure, davidedmundson, broulik Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28264: KIO: rename ProcessLauncherJob to ApplicationLauncherJob

2020-03-24 Thread David Faure
dfaure created this revision. dfaure added reviewers: davidedmundson, broulik. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY Initially I thought it could do the job both for applications (running a

Re: Problems in KWayland causes by API and ABI compatibility promises

2020-03-24 Thread David Faure
asty use-after-free in the testWaylandSurface unittest, which deletes the Registry even though some callback uses it later: https://build.kde.org/job/Frameworks/view/Platform%20-%20SUSEQt5.12/job/kwayland/job/kf5-qt5%20SUSEQt5.12/118/testReport/junit/projectroot.autotests/client/kwayland_testWaylandSurface/

D25495: Fix Sonnet autodetect failing on Indian langs

2020-03-24 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > test_autodetect.cpp:39 > + > +/* These tests will fail if you dont have the respective dictionary > installed. > + * They will also fail if the dictionary file name is different from > 'correct_lang' Is there an explicit way to check

D25495: Fix Sonnet autodetect failing on Indian langs

2020-03-24 Thread David Faure
dfaure added a comment. The new unittest doesn't pass in CI. https://build.kde.org/job/Frameworks/view/Platform%20-%20SUSEQt5.12/job/sonnet/job/kf5-qt5%20SUSEQt5.12/87/testReport/junit/projectroot/autotests/sonnet_test_autodetect/ Please have a look and submit fixes. REPOSITORY R2

D27883: Register spawned applications as an independent cgroups

2020-03-24 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. It would be good to add a link to the spec as a code comment -- but I see that the docu isn't merged yet, so can't be done yet. INLINE COMMENTS > kprocessrunner.cpp:43 > + > +#include > +

D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

2020-03-24 Thread David Faure
dfaure updated this revision to Diff 78407. dfaure added a comment. Add setDesktopName(), excellent idea by davidedmundson. Fix API docs, fix copyright years, fix "do terminate" comment. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28201?vs=78222&id=7840

D28193: [KFontChooser] Use one QFontDatabase object for the ::Private class

2020-03-24 Thread David Faure
dfaure added a comment. QMimeDatabase was modeled after QFontDatabase, so it's not surprising that they both work the same way ;) REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D28193 To: ahmadsamir, #frameworks, cfeck, apol, bport, dfaure Cc: kde-frameworks-

D28122: Copy KFontDialog from KDELibs4Support to KWidgetAddons, now KFontChooserDialog

2020-03-24 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > ahmadsamir wrote in kfontchooserdialog.cpp:97 > hmmm. dfaure knows more about exec() and event loops more than me. > @dfaure WDYT? The theoretical answer is yes, this would crash. But note that the user cannot just close the application by cl

D28193: [KFontChooser] Use one QFontDatabase object for the ::Private class

2020-03-24 Thread David Faure
dfaure added a comment. I don't think this makes any difference, they all share the same underlying private singleton. So all this does is to use a tiny bit more memory while the widget is up (the member var). The additional CPU usage without this patch or the additional memory usage wit

D28223: Add "Stat" prefix to StatDetails Enum entries

2020-03-24 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Thanks. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D28223 To: meven, #frameworks, kossebau, dfaure Cc: davidre, broulik, kde-frameworks-devel, LeG

D27951: Allow users to change dropAction to MoveAction through kdeglobals

2020-03-24 Thread David Faure
dfaure added a comment. OK, I looked at the code more closely and I see now that on different partitions it will still show the menu, rather than make any assumptions. This still creates a risk for surprises, as to whether the menu will appear or not (on Windows one can look at two paths

D28221: Don't write default value to configuration file when default value came from /etc/* file

2020-03-24 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. Note that the docu for KConfigGroup::hasDefault has this logic too: if ( (value == computedDefault) && !group.hasDefault(key) ) group.revertToDefault(key); else

D28253: ECMPoQmToolsTest: have separate moc files for tr_thread_test 1 & 2

2020-03-24 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Thanks! REPOSITORY R240 Extra CMake Modules BRANCH fixtr_thread_tests REVISION DETAIL https://phabricator.kde.org/D28253 To: kossebau, #frameworks, #build_system, dfaure Cc: kde-fra

D28223: Add "Stat" prefix to StatDetails Enum entries

2020-03-24 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > statjob.cpp:106 > { > -KIO::StatDetails detailsFlag = KIO::StatDetail::Basic; > +KIO::StatDetails detailsFlag = KIO::StatDetail::StatBasic; > if (details > 0) { This is a weird way of doing this. A C-style enum is used like KIO::Stat

D28191: [KFontChooser] General code cleanup

2020-03-24 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R236 KWidgetsAddons BRANCH l-kfontchooser (branched from master) REVISION DETAIL https://phabricator.kde.org/D28191 To: ahmadsamir, #frameworks, cfeck, apol, bport, dfaure Cc: kde-frameworks-devel,

D28128: Add force save behavior to KEntryMap

2020-03-24 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > bport wrote in kconfigtest.cpp:1970 > This is a global local file, not system wide and so not considered as default > cf. https://lxr.kde.org/source/frameworks/kconfig/src/core/kconfig.cpp#0702 > if the entry is set system wide > /etc/kde5rc > /etc

D28128: Add force save behavior to KEntryMap

2020-03-23 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. Sounds like you need another feature, not breaking an existing one. INLINE COMMENTS > kconfigtest.cpp:1965 > +QVERIFY(local.sync()); > +QCOMPARE(generalLocal.readEntry("te

D25010: [StatJob] Use A QFlag to specify the details returned by StatJob

2020-03-23 Thread David Faure
dfaure added a comment. Yes, either prefixes or the C++11 way with enum class. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D25010 To: meven, #frameworks, dfaure, kossebau Cc: mlaurent, dalbers, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

2020-03-22 Thread David Faure
dfaure added a comment. (technically, connecting to &KJob::result is missing in the dolphin port, but unlike ProcessLauncherJob, CommandLauncherJob is very unlikely to fail. QProcess will only fail to start if we're out of resources for forking or if `sh` can't be executed... we don't detect

D25660: Decouple KBookmarksMenu from KActionCollection

2020-03-22 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Oops I forgot to review this again after your last changes. Looks fine to me now, but meanwhile it needs s/5.67/5.69/ everywhere (docu and deprecation macros). Feel free to land if yo

D28122: Copy KFontDialog from KDELibs4Support to KWidgetAddons, now KFontChooserDialog

2020-03-22 Thread David Faure
dfaure accepted this revision. REPOSITORY R236 KWidgetsAddons BRANCH l-kfontdlg (branched from master) REVISION DETAIL https://phabricator.kde.org/D28122 To: ahmadsamir, #frameworks, davidedmundson, cfeck, broulik, ervin, meven, bport, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack,

D28188: breeze-icons autotests: skip symlinks in the builddir

2020-03-22 Thread David Faure
dfaure added a comment. Followup commit https://commits.kde.org/breeze-icons/4959eb87534b5676eb996cadffd1c72e33849d13 REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D28188 To: dfaure, sitter, bshah, lbeltrame Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, m

D28184: KIO autotests: repair JobTest::moveDestAlreadyExistsAutoRename

2020-03-22 Thread David Faure
dfaure added a comment. In D28184#632459 , @ahmadsamir wrote: > It was failing locally for me too, both when I had /tmp as tmpfs and when I changed it to be a regular dir on /; /home is a separate partition on my system. So I didn't understand

D28206: Port QList/QSet deprecated methods

2020-03-22 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH l-fromlist (branched from master) REVISION DETAIL https://phabricator.kde.org/D28206 To: ahmadsamir, #frameworks, dfaure, meven Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, mic

D28122: Copy KFontDialog from KDELibs4Support to KWidgetAddons, now KFontChooserDialog

2020-03-22 Thread David Faure
dfaure accepted this revision. dfaure added inline comments. INLINE COMMENTS > kfontchooserdialog.h:81 > + * > + * @param parent The parent widget of the dialog, if any. > + * @param flags Defines how the font chooser is displayed. Do @param need to be in order of the arguments? (I'm

D28188: breeze-icons autotests: skip symlinks in the builddir

2020-03-22 Thread David Faure
dfaure closed this revision. REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D28188 To: dfaure, sitter, bshah, lbeltrame Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

2020-03-22 Thread David Faure
dfaure added a comment. Overview: KRun::runService/runApplication => ProcessLauncherJob KRun::runCommand => CommandLauncherJob new KRun => OpenUrlJob, not written yet Maybe the first one should be renamed to ApplicationLauncherJob, now that I found out it wouldn't be a good place

D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

2020-03-22 Thread David Faure
dfaure added a comment. `Tools/Compare Files` still works in dolphin (it uses KRun::runCommand). http://www.davidfaure.fr/2020/dolphin.diff ports it to CommandLauncherJob, it still works afterwards. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D28201 To: dfaure,

D28187: KIO DesktopExecParser: simplify code

2020-03-22 Thread David Faure
dfaure added a comment. Thanks for the detailed review! REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D28187 To: dfaure, ahmadsamir, feverfew, ngraham Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28187: KIO DesktopExecParser: simplify code

2020-03-22 Thread David Faure
dfaure closed this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D28187 To: dfaure, ahmadsamir, feverfew, ngraham Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28201: Add KIO::CommandLauncherJob to replace KRun::runCommand

2020-03-22 Thread David Faure
dfaure created this revision. dfaure added reviewers: apol, davidedmundson, nicolasfella, vkrause, broulik. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY - Ported runCommand to CommandLauncherJob

D28122: Move/port KFontDialog from KDELibs4Support to KWidgetAddons

2020-03-22 Thread David Faure
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kfontchooserdialog.cpp:38 > + > +void stripRegularStyleName(QFont &font); > +}; Should be static. Even file-static (move the implementation further up in

D28195: Avoid double fetch and temporary hex encoding for NTFS attributes

2020-03-22 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Oh, I see. Not obvious that this is the reason. `for (decltype(length) n = 0; n < length; ++n)` might be more obvious, although maybe also not very common. I'll let you decide which one

D25301: Move createUDSEntry from file.cpp to file_unix.cpp

2020-03-22 Thread David Faure
dfaure added a comment. Fixed in commit bc2b22d2fdf5b4f932 REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D25301 To: meven, #frameworks, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2,

D25301: Move createUDSEntry from file.cpp to file_unix.cpp

2020-03-22 Thread David Faure
dfaure added a comment. Fails to build here file_unix.cpp:384:9: error: ‘appendACLAtoms’ was not declared in this scope I'll investigate. You might want to install the necessary libs for the ACL code to be enabled on your system. REPOSITORY R241 KIO REVISION DETAIL https://phab

Re: KDE CI: Frameworks » kio » kf5-qt5 FreeBSDQt5.13 - Build # 349 - Fixed!

2020-03-22 Thread David Faure
On samedi 21 mars 2020 19:07:13 CET CI System wrote: > BUILD SUCCESS > Build > URL https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.1 \o/ -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5

D28195: Avoid double fetch and temporary hex encoding for NTFS attributes

2020-03-22 Thread David Faure
dfaure added a comment. Indeed the hex roundtrip was very unnecessary, well spotted. INLINE COMMENTS > file_unix.cpp:533 > char *c = strAttr; > -char *e = hexAttr.data(); > -*e++ ='0'; > -*e++ = 'x'; > -for (auto n = 0; n < length; n++, c++) { > -*e++ = digits[(st

D27951: Allow users to change dropAction to MoveAction through kdeglobals

2020-03-22 Thread David Faure
dfaure added a comment. I fully agree with the other David, FWIW. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D27951 To: trmdi, ngraham, dfaure, meven, #vdg, davidedmundson Cc: elvisangelaccio, davidedmundson, meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, mic

D28189: karchivetest: avoid interference from kdebugsettings

2020-03-21 Thread David Faure
dfaure added a comment. Can't see what's weird about that. Logging config comes from a config file stored in QStandardPaths::GenericConfigLocation, seems pretty reasonable to me. REPOSITORY R243 KArchive REVISION DETAIL https://phabricator.kde.org/D28189 To: dfaure, apol, sandsmark Cc:

D14707: autotests: skip '/' fstab check with zfs

2020-03-21 Thread David Faure
dfaure added a comment. Yep, that's exactly what I see in the output from kmountpointtest. One entry for swap, but no entry for "/". So any file on the filesystem doesn't look like it comes from a partition that was mounted at boot time. In other words it looks like everything is from

D28189: karchivetest: avoid interference from kdebugsettings

2020-03-21 Thread David Faure
dfaure closed this revision. REPOSITORY R243 KArchive REVISION DETAIL https://phabricator.kde.org/D28189 To: dfaure, apol, sandsmark Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D21760: Add KListOpenFilesJob

2020-03-21 Thread David Faure
dfaure added a comment. The test fails on FreeBSD, in CI. https://build.kde.org/job/Frameworks/view/Platform%20-%20FreeBSDQt5.13/job/kcoreaddons/job/kf5-qt5%20FreeBSDQt5.13/118/testReport/junit/projectroot/autotests/klistopenfilesjobtest_unix/ Any plans for fixing it, or should I mar

T12173: KService: provide solution to migrate away from KServiceTypeTrader/KMimeTypeTrader for loading plugins and parts

2020-03-21 Thread David Faure
dfaure updated the task description. TASK DETAIL https://phabricator.kde.org/T12173 To: dfaure Cc: #frameworks, nicolasfella, dfaure, mart, davidre, GB_2, ekasprzak, ahmadsamir, ngraham, kpiwowarski, usta, asturmlechner, jucato, cfeck, cgiboudeaux, cullmann, vkrause, cordlandwehr, knauss

D14707: autotests: skip '/' fstab check with zfs

2020-03-21 Thread David Faure
dfaure abandoned this revision. dfaure added a comment. So, this patch is wrong, and I just disabled that part of kmountpointtest on FreeBSD (https://cgit.kde.org/kio.git/commit/?id=e1ef54531ecf7dc9966860dce890201aa100c240) The fact that KMountPoint sees no mountpoints on the FreeBSD CI

D27910: Make sure warning output is enabled before testing if the correct warning is printed

2020-03-21 Thread David Faure
dfaure added a comment. Note that in both cases the env var QT_LOGGING_RULES would break the unittest anyway... so yeah it's only about qtlogging.ini which we can easily skip with test mode. Here's my suggested fix: D28189 @apol Warnings are enab

D28189: karchivetest: avoid interference from kdebugsettings

2020-03-21 Thread David Faure
dfaure created this revision. dfaure added reviewers: apol, sandsmark. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY NO_CHANGELOG TEST PLAN test still passes REPOSITORY R243 KArchive BRANCH

D28188: breeze-icons autotests: skip symlinks in the builddir

2020-03-21 Thread David Faure
dfaure created this revision. dfaure added reviewers: sitter, bshah, lbeltrame. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY It's ok for symlinks in the builddir to be broken. We can't (easily) d

D27614: build: fix the build where install prefix is not user-writable

2020-03-21 Thread David Faure
dfaure added a comment. Nobody cares about broken CI, or my comment here wasn't noticed, so I fixed it myself. https://phabricator.kde.org/D28188 REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D27614 To: bshah, ngraham, lbeltrame, sitter Cc: dfaure, sitter, kde

D28187: KIO DesktopExecParser: simplify code

2020-03-21 Thread David Faure
dfaure created this revision. dfaure added reviewers: ahmadsamir, feverfew, ngraham. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY - Remove special case for hasSchemeHandler, it's just a hack.

D28184: KIO autotests: repair JobTest::moveDestAlreadyExistsAutoRename

2020-03-21 Thread David Faure
dfaure closed this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D28184 To: dfaure, meven, apol Cc: apol, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28186: GIT_SILENT kpasswdserver, break long lines for readability; add braces around if

2020-03-21 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Nice. clang-format can't be configured to add missing braces, and to have a max line length? REPOSITORY R241 KIO BRANCH l-kpasswdserver-long-lines (branched from master) REVISION

D28184: KIO autotests: repair JobTest::moveDestAlreadyExistsAutoRename

2020-03-21 Thread David Faure
dfaure added a comment. They are not the same. (3,4) vs (2,4) REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D28184 To: dfaure, meven, apol Cc: apol, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28169: [krununittest] Skip the test if ktelnetservice5{, .desktop} isn't found

2020-03-21 Thread David Faure
dfaure added a comment. In D28169#631893 , @ahmadsamir wrote: > Yesterday, I did try tweaking XDG_DATA_DIRS to make it find the .desktop file without it being in /usr/share/applications It's more portable to use QStandardPaths test mode

D28184: KIO autotests: repair JobTest::moveDestAlreadyExistsAutoRename

2020-03-21 Thread David Faure
dfaure created this revision. dfaure added a reviewer: meven. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY It failed for me, and for FreeBSD on CI. I guess the Linux CI has it all on the same par

D28169: [krununittest] Skip the test if ktelnetservice5{, .desktop} isn't found

2020-03-21 Thread David Faure
dfaure added a comment. OK, it's fixed now, after 3 commits :-) You can drop this change, it's better that CI actually tests the code. Thanks for your help though (with this, and with everything else - very much appreciated) REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde

D25301: Move createUDSEntry from file.cpp to file_unix.cpp

2020-03-21 Thread David Faure
dfaure accepted this revision. REPOSITORY R241 KIO BRANCH arcpatch-D25301 REVISION DETAIL https://phabricator.kde.org/D25301 To: meven, #frameworks, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28169: [krununittest] Skip the test if ktelnetservice5{, .desktop} isn't found

2020-03-21 Thread David Faure
dfaure added a comment. The bug is that, after the fuse dbus call fails, we can't just blindly go to kioexec. It depends on `supported`. ktelnetservice supports ssh, so we don't need kioexec. Will rework... REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D28169 To: ahma

D28169: [krununittest] Skip the test if ktelnetservice5{, .desktop} isn't found

2020-03-21 Thread David Faure
dfaure added a comment. In D28169#631841 , @ahmadsamir wrote: > In D28169#631809 , @dfaure wrote: > > > Ah, we had tried already to make it work uninstalled. > > > > The actual way to do that wou

D28097: GIT_SILENT clang-format kpasswdserver.*

2020-03-21 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Thanks. Let's do manual changes separately :-) REPOSITORY R241 KIO BRANCH l-kpasswdserver-clang-format (branched from master) REVISION DETAIL https://phabricator.kde.org/D28097 To:

D28169: [krununittest] Skip the test if ktelnetservice5{, .desktop} isn't found

2020-03-21 Thread David Faure
dfaure added a comment. Ah, we had tried already to make it work uninstalled. The actual way to do that would be to copy that desktop file into a local ApplicationsLocation. I'll do that, I just did something similar in KParts. REPOSITORY R241 KIO REVISION DETAIL https://phabricator

Re: KIO: try to assign an icon to action submenus

2020-03-21 Thread David Faure
tion for those submenus, where we would be able to set an icon. C++ problems can be fixed, but not conceptual ones like the one above. I'm dropping my objections. https://phabricator.kde.org/D27950 approved. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5

D27950: Assign an icon to action submenus

2020-03-21 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D27950 To: apol, #frameworks, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D25301: Move createUDSEntry from file.cpp to file_unix.cpp

2020-03-21 Thread David Faure
dfaure accepted this revision. dfaure added inline comments. INLINE COMMENTS > file_unix.cpp:147 > + > +QString getUserName(KUserId uid) > +{ static > file_unix.cpp:164 > + > +QString getGroupName(KGroupId gid) > +{ static > file_unix.cpp:248 > + > +bool createUDSEntry(const QString &filename

D27936: Windows: Add suport for file date creation

2020-03-21 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D27936 To: meven, #frameworks, elvisangelaccio, #windows, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngrah

D28128: Add force save behavior to KEntryMap

2020-03-21 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. I would prefer a unittest based on public API, showing the actual bug (before the fix) with kdeglobals and a local config file. INLINE COMMENTS > kconfigdata.h:76 > +/** > +

D27760: WIP | Added BatchMoveJob

2020-03-21 Thread David Faure
dfaure added a comment. Looks good, just a few things. INLINE COMMENTS > batchmovejobtest.cpp:91 > +job->setUiDelegate(nullptr); > +QSignalSpy spy(job, SIGNAL(fileMoved(QUrl, QUrl))); > + Use newer syntax &KIO::BatchMoveJob::fileMoved? > batchmovejobtest.cpp:104 > +

D27883: Register spawned applications as an independent cgroups

2020-03-21 Thread David Faure
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kprocessrunner.cpp:292 > + > +QDBusMessage message = > QDBusMessage::createMethodCall(QStringLiteral("org.freedesktop.systemd1"), > +

D28097: GIT_SILENT clang-format kpasswdserver.* plus some manual changes

2020-03-21 Thread David Faure
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kpasswdserver.cpp:249 > #ifdef HAVE_KF5WALLET > -if (!result && > -!m_walletDisabled && > -(info.username.isEmpty() || info.pa

D28079: [keditfiletype] Prevent removing the "main" glob pattern for mime types

2020-03-21 Thread David Faure
dfaure added a comment. I wrote QMimeDatabase, and my only intent was to move the main glob to be first. Not to prevent people from removing it. So this is a Qt bug. REPOSITORY R126 KDE CLI Utilities REVISION DETAIL https://phabricator.kde.org/D28079 To: ahmadsamir, #plasma, dfaure, dav

D25315: KDirModel: implement showing a root node for the requested URL

2020-03-21 Thread David Faure
dfaure added a subscriber: rrosch. dfaure added a comment. @rrosch Does this now behave as expected? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D25315 To: dfaure, stefanocrocco, elvisangelaccio, meven, apol Cc: rrosch, ahmadsamir, kde-frameworks-devel, LeGast00n, cbl

D27974: KParts partviewer test app: add action list to switch parts

2020-03-21 Thread David Faure
This revision was automatically updated to reflect the committed changes. Closed by commit R306:e41f42eff6af: KParts partviewer test app: add action list to switch parts (authored by dfaure). Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. REPOSITORY R306 KP

D27966: KParts: add PartLoader as replacement to KMimeTypeTrader for parts

2020-03-21 Thread David Faure
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit R306:b617fc005751: KParts: add PartLoader as replacement to KMimeTypeTrader for parts (authored by dfaure). REPOSITORY R3

D27967: KParts: add unittest for PartLoader, required shuffling things around

2020-03-21 Thread David Faure
This revision was automatically updated to reflect the committed changes. Closed by commit R306:d0665603ea33: KParts: add unittest for PartLoader, required shuffling things around (authored by dfaure). Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. CHANGED PR

D17816: Support for xattrs on kio copy/move

2020-03-21 Thread David Faure
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > cochise wrote in jobtest.cpp:118 > The check is to build infrastructure for anyone with access to the platforms > that want to add their test. > Not sure if t

<    1   2   3   4   5   6   7   8   9   10   >