D10429: Disable the title bar separator by default

2018-09-16 Thread Ilya Bizyaev 
IlyaBizyaev added a comment.


  I'll be that one person here who likes the separator and notices it easily on 
the Breeze theme.
  
  I have no objections to changing the default, but "just removing it entirely" 
is not an option.

REPOSITORY
  R31 Breeze

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

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

D15093: Add WireGuard capability.

2018-09-16 Thread Bruce Anderson 
andersonbruce updated this revision to Diff 41736.
andersonbruce added a comment.


  - Add tests for new and updated ip validators
  - Update ip validators
  - Add validator for WireGuard keys
  - Update per review comments
  - Add error messages for failure cases

REPOSITORY
  R116 Plasma Network Management Applet

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

BRANCH
  Feature/AddWireGuardVPN

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

AFFECTED FILES
  CMakeLists.txt
  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
  tests/CMakeLists.txt
  tests/simpleiplisttest.cpp
  tests/simpleipv4test.cpp
  tests/simpleipv6test.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

D15093: Add WireGuard capability.

2018-09-16 Thread Bruce Anderson 
andersonbruce marked 45 inline comments as done.
andersonbruce added inline comments.

INLINE COMMENTS

> pino wrote in wireguard.cpp:121
> better use qt's foreach here:
> 
>   Q_FOREACH (const QString , valueList)
> 
> and then using `address` instead of `valueList[i]`

Used plain 'for' instead because Nathaniel Graham commented that the Q_FOREACH 
is not acceptable anymore.

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

D15550: Move correct indices in the manual sort map when getting a move() call for group children

2018-09-16 Thread 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
  Due to not passing `parent` to QAIM::index(), we were moving top-level
  indices in the map instead. This meant the sort map would become out of
  sync with the row move, and to the user it would look like both the
  group children and unrelated top-level entries moved.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

AFFECTED FILES
  libtaskmanager/tasksmodel.cpp

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

D10429: Disable the title bar separator by default

2018-09-16 Thread Ilya Bizyaev 
IlyaBizyaev added a comment.


  I think this line's goal is not really to separate the title bar from window 
content, but instead to make the transition smoother and emphasize style 
consistency.
  
  Take a look at these 2 screenshots, for example.
  F6267692: Screenshot_20180916_124803.png 
 F6267691: Screenshot_20180916_124749.png 

  
  The one without the line looks very rectangular and sharp for eyes, as if 
someone has cut window decorations aggressively.
  The other, in my opinion, has a more natural, friendly transition and shares 
color with application controls.
  
  I don't have user stats, though.

REPOSITORY
  R31 Breeze

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

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

D15206: [Kickoff] Add a subtle separator line between the header and the content view

2018-09-16 Thread Nathaniel Graham 
ngraham added a comment.


  @mmustac, I believe I've addressed your concern now.

REPOSITORY
  R119 Plasma Desktop

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

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

D15206: [Kickoff] Add a subtle separator line between the header and the content view

2018-09-16 Thread Nathaniel Graham 
ngraham updated this revision to Diff 41788.
ngraham added a comment.


  Adjust top margin on Favorites view to make sure that the top item doesn't 
touch the new line

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15206?vs=40823=41788

BRANCH
  kickoff-header-separator (branched from master)

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

AFFECTED FILES
  applets/kickoff/package/contents/ui/FavoritesView.qml
  applets/kickoff/package/contents/ui/FullRepresentation.qml

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

D15521: Add validator for lists of IP addressesAdded as separate review per comment from Pino onreview D15093. This code will not compile withoutthe updated code in review D15520. Also includesunit te

2018-09-16 Thread Jan Grulich 
jgrulich added inline comments.

INLINE COMMENTS

> CMakeLists.txt:92
>  qca-qt5
> +KF5::NetworkManagerQt
> +KF5::Service

I'm sure you don't need to add all these and why did you remove PUBLIC and 
PRIVATE keywords?

> CMakeLists.txt:111
>  if (WITH_MODEMMANAGER_SUPPORT)
> -target_link_libraries(plasmanm_editor PUBLIC KF5::ModemManagerQt)
> +target_link_libraries(plasmanm_editor KF5::ModemManagerQt)
>  endif()

Why removing it?

> CMakeLists.txt:115
>  install(TARGETS plasmanm_editor ${INSTALL_TARGETS_DEFAULT_ARGS})
> -install(FILES plasma-networkmanagement-vpnuiplugin.desktop DESTINATION 
> ${KDE_INSTALL_KSERVICETYPES5DIR})
> +install(FILES plasma-networkmanagement-vpnuiplugin.desktop DESTINATION 
> ${SERVICETYPES_INSTALL_DIR})

This is also an unrelated change.

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

Fix indentation.

> simpleiplistvalidator.cpp:39
> +ipv4Style = SimpleIpV4AddressValidator::AddressStyle::WithPort;
> +m_ipv4Validator = new SimpleIpV4AddressValidator(this, ipv4Style);
> +}

In both validator constructors (mean also for IPv6 one), you can directly pass 
"style" param from contructor, given both enums seem to be identical.

> CMakeLists.txt:9
> +
> +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -O0 -ggdb")
> +

Do we need these flags to be set? Please leak how simply can tested be added to 
CMake

[1] - https://cgit.kde.org/networkmanager-qt.git/tree/autotests/CMakeLists.txt

REPOSITORY
  R116 Plasma Network Management Applet

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

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

D15521: Add validator for lists of IP addressesAdded as separate review per comment from Pino onreview D15093. This code will not compile withoutthe updated code in review D15520. Also includesunit te

2018-09-16 Thread Jan Grulich 
jgrulich added a comment.


  Maybe merge this review with D15520 . I 
think they should go together.

REPOSITORY
  R116 Plasma Network Management Applet

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

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

D15093: Add WireGuard capability.

2018-09-16 Thread Bruce Anderson 
andersonbruce marked an inline comment as done.
andersonbruce added inline comments.

INLINE COMMENTS

> jgrulich wrote in wireguardadvancedwidget.cpp:104
> 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 , const QString , const QString ).

Functionally I think that the current implementation does this (although I can 
change the name of the function if you want). It starts with a blank 
NMStringMap and uses setOrClear on it. Are you possibly referring to the same 
function name  used in wireguardwidget.cpp rather than here in 
wireguardadvancedwidget.cpp?

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

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

2018-09-16 Thread Nathaniel Graham 
ngraham added a comment.


  @davidedmundson ping.

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

D15093: Add WireGuard capability.

2018-09-16 Thread Andrew Crouthamel 
acrouthamel added a comment.


  Thank you @andersonbruce for all of your work on this, and quickly working on 
feedback changes! This will be a great addition.

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