D14437: Fix QFileDialog not remembering the last visited directory.

dfaure updated this revision to Diff 38826. dfaure added a comment. Remove TODO comment. Although I can say it would have definitely helped me when investigating this bug... REPOSITORY R135 Integration for Qt applications in Plasma CHANGES SINCE LAST UPDATE

D14493: fontinst quits after KJob is done

dfaure added a comment. The idea of KJob reference-counting the event loop is that if you quit the application while a kjob is executing, then the process will wait for the job to finish before quitting. (Use case back then, konqueror and a HTTP download). The reason why we don't just quit

D14546: [Service Runner] Look up relative entryPaths

dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. This relies on the fact that nowadays entryPath() is absolute for application desktop files (for a reason I forgot, must be related to the vfolder stuff), so relative means indeed relative

D14493: fontinst quits after KJob is done

dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. With your explanations, I actually have a better feeling about the first patch after all (if a job is stuck for more than 30s, then the process will never exit). But really, I'm

D14493: fontinst quits after KJob is done

dfaure added a comment. Please push the first patch ;) REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D14493 To: mathiastillman, #plasma, dfaure, aacid, gateau Cc: wbauer, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed,

D14493: fontinst quits after KJob is done

dfaure added a comment. Ah so it's kind of a daemon, that should keep running until logout? I wasn't aware of that, I thought it was a KCM. Disabling as in the first patch made sense then ;) Either version is fine with me. INLINE COMMENTS > FontInst.h:161 > +// This is needed

D14437: Fix QFileDialog not remembering the last visited directory.

dfaure added inline comments. INLINE COMMENTS > anthonyfieroni wrote in kdeplatformfiledialoghelper.cpp:118 > It works without comment, i think these connections are not needed. Yes for this problem my patch is enough, but I was surprised to find that those signals were not connected. Yes it's

D14440: File Dialog: fix testSelectUrl() again, i.e. selectUrl() should set the directory too

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit R135:22da1e661afd: File Dialog: fix testSelectUrl() again, i.e. selectUrl() should set the… (authored by dfaure). Herald

D14437: Fix QFileDialog not remembering the last visited directory.

dfaure added a comment. ping? REPOSITORY R135 Integration for Qt applications in Plasma REVISION DETAIL https://phabricator.kde.org/D14437 To: dfaure, anthonyfieroni, elvisangelaccio, plasma-devel, broulik, arichardson Cc: apol, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed,

D14440: File Dialog: fix testSelectUrl() again, i.e. selectUrl() should set the directory too

dfaure added a comment. I'll assume +1 and +1 = +2, and push... REVISION DETAIL https://phabricator.kde.org/D14440 To: dfaure, arichardson, anthonyfieroni, elvisangelaccio, plasma-devel, broulik

D14437: Fix QFileDialog not remembering the last visited directory.

dfaure closed this revision. REPOSITORY R135 Integration for Qt applications in Plasma REVISION DETAIL https://phabricator.kde.org/D14437 To: dfaure, anthonyfieroni, elvisangelaccio, plasma-devel, broulik, arichardson, ngraham Cc: ngraham, apol, ragreen, Pitel, ZrenBot, lesliezhai,

D14204: [KEditFileType] Show application icon in preference list

dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. So easy ;-) REPOSITORY R126 KDE CLI Utilities REVISION DETAIL https://phabricator.kde.org/D14204 To: broulik, dfaure, ngraham Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai,

D15429: Simplify cmake code: link to Qt5::GuiPrivate.

dfaure created this revision. dfaure added reviewers: kfunk, kossebau. Herald added a project: Plasma. Herald edited subscribers, added: plasma-devel; removed: Plasma. dfaure requested review of this revision. REVISION SUMMARY This target was added in Qt 5.9, which we can now rely upon. Also

D10024: Add supportedSchemes feature

dfaure added inline comments. INLINE COMMENTS > kfilewidget.h:498 > +/** > + * Set the URL schemes that the file widget should allow navigating to. > + * All of these public methods should indicate that the default list is *empty* and that empty means "all schemes are supported". I

D10025: Use KFileWidget::setSupportedSchemes when available

dfaure accepted this revision. REPOSITORY R135 Integration for Qt applications in Plasma BRANCH master REVISION DETAIL https://phabricator.kde.org/D10025 To: mwolff, dfaure, mart Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. I think this patch makes sense, even though I'm sad that it doesn't actually fix the bug I was seeing (you call it broken config file, in that case that would mean that the code writing

D12275: FunnelModel: fix invalid model API usage.

dfaure created this revision. dfaure added a reviewer: hein. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. dfaure requested review of this revision. REVISION SUMMARY The new asserts added in qtbase in commit 00c09e752f (Qt 5.11) are

D12275: FunnelModel: fix invalid model API usage.

dfaure closed this revision. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D12275 To: dfaure, hein, davidedmundson Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10016: [KFileTreeView] Manually expand tree when model tells us to

dfaure added a comment. Wow, I just wanted to say thank you. This bug has been bugging me for many years (though never enough to actually take the time to look into it...). You rock, Kai Uwe. REPOSITORY R135 Integration for Qt applications in Plasma REVISION DETAIL

D15429: Simplify cmake code: link to Qt5::GuiPrivate.

This revision was automatically updated to reflect the committed changes. Closed by commit R135:db3a865c8cbf: Simplify cmake code: link to Qt5::GuiPrivate. (authored by dfaure). REPOSITORY R135 Integration for Qt applications in Plasma CHANGES SINCE LAST UPDATE

D15429: Simplify cmake code: link to Qt5::GuiPrivate.

dfaure added a comment. I completely forgot about pushing it, thanks for the reminder and for the target branch information. REPOSITORY R135 Integration for Qt applications in Plasma BRANCH Plasma/5.13 REVISION DETAIL https://phabricator.kde.org/D15429 To: dfaure, kfunk, kossebau,

Quiet Mode signal

not subscribed. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5

D17652: Fix logic error in kioclient noninteractive argument handling

dfaure added a comment. No, the commit in 2006 wasn't buggy. There was special logic in KCmdLineArgs for the case of the "no" prefix (note how it said "ninteractive", the "no" was toggling the value). This is a porting error when going from KCmdLineArgs to QCommandLineParser. So it's

D17652: Fix logic error in kioclient noninteractive argument handling

dfaure added a comment. Looks good except for the TODO comment. I think the comment that is actually needed is one about why we have a "noninteractive" option and it has no effect whatsoever ;) INLINE COMMENTS > kioclient.cpp:278 > +// interactive used to be the default, but due to

D17652: Fix logic error in kioclient noninteractive argument handling

dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. I'm not sure anymore what the best default is. There are two different use cases, that's all. REPOSITORY R126 KDE CLI Utilities BRANCH master REVISION DETAIL

D15189: [KRun] Don’t follow redirection to speed up and avoid incorrect behavior

dfaure added a comment. Please add a KIO version ifdef to avoid breaking compilation with older versions of KIO. REPOSITORY R126 KDE CLI Utilities REVISION DETAIL https://phabricator.kde.org/D15189 To: achauvel, #frameworks, dfaure, cfeck Cc: plasma-devel, anthonyfieroni, ngraham,

D17372: [componentchooser KCM] Make KIO browser option the fallback only and remove from the UI

dfaure added a comment. OK, here's a bit of historical information, and then my opinion. The idea of that option was that when you click on a (e.g. HTTP) link to a PDF (e.g. in an email), the best application to view that PDF is Okular. Similarly, if it's an image, the best application

D17652: Fix logic error in kioclient noninteractive argument handling

dfaure added a comment. Good find. However for users who didn't want a progress dialog, and who never got a conflict (e.g. if they were copying into an empty dir), kioclient was non-interactive, so I think our previous reasoning still holds. REPOSITORY R126 KDE CLI Utilities REVISION

D19784: Avoid calling QT_LSTAT and accessing recent documents

dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. OK so this is about KFileItem::text() and KFileItem::iconName(). Indeed this doesn't need the stat() done by KFileItem's init(). This means the right solution is indeed for

D19784: Avoid calling QT_LSTAT and accessing recent documents

dfaure added a comment. > If they point to files on a network drive, and the network or the drive is not responding Well that's exactly the problem with network mounts, and the reason they are a sucky technical solution. KIO's async jobs never have that problem. You will never be

D19784: Avoid calling QT_LSTAT and accessing recent documents

dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. This needs a KIO version ifdef for KIO >= 5.57. `#if KIO_VERSION >= QT_VERSION_CHECK(5,57,0)` REPOSITORY R119 Plasma Desktop REVISION DETAIL

D19784: Avoid calling QT_LSTAT and accessing recent documents

This revision was automatically updated to reflect the committed changes. Closed by commit R119:408f03ca989e: Avoid calling QT_LSTAT and accessing recent documents (authored by hoffmannrobert, committed by dfaure). REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE

D18296: Add support for passing cursor information via URL parameters when running kioclient exec.

dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. Is the makeURL function still used, or should it be removed now? INLINE COMMENTS > urlinfo.h:39 > + */ > +UrlInfo(QString path) > +: line(0), column(0) const

D19500: [KDirModel] Fix job urls change signal connection

dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. If there was a unittest for this code, it wouldn't remain broken for so long... Feel free to add one :-) REPOSITORY R119 Plasma Desktop REVISION DETAIL

D19588: [Notifications] Improve finished notification

dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > anthonyfieroni wrote in Jobs.qml:117-120 > displayDestUrl = destUrl.replace(/^(file:\/{2})/, "") All of this is wrong. Removing file:/// leaves something

D19784: Avoid calling QT_LSTAT and accessing recent documents

dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > recentusagemodel.cpp:261 > > if (!url.isValid() || !(fileItem.isFile() || fileItem.isDir())) { > return QVariant(); How do you expect

D15189: [KRun] Don’t follow redirection to speed up and avoid incorrect behavior

dfaure added a comment. #include #if KIO_VERSION >= QT_VERSION_CHECK(5,55,0) ... #endif REPOSITORY R126 KDE CLI Utilities REVISION DETAIL https://phabricator.kde.org/D15189 To: achauvel, #frameworks, dfaure, cfeck Cc: plasma-devel, anthonyfieroni, ngraham,

D18296: Add support for passing cursor information via URL parameters when running kioclient exec.

dfaure accepted this revision. dfaure added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > urlinfo.h:76 > + * - make relative paths absolute using the current working > directory > + * - prefer local file, if in doubt! > + */

D18296: Add support for passing cursor information via URL parameters when running kioclient exec.

dfaure added inline comments. INLINE COMMENTS > arrowd wrote in urlinfo.h:39 > > const QString & > > There is `path.chop(match.capturedLength());`, which requires non-const > `QString`. > > > And what if it's a URL? At this point this string is pathOrUrl. > > Well, `if (QFile::exists(path))`

D17372: [componentchooser KCM] Make KIO browser option the fallback only and remove from the UI

dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. I agree that we shouldn't remove the underlying feature, in fact it's still used on other protocols than http. But we have to ensure the default behaviour matches the KCM and

D21514: MobileTextActionsToolBar: fix runtime warnings when controlRoot isn't set yet

dfaure created this revision. dfaure added reviewers: mart, apol, broulik. Herald added a project: Plasma. dfaure requested review of this revision. REVISION SUMMARY This file checks for controlRoot being undefined, everywhere else. TEST PLAN `qmlscene tests/testComboBox.qml` showed

D17372: [componentchooser KCM] Make KIO browser option the fallback only and remove from the UI

dfaure added a comment. Writing into the user's home dir is a "hack". There are better ways for distros to set defaults (such as the global mimeapps.list) but it seems KIO ignores that... (to be checked...) REPOSITORY R119 Plasma Desktop REVISION DETAIL

D21959: Fix selectedNameFilter() multiple matches

dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. +1 for the included unittest. INLINE COMMENTS > kdeplatformfiledialoghelper.cpp:80 > */ > -static QString kde2QtFilter(const QStringList , const QString ) > +static QString

D21959: Fix selectedNameFilter() multiple matches

dfaure closed this revision. REPOSITORY R135 Integration for Qt applications in Plasma REVISION DETAIL https://phabricator.kde.org/D21959 To: hoffmannrobert, #frameworks, apol, dfaure Cc: michaelweghorn, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot,

D21514: MobileTextActionsToolBar: fix runtime warnings when controlRoot isn't set yet

dfaure closed this revision. REPOSITORY R858 Qt Quick Controls 2: Desktop Style REVISION DETAIL https://phabricator.kde.org/D21514 To: dfaure, mart, apol, broulik, ngraham Cc: astippich, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai,

Re: Unified git commit message guideline

e git commit log itself. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5

D23228: Port away from deprecated KWindowSystem API

dfaure created this revision. dfaure added reviewers: zzag, graesslin, davidedmundson. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. dfaure requested review of this revision. REVISION SUMMARY Call the all-in-one method (which handles numeric keypad better), then

D23334: Remove slideWindow(QWidget*) overload with recent KWindowSystem

dfaure created this revision. dfaure added a reviewer: zzag. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. dfaure requested review of this revision. TEST PLAN Fixes the build after https://phabricator.kde.org/D23213 REPOSITORY R130 Frameworks integration plugin

D23334: Remove slideWindow(QWidget*) overload with recent KWindowSystem

dfaure closed this revision. REPOSITORY R130 Frameworks integration plugin using KWayland REVISION DETAIL https://phabricator.kde.org/D23334 To: dfaure, zzag Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai,

D23198: Skip mime type check only for files on network mounts

dfaure added a comment. What I mean is: 1. improve KFileItem::isSlow to use this Solid code 2. improve KFileItem to enable SkipMimeTypeFromContent automatically if isSlow is true However maybe we don't want that in dolphin? Not sure. (In that case a new enum value can be added,

D23198: Skip mime type check only for files on network mounts

dfaure added a comment. Why not do this in KFileItem::isSlow() itself? (and enable SkipMimeTypeFromContent when isSlow is true) REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D23198 To: meven, #plasma, dfaure Cc: davidedmundson, ngraham, broulik,

D23198: Skip mime type check only for files on network mounts

dfaure added a comment. My suggestion is (still) "improve KFileItem to enable SkipMimeTypeFromContent automatically if isSlow is true" [possibly with another flag if you think there are actually use cases for mime-magic over network mounts] Then all this code isn't needed. REPOSITORY

D23228: Port away from deprecated KWindowSystem API

dfaure closed this revision. REPOSITORY R133 KScreenLocker REVISION DETAIL https://phabricator.kde.org/D23228 To: dfaure, zzag, graesslin, davidedmundson Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed,

D23228: Port away from deprecated KWindowSystem API

dfaure added a comment. Thanks for the testing! REPOSITORY R133 KScreenLocker REVISION DETAIL https://phabricator.kde.org/D23228 To: dfaure, zzag, graesslin, davidedmundson Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf,

D23808: Don't create() windows that aren't native, upon receiving a palette change event.

dfaure closed this revision. REPOSITORY R135 Integration for Qt applications in Plasma REVISION DETAIL https://phabricator.kde.org/D23808 To: dfaure, carewolf, davidedmundson, broulik Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot,

D23823: [KMenuEdit] port away from KStandardDirs

dfaure created this revision. dfaure added a reviewer: mlaurent. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. dfaure requested review of this revision. TEST PLAN only did smoke testing (kmenuedit starts and displays the tree, clicking on one item shows details).

D23823: [KMenuEdit] port away from KStandardDirs

dfaure closed this revision. REPOSITORY R103 KMenu Editor REVISION DETAIL https://phabricator.kde.org/D23823 To: dfaure, mlaurent Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg,

D24007: [QQC2 Desktop Style] Port away from deprecated methods in Qt 5.14

dfaure created this revision. dfaure added a reviewer: mart. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. dfaure requested review of this revision. REPOSITORY R858 Qt Quick Controls 2: Desktop Style BRANCH master REVISION DETAIL

D24007: [QQC2 Desktop Style] Port away from deprecated methods in Qt 5.14

dfaure closed this revision. REPOSITORY R858 Qt Quick Controls 2: Desktop Style REVISION DETAIL https://phabricator.kde.org/D24007 To: dfaure, mart, davidedmundson Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai,

D23808: Don't create() windows that aren't native, upon receiving a palette change event.

dfaure created this revision. dfaure added reviewers: carewolf, davidedmundson, broulik. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. dfaure requested review of this revision. TEST PLAN run kmail or falkon, then run kcmshell5 fonts, change font size ASSERT:

D19784: Avoid calling QT_LSTAT and accessing recent documents

dfaure added a comment. See KFileItem::isSlow() REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D19784 To: hoffmannrobert, #frameworks, dfaure, #dolphin Cc: meven, apol, ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot,

D22525: kioclient: Don't convert `:x:y` to `?line=x=y` for URLs starting with remote schemes.

dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. I suppose the kate developers like the fact that this currently works over FTP, SFTP, FISH, SMB, etc. So maybe only HTTP[S]/WEBDAV should be blacklisted (because there queries

D21517: Fix "Type error" when creating a TextField with focus: true

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit R858:e419e9b42287: Fix Type error when creating a TextField with focus: true (authored by dfaure). Herald added a project:

D22525: kioclient: Don't convert `:x:y` to `?line=x=y` for URLs starting with remote schemes.

dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. OK, I'm wrong (not the first time this happens) :-) `kwrite /home/dfaure/.zshrc:20` works `kwrite sftp://localhost/home/dfaure/.zshrc:20` doesn't work indeed. The patch looks

D22525: kioclient: Don't convert `:x:y` to `?line=x=y` for URLs starting with remote schemes.

dfaure added a comment. Backported. https://commits.kde.org/kde-cli-tools/42ef318a9d1c454f96b60181d8231a59233720ea REPOSITORY R126 KDE CLI Utilities REVISION DETAIL https://phabricator.kde.org/D22525 To: arrowd, #frameworks, dfaure Cc: wbauer, kwrite-devel, dfaure, cfeck, plasma-devel,

D24238: Port the pager applet away from QtWidgets

dfaure created this revision. dfaure added reviewers: hein, apol. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. dfaure requested review of this revision. TEST PLAN Builds REPOSITORY R119 Plasma Desktop BRANCH Plasma/5.17 REVISION DETAIL

D24238: Port the pager applet away from QtWidgets

dfaure added inline comments. INLINE COMMENTS > broulik wrote in windowmodel.cpp:81 > Isn't that the combined geometry of all screens, not just a single screen? Oh indeed. But then this was the wrong thing to use anyway. You don't want to center the window in relation to the combined geometry,

D24255: [startplasma] don't set QT_AUTO_SCREEN_SCALE_FACTOR with Qt >= 5.14

dfaure created this revision. dfaure added a reviewer: apol. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. dfaure requested review of this revision. REVISION SUMMARY It gives the following runtime warning: Warning: QT_AUTO_SCREEN_SCALE_FACTOR is deprecated.

D24255: [startplasma] don't set QT_AUTO_SCREEN_SCALE_FACTOR with Qt >= 5.14

dfaure closed this revision. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D24255 To: dfaure, apol, davidedmundson Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai,

D25488: kwayland-integration: fix compilation with no-deprecated build of kwindowsystem

dfaure created this revision. dfaure added reviewers: kossebau, jriddell, davidedmundson. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. dfaure requested review of this revision. REVISION SUMMARY I build kwindowsystem with -DEXCLUDE_DEPRECATED_BEFORE_AND_AT=5.62.0,

D24238: Port the pager applet away from QtWidgets

dfaure added a comment. Further research leads to this possible patch http://www.davidfaure.fr/2019/windowmodel.cpp.diff REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D24238 To: dfaure, hein, apol Cc: ognarb, broulik, plasma-devel, LeGast00n,

D24238: Port the pager applet away from QtWidgets

dfaure added a comment. Fix pushed https://commits.kde.org/plasma-desktop/2b5e86323f180f0c51ef9af898a69a522bc379ad after the bug reporter confirmed it fixes the bug. Reopening this after it was committed is a bit weird. I can't close it, please do. REPOSITORY R119 Plasma Desktop

D24238: Port the pager applet away from QtWidgets

dfaure added a comment. Ah, right. Thanks. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D24238 To: davidedmundson, hein, apol, dfaure Cc: ognarb, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham,

D24238: Port the pager applet away from QtWidgets

dfaure added a comment. Oh I see. Is there a non-deprecated method to get the overall geometry of all screens combined, or do we need to iterate over the list of screens? Surprising that this doesn't seem to be anywhere in the API !? REPOSITORY R119 Plasma Desktop REVISION

D24810: Ensure defintion of XDG_DATA_DIRS

dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. Bug in the commit log: s/ also make XDG_DATA_DIRS/ also make XDG_CONFIG_DIRS/ INLINE COMMENTS > startplasma.cpp:195 > } > + // Additionally also set default value for

D24996: Create directory when saving the menu file

dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Thanks for the fix! REPOSITORY R103 KMenu Editor REVISION DETAIL https://phabricator.kde.org/D24996 To: wbauer, dfaure Cc: dfaure, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh,

D24810: Ensure defintion of XDG_DATA_DIRS

dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. Please make sure the first line of the commit log says CONFIG and not DATA, too (the phab title still says DATA). INLINE COMMENTS > startplasma.cpp:197 > +if

D13360: Touchpad KDED module: Convert to JSON metadata

dfaure added a comment. I didn't fully follow this saga, but AFAICS this patch was reverted (commit 3432c3342b1f801 , bug 395622), so we still rely on desktop files to load the touchpad kded module. Can this patch

D24238: Port the pager applet away from QtWidgets

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit R119:674dd5f7e090: Port the pager applet away from QtWidgets (authored by dfaure). REPOSITORY R119 Plasma Desktop

D25488: kwayland-integration: fix compilation with no-deprecated build of kwindowsystem

dfaure closed this revision. REPOSITORY R130 Frameworks integration plugin using KWayland REVISION DETAIL https://phabricator.kde.org/D25488 To: dfaure, kossebau, jriddell, davidedmundson Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot,

D25488: kwayland-integration: fix compilation with no-deprecated build of kwindowsystem

dfaure added a comment. Thanks for the idea, done in https://commits.kde.org/kwayland-integration/3ce73d56ca5364242be3eb7cb1b5c4f5eaf47196 REPOSITORY R130 Frameworks integration plugin using KWayland REVISION DETAIL https://phabricator.kde.org/D25488 To: dfaure, kossebau, jriddell,

Re: 2 kirigami fixes for a point release

gt; That's an easy thing to enforce, and we know it makes a huge difference to > code. > > Even if no-one comments, the extra delay of it running on your own > system delivers a lot. I fully agree. (CC'ing the developer lists) -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5

Re: 2 kirigami fixes for a point release

ss that QtQuick doesn't make it easy, but that's part of the problem...). -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5

D27954: appstreamrunner: Port to KApplicationTrader

dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. Cool to see KApplicationTrader being used :-) INLINE COMMENTS > appstreamrunner.cpp:96 > + > +if (service->desktopEntryName() == componentId) > +

D27954: appstreamrunner: Port to KApplicationTrader

dfaure accepted this revision. dfaure added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > appstreamrunner.cpp:102 > +return true; > +} else { > +const auto renamedFrom = >

Re: Fwd: KDE CI: Administration » Dependency Build Plasma stable-kf5-qt5 FreeBSDQt5.14 - Build # 7 - Still Failing!

y Jazeix in commit 220a959bf150966f807ed. Cheers, David. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5

D28383: Add PageRouter component

dfaure added a comment. The unittest in this commit appears to break in CI.

Re: Fwd: KDE CI: Administration » Dependency Build Plasma stable-kf5-qt5 FreeBSDQt5.14 - Build # 7 - Still Failing!

On Saturday, April 18, 2020 5:06:43 PM CEST Friedrich W. H. Kossebau wrote: > Am Samstag, 18. April 2020, 16:26:57 CEST schrieb David Faure: > > On Saturday, April 18, 2020 7:09:25 AM CEST Ben Cooksley wrote: > > > Hi all, > > > > > > Please see below - any i

D28649: [Notifications] Port to upstream QConcatenateTablesProxyModel

dfaure added a comment. Wait, even if the jobs model is empty (no rows), what's its columnCount()? If that's fixed, then it should all be fine. Maybe my analysis (and testcase) is incorrect. Can you check the columnCount() of the Concat model? I assume the notifications model

D28649: [Notifications] Port to upstream QConcatenateTablesProxyModel

dfaure added a comment. OK I see why this is happening. KConcatenateProxyModel's columnCount is the number of columns of the first source model. Which leads to strange things if other models have less columns. QConcatenateProxyModel takes the smallest number of columns of all source

D28990: Port quicklaunch from KRun::run to CommandLauncherJob

dfaure added a comment. Agreed about notification delegate, will do. Does this really use KServiceAction anywhere? I don't see that. Confusing this one and D28988 ? [which however can't use the KServiceAction because of special support for %D and

D28988: Port hotplug from KRun::runCommand to CommandLauncherJob.

dfaure added a comment. As it is currently implemented, CommandLauncherJob is very unlikely to ever fail. It doesn't even fail on non-existing executable in the command (because it delegates to bash) so apart from QProcess cannot fork, it never fails. Probably not worth doing something

D28990: Port quicklaunch from KRun::run to CommandLauncherJob

dfaure updated this revision to Diff 80586. dfaure added a comment. Use KNotificationJobUiDelegate REPOSITORY R114 Plasma Addons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28990?vs=80568=80586 BRANCH master REVISION DETAIL https://phabricator.kde.org/D28990 AFFECTED

D28990: Port quicklaunch from KRun::run to CommandLauncherJob

dfaure created this revision. dfaure added reviewers: drosca, davidedmundson, broulik. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. dfaure requested review of this revision. TEST PLAN Builds, but untested. REPOSITORY R114 Plasma Addons BRANCH master REVISION

D28988: Port hotplug from KRun::runCommand to CommandLauncherJob.

dfaure created this revision. dfaure added reviewers: davidedmundson, sitter. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. dfaure requested review of this revision. REVISION SUMMARY Is there a better way to report error from this code than the dialog box that KRun

D28134: Add ColorUtils

dfaure added a comment. Breaks compilation with Qt 5.13 too. /data/kde/src/5/frameworks/kirigami/src/kirigamiplugin.cpp:248:122: error: invalid user-defined conversion from ‘KirigamiPlugin::registerTypes(const char*)::’ to ‘QObject* (*)(QQmlEngine*, QJSEngine*)’ [-fpermissive]

D28512: Port Konsole and Kate runners and dataengine away from KToolInvocation

dfaure added a comment. > I kinda feel CommandLauncherJob should have a QString executable, QStringList args constructor so I don't have to escape things myself? Sounds good, excellent idea. This would simplify the code in many callers. Please add :) It still needs to support the

D28286: [Task Manager] Port backend to ApplicationLauncherJob

dfaure added a comment. That wouldn't work either, you need to be able to choose between a Notification delegate, a Dialog delegate (which lives in a different library due to the QtWidgets dependency), and some more. REPOSITORY R119 Plasma Desktop REVISION DETAIL

D28286: [Task Manager] Port backend to ApplicationLauncherJob

dfaure added a comment. That would save only one line (the call to setUiDelegate). I prefer my earlier suggestion: job->setUiDelegate(new KNotificationJobUiDelegate(KJobUiDelegate::AutoErrorHandlingEnabled)); That one alone would save 2 lines ;) REPOSITORY R119 Plasma Desktop

D28286: WIP: [Task Manager] Port backend to ApplicationLauncherJob

dfaure added inline comments. INLINE COMMENTS > backend.cpp:178 > +auto *job = new KIO::ApplicationLauncherJob(service, > serviceAction); > +// TODO uidelegate > +job->start(); Only if you like messageboxes. Plasma code might prefer something else? A

<    1   2   3   4   5   >