D13048: Move redundant logic in KIO::iconNameForUrl() into KFileItem::iconName()

2018-05-22 Thread Nathaniel Graham
ngraham planned changes to this revision.
ngraham added a comment.


  Hmm, that makes sense. OK, I'll re-work this.
  
  The problem here is that if we want to fix 356045, there's no getting around 
reading a `.directory` file as part of this process, but maybe I can narrowly 
conditionalize it so the performance penalty is acceptable.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, broulik
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns


D13048: Move redundant logic in KIO::iconNameForUrl() into KFileItem::iconName()

2018-05-22 Thread Aleix Pol Gonzalez
apol added inline comments.

INLINE COMMENTS

> global.cpp:332
> +KIO::StatJob *job = KIO::stat(url);
> +job->exec();
> +KIO::UDSEntry entry = job->statResult();

having a blocking stat isn't good. stat will take a while on some protocols and 
it's not even all that quick on local file systems. I guess that's why we had 
this code twice.

> kfileitem.cpp:951
> +
> +if (d->m_iconName == QStringLiteral("inode-directory")) {
> +d->m_iconName = QStringLiteral("folder");

Why's this needed if `inode-directory` is the same as `folder`?

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, broulik
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns


D13048: Move redundant logic in KIO::iconNameForUrl() into KFileItem::iconName()

2018-05-22 Thread Nathaniel Graham
ngraham edited the test plan for this revision.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, broulik
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D13048: Move redundant logic in KIO::iconNameForUrl() into KFileItem::iconName()

2018-05-22 Thread Nathaniel Graham
ngraham created this revision.
ngraham added reviewers: Frameworks, broulik.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  `KIO::iconNameForUrl()` was doing a lot of redundant work on file items that 
`KFileItem::iconName()` already does for us. This patchg moves most of that 
logic into `KFileItem::iconName()`, which also results in fixing 
https://bugs.kde.org/show_bug.cgi?id=356045.
  
  BUG: 356045
  FIXED-IN: 5.47

TEST PLAN
  Functionality testing:
  
  - Tested that adding a directory with a custom icon to the Places panel 
resuts in the Places panel entry inheriting its custom icon:
  - Used the system with this patch in place for 2 days; did not find any 
instances of incorrect icons
  
  Regression testing:
  
  - Tested that the trash changes its icon appropriately when adding an item to 
an empty trash, removing all items from the trash, and emptying the trash 
(testtrash test passes)
  - Tested that other icons on the Places panel look the same as they did 
before the patch
  
  Unit testing:
  
  - No test regressions (had to fix one test to expect `folder` instead of 
`inode-directory`, which is more correct anyway (and there's no visual 
difference between them because `inode-directory` is a symlink to `folder`)

REPOSITORY
  R241 KIO

BRANCH
  iconNameForUrl (branched from master)

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

AFFECTED FILES
  src/core/global.cpp
  src/core/kfileitem.cpp
  src/ioslaves/trash/tests/testtrash.cpp

To: ngraham, #frameworks, broulik
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D12992: New elisa icon

2018-05-22 Thread Diego Gangl
januz added a comment.


  In D12992#266673 , @mgallien wrote:
  
  > In D12992#266288 , @alex-l wrote:
  >
  > > F5863162: image.png 
  > >
  > > ^ I'm for this one without the quaver
  >
  >
  > I also prefer this one without the quaver.
  >  Is it possible to really simplify the background for the small size icon ? 
It would be probably much easier to read it.
  >
  > @astippich  and @januz what is your opinion ? You are also working on Elisa 
and should have the rights to speak ;)
  
  
  I agree on this one without the quaver, maybe it could also have a bit of a 
circle at the end of the (like the circles in the quaver). That said, we just 
changed icons a month or 2 ago. Are we sure about completely dropping the 
casette concept and changing again?

REPOSITORY
  R266 Breeze Icons

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

To: lshoravi, #vdg, ngraham, #elisa
Cc: januz, astippich, mgallien, alex-l, andreask, kde-frameworks-devel, 
ssteffen, lcmscheid, nhuisman, ursjoss, mykolak, jussiv, michaelh, ognarb, 
ngraham, bruns, kmf


D12896: Add Telegram plugin

2018-05-22 Thread Aleix Pol Gonzalez
apol added a comment.


  In D12896#266351 , @nicolasfella 
wrote:
  
  > I've played around with flatpaked Telegram and made it working. However:
  >  My Telegram.desktop file executes this:
  >  /usr/bin/flatpak run --branch=stable --arch=x86_64 
--command=telegram-desktop --file-forwarding org.telegram.desktop -- @@u %u @@
  >  I can share a file using
  >  /usr/bin/flatpak run --branch=stable --arch=x86_64 
--command=telegram-desktop --file-forwarding org.telegram.desktop -sendpath @@ 
/home/nico/test.py @@
  >  But that would require changing the command I get from the desktop file. I 
can achieve the same goal just by appending something to the command from the 
desktop file:
  >  /usr/bin/flatpak run --branch=stable --arch=x86_64 
--command=telegram-desktop --file-forwarding org.telegram.desktop -- @@u %u @@ 
-sendpath @@ /home/nico/test.py @@
  >  However, appending this to the command does not work with the regular 
command.
  >  I don't see any way to support all without some special-case handling for 
flatpak. I didn't even test snap.
  >  Any ideas?
  
  
  Then just read the Exec field and we'll add the sendpath by hand. This is 
ad-hoc for telegram anyway.

REPOSITORY
  R495 Purpose Library

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

To: nicolasfella, apol
Cc: ngraham, kde-frameworks-devel, apol, michaelh, bruns


D10305: Fix "Moving the mouse over a different application icon immediately changes the window list"

2018-05-22 Thread Nathaniel Graham
ngraham added a comment.


  @lshoravi are you willing and able to address the above-mentioned issues, or 
should someone else comandeer the revision and do it themselves?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: lshoravi, #plasma, #plasma_accessibility, mart, aacid
Cc: davidedmundson, kde-frameworks-devel, aacid, cfeck, ngraham, plasma-devel, 
ragreen, Pitel, michaelh, ZrenBot, bruns, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart


D13018: Fix unit test for kfileplacesmodeltest

2018-05-22 Thread Stefan Brüns
bruns added a comment.


  Shouldn't this be handled by:
  http://doc.qt.io/qt-5/qstandardpaths.html#setTestModeEnabled

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

To: renatoo, ngraham
Cc: bruns, ngraham, maximilianocuria, elvisangelaccio, kde-frameworks-devel, 
michaelh


D11236: [KCrash] Establish socket to allow change of ptrace scope

2018-05-22 Thread Christoph Roick
croick updated this revision to Diff 34673.
croick added a comment.


  - remove superfluous line...

REPOSITORY
  R285 KCrash

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11236?vs=34672=34673

BRANCH
  ptracer

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

AFFECTED FILES
  src/kcrash.cpp

To: croick, #frameworks
Cc: kde-frameworks-devel, sitter, michaelh, ngraham, bruns


D11236: [KCrash] Establish socket to allow change of ptrace scope

2018-05-22 Thread Christoph Roick
croick updated this revision to Diff 34672.
croick added a comment.
Restricted Application added a subscriber: kde-frameworks-devel.


  - use `QDir::tempPath()`

REPOSITORY
  R285 KCrash

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11236?vs=31244=34672

BRANCH
  ptracer

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

AFFECTED FILES
  src/kcrash.cpp

To: croick, #frameworks
Cc: kde-frameworks-devel, sitter, michaelh, ngraham, bruns


D11235: [DrKonqi] Request change of ptrace scope from KCrash

2018-05-22 Thread Christoph Roick
croick updated this revision to Diff 34671.
croick marked an inline comment as done.
croick added a comment.


  - check size of socket path

REPOSITORY
  R871 DrKonqi

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11235?vs=34670=34671

BRANCH
  ptracer

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

AFFECTED FILES
  src/CMakeLists.txt
  src/debuggerlaunchers.cpp
  src/debuggerlaunchers.h
  src/queryptrace.cpp
  src/queryptrace.h
  src/tests/crashtest/crashtest.cpp

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


D12156: implement reading of rating tag

2018-05-22 Thread Matthieu Gallien
mgallien added a subscriber: cgilles.
mgallien added a comment.


  @cgilles Are you somehow using KFileMetaData (seems correct if one looks at 
the Debian package dependencies) ?
  This diff is about adding a way to read ratings from "tags" native to a 
specific file format.
  
  This would be in addition to the current ratings that are independent from 
the file types.
  
  Can you comment on this patch with a digikam point of view ?
  
  I am wondering if we should not find a way to make this more transparent for 
users of the API. This is why I am asking here.

REPOSITORY
  R286 KFileMetaData

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

To: astippich, mgallien, michaelh
Cc: cgilles, kde-frameworks-devel, #baloo, bruns, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham


D12992: New elisa icon

2018-05-22 Thread Matthieu Gallien
mgallien added subscribers: astippich, januz.
mgallien added a comment.


  In D12992#266288 , @alex-l wrote:
  
  > F5863162: image.png 
  >
  > ^ I'm for this one without the quaver
  
  
  I also prefer this one without the quaver.
  Is it possible to really simplify the background for the small size icon ? It 
would be probably much easier to read it.
  
  @astippich  and @januz what is your opinion ? You are also working on Elisa 
and should have the rights to speak ;)
  
  > F5862923: image.png 
  > 
  > ^ or this one with white symbol

REPOSITORY
  R266 Breeze Icons

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

To: lshoravi, #vdg, ngraham, #elisa
Cc: januz, astippich, mgallien, alex-l, andreask, kde-frameworks-devel, 
ssteffen, lcmscheid, nhuisman, ursjoss, mykolak, jussiv, michaelh, ognarb, 
ngraham, bruns, kmf


D11235: [DrKonqi] Request change of ptrace scope from KCrash

2018-05-22 Thread Christoph Roick
croick marked 6 inline comments as done.
croick added a comment.


  Thank you for your comments, totally forgot about licensing.

INLINE COMMENTS

> maximilianocuria wrote in queryptrace.cpp:26
> Please honour TMPDIR or, even better, use QTemporaryDir (this will also make 
> the generated path unpredictable), and create the socket inside.

QTemporaryDir will not work, since the location is set in KCrash.

REPOSITORY
  R871 DrKonqi

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

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


D11235: [DrKonqi] Request change of ptrace scope from KCrash

2018-05-22 Thread Christoph Roick
croick updated this revision to Diff 34670.
croick added a comment.


  - add license text
  - use `QDir::tempPath()`
  - ifdefs to definition

REPOSITORY
  R871 DrKonqi

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11235?vs=30979=34670

BRANCH
  ptracer

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

AFFECTED FILES
  src/CMakeLists.txt
  src/debuggerlaunchers.cpp
  src/debuggerlaunchers.h
  src/queryptrace.cpp
  src/queryptrace.h
  src/tests/crashtest/crashtest.cpp

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


D13018: Fix unit test for kfileplacesmodeltest

2018-05-22 Thread Nathaniel Graham
ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Much better, thanks for indulging me. :)
  
  Now onto `kfileplacesviewtest`...

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

To: renatoo, ngraham
Cc: ngraham, maximilianocuria, elvisangelaccio, kde-frameworks-devel, michaelh, 
bruns


D13018: Fix unit test for kfileplacesmodeltest

2018-05-22 Thread Renato Oliveira Filho
renatoo added a comment.


  In D13018#266626 , @ngraham wrote:
  
  > I know you didn't change that, but it might be a nice enhancement (hint 
hint). :)
  
  
  
  
  In D13018#266626 , @ngraham wrote:
  
  > I know you didn't change that, but it might be a nice enhancement (hint 
hint). :)
  
  
  as you wish ;)

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

To: renatoo, ngraham
Cc: ngraham, maximilianocuria, elvisangelaccio, kde-frameworks-devel, michaelh, 
bruns


D13018: Fix unit test for kfileplacesmodeltest

2018-05-22 Thread Renato Oliveira Filho
renatoo updated this revision to Diff 34666.
renatoo added a comment.


  Run tests in an empty home dir

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13018?vs=34646=34666

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

AFFECTED FILES
  autotests/kfileplacesmodeltest.cpp

To: renatoo, ngraham
Cc: ngraham, maximilianocuria, elvisangelaccio, kde-frameworks-devel, michaelh, 
bruns


D13018: Fix unit test for kfileplacesmodeltest

2018-05-22 Thread Nathaniel Graham
ngraham added a comment.


  I know you didn't change that, but it might be a nice enhancement (hint 
hint). :)

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

To: renatoo, ngraham
Cc: ngraham, maximilianocuria, elvisangelaccio, kde-frameworks-devel, michaelh, 
bruns


D13018: Fix unit test for kfileplacesmodeltest

2018-05-22 Thread Renato Oliveira Filho
renatoo added a comment.


  In D13018#266623 , @ngraham wrote:
  
  > Shouldn't that be a part of the test itself? I don't think it's reasonable 
to require that testers and testing platforms manually run that every time.
  
  
  I am not sure, this is how this test works, I never changed that.  Maybe ask 
the maintainers guys.

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

To: renatoo, ngraham
Cc: ngraham, maximilianocuria, elvisangelaccio, kde-frameworks-devel, michaelh, 
bruns


D13018: Fix unit test for kfileplacesmodeltest

2018-05-22 Thread Nathaniel Graham
ngraham added a comment.


  Shouldn't that be a part of the test itself? I don't think it's reasonable to 
require that testers and testing platforms manually run that every time.

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

To: renatoo, ngraham
Cc: ngraham, maximilianocuria, elvisangelaccio, kde-frameworks-devel, michaelh, 
bruns


D13018: Fix unit test for kfileplacesmodeltest

2018-05-22 Thread Renato Oliveira Filho
renatoo added a comment.


  as maximilianocuria said, before run the tests you need to create a temporary 
home path:
  
  HOMETMP=$(mktemp -d)
  trap 'rm -rf $HOMETMP' EXIT
  export HOME="$HOMETMP"

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

To: renatoo, ngraham
Cc: ngraham, maximilianocuria, elvisangelaccio, kde-frameworks-devel, michaelh, 
bruns


D13018: Fix unit test for kfileplacesmodeltest

2018-05-22 Thread Nathaniel Graham
ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  `kiofilewidgets-kfileplacesmodeltest` still fails for me with this patch:
  
grep 'FAIL!  : KFilePlacesModelTest' -A 3 Testing/Temporary/LastTest.log 
FAIL!  : KFilePlacesModelTest::testInitialState() Compared values are not 
the same
   Actual   (m_places->rowCount()): 6
   Expected (4)   : 4
   Loc: [/home/dev/repos/kio/autotests/kfileplacesmodeltest.cpp(200)]
--
FAIL!  : KFilePlacesModelTest::testInitialList() Compared lists have 
different sizes.
   Actual   (placesUrls()) size: 6
   Expected (urls) size: 9
   Loc: [/home/dev/repos/kio/autotests/kfileplacesmodeltest.cpp(236)]
--
FAIL!  : KFilePlacesModelTest::testReparse() Compared lists have different 
sizes.
   Actual   (placesUrls()) size: 7
   Expected (urls) size: 10
   Loc: [/home/dev/repos/kio/autotests/kfileplacesmodeltest.cpp(252)]
--
FAIL!  : KFilePlacesModelTest::testMove() Compared lists have different 
sizes.
   Actual   (placesUrls()) size: 7
   Expected (urls) size: 9
   Loc: [/home/dev/repos/kio/autotests/kfileplacesmodeltest.cpp(396)]
--
FAIL!  : KFilePlacesModelTest::testPlacesLifecycle() Compared lists have 
different sizes.
   Actual   (placesUrls()) size: 8
   Expected (urls) size: 10
   Loc: [/home/dev/repos/kio/autotests/kfileplacesmodeltest.cpp(552)]
--
FAIL!  : KFilePlacesModelTest::testDevicePlugging() Compared lists differ 
at index 1.
   Actual   (placesUrls()): "/home/dev/Downloads"
   Expected (urls): "/"
   Loc: [/home/dev/repos/kio/autotests/kfileplacesmodeltest.cpp(665)]
--
FAIL!  : KFilePlacesModelTest::testDragAndDrop() Compared lists have 
different sizes.
   Actual   (placesUrls()) size: 8
   Expected (urls) size: 9
   Loc: [/home/dev/repos/kio/autotests/kfileplacesmodeltest.cpp(472)]
FAIL!  : KFilePlacesModelTest::testDeviceSetupTeardown() Compared values 
are not the same
   Actual   (spy_changed.count()): 0
   Expected (1)  : 1
   Loc: [/home/dev/repos/kio/autotests/kfileplacesmodeltest.cpp(786)]
--
FAIL!  : KFilePlacesModelTest::testRemoteUrls(Ftp) Compared lists have 
different sizes.
   Actual   (placesUrls()) size: 9
   Expected (urls) size: 10
   Loc: [/home/dev/repos/kio/autotests/kfileplacesmodeltest.cpp(853)]
--
FAIL!  : KFilePlacesModelTest::testRemoteUrls(Samba) Compared lists differ 
at index 2.
   Actual   (placesUrls()): "/home/dev/Downloads"
   Expected (urls): "trash:/"
   Loc: [/home/dev/repos/kio/autotests/kfileplacesmodeltest.cpp(853)]
--
FAIL!  : KFilePlacesModelTest::testRemoteUrls(Sftp) Compared lists have 
different sizes.
   Actual   (placesUrls()) size: 11
   Expected (urls) size: 10
   Loc: [/home/dev/repos/kio/autotests/kfileplacesmodeltest.cpp(853)]
--
FAIL!  : KFilePlacesModelTest::testRemoteUrls(Fish) Compared lists have 
different sizes.
   Actual   (placesUrls()) size: 12
   Expected (urls) size: 10
   Loc: [/home/dev/repos/kio/autotests/kfileplacesmodeltest.cpp(853)]
--
FAIL!  : KFilePlacesModelTest::testRemoteUrls(Webdav) Compared lists have 
different sizes.
   Actual   (placesUrls()) size: 13
   Expected (urls) size: 10
   Loc: [/home/dev/repos/kio/autotests/kfileplacesmodeltest.cpp(853)]
--
FAIL!  : KFilePlacesModelTest::testIconRole(Places - Trash) 
'index.data(KFilePlacesModel::IconNameRole).toString().startsWith(expectedIconName)'
 returned FALSE. ()
   Loc: [/home/dev/repos/kio/autotests/kfileplacesmodeltest.cpp(999)]
FAIL!  : KFilePlacesModelTest::testIconRole(Remote - Network) 
'index.data(KFilePlacesModel::IconNameRole).toString().startsWith(expectedIconName)'
 returned FALSE. ()
   Loc: [/home/dev/repos/kio/autotests/kfileplacesmodeltest.cpp(999)]
FAIL!  : KFilePlacesModelTest::testIconRole(Devices - Nfs) 
'index.data(KFilePlacesModel::IconNameRole).toString().startsWith(expectedIconName)'
 returned FALSE. ()
   Loc: [/home/dev/repos/kio/autotests/kfileplacesmodeltest.cpp(999)]
FAIL!  : KFilePlacesModelTest::testIconRole(Devices - foreign) 
'index.data(KFilePlacesModel::IconNameRole).toString().startsWith(expectedIconName)'
 returned FALSE. ()
   Loc: [/home/dev/repos/kio/autotests/kfileplacesmodeltest.cpp(999)]
FAIL!  : KFilePlacesModelTest::testIconRole(Devices - Floppy) 
'index.data(KFilePlacesModel::IconNameRole).toString().startsWith(expectedIconName)'
 returned FALSE. ()
   Loc: [/home/dev/repos/kio/autotests/kfileplacesmodeltest.cpp(999)]
FAIL!  : KFilePlacesModelTest::testIconRole(Devices - cdrom) 
'index.data(KFilePlacesModel::IconNameRole).toString().startsWith(expectedIconName)'
 returned FALSE. ()
   Loc: [/home/dev/repos/kio/autotests/kfileplacesmodeltest.cpp(999)]
FAIL!  : 

D12745: Unify API for file descriptor sharing

2018-05-22 Thread Chinmoy Ranjan Pradhan
chinmoyr added a comment.
Restricted Application edited subscribers, added: kde-frameworks-devel; 
removed: Frameworks.


  ping

REPOSITORY
  R241 KIO

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

To: chinmoyr, dfaure, ossi
Cc: kde-frameworks-devel, michaelh, ngraham, bruns, #frameworks


D10410: Move the task of cleaning up of socket file to file ioslave and FdReceiver

2018-05-22 Thread Chinmoy Ranjan Pradhan
chinmoyr added a comment.
Restricted Application added a subscriber: kde-frameworks-devel.


  ping

REPOSITORY
  R241 KIO

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

To: chinmoyr, #frameworks, ossi, dfaure
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D13043: Properly sanitize input

2018-05-22 Thread Aleix Pol Gonzalez
apol created this revision.
apol added a reviewer: Frameworks.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
apol requested review of this revision.

REVISION SUMMARY
  Helps to use it together with ModelTest

REPOSITORY
  R275 KItemModels

BRANCH
  master

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

AFFECTED FILES
  src/kconcatenaterowsproxymodel.cpp

To: apol, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


KDE CI: Frameworks kwayland kf5-qt5 FreeBSDQt5.10 - Build # 8 - Still Unstable!

2018-05-22 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kwayland%20kf5-qt5%20FreeBSDQt5.10/8/
 Project:
Frameworks kwayland kf5-qt5 FreeBSDQt5.10
 Date of build:
Tue, 22 May 2018 15:28:04 +
 Build duration:
11 min and counting
   JUnit Tests
  Name: (root) Failed: 15 test(s), Passed: 24 test(s), Skipped: 0 test(s), Total: 39 test(s)Failed: TestSuite.kwayland-testCompositorFailed: TestSuite.kwayland-testDataDeviceFailed: TestSuite.kwayland-testDataSourceFailed: TestSuite.kwayland-testErrorFailed: TestSuite.kwayland-testPlasmaShellFailed: TestSuite.kwayland-testRegionFailed: TestSuite.kwayland-testRemoteAccessFailed: TestSuite.kwayland-testShmPoolFailed: TestSuite.kwayland-testSubCompositorFailed: TestSuite.kwayland-testSubSurfaceFailed: TestSuite.kwayland-testWaylandConnectionThreadFailed: TestSuite.kwayland-testWaylandRegistryFailed: TestSuite.kwayland-testWaylandServerDisplayFailed: TestSuite.kwayland-testWaylandShellFailed: TestSuite.kwayland-testWaylandSurface

KDE CI: Frameworks kwayland kf5-qt5 SUSEQt5.10 - Build # 46 - Still Unstable!

2018-05-22 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kwayland%20kf5-qt5%20SUSEQt5.10/46/
 Project:
Frameworks kwayland kf5-qt5 SUSEQt5.10
 Date of build:
Tue, 22 May 2018 15:28:04 +
 Build duration:
9 min 15 sec and counting
   JUnit Tests
  Name: (root) Failed: 3 test(s), Passed: 41 test(s), Skipped: 0 test(s), Total: 44 test(s)Failed: TestSuite.kwayland-testErrorFailed: TestSuite.kwayland-testPlasmaShellFailed: TestSuite.kwayland-testRemoteAccess
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report57%
(4/7)87%
(219/251)87%
(219/251)81%
(23337/28705)50%
(11402/22722)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests.client93%
(38/41)93%
(38/41)94%
(10814/11497)48%
(7418/15593)autotests.server100%
(5/5)100%
(5/5)99%
(359/362)49%
(220/446)src.client94%
(67/71)94%
(67/71)82%
(5523/6721)63%
(1732/2752)src.server95%
(109/115)95%
(109/115)83%
(6641/7957)62%
(2032/3252)src.tools0%
(0/2)0%
(0/2)0%
(0/786)0%
(0/346)src.tools.testserver0%
(0/3)0%
(0/3)0%
(0/120)0%
(0/16)tests0%
(0/14)0%
(0/14)0%
(0/1262)0%
(0/317)

D11925: Add "SkipSwitcher" to API

2018-05-22 Thread Scott Harvey
This revision was automatically updated to reflect the committed changes.
Closed by commit R127:10b00a219e9f: Add SkipSwitcher to API 
(authored by sharvey).

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11925?vs=34636=34654

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

AFFECTED FILES
  autotests/client/test_plasma_window_model.cpp
  autotests/client/test_plasmashell.cpp
  autotests/client/test_wayland_windowmanagement.cpp
  src/client/plasmashell.cpp
  src/client/plasmashell.h
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/plasmawindowmodel.cpp
  src/client/plasmawindowmodel.h
  src/client/protocols/plasma-shell.xml
  src/client/protocols/plasma-window-management.xml
  src/client/registry.cpp
  src/server/plasmashell_interface.cpp
  src/server/plasmashell_interface.h
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h
  tests/plasmasurfacetest.cpp

To: sharvey, hein, graesslin
Cc: kde-frameworks-devel, lbeltrame, rikmills, davidedmundson, #plasma, 
graesslin, michaelh, ngraham, bruns


D13037: [Dialog] Set PlasmaShellSurface on PlasmaShell being announced

2018-05-22 Thread Martin Flöser
graesslin added a comment.


  I wouldn't change plasmaquick based on KWin's behavior. KWin is weird after 
all. We should rather work around inside KWin.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: romangg, #plasma
Cc: graesslin, davidedmundson, kde-frameworks-devel, michaelh, ngraham, bruns


D11925: Add "SkipSwitcher" to API

2018-05-22 Thread Martin Flöser
graesslin accepted this revision.
graesslin added a comment.
This revision is now accepted and ready to land.


  wonderful! Thanks to you two :-) And yes as soon as you push this change you 
can push the kwin change.

REPOSITORY
  R127 KWayland

BRANCH
  arcpatch-D11925

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

To: sharvey, hein, graesslin
Cc: kde-frameworks-devel, lbeltrame, rikmills, davidedmundson, #plasma, 
graesslin, michaelh, ngraham, bruns


KDE CI: Frameworks kwidgetsaddons kf5-qt5 SUSEQt5.9 - Build # 24 - Unstable!

2018-05-22 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kwidgetsaddons%20kf5-qt5%20SUSEQt5.9/24/
 Project:
Frameworks kwidgetsaddons kf5-qt5 SUSEQt5.9
 Date of build:
Tue, 22 May 2018 14:17:03 +
 Build duration:
2 min 32 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 18 test(s), Skipped: 0 test(s), Total: 19 test(s)Failed: TestSuite.ksplittercollapserbuttontest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report67%
(2/3)45%
(85/187)45%
(85/187)30%
(4536/15085)23%
(1915/8358)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(33/33)100%
(33/33)99%
(1663/1681)52%
(977/1878)src44%
(52/117)44%
(52/117)23%
(2873/12276)15%
(938/6303)tests0%
(0/37)0%
(0/37)0%
(0/1128)0%
(0/177)

KDE CI: Frameworks kwidgetsaddons kf5-qt5 FreeBSDQt5.10 - Build # 9 - Still Unstable!

2018-05-22 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kwidgetsaddons%20kf5-qt5%20FreeBSDQt5.10/9/
 Project:
Frameworks kwidgetsaddons kf5-qt5 FreeBSDQt5.10
 Date of build:
Tue, 22 May 2018 14:17:04 +
 Build duration:
2 min 25 sec and counting
   JUnit Tests
  Name: (root) Failed: 18 test(s), Passed: 0 test(s), Skipped: 0 test(s), Total: 18 test(s)Failed: TestSuite.kacceleratormanagertestFailed: TestSuite.kcollapsiblegroupbox_testFailed: TestSuite.kcolorbuttontestFailed: TestSuite.kdatecomboboxtestFailed: TestSuite.kdatetimeedittestFailed: TestSuite.kdualactiontestFailed: TestSuite.kmessagewidgetautotestFailed: TestSuite.knewpasswordwidgettestFailed: TestSuite.kpagedialogautotestFailed: TestSuite.kpassworddialogautotestFailed: TestSuite.kpasswordlineedittestFailed: TestSuite.kpixmapsequencewidgettestFailed: TestSuite.kselectaction_unittestFailed: TestSuite.ksplittercollapserbuttontestFailed: TestSuite.ksqueezedtextlabelautotestFailed: TestSuite.ktimecomboboxtestFailed: TestSuite.ktooltipwidgettestFailed: TestSuite.kwidgetsaddons-kcolumnresizertest

D12858: [KCharSelect] Fix table cell size

2018-05-22 Thread Christoph Feck
This revision was automatically updated to reflect the committed changes.
Closed by commit R236:43d23824a94c: [KCharSelect] Fix table cell size with Qt 
5.11 (authored by cfeck).

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12858?vs=34516=34649

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

AFFECTED FILES
  src/kcharselect.cpp

To: cfeck, #frameworks, rkflx
Cc: rkflx, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns


KDE CI: Frameworks kwidgetsaddons kf5-qt5 FreeBSDQt5.10 - Build # 8 - Unstable!

2018-05-22 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kwidgetsaddons%20kf5-qt5%20FreeBSDQt5.10/8/
 Project:
Frameworks kwidgetsaddons kf5-qt5 FreeBSDQt5.10
 Date of build:
Tue, 22 May 2018 13:50:03 +
 Build duration:
2 min 55 sec and counting
   JUnit Tests
  Name: (root) Failed: 18 test(s), Passed: 0 test(s), Skipped: 0 test(s), Total: 18 test(s)Failed: TestSuite.kacceleratormanagertestFailed: TestSuite.kcollapsiblegroupbox_testFailed: TestSuite.kcolorbuttontestFailed: TestSuite.kdatecomboboxtestFailed: TestSuite.kdatetimeedittestFailed: TestSuite.kdualactiontestFailed: TestSuite.kmessagewidgetautotestFailed: TestSuite.knewpasswordwidgettestFailed: TestSuite.kpagedialogautotestFailed: TestSuite.kpassworddialogautotestFailed: TestSuite.kpasswordlineedittestFailed: TestSuite.kpixmapsequencewidgettestFailed: TestSuite.kselectaction_unittestFailed: TestSuite.ksplittercollapserbuttontestFailed: TestSuite.ksqueezedtextlabelautotestFailed: TestSuite.ktimecomboboxtestFailed: TestSuite.ktooltipwidgettestFailed: TestSuite.kwidgetsaddons-kcolumnresizertest

D13034: Add mechanism to notify other clients of config changes over DBus

2018-05-22 Thread Vlad Zagorodniy
zzag added inline comments.

INLINE COMMENTS

> kconfig.cpp:526
> +QDBusConnection::sessionBus().send(message);
> +#endif
> +}

#else
  Q_UNUSED(changes);
  Q_UNUSED(path);
  #endif

> kconfigwatcher.cpp:82
> +KConfigGroup group = d->m_config->group(QString());//top level group
> +for(const QString : it.key().split(QLatin1Char('\x1d'))) { 
> //magic char, see KConfig
> +group = group.group(groupName);

Won't this detach?

REPOSITORY
  R237 KConfig

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

To: davidedmundson
Cc: zzag, kde-frameworks-devel, michaelh, ngraham, bruns


D13018: Fix unit test for kfileplacesmodeltest

2018-05-22 Thread Renato Oliveira Filho
renatoo added a comment.


  ok it should work now.

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

To: renatoo
Cc: maximilianocuria, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, 
bruns


D13018: Fix unit test for kfileplacesmodeltest

2018-05-22 Thread Renato Oliveira Filho
renatoo updated this revision to Diff 34646.
renatoo added a comment.


  Fixed code style

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13018?vs=34645=34646

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

AFFECTED FILES
  autotests/kfileplacesmodeltest.cpp

To: renatoo
Cc: maximilianocuria, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, 
bruns


D13018: Fix unit test for kfileplacesmodeltest

2018-05-22 Thread Renato Oliveira Filho
renatoo updated this revision to Diff 34645.
renatoo added a comment.


  Fixed all tests

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13018?vs=34644=34645

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

AFFECTED FILES
  autotests/kfileplacesmodeltest.cpp

To: renatoo
Cc: maximilianocuria, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, 
bruns


D13018: Fix unit test for kfileplacesmodeltest

2018-05-22 Thread Renato Oliveira Filho
renatoo updated this revision to Diff 34644.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13018?vs=34581=34644

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

AFFECTED FILES
  autotests/kfileplacesmodeltest.cpp

To: renatoo
Cc: maximilianocuria, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, 
bruns


D12992: New elisa icon

2018-05-22 Thread Nathaniel Graham
ngraham added a subscriber: mgallien.
ngraham added a comment.


  Let's make sure that @mgallien has a say as well. It's for his app, after 
all! :)

REPOSITORY
  R266 Breeze Icons

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

To: lshoravi, #vdg, ngraham, #elisa
Cc: mgallien, alex-l, andreask, kde-frameworks-devel, ssteffen, lcmscheid, 
nhuisman, ursjoss, mykolak, jussiv, michaelh, ognarb, januz, ngraham, bruns, kmf


D13037: [Dialog] Set PlasmaShellSurface on PlasmaShell being announced

2018-05-22 Thread Roman Gilg
romangg added a comment.


  In D13037#266379 , @davidedmundson 
wrote:
  
  > should mean the server will announce all the interfaces and we block all 
execution until we've processed it all.
  
  
  DialogShadows is created as Q_GLOBAL_STATIC. Maybe that's why it's not 
blocking for the Dialog object initializing it.
  
  Also wouldn't it be a deadlock if execution of KWin is stopped until the 
Wayland server, i.e. KWin, has processed all events? Or is the Tabbox QML code 
running in a different thread?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: romangg, #plasma
Cc: davidedmundson, kde-frameworks-devel, michaelh, ngraham, bruns


D11925: Add "SkipSwitcher" to API

2018-05-22 Thread Scott Harvey
sharvey added a comment.


  In D11925#265446 , @graesslin 
wrote:
  
  > any update on this? It's just the version increase in registry.cpp which is 
missing...
  
  
  Done. I was unclear as to what needed to change. I got a little outside help 
from @hein and `registry.cpp` is update to match the XML file.
  
  Once this is approved, is it safe to land along with D11296 
?

REPOSITORY
  R127 KWayland

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

To: sharvey, hein, graesslin
Cc: kde-frameworks-devel, lbeltrame, rikmills, davidedmundson, #plasma, 
graesslin, michaelh, ngraham, bruns


D13037: [Dialog] Set PlasmaShellSurface on PlasmaShell being announced

2018-05-22 Thread David Edmundson
davidedmundson added a comment.


  This doesn't make sense (assuming everything else works correctly, which 
clearly isn't the case)
  
registry->setup();
connection->roundtrip();
  
  should mean the server will announce all the interfaces and we block all 
execution until we've processed it all.
  
  which means we "know" by the time we finish 
DialogShadows::Private::setupWaylandIntegration() we know plasmaShellAnnounced 
will have been emitted and that the plasmashell has been set. That's how it 
works in the regular case.
  
  roundTrip goes via the QPA, and that has been implemented in Kwin's QPA.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: romangg, #plasma
Cc: davidedmundson, kde-frameworks-devel, michaelh, ngraham, bruns


D11925: Add "SkipSwitcher" to API

2018-05-22 Thread Scott Harvey
sharvey updated this revision to Diff 34636.
sharvey added a comment.


  - Increate interface versions for PlasmaShell and PlasmaWindowManagement

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11925?vs=32996=34636

BRANCH
  arcpatch-D11925

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

AFFECTED FILES
  autotests/client/test_plasma_window_model.cpp
  autotests/client/test_plasmashell.cpp
  autotests/client/test_wayland_windowmanagement.cpp
  src/client/plasmashell.cpp
  src/client/plasmashell.h
  src/client/plasmawindowmanagement.cpp
  src/client/plasmawindowmanagement.h
  src/client/plasmawindowmodel.cpp
  src/client/plasmawindowmodel.h
  src/client/protocols/plasma-shell.xml
  src/client/protocols/plasma-window-management.xml
  src/client/registry.cpp
  src/server/plasmashell_interface.cpp
  src/server/plasmashell_interface.h
  src/server/plasmawindowmanagement_interface.cpp
  src/server/plasmawindowmanagement_interface.h
  tests/plasmasurfacetest.cpp

To: sharvey, hein, graesslin
Cc: kde-frameworks-devel, lbeltrame, rikmills, davidedmundson, #plasma, 
graesslin, michaelh, ngraham, bruns


D13037: [Dialog] Set PlasmaShellSurface on PlasmaShell being announced

2018-05-22 Thread Roman Gilg
romangg added a task: T8771: PlasmaQuick::Dialog and/or KWin (internal window) 
problems.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: romangg, #plasma
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


KDE CI: Frameworks kcompletion kf5-qt5 FreeBSDQt5.10 - Build # 6 - Unstable!

2018-05-22 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kcompletion%20kf5-qt5%20FreeBSDQt5.10/6/
 Project:
Frameworks kcompletion kf5-qt5 FreeBSDQt5.10
 Date of build:
Tue, 22 May 2018 11:49:13 +
 Build duration:
1 min 27 sec and counting
   JUnit Tests
  Name: (root) Failed: 3 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 4 test(s)Failed: TestSuite.kcombobox_unittestFailed: TestSuite.kcompletioncoretestFailed: TestSuite.klineedit_unittest

D13037: [Dialog] Set PlasmaShellSurface on PlasmaShell being announced

2018-05-22 Thread Roman Gilg
romangg created this revision.
romangg added a reviewer: Plasma.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
romangg requested review of this revision.

REVISION SUMMARY
  Dialog might try to set PlasmaShellSurface before Wayland Server has sent
  the PlasmaShell proxy to DialogShadows. This can be observed with KWin's
  TabBox.
  
  Connect to DialogShadows new signal waylandPlasmaShellInterfaceAnnounced
  in case the PlasmaShell interface has not yet been announced.

TEST PLAN
  Manual debug statement shows that DialogPrivate::shellSurface is set.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  waitOnSetupWayland

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

AFFECTED FILES
  src/plasmaquick/dialog.cpp
  src/plasmaquick/dialogshadows.cpp
  src/plasmaquick/dialogshadows_p.h

To: romangg, #plasma
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D12896: Add Telegram plugin

2018-05-22 Thread Nicolas Fella
nicolasfella added a comment.


  I've played around with flatpaked Telegram and made it working. However:
  My Telegram.desktop file executes this:
  /usr/bin/flatpak run --branch=stable --arch=x86_64 --command=telegram-desktop 
--file-forwarding org.telegram.desktop -- @@u %u @@
  I can share a file using
  /usr/bin/flatpak run --branch=stable --arch=x86_64 --command=telegram-desktop 
--file-forwarding org.telegram.desktop -sendpath @@ /home/nico/test.py @@
  But that would require changing the command I get from the desktop file. I 
can achieve the same goal just by appending something to the command from the 
desktop file:
  /usr/bin/flatpak run --branch=stable --arch=x86_64 --command=telegram-desktop 
--file-forwarding org.telegram.desktop -- @@u %u @@ -sendpath @@ 
/home/nico/test.py @@
  However, appending this to the command does not work with the regular command.
  I don't see any way to support all without some special-case handling for 
flatpak. I didn't even test snap.
  Any ideas?

REPOSITORY
  R495 Purpose Library

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

To: nicolasfella, apol
Cc: ngraham, kde-frameworks-devel, apol, michaelh, bruns


D11235: [DrKonqi] Request change of ptrace scope from KCrash

2018-05-22 Thread Maximiliano Curia
maximilianocuria added inline comments.

INLINE COMMENTS

> queryptrace.cpp:26
> +server.sun_family = AF_UNIX;
> +sprintf(server.sun_path, "/tmp/kcrash_%lld", 
> QCoreApplication::applicationPid());
> +if (::connect(sfd, (struct sockaddr *), sl) == 0) {

Please honour TMPDIR or, even better, use QTemporaryDir (this will also make 
the generated path unpredictable), and create the socket inside.

REPOSITORY
  R871 DrKonqi

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

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


D11235: [DrKonqi] Request change of ptrace scope from KCrash

2018-05-22 Thread Adriaan de Groot
adridg added a comment.


  Comments directed at licensing and non-Linux cases.

INLINE COMMENTS

> queryptrace.cpp:1
> +#include 
> +

Missing file header / license bits

> queryptrace.cpp:49
> +
> +#endif

For non-Linuxen, include a null implementation here.

> queryptrace.h:1
> +#ifndef QUERYPTRACE_H
> +#define QUERYPTRACE_H

Missing a file header / license bits

> queryptrace.h:5
> +#ifdef Q_OS_LINUX
> +void queryPtrace(long long pid);
> +#else

You used qint64 elsewhere

> queryptrace.h:7
> +#else
> +#define queryPtrace(x)
> +#endif

In general, and for type safety, I'd avoid using #ifdefs to define an API. 
There should probably be **only** the one declaration of queryPtrace.

REPOSITORY
  R871 DrKonqi

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

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


D13034: Add mechanism to notify other clients of config changes over DBus

2018-05-22 Thread David Edmundson
davidedmundson created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
davidedmundson requested review of this revision.

REVISION SUMMARY
  Intention is not to create a registry like system, but to replace
  KDElibs4Support::KGlobalSettings and to replace other system settingss
  -> some app communication in a more generic way.
  
  writeEntry gains an additional flag Notify which if set, will notify
  clients of what has actually changed when we sync.
  
  Rationale to put this into KConfig was so that we could have everything
  batched and sychronised to the file sync and to get the fine detailed
  exposure of what has actually changed which we don't get with a file
  watcher.
  
  Default behaviour remains identical without any broadcast messages.
  
  As it is a new dependency it is purely optional and anything referencing
  DBus is not in the public API. Our deployment on platforms without DBus
  tend to be standalone applications anyway.

TEST PLAN
  Attached unit test

REPOSITORY
  R237 KConfig

BRANCH
  master

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

AFFECTED FILES
  CMakeLists.txt
  autotests/kconfigtest.cpp
  autotests/kconfigtest.h
  src/core/CMakeLists.txt
  src/core/kconfig.cpp
  src/core/kconfig_p.h
  src/core/kconfigbase.h
  src/core/kconfigdata.cpp
  src/core/kconfigdata.h
  src/core/kconfigwatcher.cpp
  src/core/kconfigwatcher.h
  src/gui/CMakeLists.txt

To: davidedmundson
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D13033: Expose getter method for KConfig::addConfigSources

2018-05-22 Thread David Edmundson
davidedmundson created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
davidedmundson requested review of this revision.

TEST PLAN
  Used in subsequent patch

REPOSITORY
  R237 KConfig

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

AFFECTED FILES
  src/core/kconfig.cpp
  src/core/kconfig.h

To: davidedmundson
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D13032: Fix cleanup of kconfigtest kdeglobals

2018-05-22 Thread David Edmundson
davidedmundson created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
davidedmundson requested review of this revision.

REVISION SUMMARY
  We expand the path of kdeglobals to
  ~/.qttest/kconfigtests/../kdeglobals
  
  which works during the test, but this cleanup is run both on startup and
  after removing ~/.qttest/kconfigtestsat which point the expansion
  fails.

TEST PLAN
  Ran test, looked at qttest dir. It had cleaned up

REPOSITORY
  R237 KConfig

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

AFFECTED FILES
  autotests/kconfigtest.cpp

To: davidedmundson
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D12992: New elisa icon

2018-05-22 Thread Alessandro Longo
alex-l added a comment.


  F5863162: image.png 
  
  ^ I'm for this one without the quaver
  
  F5862923: image.png 
  
  ^ or this one with white symbol

REPOSITORY
  R266 Breeze Icons

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

To: lshoravi, #vdg, ngraham, #elisa
Cc: alex-l, andreask, kde-frameworks-devel, ssteffen, lcmscheid, nhuisman, 
ursjoss, mykolak, jussiv, michaelh, ognarb, januz, ngraham, bruns, kmf, mgallien


D13018: Fix unit test for kfileplacesmodeltest

2018-05-22 Thread Maximiliano Curia
maximilianocuria added a comment.


  In D13018#266137 , 
@elvisangelaccio wrote:
  
  > Hmm, still seems to fail here:
  >
  >   $ ctest -R kfileplacesmodeltest
  >   Test project /home/elvis/dev/kde/kio/local-build
  >   Start 50: kiofilewidgets-kfileplacesmodeltest
  >   1/1 Test #50: kiofilewidgets-kfileplacesmodeltest ...***Failed0.60 sec
  >   * Start testing of KFilePlacesModelTest *
  >   Config: Using QtTest library 5.11.0, Qt 5.11.0 (x86_64-little_endian-lp64 
shared (dynamic) release build; by GCC 8.1.0)
  >   QDEBUG : KFilePlacesModelTest::initTestCase() void 
Solid::Backends::Fake::FakeManager::parseMachineFile() Parsing fake computer 
XML:  "/home/elvis/dev/kde/kio/autotests/fakecomputer.xml" 
  >  
  >   PASS   : KFilePlacesModelTest::initTestCase()
  >   FAIL!  : KFilePlacesModelTest::testInitialState() Compared values are not 
the same
  >  Actual   (m_places->rowCount()): 6
  >  Expected (4)   : 4
  >  Loc: [../autotests/kfileplacesmodeltest.cpp(200)]
  >   QDEBUG : KFilePlacesModelTest::testInitialList() Expected: 
("/home/elvis", "/", "trash:/", "remote:/", "/media/nfs", "/foreign", 
"/media/floppy0", "/media/XO-Y4", "/media/cdrom")
  >   QDEBUG : KFilePlacesModelTest::testInitialList() Got: ("/home/elvis", 
"/home/elvis/Desktop", "/home/elvis/Downloads", "/", "trash:/", "remote:/")
  >   FAIL!  : KFilePlacesModelTest::testInitialList() Compared lists have 
different sizes.
  >  Actual   (placesUrls()) size: 6
  >  Expected (urls) size: 9
  >  Loc: [../autotests/kfileplacesmodeltest.cpp(236)]
  >
  
  
  That's a different test (from the same file). That part of the test assumes 
that you have a clean home directory. I run this tests with a wrapper, 
something like:
  
  HOMETMP=$(mktemp -d)
  trap 'rm -rf $HOMETMP' EXIT
  export HOME="$HOMETMP"
  
  xvfb-run -a dbus-run-session -- obj-x86_64-linux-gnu/bin/kfileplacesmodeltest
  
  The complete version can be seen here:
   https://salsa.debian.org/qt-kde-team/kde/kio/tree/master/debian/tests
  
  That's after applying the patches:
   https://salsa.debian.org/qt-kde-team/kde/kio/tree/master/debian/patches
  
  The Debian test results can be seen here:
   https://ci.debian.net/data/autopkgtest/unstable/amd64/k/kio/342727/log.gz
  
  There is an error in a different part of this test, it seems that kio strips 
the -full part of the trash icon name but not always. (see 
src/filewidgets/kfileplacesitem.cpp : 418)

REPOSITORY
  R241 KIO

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

To: renatoo
Cc: maximilianocuria, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, 
bruns


Re: QStandardPaths::GenericDataLocation on MacOS

2018-05-22 Thread René J . V . Bertin
Hi,

I just remembered that some time ago I wrote a small library that allows me to 
use binaries compiled against my own patched qt5-kde with an "official" Qt 
build: it provides the additional symbols that are in qt5-kde but not standard 
Qt through code injection (DYLD_INSERT_LIBRARIES). That isn't really a kosher 
thing to do (and actively discouraged on newer Mac OS versions) but it brings 
me to this:

Could we design a kind of Qt plugin that provides an alternative QSP, a bit 
like how the plasma-integration plugin provides the KDE file dialog? In first 
testing/internal approach that could use a similar trick as the one I'm using 
with my QExtStandardPaths (a macro definition given in CPPFLAGS) and if that 
replaces every QStandardPaths::foo() call with (say) a 
KStandardLocations::foo() call we could just as well use an additional link 
library rather than a plugin.

But ultimately I'm thinking of a plugin that provides alternative methods in 
QStandardPaths which user code could decide to use (or not) and which would by 
default just proxy their standard counterparts. Such a system would provide the 
same kind of flexibility as a solution where qt.conf is used, but in a way that 
is probably easier to deploy on other platforms, less easy to tinker with by 
mere mortal endusers and that probably requires touching less files. It might 
thus be easier to upstream.

A long time ago David Faure pointed out to me that an application-wide solution 
that toggles QSP behaviour throughout an application is best avoided as it has 
the potential of doing that toggle at any time when an application is running. 
Instead, every dynamically loadable component should have its own, fixed QSP 
behaviour. (This is of course relevant mostly if not only for "linuxy" 
centralised installs.)