D10429: Disable the title bar separator by default

[Fabian Vogt]

This revision was automatically updated to reflect the committed changes.
Closed by commit R31:51cedac0889b: Disable the title bar separator by default 
(authored by fvogt).

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10429?vs=26883=41512

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

AFFECTED FILES
  kdecoration/breezesettingsdata.kcfg

To: fvogt, #vdg, #plasma, ngraham, abetts, romangg
Cc: romangg, mart, abetts, rizzitello, mmustac, broulik, anthonyfieroni, januz, 
rikmills, anemeth, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol

D10429: Disable the title bar separator by default

[Nathaniel Graham]

ngraham added a comment.


  In D10429#324936 , @romangg wrote:
  
  > It's unnecessary bling imo. And it can interfere with in-window elements 
(Firefox for example marks the active tab with a very similar colored line at 
the top). I would say let's try it out and listen for feedback in 5.14 cycle if 
people miss it.
  
  
  +1, I say let's go for it. Our short release cycles make this a good approach 
IMHO: we can get real feedback quickly, and react accordingly.
  
  So far, I think the feedback about the line has been mostly negative since it 
turned on by default, so I don't think this will cause any great consternation 
among our users, but let's give it a try and find out for sure.

REPOSITORY
  R31 Breeze

BRANCH
  master

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

To: fvogt, #vdg, #plasma, ngraham, abetts, romangg
Cc: romangg, mart, abetts, rizzitello, mmustac, broulik, anthonyfieroni, januz, 
rikmills, anemeth, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol

D10429: Disable the title bar separator by default

[Roman Gilg]

romangg accepted this revision.
romangg added a comment.


  It's unnecessary bling imo. And it can interfere with in-window elements 
(Firefox for example marks the active tab with a very similar colored line at 
the top). I would say let's try it out and listen for feedback in 5.14 cycle if 
people miss it.

REPOSITORY
  R31 Breeze

BRANCH
  master

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

To: fvogt, #vdg, #plasma, ngraham, abetts, romangg
Cc: romangg, mart, abetts, rizzitello, mmustac, broulik, anthonyfieroni, januz, 
rikmills, anemeth, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol

D10429: Disable the title bar separator by default

[Nathaniel Graham]

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


  I think all the +1s here deserve to amount to at least one green checkbox. :)

REPOSITORY
  R31 Breeze

BRANCH
  master

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

To: fvogt, #vdg, #plasma, ngraham
Cc: mart, abetts, rizzitello, mmustac, broulik, anthonyfieroni, januz, 
rikmills, anemeth, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol

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

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:e1c19ce4daf9: Plasmashell freezes when trying to get free 
space info from mounted remote… (authored by McPain, committed by ngraham).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14895?vs=40973=41511

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

AFFECTED FILES
  dataengines/soliddevice/CMakeLists.txt
  dataengines/soliddevice/soliddeviceengine.cpp
  dataengines/soliddevice/soliddeviceengine.h

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

D15435: Add translucent background attribute to desktop icon popup menu

[Andreas Sturmlechner]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:cc47b95094a3: Add translucent background attribute to 
desktop icon popup menu (authored by anemeth, committed by asturmlechner).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15435?vs=41432=41509

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

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

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

D14362: Use unpretty channel name in speakertest

[Nicolas Fella]

This revision was automatically updated to reflect the committed changes.
Closed by commit R115:7cd16db35a49: Use unpretty channel name in speakertest 
(authored by nicolasfella).

REPOSITORY
  R115 Plasma Audio Volume Applet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14362?vs=38406=41507

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

AFFECTED FILES
  src/kcm/package/contents/ui/Advanced.qml
  src/sink.cpp
  src/sink.h
  src/volumeobject.cpp
  src/volumeobject.h

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

D15435: Add translucent background attribute to desktop icon popup menu

[Alex Nemeth]

anemeth added a comment.


  I'm having problems with my arcanist.
  Could someone please land this for me?

REPOSITORY
  R119 Plasma Desktop

BRANCH
  fix_desktop_menu_tp

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

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

D15462: Do ignore-filtered-siblings properly

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:c81205cc458e: Do ignore-filtered-siblings properly 
(authored by hein).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15462?vs=41499=41500

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

AFFECTED FILES
  libtaskmanager/taskfilterproxymodel.cpp
  libtaskmanager/taskfilterproxymodel.h
  libtaskmanager/tasksmodel.cpp

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

D15462: Do ignore-filtered-siblings properly

[Eike Hein]

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

REVISION SUMMARY
  Turns out c8358c203f11 
 
only worked accidentally. We can't do this
  via filterModel->mapFromSource, because in a lambda connected to
  its source model's rowsInserted signal, the proxy hasn't seen that
  row yet.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

AFFECTED FILES
  libtaskmanager/taskfilterproxymodel.cpp
  libtaskmanager/taskfilterproxymodel.h
  libtaskmanager/tasksmodel.cpp

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

D15440: Translated into Russian

[Sheykhnur Latipov]

sheykhnurlatipov added a comment.


  Well, thnx for reply, I'll do it. It's just that I do it for the first time. 
:)

REPOSITORY
  R856 Plasma Browser Integration

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

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

D15256: [Mouse KCM] Avoid changes to touchpads in libinput backend

[Roman Gilg]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:a4c724173b5c: [Mouse KCM] Avoid changes to touchpads in 
libinput backend (authored by romangg).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15256?vs=40945=41495

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

AFFECTED FILES
  kcms/mouse/backends/x11/x11_libinput_dummydevice.cpp

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

D15256: [Mouse KCM] Avoid changes to touchpads in libinput backend

[Nathaniel Graham]

ngraham added a comment.


  The beta branches tomorrow. It would be nice to get this in so that people 
can test and offer feedback before the release.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  fixMouseResettingTouchpad

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

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

D15458: Re-filter launcher when a window changes identity

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:a057edc7e9fb: Re-filter launcher when a window changes 
identity (authored by hein).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15458?vs=41489=41490

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

AFFECTED FILES
  libtaskmanager/tasksmodel.cpp

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

D15458: Re-filter launcher when a window changes identity

[Eike Hein]

hein created this revision.
hein added a reviewer: broulik.
Herald added a project: Plasma.
hein requested review of this revision.

REVISION SUMMARY
  LibreOffice reuses the same main window for both its start center and
  sub-apps like Writer launched from it, changing the window metadata on
  the fly. This ensures we hide e.g. a launcher for Writer when picking
  it in the LO start center.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

AFFECTED FILES
  libtaskmanager/tasksmodel.cpp

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

D15410: Handle clients which change window metadata during early startup

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:b15eaf38b6bf: Handle clients which change window metadata 
during early startup (authored by hein).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15410?vs=41356=41487

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

AFFECTED FILES
  libtaskmanager/taskmanagerrulesrc
  libtaskmanager/tasksmodel.cpp
  libtaskmanager/tasktools.cpp
  libtaskmanager/tasktools.h
  libtaskmanager/waylandtasksmodel.cpp
  libtaskmanager/xwindowtasksmodel.cpp

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

D14426: [Folder View] Create KFilePlacesModel only when needed and listen for changes

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:dff9d7fe09f2: [Folder View] Create KFilePlacesModel only 
when needed and listen for changes (authored by broulik).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D14426?vs=41331=41485#toc

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14426?vs=41331=41485

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

AFFECTED FILES
  containments/desktop/plugins/folder/labelgenerator.cpp
  containments/desktop/plugins/folder/labelgenerator.h

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

D10855: Emit clicked when double click expires

[Roman Gilg]

romangg added a comment.


  Why is the press and hold timer still needed? It should be enough to only 
test on double click expiration with this patch and open the menu on release of 
mouse button afterwards. We should accept this behavioral change in order to 
reduce code complexity (people should automatically understand that the menu 
opens from now on on button release after they have experienced it once trying 
it with long click). But if you want to split this up in a second patch I'm 
fine with it.
  
  Otherwise I'm in favor of the change.
  
  So do you want to change this diff or put it in a second diff?

REPOSITORY
  R129 Window Decoration Library

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

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

D15430: Open Web Shorcuts KCM from Web Shortcut Runner config

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:4b4439fe2ef5: Open Web Shorcuts KCM from Web Shortcut 
Runner config (authored by broulik).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15430?vs=41411=41473

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

AFFECTED FILES
  runners/webshortcuts/CMakeLists.txt
  runners/webshortcuts/plasma-runner-webshortcuts_config.desktop

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

D15427: [Spellchecker Runner] Add button to open Spell Check KCM

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R114:1d99daa41fd6: [Spellchecker Runner] Add button to open 
Spell Check KCM (authored by broulik).

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15427?vs=41407=41467

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

AFFECTED FILES
  runners/spellchecker/spellcheck_config.cpp
  runners/spellchecker/spellcheck_config.h
  runners/spellchecker/spellcheck_config.ui

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

Fwd: network connectivity with proxy

[Harald Sitter]

Shouldn't plasma-integration install a QNetworkProxyFactory
implementing a factory based on KProtocolManager? It seems to me that
proxy settings from the KCM should "just work" when running a Qt
application under Plasma.

-- Forwarded message -
From: Harald Sitter 
Date: Wed, Sep 12, 2018 at 1:17 PM
Subject: Re: network connectivity with proxy
To: KDE-devel Mailing-List 


On Mon, Sep 10, 2018 at 10:31 PM Alexander Semke  wrote:
>
> Hi,
>
> in Knights and in LabPlot I realized recently, while being in a corporate
> network behind a proxy, that the connection to public internet servers is not
> possible. E.g. in Khights the connection to the FICS-server is done like
>
> QTcpSocket* socket = new QTcpSocket(this);
> socket->connectToHost(address, port);
>
> How to properly handle the proxy settings? Reading system settings (from the
> environment?) and calling QTcpSocket::setProxy() or
> QNetworkProxy::setApplicationProxy() somewhere  in main.cpp doesn't sound to
> me like the "standard way to do this".

If I am not mistaken Qt already looks in the environment variables for
http_proxy, https_proxy etc. etc. (I don't think Plasma sets those
from the KCM though). To that end a no-code solution is setting those.
Obviously that's not very integrated in plasma.

> In KDE's system settings there're already settings for the proxy. How to use
> them and how to deal with this on windows and mac os x where this is not
> available? Should the applications have this configuration in their settings?

There is KTcpSocket (in KIO). But at a glance I don't actually see the
class implementing proxy handling correctly, nor does it look
particularly well documented, so I am not sure it should be used.
So... maybe fix KTcpSocket first, then use it ;)

KProtocolManager is the way to get the proxy settings in your
application from which you can construct a QNetworkProxy and set that
either on an application level or per-socket (you should be able to
find some examples of that via lxr.kde.org).

That's not ideal I'd have to say though. On OSX and Windows figures
the proxy stuff out automatically as far as I know (KProtocolManager
would be falling back to QNetwork on those platforms I'd presume).

[1] https://api.kde.org/frameworks/kio/html/classKProtocolManager.html

HS

D15442: Improve DDCUtil handling in CMakeLists

[Christophe Giboudeaux]

cgiboudeaux added a comment.


  With the change, you can't tell if powerdevil must use the old or the new 
api. Suggestion: https://paste.kde.org/powk2bhxm
  
  config_powerdevil.h then defines HAVE_DDCUTIL and DDCUTIL_VERSION

REPOSITORY
  R122 Powerdevil

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

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

D15447: [libdbusmenuqt] Port to categorized logging

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:1e1ce733920d: [libdbusmenuqt] Port to categorized logging 
(authored by broulik).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15447?vs=41461=41465

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

AFFECTED FILES
  libdbusmenuqt/CMakeLists.txt
  libdbusmenuqt/dbusmenuimporter.cpp

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

D15441: Initial migration to >=ddcutil-0.8.6 API (tested with ddcutil-0.9.1)

[Bhushan Shah]

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


  Does this mean  ddcutil backend will break compilation with the ddcutil < 
0.8.6 ?

REPOSITORY
  R122 Powerdevil

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

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

D15447: [libdbusmenuqt] Port to categorized logging

[Kai Uwe Broulik]

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

REVISION SUMMARY
  And make it less chatty overall, none of these are real "warnings"

TEST PLAN
  No longer spams my log with `No menu id for xyz`

REPOSITORY
  R120 Plasma Workspace

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

AFFECTED FILES
  libdbusmenuqt/CMakeLists.txt
  libdbusmenuqt/dbusmenuimporter.cpp

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

D15440: Translated into Russian

[Alexander Potashev]

aspotashev added a comment.


  Thanks for your work, Sheykhnur,
  
  Russian translation team here :)
  
  Since you already have the translated messages, here are the easy steps left 
to submit them correctly:
  
  1. Download the file plasma-browser-extension.po from 
https://l10n.kde.org/stats/gui/trunk-kf5/team/ru/kde-workspace/
  2. Open this file in Lokalize or another .po editor (Lokalize is available 
through most of the Linux distributions).
  3. Fill in your translations into the .po file in Lokalize. This should be 
easy using Ctrl+C/Ctrl+V.
  4. Save the file in Lokalize and send it to me (aspotas...@gmail.com) or 
through the Russian team mailing list 

  5. Wait for review.

REPOSITORY
  R856 Plasma Browser Integration

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

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

D15093: Add WireGuard capability.

[Pino Toscano]

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


  In D15093#324457 , @andersonbruce 
wrote:
  
  > A couple of questions on opening other review requests:
  >
  > - Should I open a bug request on bugs.kde.org before opening the review?
  
  
  No need to, they are only internal refactorings/improvements.
  
  > - Is there any special way I need to mark dependencies? That is, the 
simpleiplistvalidator is dependent on the updates to the simpleipv4/6 
validators and WireGuard is dependent on both.
  
  I think so: see on the top-right "Edit Related Revisions" -> "Edit 
Parent/Child Revisions".
  
  > - Is there a preferred unit test framework that KDE uses?
  
  Qt ships its own QtTest -- see http://doc.qt.io/qt-5/qtest-overview.html
  
  > And do you possibly have a pointer to a sample of other projects that use 
it?
  
  One simple example is in juk, see the `tests` subdirectory.

INLINE COMMENTS

> simpleiplistvalidator.cpp:27-28
> +SimpleIpListValidator::SimpleIpListValidator(QObject *parent,
> +   enum AddressStyle 
> style,
> +   enum AddressType type)
> +: QValidator(parent)

this is not C, so 'enum' is not needed

> simpleiplistvalidator.cpp:31-34
> +addressStyle = style;
> +addressType = type;
> +ipv4Validator = nullptr;
> +ipv6Validator = nullptr;

these can be moved to the constructor initialization list, so it's slightly 
more efficient

> simpleiplistvalidator.cpp:42
> +ipv4Style = SimpleIpV4AddressValidator::AddressStyle::WithCidr;
> +ipv4Validator = new SimpleIpV4AddressValidator(nullptr, ipv4Style);
> +}

this is leaked -- either you pass 'this' as parent, or explicitly delete them 
in the class destructor

> simpleiplistvalidator.cpp:50
> +ipv6Style = SimpleIpV6AddressValidator::AddressStyle::WithCidr;
> +ipv6Validator = new SimpleIpV6AddressValidator(nullptr, ipv6Style);
> +}

ditto

> simpleiplistvalidator.cpp:61
> +// Split the incoming address on commas possibly with spaces on either 
> side
> +QStringList addressList = address.split(QRegExp("\\s*,\\s*"));
> +

a regexp is not needed here, just pass QString::SkipEmptyParts to split()

> simpleiplistvalidator.cpp:67-68
> +int i = 0;
> +QValidator::State ipv4Result = QValidator::Acceptable;
> +QValidator::State ipv6Result = QValidator::Acceptable;
> +QValidator::State result = QValidator::Acceptable;

these can be moved inside the foreach loop

> simpleiplistvalidator.cpp:78
> +if (ipv4Validator != nullptr)
> +ipv4Result = ipv4Validator->validate(const_cast(addr), 
> localPos);
> +else

this const_cast is not nice... rather keep a local copy of the string

> simpleiplistvalidator.cpp:85
> +if (ipv6Validator != nullptr)
> +ipv6Result = ipv6Validator->validate(const_cast(addr), 
> localPos);
> +else

ditto

> simpleiplistvalidator.cpp:91-92
> +if (ipv6Result == QValidator::Invalid && ipv4Result == 
> QValidator::Invalid) {
> +result = QValidator::Invalid;
> +break;
> +}

i think you can just `return QValidator::Invalid` straight away

> simpleiplistvalidator.h:42-45
> +AddressStyle addressStyle;
> +AddressType addressType;
> +SimpleIpV6AddressValidator *ipv6Validator;
> +SimpleIpV4AddressValidator *ipv4Validator;

private class variables start with "m_" prefix

> simpleipv4addressvalidator.cpp:26
>  
> -SimpleIpV4AddressValidator::SimpleIpV4AddressValidator(QObject *parent)
> +SimpleIpV4AddressValidator::SimpleIpV4AddressValidator(QObject *parent, enum 
> AddressStyle style)
>  : QValidator(parent)

as above, no "enum"

> simpleipv4addressvalidator.cpp:29
>  {
> +addressStyle = style;
>  }

in constructor initialization list

> simpleipv4addressvalidator.cpp:55-66
> +v = new QRegExpValidator(QRegExp(QLatin1String("[0-9, ]{1,3}\\.[0-9, 
> ]{1,3}\\.[0-9, ]{1,3}\\.[0-9, ]{1,3}")), 0);
> +break;
> +
> +case WithCidr:
> +v = new QRegExpValidator(QRegExp(QLatin1String("([0-9, 
> ]{1,3}\\.){3,3}[0-9, ]{1,3}/[0-9]{1,2}")), 0);
> +break;
> +

all the regexps and their validators are created every time, and this is going 
to be super slow; since the type is decided at constructor time and never 
changed, just create the validator once

(alos, this code leaks all the validators)

> simpleipv4addressvalidator.cpp:120
>  
> +
>  // replace input string with the corrected version

unneeded empty line change

> simpleipv4addressvalidator.cpp:137
> +} else {
> +value += QString::number(cidrValue);
> +return QValidator::Acceptable;

there is already `cidrParts[1]` as string, so no need to create it again from 
the value

> 

D15093: Add WireGuard capability.

[Bruce Anderson]

andersonbruce added a comment.


  In D15093#323095 , @pino wrote:
  
  > Thanks Bruce for all the updates, and patience! In return, I have more 
notes :)
  >
  > - I'd send the changes to the existing SimpleIpV4AddressValidator in an own 
review request, since it affects other code than just this new wireguard plugin 
(maybe with unit tests)
  > - I'd send the addition of SimpleIpListValidator in an own review request 
too, since it is an independent code (possibly with unit tests)
  
  
  A couple of questions on opening other review requests:
  
  - Should I open a bug request on bugs.kde.org before opening the review?
  - Is there any special way I need to mark dependencies? That is, the 
simpleiplistvalidator is dependent on the updates to the simpleipv4/6 
validators and WireGuard is dependent on both.
  - Is there a preferred unit test framework that KDE uses? And do you possibly 
have a pointer to a sample of other projects that use it? (Especially if they 
use QValidator since I tried to write some test code for my own use and I 
couldn't get it to not crash before it even got to main()).

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 41451.
andersonbruce added a comment.


  - Correct capitalization
  - Remove HTML from tooltip strings
  - Update some tooltip strings for clarity

REPOSITORY
  R116 Plasma Network Management Applet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15093?vs=41241=41451

BRANCH
  Feature/AddWireGuardVPN

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

AFFECTED FILES
  libs/editor/CMakeLists.txt
  libs/editor/simpleiplistvalidator.cpp
  libs/editor/simpleiplistvalidator.h
  libs/editor/simpleipv4addressvalidator.cpp
  libs/editor/simpleipv4addressvalidator.h
  libs/editor/simpleipv6addressvalidator.cpp
  libs/editor/simpleipv6addressvalidator.h
  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/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