D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes added a comment.


  @ngraham afaik powerdevil asks logind for system capabilites, with the 
CanSuspend, CanHibernate, etc. defined here 
.
 I added a new option if logind reports yes for CanSuspendThenHibernate option.

REPOSITORY
  R122 Powerdevil

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

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

D16365: share common values for both Breeze and Breeze-dark GTK themes

[Nathaniel Graham]

ngraham added a comment.


  @grmat, could you provide your full name and email address so I can add your 
authorship information? Thanks!

REPOSITORY
  R98 Breeze for Gtk

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

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

D13100: do not use buffered file IO

[Albert Astals Cid]

aacid accepted this revision.
aacid added a comment.
This revision is now accepted and ready to land.


  Please make me const-happy before commiting :)

INLINE COMMENTS

> pam_kwallet.c:696
>  char *salt = gcry_random_bytes(KWALLET_PAM_SALTSIZE, 
> GCRY_STRONG_RANDOM);
> -FILE *fd = fopen(path, "w");
> +int fd = open(path, O_CREAT | O_WRONLY | O_TRUNC | O_CLOEXEC, 0600);
>  

const

> pam_kwallet.c:704
>  
> -fwrite(salt, KWALLET_PAM_SALTSIZE, 1, fd);
> -fclose(fd);
> +ssize_t wlen = write(fd, salt, KWALLET_PAM_SALTSIZE);
> +close(fd);

const

> pam_kwallet.c:755
>  
> -FILE *fd = fopen(path, "r");
> -if (fd == NULL) {
> +int fd = open(path, O_RDONLY | O_CLOEXEC);
> +if (fd == -1) {

const

> pam_kwallet.c:764
>  char salt[KWALLET_PAM_SALTSIZE] = {};
> -const int bytesRead = fread(salt, 1, KWALLET_PAM_SALTSIZE, fd);
> -fclose(fd);
> +ssize_t bytesRead = read(fd, salt, KWALLET_PAM_SALTSIZE);
> +close(fd);

const

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

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

D16425: Added new Suspend then Hibernate option

[Nathaniel Graham]

ngraham added a comment.


  Very interesting.  Is there a programmatic way to detect hardware that would 
benefit from this?

REPOSITORY
  R122 Powerdevil

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

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

D16365: share common values for both Breeze and Breeze-dark GTK themes

[Nathaniel Graham]

ngraham added a comment.


  Nah, you did great! The only thing you could have done to make this easier 
would have been to submit two patches: one to templatize the code, and a second 
one to make functional changes. But Phabricator makes this workflow somewhat 
awkward and more difficult than it needs to be compared to Github for example, 
so I can understand why you didn't. Thanks again for your valuable 
contribution, and I think this will probably make it into Plasma 5.15!

REPOSITORY
  R98 Breeze for Gtk

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

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

D16365: share common values for both Breeze and Breeze-dark GTK themes

[mat gr]

grmat added a comment.


  Thanks for the comments, review and for splitting up the diff. I also know it 
was daunting to review the patch in its original form and I could've done that 
better.

REPOSITORY
  R98 Breeze for Gtk

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

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

D15786: share common values for both Breeze and Breeze-dark GTK themes

[mat gr]

grmat added a comment.


  Thanks for the review, comments and for splitting up the diff. I also know it 
was daunting to review the patch in its original form and I could've done that 
better.
  I'll see if I can test the updated version in the upcoming days.

REPOSITORY
  R98 Breeze for Gtk

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

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

D16425: Added new Suspend then Hibernate option

[Alejandro Valdes]

avaldes created this revision.
avaldes added a reviewer: broulik.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
avaldes requested review of this revision.

REVISION SUMMARY
  See bug 399727  for a good 
description of what this code is for.
  
  The new ui will show a new option like the following image F6349860: 
screenshot.png 

REPOSITORY
  R122 Powerdevil

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

AFFECTED FILES
  daemon/actions/bundled/handlebuttoneventsconfig.cpp
  daemon/actions/bundled/suspendsession.cpp
  daemon/actions/bundled/suspendsession.h
  daemon/actions/bundled/suspendsessionconfig.cpp
  daemon/backends/upower/login1suspendjob.cpp
  daemon/backends/upower/powerdevilupowerbackend.cpp
  daemon/backends/upower/upowersuspendjob.cpp
  daemon/org.freedesktop.PowerManagement.xml
  daemon/powerdevilbackendinterface.h
  daemon/powerdevilfdoconnector.cpp
  daemon/powerdevilfdoconnector.h
  daemon/powerdevilpowermanagement.cpp
  daemon/powerdevilpowermanagement.h
  kcmodule/activities/activitywidget.cpp

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

D12498: Fully remove `Application Name` from Details panel

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R121:eb9c4c080427: Fully remove `Application Name` from 
Details panel (authored by sharvey, committed by bruns).

REPOSITORY
  R121 Policykit (Polkit) KDE Agent

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12498?vs=44184=44206

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

AFFECTED FILES
  AuthDialog.cpp
  AuthDialog.h
  authdetails.ui

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

D16417: Improve replacement text when action description is not provided

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R121:9cf71f424950: Improve replacement text when action 
description is not provided (authored by bruns).

REPOSITORY
  R121 Policykit (Polkit) KDE Agent

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16417?vs=44183=44205

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

AFFECTED FILES
  AuthDialog.cpp

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

D15093: Add WireGuard capability.

[Bruce Anderson]

andersonbruce marked 9 inline comments as done.
andersonbruce added inline comments.

INLINE COMMENTS

> jgrulich wrote in plasmanetworkmanagement_wireguardui.desktop:16
> Name could be just WireGuard I guess

I left this alone because it is consistent with the names of other VPNs.

> jgrulich wrote in wireguardadvancedwidget.cpp:87
> This validation doesn't seem to work, I can click on "ok" button even when 
> the advanced widget is empty. Also it's a bit weird  that the function name 
> suggests something else then it's doing. I would suggest splitting this into 
> something like "updatePresharedKeyValidation()" and to the one you have now. 
> One would really do just the check, the other would update the color.
> 
> Same please do for classic wireguard widget.

I did this slightly different from the suggestion but it only does the checks 
once per change and I think it is correctly enabling/disabling the "Ok" button. 
By the way, the "Ok" button is enabled when all entries on the Advanced widget 
are blank because all of the entries are optional and therefore all blank is 
perfectly valid.

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.


  A couple of notes on the latest upload:
  
  1. I left the background color change code alone. Doing searches on changing 
widget background color, it looks like the preferred method is to use 
setPalette() which requires a QPalette, and after trying a couple of things I 
decided it made more sense to me to pass around two palettes by reference than 
to pass just the colors and create palettes when they were needed.
  
  2. I backed out the change that used a blank setting rather than the stored 
values. I agree that it would be nice to not hang on to any extraneous elements 
if they happened to be created by some other application but what I found was 
that I had somehow not tested it after making this change and when I did, I 
found that it had broken the entire "Advanced" widget settings. The problem is 
that if you start with a blank setting and don't open the Advanced Widget then 
all the settings in that are lost. To fix this I would have to duplicate a 
bunch of functionality in both widgets which I don't like doing. Also, if the 
underlying NetworkManager app adds something I think that it is probably better 
to not delete it if plasma-nm hasn't been updated yet so I ended up deciding to 
put it back to the way it was.

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 updated this revision to Diff 44198.
andersonbruce added a comment.


  - Address review comments

REPOSITORY
  R116 Plasma Network Management Applet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15093?vs=42290=44198

BRANCH
  Feature/AddWireGuardVPN

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

AFFECTED FILES
  libs/models/kcmidentitymodel.cpp
  vpn/CMakeLists.txt
  vpn/wireguard/CMakeLists.txt
  vpn/wireguard/nm-wireguard-service.h
  vpn/wireguard/plasmanetworkmanagement_wireguardui.desktop
  vpn/wireguard/wireguard.cpp
  vpn/wireguard/wireguard.h
  vpn/wireguard/wireguard.ui
  vpn/wireguard/wireguardadvanced.ui
  vpn/wireguard/wireguardadvancedwidget.cpp
  vpn/wireguard/wireguardadvancedwidget.h
  vpn/wireguard/wireguardauth.cpp
  vpn/wireguard/wireguardauth.h
  vpn/wireguard/wireguardauth.ui
  vpn/wireguard/wireguardkeyvalidator.cpp
  vpn/wireguard/wireguardkeyvalidator.h
  vpn/wireguard/wireguardwidget.cpp
  vpn/wireguard/wireguardwidget.h

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