D15306: BookmarksRunner: Avoid multiple connections of identical signal

[Kai Uwe Broulik]

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 provide `this` as context.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D15306

To: bruns, #plasma
Cc: broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

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

[Oleg Solovyov]

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 
code review. But sounds like you've got a shipit! You need someone to land it 
for you, right?
  
  
  Right. Nobody gave me a commit access yet

REVISION DETAIL
  https://phabricator.kde.org/D14895

To: McPain, broulik
Cc: ngraham, anthonyfieroni, davidedmundson, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

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

[David Rosca]

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://phabricator.kde.org/D15304

To: ngraham, #plasma, drosca
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15093: Add WireGuard capability.

[Bruce Anderson]

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 WireGuard requires 
that in some cases the IP address can include a CIDR at the end (e.g. 
10.2.4.6/32) and in another case the entry needs to have a port number (e.g. 
10.2.3.5:7642) and the simpleip* functions won't handle either of these as 
currently written. QHostAddress will handle the CIDR but not the port number 
version. Also I'm not really thrilled with how QHostAddress does its 
verification. I know that it is technically correct but my personal opinion is 
that "1/32" should not be accepted as a valid IPv4 address which QHostAddress 
does.

REPOSITORY
  R116 Plasma Network Management Applet

REVISION DETAIL
  https://phabricator.kde.org/D15093

To: andersonbruce, #plasma, jgrulich, pino
Cc: acrouthamel, K900, pino, lbeltrame, ngraham, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15093: Add WireGuard capability.

[Bruce Anderson]

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' function currently returns the authWidget. Since it is pure 
virtual, I need to implement it somehow, so if I delete the authWidget, should 
it just return a nullptr instead?

REPOSITORY
  R116 Plasma Network Management Applet

REVISION DETAIL
  https://phabricator.kde.org/D15093

To: andersonbruce, #plasma, jgrulich, pino
Cc: acrouthamel, K900, pino, lbeltrame, ngraham, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15306: BookmarksRunner: Avoid multiple connections of identical signal

[Stefan Brüns]

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 leads to multiple calls to the slot each
  time the signal is emitted.
  
  See also T9626 

TEST PLAN
  1. Add some debug output to the teardown() slot
  2. Open the krunner multiple times and enter some query
  3. teardown() is called exactly once

REPOSITORY
  R120 Plasma Workspace

BRANCH
  T9626

REVISION DETAIL
  https://phabricator.kde.org/D15306

AFFECTED FILES
  runners/bookmarks/bookmarksrunner.cpp

To: bruns, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

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

[Stefan Brüns]

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 created with FF 58 or later. In case the
  column does not exist the query fails completely.
  
  BUG: 398305

TEST PLAN
  1. Create a new FF profile
  2. Add some bookmarks
  3. Do some queries

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D15305

AFFECTED FILES
  runners/bookmarks/browsers/firefox.cpp

To: bruns, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

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

[Nathaniel Graham]

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 support.
  
  BUG: 397954
  FIXED-IN: 5.14.0

TEST PLAN
  All functionality tested still worked.
  
  No visual change at 1x or 2x scale factor.
  
  Improvement at fractional scale factors. Here's 1.3x:
  [images go here]

REPOSITORY
  R115 Plasma Audio Volume Applet

BRANCH
  factional-scale-factor-support (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D15304

AFFECTED FILES
  src/kcm/package/contents/ui/Advanced.qml
  src/kcm/package/contents/ui/Applications.qml
  src/kcm/package/contents/ui/CardListItem.qml
  src/kcm/package/contents/ui/DeviceComboBox.qml
  src/kcm/package/contents/ui/DeviceListItem.qml
  src/kcm/package/contents/ui/Header.qml
  src/kcm/package/contents/ui/StreamListItem.qml
  src/kcm/package/contents/ui/VolumeSlider.qml

To: ngraham, #plasma, drosca
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

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

[Nathaniel Graham]

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, cfeck, mart, davidedmundson, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

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

[Nathaniel Graham]

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 reduce the size of the active focus ring 
by removing the outer lighter line that increases its size, but the changed SVG 
was just sized back up to the original dimensions and looked even stronger. Am 
I missing something?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D15011

To: ngraham, #plasma, #vdg, davidedmundson, abetts
Cc: acrouthamel, fabianr, huftis, rooty, sharvey, romangg, broulik, 
safaalfulaij, oysteins, filipf, abetts, davidedmundson, michaeltunnell, 
plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D15286: Improve arrow key navigation of Kicker search results

[Nathaniel Graham]

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://phabricator.kde.org/D15286

To: hein, ngraham
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

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

[Nathaniel Graham]

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 
you, right?

REVISION DETAIL
  https://phabricator.kde.org/D14895

To: McPain, broulik
Cc: anthonyfieroni, davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15024: Update icons kcm docbook to 5.13

[Burkhard Lück]

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 LAST UPDATE
  https://phabricator.kde.org/D15024?vs=40526&id=41067

REVISION DETAIL
  https://phabricator.kde.org/D15024

AFFECTED FILES
  doc/kcontrol/icons/delete-theme.png
  doc/kcontrol/icons/edit-delete.png
  doc/kcontrol/icons/edit-undo.png
  doc/kcontrol/icons/effects.png
  doc/kcontrol/icons/index.docbook
  doc/kcontrol/icons/install-theme.png
  doc/kcontrol/icons/main.png
  doc/kcontrol/icons/size.png
  doc/kcontrol/icons/use-of-icons.png

To: lueck, #plasma, #documentation, yurchor
Cc: pino, abetts, plasma-devel, kde-doc-english, ragreen, Pitel, ZrenBot, 
skadinna, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

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

[Eike Hein]

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, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15286: Improve arrow key navigation of Kicker search results

[Eike Hein]

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/package/contents/ui/MenuRepresentation.qml
  applets/kicker/package/contents/ui/RunnerResultsList.qml

To: hein, ngraham
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15297: Use QOverload to select overloaded functions

[Alexander Volkov]

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

[Eike Hein]

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, 
jensreuterberg, abetts, sebas, apol, mart

D15269: [Folder View] Add checkbox for toggling previews

[Kai Uwe Broulik]

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

REVISION DETAIL
  https://phabricator.kde.org/D15269

AFFECTED FILES
  containments/desktop/package/contents/ui/FolderViewLayer.qml
  containments/desktop/plugins/folder/viewpropertiesmenu.cpp
  containments/desktop/plugins/folder/viewpropertiesmenu.h

To: broulik, #plasma, hein, #vdg
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15297: Use QOverload to select overloaded functions

[Alexander Volkov]

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/notification.cpp
  libs/declarative/networkstatus.cpp
  libs/editor/connectioneditorbase.cpp
  libs/editor/settings/bondwidget.cpp
  libs/editor/settings/connectionwidget.cpp
  libs/editor/settings/ipv4widget.cpp
  libs/editor/settings/ipv6widget.cpp
  libs/editor/settings/security802-1x.cpp
  libs/editor/settings/vlanwidget.cpp
  libs/editor/settings/wificonnectionwidget.cpp
  libs/editor/settings/wifisecurity.cpp
  libs/editor/settings/wiredconnectionwidget.cpp
  libs/editor/widgets/bssidcombobox.cpp
  libs/editor/widgets/hwaddrcombobox.cpp
  libs/editor/widgets/mobileconnectionwizard.cpp
  libs/editor/widgets/passwordfield.cpp
  libs/editor/widgets/settingwidget.cpp
  libs/editor/widgets/ssidcombobox.cpp
  vpn/openconnect/openconnectauth.cpp
  vpn/openvpn/openvpnadvancedwidget.cpp
  vpn/ssh/sshwidget.cpp
  vpn/vpnc/vpnc.cpp

To: volkov
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15287: [Logout Dialog] Add "Hibernate" option

[Nathaniel Graham]

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, #plasma, #vdg
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15286: Improve arrow key navigation of Kicker search results

[Nathaniel Graham]

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 arrow key once, the selection rect disappears! When I 
press it again, the top item becomes selected again. Only on a third press of 
the down arrow key does selection move to the next item.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D15286

To: hein, ngraham
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15233: Add a tooltip for the appentry in the kicker

[Andres Betts]

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 file
  
  
  Thank you

REVISION DETAIL
  https://phabricator.kde.org/D15233

To: underwit, #plasma
Cc: abetts, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

D14384: Renamed dot to square in variables and filenames

[Piotr Kąkol]

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, davidedmundson, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D14384: Renamed dot to square in variables and filenames

[Vlad Zagorodniy]

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, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D14384: Renamed dot to square in variables and filenames

[Piotr Kąkol]

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, davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D14384: Renamed dot to square in variables and filenames

[David Edmundson]

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 
> + * Copyright 2018 Piotr Kąkol 
>   *

you can't do this on files you didn't change

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D14384

To: piotrkakol, davidedmundson
Cc: gladhorn, ngraham, davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D14384: Renamed dot to square in variables and filenames

[Piotr Kąkol]

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

[Alexander Volkov]

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

[Kai Uwe Broulik]

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 since we're only interested in the relative order, not absolute 
precise date time values.
  
  CHANGELOG: Sorting files by date in Folder View is significantly faster now

TEST PLAN
  Having my Downloads ordner as FV sorted ascending by date went from 60ms to 
under 1ms
  Files still appear sorted correctly
  Did the same change in Dolphin a while back

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D15291

AFFECTED FILES
  containments/desktop/plugins/folder/foldermodel.cpp

To: broulik, #plasma, hein
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15289: Use lambda instead of QSignalMapper

[Alexander Volkov]

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

AFFECTED FILES
  processui/ksysguardprocesslist.cpp

To: volkov, #plasma
Cc: broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15289: Use lambda instead of QSignalMapper

[Kai Uwe Broulik]

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 still prefer 
to keep breakages to a minimum.

You could just keep the `qobject_cast`

REPOSITORY
  R111 KSysguard Library

REVISION DETAIL
  https://phabricator.kde.org/D15289

To: volkov, #plasma
Cc: broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15289: Use lambda instead of QSignalMapper

[Alexander Volkov]

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

REVISION DETAIL
  https://phabricator.kde.org/D15289

AFFECTED FILES
  processui/ksysguardprocesslist.cpp
  processui/ksysguardprocesslist.h

To: volkov
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

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

[Kai Uwe Broulik]

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/D15268?vs=40988&id=41041

REVISION DETAIL
  https://phabricator.kde.org/D15268

AFFECTED FILES
  containments/desktop/package/contents/config/main.xml
  containments/desktop/plugins/folder/foldermodel.cpp
  containments/desktop/plugins/folder/foldermodel.h
  containments/desktop/plugins/folder/previewpluginsmodel.cpp

To: broulik, #plasma, #vdg, hein
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15093: Add WireGuard capability.

[Jan Grulich]

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? If so, 
we should support both as well, if it supports only commands/snippets, we 
should leave it as it is.
  >
  > .
  >  Given this new information as well as the fact that there is a disconnect 
between what wg-quick wants and what the NM addon takes in, most notably, 
wg-quick specifically accepts multiple instances of each but the NM addon only 
allows one line of input.
  >
  > I would therefore propose that I remove all of the Pre/Post Up/Down entries 
for now since they won't do anything anyway and worry about adding them back in 
if the NM addon implements them properly and then match its implementation.
  
  
  Not sure this is a good plan either, problem is that once they are added, it 
will take for us some time to get them back and we will have to wait also for 
the next Plasma release, as you are not allowed to introduce new strings in 
stable releases.  I would keep them and hope that eventually they will get 
properly implemented by NM-Wireguard plugin. On the other hand, if those 
options are something not commonly used, it shouldn't bother anyone if we don't 
support them from the beginning. I'll leave this decision to you, I don't 
really have strong preference on this.

REPOSITORY
  R116 Plasma Network Management Applet

REVISION DETAIL
  https://phabricator.kde.org/D15093

To: andersonbruce, #plasma, jgrulich, pino
Cc: acrouthamel, K900, pino, lbeltrame, ngraham, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15269: [Folder View] Add checkbox for toggling previews

[Kai Uwe Broulik]

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
  containments/desktop/package/contents/ui/FolderViewLayer.qml
  containments/desktop/plugins/folder/viewpropertiesmenu.cpp
  containments/desktop/plugins/folder/viewpropertiesmenu.h

To: broulik, #plasma, hein, #vdg
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15287: [Logout Dialog] Add "Hibernate" option

[Kai Uwe Broulik]

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
  
  BUG: 398184
  FIXED-IN: 5.14.0

TEST PLAN
  F6236719: Screenshot_20180905_121112.png 


REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D15287

AFFECTED FILES
  lookandfeel/contents/logout/Logout.qml

To: broulik, #plasma, #vdg
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15286: Improve arrow key navigation of Kicker search results

[Eike Hein]

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 to the second search result instead of merely moving
  focus from the search field to the list, so the user doesn't have
  to press arrow-down twice anymore.
  
  It also allows using arrow-left/right to jump to the other result
  columns, when the cursor is at the start or end of the text field,
  respectively.
  
  BUG:397779

REPOSITORY
  R119 Plasma Desktop

BRANCH
  Plasma/5.13

REVISION DETAIL
  https://phabricator.kde.org/D15286

AFFECTED FILES
  applets/kicker/package/contents/ui/MenuRepresentation.qml

To: hein, ngraham
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15093: Add WireGuard capability.

[Bruce Anderson]

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 supports only commands/snippets, we 
should leave it as it is.
  
  OK, it's more complicated than I thought.
  
  As a little background 'wg-quick' is a shell script that comes with the 
WireGuard kernel module and is provided to give a simple method of setting up 
basic connections. If you are trying to do something complex, I think you have 
to do it yourself with a combination of 'wg' (a lower level command that also 
come with the WireGuard package) and other ordinary networking commands. As far 
as I can tell all of this code is well done (even Linus "loved" it)
  The NetworkManager WireGuard addon on the other hand is not an official part 
of NetworkManager (even if it is linked to on their site) and they aren't doing 
any official upkeep on it so it is sort of you get what you get. Unfortunately 
that is somebody's Senior Thesis project. I want to make clear that i am not 
saying anything bad about the author, he is up front about the origins of the 
code and it certainly works well enough that I have been using it for a few 
months now and well enough that I wanted to take the time to write the 
plasma-nm code to go along with it.
  
  All that being said, the NM addon is not complete. I knew this before because 
it do didn't anything with the DNS field (even though it had an entry and it 
stored it) and I had to add it in myself because I needed it for my use case. 
Until tonight when I did some experimenting, however, I didn't know that the 
Pre/Post Up/Down entries are like that too where they are entered but not used 
when it calls wg-quick.
  
  Given this new information as well as the fact that there is a disconnect 
between what wg-quick wants and what the NM addon takes in, most notably, 
wg-quick specifically accepts multiple instances of each but the NM addon only 
allows one line of input.
  
  I would therefore propose that I remove all of the Pre/Post Up/Down entries 
for now since they won't do anything anyway and worry about adding them back in 
if the NM addon implements them properly and then match its implementation.

REPOSITORY
  R116 Plasma Network Management Applet

REVISION DETAIL
  https://phabricator.kde.org/D15093

To: andersonbruce, #plasma, jgrulich, pino
Cc: acrouthamel, K900, pino, lbeltrame, ngraham, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15285: [kstyle] Fix deprecation warnings

[Vlad Zagorodniy]

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://phabricator.kde.org/D15285

AFFECTED FILES
  kstyle/breezestyle.cpp
  kstyle/breezewindowmanager.cpp

To: zzag, #plasma, broulik
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15285: [kstyle] Fix deprecation warnings

[Vlad Zagorodniy]

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
  Compiles.

REPOSITORY
  R31 Breeze

BRANCH
  fix-depracation-warnings

REVISION DETAIL
  https://phabricator.kde.org/D15285

AFFECTED FILES
  kstyle/breezestyle.cpp
  kstyle/breezewindowmanager.cpp

To: zzag, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15233: Add a tooltip for the appentry in the kicker

[Ivan Razzhivin]

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, 
jensreuterberg, sebas, apol, mart

D15233: Add a tooltip for the appentry in the kicker

[Ivan Razzhivin]

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
  applets/kicker/plugin/abstractentry.h
  applets/kicker/plugin/abstractmodel.cpp
  applets/kicker/plugin/actionlist.h
  applets/kicker/plugin/appentry.cpp
  applets/kicker/plugin/appentry.h
  applets/kicker/plugin/appsmodel.cpp

To: underwit, #plasma
Cc: abetts, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

D15093: Add WireGuard capability.

[Jan Grulich]

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;
> +}

You multiple times return just result, you should also set error message with 
the reason so users know why the import failed.

> wireguardadvancedwidget.cpp:104
> +
> +setOrClear(data, QLatin1String(NM_WG_KEY_LISTEN_PORT), 
> m_ui->listenPortLineEdit->displayText());
> +setOrClear(data, QLatin1String(NM_WG_KEY_MTU), 
> m_ui->mTULineEdit->displayText());

I would prefer having just an empty map with data where you just set everything 
the user configured in UI, removing options from existing data map might work, 
but if someone configure a connection somewhere else with options we don't 
support, they will stay there as you will not remove them. Also change the 
setOrClear() function to something like setProperty(const NMStringMap &data, 
const QString &key, const QString &value).

> wireguardwidget.cpp:31
> +
> +#include 
> +#include 

Not needed.

> wireguardwidget.cpp:129
> +{
> +// Currently WireGuard does not have any secrets
> +}

Add Q_UNUSED(setting);

> wireguardwidget.h:33
> +
> +class QUrl;
> +class QLineEdit;

QUrl and QLineEdit not needed.

> wireguardwidget.h:41
> +explicit WireGuardSettingWidget(const NetworkManager::VpnSetting::Ptr 
> &setting,
> +const struct WireGuardRegexStrings 
> &strings,
> +QWidget *parent = 0);

This paramater can be removed if you switch to using our IPv4 and IPv6 
validators, also you can completely remove wireguardstrings structure you 
defined. Even if anything like this would be needed, you basically need it in 
one class, why not defining it there? If our validators are not enought, then 
they should be improved so every class using them can benefit from that.

REPOSITORY
  R116 Plasma Network Management Applet

REVISION DETAIL
  https://phabricator.kde.org/D15093

To: andersonbruce, #plasma, jgrulich, pino
Cc: acrouthamel, K900, anthonyfieroni, pino, lbeltrame, ngraham, plasma-devel, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D15093: Add WireGuard capability.

[Jan Grulich]

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 file 
paths, better use a KUrlRequester widget (limited to local existing files only, 
no URLs), so the users have a Browse button next to each line edit that can be 
used to open a file dialog
  >
  >
  > I debated with myself when I started this whether to include these at all. 
They are included in the base NetworkManager implementation which "inherited" 
them from the underlying wg-quick command but they duplicate functionality that 
NM provides directly and it seems to me that if someone is using NM then they 
can use those methods instead. Also, wg-quick specifies these as "script 
snippets" meaning actual direct commands that are executed by bash not 
necessarily a shell script. It also specifies that there can be multiple 
instances of each, a capability that the base NM implementation does not 
support. So my quandary is, do I implement this like the base NM does and 
possibly, as you suggest, force it to be a single shell script which sort of 
violates the spirit of the wg-quick command or do I delete it completely and 
not support something that base NM does, or do I leave it like it is?
  >
  > Personally I think that the base NM should get rid of these and force users 
to rely on the capability in NM to perform pre and post operations but given 
what exists, I don't think any of the alternatives are good and I'm not sure 
what the "least bad" solution is. If someone uses nm-connection-editor and 
enters something which is not a script and then opens the connection in a 
plasma-nm interface which only supports a file, I'm not sure what will happen. 
On the other hand if I delete the fields completely and open something created 
in nm-connection-editor with these fields, that's not good either.
  >
  > Since I initially was doing this only for my own use and was probably going 
to use NM for this, I admit that I took the easiest way out and duplicated what 
base NM has, which is a single string which can contain a shell script but also 
a snippet as the base WireGuard does and then said in the tool-tip that it was 
preferable to use NM capability instead.
  >
  > If you as a representative of the plasma-nm philosophy have a preference on 
which way to go or have a brilliant idea which solves all the problems, I will 
follow your lead.
  
  
  Does wg-quick support both, like simple commands and script files? If so, we 
should support both as well, if it supports only commands/snippets, we should 
leave it as it is.

INLINE COMMENTS

> plasmanetworkmanagement_wireguardui.desktop:17
> +Name=WireGuard
> +Comment=Handles WireGuard VPN connections

Maybe use better description, most of the plugins say "Compatible with Foo 
servers" so perhaps "Compatible with WireGuard VPN servers"

> wireguard.ui:46
> +
> + Address (IPv6)
> +

Use "Property name:" for each property, every label in plasma-nm use colon at 
the end of property name.

REPOSITORY
  R116 Plasma Network Management Applet

REVISION DETAIL
  https://phabricator.kde.org/D15093

To: andersonbruce, #plasma, jgrulich, pino
Cc: acrouthamel, K900, anthonyfieroni, pino, lbeltrame, ngraham, plasma-devel, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart