D8862: Extend KFilePlacesModel API

[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<KFilePlacesItem *>(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 &parent) 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20171120/77c86888/attachment.html>