D5872: pidChanged also signals dataChanged in WindowModel

2017-05-15 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: seb

D5872: pidChanged also signals dataChanged in WindowModel

2017-05-15 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 ot

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.

D5867: Add or improve "Generated. Don't edit" messages and make consistent

2017-05-15 Thread Aleix Pol Gonzalez
apol accepted this revision. This revision is now accepted and ready to land. REPOSITORY R240 Extra CMake Modules BRANCH improvegeneratednote REVISION DETAIL https://phabricator.kde.org/D5867 To: kossebau, #frameworks, #build_system, apol

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 muc

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, el

D5877: Use absolute path instead of "." as path to watch

2017-05-15 Thread Elvis Angelaccio
elvisangelaccio edited the summary of this revision. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D5877 To: elvisangelaccio, dfaure Cc: #frameworks

D5877: Use absolute path instead of "." as path to watch

2017-05-15 Thread Elvis Angelaccio
elvisangelaccio edited the summary of this revision. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D5877 To: elvisangelaccio, dfaure Cc: #frameworks

D5877: Use absolute path instead of "." as path to watch

2017-05-15 Thread Elvis Angelaccio
elvisangelaccio edited the summary of this revision. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D5877 To: elvisangelaccio, dfaure Cc: #frameworks

D5877: Use absolute path instead of "." as path to watch

2017-05-15 Thread Elvis Angelaccio
elvisangelaccio created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY Either "." was explicitly added (in which case we use the absolute path of the cwd), or it's added as side effect of something else

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, se

D3883: Generate gperf output at build time

2017-05-15 Thread Pino Toscano
This revision was automatically updated to reflect the committed changes. Closed by commit R270:883e1ada57c4: Generate gperf output at build time (authored by pino). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D3883?vs=9532&id=14569#toc REPOSITORY R270 KCodecs CHANGES SINCE LAST UPD

D5871: Remove obviously wrongly-named symbolic links

2017-05-15 Thread Jonathan Marten
This revision was automatically updated to reflect the committed changes. Closed by commit R267:1b641f89d752: Remove obviously wrongly-named symbolic links (authored by marten). REPOSITORY R267 Oxygen Icons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D5871?vs=14560&id=14564 REVISI

D5871: Remove obviously wrongly-named symbolic links

2017-05-15 Thread David Edmundson
davidedmundson accepted this revision. This revision is now accepted and ready to land. REPOSITORY R267 Oxygen Icons REVISION DETAIL https://phabricator.kde.org/D5871 To: marten, #plasma, davidedmundson Cc: plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jen

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, plasma-devel,

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 shouldn't

D5871: Remove obviously wrongly-named symbolic links

2017-05-15 Thread Jonathan Marten
marten created this revision. Restricted Application added projects: Plasma, Frameworks. Restricted Application added subscribers: Frameworks, plasma-devel. REVISION SUMMARY It's not clear where these entries came from, but they are clearly a mistake - either scripting or copy-and-paste. The

D3830: Add a new FindGperf module

2017-05-15 Thread Pino Toscano
This revision was automatically updated to reflect the committed changes. Closed by commit R240:c61aee80d647: Add a new FindGperf module (authored by pino). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D3830?vs=9502&id=14562#toc REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST

D5866: ecm_qt_declare_logging_category(): more unique include guard for header

2017-05-15 Thread Friedrich W. H. Kossebau
kossebau added a comment. A different reasoning for why not yet using `#pragma once`: this macro targets users of projects with at least cmake and Qt. Unless Qt itself does not use that pragma, let's not risk to screw over people who try to reuse ECM for some non-mainstream setup, unless we

D5866: ecm_qt_declare_logging_category(): more unique include guard for header

2017-05-15 Thread Friedrich W. H. Kossebau
kossebau added a comment. Yes, `#pragma once` might be the nicer solution here. I stayed away from proposing it though, as for one it is not a real standard by specifications and also by KDE coding traditions. And I would not like to be the one adding (and thus being responsible) the g

D5867: Add or improve "Generated. Don't edit" messages and make consistent

2017-05-15 Thread Friedrich W. H. Kossebau
kossebau created this revision. Restricted Application added projects: Frameworks, Build System. REPOSITORY R240 Extra CMake Modules BRANCH improvegeneratednote REVISION DETAIL https://phabricator.kde.org/D5867 AFFECTED FILES modules/ECMQmLoader.cpp.in modules/ECMQtDeclareLoggingCateg

D5866: ecm_qt_declare_logging_category(): more unique include guard for header

2017-05-15 Thread Elvis Angelaccio
elvisangelaccio added a comment. What about `#pragma once`? Is there some real-world compiler that doesn't support it? REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D5866 To: kossebau, #frameworks, #build_system Cc: elvisangelaccio

D5865: Add missing KDE_ENABLE_NAMED_OPERATORS function

2017-05-15 Thread René J . V . Bertin
rjvbb marked an inline comment as done. rjvbb added a comment. > I'll hope to find some time to test on Window, wouldn't want this to be merged before that. Should we read "not to be merged before *you* have tested it"? I have nothing against that, esp. if you also find the exact value t

D5866: ecm_qt_declare_logging_category(): more unique include guard for header

2017-05-15 Thread Friedrich W. H. Kossebau
kossebau created this revision. Restricted Application added projects: Frameworks, Build System. REVISION SUMMARY The old guard was created just from the identifier + _H, which runs the chance to clash in projects which use an identifier matching the project name and which also have a class

D5865: Add missing KDE_ENABLE_NAMED_OPERATORS function

2017-05-15 Thread Kevin Funk
kfunk added a comment. LGTM in general. I'll hope to find some time to test on Window, wouldn't want this to be merged before that. INLINE COMMENTS > KDECompilerSettings.cmake:226 > +if (MSVC) > +if (${MSVC_VERSION} GREATER 1900) > +# /permissive- became the preferred

D5865: Add missing KDE_ENABLE_NAMED_OPERATORS function

2017-05-15 Thread René J . V . Bertin
rjvbb created this revision. Restricted Application added projects: Frameworks, Build System. REVISION SUMMARY A recent commit disabled support for named (logical) operators (and, not, bitand, &c) when using GCC, Clang or ICC. This patch adds a function to (re)enable them in a cross-platform

D5856: Use KDirWatch removeDir/addDir instead of stopDirScan/restartDirScan

2017-05-15 Thread David Faure
dfaure added a comment. In fact, CopyJob has no way to know whether a view somewhere is watching that directory. When an app other than Dolphin uses CopyJob, there isn't going to be any watching, possibly. So I think we need to remove the qDebug in KDirWatch so that removeDir is silent if

D5856: Use KDirWatch removeDir/addDir instead of stopDirScan/restartDirScan

2017-05-15 Thread David Faure
dfaure updated this revision to Diff 14545. dfaure added a comment. avoid doing removeDir twice REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D5856?vs=14515&id=14545 BRANCH master REVISION DETAIL https://phabricator.kde.org/D5856 AFFECTED FILES src/cor

D5775: Don't include the pid in the dbus path when on flatpak

2017-05-15 Thread Aleix Pol Gonzalez
apol updated this revision to Diff 14541. apol added a comment. Fix regression REPOSITORY R271 KDBusAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D5775?vs=14301&id=14541 BRANCH arcpatch-D5775 REVISION DETAIL https://phabricator.kde.org/D5775 AFFECTED FILES src/kdbu

D5851: Fix the PID updated in PlasmaWindowedModel

2017-05-15 Thread David Edmundson
davidedmundson abandoned this revision. davidedmundson added a comment. That would also be completely fine from my POV. I should have read the original review request, I was just fixing the tests based on what was currently here. REPOSITORY R127 KWayland REVISION DETAIL https://phab