[Differential] [Commented On] D4690: Import remote ioslave from plasma-workspace
dfaure added a comment.
Looks good to me, just found two more minor things.
INLINE COMMENTS
> remoteimpl.cpp:31
> +
> +#include
> +
probably unused at this point
> remoteimpl.cpp:87
> +QDir dir = *dirpath;
> +if (!dir.exists()) {
> +continue;
The QDir object and this check are now redundant. It's all covered by the
QFileInfo::exists check below.
REPOSITORY
R241 KIO
BRANCH
import-remote
REVISION DETAIL
https://phabricator.kde.org/D4690
EMAIL PREFERENCES
https://phabricator.kde.org/settings/panel/emailpreferences/
To: elvisangelaccio, dfaure, davidedmundson
Cc: ltoscano, #plasma, #frameworks
[Differential] [Commented On] D4663: Allow setting the timeout value.
mck182 added a comment. > I don't think that KNotification as a framework should hide this feature from the developer. I, on the other hand, don't see why the developer should need this feature; as I said before - there's no scenario where the app should want either really short or really long time on the screen - this should always be fully up to the shell displaying the notifications. If you do need to have the notification bubble on the screen for really long time, then imho that's a wrong solution to the problem it'd be solving. And that applies to this case as well. KDE's Bluetooth integration is solving the same problem (pairing devices) and it uses a dialog, which imho makes sense, because with notification you're supposed to inform the user that something has happened and he may or may not react to that something, while what you're doing requires direct user interaction and what you're trying to solve is the possible lack of that direct interaction. So I honestly think that a notification is not even the right approach here altogether. > they do that using the notifications cross-desktop standard. So I wouldn't criticize Gnome for doing their own thing here. Oh really? :) Ever wondered why the spec you're referring to is full of only Gnome's extensions and is hosted on Gnome's servers? It's because Gnome considers the Galago project, the actual cross-desktop notifications project, dead. They took over libnotify and they consider that implementation to be the canonical upstream for the specification. In other words, Gnome has full control over that spec, can do what they want and can just as easily block any of our contributions. Sure, it is based and backwards-compatible with the cross-desktop standard, but it stopped being actual cross-desktop standard long time ago. If you feel like changing that, this would be a good start: https://bugzilla.gnome.org/show_bug.cgi?id=748145 > @colomar: This would be perfect, but it would need a whole lot of work from both the desktop environment and the application side. It's actually not that hard as far as the last three points go. This can be fully implemented in the notification's server (ie. Plasma) and the apps changes would be minimal. I even had a branch doing exactly that, I just never had the time to finish it. But basically the idea is - every new notification that arrives to the server is directly mapped to a KSNI - title to title, text to subtitle, actions to SNI actions. The lifecycle is a bit tricky (eg. when it is not needed anymore) but still can be done. I meant to mostly mimic what Android is doing. The only part remaining to solve is the drawer that has all the SNIs and their action in some sensible way. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D4663 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: albertvaka, #frameworks, apol Cc: colomar, mck182, #frameworks
[Differential] [Commented On] D4663: Allow setting the timeout value.
colomar added a comment. I agree that what Android does makes a lot of sense. What they have is - a permanent icon in the top bar for each application that still has an open notification - basically an SNI, minus the direct interactivity (which makes sense given that tiny icons are not much fun to interact with on a touchscreen) - a drawer that shows all notifications that are still valid (plus the same on the lockscreen if enabled) I have not seen a better notification system anywhere so far, to be honest. It would make perfect sense for me if Plasma did exactly the same, but we'd still need to change a couple of things for that to work: - We'd need a drawer to show all notifications that are still valid (similar to the notification history that we had in Plasma 4, but with some very important differences, see below) - We'd need to automatically create an SNI for every application that sends a notification - Notifications would need to be grouped by application (the notification drawer on iOS shows what a horrible mess you get if you don't do that) - Notifications would need to be cleaned up by applications when no longer needed (this would need a new API for applications) This would be perfect, but it would need a whole lot of work from both the desktop environment and the application side. As long as we can't get there, I agree with Martin that an SNI is a better solution than a permanently shown popup notification, as it attracts attention without covering anything on the screen. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D4663 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: albertvaka, #frameworks, apol Cc: colomar, mck182, #frameworks
[Differential] [Request, 15 lines] D4713: Fix binding loop regression in FrameSVGItem
davidedmundson created this revision. Restricted Application added projects: Plasma, Frameworks. Restricted Application added a subscriber: plasma-devel. REVISION SUMMARY https://phabricator.kde.org/R242:d8a1a9eb084b19e552c789244267f7346e1b27a8 introduces an unintended code change, resizeFrame() updates the margins and in turns calls repaintNeeded. This isn't needed and is a binding loop if we ever have a frameSVGItem whose size depends on it's own margins. resizeFrame is different from setEnabledBorders / setElementPrefix / theme changes because even though we need to create a new FrameData we know any hints and margins won't change. FrameSvgItem::updateSizes doesn't depend on the size in any way. The old code did pointlessly call updateSizes, but didn't emit anything. This patch that introduces a flag to updateFrameData to determine if we should re-evaluate size hints or not. TEST PLAN GDB showed where the loop was. Read the old code, and looked for differences Ran plasmashell, checked I had no binding loop, frames including button which have composeOverBorder which need the new FrameData all rendered correctly. REPOSITORY R242 Plasma Framework (Library) BRANCH master REVISION DETAIL https://phabricator.kde.org/D4713 AFFECTED FILES src/plasma/framesvg.cpp src/plasma/private/framesvg_p.h EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: davidedmundson, #plasma, #frameworks Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
[Differential] [Commented On] D4663: Allow setting the timeout value.
albertvaka added a comment. I agree that just rising the timeout is suboptimal but I still like it better than a modal dialog... Actually, notifications were invented as a way to avoid harassing the user with modal dialogs when apps need attention. Apart from that, and even if we end up implementing this in a diferent way for KDE Connect, I don't think that KNotification as a framework should hide this feature from the developer. Regardless of the notification server implementation, the underlying protocol that KNotification exposes does allow defining a timeout. And, about SNIs... It's true that they have a title, text and actions like notifications. But they also happen to be represented in every major desktop (eg: Windows, MacOS, Gnome, Unity and Plasma) as an icon in the system tray that you have to right click to access a context menu with the actions (unlike notifications). Are you sure that's what Android does? Gnome implements something much more similar to the notification systems nowadays present in Android and iOS (and even Windows and MacOS, which are trying to deprecate their system tray icons), and they do that using the notifications cross-desktop standard. So I wouldn't criticize Gnome for doing their own thing here. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D4663 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: albertvaka, #frameworks, apol Cc: mck182, #frameworks
[Differential] [Requested Changes] D4619: Printing: Respect footer font, fix footer vertical position, make header/footer separator line visually lighter
dhaumann requested changes to this revision. dhaumann added a comment. This revision now requires changes to proceed. I tested this patch: I think, the footer's height is not correct when the box is disabled, see: http://i.imgur.com/Te5XigY.png When the box is activated, it has the same height as the header. I think it should always have the same height. Could you have a look again? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D4619 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: atomsymbol, kfunk, mwolff, dhaumann Cc: dhaumann, kwrite-devel, #frameworks
[Differential] [Request, 14 lines] D4711: Ungrab mouse on menu close
anthonyfieroni created this revision. Restricted Application added projects: Plasma, Frameworks. Restricted Application added subscribers: Frameworks, plasma-devel. REVISION SUMMARY Qt 5.8 has a bit changed and ungrabbing at show does not work. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D4711 AFFECTED FILES src/scriptengines/qml/plasmoid/appletinterface.cpp src/scriptengines/qml/plasmoid/containmentinterface.cpp EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: anthonyfieroni, #plasma, broulik, mart Cc: plasma-devel, #frameworks, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
[Differential] [Commented On] D4690: Import remote ioslave from plasma-workspace
elvisangelaccio added a comment. In https://phabricator.kde.org/D4690#88350, @ltoscano wrote: > Is it an import with the git history as well? Initially I wanted to do that, but then I decided against it because it's not worth the effort imho. There are very few changes and a lot of merge commits (seeing "Merge branch Plasma/5.x" in the kio git log would be weird). You can see the instructions I'm planning to push in the first commit message (from the "Commits" tab of this review). REPOSITORY R241 KIO BRANCH import-remote REVISION DETAIL https://phabricator.kde.org/D4690 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: elvisangelaccio, dfaure, davidedmundson Cc: ltoscano, #plasma, #frameworks
[Differential] [Commented On] D4690: Import remote ioslave from plasma-workspace
ltoscano added a comment. Is it an import with the git history as well? REPOSITORY R241 KIO BRANCH import-remote REVISION DETAIL https://phabricator.kde.org/D4690 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: elvisangelaccio, dfaure, davidedmundson Cc: ltoscano, #plasma, #frameworks
[Differential] [Commented On] D4690: Import remote ioslave from plasma-workspace
elvisangelaccio added a comment. In https://phabricator.kde.org/D4690#88306, @dfaure wrote: > I know this isn't your code and you're just moving it to solve a dependency issue. No problem, go ahead. > > But while at it I took at it, I look at the code and found a few issues, feel free to fix them ;) > > Unittests would be good, too OK ok I'm pushing it :) No problem, thanks for spotting the issues :) REPOSITORY R241 KIO BRANCH import-remote REVISION DETAIL https://phabricator.kde.org/D4690 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: elvisangelaccio, dfaure, davidedmundson Cc: #plasma, #frameworks
[Differential] [Updated, 909 lines] D4690: Import remote ioslave from plasma-workspace
elvisangelaccio updated this revision to Diff 11597. elvisangelaccio marked 2 inline comments as done. elvisangelaccio added a comment. - renamed kded module to allow co-installability with plasma-workspace <= 5.9 - fixed issues spotted by David REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D4690?vs=11560=11597 BRANCH import-remote REVISION DETAIL https://phabricator.kde.org/D4690 AFFECTED FILES src/ioslaves/CMakeLists.txt src/ioslaves/remote/CMakeLists.txt src/ioslaves/remote/kdedmodule/CMakeLists.txt src/ioslaves/remote/kdedmodule/remotedirnotify.cpp src/ioslaves/remote/kdedmodule/remotedirnotify.desktop src/ioslaves/remote/kdedmodule/remotedirnotify.h src/ioslaves/remote/kdedmodule/remotedirnotifymodule.cpp src/ioslaves/remote/kdedmodule/remotedirnotifymodule.h src/ioslaves/remote/kio_remote.cpp src/ioslaves/remote/kio_remote.h src/ioslaves/remote/remote.json src/ioslaves/remote/remoteimpl.cpp src/ioslaves/remote/remoteimpl.h EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: elvisangelaccio, dfaure, davidedmundson Cc: #plasma, #frameworks
[Differential] [Closed] D4688: [FrameSvgItemMargins] Don't update on repaintNeeded
This revision was automatically updated to reflect the committed changes. Closed by commit R242:408b31166227: [FrameSvgItemMargins] Don't update on repaintNeeded (authored by broulik). REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D4688?vs=11550=11595 REVISION DETAIL https://phabricator.kde.org/D4688 AFFECTED FILES src/declarativeimports/core/framesvgitem.cpp EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: broulik, #plasma, davidedmundson, drosca Cc: plasma-devel, #frameworks, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
[Differential] [Updated] D4707: move setImagePath logic into updateFrameData()
mart edited the test plan for this revision. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D4707 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: mart Cc: #frameworks
Jenkins-kde-ci: kio master kf5-qt5 » Linux,gcc - Build # 433 - Fixed!
GENERAL INFO BUILD SUCCESS Build URL: https://build.kde.org/job/kio%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/433/ Project: PLATFORM=Linux,compiler=gcc Date of build: Tue, 21 Feb 2017 16:28:38 + Build duration: 12 min CHANGE SET Revision a942db1a0dd5462127b5f7c276690a0694b2daf1 by kde: ([PreviewJob] Remove maximum size for local files by default) change: edit src/widgets/previewjob.cpp JUNIT RESULTS Name: (root) Failed: 0 test(s), Passed: 53 test(s), Skipped: 0 test(s), Total: 53 test(s) COBERTURA RESULTS Cobertura Coverage Report PACKAGES 21/21 (100%)FILES 273/342 (80%)CLASSES 273/342 (80%)LINE 29736/51623 (58%)CONDITIONAL 16330/38757 (42%) By packages autotests FILES 66/66 (100%)CLASSES 66/66 (100%)LINE 7918/8240 (96%)CONDITIONAL 4426/8660 (51%) autotests.http FILES 9/9 (100%)CLASSES 9/9 (100%)LINE 543/544 (100%)CONDITIONAL 200/336 (60%) autotests.kcookiejar FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 179/198 (90%)CONDITIONAL 60/90 (67%) src.core FILES 97/117 (83%)CLASSES 97/117 (83%)LINE 8068/14178 (57%)CONDITIONAL 4426/9259 (48%) src.core.kssl FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 35/93 (38%)CONDITIONAL 3/6 (50%) src.filewidgets FILES 26/36 (72%)CLASSES 26/36 (72%)LINE 3452/7559 (46%)CONDITIONAL 1282/4381 (29%) src.gui FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 104/110 (95%)CONDITIONAL 46/72 (64%) src.ioslaves.file FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 447/849 (53%)CONDITIONAL 330/749 (44%) src.ioslaves.http FILES 8/8 (100%)CLASSES 8/8 (100%)LINE 1758/3780 (47%)CONDITIONAL 1269/3460 (37%) src.ioslaves.http.kcookiejar FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 621/782 (79%)CONDITIONAL 607/839 (72%) src.ioslaves.trash FILES 8/10 (80%)CLASSES 8/10 (80%)LINE 715/1139 (63%)CONDITIONAL 411/833 (49%) src.ioslaves.trash.tests FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 686/764 (90%)CONDITIONAL 445/936 (48%) src.kioslave FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 14/27 (52%)CONDITIONAL 5/10 (50%) src.kntlm FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 373/385 (97%)CONDITIONAL 111/138 (80%) src.kpasswdserver FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 377/594 (63%)CONDITIONAL 280/580 (48%) src.kpasswdserver.autotests FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 283/286 (99%)CONDITIONAL 146/256 (57%) src.urifilters.fixhost FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 25/34 (74%)CONDITIONAL 36/54 (67%) src.urifilters.ikws FILES 5/10 (50%)CLASSES 5/10 (50%)LINE 242/727 (33%)CONDITIONAL 150/546 (27%) src.urifilters.localdomain FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 21/29 (72%)CONDITIONAL 16/26 (62%) src.urifilters.shorturi FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 237/266 (89%)CONDITIONAL 332/412 (81%) src.widgets FILES 32/64 (50%)CLASSES 32/64 (50%)LINE 3638/11039 (33%)CONDITIONAL 1749/7114 (25%)
Jenkins-kde-ci: kio master kf5-qt5 » Linux,gcc - Build # 433 - Fixed!
GENERAL INFO BUILD SUCCESS Build URL: https://build.kde.org/job/kio%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/433/ Project: PLATFORM=Linux,compiler=gcc Date of build: Tue, 21 Feb 2017 16:28:38 + Build duration: 12 min CHANGE SET Revision a942db1a0dd5462127b5f7c276690a0694b2daf1 by kde: ([PreviewJob] Remove maximum size for local files by default) change: edit src/widgets/previewjob.cpp JUNIT RESULTS Name: (root) Failed: 0 test(s), Passed: 53 test(s), Skipped: 0 test(s), Total: 53 test(s) COBERTURA RESULTS Cobertura Coverage Report PACKAGES 21/21 (100%)FILES 273/342 (80%)CLASSES 273/342 (80%)LINE 29736/51623 (58%)CONDITIONAL 16330/38757 (42%) By packages autotests FILES 66/66 (100%)CLASSES 66/66 (100%)LINE 7918/8240 (96%)CONDITIONAL 4426/8660 (51%) autotests.http FILES 9/9 (100%)CLASSES 9/9 (100%)LINE 543/544 (100%)CONDITIONAL 200/336 (60%) autotests.kcookiejar FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 179/198 (90%)CONDITIONAL 60/90 (67%) src.core FILES 97/117 (83%)CLASSES 97/117 (83%)LINE 8068/14178 (57%)CONDITIONAL 4426/9259 (48%) src.core.kssl FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 35/93 (38%)CONDITIONAL 3/6 (50%) src.filewidgets FILES 26/36 (72%)CLASSES 26/36 (72%)LINE 3452/7559 (46%)CONDITIONAL 1282/4381 (29%) src.gui FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 104/110 (95%)CONDITIONAL 46/72 (64%) src.ioslaves.file FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 447/849 (53%)CONDITIONAL 330/749 (44%) src.ioslaves.http FILES 8/8 (100%)CLASSES 8/8 (100%)LINE 1758/3780 (47%)CONDITIONAL 1269/3460 (37%) src.ioslaves.http.kcookiejar FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 621/782 (79%)CONDITIONAL 607/839 (72%) src.ioslaves.trash FILES 8/10 (80%)CLASSES 8/10 (80%)LINE 715/1139 (63%)CONDITIONAL 411/833 (49%) src.ioslaves.trash.tests FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 686/764 (90%)CONDITIONAL 445/936 (48%) src.kioslave FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 14/27 (52%)CONDITIONAL 5/10 (50%) src.kntlm FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 373/385 (97%)CONDITIONAL 111/138 (80%) src.kpasswdserver FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 377/594 (63%)CONDITIONAL 280/580 (48%) src.kpasswdserver.autotests FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 283/286 (99%)CONDITIONAL 146/256 (57%) src.urifilters.fixhost FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 25/34 (74%)CONDITIONAL 36/54 (67%) src.urifilters.ikws FILES 5/10 (50%)CLASSES 5/10 (50%)LINE 242/727 (33%)CONDITIONAL 150/546 (27%) src.urifilters.localdomain FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 21/29 (72%)CONDITIONAL 16/26 (62%) src.urifilters.shorturi FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 237/266 (89%)CONDITIONAL 332/412 (81%) src.widgets FILES 32/64 (50%)CLASSES 32/64 (50%)LINE 3638/11039 (33%)CONDITIONAL 1749/7114 (25%)
[Differential] [Abandoned] D4362: RFC: [AppletQuickItem] Cache QQmlComponent used for settings QtQuick Controls 1 style
broulik abandoned this revision. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D4362 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: broulik, #plasma Cc: mart, davidedmundson, plasma-devel, #frameworks, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
[Differential] [Updated, 169 lines] D4707: move setImagePath logic into updateFrameData()
mart updated this revision to Diff 11593. mart added a comment. - less svg renderer creation REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D4707?vs=11589=11593 BRANCH phab/framesvgrefactor REVISION DETAIL https://phabricator.kde.org/D4707 AFFECTED FILES src/plasma/framesvg.cpp src/plasma/private/framesvg_p.h src/plasma/svg.cpp EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: mart Cc: #frameworks
[Differential] [Planned Changes To] D4707: move setImagePath logic into updateFrameData()
mart planned changes to this revision. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D4707 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: mart Cc: #frameworks
[Differential] [Closed] D4552: [PreviewJob] Remove maximum size for local files by default
This revision was automatically updated to reflect the committed changes. Closed by commit R241:a942db1a0dd5: [PreviewJob] Remove maximum size for local files by default (authored by broulik). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D4552?vs=11185=11592#toc REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D4552?vs=11185=11592 REVISION DETAIL https://phabricator.kde.org/D4552 AFFECTED FILES src/widgets/previewjob.cpp EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: broulik, emmanuelp, hein, dfaure Cc: #frameworks
[Differential] [Accepted] D4690: Import remote ioslave from plasma-workspace
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.
I know this isn't your code and you're just moving it to solve a dependency
issue. No problem, go ahead.
But while at it I took at it, I look at the code and found a few issues, feel
free to fix them ;)
Unittests would be good, too OK ok I'm pushing it :)
INLINE COMMENTS
> remoteimpl.cpp:42
> +if (!dir.exists()) {
> +dir.cdUp();
> +dir.mkdir(QStringLiteral("remoteview"));
This should probably be QDir().mkpath(path) instead of the whole if exists +
cdUp + mkdir dance, which would fail if the parent doesn't exist either (ok,
unlikely, but anyway, mkpath is shorter and safer)
> remoteimpl.cpp:96
> +
> +QStringList filenames = dir.entryList(QDir::Files | QDir::Readable);
> +
Wow this is convoluted, a simple QFile::exists(*dirpath + '/' + filename)
should be enough rather than a full directory listing.
> remoteimpl.cpp:155
> +if (service && service->isValid()) {
> +
> url.setPath(QStandardPaths::locate(QStandardPaths::ApplicationsLocation,
> +
> QStringLiteral("%1.desktop").arg(WIZARD_SERVICE)));
wrong port to QUrl, should be url = QUrl::fromLocalFile(...)
REPOSITORY
R241 KIO
BRANCH
import-remote
REVISION DETAIL
https://phabricator.kde.org/D4690
EMAIL PREFERENCES
https://phabricator.kde.org/settings/panel/emailpreferences/
To: elvisangelaccio, dfaure, davidedmundson
Cc: #plasma, #frameworks
[Differential] [Updated] D4552: [PreviewJob] Remove maximum size for local files by default
dfaure added a comment. No objection. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D4552 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: broulik, emmanuelp, hein, dfaure Cc: #frameworks
[Differential] [Commented On] D4552: [PreviewJob] Remove maximum size for local files by default
broulik added a comment. David? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D4552 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: broulik, dfaure, emmanuelp, hein Cc: #frameworks
[Differential] [Planned Changes To] D4707: move setImagePath logic into updateFrameData()
mart planned changes to this revision. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D4707 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: mart Cc: #frameworks
[Differential] [Request, 164 lines] D4707: move setImagePath logic into updateFrameData()
mart created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY make sure the framedata creation/destruction is completely in updateFrameData, makes easier to track and possible to use the repaintsblocked logic. now only one framedata instance should be created at startup. CCBUG:376754 TEST PLAN autotests pass, plasma runs ok, crash on 376754 not reproducible anymore TODO: benchmark REPOSITORY R242 Plasma Framework (Library) BRANCH phab/framesvgrefactor REVISION DETAIL https://phabricator.kde.org/D4707 AFFECTED FILES src/plasma/framesvg.cpp src/plasma/private/framesvg_p.h EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: mart Cc: #frameworks
[Differential] [Commented On] D4663: Allow setting the timeout value.
mck182 added a comment. Well, I would argue that Android is using exactly indicators which we know as SNIs. It's the same thing - it's an icon in your top panel and when you pull the panel down, you see the title, text and available actions. This is _exactly_ what SNI does, it just so happens that on desktop the representation is always an icon with context menu. It has title, text and actions and is often accompanied by a popup notification. That said, I realize you want to target as broad userbase as possible, but if Gnome decides to not be cross-desktop compatible, well screw 'em, their fault. It bothers me that people always have to find alternate, many times sub-par solutions and the reason is "Gnome" that never wants to play along with about anything. But I digress. I still think having a notification sticked to the screen with indefinitely-high timeout is not the solution. If only for the unpredictability of servers. Given the user has to initiate the pairing and the user is then expected and required to complete the pairing, how about using some modal way to confirm the pairing? Like a popup dialog? I mean if the point of the interaction is to complete the pairing and if it's imperative that the pairing completes, then just put it in front of the user square and center to confirm. Raising the notification timeout for this to me is a workaround at best. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D4663 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: albertvaka, #frameworks, apol Cc: mck182, #frameworks
[Differential] [Closed] D4702: IconItemTest: Fix loadPixmap and loadImage tests
This revision was automatically updated to reflect the committed changes. Closed by commit R242:148ff6311bfe: IconItemTest: Fix loadPixmap and loadImage tests (authored by drosca). REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D4702?vs=11582=11583 REVISION DETAIL https://phabricator.kde.org/D4702 AFFECTED FILES autotests/iconitemtest.cpp EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: drosca, #plasma, davidedmundson Cc: plasma-devel, #frameworks, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
[Differential] [Accepted] D4702: IconItemTest: Fix loadPixmap and loadImage tests
davidedmundson accepted this revision. This revision is now accepted and ready to land. REPOSITORY R242 Plasma Framework (Library) BRANCH master REVISION DETAIL https://phabricator.kde.org/D4702 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: drosca, #plasma, davidedmundson Cc: plasma-devel, #frameworks, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
[Differential] [Request, 2 lines] D4702: IconItemTest: Fix loadPixmap and loadImage tests
drosca created this revision. Restricted Application added projects: Plasma, Frameworks. Restricted Application added subscribers: Frameworks, plasma-devel. REVISION SUMMARY It only works on CI because data/test_image.png size is the same as implicitSize of IconItem (KIconLoader::Dialog). TEST PLAN Test pass locally (KIconLoader::Dialog == 64) REPOSITORY R242 Plasma Framework (Library) BRANCH master REVISION DETAIL https://phabricator.kde.org/D4702 AFFECTED FILES autotests/iconitemtest.cpp EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: drosca, #plasma, davidedmundson Cc: plasma-devel, #frameworks, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
[Differential] [Commented On] D4689: IconItem: Add roundToIconSize property
drosca added a comment. Yes, I want to use it in plasma-pa applet for volume indicator icons (the small icon next to slider). Currently, they are too small but next round icon size is already too big. This change will make it possible to make them just few pixels bigger. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D4689 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: drosca, #plasma Cc: davidedmundson, plasma-devel, #frameworks, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
[Differential] [Commented On] D4633: WIP: Updated folder decrypted and encrypted icons
alex-l added a comment. I'd make the chess background full the folder's front. REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D4633 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: ivan, andreaska, alex-l Cc: #frameworks
[Differential] [Updated] D4633: WIP: Updated folder decrypted and encrypted icons
alex-l added a comment. I'd make the chess background full the folder's front. REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D4633 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: ivan, andreaska, alex-l Cc: #frameworks
