D24773: kio_trash: Add size, modification, access and create date for trash:/

2020-04-13 Thread David Faure
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > trashsizecache.cpp:131 > +qint64 max_mtime = 0; > +const auto checMaxTime = [max_mtime] (const qint64 lastModTime) -> > qint64 { > +return

D24773: kio_trash: Add size, modification, access and create date for trash:/

2020-04-13 Thread David Faure
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > trashimpl.cpp:1098 > + > +// Find lateest modification date > +if (res.mtime > latestModifiedDate) { typo: latest > trashsizecache.cpp:144 >

D28774: [KFontRequester] Port from QFontDialog to KFontChooserDialog

2020-04-13 Thread David Faure
dfaure accepted this revision. dfaure added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > kfontrequester.cpp:188 > + > +const int result = KFontChooserDialog::getFont(m_selFont, flags, > q->parentWidget()); > It's really not obvious from the above

D28746: Show previews on encrypted filesystems

2020-04-13 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > previewjob.cpp:307 > > +//Prepare encryptedMountsList for which will be used in ::slotThumbData > const auto mountsList = KMountPoint::currentMountPoints(); "for which will be used" ... remove `for` ? REPOSITORY R241 KIO REVISION

D8274: Don't create new symlinks when copying symlinks - copy the file's contents - like /usr/bin/cp

2020-04-12 Thread David Faure
dfaure added a comment. Herald edited subscribers, added: kde-frameworks-devel; removed: Frameworks. @shlomif any plans on redoing this another way? Otherwise can you close the review request, to clean up dashboards? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8274

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

2020-04-12 Thread David Faure
dfaure updated this revision to Diff 79906. dfaure added a comment. Use @warning. Thanks for the tip! REPOSITORY R295 KCMUtils CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28765?vs=79893=79906 BRANCH master REVISION DETAIL https://phabricator.kde.org/D28765 AFFECTED

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

2020-04-11 Thread David Faure
dfaure created this revision. dfaure added reviewers: pino, broulik, mart, davidedmundson. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY KPluginInfo evolved into an abstraction over old-style

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

2020-04-11 Thread David Faure
dfaure updated this revision to Diff 79892. dfaure edited the summary of this revision. dfaure removed a subscriber: pino. dfaure added a comment. Use QMap, thanks Pino for the idea. REPOSITORY R295 KCMUtils CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28760?vs=79877=79892

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

2020-04-11 Thread David Faure
dfaure created this revision. dfaure added reviewers: apol, broulik, davidedmundson, kossebau. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY I had every toplevel entry doubled in kontact's

D24773: kio_trash: Add size, modification, access and create date for trash:/

2020-04-11 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH arcpatch-D24773_1 REVISION DETAIL https://phabricator.kde.org/D24773 To: meven, #frameworks, ngraham, elvisangelaccio, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh,

D28743: Port kruntest to ApplicationLauncherJob

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

D28758: [KFontChooser] Make the code slightly more readable

2020-04-11 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Thanks ;) REPOSITORY R236 KWidgetsAddons BRANCH l-kfontchooser-5 (branched from master) REVISION DETAIL https://phabricator.kde.org/D28758 To: ahmadsamir, #frameworks, dfaure Cc:

D28675: [KMimeTypeChooser] Add the ability to filter the treeview with a QSFPM

2020-04-11 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R236 KWidgetsAddons BRANCH l-kmimechooser (branched from master) REVISION DETAIL https://phabricator.kde.org/D28675 To: ahmadsamir, #frameworks, cfeck, dfaure Cc: kde-frameworks-devel, LeGast00n,

D28742: Add KDialogJobUiDelegate(KJobUiDelegate::Flags) constructor

2020-04-11 Thread David Faure
dfaure closed this revision. REPOSITORY R288 KJobWidgets REVISION DETAIL https://phabricator.kde.org/D28742 To: dfaure, broulik, davidedmundson, ervin Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28756: [findreplace] Handle searching for WholeWordsOnly in Regex mode

2020-04-11 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R310 KTextWidgets BRANCH l-wholewords (branched from master) REVISION DETAIL https://phabricator.kde.org/D28756 To: ahmadsamir, #frameworks, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack,

D28742: Add KDialogJobUiDelegate(KJobUiDelegate::Flags) constructor

2020-04-11 Thread David Faure
dfaure added a comment. In D28742#646009 , @kossebau wrote: > `window` parameter wants API dox mentioning, though. Oops, I thought I did that. Fixed. > And perhaps could be defaulted to nullptr, for use-cases which do not have a

D28742: Add KDialogJobUiDelegate(KJobUiDelegate::Flags) constructor

2020-04-11 Thread David Faure
dfaure updated this revision to Diff 79857. dfaure added a comment. Expand docs about the associated window REPOSITORY R288 KJobWidgets CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28742?vs=79848=79857 BRANCH master REVISION DETAIL https://phabricator.kde.org/D28742

D28753: Add KNotificationJobUiDelegate(KJobUiDelegate::Flags) constructor

2020-04-11 Thread David Faure
dfaure closed this revision. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D28753 To: dfaure, broulik, davidedmundson, ervin Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28742: Add KDialogJobUiDelegate(KJobUiDelegate::Flags) constructor

2020-04-11 Thread David Faure
dfaure updated this revision to Diff 79848. dfaure added a comment. Add QWidget *window parameter. Even better, no? Needed for dialog boxes to respect stacking order, centering to parent, focus going back to parent after closing... REPOSITORY R288 KJobWidgets CHANGES SINCE LAST

D28274: [KFontChooser] Add a checkbox to toggle showing only monospaced fonts

2020-04-11 Thread David Faure
dfaure accepted this revision. dfaure added inline comments. INLINE COMMENTS > kfontchooser.cpp:393 > q->setFont(QFontDatabase::systemFont(QFontDatabase::FixedFont), > usingFixed); > +onlyFixedCheckbox->setChecked(usingFixed); > } else { would `true` be more readable?

D28753: Add KNotificationJobUiDelegate(KJobUiDelegate::Flags) constructor

2020-04-11 Thread David Faure
dfaure created this revision. dfaure added reviewers: broulik, davidedmundson, ervin. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. TEST PLAN Builds REPOSITORY R289 KNotifications BRANCH master REVISION

D28742: Add KDialogJobUiDelegate(KJobUiDelegate::Flags) constructor

2020-04-11 Thread David Faure
dfaure updated this revision to Diff 79840. dfaure marked an inline comment as done. dfaure added a comment. explicit REPOSITORY R288 KJobWidgets CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28742?vs=79820=79840 BRANCH master REVISION DETAIL

D28742: Add KDialogJobUiDelegate(KJobUiDelegate::Flags) constructor

2020-04-11 Thread David Faure
dfaure marked an inline comment as done. dfaure added inline comments. INLINE COMMENTS > kossebau wrote in kdialogjobuidelegate.h:51 > Why no explicit? good point REPOSITORY R288 KJobWidgets BRANCH master REVISION DETAIL https://phabricator.kde.org/D28742 To: dfaure, broulik,

D28741: [KJobUiDelegate] Add AutoHandlingEnabled flag

2020-04-11 Thread David Faure
dfaure closed this revision. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D28741 To: dfaure, broulik, davidedmundson, ervin Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28406: Fix sonnet autodetect test failure

2020-04-11 Thread David Faure
dfaure added a comment. I pushed it with my changes included. The test is fixed: https://build.kde.org/job/Frameworks/view/Platform%20-%20SUSEQt5.12/job/sonnet/job/kf5-qt5%20SUSEQt5.12/ REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D28406 To: dfaure, waqar Cc:

D28406: Fix sonnet autodetect test failure

2020-04-11 Thread David Faure
dfaure commandeered this revision. dfaure edited reviewers, added: waqar; removed: dfaure. REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D28406 To: dfaure, waqar Cc: dfaure, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28406: Fix sonnet autodetect test failure

2020-04-11 Thread David Faure
This revision was not accepted when it landed; it landed in state "Needs Revision". This revision was automatically updated to reflect the committed changes. Closed by commit R246:78450298149e: Fix sonnet autodetect test failure (authored by waqar, committed by dfaure). CHANGED PRIOR TO COMMIT

D20749: autotests: don't fail appstream test because of anything on stderr

2020-04-11 Thread David Faure
dfaure added a comment. Similar to this... is it expected that a glib error makes the test fail? Is the error severe or harmless? CMake Error at /home/jenkins/workspace/Frameworks/kpackage/kf5-qt5 SUSEQt5.12/autotests/kpackagetoolappstreamtest.cmake:45 (message): appstream data

D27097: [KTextWidgets] Port from QRegExp to QRegularExpression

2020-04-11 Thread David Faure
dfaure added a comment. CI doesn't like this commit. https://build.kde.org/job/Frameworks/job/ktextwidgets/job/kf5-qt5%20SUSEQt5.14/4/testReport/junit/projectroot/autotests/ktextwidgets_kfindtest/ FAIL! : TestKFind::testStaticFindRegexp(whole words ok) Compared values are not

D28675: [KMimeTypeChooser] Add the ability to filter the treeview with a QSFPM

2020-04-11 Thread David Faure
dfaure added a comment. In D28675#645733 , @ahmadsamir wrote: > A tooltip isn't good enough here? I don't know. I don't claim to be a UI expert. But for instance your lineedit could filter on the comment as well, to help the user

D28743: Port kruntest to ApplicationLauncherJob

2020-04-11 Thread David Faure
dfaure created this revision. dfaure added reviewers: broulik, davidedmundson, ervin. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY NO_CHANGELOG TEST PLAN The "run(doesnotexist, no url)" button

D28742: Add KDialogJobUiDelegate(KJobUiDelegate::Flags) constructor

2020-04-11 Thread David Faure
dfaure created this revision. dfaure added reviewers: broulik, davidedmundson, ervin. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY Requires: D28741 TEST PLAN

D28741: [KJobUiDelegate] Add AutoHandlingEnabled flag

2020-04-11 Thread David Faure
dfaure created this revision. dfaure added reviewers: broulik, davidedmundson, ervin. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY This allows to simplify code from KDialogJobUiDelegate

D28478: [FileProtocol] change statx stat_dev() to return makedev(major, minor)

2020-04-11 Thread David Faure
dfaure added a comment. In D28478#645702 , @ahmadsamir wrote: > I meant something like: > const int i = foo(); int is a very bad example for this reasoning. It's cheaper to copy an int than to use a reference. That's not the case for

D28675: [KMimeTypeChooser] Add the ability to filter the treeview with a QSFPM

2020-04-11 Thread David Faure
dfaure added a comment. Nice work. But can you give the reasoning for removing the comment column? For non-english speakers this might be the only thing that's actually readable? I'm also curious about the use of QStandardItemModel which I consider to be at best a class for unittesting

KDE Frameworks 5.69.0 released

2020-04-11 Thread David Faure
code has been GPG-signed using the following key: pub rsa2048/58D0EE648A48B3BB 2016-09-05 David Faure Primary key fingerprint: 53E6 B47B 45CE A3E0 D5B7 4577 58D0 EE64 8A48 B3BB http://kde.org/announcements/kde-frameworks-5.69.0.php -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5

KDE Frameworks 5.69.0 released

2020-04-11 Thread David Faure
code has been GPG-signed using the following key: pub rsa2048/58D0EE648A48B3BB 2016-09-05 David Faure Primary key fingerprint: 53E6 B47B 45CE A3E0 D5B7 4577 58D0 EE64 8A48 B3BB http://kde.org/announcements/kde-frameworks-5.69.0.php -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5

D24773: kio_trash: Add size, modification, access and create date for trash:/

2020-04-11 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > kfileitem.h:349 > + * > + * Initialy only implemented for trash:/ > + * typo: Initially > trashimpl.cpp:1099 > + > +info = QFileInfo(trashPath); > +modified = info.lastModified(); Reusing info is more expensive (and

D25707: [renamedialog] Replace KIconLoader usage with QIcon::fromTheme

2020-04-11 Thread David Faure
dfaure added a comment. OK. Then let me change my comment to: please document in kiconloader.h how to port away from loadMimeTypeIcon, even if it's not actually deprecated. I remember wondering the same thing at one point. REPOSITORY R241 KIO BRANCH rena REVISION DETAIL

D28679: [KPropertiesDialog] Disable changing remote dir icons

2020-04-11 Thread David Faure
dfaure accepted this revision. dfaure added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > kpropertiesdialog.cpp:1285 > +// exclude remote dirs as changing the icon has no effect (bug > 205954) > +if (item.isLocalFile()|| item.url().scheme()

D28271: [KFontChooser] More code cleanup

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

D28478: [FileProtocol] change statx stat_dev() to return makedev(major, minor)

2020-04-11 Thread David Faure
dfaure added a comment. In D28478#645019 , @ahmadsamir wrote: > hmm... first this is a copy-paste "error" on my part, I personally never use & when the RHS is a temporary (I don't see the point). The point is to avoid a copy ;-) If

D28647: Fix KIO::Scheduler::emitReparseSlaveConfiguration() to work if called twice in same process

2020-04-11 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. The new code structure screams for killing the bool and just extracting the code (all of slotReparseSlaveConfiguration) to a different method, called directly from

D25707: [renamedialog] Replace KIconLoader usage with QIcon::fromTheme

2020-04-11 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. loadMimeTypeIcon isn't deprecated, so what's the reason? Porting away from kiconthemes? Should we actually deprecate it? REPOSITORY R241 KIO BRANCH rena REVISION DETAIL

D24773: kio_trash: Add size, modification, access and create date for trash:/

2020-04-10 Thread David Faure
dfaure added a comment. This looks good. I think I failed to notice the last update, sorry for the delay. Can you rebase (I suspect a few conflicts) and update version numbers to 5.70? Then it'll be good to go. Ah, and the documentation for the KFileItem getter needs to say that

D28669: make CopyJob non-recursive

2020-04-09 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > anthonyfieroni wrote in copyjob.cpp:814 > > Which threads? There are no threads involved here. > > Yes, i'm afraid of loop that can block the event queue, !isKilled will not > work in single thread environment. Right. If this loop can really

D28669: make CopyJob non-recursive

2020-04-09 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > meven wrote in copyjob.cpp:915 > When reaching here the `while (m_currentStatSrc != m_srcList.constEnd()) {` > means we have nothing left to stat, meaning stating phase is indeed finished. Not if we just launched subjobs and we haven't gotten the

D28669: make CopyJob non-recursive

2020-04-08 Thread David Faure
dfaure requested changes to this revision. dfaure added inline comments. INLINE COMMENTS > copyjob.cpp:809 > ++m_currentStatSrc; > -statCurrentSrc(); > } Why did you remove this call? Your patch has no context (please use `arc` to upload patches to phabricator), but I can see here

D28406: Fix sonnet autodetect test failure

2020-04-08 Thread David Faure
dfaure added a comment. @waqar ping? What do you think about my suggestion? If you agree, can you update the patch? REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D28406 To: waqar, dfaure Cc: dfaure, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham,

D28406: Fix sonnet autodetect test failure

2020-04-08 Thread David Faure
dfaure requested changes to this revision. This revision now requires changes to proceed. REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D28406 To: waqar, dfaure Cc: dfaure, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28406: Fix sonnet autodetect test failure

2020-04-08 Thread David Faure
dfaure added a reviewer: dfaure. REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D28406 To: waqar, dfaure Cc: dfaure, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

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

2020-04-07 Thread David Faure
dfaure added a comment. I like your first suggestion. I don't like the second one, because QT_LOGGING_RULES is used (by at least CI and myself) to enable *more* debug output. unsetenv would break that. REPOSITORY R243 KArchive REVISION DETAIL https://phabricator.kde.org/D27910 To:

D28603: API dox: document more of the default property values of KUrlRequester

2020-04-05 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH documentmorekrulrequesterdefaults REVISION DETAIL https://phabricator.kde.org/D28603 To: kossebau, #frameworks, meven, dfaure, ahmadsamir Cc: kde-frameworks-devel, LeGast00n,

D28601: Fix DirectorySizeJob so it doesn't depend on listing order

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

D28472: [DirectorySizeJob] Fix sub-dirs count when resolving symlinks to dirs

2020-04-05 Thread David Faure
dfaure added a comment. Suggested fix in D28601 REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D28472 To: ahmadsamir, #frameworks, dfaure, meven Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28601: Fix DirectorySizeJob so it doesn't depend on listing order

2020-04-05 Thread David Faure
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. TEST PLAN jobtest passes locally again REPOSITORY R241 KIO BRANCH master REVISION DETAIL

D28472: [DirectorySizeJob] Fix sub-dirs count when resolving symlinks to dirs

2020-04-05 Thread David Faure
dfaure added a comment. Does the test pass for you now? For me it fails (and it fails on FreeBSD CI too https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.14/37/testReport/junit/projectroot/autotests/kiocore_jobtest/) FAIL! : JobTest::directorySize() Compared

D28598: KRun: fix assert when failing to start an application

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

D27216: [KProcessRunner] Improve error messages on failure

2020-04-05 Thread David Faure
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kprocessrunner.cpp:279 > +if (exitCode != 0) { > +KMessageBox::error(nullptr, i18n("The program '%1' terminated > abnormally.\n" > +

D27216: [KProcessRunner] Improve error messages on failure

2020-04-05 Thread David Faure
dfaure added reviewers: davidedmundson, broulik. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D27216 To: ahmadsamir, #frameworks, dfaure, davidedmundson, broulik Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27097: [KTextWidgets] Port from QRegExp to QRegularExpression

2020-04-05 Thread David Faure
dfaure added a comment. It's tagged, you can go ahead. REPOSITORY R310 KTextWidgets BRANCH l-qregularexpression (branched from master) REVISION DETAIL https://phabricator.kde.org/D27097 To: ahmadsamir, #frameworks, mlaurent, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2,

D28598: KRun: fix assert when failing to start an application

2020-04-05 Thread David Faure
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. TEST PLAN 'sudo chmod a-x /usr/bin/gwenview' Try opening a picure file from e.g. dolphin, you get

D28592: [ApplicationLauncherJob] Guard against m_pids being empty

2020-04-05 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. This, hmm, sounds wrong. Is the caller calling pid() even though the job finished with an error code? Ah, yes, indeed, in the case of KRun. Whoops. Here's my suggested fix

D27097: [KTextWidgets] Port from QRegExp to QRegularExpression

2020-04-05 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Thanks! REPOSITORY R310 KTextWidgets BRANCH l-qregularexpression (branched from master) REVISION DETAIL https://phabricator.kde.org/D27097 To: ahmadsamir, #frameworks, mlaurent,

D28528: UDSEntry: add constructor variant with std::initializer_list

2020-04-05 Thread David Faure
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > udsentry.h:132 > +const long long l; > +char s[sizeof(QString)]; > +} m_u; Why not simply QString*? > udsentrybenchmark.cpp:141 > +

D28409: ECM: attempt to fix KDEInstallDirsTest.relative_or_absolute on Windows

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

D28409: ECM: attempt to fix KDEInstallDirsTest.relative_or_absolute on Windows

2020-04-05 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > CMakeLists.txt:146 > set(KDEInstallDirsTest.relative_or_absolute_usr_EXTRA_OPTIONS > --build-options -DCMAKE_INSTALL_PREFIX=/usr > -DKDE_INSTALL_USE_QT_SYS_PATHS=FALSE This test uses /usr and passes on Windows. It's not

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

2020-04-05 Thread David Faure
dfaure added a comment. I have a hard time accepting that the documentation was wrong -- and if it was, then this commit has to fix it, and port as much of the app code that does exactly this, as possible. I don't really know what mReference is. What about a test that uses KConfig

D28577: Add StatusBarExtension(KParts::Part *) overloaded constructor.

2020-04-05 Thread David Faure
dfaure closed this revision. REPOSITORY R306 KParts REVISION DETAIL https://phabricator.kde.org/D28577 To: dfaure, vkrause, aacid, cgiboudeaux, kossebau Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27216: [KProcessRunner] Improve error reported to user when exit code != 0

2020-04-05 Thread David Faure
dfaure added a comment. Oops I totally missed this one, sorry. Does this testcase work better with the new code in kiogui? Hopefully QProcess start should fail? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D27216 To: ahmadsamir, #frameworks, dfaure Cc:

D28033: Create ExpandableListItem

2020-04-05 Thread David Faure
dfaure added a comment. This commit broke the i18ndcheck unittest. https://build.kde.org/job/Frameworks/view/Platform%20-%20SUSEQt5.14/job/plasma-framework/job/kf5-qt5%20SUSEQt5.14/9/testReport/junit/projectroot/autotests/i18ndcheck/ Reproducible locally. Works before and breaks

D28530: newServicePath(): Docs say add prefix but it's actually suffix

2020-04-04 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Buggy since 2003 :-) REPOSITORY R309 KService BRANCH l-docs (branched from master) REVISION DETAIL https://phabricator.kde.org/D28530 To: ahmadsamir, #frameworks, dfaure, apol Cc:

D28295: Introduce KNotificationJobUiDelegate

2020-04-04 Thread David Faure
dfaure added a comment. Done, thanks for the pointer. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D28295 To: broulik, #frameworks, nicolasfella, dfaure Cc: ahmadsamir, kossebau, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28577: Add StatusBarExtension(KParts::Part *) overloaded constructor.

2020-04-04 Thread David Faure
dfaure created this revision. dfaure added reviewers: vkrause, aacid, cgiboudeaux, kossebau. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY There's nothing specific to ReadOnlyPart about the

D28555: File ioslave : use Better setting for sendfile syscall

2020-04-04 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Ah sorry, I missed the fact that one doesn't actually need to call sendfile64() explicitly. And if sendfile fails we have a fall back, so yeah, might as well try in all cases.

D28555: File ioslave : use Better setting for sendfile syscall

2020-04-04 Thread David Faure
dfaure added a comment. Sounds like the size check should stay then. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D28555 To: meven, #frameworks, dfaure Cc: bruns, broulik, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham

D28555: File ioslave : use Better setting for sendfile syscall

2020-04-04 Thread David Faure
dfaure added a comment. any idea what buff_src.st_size < 0x7FFF was for? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D28555 To: meven, #frameworks, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28552: [CommandLauncherJob] Also mention setDesktopName in constructor

2020-04-04 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/D28552 To: broulik, #frameworks, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28515: [CommandLauncherJob] Add constructor taking an executable and argument list

2020-04-04 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Awesome. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D28515 To: broulik, #frameworks, dfaure Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham,

D28128: Add force save behavior to KEntryMap

2020-04-03 Thread David Faure
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D28128 To: bport, ervin, dfaure, meven, crossi, hchain Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27455: FileWidgets: Ignore Return events from KDirOperator

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

D28421: Add static method to check start condition

2020-04-03 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > kautostart.h:289 > + */ > +static bool isStartConditionMet(const QString ); > + This is new public API, it's missing @since 5.69 REPOSITORY R309 KService REVISION DETAIL https://phabricator.kde.org/D28421 To: hchain, davidedmundson,

D28515: [CommandLauncherJob] Add constructor taking an executable and argument list

2020-04-03 Thread David Faure
dfaure added a comment. You could create a subdir of a QTemporaryDir with a fixed name like "abc def", copy `cp` or `copy.exe` in there, and then run that with an absolute path? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D28515 To: broulik, #frameworks, dfaure Cc:

D28515: [CommandLauncherJob] Add constructor taking an executable and argument list

2020-04-02 Thread David Faure
dfaure added a comment. Good question. I haven't seen that done in most callers, but indeed, what if it's in a path with a space, like often happens on Windows? It sounds like the answer is yes, it should be quoted. The real answer is to write a CommandLauncherJob unittest for such a

D28515: [CommandLauncherJob] Add constructor taking an executable and argument list

2020-04-02 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. KProcessRunner::KProcessRunner(QString cmd) calls KProcess::setShellCommand(cmd) which ... has two modes. Either KShell::splitArgs()[0] finds an executable that exists (checking that there

D28295: Introduce KNotificationJobUiDelegate

2020-04-02 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. I'm OK with this landing into 5.69 Feel free to emit a description from the two new jobs. Thanks, David. REPOSITORY R289 KNotifications REVISION DETAIL

D28472: [DirectorySizeJob] Fix sub-dirs count when resolving symlinks to dirs

2020-04-01 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Yay for unittests (and my bad for not running them when making that change). Thanks for the fix! REPOSITORY R241 KIO BRANCH l-dirsizejob (branched from master) REVISION DETAIL

D28409: ECM: attempt to fix KDEInstallDirsTest.relative_or_absolute on Windows

2020-03-29 Thread David Faure
dfaure created this revision. dfaure added reviewers: kossebau, apol, cgiboudeaux. Herald added projects: Frameworks, Build System. Herald added subscribers: kde-buildsystem, kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY Not passing CMAKE_INSTALL_PREFIX is a

D28406: Fix sonnet autodetect test failure

2020-03-29 Thread David Faure
dfaure added a comment. Well if the test is tolerant about correctLangCode, it might as well not require a specific variant upfront. What about replacing en_US-large with en_US and de_DE_frami with de_DE in SonnetAutoDetectTest::autodetect_data? This makes the first test pass for me

D28388: kio_file: honour KIO::StatResolveSymlink for UDS_DEVICE_ID and UDS_INODE

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

D28388: kio_file: honour KIO::StatResolveSymlink for UDS_DEVICE_ID and UDS_INODE

2020-03-29 Thread David Faure
dfaure added a comment. In D28388#637417 , @ahmadsamir wrote: > (Now the other issue, "DEVICE" (from the kproperties patch) is always "8", whether I use ~/.bashrc, /usr/bin/file, or some file on a usb stick. But that's not really related to

D28388: kio_file: honour KIO::StatResolveSymlink for UDS_DEVICE_ID and UDS_INODE

2020-03-29 Thread David Faure
dfaure added a comment. kdeinit5 is the one who needs to see that QT_PLUGIN_PATH in order to locate kio_file.so. $ export QT_PLUGIN_PATH=$PWD/bin $ kdeinit5 $ kioclient5 openProperties ~/.bashrc 19:23:54.253 kio_file(27069/27069) createUDSEntry HELLO AHMAD REPOSITORY

D28388: kio_file: honour KIO::StatResolveSymlink for UDS_DEVICE_ID and UDS_INODE

2020-03-29 Thread David Faure
dfaure added a comment. Type `kdeinit5` in a terminal, the output from ioslaves (created after that) will go there. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D28388 To: dfaure, trmdi, ahmadsamir, meven Cc: kde-frameworks-devel, LeGast00n, cblack,

D28406: "Fix sonnet autodetect test failure"

2020-03-29 Thread David Faure
dfaure added a comment. I'm no expert in this stuff, but here's my feedback 1. on my own system (!= CI), all three tests were failing before, due to finding en_AU-variant_1, en_AU-variant_1 and "". 2. now the test passes locally, but because all tests are skipped SKIP :

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

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

D28295: Introduce KNotificationJobUiDelegate

2020-03-29 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > broulik wrote in knotificationjobuidelegate.cpp:24 > I thought we wanted to migrate towards nested `Private` class? :0 I don't remember any past discussion about this, but as I discovered in KIO commit 3d2330968b

D28367: KServiceAction: store parent service

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

D28266: KPropertiesDialog: port KRun::run to ApplicationLauncherJob

2020-03-29 Thread David Faure
dfaure closed this revision. 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

D28266: KPropertiesDialog: port KRun::run to ApplicationLauncherJob

2020-03-29 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > broulik wrote in applicationlauncherjob.h:50 > Please add this in a separate commit Done in https://commits.kde.org/kio/01a4b70803d5bc9144a7ca7aec12081b4356c638 (also in CommandLauncherJob) > broulik wrote in kpropertiesdialog.cpp:1458 > Call

D28390: KIO: remove waitForStarted() from the launcher jobs public API

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

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