[Differential] [Accepted] D1865: TasksModel: cache launcherCount().

[hein (Eike Hein)]

hein accepted this revision.
hein added a comment.
This revision is now accepted and ready to land.


  Since I'm only asking for comment changes I'm accepting it code-wise :)

INLINE COMMENTS

> tasksmodel.cpp:744
>          if (!filterIndex.data(AbstractTasksModel::IsLauncher).toBool()) {
> +            // TODO: it would be much faster if we didn't ask for a URL with serialized PNG data in it, just to discard it a few lines below
>              const QUrl &launcherUrl = filterIndex.data(AbstractTasksModel::LauncherUrl).toUrl();

(Very) pedantic: "It" should be uppercase and the sentence end in a period.

> tasksmodel.cpp:1324
>                  filteredIndex.data(AbstractTasksModel::LauncherUrl).toUrl(), IgnoreQueryItems))) {
> -                emit launcherCountChanged();
> +                // Not sure why this is here. We're filtering, not handling a change.
> +                QMetaObject::invokeMethod(const_cast<TasksModel *>(this), "updateLauncherCount", Qt::QueuedConnection);

If filterAcceptsRow() decides to filter out a launcher task, the count might have changed. Of course it would also be possible to do this outside of filtering, but ... can you drop this comment for now?

REPOSITORY
  rPLASMAWORKSPACE Plasma Workspace

BRANCH
  master

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: dfaure, hein
Cc: broulik, plasma-devel, jensreuterberg, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160614/e946e1f9/attachment-0001.html>