D8862: Extend KFilePlacesModel API

2017-11-20 Thread Milian Wolff 
mwolff requested changes to this revision.

REPOSITORY
  R241 KIO

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

To: renatoo, dfaure, mwolff
Cc: mwolff, dfaure, ngraham, #frameworks

D8332: Added baloo urls into places model

2017-11-20 Thread Milian Wolff 
mwolff accepted this revision.
mwolff added a comment.


  lgtm from my side

REPOSITORY
  R241 KIO

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

To: renatoo, #frameworks, #dolphin, #kde_applications, dvratil, #vdg, ngraham, 
ervin, mlaurent, dfaure, mwolff
Cc: mwolff, dfaure, ervin, usta, mlaurent, dvratil, ngraham, #frameworks

D8791: Avoid inconsistent passworddialog

2017-11-20 Thread David Faure 
dfaure added a comment.


  The whole point of KDE Frameworks is that you don't need a "full up-to-date 
KDE development system".
  
  You can just install distro packages for Qt5 devel, and ECM, and then compile 
and test kwidgetaddons from git without even having to install it, using a 
unittest and/or a test program.
  I worked on making this possible, so if something doesn't work, let me know 
;-)
  
  (depending on your distro, you might have to lower the required version of 
ECM in CMakeLists.txt, but that is usually fine)

REPOSITORY
  R236 KWidgetsAddons

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

To: cryptodude, dfaure, cfeck
Cc: ngraham, #frameworks

D8791: Avoid inconsistent passworddialog

2017-11-20 Thread Kees vd Broek 
cryptodude added a comment.


  The changes are pretty simple, but testing it would require a lot of work as 
it requires a full up-to-date KDE development system and I just don't have that 
option.
  
  So I can't give you a screenshot, as the "test plan" indicates, I visually 
can't test it.

REPOSITORY
  R236 KWidgetsAddons

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

To: cryptodude, dfaure, cfeck
Cc: ngraham, #frameworks

D8870: Synchronize the component with the one in Kirigami

2017-11-20 Thread Marco Martin 
mart accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  master

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

To: apol, #plasma, mart
Cc: broulik, plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D8862: Extend KFilePlacesModel API

2017-11-20 Thread David Faure 
dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kfileplacesmodel.cpp:281
>  KFilePlacesItem *item = static_cast *>(index.internalPointer());
> -
> -if (!item->isDevice()) {
> -return item->bookmark();
> -} else {
> -return KBookmark();
> -}
> + return item->bookmark();
>  }

Please double-check indentation, looks like 5 spaces.

> kfileplacesmodel.cpp:786
> +
> +QString onlyInApp =  bookmark.metaDataItem(QStringLiteral("OnlyInApp"));
> +if (onlyInApp != appName) {

remove one of the two spaces after '='

Add const in front, while at it.

> kfileplacesmodel.cpp:787
> +QString onlyInApp =  bookmark.metaDataItem(QStringLiteral("OnlyInApp"));
> +if (onlyInApp != appName) {
> +bookmark.setMetaDataItem(QStringLiteral("OnlyInApp"), appName);

Please swap these two arguments around. In all preceding if()s, you have new != 
old, while here it's old != new, makes reading more difficult.

> kfileplacesmodel.h:133
>int row, int column, const QModelIndex ) 
> Q_DECL_OVERRIDE;
> +void refresh() const;
>  

Missing method documentation, including `@since 5.41`.

REPOSITORY
  R241 KIO

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

To: renatoo, dfaure
Cc: dfaure, ngraham, #frameworks

D8908: KDateComboBox: fix dateChanged() not emitted after typing a date

2017-11-20 Thread Anthony Fieroni 
anthonyfieroni added a comment.


  +1

REPOSITORY
  R236 KWidgetsAddons

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

To: dfaure, cfeck, dvratil
Cc: anthonyfieroni, #frameworks, #kde_pim

D8871: Don't look for /etc/kderc every single time

2017-11-20 Thread David Faure 
dfaure accepted this revision.
dfaure added a comment.


  Patch looks good. Commit log looks wrong, it's not /etc/kderc but /etc/kde5rc.

REPOSITORY
  R237 KConfig

BRANCH
  master

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

To: apol, #frameworks, mpyne, dfaure
Cc: dfaure, mpyne

D8908: KDateComboBox: fix dateChanged() not emitted after typing a date

2017-11-20 Thread David Faure 
dfaure updated this revision to Diff 22624.
dfaure added a comment.


  use enterDate; don't emit changed if nothing changed

REPOSITORY
  R236 KWidgetsAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8908?vs=22621=22624

BRANCH
  master

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

AFFECTED FILES
  autotests/kdatecomboboxtest.cpp
  autotests/kdatecomboboxtest.h
  src/kdatecombobox.cpp
  src/kdatecombobox.h

To: dfaure, cfeck, dvratil
Cc: anthonyfieroni, #frameworks, #kde_pim

D8908: KDateComboBox: fix dateChanged() not emitted after typing a date

2017-11-20 Thread David Faure 
dfaure added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in kdatecombobox.cpp:350-351
> Why not
> 
>   d->enterDate(date());
> 
> ?

Good point, `enterDate()` enables the warning-on-invalid when this option is 
set, and I suppose we want that on Key_Return as well.
However I still need to emit `dateChanged()`, since `setDate()` doesn't do that 
in this case (it thinks the date hasn't changed, given that we're calling 
`setDate(date())`).

REPOSITORY
  R236 KWidgetsAddons

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

To: dfaure, cfeck, dvratil
Cc: anthonyfieroni, #frameworks, #kde_pim

<    1   2