D8855: Use Kio::KPlacesModel as source model for PlacesItemModel

[Anthony Fieroni]

anthonyfieroni added reviewers: elvisangelaccio, emmanuelp.
anthonyfieroni added inline comments.

INLINE COMMENTS

> placesitem.cpp:159
> +    const QString urlScheme = url().scheme();
> +    return (urlScheme.contains(QStringLiteral("search")) || urlScheme.contains(QStringLiteral("timeline")));
>  }

scheme can be filenamesearch and it's not baloo.

> placesitemmodel.cpp:93
>  {
> -    const KBookmark bookmark = PlacesItem::createBookmark(m_bookmarkManager, text, url, iconName);
> -    return new PlacesItem(bookmark);
> +    m_sourceModel->addPlace(text, url, iconName, "", mapToSource(after));
>  }

For empty string do not use "", here use {}
In function declaration

  void setBla(const QString &name = {})

> placesitemmodel.cpp:159
> +
> +        if (bookmarkId(m_sourceModel->bookmarkForIndex(sIndex)) == bookmarkId(iBookmark)) {
>              break;

You should not call bookmarId(iBookmark) on every iteration

  const QString itemBookmarkId = bookmarkId(item->bookmark);

> placesitemmodel.h:56-59
>      /**
>       * @return A new instance of a places item with the given
>       *         attributes.
>       */

Docs does not match anymore to function

> placesitemmodel.h:149-152
> +    void onSourceModelRowsInserted(const QModelIndex &parent, int first, int last);
> +    void onSourceModelRowsAboutToBeRemoved(const QModelIndex &parent, int first, int last);
> +    void onSourceModelRowsMoved(const QModelIndex &parent, int start, int end, const QModelIndex &destination, int row);
> +    void onSourceModelDataChanged(const QModelIndex &topLeft, const QModelIndex &bottomRight, const QVector<int> &roles);

Q_DECL_OVERRIDE

REPOSITORY
  R318 Dolphin

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

To: renatoo, elvisangelaccio, emmanuelp
Cc: anthonyfieroni, nicolasfella, ngraham, #dolphin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20171121/e751c2eb/attachment.htm>