D15306: BookmarksRunner: Avoid multiple connections of identical signal

broulik added a comment. Can't you connect to the slot directly and use `Qt::UniqueConnection`? INLINE COMMENTS > bookmarksrunner.cpp:66 > +m_browser = browser; > +connect(this, &Plasma::AbstractRunner::teardown, [this]() { > m_browser->teardown(); }); > +} Please provid

D14895: Plasmashell freezes when trying to get free space info from mounted remote filesystem after losing connection to it

McPain added a subscriber: ngraham. McPain added a comment. In D14895#320987 , @ngraham wrote: > +1 for the concept. No opinion about the notification, but I think it's fine. Removing myself as a reviewer since I don't feel qualified to offer a

D15304: [KCM] Port label-bearing controls to QQC2 to fix fractional scaling support

drosca added a comment. I'd prefer to port it completely to QQC2. As it is now, in some files you just changed Labels with QQC2 import and in others you changed QQC1->QQC2 import while there are also other QQC items. REPOSITORY R115 Plasma Audio Volume Applet REVISION DETAIL https://pha

D15093: Add WireGuard capability.

andersonbruce added inline comments. INLINE COMMENTS > jgrulich wrote in wireguard.cpp:71 > You can use simpleipv[4,6]validator we have in plasma-nm instead of using > everything below. Or maybe QHostAddress can validate it for you? The problem with using the simpleipv[4,6]validator is that Wir

D15093: Add WireGuard capability.

andersonbruce added a comment. In D15093#320599 , @jgrulich wrote: > I think you can completely remove WireguardAuth dialog if there is no use for it. I also spotted few trailing spaces in the patch, please remove them. The 'askUser' func

D15306: BookmarksRunner: Avoid multiple connections of identical signal

bruns created this revision. bruns added a reviewer: Plasma. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. bruns requested review of this revision. REVISION SUMMARY The factory returns the same object when the browser name is not changed. Connecting the signal again

D15305: KRunner: remove no longer existant and unused column from SQL query

bruns created this revision. bruns added a reviewer: Plasma. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. bruns requested review of this revision. REVISION SUMMARY The favicon_id is not used in the query results, and may not even exist when the places db has been cr

D15304: [KCM] Port label-bearing controls to QQC2 to fix fractional scaling support

ngraham created this revision. ngraham added reviewers: Plasma, drosca. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. ngraham requested review of this revision. REVISION SUMMARY This patch ports label-bearing controls to use QQC2, which fixes fractional scaling suppor

D12040: Add wallpaperplugin.knsrc + QML function to open GHNS dialog

ngraham added a comment. Plasma 5.14.0 will depend on Frameworks 5.50 (see https://community.kde.org/Schedules/Plasma_5), which is why this has to go into Plasma 5.15.0. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D12040 To: Zren, #plasma Cc: ngraham, c

D15011: [Kickoff] Make the search field always look like a search field

ngraham added a comment. It looks like we've generally achieved consensus on the visual changes. Could folks review D15194 and D15206 ? As for the focus ring, I could use a hand with this. I tried editing the SVG to re

D15286: Improve arrow key navigation of Kicker search results

ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. Much better, thanks! Verified that the bug is fixed now, and I can't find any behavioral regressions. REPOSITORY R119 Plasma Desktop BRANCH Plasma/5.13 REVISION DETAIL https://pha

D14895: Plasmashell freezes when trying to get free space info from mounted remote filesystem after losing connection to it

ngraham resigned from this revision. ngraham added a comment. +1 for the concept. No opinion about the notification, but I think it's fine. Removing myself as a reviewer since I don't feel qualified to offer a code review. But sounds like you've got a shipit! You need someone to land it for

D15024: Update icons kcm docbook to 5.13

This revision was automatically updated to reflect the committed changes. Closed by commit R119:6864d6fee4a2: Update icons kcm docbook to 5.13 (authored by lueck). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D15024?vs=40526&id=41067#toc REPOSITORY R119 Plasma Desktop CHANGES SINCE L

D15162: [Folder View] Disable the actions themselves rather than just not adding them to the menu

hein accepted this revision. hein added a comment. This revision is now accepted and ready to land. Good patch! REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D15162 To: broulik, #plasma, davidedmundson, hein Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezh

D15286: Improve arrow key navigation of Kicker search results

hein updated this revision to Diff 41061. hein added a comment. Fix. REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15286?vs=41036&id=41061 BRANCH Plasma/5.13 REVISION DETAIL https://phabricator.kde.org/D15286 AFFECTED FILES applets/kicker/p

D15297: Use QOverload to select overloaded functions

volkov closed this revision. REPOSITORY R116 Plasma Network Management Applet REVISION DETAIL https://phabricator.kde.org/D15297 To: volkov, jgrulich Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15286: Improve arrow key navigation of Kicker search results

hein added a comment. Indeed. Sorry, I introduced a small bug with a fatal copy and paste mistake just before uploading. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D15286 To: hein, ngraham Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed

D15269: [Folder View] Add checkbox for toggling previews

This revision was automatically updated to reflect the committed changes. Closed by commit R119:b2ae7ecf2035: [Folder View] Add checkbox for toggling previews (authored by broulik). REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15269?vs=41040&id=41058

D15297: Use QOverload to select overloaded functions

volkov created this revision. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. volkov requested review of this revision. REPOSITORY R116 Plasma Network Management Applet BRANCH master REVISION DETAIL https://phabricator.kde.org/D15297 AFFECTED FILES kded/notifica

D15287: [Logout Dialog] Add "Hibernate" option

ngraham accepted this revision as: VDG. ngraham added a comment. +1. Nice that it doesn't appear if hibernate isn't supported. Keyboard navigation still works when hibernate isn't shown too. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D15287 To: broulik,

D15286: Improve arrow key navigation of Kicker search results

ngraham requested changes to this revision. ngraham added a comment. This revision now requires changes to proceed. Hmm, this doesn't quite fix the issue for me. After searching for something, the top item in the list is still highlighted as before, but with this patch when I press the down a

D15233: Add a tooltip for the appentry in the kicker

abetts added a comment. In D15233#320616 , @underwit wrote: > Screenshot with tooltip > F6236529: Screenshot_20180905_105259.png > > For tooltip i use "comment", "genericname" or "name" from .desktop

D14384: Renamed dot to square in variables and filenames

piotrkakol added a comment. Yes, but I forgeted to put them in the last diff. Do I really have to make a separate diff to change it? REPOSITORY R114 Plasma Addons REVISION DETAIL https://phabricator.kde.org/D14384 To: piotrkakol, davidedmundson Cc: zzag, gladhorn, ngraham, davidedmundso

D14384: Renamed dot to square in variables and filenames

zzag added a comment. w.r.t. this patch, those 2 copyright changes look really unrelated. REPOSITORY R114 Plasma Addons REVISION DETAIL https://phabricator.kde.org/D14384 To: piotrkakol, davidedmundson Cc: zzag, gladhorn, ngraham, davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, l

D14384: Renamed dot to square in variables and filenames

piotrkakol added a comment. I did change main.qml as you can see here: https://phabricator.kde.org/R114:9eae090a3038ebf97ffeee8c9b773a8610e13021 REPOSITORY R114 Plasma Addons REVISION DETAIL https://phabricator.kde.org/D14384 To: piotrkakol, davidedmundson Cc: gladhorn, ngraham, davide

D14384: Renamed dot to square in variables and filenames

davidedmundson requested changes to this revision. davidedmundson added a comment. This revision now requires changes to proceed. this is putting big assumptions about the UI in the code and doesn't make things better. INLINE COMMENTS > main.qml:3 > * Copyright 2014 Joseph Wenninger > + *

D14384: Renamed dot to square in variables and filenames

piotrkakol added a comment. Ping. REPOSITORY R114 Plasma Addons REVISION DETAIL https://phabricator.kde.org/D14384 To: piotrkakol Cc: gladhorn, ngraham, davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15289: Use lambda instead of QSignalMapper

volkov closed this revision. REPOSITORY R111 KSysguard Library REVISION DETAIL https://phabricator.kde.org/D15289 To: volkov, #plasma, broulik Cc: broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15291: [Folder View] Compare UDS entry times directly instead of going through KFileItem

broulik created this revision. broulik added reviewers: Plasma, hein. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. broulik requested review of this revision. REVISION SUMMARY This avoids creating a QDateTime object with all the timezone processing that comes with it

D15289: Use lambda instead of QSignalMapper

volkov updated this revision to Diff 41045. volkov added a comment. revert change of actionTriggered's signature REPOSITORY R111 KSysguard Library CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15289?vs=41043&id=41045 BRANCH master REVISION DETAIL https://phabricator.kde.org

D15289: Use lambda instead of QSignalMapper

broulik added inline comments. INLINE COMMENTS > ksysguardprocesslist.h:49 > */ > class Q_DECL_EXPORT KSysGuardProcessList : public QWidget > { That class is exported, so changing method signatures will be ABI-incompatible. Not sure how many guarantees we give for libksysguard but I would s

D15289: Use lambda instead of QSignalMapper

volkov created this revision. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. volkov requested review of this revision. REVISION SUMMARY QSignalMapper is obsolete and the code with lambda is shorter and clearer. REPOSITORY R111 KSysguard Library BRANCH master REVI

D15268: [Folder View] Use KIO::PreviewJob::defaultPlugins() for default plugins

This revision was automatically updated to reflect the committed changes. Closed by commit R119:d4839a256cb7: [Folder View] Use KIO::PreviewJob::defaultPlugins() for default plugins (authored by broulik). REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D1

D15093: Add WireGuard capability.

jgrulich added a comment. In D15093#320672 , @andersonbruce wrote: > In D15093#320577 , @jgrulich wrote: > > > > > > > > > Does wg-quick support both, like simple commands and script files

D15269: [Folder View] Add checkbox for toggling previews

broulik updated this revision to Diff 41040. broulik added a comment. - Change wording to "Show Previews" REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15269?vs=40989&id=41040 REVISION DETAIL https://phabricator.kde.org/D15269 AFFECTED FILES c

D15287: [Logout Dialog] Add "Hibernate" option

broulik created this revision. broulik added reviewers: Plasma, VDG. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. broulik requested review of this revision. REVISION SUMMARY We offer both Suspend and Hibernate in the application launcher so we should do the same here

D15286: Improve arrow key navigation of Kicker search results

hein created this revision. hein added a reviewer: ngraham. Herald added a project: Plasma. hein requested review of this revision. REVISION SUMMARY Kicker already makes sure the first search result is highlighted and actionable while searching. This patch makes arrow-down after typing move

D15093: Add WireGuard capability.

andersonbruce marked an inline comment as done. andersonbruce added a comment. In D15093#320577 , @jgrulich wrote: > > Does wg-quick support both, like simple commands and script files? If so, we should support both as well, if it s

D15285: [kstyle] Fix deprecation warnings

This revision was automatically updated to reflect the committed changes. Closed by commit R31:7e1c0fb1fc3d: [kstyle] Fix deprecation warnings (authored by zzag). REPOSITORY R31 Breeze CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15285?vs=41031&id=41032 REVISION DETAIL https://p

D15285: [kstyle] Fix deprecation warnings

zzag created this revision. zzag added a reviewer: Plasma. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. zzag requested review of this revision. REVISION SUMMARY See https://build.kde.org/job/Plasma%20breeze%20kf5-qt5%20FreeBSDQt5.11/4/warnings12Result/ TEST PLAN C

D15233: Add a tooltip for the appentry in the kicker

underwit added a comment. Screenshot with tooltip F6236529: Screenshot_20180905_105259.png REVISION DETAIL https://phabricator.kde.org/D15233 To: underwit, #plasma Cc: abetts, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreut

D15233: Add a tooltip for the appentry in the kicker

underwit updated this revision to Diff 41027. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15233?vs=40896&id=41027 REVISION DETAIL https://phabricator.kde.org/D15233 AFFECTED FILES applets/kicker/package/contents/ui/ItemListDelegate.qml applets/kicker/plugin/abstractentry.cpp

D15093: Add WireGuard capability.

jgrulich added a comment. I think you can completely remove WireguardAuth dialog if there is no use for it. I also spotted few trailing spaces in the patch, please remove them. INLINE COMMENTS > wireguard.cpp:189 > +} else { // Error condition > +return result; >

D15093: Add WireGuard capability.

jgrulich added a comment. In D15093#319336 , @andersonbruce wrote: > In D15093#319253 , @pino wrote: > > > Much better now! > > > > - regarding the UI for all the pre/post scripts: since they are