D5872: pidChanged also signals dataChanged in WindowModel

2017-05-17 Thread Sebastian Kügler
sebas abandoned this revision. sebas added a comment. Okay, thanks David. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D5872 To: sebas, #plasma, hein, davidedmundson Cc: graesslin, bshah, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff,

D5872: pidChanged also signals dataChanged in WindowModel

2017-05-17 Thread David Edmundson
davidedmundson added a comment. I merged a different thing that Martin was happy wtih. This is closable. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D5872 To: sebas, #plasma, hein, davidedmundson Cc: graesslin, bshah, davidedmundson, plasma-devel, #frameworks,

D5872: pidChanged also signals dataChanged in WindowModel

2017-05-16 Thread Martin Flöser
graesslin added a comment. In https://phabricator.kde.org/D5872#109962, @hein wrote: > I'm fine with no change signal, but heads-up that libtaskmanager has code connecting to it, so if you remove it from kwayland please also adapt plasma-workspace or the build breaks. I highly

D5872: pidChanged also signals dataChanged in WindowModel

2017-05-16 Thread Eike Hein
hein added a comment. No, I don't. libtaskmanager doesn't use PlasmaWindowModel, it uses PlasmaWindow. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D5872 To: sebas, #plasma, hein, davidedmundson Cc: graesslin, bshah, davidedmundson, plasma-devel, #frameworks,

D5872: pidChanged also signals dataChanged in WindowModel

2017-05-16 Thread David Edmundson
davidedmundson added a comment. No you don't. You connect via QAIM;: dataChanged which is still there. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D5872 To: sebas, #plasma, hein, davidedmundson Cc: graesslin, bshah, davidedmundson, plasma-devel, #frameworks,

D5872: pidChanged also signals dataChanged in WindowModel

2017-05-16 Thread Eike Hein
hein added a comment. I'm fine with no change signal, but heads-up that libtaskmanager has code connecting to it, so if you remove it from kwayland please also adapt plasma-workspace or the build breaks. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D5872 To:

D5872: pidChanged also signals dataChanged in WindowModel

2017-05-16 Thread Martin Flöser
graesslin added a comment. like David explained: we don't need the change signal and IMHO we shouldn't expose "wrong API". I consider having a changed signal for a PID wrong API as it indicates that the PID could change. But it won't ever change, so we shouldn't expose it. I know that all

D5872: pidChanged also signals dataChanged in WindowModel

2017-05-15 Thread David Edmundson
davidedmundson requested changes to this revision. davidedmundson added a comment. This revision now requires changes to proceed. > it can still be set after being initially 0. In fact, it has to be. Not at the time the client sees it. see initialStateCallback which is important.

D5872: pidChanged also signals dataChanged in WindowModel

2017-05-15 Thread Sebastian Kügler
sebas added a comment. Possibly, in this case, I disagree. The pid is initially unknown set to in the client, and can change afterwards. In reality, the pid of the window doesn't change, but it can still be set after being initially 0. In fact, it has to be. In the end, I don't care

D5872: pidChanged also signals dataChanged in WindowModel

2017-05-15 Thread Sebastian Kügler
sebas requested review of this revision. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D5872 To: sebas, #plasma, hein, davidedmundson Cc: bshah, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts,

D5872: pidChanged also signals dataChanged in WindowModel

2017-05-15 Thread Bhushan Shah
bshah added a comment. In https://phabricator.kde.org/D5872#109874, @sebas wrote: > What's d581? I think he meant https://phabricator.kde.org/D5851 REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D5872 To: sebas, #plasma, hein, davidedmundson Cc: bshah,

D5872: pidChanged also signals dataChanged in WindowModel

2017-05-15 Thread Sebastian Kügler
sebas added a comment. What's d581? REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D5872 To: sebas, #plasma, hein, davidedmundson Cc: davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp,

D5872: pidChanged also signals dataChanged in WindowModel

2017-05-15 Thread David Edmundson
davidedmundson requested changes to this revision. davidedmundson added a comment. This revision now requires changes to proceed. See d581 REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D5872 To: sebas, #plasma, hein, davidedmundson Cc: davidedmundson,

D5872: pidChanged also signals dataChanged in WindowModel

2017-05-15 Thread Sebastian Kügler
sebas created this revision. Restricted Application added projects: Plasma on Wayland, Frameworks. Restricted Application added subscribers: Frameworks, plasma-devel. REVISION SUMMARY Eike removed this, but I'm not sure why. All other roles trigger datachanged, so I don't see why pid