D29602: Add field "Folder" to theme content items and to message tool tip

[Daniel Vrátil]

dvratil added inline comments.

INLINE COMMENTS

> storagemodel.cpp:262
> +        collection = collection.parentCollection();
> +    }
> +    mi->setFolder(folder);

I would trim the trailing '/' from the folder path here, so that it looks like "Local Folders/Inbox" rather than "Local Folders/Inbox/"

> gordin wrote in storagemodel.cpp:263
> What do you mean by "selected folder"? I thought the collection ID is globally unique? Why can it not be re-used independent from any selection?

Collection ID is globally unique, this is about saving resources. Note that you are touching some very very performance and memory-sensitive parts of code here.

If you open a virtual search folder, each item can be from a different folder, so you will populate the hashtable with many mappings. However, when you then switch to a regular folder, you only need a single mapping, because all Items there belong to a single collection. Without clearing the hashtable on folder switch, you would be

1. keeping unused mappings allocated and using memory
2. forcing the lookup code to look up through all the irrelevant mappings just to find that one for the current folder - doing that for each Item

Since majority of usecases is browsing regular folders, where there will be only single mapping, you should optimize for that.

> gordin wrote in storagemodel.cpp:506
> Actually, this does not work. Neither passing "d->mChildrenFilterModel" nor "static_cast<QAbstractProxyModel *>(d->mChildrenFilterModel)" will produce a result, "idx" is invalid.

Interesting, but very well, then.

REPOSITORY
  R94 PIM: Message Library

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

To: gordin, #vdg, dvratil
Cc: dvratil, kde-pim, jamesth, fbampaloukas, dvasin, hrouis, rodsevich, ach, winterz, vkrause, mlaurent, knauss
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20200513/edc0a726/attachment.htm>