D14360: Remove custom icon selection for trash

2018-07-25 Thread Shubham
shubham added a comment. pino: I mean the very same m_iconButton is common to all other places entries like root,home etc, we are just creating it depending on whether it is trash or not REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14360 To: shubham, ngraham,

D14360: Remove custom icon selection for trash

2018-07-25 Thread Pino Toscano
pino added a comment. In D14360#298208 , @shubham wrote: > pino: I don't why m_iconButton isn't needed(because it is being used by other entries also ), am I right? I don't understandwhat you mean, can you please rephrase it? The

D14360: Remove custom icon selection for trash

2018-07-25 Thread Shubham
shubham added a comment. pino: I don't why m_iconButton isn't needed(because it is being used by other entries also ), am I right? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14360 To: shubham, ngraham, broulik, #dolphin, #frameworks Cc: pino, kde-frameworks-devel,

D14360: Remove custom icon selection for trash

2018-07-25 Thread Pino Toscano
pino added a comment. TBH creating a widget without adding it to a layout does not make much sense to me -- in the past I saw those situations as "floating" widgets usually anchored at the top-left corner of the parent widget. The best way here is to just not create `m_iconButton` at all,

D14360: Remove custom icon selection for trash

2018-07-25 Thread Shubham
shubham updated this revision to Diff 38468. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14360?vs=38452=38468 REVISION DETAIL https://phabricator.kde.org/D14360 AFFECTED FILES src/filewidgets/kfileplaceeditdialog.cpp To: shubham, ngraham, broulik,

KDE CI: Frameworks purpose kf5-qt5 SUSEQt5.10 - Build # 91 - Fixed!

2018-07-25 Thread CI System
BUILD SUCCESS Build URL https://build.kde.org/job/Frameworks%20purpose%20kf5-qt5%20SUSEQt5.10/91/ Project: Frameworks purpose kf5-qt5 SUSEQt5.10 Date of build: Thu, 26 Jul 2018 03:45:50 + Build duration: 9 min 10 sec and counting JUnit Tests

D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread David Faure
dfaure added a comment. In D14302#298099 , @thiago wrote: > In D14302#297515 , @dfaure wrote: > > > The idea of the old code was: if I can't get the lock, then someone else is already in the process

D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread Thiago Macieira
thiago added a comment. In D14302#297515 , @dfaure wrote: > The idea of the old code was: if I can't get the lock, then someone else is already in the process of starting kdeinit, so I'll just wait for that to happen, by locking again, i.e.

D10040: Add serial number and EISA ID to OutputDevice interface

2018-07-25 Thread David Edmundson
davidedmundson commandeered this revision. davidedmundson added a reviewer: dvratil. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D10040 To: davidedmundson, graesslin, sebas, #kwin, dvratil Cc: kde-frameworks-devel, davidedmundson, plasma-devel, ragreen, Pitel,

D14360: Remove custom icon selection for trash

2018-07-25 Thread Nathaniel Graham
ngraham requested changes to this revision. ngraham added a reviewer: Frameworks. ngraham added a comment. This revision now requires changes to proceed. It's important to test your changes before submitting. :) This now causes Gwenview to segfault when you try to edit its Trash entry. The

D14360: Remove custom icon selection for trash

2018-07-25 Thread Shubham
shubham updated this revision to Diff 38452. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14360?vs=38405=38452 REVISION DETAIL https://phabricator.kde.org/D14360 AFFECTED FILES src/filewidgets/kfileplaceeditdialog.cpp To: shubham, ngraham, broulik,

D14360: Remove custom icon selection for trash

2018-07-25 Thread Nathaniel Graham
ngraham added a reviewer: Dolphin. ngraham requested changes to this revision. ngraham added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kfileplaceeditdialog.cpp:123 > m_iconButton = new KIconButton(this); > +m_iconButton->setVisible(url.scheme() !=

D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread Jaime Torres Amate
This revision was automatically updated to reflect the committed changes. Closed by commit R271:8097e1aa0d21: Dont block forever in ensureKdeinitRunning (authored by jtamate). REPOSITORY R271 KDBusAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14302?vs=38403=38448 REVISION

D14360: Remove custom icon selection for trash

2018-07-25 Thread Nathaniel Graham
ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. This works from Gwenview (for example), but does not work from Dolphin, because sadly Dolphin has its own duplicate of this code. :( Could you submit another patch for Dolphin too? It's

D12820: Add KWayland virtual desktop protocol

2018-07-25 Thread Marco Martin
mart added a comment. RFC: add back some info for the layout? (in particular: rows and orientation, tought orientation is a bit concerning as is view-spacific) REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D12820 To: mart, #kwin, #plasma, graesslin, hein Cc:

D10343: Create containment on specified screen

2018-07-25 Thread Nathaniel Graham
ngraham added a reviewer: Plasma. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D10343 To: hoffmannrobert, #plasma Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13743: Migrate build system to use find_package in autotests/ki18n_install

2018-07-25 Thread Ralf Habacker
habacker added a comment. ping REPOSITORY R249 KI18n REVISION DETAIL https://phabricator.kde.org/D13743 To: habacker, apol, ilic Cc: bcooksley, ltoscano, kde-frameworks-devel, michaelh, ngraham, bruns

D10343: Create containment on specified screen

2018-07-25 Thread Robert Hoffmann
hoffmannrobert updated this revision to Diff 38410. hoffmannrobert edited the test plan for this revision. hoffmannrobert added a comment. Restricted Application edited subscribers, added: kde-frameworks-devel; removed: Frameworks. - Remove unnecessary Containment::setLastScreen() REPOSITORY

D14360: Remove custom icon selection for trash

2018-07-25 Thread Shubham
shubham updated this revision to Diff 38405. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14360?vs=38402=38405 REVISION DETAIL https://phabricator.kde.org/D14360 AFFECTED FILES src/filewidgets/kfileplaceeditdialog.cpp To: shubham, ngraham, broulik Cc:

D14360: Remove custom icon selection for trash

2018-07-25 Thread Kai Uwe Broulik
broulik added a comment. If the URL of the places item is of scheme `trash` (Scheme is the part before the colon, e.g. `file`, `http`, …), then it should hide it, yes. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14360 To: shubham, ngraham, broulik Cc:

D14360: Remove custom icon selection for trash

2018-07-25 Thread Shubham
shubham added a comment. broulik: you mean the visibility of button will depend on the type of KFilePlacesItem(if its trash , its hidden, else not) REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14360 To: shubham, ngraham, broulik Cc: kde-frameworks-devel, michaelh,

D14360: Remove custom icon selection for trash

2018-07-25 Thread Kai Uwe Broulik
broulik added a comment. Thanks a lot for your patch! I think it is cleaner if the button is setup but then hidden: m_iconButton->setVisible(url.toString() != QLatin1String("trash:/")); Curiously, Dolphin overwrites the icon whenever `url.protocol() == "trash` whereas

D13782: RFC: Ignore NTFS hidden flag for root volume

2018-07-25 Thread Wolfgang Bauer
wbauer added a comment. In D13782#297639 , @wbauer wrote: > Using QDir::canonicalPath() (instead of QDir::cleanPath() ) would fix all 3 cases: > > const QString fullFilePath = QDir(path + QLatin1Char('/') + filename).canonicalPath(); >

D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. That's reasonable. Still when you see this warning in the future, it would be useful to find out what kdeinit+klauncher are actually doing, if klauncher doesn't manage to register with

D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread Jaime Torres Amate
jtamate updated this revision to Diff 38403. jtamate edited the summary of this revision. jtamate added a comment. Don't wait forever, if the lock is blocked for more than 30 seconds, write a warning and don't try to start kdeinit5 again. REPOSITORY R271 KDBusAddons CHANGES SINCE LAST

D14360: Remove custom icon selection for trash

2018-07-25 Thread Shubham
shubham created this revision. shubham added reviewers: ngraham, broulik. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: kde-frameworks-devel. shubham requested review of this revision. REVISION SUMMARY BUG: 391200 TEST PLAN 1. Right click

D13782: RFC: Ignore NTFS hidden flag for root volume

2018-07-25 Thread Wolfgang Bauer
wbauer added a comment. In D13782#297547 , @wbauer wrote: > With additional debug output I do see the '.' entry in the root dir still marked as hidden (as mentioned in the test plan), and also the '..' entry in (first-level) subdirs is hidden.

D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. The comments look good. The actual change, not so much ;) INLINE COMMENTS > kdeinitinterface.cpp:72 > +QProcess proc; > +// started asynchronously, so even if kdeinit5

D13782: RFC: Ignore NTFS hidden flag for root volume

2018-07-25 Thread Wolfgang Bauer
wbauer added a subscriber: ngraham. wbauer added a comment. In D13782#296381 , @ngraham wrote: > @wbauer, does this work for you now? Yes, it does work as expected now here in all cases I tested, and I didn't notice problems. With

D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread Jaime Torres Amate
jtamate updated this revision to Diff 38387. jtamate edited the summary of this revision. jtamate added a comment. > I said from the start that it wasn't tryLock() that was blocking but lock(), good to see that this is now confirmed. I trusted blindly in gdb stacktrace, and also I didn't

D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread David Faure
dfaure added a comment. The idea of the old code was: if I can't get the lock, then someone else is already in the process of starting kdeinit, so I'll just wait for that to happen, by locking again, i.e. blocking on purpose, and then checking that the DBus name is up, i.e. the other

D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread Jaime Torres Amate
jtamate updated this revision to Diff 38372. jtamate edited the summary of this revision. jtamate edited the test plan for this revision. jtamate added a comment. It is my first time of a backtrace that doesn't reflect the true state of a program. But I've learned the lesson, in case of