D16093: Consistent arrow key handling in the Informative Alt+Tab skin

[Nathaniel Graham]

ngraham added a comment.


  Hmm, this does not actually work for me. Seems like it logically should, but 
it doesn't.

REPOSITORY
  R114 Plasma Addons

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

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

D16889: Listen to KDE Connect device signals

[Kai Uwe Broulik]

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

REVISION SUMMARY
  Makes sure we detect devices being added, removed, or becoming (in)visible at 
runtime.

TEST PLAN
  Probably never touched this code after it "worked as a proof of concept"
  
  - Turned wifi on on my phone, kde connect device context menu entry showed up
  - turned wifi off on my phone, kde connect device context menu entry vanished
  - reloaded extension while phone was connected: still got context menu entry
  - sending links to phone still works

REPOSITORY
  R856 Plasma Browser Integration

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

AFFECTED FILES
  extension/extension.js
  host/kdeconnectplugin.cpp
  host/kdeconnectplugin.h

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

D16888: Recreate interrupted download when it is resumed

[Kai Uwe Broulik]

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

REVISION SUMMARY
  When a download is canceled, finishes, or is interrupted (e.g. loss of 
network connection) we finish the `KJob` to remove it from notification area.
  However, when a job was interrupted and is then resumed, the download ID is 
reused by the browser and we only get sent updates for a job we no longer have.
  When we get a change event from `interrupted` to `in_progress` just pretend a 
new download was created and carry on as normal.
  
  BUG: 401038

TEST PLAN
  - Started a download, cut my network, waited for it to fail, resumed it: now 
got a download job in notification area, previously nothing would happen
  - Canceling and restarting the download still works fine
  - Starting a new download still works fine

REPOSITORY
  R856 Plasma Browser Integration

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

AFFECTED FILES
  extension/extension.js

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

D16887: Send all active downloads on load

[Kai Uwe Broulik]

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

REVISION SUMMARY
  Ensures when the extension is reloaded any downloads currently running are 
still shown in notifications

TEST PLAN
  I remember getting some bogus `downloads.onCreated` events on browser startup 
but when reloading the extension doesn't seem to do this
  
  - started a download, reloaded the extension, download was removed and 
re-created in notifications applet instead of just vanishing

REPOSITORY
  R856 Plasma Browser Integration

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

AFFECTED FILES
  extension/extension.js

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

D16886: [windowswitcher] Implement keyboard navigation

[Nathaniel Graham]

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

REVISION SUMMARY
  Implements keyboard navigation via the up and down arrow keys for the Breeze 
window switcher.
  
  CCBUG: 370185

TEST PLAN
  Alt-tab, use arrow keys, it works!

REPOSITORY
  R120 Plasma Workspace

BRANCH
  breeze-window-switcher-keyboard-navigation (branched from master)

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

AFFECTED FILES
  lookandfeel/contents/windowswitcher/WindowSwitcher.qml

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

D16410: [Bookmarks Runner] Avoid leaking FetchSqlite instances

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:9e90fdd2a593: [Bookmarks Runner] Avoid leaking 
FetchSqlite instances (authored by bruns).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16410?vs=44170=45490

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

AFFECTED FILES
  runners/bookmarks/browsers/firefox.cpp

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

D16409: [Bookmarks Runner] Open database connection in the query thread

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:145caeac1050: [Bookmarks Runner] Open database connection 
in the query thread (authored by bruns).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16409?vs=44169=45489

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

AFFECTED FILES
  runners/bookmarks/fetchsqlite.cpp
  runners/bookmarks/fetchsqlite.h

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

D16861: Improve contrast for crosshair cursors

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R31:39c5b09297cc: Improve contrast for crosshair cursors 
(authored by ndavis, committed by ngraham).

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16861?vs=45482=45484

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

AFFECTED FILES
  cursors/Breeze/Breeze/cursors/crosshair
  cursors/Breeze/build/x1/crosshair.png
  cursors/Breeze/build/x1_5/crosshair.png
  cursors/Breeze/build/x2/crosshair.png
  cursors/Breeze/src/cursors.svg
  cursors/Breeze_Snow/Breeze_Snow/cursors/crosshair
  cursors/Breeze_Snow/build/x1/crosshair.png
  cursors/Breeze_Snow/build/x1_5/crosshair.png
  cursors/Breeze_Snow/build/x2/crosshair.png
  cursors/Breeze_Snow/src/cursors.svg

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

D16082: Notify headphone being plugged in on some hardware

[Nathaniel Graham]

ngraham added a comment.


  My recommendation would be to abandon the diff or change it to address the 
problem of the OSD checkbox not being clear about what it affects.

REPOSITORY
  R115 Plasma Audio Volume Applet

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

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

D16082: Notify headphone being plugged in on some hardware

[Thomas Surrel]

thsurrel added a comment.


  Should I do anything special about this Diff ? Do we just keep it as it is 
for reference ?

REPOSITORY
  R115 Plasma Audio Volume Applet

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

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

D16861: Improve contrast for crosshair cursors

[Nathaniel Graham]

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


  Lovely work :)
  
  I notice this is now branched off of master. Could you land this on the 
`Plasma/5.12` branch and then merge to `Plasma/5.14` and then master? This is 
documented at 
https://community.kde.org/Infrastructure/Phabricator#Landing_on_the_.22Stable_branch.22,
 but please feel free to hit me up for details if anything isn't clear.

REPOSITORY
  R31 Breeze

BRANCH
  master

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

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

D16861: Improve contrast for crosshair cursors

[Noah Davis]

ndavis updated this revision to Diff 45482.
ndavis added a comment.


  Only change crosshair cursors and source SVG

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16861?vs=45434=45482

BRANCH
  master

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

AFFECTED FILES
  cursors/Breeze/Breeze/cursors/crosshair
  cursors/Breeze/build/x1/crosshair.png
  cursors/Breeze/build/x1_5/crosshair.png
  cursors/Breeze/build/x2/crosshair.png
  cursors/Breeze/src/cursors.svg
  cursors/Breeze_Snow/Breeze_Snow/cursors/crosshair
  cursors/Breeze_Snow/build/x1/crosshair.png
  cursors/Breeze_Snow/build/x1_5/crosshair.png
  cursors/Breeze_Snow/build/x2/crosshair.png
  cursors/Breeze_Snow/src/cursors.svg

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

D16082: Notify headphone being plugged in on some hardware

[Nathaniel Graham]

ngraham added a comment.


  In D16082#359352 , @thsurrel wrote:
  
  > Absolutely, as said in my previous comment, I agree the usefulness of the 
OSD in that case is very much questionable.
  >  I was raising questions on a different level:
  >
  > - would it be import to be consistent and show an OSD in the jack case if 
we are showing one in the bluetooth case ?
  
  
  IMHO no. The bluetooth OSD is useful, while the jack OSD is annoying noise.
  
  > - the plasma-pa option is hard to understand if a user will not see any 
difference in behavior between checked and not checked.
  
  That's true. Perhaps we could re-word it to better describe under what 
circumstances it will show an OSD.
  
  > That being said, I lived happily with no OSD notification and would have 
turned it off had this patch made its way in.
  >  I posted this patch in the first place only because _I assumed_ this was 
working for other laptops. I thought I was fixing a corner case, not changing a 
de facto behavior.
  
  Yeah, it seems that this was not a bug but rather a feature. :)

REPOSITORY
  R115 Plasma Audio Volume Applet

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

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

D16875: [kded] Cleanup KScreenDaemon class

[Roman Gilg]

This revision was automatically updated to reflect the committed changes.
Closed by commit R104:4a177884b052: [kded] Cleanup KScreenDaemon class 
(authored by romangg).

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16875?vs=45466=45469

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

AFFECTED FILES
  kded/daemon.cpp
  kded/daemon.h

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

D16874: [kded] Replace configReady functon with lambda

[Roman Gilg]

This revision was automatically updated to reflect the committed changes.
Closed by commit R104:980b100770bb: [kded] Replace configReady functon with 
lambda (authored by romangg).

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16874?vs=45446=45467

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

AFFECTED FILES
  kded/daemon.cpp
  kded/daemon.h

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

D16875: [kded] Cleanup KScreenDaemon class

[Roman Gilg]

romangg updated this revision to Diff 45466.
romangg added a comment.


  - Add back Q_SLOTS specifier to be sure

REPOSITORY
  R104 KScreen

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16875?vs=45448=45466

BRANCH
  codeCleanupExport

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

AFFECTED FILES
  kded/daemon.cpp
  kded/daemon.h

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

D16875: [kded] Cleanup KScreenDaemon class

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> romangg wrote in daemon.h:47
> Are you sure this is a problem? I just tested it manually with qdbusviewer 
> and the method was called accordingly.
> 
> Nowadays in Qt can't every function act as a slot, even without the Q_SLOTS 
> specifier?

If it's done with QDBusConnection::registerObject(this)  then it definitely 
needs runtime introspection.

If it's done with an adaptor pattern, then maybe not.

REPOSITORY
  R104 KScreen

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

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

D16875: [kded] Cleanup KScreenDaemon class

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

> romangg wrote in daemon.h:47
> Are you sure this is a problem? I just tested it manually with qdbusviewer 
> and the method was called accordingly.
> 
> Nowadays in Qt can't every function act as a slot, even without the Q_SLOTS 
> specifier?

For function ptr connect, yes, for runtime introspection, no

REPOSITORY
  R104 KScreen

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

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

D16875: [kded] Cleanup KScreenDaemon class

[Roman Gilg]

romangg added inline comments.

INLINE COMMENTS

> davidedmundson wrote in daemon.h:47
> this has moved from being a slot.
> 
> If it is invoked from DBus as the comment suggests that will be a problem

Are you sure this is a problem? I just tested it manually with qdbusviewer and 
the method was called accordingly.

Nowadays in Qt can't every function act as a slot, even without the Q_SLOTS 
specifier?

REPOSITORY
  R104 KScreen

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

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

D16879: [Login and Lock screens] Improve UI elements' contrast a bit

[Root]

rooty added a comment.


  nice! i've been tinkering with this some more over the past few days (it's 
been driving me a little nuts), and i've been using "spread: 0.1" on the clock 
shadow while keeping "spread: 0.35" on the username and action button shadows
  i've been doing this because "0.35" provides a strong outline that seems to 
make the lettering stick out on a very bright/white/lit up background, which is 
perfect for the username/action buttons but might be a little too much for the 
clock to handle
  
  nate/guys, what do you think?
  
  F6424178: image.png  with the clock 
shadow set to "spread: 0.35" (slightly rough around the edges... so to speak :D)
  F6424174: image.png  with the clock 
shadow set to "spread: 0.1" (too weak? above 0.1 the kinks/bends in the 
typeface get slightly distorted by the drop shadows, as in the 0.35 screenshot)

REPOSITORY
  R120 Plasma Workspace

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

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

D16031: [SDDM theme] remove blur and increase UI contrast so that it's not required

[Nathaniel Graham]

ngraham added a comment.


  @davidedmundson I've put only the fairly non-controversial changes into 
D16879 .

REPOSITORY
  R120 Plasma Workspace

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

To: ngraham, #vdg, #plasma, rizzitello, davidedmundson
Cc: romangg, davidedmundson, rizzitello, abetts, pstefan, broulik, rikmills, 
filipf, rooty, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

D16879: [Login and Lock screens] Improve UI elements' contrast a bit

[Nathaniel Graham]

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

REVISION SUMMARY
  As requested in D16031 , tweak the login 
and lock screens in only fairly non-controversial ways:
  
  - Add a dark background behind the user avatar
  - Add shadows behind the username and the buttons
  - Tweak the clock shadow to make it a tiny bit stronger
  - Reduce clock shadow redundancy by moving it into the `Clock` item and out 
of the users and not using hardcoded colors anymore
  
  This diff does NOT include the following changes from D16031 
:
  
  - Blur-less login screen
  - New icons for the buttons
  - Horizontal bar on the bottom to hold the buttons for the virtual keyboard, 
session chooser, and bettery status

TEST PLAN
  [images go here]

REPOSITORY
  R120 Plasma Workspace

BRANCH
  lock-and-login-screen-contrast-tweaks (branched from master)

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

AFFECTED FILES
  lookandfeel/contents/components/ActionButton.qml
  lookandfeel/contents/components/Clock.qml
  lookandfeel/contents/components/UserDelegate.qml
  lookandfeel/contents/lockscreen/LockScreenUi.qml
  sddm-theme/Main.qml

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

D16082: Notify headphone being plugged in on some hardware

[Thomas Surrel]

thsurrel added a comment.


  Absolutely, as said in my previous comment, I agree the usefulness of the OSD 
in that case is very much questionable.
  I was raising questions on a different level:
  
  - would it be import to be consistent and show an OSD in the jack case if we 
are showing one in the bluetooth case ?
  - the plasma-pa option is hard to understand if a user will not see any 
difference in behavior between checked and not checked.
  
  That being said, I lived happily with no OSD notification and would have 
turned it off had this patch made its way in.
  I posted this patch in the first place only because _I assumed_ this was 
working for other laptops. I thought I was fixing a corner case, not changing a 
de facto behavior.

REPOSITORY
  R115 Plasma Audio Volume Applet

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

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

D16082: Notify headphone being plugged in on some hardware

[Nathaniel Graham]

ngraham added a comment.


  Well, there are types of visual feedback that are useful and there are types 
that aren't. A "Bluetooth device successfully paired" notification is useful 
because there was always some chance that it would fail, and you need to know 
whether or not that happened. I could see a "headphones/speakers plugged in" 
notification being //theoretically// useful because it communicates that all 
running streams will be redirected to the new audio output... but isn't that 
what the user was expecting to happen anyway? Is there any value to notifying 
the user that what they expected to happen happened when they performed an 
action that was 100% guaranteed to succeed? This may be just me, but I find 
such feedback to be really annoying. People likewise complain about the "you 
trashed this item!" notification in Dolphin 
. They think, "duh I trashed it, 
you don't need to tell me that it worked!"

REPOSITORY
  R115 Plasma Audio Volume Applet

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

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

D15512: [startplasmacompositor] Add Wayland socket argument

[Fabian Vogt]

fvogt added a comment.


  It looks like all of that is already implemented in libwayland-server.
  kwin_wayland just needs to make use of `wl_display_add_socket_auto` (other 
compositors do as well).
  
  In D15512#359308 , @graesslin 
wrote:
  
  > In D15512#359253 , @fvogt wrote:
  >
  > > I'd be fine with this change if it were more generic and allowed to add 
arbitrary arguments to kwin_wayland.
  >
  >
  > No other option makes sense to pass to kwin from startplasmacompositor.
  
  
  I would say most of the options used for kwin_wayland make sense except for 
"--help" and such. Let's say `dbus-run-session startplasmacompositor 
--output-count 2` to start a windowed plasma session with two windows for 
example.

REPOSITORY
  R120 Plasma Workspace

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

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

D15512: [startplasmacompositor] Add Wayland socket argument

[Martin Flöser]

graesslin added a comment.


  In D15512#359253 , @fvogt wrote:
  
  > > KWin doesn't need this functionality and shouldn't have this 
functionality and it would be difficult to implement.
  >
  > Why? That goes entirely against what I and probably many others expect from 
a kwin_wayland call without explicit socket argument.
  
  
  This was a deliberate design decision when writing the code. One reason is 
that code performing the check and then trying to create the socket is racy. 
Such a race does not belong into KWin.
  
  Also for me the reason "it was like that in X" is not valid. The only valid 
question is: what makes sense from a KWin perspective.
  
  > kwin_wayland already checks whether a socket is used, it can't be that hard 
to extend that to iterate a few socket names.
  
  No kwin_wayland does not check whether a socket is used.
  
  > If you suggest to implement that in a layer higher than kwin, that's not 
possible as kwin has to create the .lock file itself.
  
  The lock file is not created by KWin, but by the wayland library. That is a 
layer higher than KWin. If such functionality should exist it should be in 
libwayland-server and from there available to all compositors. I refuse to 
accept that every compositor has to implement racy code.
  
  > I'd be fine with this change if it were more generic and allowed to add 
arbitrary arguments to kwin_wayland.
  
  No other option makes sense to pass to kwin from startplasmacompositor.

REPOSITORY
  R120 Plasma Workspace

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

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

D16875: [kded] Cleanup KScreenDaemon class

[David Edmundson]

davidedmundson requested changes to this revision.
davidedmundson added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> daemon.h:47
> +// DBus
> +void applyLayoutPreset(const QString );
> +

this has moved from being a slot.

If it is invoked from DBus as the comment suggests that will be a problem

REPOSITORY
  R104 KScreen

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

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

D16874: [kded] Replace configReady functon with lambda

[David Edmundson]

davidedmundson added a comment.


  > Note, that the configReady function was a public slot in an exported class, 
so is it safe?
  
  it's not in a library that people link to, so you can do whatever.

REPOSITORY
  R104 KScreen

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

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

D16875: [kded] Cleanup KScreenDaemon class

[Roman Gilg]

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

REVISION SUMMARY
  Do not export class, privatize and unvirtualize members.
  Cleanup white space.

TEST PLAN
  Manually in Wayland session.

REPOSITORY
  R104 KScreen

BRANCH
  codeCleanupExport

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

AFFECTED FILES
  kded/daemon.cpp
  kded/daemon.h

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

D16874: [daemon] Replace configReady functon with lambda

[Roman Gilg]

romangg added a comment.


  Note, that the configReady function was a public slot in an exported class, 
so is it safe? I would like to change other stuff in this class definition. I 
don't know why the class is exported and who consumes it.

REPOSITORY
  R104 KScreen

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

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

D16874: [daemon] Replace configReady functon with lambda

[Roman Gilg]

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

REVISION SUMMARY
  Purely for cosmetic reasons.

TEST PLAN
  Compiles.

REPOSITORY
  R104 KScreen

BRANCH
  codeCleanupDaemonStartup

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

AFFECTED FILES
  kded/daemon.cpp
  kded/daemon.h

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

D16082: Notify headphone being plugged in on some hardware

[Thomas Surrel]

thsurrel added a comment.


  Having a visual feedback would be useful when pairing bluetooth speakers IMO 
(does it ? I don't have any to test with).
  
  For a jack connection, that is indeed less useful, but that would be 
consistent. In addition, the plasma-pa has an option to turn the OSD off, right 
now having it checked or not does not change anything when plugging headphones 
in a laptop.

REPOSITORY
  R115 Plasma Audio Volume Applet

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

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

D15512: [startplasmacompositor] Add Wayland socket argument

[Fabian Vogt]

fvogt added a comment.


  > KWin doesn't need this functionality and shouldn't have this functionality 
and it would be difficult to implement.
  
  Why? That goes entirely against what I and probably many others expect from a 
kwin_wayland call without explicit socket argument.
  
  kwin_wayland already checks whether a socket is used, it can't be that hard 
to extend that to iterate a few socket names.
  
  If you suggest to implement that in a layer higher than kwin, that's not 
possible as kwin has to create the .lock file itself.
  
  I'd be fine with this change if it were more generic and allowed to add 
arbitrary arguments to kwin_wayland.

REPOSITORY
  R120 Plasma Workspace

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

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