D21959: Fix selectedNameFilter() multiple matches

2019-06-27 Thread David Faure 
dfaure closed this revision.

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

To: hoffmannrobert, #frameworks, apol, dfaure
Cc: michaelweghorn, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

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

2019-06-27 Thread David Faure 
dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> global.cpp:251
> +KIO::UDSEntry entry = job->statResult();
> +const KFileItem item = KFileItem(entry, url);
> +i = item.iconName();

I hate nested event loops (job->exec()). And I wonder if you need one.
KFileItem is able to stat local files all by itself, in process. So, wouldn't 
this work?

  const KFileItem item(url, mt.name());
  i = item.iconName();

> global.cpp:255
> +
> +// It's non-local and maybe on a slow filesystem
> +} else {

It's a bit confusing to me to have this comment before the "else" statement, 
rather than inside the "else {" block...

REPOSITORY
  R241 KIO

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

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

D19585: [CopyJob] Report results after finishing copy

2019-06-27 Thread David Faure 
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  OK I see what you mean, after looking more at slotReport().

REPOSITORY
  R241 KIO

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

To: broulik, dfaure, chinmoyr
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21882: RFC: [CopyJob] Batch reporting processed amount

2019-06-27 Thread David Faure 
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  Excellent point.

REPOSITORY
  R241 KIO

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

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

D17071: Don't include any directory sizes in DirectorySizeJob

2019-06-27 Thread seb sasch 
sasch added a comment.


  @davidedmundson any reference or version when you plan to make this "live"?

REPOSITORY
  R241 KIO

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

To: davidedmundson, dfaure, apol
Cc: bruns, sasch, adridg, apol, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham

D21402: Avoid double overwrite check

2019-06-27 Thread Christoph Cullmann 
cullmann added a comment.


  Hmm, doesn't this only rely on the behavior of QFileDialog::getSaveFileUrl?

REPOSITORY
  R40 Kate

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

To: meven, #kate, #frameworks, cullmann
Cc: kossebau, apol, dhaumann, kwrite-devel, domson, michaelh, ngraham, 
demsking, cullmann, sars

D21760: Add KListOpenFiles::listProcessesWithOpenFiles

2019-06-27 Thread David Hallas 
hallas added a comment.


  Ping :D

REPOSITORY
  R244 KCoreAddons

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

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

D10747: Implement zwp_linux_dmabuf_v1

2019-06-27 Thread Roman Gilg 
romangg added a dependent revision: D10750: wayland: Add support for 
zwp_linux_dmabuf.

REPOSITORY
  R127 KWayland

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

To: romangg, #kwin, #plasma, graesslin, davidedmundson, mart, fredrik
Cc: kde-frameworks-devel, romangg, plasma-devel, LeGast00n, hannahk, jraleigh, 
sgureev, fbampaloukas, GB_2, ragreen, Pitel, schernikov, michaelh, ZrenBot, 
ngraham, bruns, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, eliasp, sebas, apol, mart, hein

D22108: [TabBar] Remove exterior margins

2019-06-27 Thread Noah Davis 
ndavis accepted this revision.
ndavis added a comment.
This revision is now accepted and ready to land.


  In D22108#487329 , @filipf wrote:
  
  > In D22108#487218 , @ndavis wrote:
  >
  > > 1+ visually. How does it look with a vertical panel?
  >
  >
  > Still not gliding on the line, but that should be resolved in Kickoff 
somehow:
  >
  > F6938487: image.png 
  
  
  That's a shame, but the patch doesn't introduce any visual regressions 
compared to git master, so I'll still accept this.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  remove-tabbar-margins (branched from master)

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

To: filipf, #vdg, #plasma, ngraham, ndavis
Cc: ndavis, apol, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D22108: [TabBar] Remove exterior margins

2019-06-27 Thread Filip Fila 
filipf added a comment.


  In D22108#487218 , @ndavis wrote:
  
  > 1+ visually. How does it look with a vertical panel?
  
  
  Still not gliding on the line, but that should be resolved in Kickoff somehow:
  
  F6938487: image.png 

REPOSITORY
  R242 Plasma Framework (Library)

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

To: filipf, #vdg, #plasma, ngraham
Cc: ndavis, apol, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D22083: introduce concept of header and footer for kpageview

2019-06-27 Thread Hugo Pereira Da Costa 
hpereiradacosta added a comment.


  On the breeze side, asside from the layout margins/spacing issues, there is 
at least one hard-coded pixel in the frame rendering that must be removed. In 
fact I have this committed in my local breeze version already. The 
corresponding patch is rather simple and attached. Feel free to add this or 
something similar to any other modification you plan to do. F6938398: 
patch.diff 

REPOSITORY
  R236 KWidgetsAddons

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

To: mart, #plasma, #frameworks, #vdg
Cc: hpereiradacosta, cfeck, ndavis, ngraham, kde-frameworks-devel, LeGast00n, 
michaelh, bruns

D21901: Calendar events: allow plugins to show event details

2019-06-27 Thread Daniel Vrátil 
dvratil added a comment.


  So, we are doomed to `CalendarEventsPluginV2`?  *sadpanda*

REPOSITORY
  R296 KDeclarative

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

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

D21901: Calendar events: allow plugins to show event details

2019-06-27 Thread Kai Uwe Broulik 
broulik added a comment.


  Adding a new `virtual` to a non-leaf class is never ABI compatible.

REPOSITORY
  R296 KDeclarative

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

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

D21901: Calendar events: allow plugins to show event details

2019-06-27 Thread Daniel Vrátil 
dvratil added a reviewer: mart.

REPOSITORY
  R296 KDeclarative

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

To: dvratil, #frameworks, mart
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21721: [WIP] Bring KNewStuffQuick to feature parity with KNewStuff(Widgets)

2019-06-27 Thread Dan Leinir Turthra Jensen 
leinir updated this revision to Diff 60729.
leinir added a comment.


  - Work on the comments delegate's look some

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21721?vs=60689=60729

BRANCH
  knsquick-feature-parity-with-kns (branched from master)

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

AFFECTED FILES
  src/attica/atticaprovider.cpp
  src/attica/atticaprovider_p.h
  src/core/CMakeLists.txt
  src/core/commentsmodel.cpp
  src/core/commentsmodel.h
  src/core/engine.cpp
  src/core/engine.h
  src/core/entryinternal.cpp
  src/core/installation.cpp
  src/core/itemsmodel.cpp
  src/core/itemsmodel.h
  src/core/provider.h
  src/core/question.h
  src/qtquick/CMakeLists.txt
  src/qtquick/categoriesmodel.cpp
  src/qtquick/categoriesmodel.h
  src/qtquick/qml/ConditionalLoader.qml
  src/qtquick/qml/EntryCommentDelegate.qml
  src/qtquick/qml/EntryCommentsPage.qml
  src/qtquick/qml/EntryScreenshots.qml
  src/qtquick/qml/GridTileDelegate.qml
  src/qtquick/qml/NewStuffButton.qml
  src/qtquick/qml/NewStuffDialog.qml
  src/qtquick/qml/NewStuffDialogContent.qml
  src/qtquick/qml/NewStuffDownloadItemsSheet.qml
  src/qtquick/qml/NewStuffEntryDetails.qml
  src/qtquick/qml/NewStuffPage.qml
  src/qtquick/qml/NewStuffQuestionAsker.qml
  src/qtquick/qml/Rating.qml
  src/qtquick/qml/Shadow.qml
  src/qtquick/qmldir
  src/qtquick/qmlplugin.cpp
  src/qtquick/quickengine.cpp
  src/qtquick/quickengine.h
  src/qtquick/quickitemsmodel.cpp
  src/qtquick/quickitemsmodel.h
  src/qtquick/quickquestionlistener.cpp
  src/qtquick/quickquestionlistener.h
  tests/CMakeLists.txt
  tests/khotnewstuff-dialog-ui/main.qml
  tests/khotnewstuff-dialog.cpp

To: leinir
Cc: anthonyfieroni, pino, ngraham, kde-frameworks-devel, LeGast00n, michaelh, 
bruns

D21402: Avoid double overwrite check

2019-06-27 Thread Friedrich W. H. Kossebau 
kossebau added a comment.


  As wm4 just pointed out on irc, this might need fallback code for older 
versions of KF, no? Currently Kate has KF5_DEP_VERSION 5.40, so you might 
either want to bump that min dep to match the version of ktexteditor which does 
the check itself, or add some (runtime/buildtime) check for the KF version.

REPOSITORY
  R40 Kate

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

To: meven, #kate, #frameworks, cullmann
Cc: kossebau, apol, dhaumann, kwrite-devel, domson, michaelh, ngraham, 
demsking, cullmann, sars

D22108: [TabBar] Remove exterior margins

2019-06-27 Thread Noah Davis 
ndavis added a comment.


  1+ visually. How does it look with a vertical panel?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: filipf, #vdg, #plasma, ngraham
Cc: ndavis, apol, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D22108: [TabBar] Remove exterior margins

2019-06-27 Thread Filip Fila 
filipf added a comment.


  In D22108#487140 , @apol wrote:
  
  > Makes sense to me, but why are the computer icons different? ^^'
  
  
  Some bug in Kickoff or with the icon, sometimes (maybe after you change a 
desktop theme) it shows the laptop icon, and then after a while the monitor 
icon. Should have made another screenshot but oh well :P

REPOSITORY
  R242 Plasma Framework (Library)

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

To: filipf, #vdg, #plasma, ngraham
Cc: apol, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D22108: [TabBar] Remove exterior margins

2019-06-27 Thread Aleix Pol Gonzalez 
apol added a comment.


  Makes sense to me, but why are the computer icons different? ^^'

REPOSITORY
  R242 Plasma Framework (Library)

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

To: filipf, #vdg, #plasma, ngraham
Cc: apol, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21882: RFC: [CopyJob] Batch reporting processed amount

2019-06-27 Thread Kai Uwe Broulik 
broulik added a comment.


  Well, that would just be another symptom of the "doesn't call `slotReport` 
when finishing the call, no? cf. D19585 

REPOSITORY
  R241 KIO

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

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

D22105: WIP : Fix SFTP Plugin of KIO for Windows

2019-06-27 Thread Hannah von Reth 
vonreth added reviewers: sitter, dfaure.

REPOSITORY
  R320 KIO Extras

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

To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure
Cc: andriusr, kde-frameworks-devel, kfm-devel, fprice, LeGast00n, fbampaloukas, 
alexde, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov

D22107: Add MediaTransport API

2019-06-27 Thread Manuel Weichselbaumer 
mweichselbaumer added inline comments.

INLINE COMMENTS

> ltoscano wrote in a2dp-codecs.h:33
> Will it work with older versions of bluez?

Bluez just defines this constant, but does not use it internally (nor we do).
So, yes, this works with any version of bluez.

REPOSITORY
  R269 BluezQt

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

To: mweichselbaumer, drosca
Cc: ltoscano, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D22107: Add MediaTransport API

2019-06-27 Thread Luigi Toscano 
ltoscano added inline comments.

INLINE COMMENTS

> mweichselbaumer wrote in a2dp-codecs.h:33
> Yes, this has also been fixed by bluez as of 2018-12-28.

Will it work with older versions of bluez?

REPOSITORY
  R269 BluezQt

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

To: mweichselbaumer, drosca
Cc: ltoscano, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D22107: Add MediaTransport API

2019-06-27 Thread Manuel Weichselbaumer 
mweichselbaumer marked an inline comment as done.
mweichselbaumer added inline comments.

INLINE COMMENTS

> drosca wrote in a2dp-codecs.h:33
> Are you sure about this?

Yes, this has also been fixed by bluez as of 2018-12-28.

> drosca wrote in tpendingcall.h:45
> Is this really needed? In the end, it doesn't really make the code that much 
> better:
> 
>   TPendingCall *fd = 
> transport->tryAcquire();
>   fd->valueAt<0>();
>   fd->valueAt<1>();
>   fd->valueAt<2>();
> 
> vs
> 
>   PendingCall *fd = transport->tryAcquire();
>   fd->values().at(0).value();
>   fd->values().at(1).value();
>   fd->values().at(2).value();
> 
> Or we can add convenience method  `T valueAt(int)` so it becomes:
> 
>   PendingCall *fd = transport->tryAcquire();
>   fd->valueAt(0);
>   fd->valueAt(1);
>   fd->valueAt(2);

Actually, my intention was to provide a type-safe way to obtain return values 
from PendingCall and to express explicitly, what the corresponding method 
returns in API header.
As a client you can make use of "auto".

Consider:

  auto *fd = transport->tryAcquire(); // Method declaration will define return 
type.
  auto ret1 = fd->valueAt<0>();
  auto ret2 = fd->valueAt<1>();
  auto ret3 = fd->valueAt<2>();

Thus, return type (of PendingCall) is expressed in API header and clients do 
not need to express the return types themselves.

REPOSITORY
  R269 BluezQt

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

To: mweichselbaumer, drosca
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D22082: WIP Add an ioslave to access KActivityStat data

2019-06-27 Thread Méven Car 
meven updated this revision to Diff 60708.
meven added a comment.


  - Add a logging category kf5.kactivity.stat

REPOSITORY
  R159 KActivities Statistics

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22082?vs=60676=60708

BRANCH
  add-logging

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

AFFECTED FILES
  src/CMakeLists.txt
  src/ioslave/CMakeLists.txt
  src/ioslave/recentdocumentsactivities.cpp
  src/ioslave/recentdocumentsactivities.h
  src/ioslave/recentdocumentsactivities.protocol
  src/resultmodel.cpp
  src/resultset.cpp
  src/resultwatcher.cpp

To: meven, #frameworks, ngraham, ivan
Cc: ivan, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns