date:20180902

D15093: Add WireGuard capability.

2018-09-02 Thread Jan Grulich

jgrulich added inline comments.

INLINE COMMENTS

> wireguard.cpp:71
> +{
> +regexStrings.ip4Range = new QString(
> +"(?:[0-1]?[0-9]?[0-9]|2[0-4][0-9]|25[0-5])");

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?

> wireguard.cpp:185
> +}
> +else if (addressIn.first.protocol() == 
> QAbstractSocket::NetworkLayerProtocol::IPv6Protocol) {
> +dataMap.insert(QLatin1String(NM_WG_KEY_ADDR_IP6), 
> addressList[i]);

Coding style.

> wireguard.cpp:215
> +}
> +else {
> +return result;

Coding style.

> wireguard.cpp:318
> +if (!haveAddress || !havePrivateKey || !havePublicKey || 
> !haveAllowedIps) {
> +
> +mError = VpnUiPlugin::Error;

Remove space.

> wireguard.h:34
> +public:
> +explicit WireGuardUiPlugin(QObject* parent = nullptr, const 
> QVariantList& = QVariantList());
> +~WireGuardUiPlugin() override;

Coding style. You mix funcName(Bar* foo) with funcName(Bar * foo) and 
funcName(Bar *foo), plese change it all to the last one. Same goes for 
functions below.

> wireguardadvancedwidget.h:55
> +
> +private Q_SLOTS:
> +

Can be removed if you don't have any private slot.

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.

2018-09-02 Thread Pino Toscano

pino requested changes to this revision.
pino added a comment.
This revision now requires changes to proceed.


  Much better now!
  
  General notes:
  
  - there are various checks on lengths of strings like `str.length() > 0` or 
`str.length() != 0`: if all you need is check whether a string is empty or not, 
just use `str.isEmpty()`
  - 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

INLINE COMMENTS

> nm-wireguard-service.h:2
> +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */
> +/* nm-openvpn-service - openvpn integration with NetworkManager
> + *

this comment needs to be fixed

> wireguard.cpp:25
> +#include 
> +#include 
> +#include 

unused

> wireguard.cpp:29-30
> +#include 
> +#include 
> +#include 
> +#include 

both unused

> wireguard.cpp:41
> +
> +#include 
> +

unused

> wireguard.cpp:60
> +#define NMV_WG_TAG_FWMARK"FwMark"
> +#define NMV_WG_ASSIGN"="
> +

unused

> wireguard.cpp:151-153
> +KConfig importFile(fileName, KConfig::NoGlobals);
> +KConfigGroup interfaceGroup = importFile.group(NMV_WG_TAG_INTERFACE);
> +KConfigGroup peerGroup = importFile.group(NMV_WG_TAG_PEER);;

make all 3 as `const`, so it is clear they are read-only (and attempts to use 
writeEntry() will result in build failures)

> wireguard.cpp:153
> +KConfigGroup interfaceGroup = importFile.group(NMV_WG_TAG_INTERFACE);
> +KConfigGroup peerGroup = importFile.group(NMV_WG_TAG_PEER);;
> +

extra ';'

> wireguard.cpp:166-172
> +// The config file must have both [Interface] and [Peer] sections
> +if (!importFile.groupList().contains("Interface")
> +|| !importFile.groupList().contains("Peer")) {
> +mError = VpnUiPlugin::Error;
> +mErrorMessage = i18n("Could not open file");
> +return result;
> +}

These checks can be moved right after reading the config file and getting the 
KConfigGroup objects.

> wireguard.cpp:167-168
> +// The config file must have both [Interface] and [Peer] sections
> +if (!importFile.groupList().contains("Interface")
> +|| !importFile.groupList().contains("Peer")) {
> +mError = VpnUiPlugin::Error;

You do not need to get the list of groups in the config file (twice): earlier 
you get the KConfigGroup objects for the groups, so checking eg 
`interfaceGroup.exists()` should do the job.

> wireguard.cpp:175-178
> +value = interfaceGroup.readEntry(NMV_WG_TAG_ADDRESS);
> +{
> +QStringList addressList;
> +addressList << value.split(QRegExp("\\s*,\\s*"));

KConfig already supports comma-separated lists -- just pass QStringList() as 
`default` value to readEntry(), so KConfigGroup knows the value is a list.

> wireguard.cpp:195
> +// Listen Port
> +value = interfaceGroup.readEntry(NMV_WG_TAG_LISTEN_PORT);
> +if (value.length() > 0) {

If you specify `0` as default parameter, KConfigGroup will try to decode the 
value as integer automatically. If `0` is a valid value for the port, then use 
`-1`.
After doing that, you do not need a validator anymore, just a manual range 
check will do the job.

> wireguard.cpp:209
> +QRegExp validatorRegex(*regexStrings.keySpec);
> +QRegExpValidator validator(validatorRegex);
> +int pos = 0;

this validator is not needed, just use `validatorRegex` directly

> wireguard.cpp:231
> +// MTU
> +value = interfaceGroup.readEntry(NMV_WG_TAG_MTU);
> +if (value.length() > 0) {

same as for the listen port: please read the value directly as integer.

> wireguard.cpp:271
> +QRegExp validatorRegex(*regexStrings.keySpec);
> +QRegExpValidator validator(validatorRegex);
> +int pos = 0;

as above, no need for a validator, just use the QRegExp directly

> wireguard.cpp:288
> + + ", *)*" + *regexStrings.ip4Orip6Address);
> +QRegExpValidator *validator = new QRegExpValidator(allowedIPsRegex, 
> this);
> +if (validator->validate(value, pos) != QValidator::State::Invalid) {

as above, no need for a validator, just use the QRegExp directly

> wireguard.cpp:306
> +QRegExp validatorRegex(*regexStrings.keySpec);
> +QRegExpValidator validator(validatorRegex);
> +int pos = 0;

as above, no need for a validator, just use the QRegExp directly

> wireguard.cpp:350-353
> +value = dataMap[NM_WG_KEY_ADDR_IP4];
> +if (dataMap.contains(QLatin1String(NM_WG_KEY_ADDR_IP6))) {
> +value += "," + dataMap[NM_WG_KEY_ADDR_IP6];
> +}

as mentioned above: KConfigGroup supports lists

> wireguard.cpp:361-365
> +// Do Private Key
> +if (dataMap.contains(QLatin1String(NM_WG_KEY_PRIVATE_KEY)))
> +interfaceGroup.writeEntry(NMV_WG_TAG_PRIVATE_KEY, 
> dataMap[NM_

[kio-extras] [Bug 375826] Copying files to a remote SFTP folder opens a popup "Could not change permissions.."

2018-09-02 Thread Andreas Schneider

https://bugs.kde.org/show_bug.cgi?id=375826

Andreas Schneider  changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEEDSINFO
 Resolution|--- |WAITINGFORINFO
 CC||a...@cryptomilk.org

--- Comment #1 from Andreas Schneider  ---
What kind of sftp server is this? This sounds like a broken sftp protocol.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[kio-extras] [Bug 368877] log_kio_sftp log flood

2018-09-02 Thread Andreas Schneider

https://bugs.kde.org/show_bug.cgi?id=368877

Andreas Schneider  changed:

   What|Removed |Added

   Assignee|plasma-devel@kde.org|a...@cryptomilk.org
 CC||a...@cryptomilk.org

-- 
You are receiving this mail because:
You are the assignee for the bug.

[kio-extras] [Bug 375305] SFTP file create date is wrong

2018-09-02 Thread Andreas Schneider

https://bugs.kde.org/show_bug.cgi?id=375305

Andreas Schneider  changed:

   What|Removed |Added

 CC||a...@cryptomilk.org
   Assignee|plasma-devel@kde.org|a...@cryptomilk.org

--- Comment #2 from Andreas Schneider  ---
You mean you just connect to a sftp server, choose a file, right click on it an
select properties?

-- 
You are receiving this mail because:
You are the assignee for the bug.

D15093: Add WireGuard capability.

2018-09-02 Thread Andrew Crouthamel

acrouthamel added a comment.


  In D15093#319245 , @andersonbruce 
wrote:
  
  > I'm not sure if the author or the reviewer is supposed to check the "Done" 
box on the inline comments but I think that I have addressed  all the various 
comments made, both inline and separately.
  
  
  The author can. :)

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.

2018-09-02 Thread Bruce Anderson

andersonbruce added a comment.


  I'm not sure if the author or the reviewer is supposed to check the "Done" 
box on the inline comments but I think that I have addressed  all the various 
comments made, both inline and separately.

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.

2018-09-02 Thread Bruce Anderson

andersonbruce updated this revision to Diff 40885.
andersonbruce added a comment.


  Updates from review comments
  
  - Add copyright notices
  - Delete unneeded files
  - Formatting changed to agree with coding standards
  - Changed validation method removing color overrides
  - Removed hard coded fonts and sizes from UI files

REPOSITORY
  R116 Plasma Network Management Applet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15093?vs=40489&id=40885

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/wireguardstrings.h
  vpn/wireguard/wireguardwidget.cpp
  vpn/wireguard/wireguardwidget.h

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

D15229: Specify minimum version for KScreenlocker, use correct DBUS path

2018-09-02 Thread Stefan Brüns

bruns created this revision.
bruns added reviewers: Plasma, davidedmundson.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  org.kde.screensaver.xml is only part of KScreenlocker starting with
  version 5.13.80, also the installation directory is not exported in
  earlier versions.
  
  Depends on D15228 

TEST PLAN
  Try to build with kscreenlocker 5.13.0
  Try to build with kscreenlocker 5.13.80

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

AFFECTED FILES
  CMakeLists.txt
  ksmserver/CMakeLists.txt

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

D15187: Merge switch user dialog into lockscreen

2018-09-02 Thread Stefan Brüns

bruns added a comment.


  See D15228  for the 
KSCREENLOCKER_DBUS_INTERFACES_DIR part ...

REPOSITORY
  R120 Plasma Workspace

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

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

D15228: Export install location for DBUS interfaces via CMake

2018-09-02 Thread Stefan Brüns

bruns created this revision.
bruns added reviewers: Plasma, davidedmundson.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  This is useful for any other package depending on the interface
  description at build time, and mimics other packages which export
  DBUS interfaces.

REPOSITORY
  R133 KScreenLocker

BRANCH
  master

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

AFFECTED FILES
  KScreenLockerConfig.cmake.in

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

D15187: Merge switch user dialog into lockscreen

2018-09-02 Thread Stefan Brüns

bruns added inline comments.

INLINE COMMENTS

> CMakeLists.txt:47
>  
> +qt5_add_dbus_interface( ksmserver_KDEINIT_SRCS 
> ${KDE_INSTALL_FULL_DBUSINTERFACEDIR}/org.kde.screensaver.xml 
> kscreenlocker_interface )
> +

This breaks in two ways:

- There is no specified dependency on KScreenlocker 5.13.80
- One cannot build easily when dependencies have a different prefix as used for 
building plasma

The latter needs something like `set(KSCREENLOCKER_DBUS_INTERFACES_DIR 
"${PACKAGE_PREFIX_DIR}/share/dbus-1/interfaces")
` in KScreenLockerConfig.cmake
This is done by several other components which install DBUS interfaces (Solid, 
KWallet, Baloo, KNotifications).

REPOSITORY
  R120 Plasma Workspace

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

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

D15226: Check against QRect whether pointer is inside DecorationButton

2018-09-02 Thread Vlad Zagorodniy

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

REVISION SUMMARY
  If several buttons share an edge, i.e. spacing between decoration
  buttons is set to 0, and the pointer is on that edge, both buttons will
  be hovered.
  
  This happens because QRectF::contains returns true for points that are
  on "outer" edges, e.g.
  
QRectF rect(0, 0, 10, 10);
rect.contains(QPointF(10, 5)); // returns true

TEST PLAN
  Manually.

REPOSITORY
  R129 Window Decoration Library

BRANCH
  fix-hover-with-zero-spacing

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

AFFECTED FILES
  src/decoration.cpp
  src/decorationbutton.cpp

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

D14806: [AppStream Runner] Reduce verbosity of log output

2018-09-02 Thread Stefan Brüns

bruns added a comment.


  Would be nice to get this in soon, the current state logs a message everytime 
you type a character in KRunner.

REPOSITORY
  R120 Plasma Workspace

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

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

[kio-extras] [Bug 362988] sftp connection win dolphin hangs/stops working after few seconds

2018-09-02 Thread Andreas Schneider

https://bugs.kde.org/show_bug.cgi?id=362988

Andreas Schneider  changed:

   What|Removed |Added

 Ever confirmed|0   |1
 CC||a...@cryptomilk.org
   Assignee|plasma-devel@kde.org|a...@cryptomilk.org
 Status|UNCONFIRMED |ASSIGNED

-- 
You are receiving this mail because:
You are the assignee for the bug.

D7087: Add "Copy Info" button to the About System KCM

2018-09-02 Thread gregormi

This revision was automatically updated to reflect the committed changes.
Closed by commit R102:c0f28efd280f: Add "Copy Info" button to the 
About System KCM (authored by gregormi).

REPOSITORY
  R102 KInfoCenter

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7087?vs=40853&id=40854

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

AFFECTED FILES
  Modules/about-distro/src/Module.cpp
  Modules/about-distro/src/Module.h
  Modules/about-distro/src/Module.ui

To: gregormi, ngraham, dhaumann
Cc: rkflx, dhaumann, ltoscano, sebas, elvisangelaccio, cfeck, plasma-devel, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, apol, 
mart

D7087: Add "Copy Info" button to the About System KCM

2018-09-02 Thread gregormi

gregormi updated this revision to Diff 40853.
gregormi added a comment.


  - Use QVector instead of QList
  - Use the term "Operating System:" instead of "Distro"

REPOSITORY
  R102 KInfoCenter

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7087?vs=39321&id=40853

BRANCH
  arcpatch-D7087_1

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

AFFECTED FILES
  Modules/about-distro/src/Module.cpp
  Modules/about-distro/src/Module.h
  Modules/about-distro/src/Module.ui

To: gregormi, ngraham, dhaumann
Cc: rkflx, dhaumann, ltoscano, sebas, elvisangelaccio, cfeck, plasma-devel, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, apol, 
mart

D15093: Add WireGuard capability.

D15093: Add WireGuard capability.

[kio-extras] [Bug 375826] Copying files to a remote SFTP folder opens a popup "Could not change permissions.."

[kio-extras] [Bug 368877] log_kio_sftp log flood

[kio-extras] [Bug 375305] SFTP file create date is wrong

D15093: Add WireGuard capability.

D15093: Add WireGuard capability.

D15093: Add WireGuard capability.

D15229: Specify minimum version for KScreenlocker, use correct DBUS path

D15187: Merge switch user dialog into lockscreen

D15228: Export install location for DBUS interfaces via CMake

D15187: Merge switch user dialog into lockscreen

D15226: Check against QRect whether pointer is inside DecorationButton

D14806: [AppStream Runner] Reduce verbosity of log output

[kio-extras] [Bug 362988] sftp connection win dolphin hangs/stops working after few seconds

D7087: Add "Copy Info" button to the About System KCM

D7087: Add "Copy Info" button to the About System KCM

17 matches

Site Navigation

Mail list logo

Footer information