D29024: feat(wayland): support multiple protocol extensions through plugin system

[Daniel Vrátil]

dvratil added a comment.


  If I may add my two cents here, I agree with David that introducing a plugin 
for a plugin is a bit over the top. Whats the issue with contributing your 
backend into libkscreen upstream instead? If there's any code that could be 
shared between KWayland backend and your new backend, you can create a small 
static library (basically reusing most of what you've already done). Otherwise 
you are basically forcing libkscreen to accommodate a single usecase for your 
fork, and the community has the right to push back on that as it makes things 
more complicated for them just to make live easier for you.
  
  Regarding the asynchronous plugin loading. You still need to block the 
constructor of the KWayland backend in order to figure out which plugin you 
want to load, because the KScreen backend loading code synchronously calls 
`isValid()` just after constructing the backend. So it's nice you try loading 
your plugins asynchronously, but it doesn't solve much since all you do is 
blocking-waiting for a bunch o threads to finish, rather than just 
blocking-waiting for the calls to the registry to finish... The current backend 
loading code in libkscreen is of course simple and stupid because it was first 
designed only with XRandR and Fake backends in mind. Later on QScreen was added 
which also works synchronously. When Wayland support was introduced, Sebas 
rewrote lot of the code, but some of the assumptions were kept, like plugins 
initializing synchronously. It's not too hard to fix the backend loading code 
to not only make assumptions about backends based on Qt platform and to allow 
for true asynchronous initialization.

REPOSITORY
  R110 KScreen Library

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

To: romangg, #plasma
Cc: dvratil, ngraham, davidedmundson, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra, mart

D21905: [Digital Clock] Open KOrganizer with event details on click

[Daniel Vrátil]

dvratil added a comment.


  Ping?

REPOSITORY
  R120 Plasma Workspace

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

To: dvratil, #plasma
Cc: broulik, apol, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, ahiemstra, mart

D27675: feat: avoid duplicated text when assembling user-facing output names

[Daniel Vrátil]

dvratil updated this revision to Diff 77838.
dvratil added a comment.


  - Simplify a bit more

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27675?vs=77837=77838

BRANCH
  arcpatch-D27675

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

AFFECTED FILES
  common/utils.cpp

To: dvratil, #plasma, romangg
Cc: ahmadsamir, romangg, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra, mart

D27675: feat: avoid duplicated text when assembling user-facing output names

[Daniel Vrátil]

dvratil updated this revision to Diff 77837.
dvratil added a comment.


  - Make the lambda more explicit
  - Address review comments

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27675?vs=76457=77837

BRANCH
  arcpatch-D27675

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

AFFECTED FILES
  common/utils.cpp

To: dvratil, #plasma, romangg
Cc: ahmadsamir, romangg, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra, mart

D10042: feat: parse EDID on the backend side

[Daniel Vrátil]

dvratil added a comment.


  Agreed. deviceId is not actually used (it's stored in KDED config files, but 
the value doesn't seem to be used anywhere). I'll fix it in a separate commit 
(since it also needs adjusting a bunch of unit-tests in libkscreen and kscreen).

REPOSITORY
  R110 KScreen Library

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

To: dvratil, sebas, #plasma, romangg
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27677: feat: fix build after EDID changes in libkscreen

[Daniel Vrátil]

dvratil updated this revision to Diff 77830.
dvratil added a comment.


  - Fix commit message

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27677?vs=76460=77830

BRANCH
  master

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

AFFECTED FILES
  console/console.cpp

To: dvratil, #plasma
Cc: romangg, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D10042: feat: parse EDID on the backend side

[Daniel Vrátil]

dvratil added a comment.


  Here's kscreen-console output for one of my monitors under Wayland with this 
patch applied:
  
[15:31:16.520] kscreen-console(422251:422251) unknown: Id:  3
[15:31:16.520] kscreen-console(422251:422251) unknown: Name:  "Eizo Nanao 
Corporation DP-7-EV2736W/96211045"
[15:31:16.520] kscreen-console(422251:422251) unknown: Type:  "DisplayPort"
[15:31:16.520] kscreen-console(422251:422251) unknown: Connected:  true
[15:31:16.520] kscreen-console(422251:422251) unknown: Enabled:  true
[15:31:16.520] kscreen-console(422251:422251) unknown: Primary:  true
[15:31:16.520] kscreen-console(422251:422251) unknown: Rotation:  2
[15:31:16.520] kscreen-console(422251:422251) unknown: Pos:  QPoint(0,0)
[15:31:16.520] kscreen-console(422251:422251) unknown: MMSize:  QSize(600, 
340)
[15:31:16.520] kscreen-console(422251:422251) unknown: FollowPreferredMode: 
 false
[15:31:16.520] kscreen-console(422251:422251) unknown: Size:  QSize(-1, -1)
[15:31:16.520] kscreen-console(422251:422251) unknown: Scale:  1
[15:31:16.520] kscreen-console(422251:422251) unknown: Clones:  None
[15:31:16.520] kscreen-console(422251:422251) unknown: Mode:  "0"
[15:31:16.520] kscreen-console(422251:422251) unknown: Preferred Mode:  "0"
[15:31:16.520] kscreen-console(422251:422251) unknown: Preferred modes:  
("0")
[15:31:16.520] kscreen-console(422251:422251) unknown: Modes: 
[15:31:16.520] kscreen-console(422251:422251) unknown:   "0"
"2560x1440@60"   QSize(2560, 1440)   59.951
[15:31:16.520] kscreen-console(422251:422251) unknown:   "1"
"2560x1440@30"   QSize(2560, 1440)   29.935
[15:31:16.520] kscreen-console(422251:422251) unknown:   "10"
"1280x720@60"   QSize(1280, 720)   60
[15:31:16.520] kscreen-console(422251:422251) unknown:   "11"
"1280x720@60"   QSize(1280, 720)   59.94
[15:31:16.520] kscreen-console(422251:422251) unknown:   "12"
"1024x768@60"   QSize(1024, 768)   60.004
[15:31:16.520] kscreen-console(422251:422251) unknown:   "13"
"800x600@60"   QSize(800, 600)   60.317
[15:31:16.521] kscreen-console(422251:422251) unknown:   "14"
"720x480@60"   QSize(720, 480)   60
[15:31:16.521] kscreen-console(422251:422251) unknown:   "15"
"720x480@60"   QSize(720, 480)   60
[15:31:16.521] kscreen-console(422251:422251) unknown:   "16"
"720x480@60"   QSize(720, 480)   59.94
[15:31:16.521] kscreen-console(422251:422251) unknown:   "17"
"720x480@60"   QSize(720, 480)   59.94
[15:31:16.521] kscreen-console(422251:422251) unknown:   "18"
"720x480@60"   QSize(720, 480)   59.94
[15:31:16.521] kscreen-console(422251:422251) unknown:   "19"
"640x480@60"   QSize(640, 480)   60
[15:31:16.521] kscreen-console(422251:422251) unknown:   "2"
"1920x1200@60"   QSize(1920, 1200)   59.885
[15:31:16.521] kscreen-console(422251:422251) unknown:   "20"
"640x480@60"   QSize(640, 480)   59.94
[15:31:16.521] kscreen-console(422251:422251) unknown:   "21"
"640x480@60"   QSize(640, 480)   59.94
[15:31:16.521] kscreen-console(422251:422251) unknown:   "22"
"720x400@70"   QSize(720, 400)   70.082
[15:31:16.521] kscreen-console(422251:422251) unknown:   "3"
"1920x1080@60"   QSize(1920, 1080)   60
[15:31:16.521] kscreen-console(422251:422251) unknown:   "4"
"1920x1080@60"   QSize(1920, 1080)   60
[15:31:16.521] kscreen-console(422251:422251) unknown:   "5"
"1920x1080@60"   QSize(1920, 1080)   59.94
[15:31:16.521] kscreen-console(422251:422251) unknown:   "6"
"1600x1200@60"   QSize(1600, 1200)   60
[15:31:16.521] kscreen-console(422251:422251) unknown:   "7"
"1280x1024@60"   QSize(1280, 1024)   60.02
[15:31:16.521] kscreen-console(422251:422251) unknown:   "8"
"1280x960@60"   QSize(1280, 960)   60
[15:31:16.521] kscreen-console(422251:422251) unknown:   "9"
"1280x720@60"   QSize(1280, 720)   60
[15:31:16.521] kscreen-console(422251:422251) unknown: EDID Info: 
[15:31:16.521] kscreen-console(422251:422251) unknown:  Device ID:  
"xrandr-Eizo Nanao Corporation-DP-7-EV2736W/96211045"
[15:31:16.521] kscreen-console(422251:422251) unknown:  Name:  
"DP-7-EV2736W/96211045"
[15:31:16.521] kscreen-console(422251:422251) unknown:  Vendor:  "Eizo 
Nanao Corporation"
[15:31:16.521] kscreen-console(422251:422251) unknown:  Serial:  ""
[15:31:16.521] kscreen-console(422251:422251) unknown:  EISA ID:  ""
[15:31:16.521] kscreen-console(422251:422251) unknown:  Hash:  
"0aaedf5ca2"
[15:31:16.521] kscreen-console(422251:422251) unknown:  Width:  60
[15:31:16.521] kscreen-console(422251:422251) unknown:  Height:  34
[15:31:16.521] kscreen-console(422251:422251) unknown:  Gamma:  0
[15:31:16.521] kscreen-console(422251:422251) unknown:  Red:  

D10042: feat: parse EDID on the backend side

[Daniel Vrátil]

dvratil updated this revision to Diff 77825.
dvratil edited the summary of this revision.
dvratil added a comment.


  - Rebase on current master
  - Move Edid construction to WaylandOutput::edid

REPOSITORY
  R110 KScreen Library

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10042?vs=76459=77825

BRANCH
  arcpatch-D10042

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

AFFECTED FILES
  CMakeLists.txt
  autotests/testconfigserializer.cpp
  autotests/testkwaylandbackend.cpp
  autotests/testqscreenbackend.cpp
  backends/fake/fake.cpp
  backends/fake/fake.h
  backends/kwayland/waylandbackend.cpp
  backends/kwayland/waylandbackend.h
  backends/kwayland/waylandoutput.cpp
  backends/kwayland/waylandoutput.h
  backends/xrandr/xrandr.cpp
  backends/xrandr/xrandr.h
  interfaces/org.kde.KScreen.Backend.xml
  src/CMakeLists.txt
  src/abstractbackend.cpp
  src/abstractbackend.h
  src/backendlauncher/backenddbuswrapper.cpp
  src/backendlauncher/backenddbuswrapper.h
  src/backendmanager.cpp
  src/configserializer.cpp
  src/configserializer_p.h
  src/doctor/doctor.cpp
  src/edid.cpp
  src/edid.h
  src/edid_p.h
  src/getconfigoperation.cpp
  src/output.cpp
  src/output.h
  src/types.h
  tests/kwayland/waylandconfigreader.cpp

To: dvratil, sebas, #plasma, romangg
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D10042: feat: parse EDID on the backend side

[Daniel Vrátil]

dvratil marked an inline comment as done.
dvratil added inline comments.

INLINE COMMENTS

> romangg wrote in waylandbackend.cpp:81
> Let the backend populate the Private here is unusual, or is it not? On the 
> other side it makes sense. Let the consumer only access the public interface 
> and the backend access the private one. I don't know what's the commonly used 
> approach to that in Qt dev. Out of interest you can say? In any case we can 
> leave it like that for now.

I found this to be the simplest solution to avoid exposing some setters/factory 
functions into `Edid'`public interface. It's all internal code, so no risk of 
API/ABI breakages...

REPOSITORY
  R110 KScreen Library

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

To: dvratil, sebas, #plasma, romangg
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27675: feat: avoid duplicated text when assembling user-facing output names

[Daniel Vrátil]

dvratil added a comment.


  The system makes sense on X11 where you get something like `HDMI-1 (Sony 
GigaTV-12345)` so you see which device it is and what physical output it's 
connected to. Since KWin does not use to output names but only the names of the 
actually connected devices, the needed behavior is different.

INLINE COMMENTS

> ahmadsamir wrote in utils.cpp:64
> If I am reading this correctly, you could keep the old code and use 
> simplified():
> const QString outName = output->name().simplified();
> 
> then later on:
> name = name.simplified();
> if (!name.isEmpty() && name != oName) {
> 
>   return name + QLatin1Char('(') + outName + QLatin1Char(')');
> 
> }

Not really, the catch is in the whitespace at the end of `name` (then `"Vendor 
Model " == "Vendor Model"` is never true)

REPOSITORY
  R104 KScreen

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

To: dvratil, #plasma, romangg
Cc: ahmadsamir, romangg, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra, mart

D27677: feat: fix build after EDID changes in libkscreen

[Daniel Vrátil]

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

REPOSITORY
  R104 KScreen

BRANCH
  master

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

AFFECTED FILES
  console/console.cpp

To: dvratil, #plasma
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D10042: Parse EDID on the backend side

[Daniel Vrátil]

dvratil updated this revision to Diff 76459.
dvratil added a comment.


  - Rebased on master

REPOSITORY
  R110 KScreen Library

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10042?vs=25805=76459

BRANCH
  arcpatch-D10042

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

AFFECTED FILES
  CMakeLists.txt
  autotests/testconfigserializer.cpp
  autotests/testkwaylandbackend.cpp
  autotests/testqscreenbackend.cpp
  backends/fake/fake.cpp
  backends/fake/fake.h
  backends/kwayland/waylandbackend.cpp
  backends/kwayland/waylandbackend.h
  backends/xrandr/xrandr.cpp
  backends/xrandr/xrandr.h
  interfaces/org.kde.KScreen.Backend.xml
  src/CMakeLists.txt
  src/abstractbackend.cpp
  src/abstractbackend.h
  src/backendlauncher/backenddbuswrapper.cpp
  src/backendlauncher/backenddbuswrapper.h
  src/backendmanager.cpp
  src/configserializer.cpp
  src/configserializer_p.h
  src/doctor/doctor.cpp
  src/edid.cpp
  src/edid.h
  src/edid_p.h
  src/getconfigoperation.cpp
  src/output.cpp
  src/output.h
  src/types.h
  tests/kwayland/waylandconfigreader.cpp

To: dvratil, sebas
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27675: feat: avoid duplicated text when assembling user-facing output names

[Daniel Vrátil]

dvratil added a comment.


  This is needed for D10042  (which I'm 
rebasing right now) when even on KWayland the `KScreen::Output` will have EDID 
attached (assembled from information provided by KWayland/KWin). Since on 
KWayland the output name already is `Vendor Model` adding the information from 
EDID ends up with the string `Vendor Model (Vendor Model)`, that is in my case 
`Eizo Nanao Corporation DP-5-EV2736W/96211045 (Eizo Nanao Corporation 
DP-5-EV2736W/96211045)`, which is really long and kinda pointless.

REPOSITORY
  R104 KScreen

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

To: dvratil, #plasma
Cc: romangg, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27675: feat: Avoid duplicated text when assembling user-facing output names

[Daniel Vrátil]

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

REVISION SUMMARY
  The string format by default is `Vendor Model (output)`, where `Vendor`
  and `Model` are read from EDID and `output` is the name of the output
  reported by the platform.
  
  This patch adds logic to skip the `(output)` part if its the same as
  `Vendor Model`.

REPOSITORY
  R104 KScreen

BRANCH
  master

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

AFFECTED FILES
  common/utils.cpp

To: dvratil, #plasma
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27626: refactor: fix various compiler warnings

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R104:51fd586686a8: refactor: fix various compiler warnings 
(authored by dvratil).

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27626?vs=76302=76377

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

AFFECTED FILES
  common/control.cpp
  common/orientation_sensor.h
  kcm/kcm.cpp
  kcm/output_model.cpp
  kcm/output_model.h
  kded/config.cpp
  kded/generator.cpp
  kded/output.cpp
  tests/kded/configtest.cpp

To: dvratil, #plasma, romangg
Cc: romangg, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27626: refactor: fix various compiler warnings

[Daniel Vrátil]

dvratil added a comment.


  Also note that `noexcept` is not a compile-time check, it's a hint to enable 
some optimizations. It makes a difference when using standard library 
containers, e.g. `std::vector` will perform a copy if `T`'s move constructor 
is not `noexcept` in order to guarantee strong exceptions safety.

REPOSITORY
  R104 KScreen

BRANCH
  master

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

To: dvratil, #plasma, romangg
Cc: romangg, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27626: refactor: fix various compiler warnings

[Daniel Vrátil]

dvratil added a comment.


  A good rule of a thumb is that your move constructor and assignment operator 
should always be `noexcept` unless you have a very good reason for them not to 
be. Qt has most move constructors marked as noexcept - I found a few that are 
not marked but that looks more like an oversight rather than intent, more so 
that Qt does not use exceptions - so if you have a type composed Qt types, 
making the constructor `noexcept` is perfectly safe. For custom types, even if 
their move constructors/assignment operators are not marked as `noexcept` it's 
fairly safe to assume they do not throw since we generally do not use 
exceptions in KDE.

REPOSITORY
  R104 KScreen

BRANCH
  master

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

To: dvratil, #plasma, romangg
Cc: romangg, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27625: fix: handle when backend fails to load/initialize

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R110:ff98585ea554: fix: handle when backend fails to 
load/initialize (authored by dvratil).

REPOSITORY
  R110 KScreen Library

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27625?vs=76295=76359

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

AFFECTED FILES
  src/getconfigoperation.cpp

To: dvratil, #plasma, romangg
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27626: refactor: fix various compiler warnings

[Daniel Vrátil]

dvratil added inline comments.

INLINE COMMENTS

> romangg wrote in output_model.h:90
> Why to add and why noexcept?

Because in `output_model.cpp:224` we do `m_outputs.insert(i, Output(output, 
pos))`, which can make use of move semantics.

Making move operations `noexcept` is part of Cpp Core Guidelines 

 and because `QSharedPointer`'s move ctor is `noexcept` and `QPoint` is 
trivially movable, the move operations here are also `noexcept`.

If it has any benefit here, since we possibly don't even have exceptions 
enabled in kscreen, is a question - I wrote it by force of habit.

REPOSITORY
  R104 KScreen

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

To: dvratil, #plasma
Cc: romangg, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27096: Refactoring, improve validation in kcm, optimize

[Daniel Vrátil]

dvratil added inline comments.

INLINE COMMENTS

> config_keys.h:23
>  
> -#include 
> +static const char CONFIG_TRIGGERWORD[] = "triggerWord";
> +static const char CONFIG_ALIASES[] = "aliases";

You could use `static constexpr QStringView CONFIG_TRIGGERWORD = 
u"triggerWord"` to get something that's compile-time and easily used with 
QString API

REPOSITORY
  R114 Plasma Addons

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

To: alex, davidedmundson, ngraham, sitter, broulik, #plasma
Cc: dvratil, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27626: refactor: fix various compiler warnings

[Daniel Vrátil]

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

REVISION SUMMARY
  Fixes pessimizing move, implicit copy assignment operator, copies
  in range for loops, deprecated Qt methods, missing `final` keyword
  and unused lambda capture.
  
  Built with Clang 10

REPOSITORY
  R104 KScreen

BRANCH
  master

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

AFFECTED FILES
  common/control.cpp
  common/orientation_sensor.h
  kcm/kcm.cpp
  kcm/output_model.cpp
  kcm/output_model.h
  kded/config.cpp
  kded/generator.cpp
  kded/output.cpp
  tests/kded/configtest.cpp

To: dvratil, #plasma
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27625: fix: handle when backend fails to load/initialize

[Daniel Vrátil]

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

REVISION SUMMARY
  Fixes a null pointer dereference in GetConfigOperation when BackendManager
  fails to load a valid backend.

REPOSITORY
  R110 KScreen Library

BRANCH
  Plasma/5.18

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

AFFECTED FILES
  src/getconfigoperation.cpp

To: dvratil, #plasma
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27618: fix(kwayland): wait longer for connection timeout and retry

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R110:691c85879caf: fix(kwayland): wait longer for connection 
timeout and retry (authored by dvratil).

REPOSITORY
  R110 KScreen Library

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27618?vs=76269=76293

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

AFFECTED FILES
  backends/kwayland/waylandbackend.cpp
  backends/kwayland/waylandbackend.h
  backends/kwayland/waylandconfig.cpp
  backends/kwayland/waylandconfig.h

To: dvratil, #plasma, romangg
Cc: romangg, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27618: fix(kwayland): fix crash when KWayland connection times out

[Daniel Vrátil]

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

REVISION SUMMARY
  During login creating the connection to Wayland server may take longer
  than 1 second (KWin busy, high CPU and IO load, ...) causing
  WaylandConfig to time out and leave the backend in an inconsistent
  state.
  
  This patch increases the timeout to 3 seconds and sets the overall
  backend status to invalid. This will abort the pending KScreen operation
  and prevent it from interacting with the broken backend. The next
  KScreen operation will try to set up the backend again, hopefully
  successfuly this time.
  
  BUG: 416566
  FIXED-IN: 5.18.2

REPOSITORY
  R110 KScreen Library

BRANCH
  Plasma/5.18

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

AFFECTED FILES
  backends/kwayland/waylandbackend.cpp
  backends/kwayland/waylandbackend.h
  backends/kwayland/waylandconfig.cpp
  backends/kwayland/waylandconfig.h

To: dvratil, #plasma
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27537: Fix simpleWrite test in kwaylandbackendtest

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R110:2fcd093919b8: fix(autotests): fix simpleWrite test in 
kwaylandbackendtest (authored by dvratil).

REPOSITORY
  R110 KScreen Library

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27537?vs=76083=76104

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

AFFECTED FILES
  autotests/testkwaylandbackend.cpp

To: dvratil, #plasma, romangg
Cc: romangg, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra, mart

D27536: fix(kwayland): remove code with side-effects from Q_ASSERT

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R110:63cab28e74ee: fix(kwayland): remove code with 
side-effects from Q_ASSERT (authored by dvratil).

REPOSITORY
  R110 KScreen Library

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27536?vs=76082=76095

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

AFFECTED FILES
  backends/kwayland/waylandconfig.cpp

To: dvratil, #plasma, davidedmundson, meven
Cc: meven, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27537: Fix simpleWrite test in kwaylandbackendtest

[Daniel Vrátil]

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

REVISION SUMMARY
  The test still fails on removeOutput test, but that's fixed by
  D27536 .

TEST PLAN
  simpleWrite test passes

REPOSITORY
  R110 KScreen Library

BRANCH
  master

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

AFFECTED FILES
  autotests/testkwaylandbackend.cpp

To: dvratil, #plasma
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27536: fix(kwayland): remove code with side-effects from Q_ASSERT

[Daniel Vrátil]

dvratil updated this revision to Diff 76082.
dvratil added a comment.


  - Add bug reference

REPOSITORY
  R110 KScreen Library

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27536?vs=76080=76082

BRANCH
  Plasma/5.18

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

AFFECTED FILES
  backends/kwayland/waylandconfig.cpp

To: dvratil, #plasma
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27536: fix(kwayland): remove code with side-effects from Q_ASSERT

[Daniel Vrátil]

dvratil updated this revision to Diff 76080.
dvratil added a comment.


  - Fix typo

REPOSITORY
  R110 KScreen Library

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27536?vs=76079=76080

BRANCH
  Plasma/5.18

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

AFFECTED FILES
  backends/kwayland/waylandconfig.cpp

To: dvratil, #plasma
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27536: fix(kwayland): remove code with side-effects from Q_ASSERT

[Daniel Vrátil]

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

REPOSITORY
  R110 KScreen Library

BRANCH
  Plasma/5.18

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

AFFECTED FILES
  backends/kwayland/waylandconfig.cpp

To: dvratil
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D21905: [Digital Clock] Open KOrganizer with event details on click

[Daniel Vrátil]

dvratil added a comment.


  Also, could someone take a look at https://phabricator.kde.org/D21902 please? 
:)

REPOSITORY
  R120 Plasma Workspace

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

To: dvratil, #plasma
Cc: broulik, apol, ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, mart

D21905: [Digital Clock] Open KOrganizer with event details on click

[Daniel Vrátil]

dvratil added inline comments.

INLINE COMMENTS

> apol wrote in CalendarView.qml:196
> Maybe it should only be enabled if there's events to show?

This ListItem represents an event, so there's always an event :)

REPOSITORY
  R120 Plasma Workspace

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

To: dvratil, #plasma
Cc: broulik, apol, ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, mart

D21905: [Digital Clock] Open KOrganizer with event details on click

[Daniel Vrátil]

dvratil updated this revision to Diff 62597.
dvratil added a comment.


  - Implement Kai's suggestion

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21905?vs=60070=62597

BRANCH
  master

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

AFFECTED FILES
  applets/digital-clock/package/contents/ui/CalendarView.qml

To: dvratil, #plasma
Cc: broulik, apol, ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, mart

Re: plasma-thunderbolt

[Daniel Vrátil]

Hi Jonathan,

On Tuesday, 23 July 2019 16:26:21 CEST Jonathan Riddell wrote:
> Is Plasma Thunderbolt expected to be a new git repo to be released as part
> of Plasma 5.17?  If so it should go through kdereview pronto.

plasma-thunderbolt already went through kdereview back in May, you even 
commented on it [0] :) It's now in kde/workspace namespace.

I would like to have it shipped in Plasma 5.17, as I think it's ready to be 
released - let me know if there's anything that I need to take care of, 
please.

- Dan

[0] https://marc.info/?l=kde-core-devel=155842656405139=2


> 
> Jonathan


-- 
Daniel Vrátil
www.dvratil.cz | dvra...@kde.org
IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde)

GPG Key: 0x4D69557AECB13683
Fingerprint: 0ABD FA55 A4E6 BEA9 9A83 EA97 4D69 557A ECB1 3683

signature.asc
Description: This is a digitally signed message part.

D21905: [Digital Clock] Open KOrganizer with event details on click

[Daniel Vrátil]

dvratil added a comment.


  Nothing. Same will happen for example when users click on an event from the 
Holidays or Astronomical Events calendar plugins as there are no viewers for 
those events and KOrganizer can only open events provided by the PIM plugin.
  
  As far as I know, the agenda view is only implemented in the Digital Clock 
applet.

REPOSITORY
  R120 Plasma Workspace

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

To: dvratil, #plasma
Cc: ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D21905: [Digital Clock] Open KOrganizer with event details on click

[Daniel Vrátil]

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

REVISION SUMMARY
  Note that this depends on API that will be available first in KDE Frameworks 
5.60,
  so this should not be merged until Plasma master can depend on KF5 5.60, I'm 
just
  putting it here otherwise I'd lose the patch somewhere :) Alternatively this 
just
  adds a simple function call from QML, so if merged before 5.60 it will only 
cause
  a runtime warning and nothing will happen.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

AFFECTED FILES
  applets/digital-clock/package/contents/ui/CalendarView.qml

To: dvratil, #plasma
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D19011: Thunderbolt KCM and KDED module

[Daniel Vrátil]

dvratil abandoned this revision.
dvratil added a comment.


  The code has been moved to a standalone plasma-thunderbolt repository, passed 
through kde-review and will be included in Plasmas 5.17 release.
  
  Thanks everyone for your feedback and comments!

REPOSITORY
  R119 Plasma Desktop

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

To: dvratil, #vdg, #plasma
Cc: pino, zzag, GB_2, knambiar, bernieinnocenti, andreask, ndavis, 
trickyricky26, abetts, fabianr, ngraham, broulik, ognarb, yurchor, 
asturmlechner, plasma-devel, LeGast00n, jraleigh, fbampaloukas, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D21873: search lib paths for boltd for debian systems

[Daniel Vrátil]

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


  Thanks!

REPOSITORY
  R1044 Plasma Thunderbolt Integration

BRANCH
  master

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

To: sitter, dvratil
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D21527: Fix the 'Report bug' button being always disabled

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R871:dd95de337e30: Fix the Report bug button being 
always disabled (authored by dvratil).

REPOSITORY
  R871 DrKonqi

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21527?vs=58992=59019

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

AFFECTED FILES
  src/config-drkonqi.h.cmake
  src/drkonqidialog.cpp

To: dvratil, dfaure, #plasma
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D21527: Fix the 'Report bug' button being always disabled

[Daniel Vrátil]

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

REVISION SUMMARY
  The commit d4ae04a7 
 has 
changed #define of HAVE_XMLRPCLCIENT
  to #cmakedefine01 - since HAVE_XMLRPCCLIENT is not defined anywhere
  in the CMakeFiles.txt, this lead to the 'Report bug' button being
  unconditionally disabled at compile time.
  
  Since KXmlRpcClient is a REQUIRED dependency (and has been for a while),
  we can just remove the macro altogether.
  
  BUG: 406606
  FIXED-IN: 5.16.0
  CCMAIL: fa...@kde.org

TEST PLAN
  The 'Report bug' button is enabled

REPOSITORY
  R871 DrKonqi

BRANCH
  Plasma/5.16

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

AFFECTED FILES
  src/config-drkonqi.h.cmake
  src/drkonqidialog.cpp

To: dvratil, dfaure
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

Re: Review Request: plasma-thunderbolt

[Daniel Vrátil]

On Wednesday, 15 May 2019 23:08:57 CEST Albert Astals Cid wrote:
> El dimecres, 15 de maig de 2019, a les 15:27:07 CEST, Daniel Vrátil va 
escriure:
> > Hi all,
> > 
> > plasma-thunderbolt is a new repo containing, you guessed it, Thunderbolt
> > KCM for Plasma. I initially submitted the code as a patch against
> > plasma-desktop [0], where it got reviewed, but it was ultimately decided
> > to better put it into a separate repository, since it's not just a KCM
> > but also a library and a KDED module. I have backported all the changes
> > from the Phabricator review back to the repository, so the code in the
> > repo is identical to the one in the Phab review (minus buildsystem
> > changes and a small build fix for clang).
> > 
> > However, since this is still a new code, it must formally pass through
> > kdereview before I can submit it into Plasma as a new module.
> > 
> > Thus I'd kindly ask you to take one more look at the codebase [1] and let
> > me know if there are any more issues to fix, or if we can proceed to
> > include this in the next Plasma release.
> 
> There's this cmake warning
> 
> CMake Warning at /usr/lib64/cmake/KF5Package/KF5PackageMacros.cmake:74
> (message): KPackage components should be specified in reverse domain
> notation. Appstream information won't be generated for kcm_bolt.
> Call Stack (most recent call first):
>   src/kcm/CMakeLists.txt:22 (kpackage_install_package)

I need some help with this - I looked into plasma-desktop, but all KCMs 
generate this warning and after renaming the package I just wasn't able to 
pair it with the library (which makes not sense to be called 
org.kde.plasma.thunderbolt.so) - so for now I'd just ignore the warning (we 
don't need Appstream data for KCM anyway, do we?).

> clazy complains about a few Q_PROPERTY that should ideally either have a
> NOTIFY signal or be marked as CONSTANT

I fixed the ones in the code, I can't fix the ones in the generated DBus 
interfaces and adaptors.


> it also complains about a few const slots that return values. Seems like
> what you want is to actually mark them as invokable/scriptable and not as
> slots?

Good point, fixed those.

Thanks for the review,
Dan

> 
> Cheers,
>   Albert
> 
> > Thanks,
> > - Dan
> > 
> > [0] https://phabricator.kde.org/D19011
> > [1] https://cgit.kde.org/plasma-thunderbolt.git


-- 
Daniel Vrátil
www.dvratil.cz | dvra...@kde.org
IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde)

GPG Key: 0x4D69557AECB13683
Fingerprint: 0ABD FA55 A4E6 BEA9 9A83 EA97 4D69 557A ECB1 3683

signature.asc
Description: This is a digitally signed message part.

Re: Review Request: plasma-thunderbolt

[Daniel Vrátil]

On Wednesday, 15 May 2019 15:55:01 CEST Friedrich W. H. Kossebau wrote:
> Am Mittwoch, 15. Mai 2019, 15:27:07 CEST schrieb Daniel Vrátil:
> > Thus I'd kindly ask you to take one more look at the codebase [1] and let
> > me know if there are any more issues to fix, or if we can proceed to
> > include this in the next Plasma release.
> 
> Pushed some small fixes to toplevel CMakeLists.txt
> 
> Other things seen on quick look at code (also not tested runtime):
> * kded misses a Messages.sh file.
> * no COPYRIGHT license files in the repo
> * kde_enable_exceptions() duplicated a few times, perhaps only do in subdirs
> where needed or use of kde_target_enable_exceptions() if fitting
> * libkbolt being a private library could be reflected in the libname, also
> get install(TARGETS kbolt ${KDE_INSTALL_TARGETS_DEFAULT_ARGS} LIBRARY
> NAMELINK_SKIP)

All fixed, thanks for the review (and the fixes)!

Dan

> 
> Cheers
> Friedrich


-- 
Daniel Vrátil
www.dvratil.cz | dvra...@kde.org
IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde)

GPG Key: 0x4D69557AECB13683
Fingerprint: 0ABD FA55 A4E6 BEA9 9A83 EA97 4D69 557A ECB1 3683

signature.asc
Description: This is a digitally signed message part.

Review Request: plasma-thunderbolt

[Daniel Vrátil]

Hi all,

plasma-thunderbolt is a new repo containing, you guessed it, Thunderbolt KCM 
for Plasma. I initially submitted the code as a patch against plasma-desktop 
[0], where it got reviewed, but it was ultimately decided to better put it 
into a separate repository, since it's not just a KCM but also a library and a 
KDED module. I have backported all the changes from the Phabricator review 
back to the repository, so the code in the repo is identical to the one in the 
Phab review (minus buildsystem changes and a small build fix for clang).

However, since this is still a new code, it must formally pass through 
kdereview before I can submit it into Plasma as a new module.

Thus I'd kindly ask you to take one more look at the codebase [1] and let me 
know if there are any more issues to fix, or if we can proceed to include this 
in the next Plasma release.

Thanks,
- Dan

[0] https://phabricator.kde.org/D19011
[1] https://cgit.kde.org/plasma-thunderbolt.git

-- 
Daniel Vrátil
www.dvratil.cz | dvra...@kde.org
IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde)

GPG Key: 0x4D69557AECB13683
Fingerprint: 0ABD FA55 A4E6 BEA9 9A83 EA97 4D69 557A ECB1 3683

signature.asc
Description: This is a digitally signed message part.

D21184: Take the screen geometry into account when positioning popup notifications

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:1e175254dd8c: [Notifications] Take the screen geometry 
into account when positioning popup… (authored by dvratil).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21184?vs=57994=57999

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

AFFECTED FILES
  applets/notifications/package/contents/ui/global/Globals.qml

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

D21184: Take the screen geometry into account when positioning popup notifications

[Daniel Vrátil]

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

REVISION SUMMARY
  The availableScreenRect is relative to the current screen, so if the 
notification
  applet is not on the top-left screen, the notification popup will not appear 
next
  to where the applet is but on the top-left screen instead. Adding the absolute
  screen geometry to the availableScreenRect ensures that the popup is 
positioned
  on the right screen.

TEST PLAN
  Position two screens above each other, place the panel on the bottom screen, 
the
  notification now correctly appears on the bottom screen next to the applet.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

AFFECTED FILES
  applets/notifications/package/contents/ui/global/Globals.qml

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

D19011: Thunderbolt KCM and KDED module

[Daniel Vrátil]

dvratil added a comment.


  @broulik I asked on plasma-devel where to put this and got no definitive 
answer, so I put it here. Separate repo is fine with me, too. What name for the 
repo would you prefer? plasma-thunderbolt? I already have kcm_bolt scratch 
repo, so I can just "backport" the changes from here into it and ask sysadmins 
to move it to kde/workspace and rename it.
  
  The password prompt, I believe, is from polkit - I did not see it on my 
system. Maybe the polkit rules in boltd are wrong, but I believe we only do 
read-only operations when you open the KCM.

REPOSITORY
  R119 Plasma Desktop

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

To: dvratil, #vdg, #plasma
Cc: zzag, GB_2, knambiar, bernieinnocenti, andreask, ndavis, trickyricky26, 
abetts, fabianr, ngraham, broulik, ognarb, yurchor, asturmlechner, 
plasma-devel, jraleigh, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

D19011: Thunderbolt KCM and KDED module

[Daniel Vrátil]

dvratil updated this revision to Diff 57147.
dvratil added a comment.


  - Update the icon
  - Rebase on current master

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19011?vs=54625=57147

BRANCH
  arcpatch-D19011

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

AFFECTED FILES
  CMakeLists.txt
  autotests/CMakeLists.txt
  autotests/bolt/CMakeLists.txt
  autotests/bolt/data/default.json
  autotests/bolt/fakeserver/CMakeLists.txt
  autotests/bolt/fakeserver/fakedevice.cpp
  autotests/bolt/fakeserver/fakedevice.h
  autotests/bolt/fakeserver/fakemanager.cpp
  autotests/bolt/fakeserver/fakemanager.h
  autotests/bolt/fakeserver/fakeserver.cpp
  autotests/bolt/fakeserver/fakeserver.h
  autotests/bolt/fakeserver/main.cpp
  autotests/bolt/kded/CMakeLists.txt
  autotests/bolt/kded/kdedtest.cpp
  autotests/bolt/lib/CMakeLists.txt
  autotests/bolt/lib/devicetest.cpp
  autotests/bolt/lib/managertest.cpp
  cmake/modules/FindBolt.cmake
  kcms/CMakeLists.txt
  kcms/bolt/CMakeLists.txt
  kcms/bolt/Messages.sh
  kcms/bolt/kcm_bolt.cpp
  kcms/bolt/kcm_bolt.desktop
  kcms/bolt/kcm_bolt.h
  kcms/bolt/package/contents/ui/DeviceList.qml
  kcms/bolt/package/contents/ui/DeviceView.qml
  kcms/bolt/package/contents/ui/main.qml
  kcms/bolt/package/contents/ui/utils.js
  kcms/bolt/package/metadata.desktop
  kded/CMakeLists.txt
  kded/bolt/CMakeLists.txt
  kded/bolt/kded_bolt.cpp
  kded/bolt/kded_bolt.desktop
  kded/bolt/kded_bolt.h
  kded/bolt/kded_bolt.notifyrc
  kded/bolt/main.cpp
  libs/CMakeLists.txt
  libs/bolt/CMakeLists.txt
  libs/bolt/dbushelper.cpp
  libs/bolt/dbushelper.h
  libs/bolt/device.cpp
  libs/bolt/device.h
  libs/bolt/devicemodel.cpp
  libs/bolt/devicemodel.h
  libs/bolt/enum.cpp
  libs/bolt/enum.h
  libs/bolt/interfaces/org.freedesktop.bolt1.device.xml
  libs/bolt/interfaces/org.freedesktop.bolt1.manager.xml
  libs/bolt/manager.cpp
  libs/bolt/manager.h

To: dvratil, #vdg, #plasma
Cc: zzag, GB_2, knambiar, bernieinnocenti, andreask, ndavis, trickyricky26, 
abetts, fabianr, ngraham, broulik, ognarb, yurchor, asturmlechner, 
plasma-devel, jraleigh, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

D19011: Thunderbolt KCM and KDED module

[Daniel Vrátil]

dvratil added a comment.


  I realized we still need an icon for Thunderbolt (and thus for this KCM) - 
@abetts, @andreask, @trickyricky26, @ndavis - could one of you help me out, 
please? I have no clue about icons...

REPOSITORY
  R119 Plasma Desktop

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

To: dvratil, #vdg, #plasma
Cc: GB_2, knambiar, bernieinnocenti, andreask, ndavis, trickyricky26, abetts, 
fabianr, ngraham, broulik, ognarb, yurchor, asturmlechner, plasma-devel, 
jraleigh, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

Re: CI system maintainability

[Daniel Vrátil]

On Thursday, March 28, 2019 3:39:54 PM CET Friedrich W. H. Kossebau wrote:
> Am Donnerstag, 28. März 2019, 11:27:44 CET schrieb Daniel Vrátil:
> > I'm completely fine with mandatory code review for everything and I'd be
> > happy to have this in PIM. I think the biggest problem in PIM to overcome
> > will be finding the reviewers - I dare say I'm currently the only one who
> > has at least a little idea about what's going on in Akonadi, so getting
> > for
> > instance Laurent to review my Akonadi patches might be hard - same for me
> > reviewing Laurent's KMail patches. This will require non-trivial amount of
> > effort for all members of the community to gain deeper understanding of
> > the
> > other components within the project and a willingness to step up and do a
> > code review even if they don't feel 100% comfortable with the code base.
> > But I believe that in the long run the benefits clearly out-weight the
> > cost.
> So why do you not do this already? Why would you only do this is there is a
> policy requiring you to do it?
> And why do you think this policy-driven behavioural change will work or is
> needed with everyone else?

I wrote this with my PIM hat on and targeted the PIM community, without 
realizing this is a cross-post, so I apologize for the confusion. I believe 
each project should choose whatever workflow and policy suits them.

That said, I do put up for review everything that is not Akonadi/PIM core 
(even changes to PIM components that are not "mine"). The reason I don't do 
this for Akonadi is that there's no-one really to review my code, because no-
one else works on it, or at least that has been my perception of the situation 
lately.

I've been thinking about bringing this topic up on the upcoming PIM sprint as 
I would like to have my code reviewed, I think it's a very good and healthy 
thing, but in case of KDE PIM, I think we need to agree that we'll actually 
review each others code, even if it's not "our own" code-base.

> Also, how do you ensure the review is actually of quality?

How do you ensure that the code people commit without a review is of quality?

> And not just socially driven "+1 for my buddie!", where buddy then also
> mentally shifts part of responsibility to other buddy, because, he, more
> eyes means I do not need to do the full work myself, glitches will be
> caught be them surely! Reviewer found a code formatting issue, done here,
> review happened.

This is a fair point, and I certainly am guilty of doing this occasionally in 
the past. But even reviewer can have a reviewer: if you keep accepting patches 
of "dubious quality" (break build, don't work, contain typos ...), someone 
else from the community will eventually notice and point out you should maybe 
be more careful with your reviews or pay attention to certain aspects (like 
"does it actually work?).

> The opposite also has been seen, reviewers feel they need to do "real"
> review and find things, so start to nitpick or to demand wrong changes,
> wasting everyone's time.
> 
> For myself I know I will happily have other people review my patches, but
> only if there are qualified people to be expected to do it with the needed
> quality in a reasonable time.
> 
> Then, trying to force by that policy other people to become proper
> specialist for certain other code projects to do qualified reviews,
> actually is a trade- off. They will have less time for their actual code
> project, becoming less a specialist there (or having time to review other
> people's contribution to that project).
> We need more contributors, not force existing contributors to distribute
> their abilities & resources over more projects (and thus having less for
> their actual one).

AFAIU the policy right now is keep putting up patches for review until you get 
commit access, then continue doing so until you feel like you don't have to 
anymore. 

But having experienced contributors putting up their code for review is a good 
thing as it allows newcomers (and not just them!) to better understand what is 
happening in the project and simple reviews can be a good starting point for 
them to get more involved in the community.

> I am also not aware of many contributors to KDE projects who are not capable
> to make a responsible decision between the few obvious simple fixes and
> those normal changes which better get feedback from others first. If one is
> unsure about one's post-beer late-night quick hack, one will upload it for
> review, no? At least if one's previous late-night commits turned out to be
> late-night mental state impacted.

This I think is somewhat related to the tooling as I said in my previous 
email. If your workflow for simple obvious fix would be "git push HE

Re: CI system maintainability

[Daniel Vrátil]

On Thursday, March 28, 2019 9:50:47 AM CET Kevin Ottens wrote:
> Hello,
> 
> On Thursday, 28 March 2019 09:41:29 CET Luca Beltrame wrote:
> > In data giovedì 28 marzo 2019 09:29:22 CET, Kevin Ottens ha scritto:
> > > at your screen or pair with you" in the past. Clearly this compromise
> > > gets
> > > somewhat exploited and that's especially bad in the case of a fragile
> > > and
> > > central component like KDE PIM.
> > 
> > I'm not sure I agree. I can't speak for seasoned developers, but I've
> > found
> > myself in a situation (more than once) where the fix is trivial (compile
> > error, missing ";", etc) and being forced to go through review would (IMO)
> > unnecessarily raise friction.

This is partially a problem of tooling IMO - review for even a trivial change 
will not cause any friction, if the tooling makes it super-easy and natural 
part of your workflow (and then you can always poke someone on IRC "hey, do 
you have 5 seconds for this super-simple review?").

Using arc with Phab is a bit annoying, so I can understand people fighting 
this. Gitlab with their "click this link to turn your commit into a merge 
request" is much better, plus it can be merged by the reviewer with a single 
click so you as a developer don't even need to care about the MR anymore.

> I don't fully disagree with this (although I'd have stories on nefarious
> typos even for what was supposed to be a "trivial fix"). But it becomes a
> question of trade-off, IOW "how hurtful to the project mandatory reviews?"
> vs "how hurtful to the project a central component being very regularly
> broken?".
> 
> I'd argue we're loosing more with the current state of PIM than we'd loose
> with mandatory reviews.

I'm completely fine with mandatory code review for everything and I'd be happy 
to have this in PIM. I think the biggest problem in PIM to overcome will be 
finding the reviewers - I dare say I'm currently the only one who has at least 
a little idea about what's going on in Akonadi, so getting for instance 
Laurent to review my Akonadi patches might be hard - same for me reviewing 
Laurent's KMail patches. This will require non-trivial amount of effort for 
all members of the community to gain deeper understanding of the other 
components within the project and a willingness to step up and do a code 
review even if they don't feel 100% comfortable with the code base. But I 
believe that in the long run the benefits clearly out-weight the cost.

Btw we practice this policy at work and I do truly appreciate it, not only as 
a huge learning experience but so many times just having a second pair of eyes 
to glance over my code has revealed issues that sometimes almost make me 
question my career choice as a programmer :-) I think this is especially 
important for projects like PIM, where most of us contribute at work in 
between waiting for CI and replying to 15 Slack threads or in the evening 
after a long day

Cao,
Dan

> Regards.


-- 
Daniel Vrátil
www.dvratil.cz | dvra...@kde.org
IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde)

GPG Key: 0x4D69557AECB13683
Fingerprint: 0ABD FA55 A4E6 BEA9 9A83 EA97 4D69 557A ECB1 3683

signature.asc
Description: This is a digitally signed message part.

D19011: Thunderbolt KCM and KDED module

[Daniel Vrátil]

dvratil updated this revision to Diff 54625.
dvratil added a comment.


  Remove 'Help', 'Default' and 'Apply' buttons as they serve no purpose here,
  there's no help at the moment, there's no default state to revert to and the
  changes to devices are applied immediately.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19011?vs=53323=54625

BRANCH
  arcpatch-D19011

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

AFFECTED FILES
  CMakeLists.txt
  autotests/CMakeLists.txt
  autotests/bolt/CMakeLists.txt
  autotests/bolt/data/default.json
  autotests/bolt/fakeserver/CMakeLists.txt
  autotests/bolt/fakeserver/fakedevice.cpp
  autotests/bolt/fakeserver/fakedevice.h
  autotests/bolt/fakeserver/fakemanager.cpp
  autotests/bolt/fakeserver/fakemanager.h
  autotests/bolt/fakeserver/fakeserver.cpp
  autotests/bolt/fakeserver/fakeserver.h
  autotests/bolt/fakeserver/main.cpp
  autotests/bolt/kded/CMakeLists.txt
  autotests/bolt/kded/kdedtest.cpp
  autotests/bolt/lib/CMakeLists.txt
  autotests/bolt/lib/devicetest.cpp
  autotests/bolt/lib/managertest.cpp
  cmake/modules/FindBolt.cmake
  kcms/CMakeLists.txt
  kcms/bolt/CMakeLists.txt
  kcms/bolt/Messages.sh
  kcms/bolt/kcm_bolt.cpp
  kcms/bolt/kcm_bolt.desktop
  kcms/bolt/kcm_bolt.h
  kcms/bolt/package/contents/ui/DeviceList.qml
  kcms/bolt/package/contents/ui/DeviceView.qml
  kcms/bolt/package/contents/ui/main.qml
  kcms/bolt/package/contents/ui/utils.js
  kcms/bolt/package/metadata.desktop
  kded/CMakeLists.txt
  kded/bolt/CMakeLists.txt
  kded/bolt/kded_bolt.cpp
  kded/bolt/kded_bolt.desktop
  kded/bolt/kded_bolt.h
  kded/bolt/kded_bolt.notifyrc
  kded/bolt/main.cpp
  libs/CMakeLists.txt
  libs/bolt/CMakeLists.txt
  libs/bolt/dbushelper.cpp
  libs/bolt/dbushelper.h
  libs/bolt/device.cpp
  libs/bolt/device.h
  libs/bolt/devicemodel.cpp
  libs/bolt/devicemodel.h
  libs/bolt/enum.cpp
  libs/bolt/enum.h
  libs/bolt/interfaces/org.freedesktop.bolt1.device.xml
  libs/bolt/interfaces/org.freedesktop.bolt1.manager.xml
  libs/bolt/manager.cpp
  libs/bolt/manager.h

To: dvratil, #vdg, #plasma
Cc: knambiar, bernieinnocenti, andreask, ndavis, trickyricky26, abetts, 
fabianr, ngraham, broulik, ognarb, yurchor, asturmlechner, plasma-devel, 
jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

D19011: Thunderbolt KCM and KDED module

[Daniel Vrátil]

dvratil added a comment.


  Ping, VDG folks are OK with this, how about Plasma :)?

REPOSITORY
  R119 Plasma Desktop

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

To: dvratil, #vdg, #plasma
Cc: knambiar, bernieinnocenti, andreask, ndavis, trickyricky26, abetts, 
fabianr, ngraham, broulik, ognarb, yurchor, asturmlechner, plasma-devel, 
jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

D19011: Thunderbolt KCM and KDED module

[Daniel Vrátil]

dvratil updated this revision to Diff 53323.
dvratil added a comment.


  - Wording: Trust this Device/Revoke Trust button labels

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19011?vs=52775=53323

BRANCH
  arcpatch-D19011

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

AFFECTED FILES
  CMakeLists.txt
  autotests/CMakeLists.txt
  autotests/bolt/CMakeLists.txt
  autotests/bolt/data/default.json
  autotests/bolt/fakeserver/CMakeLists.txt
  autotests/bolt/fakeserver/fakedevice.cpp
  autotests/bolt/fakeserver/fakedevice.h
  autotests/bolt/fakeserver/fakemanager.cpp
  autotests/bolt/fakeserver/fakemanager.h
  autotests/bolt/fakeserver/fakeserver.cpp
  autotests/bolt/fakeserver/fakeserver.h
  autotests/bolt/fakeserver/main.cpp
  autotests/bolt/kded/CMakeLists.txt
  autotests/bolt/kded/kdedtest.cpp
  autotests/bolt/lib/CMakeLists.txt
  autotests/bolt/lib/devicetest.cpp
  autotests/bolt/lib/managertest.cpp
  cmake/modules/FindBolt.cmake
  kcms/CMakeLists.txt
  kcms/bolt/CMakeLists.txt
  kcms/bolt/Messages.sh
  kcms/bolt/kcm_bolt.cpp
  kcms/bolt/kcm_bolt.desktop
  kcms/bolt/kcm_bolt.h
  kcms/bolt/package/contents/ui/DeviceList.qml
  kcms/bolt/package/contents/ui/DeviceView.qml
  kcms/bolt/package/contents/ui/main.qml
  kcms/bolt/package/contents/ui/utils.js
  kcms/bolt/package/metadata.desktop
  kded/CMakeLists.txt
  kded/bolt/CMakeLists.txt
  kded/bolt/kded_bolt.cpp
  kded/bolt/kded_bolt.desktop
  kded/bolt/kded_bolt.h
  kded/bolt/kded_bolt.notifyrc
  kded/bolt/main.cpp
  libs/CMakeLists.txt
  libs/bolt/CMakeLists.txt
  libs/bolt/dbushelper.cpp
  libs/bolt/dbushelper.h
  libs/bolt/device.cpp
  libs/bolt/device.h
  libs/bolt/devicemodel.cpp
  libs/bolt/devicemodel.h
  libs/bolt/enum.cpp
  libs/bolt/enum.h
  libs/bolt/interfaces/org.freedesktop.bolt1.device.xml
  libs/bolt/interfaces/org.freedesktop.bolt1.manager.xml
  libs/bolt/manager.cpp
  libs/bolt/manager.h

To: dvratil, #vdg, #plasma
Cc: knambiar, bernieinnocenti, andreask, ndavis, trickyricky26, abetts, 
fabianr, ngraham, broulik, ognarb, yurchor, asturmlechner, plasma-devel, 
jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

D19011: Thunderbolt KCM and KDED module

[Daniel Vrátil]

dvratil updated this revision to Diff 52775.
dvratil added a comment.


  - Use colors in device status indicators

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19011?vs=52593=52775

BRANCH
  arcpatch-D19011

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

AFFECTED FILES
  CMakeLists.txt
  autotests/CMakeLists.txt
  autotests/bolt/CMakeLists.txt
  autotests/bolt/data/default.json
  autotests/bolt/fakeserver/CMakeLists.txt
  autotests/bolt/fakeserver/fakedevice.cpp
  autotests/bolt/fakeserver/fakedevice.h
  autotests/bolt/fakeserver/fakemanager.cpp
  autotests/bolt/fakeserver/fakemanager.h
  autotests/bolt/fakeserver/fakeserver.cpp
  autotests/bolt/fakeserver/fakeserver.h
  autotests/bolt/fakeserver/main.cpp
  autotests/bolt/kded/CMakeLists.txt
  autotests/bolt/kded/kdedtest.cpp
  autotests/bolt/lib/CMakeLists.txt
  autotests/bolt/lib/devicetest.cpp
  autotests/bolt/lib/managertest.cpp
  cmake/modules/FindBolt.cmake
  kcms/CMakeLists.txt
  kcms/bolt/CMakeLists.txt
  kcms/bolt/Messages.sh
  kcms/bolt/kcm_bolt.cpp
  kcms/bolt/kcm_bolt.desktop
  kcms/bolt/kcm_bolt.h
  kcms/bolt/package/contents/ui/DeviceList.qml
  kcms/bolt/package/contents/ui/DeviceView.qml
  kcms/bolt/package/contents/ui/main.qml
  kcms/bolt/package/contents/ui/utils.js
  kcms/bolt/package/metadata.desktop
  kded/CMakeLists.txt
  kded/bolt/CMakeLists.txt
  kded/bolt/kded_bolt.cpp
  kded/bolt/kded_bolt.desktop
  kded/bolt/kded_bolt.h
  kded/bolt/kded_bolt.notifyrc
  kded/bolt/main.cpp
  libs/CMakeLists.txt
  libs/bolt/CMakeLists.txt
  libs/bolt/dbushelper.cpp
  libs/bolt/dbushelper.h
  libs/bolt/device.cpp
  libs/bolt/device.h
  libs/bolt/devicemodel.cpp
  libs/bolt/devicemodel.h
  libs/bolt/enum.cpp
  libs/bolt/enum.h
  libs/bolt/interfaces/org.freedesktop.bolt1.device.xml
  libs/bolt/interfaces/org.freedesktop.bolt1.manager.xml
  libs/bolt/manager.cpp
  libs/bolt/manager.h

To: dvratil, #vdg, #plasma
Cc: knambiar, bernieinnocenti, andreask, ndavis, trickyricky26, abetts, 
fabianr, ngraham, broulik, ognarb, yurchor, asturmlechner, plasma-devel, 
jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

D19011: Thunderbolt KCM and KDED module

[Daniel Vrátil]

dvratil updated this revision to Diff 52593.
dvratil added a comment.


  - Wording: improved trust button labels, thanks for suggestions, Nate.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19011?vs=52568=52593

BRANCH
  arcpatch-D19011

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

AFFECTED FILES
  CMakeLists.txt
  autotests/CMakeLists.txt
  autotests/bolt/CMakeLists.txt
  autotests/bolt/data/default.json
  autotests/bolt/fakeserver/CMakeLists.txt
  autotests/bolt/fakeserver/fakedevice.cpp
  autotests/bolt/fakeserver/fakedevice.h
  autotests/bolt/fakeserver/fakemanager.cpp
  autotests/bolt/fakeserver/fakemanager.h
  autotests/bolt/fakeserver/fakeserver.cpp
  autotests/bolt/fakeserver/fakeserver.h
  autotests/bolt/fakeserver/main.cpp
  autotests/bolt/kded/CMakeLists.txt
  autotests/bolt/kded/kdedtest.cpp
  autotests/bolt/lib/CMakeLists.txt
  autotests/bolt/lib/devicetest.cpp
  autotests/bolt/lib/managertest.cpp
  cmake/modules/FindBolt.cmake
  kcms/CMakeLists.txt
  kcms/bolt/CMakeLists.txt
  kcms/bolt/Messages.sh
  kcms/bolt/kcm_bolt.cpp
  kcms/bolt/kcm_bolt.desktop
  kcms/bolt/kcm_bolt.h
  kcms/bolt/package/contents/ui/DeviceList.qml
  kcms/bolt/package/contents/ui/DeviceView.qml
  kcms/bolt/package/contents/ui/main.qml
  kcms/bolt/package/contents/ui/utils.js
  kcms/bolt/package/metadata.desktop
  kded/CMakeLists.txt
  kded/bolt/CMakeLists.txt
  kded/bolt/kded_bolt.cpp
  kded/bolt/kded_bolt.desktop
  kded/bolt/kded_bolt.h
  kded/bolt/kded_bolt.notifyrc
  kded/bolt/main.cpp
  libs/CMakeLists.txt
  libs/bolt/CMakeLists.txt
  libs/bolt/dbushelper.cpp
  libs/bolt/dbushelper.h
  libs/bolt/device.cpp
  libs/bolt/device.h
  libs/bolt/devicemodel.cpp
  libs/bolt/devicemodel.h
  libs/bolt/enum.cpp
  libs/bolt/enum.h
  libs/bolt/interfaces/org.freedesktop.bolt1.device.xml
  libs/bolt/interfaces/org.freedesktop.bolt1.manager.xml
  libs/bolt/manager.cpp
  libs/bolt/manager.h

To: dvratil, #vdg, #plasma
Cc: knambiar, bernieinnocenti, andreask, ndavis, trickyricky26, abetts, 
fabianr, ngraham, broulik, ognarb, yurchor, asturmlechner, plasma-devel, 
jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

D19011: Thunderbolt KCM and KDED module

[Daniel Vrátil]

dvratil updated this revision to Diff 52568.
dvratil added a comment.


  - Wording: replaced Stored with Trusted
  - Layout: centered all controls on device view page

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19011?vs=52255=52568

BRANCH
  arcpatch-D19011

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

AFFECTED FILES
  CMakeLists.txt
  autotests/CMakeLists.txt
  autotests/bolt/CMakeLists.txt
  autotests/bolt/data/default.json
  autotests/bolt/fakeserver/CMakeLists.txt
  autotests/bolt/fakeserver/fakedevice.cpp
  autotests/bolt/fakeserver/fakedevice.h
  autotests/bolt/fakeserver/fakemanager.cpp
  autotests/bolt/fakeserver/fakemanager.h
  autotests/bolt/fakeserver/fakeserver.cpp
  autotests/bolt/fakeserver/fakeserver.h
  autotests/bolt/fakeserver/main.cpp
  autotests/bolt/kded/CMakeLists.txt
  autotests/bolt/kded/kdedtest.cpp
  autotests/bolt/lib/CMakeLists.txt
  autotests/bolt/lib/devicetest.cpp
  autotests/bolt/lib/managertest.cpp
  cmake/modules/FindBolt.cmake
  kcms/CMakeLists.txt
  kcms/bolt/CMakeLists.txt
  kcms/bolt/Messages.sh
  kcms/bolt/kcm_bolt.cpp
  kcms/bolt/kcm_bolt.desktop
  kcms/bolt/kcm_bolt.h
  kcms/bolt/package/contents/ui/DeviceList.qml
  kcms/bolt/package/contents/ui/DeviceView.qml
  kcms/bolt/package/contents/ui/main.qml
  kcms/bolt/package/contents/ui/utils.js
  kcms/bolt/package/metadata.desktop
  kded/CMakeLists.txt
  kded/bolt/CMakeLists.txt
  kded/bolt/kded_bolt.cpp
  kded/bolt/kded_bolt.desktop
  kded/bolt/kded_bolt.h
  kded/bolt/kded_bolt.notifyrc
  kded/bolt/main.cpp
  libs/CMakeLists.txt
  libs/bolt/CMakeLists.txt
  libs/bolt/dbushelper.cpp
  libs/bolt/dbushelper.h
  libs/bolt/device.cpp
  libs/bolt/device.h
  libs/bolt/devicemodel.cpp
  libs/bolt/devicemodel.h
  libs/bolt/enum.cpp
  libs/bolt/enum.h
  libs/bolt/interfaces/org.freedesktop.bolt1.device.xml
  libs/bolt/interfaces/org.freedesktop.bolt1.manager.xml
  libs/bolt/manager.cpp
  libs/bolt/manager.h

To: dvratil, #vdg, #plasma
Cc: knambiar, bernieinnocenti, andreask, ndavis, trickyricky26, abetts, 
fabianr, ngraham, broulik, ognarb, yurchor, asturmlechner, plasma-devel, 
jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

D19011: Thunderbolt KCM and KDED module

[Daniel Vrátil]

dvratil added a comment.


  "Stored" is a term for a remembered device. When you authorize a device, the 
authorization is "stored", or remembered, so you don't have to authorize it 
again next time. Maybe "Remember" would be a better term, then?

REPOSITORY
  R119 Plasma Desktop

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

To: dvratil, #vdg, #plasma
Cc: bernieinnocenti, andreask, ndavis, trickyricky26, abetts, fabianr, ngraham, 
broulik, ognarb, yurchor, asturmlechner, plasma-devel, jraleigh, GB_2, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D19011: Thunderbolt KCM and KDED module

[Daniel Vrátil]

dvratil updated this revision to Diff 52255.
dvratil added a comment.


  - fix look of the device list

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19011?vs=51737=52255

BRANCH
  arcpatch-D19011

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

AFFECTED FILES
  CMakeLists.txt
  autotests/CMakeLists.txt
  autotests/bolt/CMakeLists.txt
  autotests/bolt/data/default.json
  autotests/bolt/fakeserver/CMakeLists.txt
  autotests/bolt/fakeserver/fakedevice.cpp
  autotests/bolt/fakeserver/fakedevice.h
  autotests/bolt/fakeserver/fakemanager.cpp
  autotests/bolt/fakeserver/fakemanager.h
  autotests/bolt/fakeserver/fakeserver.cpp
  autotests/bolt/fakeserver/fakeserver.h
  autotests/bolt/fakeserver/main.cpp
  autotests/bolt/kded/CMakeLists.txt
  autotests/bolt/kded/kdedtest.cpp
  autotests/bolt/lib/CMakeLists.txt
  autotests/bolt/lib/devicetest.cpp
  autotests/bolt/lib/managertest.cpp
  cmake/modules/FindBolt.cmake
  kcms/CMakeLists.txt
  kcms/bolt/CMakeLists.txt
  kcms/bolt/Messages.sh
  kcms/bolt/kcm_bolt.cpp
  kcms/bolt/kcm_bolt.desktop
  kcms/bolt/kcm_bolt.h
  kcms/bolt/package/contents/ui/DeviceList.qml
  kcms/bolt/package/contents/ui/DeviceView.qml
  kcms/bolt/package/contents/ui/main.qml
  kcms/bolt/package/contents/ui/utils.js
  kcms/bolt/package/metadata.desktop
  kded/CMakeLists.txt
  kded/bolt/CMakeLists.txt
  kded/bolt/kded_bolt.cpp
  kded/bolt/kded_bolt.desktop
  kded/bolt/kded_bolt.h
  kded/bolt/kded_bolt.notifyrc
  kded/bolt/main.cpp
  libs/CMakeLists.txt
  libs/bolt/CMakeLists.txt
  libs/bolt/dbushelper.cpp
  libs/bolt/dbushelper.h
  libs/bolt/device.cpp
  libs/bolt/device.h
  libs/bolt/devicemodel.cpp
  libs/bolt/devicemodel.h
  libs/bolt/enum.cpp
  libs/bolt/enum.h
  libs/bolt/interfaces/org.freedesktop.bolt1.device.xml
  libs/bolt/interfaces/org.freedesktop.bolt1.manager.xml
  libs/bolt/manager.cpp
  libs/bolt/manager.h

To: dvratil, #vdg, #plasma
Cc: bernieinnocenti, andreask, ndavis, trickyricky26, abetts, fabianr, ngraham, 
broulik, ognarb, yurchor, asturmlechner, plasma-devel, jraleigh, GB_2, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D19011: Thunderbolt KCM and KDED module

[Daniel Vrátil]

dvratil added a comment.


  Thanks for the feedback, I'll look into it asap. In the meantime, could I ask 
the VDG to create a Breeze icon for Thunderbolt? @abetts, maybe?

REPOSITORY
  R119 Plasma Desktop

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

To: dvratil, #vdg, #plasma
Cc: abetts, fabianr, ngraham, broulik, ognarb, yurchor, asturmlechner, 
plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

D19011: Thunderbolt KCM and KDED module

[Daniel Vrátil]

dvratil added inline comments.

INLINE COMMENTS

> broulik wrote in DeviceList.qml:11
> You might want to be using a `ScrollViewKCM` and put the enable checkbox in 
> the `header` and the `ListView` in `view`, see for instance KWin's virtual 
> desktop KCM

Hmm, but then I couldn't push the device view page on the stack, because there 
would be no PageRow, right?

> broulik wrote in device.h:45
> Does bolt not notify property changes on dbus? Not a huge fan of this Timer 
> refresh hack

Unfortunately not, the DBus interface is a bit weird and the implementation 
even weirder.

REPOSITORY
  R119 Plasma Desktop

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

To: dvratil
Cc: broulik, ognarb, yurchor, asturmlechner, plasma-devel, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D19011: Thunderbolt KCM and KDED module

[Daniel Vrátil]

dvratil updated this revision to Diff 51737.
dvratil marked 9 inline comments as done.
dvratil added a comment.


  - Fixed typos
  - Unified license
  - Addresses comments from kbroulik

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19011?vs=51693=51737

BRANCH
  boltkcm

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

AFFECTED FILES
  CMakeLists.txt
  autotests/CMakeLists.txt
  autotests/bolt/CMakeLists.txt
  autotests/bolt/data/default.json
  autotests/bolt/fakeserver/CMakeLists.txt
  autotests/bolt/fakeserver/fakedevice.cpp
  autotests/bolt/fakeserver/fakedevice.h
  autotests/bolt/fakeserver/fakemanager.cpp
  autotests/bolt/fakeserver/fakemanager.h
  autotests/bolt/fakeserver/fakeserver.cpp
  autotests/bolt/fakeserver/fakeserver.h
  autotests/bolt/fakeserver/main.cpp
  autotests/bolt/kded/CMakeLists.txt
  autotests/bolt/kded/kdedtest.cpp
  autotests/bolt/lib/CMakeLists.txt
  autotests/bolt/lib/devicetest.cpp
  autotests/bolt/lib/managertest.cpp
  cmake/modules/FindBolt.cmake
  kcms/CMakeLists.txt
  kcms/bolt/CMakeLists.txt
  kcms/bolt/Messages.sh
  kcms/bolt/kcm_bolt.cpp
  kcms/bolt/kcm_bolt.desktop
  kcms/bolt/kcm_bolt.h
  kcms/bolt/package/contents/ui/DeviceList.qml
  kcms/bolt/package/contents/ui/DeviceView.qml
  kcms/bolt/package/contents/ui/main.qml
  kcms/bolt/package/contents/ui/utils.js
  kcms/bolt/package/metadata.desktop
  kded/CMakeLists.txt
  kded/bolt/CMakeLists.txt
  kded/bolt/kded_bolt.cpp
  kded/bolt/kded_bolt.desktop
  kded/bolt/kded_bolt.h
  kded/bolt/kded_bolt.notifyrc
  kded/bolt/main.cpp
  libs/CMakeLists.txt
  libs/bolt/CMakeLists.txt
  libs/bolt/dbushelper.cpp
  libs/bolt/dbushelper.h
  libs/bolt/device.cpp
  libs/bolt/device.h
  libs/bolt/devicemodel.cpp
  libs/bolt/devicemodel.h
  libs/bolt/enum.cpp
  libs/bolt/enum.h
  libs/bolt/interfaces/org.freedesktop.bolt1.device.xml
  libs/bolt/interfaces/org.freedesktop.bolt1.manager.xml
  libs/bolt/manager.cpp
  libs/bolt/manager.h

To: dvratil
Cc: broulik, ognarb, yurchor, asturmlechner, plasma-devel, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D19011: Thunderbolt KCM and KDED module

[Daniel Vrátil]

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

REVISION SUMMARY
  The KCM allows user to authorize and store (or forget) the authorization
  of Thunderbolt 3 devices. This is done by communicating with the Bolt
  daemon via DBus (throug libkbolt).
  
  The KDED module listens for new unauthorized devices and shows a
  notification with a button to authorize when such device is connected.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  boltkcm

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

AFFECTED FILES
  CMakeLists.txt
  autotests/CMakeLists.txt
  autotests/bolt/CMakeLists.txt
  autotests/bolt/data/default.json
  autotests/bolt/fakeserver/CMakeLists.txt
  autotests/bolt/fakeserver/fakedevice.cpp
  autotests/bolt/fakeserver/fakedevice.h
  autotests/bolt/fakeserver/fakemanager.cpp
  autotests/bolt/fakeserver/fakemanager.h
  autotests/bolt/fakeserver/fakeserver.cpp
  autotests/bolt/fakeserver/fakeserver.h
  autotests/bolt/fakeserver/main.cpp
  autotests/bolt/kded/CMakeLists.txt
  autotests/bolt/kded/kdedtest.cpp
  autotests/bolt/lib/CMakeLists.txt
  autotests/bolt/lib/devicetest.cpp
  autotests/bolt/lib/managertest.cpp
  cmake/modules/FindBolt.cmake
  kcms/CMakeLists.txt
  kcms/bolt/CMakeLists.txt
  kcms/bolt/Messages.sh
  kcms/bolt/kcm_bolt.cpp
  kcms/bolt/kcm_bolt.desktop
  kcms/bolt/kcm_bolt.h
  kcms/bolt/package/contents/ui/DeviceList.qml
  kcms/bolt/package/contents/ui/DeviceView.qml
  kcms/bolt/package/contents/ui/main.qml
  kcms/bolt/package/contents/ui/utils.js
  kcms/bolt/package/metadata.desktop
  kded/CMakeLists.txt
  kded/bolt/CMakeLists.txt
  kded/bolt/kded_bolt.cpp
  kded/bolt/kded_bolt.desktop
  kded/bolt/kded_bolt.h
  kded/bolt/kded_bolt.notifyrc
  kded/bolt/main.cpp
  libs/CMakeLists.txt
  libs/bolt/CMakeLists.txt
  libs/bolt/dbushelper.cpp
  libs/bolt/dbushelper.h
  libs/bolt/device.cpp
  libs/bolt/device.h
  libs/bolt/devicemodel.cpp
  libs/bolt/devicemodel.h
  libs/bolt/enum.cpp
  libs/bolt/enum.h
  libs/bolt/interfaces/org.freedesktop.bolt1.device.xml
  libs/bolt/interfaces/org.freedesktop.bolt1.manager.xml
  libs/bolt/manager.cpp
  libs/bolt/manager.h

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

Re: Thunderbolt KCM

[Daniel Vrátil]

On Thursday, February 7, 2019 6:22:59 AM CET Rajeesh K V wrote:
> First, is this something you Plasma folks would like to have in regular
> 
> > Plasma
> > releases, or should I just make it an extragear project? If you think it
> > makes
> > sense to include it in Plasma, should it go to plasma-workspace, or should
> > it
> > be a standalone thing, like plasma-nm? It does have a runtime dependency
> > on
> > the Bolt daemon, which people without Thunderbolt in their computer don't
> > really need...
> 
> FWIW, Gnome has great support for Thunderbolt devices (see [1] and blog
> posts therein, and the design [2]). With more and more devices supporting
> Thunderbolt, I suspect it would be great to offer the support in Plasma
> Workspace. Just my 2¢.

The guy who did the Thunderbolt integration for Gnome also wrote and maintains 
the Bolt daemon [0] as a Freedesktop project. My KCM is talking to this daemon 
as well, so we can easily offer the same level of functionality and 
integration as Gnome.

[0] https://gitlab.freedesktop.org/bolt/bolt
> 
> [1]
> https://christian.kellner.me/2018/04/23/the-state-of-thunderbolt-3-in-fedora
> -28/ [2]  https://wiki.gnome.org/Design/Whiteboards/ThunderboltAccess


-- 
Daniel Vrátil
www.dvratil.cz | dvra...@kde.org
IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde)

GPG Key: 0x4D69557AECB13683
Fingerprint: 0ABD FA55 A4E6 BEA9 9A83 EA97 4D69 557A ECB1 3683

signature.asc
Description: This is a digitally signed message part.

Thunderbolt KCM

[Daniel Vrátil]

Hi all,

last year I got a new Dell laptop at work with a docking station connected via 
the Thunderbolt thingy and I found out that there's a DBus daemon for Linux 
that allows users to authorize TB devices...long story short I somehow 
happened to have created a KCM to manage TB devices and a KDED daemon that 
shows a notification when an unauthorized device is connected. 

Today I finally got the thing into a working and somewhat presentable state, 
so I think it's about time to get some input and help from you :-)

First, is this something you Plasma folks would like to have in regular Plasma 
releases, or should I just make it an extragear project? If you think it makes 
sense to include it in Plasma, should it go to plasma-workspace, or should it 
be a standalone thing, like plasma-nm? It does have a runtime dependency on 
the Bolt daemon, which people without Thunderbolt in their computer don't 
really need...

If you are interested, the code currently lives in my scratch repo [0]. It's 
kinda hard to test if you don't happen to have some Thunderbolt hardware at 
hand, so there's a "fakeserver" binary in autotests that pretends to be the 
Bolt daemon:

./fakeserver --cfg kcm_bolt/autotest/data/default.json
KBOLT_FAKE=1 kcmshell5 kcm_bolt

You should see some devices in the KCM now and you should be able to click 
around a little. But as I said, it's hard to test it properly without real 
hardware (which is why I'm still sitting in the office right now :)), but it 
works for me (TM). Anyway I uploaded a short shaky and out-of-focus video of 
the KCM in action to Youtube [1] for you to see how it works with real 
hardware.

Let me know what you guys think :)

Cheers,
Daniel

PS: looking for a volunteer to create a Thunderbolt icon for Breeze for me ;-)

[0] https://cgit.kde.org/scratch/dvratil/kcm_bolt.git
[1] https://www.youtube.com/watch?v=Ivx7RHH_LBE

-- 
Daniel Vrátil
www.dvratil.cz | dvra...@kde.org
IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde)

GPG Key: 0x4D69557AECB13683
Fingerprint: 0ABD FA55 A4E6 BEA9 9A83 EA97 4D69 557A ECB1 3683


signature.asc
Description: This is a digitally signed message part.

D18093: Calculate screen scaling dynamically, so it always fits to the page

[Daniel Vrátil]

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


  Looks good

REPOSITORY
  R104 KScreen

BRANCH
  dynamic_scale

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

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

D18101: Don't reset the moved display position

[Daniel Vrátil]

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


  Looks OK to me.

REPOSITORY
  R104 KScreen

BRANCH
  fix_reset_output_pos

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

To: amantia, dvratil
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17682: Show unconnected displays at the right side of the main screen

[Daniel Vrátil]

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


  Sorry for the delay, I did not notice the review.
  
  Looks good.

INLINE COMMENTS

> qmlscreen.cpp:178
>  {
> -return 1.0 / 8.0;
> +return 1.0 / 12.0;
>  }

Maybe the scale could be adaptive based on the number of connected outputs? 
Just an idea, if you are planning to play around with this code a bit more :-)

REPOSITORY
  R104 KScreen

BRANCH
  rework_qml_part

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

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

D17686: Redraw the slider if modes have changed

[Daniel Vrátil]

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


  Looks good to me. This whole thing could probably use some cleanup, but 
that's for another day :-)

REPOSITORY
  R104 KScreen

BRANCH
  Plasma/5.12

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

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

D17657: Notify also if modes have changed

[Daniel Vrátil]

dvratil added inline comments.

INLINE COMMENTS

> amantia wrote in output.cpp:583-585
> Good point, missed it. @dvratil any reason why it emits outputChanged and not 
> modesChanged? Actually setModes emits both, so maybe indeed it is easier to 
> just put modesChnaged there as well. I'm still curious why we need both.

There are properties that are related to the overall output state and will 
never change individually (e.g. id, type, name). Technically modes /may/ change 
independently of the output (it is possible to add modes in XRandR), but 
historically we did not support that, so modes were always coupled with the 
output state as well. I think this is the correct approach to fix a bug, but 
longterm someone should probably look into what happens when the 
`outputChanged()` signal is removed from here and from `setModes()` as there 
might be some code listening to it instead of `modesChanged()`.

REPOSITORY
  R110 KScreen Library

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

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

D17685: Notify also if modes have changed

[Daniel Vrátil]

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


  You can close the review for master and commit this to the stable branch, 
then merge stable to master.

REPOSITORY
  R110 KScreen Library

BRANCH
  Plasma/5.12

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

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

D16898: [kcm] Remove profiles code

[Daniel Vrátil]

dvratil added a subscriber: afiestas.
dvratil added a comment.


  Profiles were supposed to allow various screen layouts per single setup, for 
instance, if you have your monitor and TV both connected to your PC, you may 
want to have "Monitor", "Monitor+TV" and "TV-only" profiles. The idea was to 
have a combobox in the KCM that would allow you to quickly switch between those 
profiles without having to fiddle with the rest of the GUI every time. IIRC we 
did not agree with @afiestas on this feature back then, so it remained disabled 
(should've been on a branch instead of #ifdefs on master, I admit that ) and 
while I was still working on kscreen, I never got around to actually finish it 
and push it.

REPOSITORY
  R104 KScreen

BRANCH
  removeProfilesCode

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

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

D10040: Add serial number and EISA ID to OutputDevice interface

[Daniel Vrátil]

dvratil added a comment.


  In D10040#286064 , @davidedmundson 
wrote:
  
  > @dvratil want me to finish this?
  
  
  @davidedmundson  Yes, I'd appreciate it. I won't have time to look into this 
any time soon, sorry :(

REPOSITORY
  R127 KWayland

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

To: dvratil, graesslin, sebas, #kwin
Cc: kde-frameworks-devel, davidedmundson, plasma-devel, ragreen, Pitel, 
schernikov, michaelh, ZrenBot, ngraham, bruns, alexeymin, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein

D13487: Fix build against static Qt without QLibrary

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R169:8a473e6e9ef0: Fix build against static Qt without 
QLibrary (authored by dvratil).

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13487?vs=36028=36408

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

AFFECTED FILES
  src/libkirigami/platformtheme.cpp

To: dvratil, mart
Cc: plasma-devel, apol, davidedmundson, mart, hein

D13487: Fix build against static Qt without QLibrary

[Daniel Vrátil]

dvratil created this revision.
Restricted Application added a project: Kirigami.
Restricted Application added a subscriber: plasma-devel.
dvratil requested review of this revision.

REVISION SUMMARY
  Sometimes when Qt is built statically it is also built without support for 
QLibrary and friends, so wrap relevant code in Kirigami in `if 
QT_CONFIG(library)`.

REPOSITORY
  R169 Kirigami

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

AFFECTED FILES
  src/libkirigami/platformtheme.cpp

To: dvratil
Cc: plasma-devel, apol, davidedmundson, mart, hein

D12685: Fix calendar view: hide unused start/end time labels in day events list

[Daniel Vrátil]

dvratil added a comment.


  I could imagine using something like  "14:00 →" to indicate that the event 
starts today at 2PM but does not end today, and "→ 16:00" to indicate that this 
is a multi-day event that didn't start today and ends today at 4PM.
  
  Not sure how to indicate a multi-day event that does not start or end today. 
Maybe just with the arrow without any time?

REPOSITORY
  R120 Plasma Workspace

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

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

D11519: NotifcationPopup: workaround crash in Qt when appending empty actions array

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:27854ca9f48a: NotifcationPopup: workaround crash in Qt 
when appending empty actions array (authored by dvratil).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11519?vs=30041=30053

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

AFFECTED FILES
  applets/notifications/package/contents/ui/NotificationPopup.qml

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

D11519: NotifcationPopup: workaround crash in Qt when appending empty actions array

[Daniel Vrátil]

dvratil updated this revision to Diff 30041.
dvratil added a comment.


  Update explanation

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11519?vs=30030=30041

BRANCH
  master

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

AFFECTED FILES
  applets/notifications/package/contents/ui/NotificationPopup.qml

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

D11519: NotifcationPopup: workaround crash in Qt when appending empty actions array

[Daniel Vrátil]

dvratil updated this revision to Diff 30030.
dvratil added a comment.


  Add comment with explanation

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11519?vs=30020=30030

BRANCH
  master

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

AFFECTED FILES
  applets/notifications/package/contents/ui/NotificationPopup.qml

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

D11518: Use model reset rather than beginRemoveRows() to clear font groups

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:d97d360ba408: Use model reset rather than 
beginRemoveRows() to clear font groups (authored by dvratil).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11518?vs=30017=30027

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

AFFECTED FILES
  kcms/kfontinst/kcmfontinst/GroupList.cpp

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

D11519: NotifcationPopup: workaround crash in Qt when appending empty actions array

[Daniel Vrátil]

dvratil created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
dvratil requested review of this revision.

REVISION SUMMARY
  Appending an empty array of actions to QQmlListModel triggers an assert
  in Qt when built in debug mode, see 
https://codereview.qt-project.org/#/c/223985/
  for details.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

AFFECTED FILES
  applets/notifications/package/contents/ui/NotificationPopup.qml

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

D11518: Use model reset rather than beginRemoveRows() to clear font groups

[Daniel Vrátil]

dvratil created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
dvratil requested review of this revision.

REVISION SUMMARY
  Calling beginRemoveRows() when the model is empty hits an assert in
  QAbstractItemModel which requires that the "last" argument is less
  than model row count, which is not true when the model is empty.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  master

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

AFFECTED FILES
  kcms/kfontinst/kcmfontinst/GroupList.cpp

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

D10896: Show config selection OSD when display button is pressed

[Daniel Vrátil]

dvratil accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R104 KScreen

BRANCH
  sebas/displaybutton

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

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

D10896: Show config selection OSD when display button is pressed

[Daniel Vrátil]

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


  Hmm, I guess you can now kill `m_buttonTimer`, `m_iteration`,  
`resetDisplaySwitch()` and `applyGenericConfig()` as well, as those are used 
exclusively by the config iteration. You probably also want to check if the 
Selection OSD is already shown, otherwise pressing the Display Button multiple 
times would create multiple OSDs.

REPOSITORY
  R104 KScreen

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

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

D9414: Implement OSD to select action when unknown monitors is connected

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R104:5feb84305560: Merge branch dvratil/osd 
(authored by dvratil).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D9414?vs=25635=27873#toc

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9414?vs=25635=27873

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

AFFECTED FILES
  CMakeLists.txt
  kded/CMakeLists.txt
  kded/daemon.cpp
  kded/daemon.h
  kded/osd.cpp
  kded/osd.h
  kded/osdmanager.cpp
  kded/osdmanager.h
  kded/qml/Osd.qml
  kded/qml/OsdItem.qml
  kded/qml/OsdSelector.qml
  kded/qml/OutputIdentifier.qml
  tests/CMakeLists.txt
  tests/osd/CMakeLists.txt
  tests/osd/main.cpp
  tests/osd/osdtest.cpp
  tests/osd/osdtest.h

To: dvratil, sebas, davidedmundson, #vdg
Cc: abetts, broulik, kamathraghavendra, graesslin, ngraham, plasma-devel, 
mlaurent, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D9414: Implement OSD to select action when unknown monitors is connected

[Daniel Vrátil]

dvratil added a comment.


  @ngraham Yes, the screenshot is still accurate. If you want a screenshot with 
more of background or another background, I can provide one.

REPOSITORY
  R104 KScreen

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

To: dvratil, sebas, davidedmundson, #vdg
Cc: abetts, broulik, kamathraghavendra, graesslin, ngraham, plasma-devel, 
mlaurent, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D10040: Add serial number and EISA ID to OutputDevice interface

[Daniel Vrátil]

dvratil updated this revision to Diff 25806.
dvratil marked an inline comment as done.
dvratil added a comment.


  Fix versioning

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10040?vs=25802=25806

BRANCH
  master

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

AFFECTED FILES
  src/client/outputdevice.cpp
  src/client/outputdevice.h
  src/client/protocols/outputdevice.xml
  src/server/outputdevice_interface.cpp
  src/server/outputdevice_interface.h

To: dvratil, graesslin, sebas
Cc: davidedmundson, plasma-devel, #frameworks, schernikov, ZrenBot, ngraham, 
alexeymin, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, 
sebas, apol, mart, hein

D10040: Add serial number and EISA ID to OutputDevice interface

[Daniel Vrátil]

dvratil marked 2 inline comments as done.
dvratil added inline comments.

INLINE COMMENTS

> davidedmundson wrote in outputdevice_interface.cpp:450
> You're not actually sending the serial number or eisa anywhere..
> 
> The others do it in sendGeometry (including the manufacturer)
> 
> When you do add it, you need to check the client is bound with version 2, 
> otherwise you'll crash the client.
> 
> Something like
>  if(wl_proxy_get_version(resourceOfBoundClient) >= 
> ORG_KDE_KWIN_SETSERIAL_SINCE_VERSION) {
> 
>   org_kde_output_...

I am sending the stuff in `sendGeometry()` (see right above)

REPOSITORY
  R127 KWayland

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

To: dvratil, graesslin, sebas
Cc: davidedmundson, plasma-devel, #frameworks, schernikov, ZrenBot, ngraham, 
alexeymin, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, 
sebas, apol, mart, hein

D10042: Parse EDID on the backend side

[Daniel Vrátil]

dvratil added a dependency: D10040: Add serial number and EISA ID to 
OutputDevice interface.

REPOSITORY
  R110 KScreen Library

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

To: dvratil, sebas
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D10040: Add serial number and EISA ID to OutputDevice interface

[Daniel Vrátil]

dvratil added a dependent revision: D10042: Parse EDID on the backend side.

REPOSITORY
  R127 KWayland

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

To: dvratil, graesslin, sebas
Cc: davidedmundson, plasma-devel, #frameworks, schernikov, ZrenBot, ngraham, 
alexeymin, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, 
sebas, apol, mart, hein

D10042: Parse EDID on the backend side

[Daniel Vrátil]

dvratil created this revision.
dvratil added a reviewer: sebas.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
dvratil requested review of this revision.

REVISION SUMMARY
  Parse EDID on the backend side so that backends that don't have access
  to raw EDID blob can still populate the Edid object with data they have
  available.

REPOSITORY
  R110 KScreen Library

BRANCH
  master

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

AFFECTED FILES
  autotests/testconfigserializer.cpp
  autotests/testkwaylandbackend.cpp
  autotests/testqscreenbackend.cpp
  backends/fake/fake.cpp
  backends/fake/fake.h
  backends/kwayland/waylandbackend.cpp
  backends/kwayland/waylandbackend.h
  backends/xrandr/xrandr.cpp
  backends/xrandr/xrandr.h
  interfaces/org.kde.KScreen.Backend.xml
  src/abstractbackend.cpp
  src/abstractbackend.h
  src/backendlauncher/backenddbuswrapper.cpp
  src/backendlauncher/backenddbuswrapper.h
  src/backendmanager.cpp
  src/configserializer.cpp
  src/configserializer_p.h
  src/doctor/doctor.cpp
  src/edid.cpp
  src/edid.h
  src/edid_p.h
  src/getconfigoperation.cpp
  src/output.cpp
  src/output.h
  src/types.h
  tests/kwayland/waylandconfigreader.cpp

To: dvratil, sebas
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D10040: Add serial number and EISA ID to OutputDevice interface

[Daniel Vrátil]

dvratil added a dependent revision: D10041: Improve EDID information on Wayland.

REPOSITORY
  R127 KWayland

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

To: dvratil, graesslin, sebas
Cc: plasma-devel, #frameworks, schernikov, ZrenBot, ngraham, alexeymin, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, 
apol, mart, hein

D10040: Add serial number and EISA ID to OutputDevice interface

[Daniel Vrátil]

dvratil created this revision.
dvratil added reviewers: graesslin, sebas.
Restricted Application added subscribers: Frameworks, plasma-devel.
Restricted Application added projects: Plasma on Wayland, Frameworks.
dvratil requested review of this revision.

REPOSITORY
  R127 KWayland

BRANCH
  master

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

AFFECTED FILES
  src/client/outputdevice.cpp
  src/client/outputdevice.h
  src/client/protocols/outputdevice.xml
  src/server/outputdevice_interface.cpp
  src/server/outputdevice_interface.h

To: dvratil, graesslin, sebas
Cc: plasma-devel, #frameworks, schernikov, ZrenBot, ngraham, alexeymin, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, 
apol, mart, hein

D9414: Implement OSD to select action when unknown monitors is connected

[Daniel Vrátil]

dvratil updated this revision to Diff 25635.
dvratil added a comment.


  Please disregard the noise in this patch, sebas merged master into the 
dvratil/osd branch so I can't generate a proper diff against the base revision 
without the noise from master.

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9414?vs=25432=25635

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

AFFECTED FILES
  .reviewboardrc
  CMakeLists.txt
  kcm/kcm_kscreen.desktop.cmake
  kded/CMakeLists.txt
  kded/daemon.cpp
  kded/daemon.h
  kded/osd.cpp
  kded/osd.h
  kded/osdmanager.cpp
  kded/osdmanager.h
  kded/qml/Osd.qml
  kded/qml/OsdSelector.qml
  kded/serializer.cpp
  tests/osd/CMakeLists.txt
  tests/osd/main.cpp
  tests/osd/osdtest.cpp
  tests/osd/osdtest.h

To: dvratil, sebas, davidedmundson, #vdg
Cc: abetts, broulik, kamathraghavendra, graesslin, ngraham, plasma-devel, 
mlaurent, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, 
apol, mart

D9414: Implement OSD to select action when unknown monitors is connected

[Daniel Vrátil]

dvratil marked 15 inline comments as done.
dvratil added inline comments.

INLINE COMMENTS

> broulik wrote in osdmanager.cpp:60
> Shouldn't it rather do that in the plugin's `registerTypes`?

We don't have a QML plugin :( This is the only C++ class that is exposed to the 
OSD QML.

> broulik wrote in OsdSelector.qml:79
> Why do you need an `IconItem` inside? The `Button` can have an icon of its own

It's a workaround for Button's iconSource displaying the icon as 24x24  
regardless of the size of the button, so we end up with massive button with a 
tiny icon.

REPOSITORY
  R104 KScreen

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

To: dvratil, sebas, davidedmundson, #vdg
Cc: abetts, broulik, kamathraghavendra, graesslin, ngraham, plasma-devel, 
mlaurent, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, 
apol, mart

D9462: KCM: show scaled outputs correctly

[Daniel Vrátil]

dvratil added a comment.


  Indeed, I'll rework the whole scaling thing to abstract away the difference 
between scaling in Wayland and in X11 including the UI side (it's not related 
to the OSD changes, I'm just trying to work around my silly decision to get a 
hi-dpi monitor alongside a regular one)

REPOSITORY
  R104 KScreen

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

To: dvratil, sebas, davidedmundson
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9414: Implement OSD to select action when unknown monitors is connected

[Daniel Vrátil]

dvratil added a comment.


  In https://phabricator.kde.org/D9414#191653, @broulik wrote:
  
  > I think `OnScreenDisplay` implies being non-interactive on Wayland
  
  
  I could interact with the dialog on Wayland, so either it's not implied, or 
there's a bug in KWin :-)

REPOSITORY
  R104 KScreen

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

To: dvratil, sebas, davidedmundson, #vdg
Cc: broulik, kamathraghavendra, graesslin, ngraham, plasma-devel, mlaurent, 
ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D9908: Wayland: add support for output type detection

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R110:b1428d1381e8: Wayland: add support for output type 
detection (authored by dvratil).

REPOSITORY
  R110 KScreen Library

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9908?vs=25431=25486

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

AFFECTED FILES
  backends/kwayland/CMakeLists.txt
  backends/kwayland/waylandoutput.cpp
  backends/utils.cpp
  backends/utils.h
  backends/xrandr/CMakeLists.txt
  backends/xrandr/xrandroutput.cpp

To: dvratil, sebas, davidedmundson
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9906: Wayland: don't update output configuration if there are no changes

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R110:daf107948d58: Wayland: dont update output 
configuration if there are no changes (authored by dvratil).

REPOSITORY
  R110 KScreen Library

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9906?vs=25428=25485

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

AFFECTED FILES
  backends/kwayland/waylandconfig.cpp

To: dvratil, sebas, davidedmundson
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9905: Don't create multiple in-process backend instances

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R110:6e37f3034f6c: Dont create multiple in-process 
backend instances (authored by dvratil).

REPOSITORY
  R110 KScreen Library

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9905?vs=25427=25484

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

AFFECTED FILES
  src/backendmanager.cpp

To: dvratil, sebas, davidedmundson
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D9904: Wayland: don't share the same Config between backend and clients

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R110:4abdc5b134a3: Wayland: dont share the same Config 
between backend and clients (authored by dvratil).

REPOSITORY
  R110 KScreen Library

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9904?vs=25424=25483

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

AFFECTED FILES
  src/backendmanager.cpp
  src/configmonitor.cpp
  src/getconfigoperation.cpp

To: dvratil, sebas, davidedmundson
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart