D29445: [KOpenWithDialog] When pointing at a non-executable file print more meaningful error

2020-05-29 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. I have modified DesktopExecParser in kio commit d1e9325fba37eddb9cb44173edfc1fee122a0c57 to return an

D26113: Places: Use Solid::Device::DisplayName for DisplayRole

2020-05-29 Thread David Faure
dfaure added a comment. Please check that kfileplacesmodeltest and kfileplacesviewtest still pass. (they fail here with baloosearch: stuff for some reason, I didn't investigate; but it passes on CI) REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26113 To: meven,

D29814: Fix segfault on no restart args

2020-05-29 Thread David Faure
dfaure added a comment. Can you explain how to trigger this crash in the first place? Which application triggers it? INLINE COMMENTS > jpalecek wrote in kcrash.cpp:272 > Well I was thinking about `resize()`, then found there wasn't any. However, > if we want to be clear, maybe `args = {

D29814: Fix segfault on no restart args

2020-05-27 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. I'm a bit confused by the bug this is fixing. Surely this doesn't happen to all cases of crashes without autorestart enabled?? Also, it sounds like a null check might be

D29810: Don't use the setenv function after fork

2020-05-27 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R285 KCrash BRANCH for-upstream REVISION DETAIL https://phabricator.kde.org/D29810 To: jpalecek, #frameworks, dfaure Cc: bruns, apol, anthonyfieroni, kde-frameworks-devel, LeGast00n, cblack,

D29809: Don't invoke qstring localized stuff in critical section

2020-05-27 Thread David Faure
dfaure added a comment. Makes sense; just two minor things. INLINE COMMENTS > kcrash.cpp:95 > +#ifdef Q_OS_LINUX > +QByteArray socketpath; > +#endif prepend `static`, it's only used in this file. > kcrash.cpp:662 > > -// Create socket path to transfer ptrace scope and open

D29668: Do not reject icon theme dir with invalid context/type.

2020-05-26 Thread David Faure
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kicontheme.cpp:761 > } else if (tmp.isEmpty()) { > // do nothing. key not required > } else { (OK, if this key is not required then surely

Re: Recent breakage in kwallet

2020-05-26 Thread David Faure
Thanks for the pointer. Pretty sure it's wrong, but I'll reply there. On mardi 26 mai 2020 09:31:26 CEST Marco Martin wrote: > would the recent patch > https://invent.kde.org/frameworks/kwallet/-/merge_requests/1 > fix anything? > > On Sat, May 16, 2020 at 10:40 AM Dav

Re: KDE CI: Frameworks » ktexteditor » kf5-qt5 SUSEQt5.14 - Build # 19 - Still Unstable!

2020-05-25 Thread David Faure
gt; > 88% (15/17) > > 88% (15/17) > > 57% (3782/6599) > > 43% (1661/3880) > > > > src.vimode > > 100% (30/30) > > 100% (30/30) > > 81% (1898/2332) > > 61% (979/1599) > > > > src.vimode.config > > 0% (0/1) > > 0% (0/1) > > 0% (0/120) > > 0% (0/76) > > > > src.vimode.emulatedcommandbar > > 100% (12/12) > > 100% (12/12) > > 98% (896/916) > > 90% (588/652) > > > > src.vimode.modes > > 100% (10/10) > > 100% (10/10) > > 87% (3266/3761) > > 79% (1884/2374) > > > > Links: > > -- > > [1] > > https://build.kde.org/job/Frameworks/job/ktexteditor/job/kf5-qt5%20SUSEQt5 > > .14/19/artifact/abi-compatibility-results.yaml [2] > > https://build.kde.org/job/Frameworks/job/ktexteditor/job/kf5-qt5%20SUSEQt5 > > .14/19/artifact/acc/KF5TextEditor-5.71.0.xml [3] > > https://build.kde.org/job/Frameworks/job/ktexteditor/job/kf5-qt5%20SUSEQt5 > > .14/19/artifact/compat_reports/KF5TextEditor_compat_report.html [4] > > https://build.kde.org/job/Frameworks/job/ktexteditor/job/kf5-qt5%20SUSEQt5 > > .14/19/artifact/logs/KF5TextEditor/5.71.0/log.txt -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5

D28590: Add a QString Solid::Device::displayName, used in Fstab Device for network mounts

2020-05-23 Thread David Faure
dfaure added a comment. @meven you're confusing me with my clone @ervin. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D28590 To: meven, #frameworks, bruns, sitter, dfaure Cc: dfaure, broulik, ervin, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28590: Add a QString Solid::Device::displayName, used in Fstab Device for network mounts

2020-05-23 Thread David Faure
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > fstabdevice.cpp:73 > +if (m_displayName.isEmpty()) { > +QStringList currentMountPoints = > FstabHandling::currentMountPoints(m_device); > +

D28765: KSettings::Dialog: add support for KPluginInfos without a KService

2020-05-20 Thread David Faure
dfaure added a comment. https://invent.kde.org/frameworks/kcmutils/-/merge_requests/2 REPOSITORY R295 KCMUtils REVISION DETAIL https://phabricator.kde.org/D28765 To: dfaure, pino, broulik, mart, davidedmundson, ngraham, svuorela Cc: ahmadsamir, rikmills, wbauer, kossebau, svuorela,

D29738: Fix service file specifying 'Run in terminal' giving an error code 100

2020-05-17 Thread David Faure
dfaure added a comment. (Intrusive) fix is up at https://invent.kde.org/frameworks/kio/-/merge_requests/3 REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D29738 To: marten, #frameworks, dfaure Cc: meven, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29738: Fix service file specifying 'Run in terminal' giving an error code 100

2020-05-17 Thread David Faure
dfaure added a comment. Well spotted. Indeed, when using kioexec (because of the DeleteTemporaryFiles option) we no longer detect non-existing executables. I'll look into fixing this. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D29738 To: marten, #frameworks,

D29558: Add KIO::OpenUrlJob::setShowOpenWithDialog as replacement for KRun::displayOpenWithDialog

2020-05-17 Thread David Faure
dfaure added a comment. Yeah, the full story is that I pushed to git.kde.org, reverted because I changed my mind, then the sysadmins told me I wasn't supposed to push to git.kde.org at all (it was blocked for everyone, but I have sysadmin privileges for historical and bus-factor reasons).

D29558: Add KIO::OpenUrlJob::setShowOpenWithDialog as replacement for KRun::displayOpenWithDialog

2020-05-17 Thread David Faure
dfaure reopened this revision. dfaure added a comment. This revision is now accepted and ready to land. Not committed after all. REPOSITORY R241 KIO BRANCH 2020_05_displayOpenWithDialog REVISION DETAIL https://phabricator.kde.org/D29558 To: dfaure, ahmadsamir, broulik, svuorela Cc:

D29800: Fix URL being passed as argument when launching a .desktop file

2020-05-16 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Thanks for the unittest and fix! Oversight on my side indeed. INLINE COMMENTS > openurljobtest.h:58 > void takeOverAfterMimeTypeFound(); > +void runDeskopFileDirectly(); >

D29782: [StatJob] Make mostLocalUrl ignore remote (ftp, http...etc) URLs

2020-05-16 Thread David Faure
dfaure added a comment. OK, I picked testtrash because kio_trash *is* a :local protocol. If it doesn't use UDS_LOCAL_PATH, at least it means the job will actually go through (no early filtering out). That's at least interesting to test too. Good that http will give no error. That's

D29558: Add KIO::OpenUrlJob::setShowOpenWithDialog as replacement for KRun::displayOpenWithDialog

2020-05-16 Thread David Faure
dfaure added a comment. OK I'm having second thoughts about this. Because of Windows, and because of the case of multiple URLs. There's "displaying an open with dialog because we couldn't find any app, after a left-click on a file" and there's "displaying an open with dialog because

D29558: Add KIO::OpenUrlJob::setShowOpenWithDialog as replacement for KRun::displayOpenWithDialog

2020-05-16 Thread David Faure
dfaure closed this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D29558 To: dfaure, ahmadsamir, broulik, svuorela Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29558: Add KIO::OpenUrlJob::setShowOpenWithDialog as replacement for KRun::displayOpenWithDialog

2020-05-16 Thread David Faure
dfaure updated this revision to Diff 82992. dfaure marked 2 inline comments as done. dfaure added a comment. API docs: remove trailing dot, mention 2 default values REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29558?vs=82358=82992 BRANCH

D29782: [StatJob] Make mostLocalUrl ignore remote (ftp, http...etc) URLs

2020-05-16 Thread David Faure
dfaure added a comment. Can you add a unittest for a KIO::mostLocalUrl() in testtrash.cpp (which is :local, so it should work) and another one in jobtest.cpp for a http URL (e.g. to test which error code we're getting, if any?) + call mostLocalUrl() on a "normal" StatJob in

Re: Request for ktexteditor patch release

2020-05-16 Thread David Faure
gnored after session reopening (Kate, KDevelop) (bug 421375) -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5

Re: Recent breakage in kwallet

2020-05-16 Thread David Faure
age Any neon-specific patches to kwallet? -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5

D29610: [kio_file] Handle renaming file 'A' to 'a' on FAT32 filesystems

2020-05-16 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. OK, at least we'll have moved the code to the right place :-) INLINE COMMENTS > ahmadsamir wrote in file_unix.cpp:1052 > const QByteArray dest1 = "/mnt/fat32/A"; > const QByteArray

D29787: Fix krununittest

2020-05-16 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Let's hope the CI has xterm installed I doubt it. Maybe we'll need to pick "ls" instead, even if that will look weird. REPOSITORY R241 KIO BRANCH l-terminal (branched from master)

D29385: Introduce KIO::OpenUrlJob, a rewrite and replacement for KRun

2020-05-15 Thread David Faure
dfaure added a comment. That's the point, a NFS mount *is* a local URL, so we do use QFile for it. And then it takes forever because the kernel has to talk over a socket to answer us. Yes this is horrendous. I hate network mounts :-) REPOSITORY R241 KIO REVISION DETAIL

D29385: Introduce KIO::OpenUrlJob, a rewrite and replacement for KRun

2020-05-15 Thread David Faure
dfaure added a comment. It's 3 times faster on my local SSD. Now think of a NFS mount on a server from another country REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D29385 To: dfaure, ahmadsamir, broulik, meven, kossebau, davidedmundson, nicolasfella,

Re: Request for ktexteditor patch release

2020-05-15 Thread David Faure
5a36e21473c00a8d30cbea0f150a13f c7b568e75c147161992f8875fe36fb46885bccddb63c22edaf81071583f4204c sources/ktexteditor-5.70.1.tar.xz Please add a description of the bug in www/info/kde-frameworks-5.70.0.php (or give me a patch if you can't push). Also, please make sure to write a unittest for this regression so we catch it next time. -- D

D29767: CopyJob: Check if destination dir is a symlink

2020-05-15 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Thanks for the quick fix and for the test! Just minor issues in comments, feel free to push after fixing. INLINE COMMENTS > jobtest.cpp:626 > +// just the same as

D29738: Fix service file specifying 'Run in terminal' giving an error code 100

2020-05-14 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Nice! In addition to the bugfix, calling resultingArguments() only once is a definite improvement, given everything that happens in there. I didn't realize we were calling it twice.

Re: Request for plasma framework patch release

2020-05-14 Thread David Faure
On jeudi 14 mai 2020 15:24:57 CEST David Faure wrote: > On jeudi 14 mai 2020 13:56:50 CEST David Edmundson wrote: > > We seem to have a common crasher newly introduced in plasma-framework. A > > dozen reports in a few days. > > > > Can I ask fo

Re: Request for plasma framework patch release

2020-05-14 Thread David Faure
; Sorry. Here you go. plasma-framework v5.70.1 bfea43379bc364bc1bb0719d9a1a73a57284214d e28884da2e3389513d96794f71fdfa22a8b4ba62396b7d061b4f18d9b5549d19 sources/plasma-framework-5.70.1.tar.xz -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5

D29610: [kio_file] Handle renaming file 'A' to 'a' on FAT32 filesystems

2020-05-14 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > file_unix.cpp:1074 > > if (::rename(_src.data(), _dest.data())) { > if (auto err = execWithElevatedPrivilege(RENAME, {_src, _dest}, > errno)) { Wouldn't it be enough to just call QFile::rename here? The whole idea is: if

D29610: [kio_file] Handle renaming file 'A' to 'a' on FAT32 filesystems

2020-05-13 Thread David Faure
dfaure requested changes to this revision. This revision now requires changes to proceed. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D29610 To: ahmadsamir, #frameworks, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29610: [kio_file] Handle renaming file 'A' to 'a' on FAT32 filesystems

2020-05-13 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > ahmadsamir wrote in copyjob.cpp:1965 > Good point, I missed that the original code was checking the file names and > the paths with the same compare() call. > > About /dir/file and /DIR/FILE, the parent dirs is actually one dir since > "dir" and

D29708: Introduce EXCLUDE_DEPRECATED_BEFORE_AND_AT

2020-05-13 Thread David Faure
dfaure added a comment. > there are lots of KRun::run* usages in non-deprecated API sadly in KIO :( OK, I'll fix that. REPOSITORY R241 KIO BRANCH excludedeprecated REVISION DETAIL https://phabricator.kde.org/D29708 To: kossebau, #frameworks, dfaure, meven, ahmadsamir Cc:

Re: kcoreaddons_add_plugin executable/plugin directory conflict

2020-05-11 Thread David Faure
path, it has less priority than QT_PLUGIN_PATH, so it was picking up my installed plugins instead of the locally built ones. So now modules/ECMAddTests.cmake sets the QT_PLUGIN_PATH env var for when running tests (https://phabricator.kde.org/D8660). Therefore we could indeed move all plugins to a subdir. T

D27203: Don't try to open files we can't figure out where they are

2020-05-11 Thread David Faure
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kdeplatformfiledialoghelper.cpp:226 > +//passing non-local files as the working directory is not supported. > +//See

D26066: [KProcessInfo] Add parent PID

2020-05-11 Thread David Faure
dfaure added a comment. Yep, seems fine to me. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D26066 To: broulik, #frameworks, davidedmundson Cc: meven, dfaure, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29668: Do not reject icon theme dir with invalid context/type.

2020-05-11 Thread David Faure
dfaure added a comment. What are the values of Context and Type? "Legacy" and "UI" ? I can't see anything in index.theme https://ftp.gnome.org/pub/GNOME/sources/adwaita-icon-theme/3.36/ REPOSITORY R302 KIconThemes REVISION DETAIL https://phabricator.kde.org/D29668 To:

Re: Fwd: KDE CI: Administration » Dependency Build Calligra stable-kf5-qt5 WindowsMSVCQt5.14 - Build # 24 - Still Failing!

2020-05-10 Thread David Faure
On Sunday, May 10, 2020 11:53:31 AM CEST Ben Cooksley wrote: > Can someone investigate the below please? Fixed by Ahmad today in https://phabricator.kde.org/D29599 -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5

D29610: [CopyJob] Use stricter conditions when using QFile::rename in slotResultRenaming

2020-05-10 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. OK for now, to fix the unittests. The *real* fix however is to use QFile::rename in kio_file so that this failure to rename doesn't even happen in the first place. In this commit can

D28760: KSettings::Dialog: avoid duplicate entries due cascading $XDG_DATA_DIRS

2020-05-10 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > svuorela wrote in dialog.cpp:317 > I think I read once that whenever you used a ordered map over an unordered > map, you need to justify it by talking to your manager about it. But that's > also a bit from the bucket of nitpickery unless we are

D28760: KSettings::Dialog: avoid duplicate entries due cascading $XDG_DATA_DIRS

2020-05-10 Thread David Faure
dfaure closed this revision. REPOSITORY R295 KCMUtils REVISION DETAIL https://phabricator.kde.org/D28760 To: dfaure, apol, broulik, davidedmundson, kossebau, svuorela Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29547: KRun: deprecate all static 'run*' methods, with full porting instructions

2020-05-10 Thread David Faure
dfaure closed this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D29547 To: dfaure, ahmadsamir, broulik, svuorela Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29547: KRun: deprecate all static 'run*' methods, with full porting instructions

2020-05-10 Thread David Faure
dfaure updated this revision to Diff 82453. dfaure marked 2 inline comments as done. dfaure added a comment. take review into account REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29547?vs=82325=82453 BRANCH 2020_05_deprecate_KRun_run_methods REVISION

D29547: KRun: deprecate all static 'run*' methods, with full porting instructions

2020-05-10 Thread David Faure
dfaure marked 2 inline comments as done. dfaure added inline comments. INLINE COMMENTS > ahmadsamir wrote in krun.h:216 > I don't think you want both @deprecated? I did, but Friedrich had a less confusing suggestion: @deprecated since 5.6. Since 5.71 use ApplicationLauncherJob, otherwise

D23875: KCoreDirLister: fix crash when creating new folders from kfilewidget

2020-05-10 Thread David Faure
dfaure added a subscriber: chinmoyr. dfaure added a comment. In D23875#532045 , @dhaumann wrote: > @dfaure: D21197 can be closed / abandoned? Yes, but @chinmoyr has disappeared, it seems? REPOSITORY

D28760: KSettings::Dialog: avoid duplicate entries due cascading $XDG_DATA_DIRS

2020-05-10 Thread David Faure
dfaure added a comment. ping? REPOSITORY R295 KCMUtils REVISION DETAIL https://phabricator.kde.org/D28760 To: dfaure, apol, broulik, davidedmundson, kossebau, svuorela Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29547: KRun: deprecate all static 'run*' methods, with full porting instructions

2020-05-10 Thread David Faure
dfaure added a reviewer: svuorela. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D29547 To: dfaure, ahmadsamir, broulik, svuorela Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29558: Add KIO::OpenUrlJob::setShowOpenWithDialog as replacement for KRun::displayOpenWithDialog

2020-05-10 Thread David Faure
dfaure added a reviewer: svuorela. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D29558 To: dfaure, ahmadsamir, broulik, svuorela Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29599: [CopyJob] Try to fix windows build

2020-05-10 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Bonus points for keeping an eye on CI, I wish everyone did that ;-) REPOSITORY R241 KIO BRANCH l-fix-win-build (branched from master) REVISION DETAIL

D29577: Stop using deprecated methods

2020-05-09 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R320 KIO Extras BRANCH master REVISION DETAIL https://phabricator.kde.org/D29577 To: aacid, dfaure Cc: kossebau, kde-frameworks-devel, kfm-devel, dfaure, waitquietly, azyx, nikolaik, pberestov,

D29361: KCrash: remove debug output which breaks unittests from using ~/.qttest/config for categorized logging

2020-05-09 Thread David Faure
This revision was automatically updated to reflect the committed changes. Closed by commit R285:58afbdf29a83: KCrash: remove debug output which breaks unittests from using ~/.qttest/config… (authored by dfaure). REPOSITORY R285 KCrash CHANGES SINCE LAST UPDATE

D29574: Use KSERVICE_DEPRECATED_VERSION_BELATED

2020-05-09 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R309 KService BRANCH usebelated REVISION DETAIL https://phabricator.kde.org/D29574 To: kossebau, #frameworks, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29573: ECMGenerateExportHeader: add generation of *_DEPRECATED_VERSION_BELATED()

2020-05-09 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. So many things to take care of :-) REPOSITORY R240 Extra CMake Modules BRANCH addbelated REVISION DETAIL https://phabricator.kde.org/D29573 To: kossebau, #frameworks,

D29572: Build with EXCLUDE_DEPRECATED_BEFORE_AND_AT=CURRENT

2020-05-09 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Thanks! REPOSITORY R309 KService BRANCH buildwithoutdeprecated REVISION DETAIL https://phabricator.kde.org/D29572 To: kossebau, #frameworks, dfaure Cc: kde-frameworks-devel,

D29561: Add missing compiler deprecation tag for 5-args KServiceAction constructor

2020-05-09 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R309 KService BRANCH addmissingdeprecationtag REVISION DETAIL https://phabricator.kde.org/D29561 To: kossebau, #frameworks, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham,

D29485: [CopyJob] Check free space for remote urls before copying and other improvements

2020-05-09 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > ahmadsamir wrote in copyjob.cpp:444 > Excellent points; I was scratching my head trying to figure out what use > UDS_LOCAL_PATH is when, while testing with qDebug(), it was _always_ empty, > the only time it wasn't empty was in testtrash unit

D29558: Add KIO::OpenUrlJob::setShowOpenWithDialog as replacement for KRun::displayOpenWithDialog

2020-05-09 Thread David Faure
dfaure created this revision. dfaure added reviewers: ahmadsamir, broulik. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY This will not look up the preferred app or run scripts/executables, it

D29547: KRun: deprecate all static 'run*' methods, with full porting instructions

2020-05-09 Thread David Faure
dfaure added a dependent revision: D29557: Add KIO::OpenUrlJob::setShowOpenWithDialog as replacement for KRun::displayOpenWithDialog. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D29547 To: dfaure, ahmadsamir, broulik Cc: kde-frameworks-devel, LeGast00n, cblack,

D29485: [CopyJob] Check free space for remote urls before copying and other improvements

2020-05-09 Thread David Faure
dfaure accepted this revision. dfaure added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > ahmadsamir wrote in copyjob.cpp:477 > I meant connection to the remote ulr/host; however e.g Dolphin already > reports those connection errors in the status bar when

D29547: KRun: deprecate all static 'run*' methods, with full porting instructions

2020-05-08 Thread David Faure
dfaure created this revision. dfaure added reviewers: ahmadsamir, broulik. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY [Next up, KRun::displayOpenWithDialog] REPOSITORY R241 KIO BRANCH

D29541: KBookmarkMenuTest: extend unittest to cover undeprecated API

2020-05-08 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R294 KBookmarks BRANCH extendunittestfornudeprecated REVISION DETAIL https://phabricator.kde.org/D29541 To: kossebau, #frameworks, nicolasfella, dfaure, ahmadsamir Cc: kde-frameworks-devel,

D29485: [CopyJob] Check free space for remote urls before copying and other improvements

2020-05-08 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. Sorry, I have one last comment about a comment :) INLINE COMMENTS > copyjob.cpp:477 > +// Check available free space for remote urls > +// TODO: find a

D29537: [CopyJob] Get rid of an old TODO and use QFile::rename()

2020-05-08 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Nice! REPOSITORY R241 KIO BRANCH l-qfile-rename (branched from master) REVISION DETAIL https://phabricator.kde.org/D29537 To: ahmadsamir, #frameworks, dfaure Cc:

D29528: [OpenUrlJob] Improve comments/docs

2020-05-08 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH l-late (branched from master) REVISION DETAIL https://phabricator.kde.org/D29528 To: ahmadsamir, #frameworks, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham,

D29485: [CopyJob] Check free space for remote urls before copying and other improvements

2020-05-08 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > copyjob.cpp:430 > +if (!m_privilegeExecutionEnabled && !isWritable) { > +const QUrl dest = m_asMethod ? > m_dest.adjusted(QUrl::RemoveFilename) : m_dest; > +q->setError(ERR_WRITE_ACCESS_DENIED); This is

D29397: KPreviewJob : Support for DeviceRatioPixel

2020-05-08 Thread David Faure
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > previewjob.cpp:173 > + > +void PreviewJob::setDefaultDevicePixelRatio(qreal defaultDevicePixelRatio) { > +s_defaultDevicePixelRatio =

D29528: [OpenUrlJob] Improve comments/docs

2020-05-08 Thread David Faure
dfaure added a comment. Thanks! INLINE COMMENTS > openurljob.cpp:393 > +if (mime.isValid() && mimeName != m_mimeTypeName) { > +m_mimeTypeName =mimeName; > } missing space after '=' > openurljob.cpp:590 > +const QMimeType mimeType =

D19080: Make file overwrite a bit safer

2020-05-08 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > dfaure wrote in jobtest.cpp:1759 > The other problems were fixed, but this test still fails randomly. > > In fact, why are we checking that the dest file already started to be created > when totalSize is emitted? > Surely copying involves looking

D29524: ECMGeneratePriFile: fix for ECM_MKSPECS_INSTALL_DIR being absolute

2020-05-08 Thread David Faure
dfaure closed this revision. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D29524 To: dfaure, cgiboudeaux, vatra, kfunk, apol, vkrause Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, bencreasy, michaelh, ngraham, bruns

D19080: Make file overwrite a bit safer

2020-05-08 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > jobtest.cpp:1759 > +Q_UNUSED(totalSize); > +QCOMPARE(destFileExists, QFile::exists(destPartFile)); > +}); The other problems were fixed, but this test still fails randomly. In fact, why are we checking that the dest file

D29524: ECMGeneratePriFile: fix for ECM_MKSPECS_INSTALL_DIR being absolute

2020-05-08 Thread David Faure
dfaure created this revision. dfaure added reviewers: cgiboudeaux, vatra, kfunk, apol, vkrause. Herald added projects: Frameworks, Build System. Herald added subscribers: kde-buildsystem, kde-frameworks-devel. dfaure requested review of this revision. TEST PLAN works as before for the case

D29485: [CopyJob] Check free space for remote urls before copying and other improvements

2020-05-08 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. Good idea overall. INLINE COMMENTS > copyjob.cpp:430 > +if (!m_privilegeExecutionEnabled && !isWritable) { > +// In copy-as mode, we want to check the

D29274: ECMGeneratePriFile: make the pri files relocatable

2020-05-08 Thread David Faure
dfaure closed this revision. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D29274 To: dfaure, vatra, kfunk, apol, vkrause Cc: ablu, kossebau, kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, bencreasy, michaelh, ngraham, bruns

D29347: KAuthorized: export method to reload restrictions

2020-05-08 Thread David Faure
dfaure added a comment. Good idea, done in https://commits.kde.org/kconfig/8e0f84030cc1d1e517ca75311ed9850ce88fac41 REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D29347 To: dfaure, aacid, apol, mdawson Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack,

D29385: Introduce KIO::OpenUrlJob, a rewrite and replacement for KRun

2020-05-08 Thread David Faure
dfaure closed this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D29385 To: dfaure, ahmadsamir, broulik, meven, kossebau, davidedmundson, nicolasfella, svuorela Cc: feverfew, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29397: KPreviewJob : Support for DeviceRatioPixel

2020-05-06 Thread David Faure
dfaure added a comment. In D29397#664536 , @meven wrote: > In D29397#663800 , @dfaure wrote: > > > Oh, I thought it was sent as an int. But 8 is QImage::Format_ARGB8565_Premultiplied. Did you mean

D29385: Introduce KIO::OpenUrlJob, a rewrite and replacement for KRun

2020-05-06 Thread David Faure
dfaure updated this revision to Diff 82058. dfaure added a comment. -void KIO::OpenUrlJob::setRunFlags(KIO::ApplicationLauncherJob::RunFlags runFlags) +void KIO::OpenUrlJob::setDeleteTemporaryFile(bool b) The more I think about it, the least I like the use of flags here. 1. they

D29445: [KOpenWithDialog] When pointing at a non-executable file print more meaningful error

2020-05-05 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > apol wrote in kopenwithdialog.cpp:1006 > This patch D29170 suggests that > findExecutable won't find non-executables. > > Something's wrong. Right, hence the isEmpty() here. This check passes if 1) it doesn't

D29397: KPreviewJob : Support for DeviceRatioPixel

2020-05-05 Thread David Faure
dfaure added a comment. Oh, I thought it was sent as an int. But 8 is QImage::Format_ARGB8565_Premultiplied. Did you mean 0x80? INLINE COMMENTS > thumbcreator.h:191 > + * @class ThumbCreatorV3 thumbcreator.h > + * @since 5.70 > + */ 5.70 is tagged already REPOSITORY R241 KIO REVISION

D29427: [KBookmarkMenu] Assign m_actionCollection early to prevent crash

2020-05-04 Thread David Faure
dfaure added a comment. Yep, doing it already. REPOSITORY R294 KBookmarks REVISION DETAIL https://phabricator.kde.org/D29427 To: ahmadsamir, #frameworks, dfaure, kossebau, nicolasfella Cc: rikmills, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29427: [KBookmarkMenu] Assign m_actionCollection early to prevent crash

2020-05-04 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R294 KBookmarks BRANCH l-bookmarkmenu-assign-actioncoll (branched from master) REVISION DETAIL https://phabricator.kde.org/D29427 To: ahmadsamir, #frameworks, dfaure, kossebau, nicolasfella Cc:

D29385: Introduce KIO::OpenUrlJob, a rewrite and replacement for KRun

2020-05-04 Thread David Faure
dfaure updated this revision to Diff 81840. dfaure added a comment. Remove local-files fast path. This also showed a small mistake in the handling of error texts (this job isn't a KIO::Job so we need to call buildErrorString ourselves). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D29397: KPreviewJob : Support for DeviceRatioPixel

2020-05-04 Thread David Faure
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > previewjob.cpp:717 > +qreal imgDevicePixelRatio; > +str >> width >> height >> iFormat >> imgDevicePixelRatio; > QImage::Format format

D29385: Introduce KIO::OpenUrlJob, a rewrite and replacement for KRun

2020-05-03 Thread David Faure
dfaure updated this revision to Diff 81830. dfaure marked an inline comment as done. dfaure added a comment. Multiple fixes thanks to review comments; didn't remove the fast path yet (a unittest needs fixup) REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D29385: Introduce KIO::OpenUrlJob, a rewrite and replacement for KRun

2020-05-03 Thread David Faure
dfaure marked 2 inline comments as done. dfaure added inline comments. INLINE COMMENTS > broulik wrote in openurljob.cpp:261 > I know what you always say when I say what I always say, but why not just > always stat/mimetype job? LOL we're like an old couple, the explicit discussion doesn't

D29385: Introduce KIO::OpenUrlJob, a rewrite and replacement for KRun

2020-05-03 Thread David Faure
dfaure added a comment. In D29385#662439 , @feverfew wrote: > Ahhh yes correct. Seeming as these jobs are now async (unlike KRun IIRC?), it isn't a problem that `resultingArguments` makes several blocking calls to the KIOFuse daemon?

D29385: Introduce KIO::OpenUrlJob, a rewrite and replacement for KRun

2020-05-03 Thread David Faure
dfaure added a comment. In D29385#662422 , @feverfew wrote: > Quick question, how does this affect D23384 ? Previously KRun used KIO::DesktopExecParser::resultingArguments() which handled the conversion of

D29385: Introduce KIO::OpenUrlJob, a rewrite and replacement for KRun

2020-05-03 Thread David Faure
dfaure updated this revision to Diff 81814. dfaure marked 2 inline comments as done. dfaure added a comment. Adjusments based on Friedrich's initial review REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29385?vs=81802=81814 BRANCH 2020_05_openurljob

D29385: Introduce KIO::OpenUrlJob, a rewrite and replacement for KRun

2020-05-03 Thread David Faure
dfaure marked 5 inline comments as done. dfaure added a comment. In D29385#662336 , @svuorela wrote: > I've been looking at it for a while, and after trying to decipher the long lambdas, which might have been better as named functions, I think

Re: Adding a patch to 5.70

2020-05-03 Thread David Faure
bff1edb0352028c6b6e47364ba5 sources/plasma-framework-5.70.0.tar.xz -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5

D29385: Introduce KIO::OpenUrlJob, a rewrite and replacement for KRun

2020-05-03 Thread David Faure
dfaure created this revision. dfaure added reviewers: ahmadsamir, broulik, meven, kossebau, davidedmundson, nicolasfella, svuorela. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY - KRun::runUrl()

D29347: KAuthorized: export method to reload restrictions

2020-05-03 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > kossebau wrote in kauthorized.cpp:247 > Please also add a note next to this that it is for unit test, so people are > not wondering about this random KCONFIGCORE_EXPORT and first have to research > commit history. Good idea, done:

D29347: KAuthorized: export method to reload restrictions

2020-05-03 Thread David Faure
dfaure closed this revision. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D29347 To: dfaure, aacid, apol, mdawson Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29371: KMainWindow: remove doc paragraph about KGlobal::ref usage

2020-05-03 Thread David Faure
dfaure added a comment. `src/kmainwindow_p.h:59:QEventLoopLocker locker;` so the refcounting effect is the same as documented, just with a different "backend". REPOSITORY R263 KXmlGui REVISION DETAIL https://phabricator.kde.org/D29371 To: dvratil, dfaure, #frameworks,

D29371: KMainWindow: remove doc paragraph about KGlobal::ref usage

2020-05-02 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. Please don't remove this, but port it to QEventLoopLocker. REPOSITORY R263 KXmlGui REVISION DETAIL https://phabricator.kde.org/D29371 To: dvratil, dfaure, #frameworks,

D29347: KAuthorized: export method to reload restrictions

2020-05-02 Thread David Faure
dfaure added a comment. Yes declaring the function there, as in the code sample shown here. We do the same for internally-exported variables like KSERVICE_EXPORT int ksycoca_ms_between_checks; Qt does the same kind of things. A _p.h header would have to be installed, which we

  1   2   3   4   5   6   7   8   9   10   >