D7700: Show list of tags in PlacesView

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> placesitemmodel.cpp:84
> +    m_tags(),
> +    m_dirLister()
>  {

Remove it, it's pointer type

> placesitemmodel.cpp:98
> +    if (KProtocolInfo::isKnownProtocol(QStringLiteral("tags"))) {
> +        connect(m_dirLister, &KCoreDirLister::itemsAdded, this, &PlacesItemModel::addTags);
> +        m_dirLister->openUrl(QUrl(m_tagsUrlBase));

Remove tag?

> placesitemmodel.cpp:694
>  
> +        if (item->groupType()==PlacesItem::TagsType){
> +            continue;

a == b

> placesitemmodel.cpp:1188
> +
> +void PlacesItemModel::addTags(const QUrl& directoryUrl, const KFileItemList& items)
> +{

Name should be addTag, because you add it one by one.

> placesitemmodel.cpp:1191
> +    Q_UNUSED(directoryUrl);
> +    int maxTags = 8;
> +    int numTags = 0;

const

> placesitemmodel.cpp:1192
> +    int maxTags = 8;
> +    int numTags = 0;
> +

Remove it, use m_tags.size instead.

> placesitemmodel.cpp:1194
> +
> +    for (KFileItem item: items) {
> +        QString name = item.name();

This is a copy, use const kFileItem &item instead

> placesitemmodel.cpp:1195
> +    for (KFileItem item: items) {
> +        QString name = item.name();
> +

const, make all unmodified variables const ones.

> placesitemmodel.cpp:1197-1202
> +        if (m_tags.isEmpty()) {
> +            PlacesItem* allTagsItem = createSystemPlacesItem(SystemBookmarkData(QUrl(m_tagsUrlBase),
> +                                                             QStringLiteral("tag"),
> +                                                             I18N_NOOP2("KFile System Bookmarks", "All tags")));
> +            appendItemToGroup(allTagsItem);
> +        }

Move out from loop, it's true only one time

> placesitemmodel.h:291
> +    QList<QString> m_tags;
> +    QString m_tagsUrlBase = QStringLiteral("tags:/");
> +    KCoreDirLister* m_dirLister;

Make it const and you can initialize it in constructor time.

REPOSITORY
  R318 Dolphin

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

To: nicolasfella, #dolphin, #kde_applications, alexeymin
Cc: anthonyfieroni, cfeck, elvisangelaccio, emmanuelp, ngraham, alexeymin, #dolphin, navarromorales, firef, andrebarros
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20170913/5ef609af/attachment.htm>