D29397: KPreviewJob : Support for DeviceRatioPixel

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

D29347: KAuthorized: export method to reload restrictions

dfaure edited the summary of 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

D29210: [NestedListHelper] Fix indentation of selection, add tests

dfaure accepted this revision. This revision is now accepted and ready to land. BRANCH change-indent (branched from master) REVISION DETAIL https://phabricator.kde.org/D29210 To: poboiko, #frameworks, dfaure, mlaurent Cc: kde-frameworks-devel

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

dfaure created this revision. dfaure added reviewers: kossebau, davidedmundson. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY kdebugsettings --test-mode allows to tune the categorized logging for

D29208: [NestedListHelper] Improve indentation code

dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R310 KTextWidgets BRANCH unused (branched from master) REVISION DETAIL https://phabricator.kde.org/D29208 To: poboiko, #frameworks, dfaure, mlaurent Cc: kde-frameworks-devel, LeGast00n, cblack,

D29208: [NestedListHelper] Improve indentation code

dfaure added inline comments. INLINE COMMENTS > poboiko wrote in nestedlisthelper.cpp:87 > That's the current block being checked, not the next one. I've just checked > to be sure, last block can be unindented :) > > TBH, I don't really know if it's even possible for the current block to be >

D29347: KAuthorized: export method to reload restrictions

dfaure created this revision. dfaure added reviewers: aacid, apol, mdawson. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY This is useful for unittests. Example: KCONFIGCORE_EXPORT void

D29096: Prefix includes and libs dir with QT_SYSROOT

dfaure added a comment. I'm using conan, not doing cross compilation. But with relative paths it's no problem, both use cases work. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D29096 To: ablu, #build_system, apol, vkrause, kfunk Cc: dfaure,

D29208: [NestedListHelper] Improve indentation code

dfaure added inline comments. INLINE COMMENTS > nestedlisthelper.cpp:87 > +QTextBlock nextBlock = block.next(); > +if (!block.isValid()) { > +return false; So the last block cannot be unindented? How come? REPOSITORY R310 KTextWidgets REVISION DETAIL

Re: Having $(framework)_global.h headers (was: Re: building KF5 projects against a different Qt5 version [...])

On lundi 27 avril 2020 14:28:42 CEST Friedrich W. H. Kossebau wrote: > Am Sonntag, 26. April 2020, 16:12:31 CEST schrieb Friedrich W. H. Kossebau: > > Am Sonntag, 26. April 2020, 15:46:35 CEST schrieb David Faure: > > > On Sunday, April 26, 2020 3:21:34 PM CEST René

D29096: Prefix includes and libs dir with QT_SYSROOT

dfaure added a comment. Thanks for the quick test! I'll indeed proceed with the other one, which is more generic (our use case isn't related to QT_SYSROOT). Actually: can you add your comment (that it works for you) in the other review request, so people know it's been tested by

D29274: ECMGeneratePriFile: make the pri files relocatable

dfaure updated this revision to Diff 81519. dfaure added a comment. Fix error: regex "[^/]*" matched an empty string. REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29274?vs=81506=81519 BRANCH master REVISION DETAIL

D29096: Prefix includes and libs dir with QT_SYSROOT

dfaure added a comment. Hi @ablu, Could you test if my patch in D29274 solves your problem? By making these lines relative to the location of the .pri file, it should work very well in a sysroot context as well. REPOSITORY R240 Extra CMake Modules

D29274: ECMGeneratePriFile: make the pri files relocatable

dfaure created this revision. dfaure added reviewers: vatra, kfunk, apol. Herald added projects: Frameworks, Build System. Herald added subscribers: kde-buildsystem, kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY Instead of generating QT.KArchive.includes =

D29251: API dox: use ulong typedef with Q_PROPERTY(percent) to avoid doxygen bug

dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Yes indeed, for sure "int" would be enough. REPOSITORY R244 KCoreAddons BRANCH fixkjobpercentdocs REVISION DETAIL https://phabricator.kde.org/D29251 To: kossebau, #frameworks,

D29153: Move handling of untrusted programs to ApplicationLauncherJob.

dfaure added inline comments. INLINE COMMENTS > broulik wrote in applicationlauncherjob.cpp:155 > With this it starts to look as hard to follow as KRun :) Not even close :-) (OpenUrlJob will be more complicated, somewhere in between this one and KRun...) REPOSITORY R241 KIO BRANCH

D29153: Move handling of untrusted programs to ApplicationLauncherJob.

This revision was automatically updated to reflect the committed changes. Closed by commit R241:f0ae038490e6: Move handling of untrusted programs to ApplicationLauncherJob. (authored by dfaure). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29153?vs=81173=81472

D29153: Move handling of untrusted programs to ApplicationLauncherJob.

dfaure added a comment. @broulik ping? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D29153 To: dfaure, ahmadsamir, broulik, ngraham, mdlubakowski Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29206: Move check for invalid service from KDesktopFileActions to ApplicationLauncherJob

dfaure closed this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D29206 To: dfaure, ahmadsamir, broulik Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29170: Detect executables without +x permission in $PATH to improve error message

dfaure closed this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D29170 To: dfaure, ahmadsamir Cc: ngraham, meven, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns

D29239: [KPlotting] foreach is gone, build with -DQT_NO_FOREACH

dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R277 KPlotting BRANCH l-no-foreach (branched from master) REVISION DETAIL https://phabricator.kde.org/D29239 To: ahmadsamir, #frameworks, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack,

D29170: Detect executables without +x permission in $PATH to improve error message

dfaure added a comment. In D29170#658605 , @ngraham wrote: > What does this mean for AppImages? They are not desktop files, they don't come into this code path (which takes a KService as input). Clicking on a +x AppImage asks for

D29233: [Solid] Port foreach to range/index for

dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R245 Solid BRANCH l-foreach-3 (branched from master) REVISION DETAIL https://phabricator.kde.org/D29233 To: ahmadsamir, #frameworks, dfaure, bruns, meven Cc: kde-frameworks-devel, LeGast00n,

D29229: [KPlotting] Port foreach (deprecated) to range for

dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R277 KPlotting BRANCH l-foreach (branched from master) REVISION DETAIL https://phabricator.kde.org/D29229 To: ahmadsamir, #frameworks, dfaure, apol, meven Cc: kde-frameworks-devel, LeGast00n,

D29221: [FakeCdrom] Add a new UnknownMediumType enumerator to MediumType

dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R245 Solid BRANCH l-mediumtypes (branched from master) REVISION DETAIL https://phabricator.kde.org/D29221 To: ahmadsamir, #frameworks, dfaure, meven, bruns Cc: kde-frameworks-devel, LeGast00n,

D29170: Detect executables without +x permission in $PATH to improve error message

dfaure updated this revision to Diff 81377. dfaure added a comment. Remove unused QFileInfo include REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29170?vs=81376=81377 BRANCH 2020_04_findExecutable REVISION DETAIL https://phabricator.kde.org/D29170

D29216: Remove dead code since KF5.0: mount/umount devices in their contextmenu

dfaure closed this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D29216 To: dfaure, apol, broulik Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29170: Detect executables without +x permission in $PATH to improve error message

dfaure updated this revision to Diff 81376. dfaure added a comment. Remove QDir::current REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29170?vs=81275=81376 BRANCH 2020_04_findExecutable REVISION DETAIL https://phabricator.kde.org/D29170 AFFECTED FILES

D29170: Detect executables without +x permission in $PATH to improve error message

dfaure added inline comments. INLINE COMMENTS > ahmadsamir wrote in kprocessrunner.cpp:65 > I guess there is no need to use QDir::current() any more. Good point, removing. REPOSITORY R241 KIO BRANCH 2020_04_findExecutable REVISION DETAIL https://phabricator.kde.org/D29170 To: dfaure,

D29206: Move check for invalid service from KDesktopFileActions to ApplicationLauncherJob

dfaure added inline comments. INLINE COMMENTS > ahmadsamir wrote in kprocessrunner.cpp:55 > IMHO, the word "entry" here is confusing, the first thing that comes to mind > is that an entry (i.e. a line) in the file is invalid. I see what you're saying. It's the name of the spec, though.

D29170: Detect executables without +x permission in $PATH to improve error message

dfaure added a comment. In D29170#658058 , @meven wrote: > Is it not sufficient to fix bug 415567 ? In which case replace in commit comment CCBUG by BUG No, that bug has two issues. "Program not found" and "Missing lib" -- which is

D29219: [KFontChooser] Remove NoFixedCheckBox DisplayFlag, redundant

dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R236 KWidgetsAddons BRANCH l-remove-redundant-displayflag (branched from master) REVISION DETAIL https://phabricator.kde.org/D29219 To: ahmadsamir, #frameworks, dfaure, cfeck, bport Cc:

D29210: [NestedListHelper] Fix indentation of selection, add tests

dfaure added inline comments. INLINE COMMENTS > nestedlisthelper.cpp:225 > +if (delta > 0) { > +// No list, we're increasing indentation -> create a new one > +listFmt.setStyle(QTextListFormat::ListDisc); should the opposite happen? delete the list

Re: KF 5 & C++14?

4.8 to build the current version of KF5. One way to find out is to... do nothing. Leave the above as is and see if anyone actually complains that it doesn't match our promise. This isn't however a green light for doing this everywhere, not until we wait multiple months for feedback. Otherwise

D29216: Remove dead code since KF5.0: mount/umount devices in their contextmenu

dfaure created this revision. dfaure added reviewers: apol, broulik. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY This code was disabled in preparation for KF 5.0 to remove kio's dependency on

D29206: Move check for invalid service from KDesktopFileActions to ApplicationLauncherJob

dfaure updated this revision to Diff 81281. dfaure retitled this revision from "Move check for invalid service from KDesktopFileActions to ApplicationLauncherJobTest" to "Move check for invalid service from KDesktopFileActions to ApplicationLauncherJob". dfaure added a comment. fix wrong

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

dfaure added a reviewer: svuorela. 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

D29170: Detect executables without +x permission in $PATH to improve error message

dfaure updated this revision to Diff 81275. dfaure added a comment. Remove support for relative executables, as discussed REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29170?vs=81228=81275 BRANCH 2020_04_findExecutable REVISION DETAIL

Re: building KF5 projects against a different Qt5 version (than the one the KF5 frameworks were built against)

On Sunday, April 26, 2020 5:50:40 PM CEST René J.V. Bertin wrote: > On Sunday April 26 2020 16:58:10 David Faure wrote: > >> As a side-note, I'd even argue that > >> it would make sense to provide an all-inclusive header per framework, > >> just > >> like

D29170: Detect executables without +x permission in $PATH to improve error message

dfaure added a comment. In D29170#657450 , @ahmadsamir wrote: > I don't think you need to go out of your way to support custom setups, after all it's quite simple for the user to edit the .desktop file and specify the path to the executable.

Re: building KF5 projects against a different Qt5 version (than the one the KF5 frameworks were built against)

On Sunday, April 26, 2020 4:32:46 PM CEST René J.V. Bertin wrote: > On Sunday April 26 2020 15:46:35 David Faure wrote: > >Well, yeah, you can't have it all. > > I suppose that the appropriate ECM could provide a function that returns the > path to the "official"

D29206: Move check for invalid service from KDesktopFileActions to ApplicationLauncherJobTest

dfaure updated this revision to Diff 81250. dfaure added a comment. remove redundant line REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29206?vs=81249=81250 BRANCH 2020_04_invalid_service REVISION DETAIL https://phabricator.kde.org/D29206 AFFECTED

D29206: Move check for invalid service from KDesktopFileActions to ApplicationLauncherJobTest

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. TEST PLAN New unittest passes REPOSITORY R241 KIO BRANCH 2020_04_invalid_service

D28264: KIO: rename ProcessLauncherJob to ApplicationLauncherJob

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/D28264 To: dfaure, davidedmundson, broulik Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28020: New class ProcessLauncherJob in KIOGui

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/D28020 To: dfaure, apol, davidedmundson, nicolasfella, vkrause, broulik Cc: jbbgameich, kde-frameworks-devel,

D29153: Move handling of untrusted programs to ApplicationLauncherJob.

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/D29153 To: dfaure, ahmadsamir, broulik, ngraham, mdlubakowski Cc: kde-frameworks-devel, LeGast00n, cblack,

Re: building KF5 projects against a different Qt5 version (than the one the KF5 frameworks were built against)

On Sunday, April 26, 2020 3:21:34 PM CEST René J.V. Bertin wrote: > On Sunday April 26 2020 14:12:01 David Faure wrote: > >> (and possibly LD_PRELOAD). > > > >I don't see why you would set LD_PRELOAD in this configuration. > > This may be necessary if even one of t

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

dfaure added a comment. Ah! Now it actually makes sense to me. If we are changing what revertToDefault() does, then it makes sense to change the if() condition for calling it. Basically, now that it does the right thing in both cases (default from C++ and default from system file) it can be

D29170: Detect executables without +x permission in $PATH to improve error message

dfaure updated this revision to Diff 81228. dfaure added a comment. add missing braces REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29170?vs=81213=81228 BRANCH 2020_04_findExecutable REVISION DETAIL https://phabricator.kde.org/D29170 AFFECTED FILES

D29170: Detect executables without +x permission in $PATH to improve error message

dfaure added a comment. Meanwhile I sent an email to kde-frameworks-devel to get more input on the question. Supporting custom setups is a great feature for a powerful desktop environment. E.g. I deploy custom self-built apps for my users, so I use some of those custom KDE features. Not

Re: changing icon sizes no longer emits signal

t code path. The call to KIconLoader::emitChange is deeply nested in KDE4 compat stuff, with possible exit paths before. A patch like the one attached seems to help, but someone who knows the KCM better (or has time to dig) should make this conditional on the user actually changing icon sizes, and onl

Re: building KF5 projects against a different Qt5 version (than the one the KF5 frameworks were built against)

there is a trick to this I'd > love to hear it. Build against the old Qt, run against the new Qt? Then it's back to "only" LD_LIBRARY_PATH switcheroo which works well, while at the buildsystem level it's all based upon the old Qt so no mixup there. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5

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

dfaure closed this revision. REPOSITORY R295 KCMUtils REVISION DETAIL https://phabricator.kde.org/D28765 To: dfaure, pino, broulik, mart, davidedmundson, ngraham, svuorela Cc: svuorela, cblack, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

RFC: relative executables in desktop files

top-entry-spec/desktop-entry-spec-latest.html#exec-variables -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5

D29170: Detect executables without +x permission in $PATH to improve error message

dfaure updated this revision to Diff 81213. dfaure added a comment. Resolve relative executables using the directory of the .desktop file referring to them Not useful for /usr/share/applications stuff, but useful for custom setups where a custom desktop file refers to a local

D29170: Detect executables without +x permission in $PATH to improve error message

dfaure updated this revision to Diff 81209. dfaure added a comment. Better support for relative vs absolute executable names, with unittest. But I'll look into relative-to-desktop-file instead of relative-to-QDir::current. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D29170: Detect executables without +x permission in $PATH to improve error message

dfaure marked an inline comment as done. dfaure added inline comments. INLINE COMMENTS > ahmadsamir wrote in kprocessrunner.cpp:53 > KProcessRunner tries finding the executable in the current dir too, so to be > precise in the reported error message maybe append currentDir.absolutePath() > to

D29153: Move handling of untrusted programs to ApplicationLauncherJob.

dfaure added a comment. > The question is how that will work in conjunction with KNotificationJobUiDelegate? In principle we could also make it emit a KNotification with buttons We would need a KIO::NotificationJobUiDelegate subclass of KNotificationJobUiDelegate in KIOGui, which also

D29153: Move handling of untrusted programs to ApplicationLauncherJob.

dfaure updated this revision to Diff 81173. dfaure added a comment. Make UntrustedProgramHandlerInterface async. This required a nasty QEventLoop for KRun though, since it has a sync API. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29153?vs=81158=81173

D28909: smb: port to Result system to force serialization of error/finish condition

dfaure added inline comments. INLINE COMMENTS > sitter wrote in kio_smb.h:96 > Sure, if you think it's solid enough from an API POV. > > I was thinking that we should amend the slavebase API for KF6 in general. > Instead of having error/finished/opened all functions on an API level should >

D27216: [KProcessRunner] Improve error messages on failure

dfaure added a comment. Done in D29170 . Now we can start discussing the other part of the bug, executable doesn't launch due to missing libs. Ah BTW the unittest leads to "cp" failing because it deletes the temporary dir under cp's feet. This could

D29170: Detect executables without +x permission in $PATH to improve error message

dfaure created this revision. dfaure added a reviewer: ahmadsamir. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY QStandardPaths::findExecutable will not return to us a non-executable binary. So

D29153: Move handling of untrusted programs to ApplicationLauncherJob.

dfaure added inline comments. INLINE COMMENTS > broulik wrote in untrustedprogramhandlerinterface.h:79 > I was wondering if this should be done async? Nested event loops are quite a > problem when QML is involved. I don't see a nested event loop in makeServiceFileExecutable. I guess your

D27216: [KProcessRunner] Improve error messages on failure

dfaure added a comment. Please ignore my previous comment. When launching gwenview by clicking on an image (and not on the gwenview executable itself), we indeed do end up with KProcessRunner failing to find the executable because of the missing -x. I'll implement the proper investigation

D29153: Move handling of untrusted programs to ApplicationLauncherJob.

dfaure updated this revision to Diff 81158. dfaure added a comment. Also set a KIO::JobUiDelegate in KRun itself (which simplifies its code) REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29153?vs=8=81158 BRANCH 2020_04_UntrustedProgramHandler REVISION

D27216: [KProcessRunner] Improve error messages on failure

dfaure added a comment. Note that D29153 affects this since it will detect the lack of +x ahead of time in ApplicationLauncherJob and prompt the user [if the application uses KIOWidgets' JobUiDelegate -- oh I need to do this in KRun, will do right away].

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

dfaure added a comment. Thanks for the review! I was getting desperate to get one ;-) INLINE COMMENTS > svuorela wrote in kcmoduleinfo.cpp:73 > At a later point, I*m not sure what the purpose is for these members are - > but that's probably for another changeset. Right, I was wondering the

D29153: Move handling of untrusted programs to ApplicationLauncherJob.

dfaure added a reviewer: mdlubakowski. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D29153 To: dfaure, ahmadsamir, broulik, ngraham, mdlubakowski Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29153: Move handling of untrusted programs to ApplicationLauncherJob.

dfaure updated this revision to Diff 8. dfaure added a comment. Fix error message box after the user clicks Cancel. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29153?vs=81108=8 BRANCH 2020_04_UntrustedProgramHandler REVISION DETAIL

D29153: Move handling of untrusted programs to ApplicationLauncherJob.

dfaure updated this revision to Diff 81108. dfaure added a comment. improve docu, add test in kruntest REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29153?vs=81101=81108 BRANCH 2020_04_UntrustedProgramHandler REVISION DETAIL

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