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
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel