[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    [kactivities] src: Sorting activities in the cache alphabetically by name
From:       Ivan Čukić <ivan.cukic () kde ! org>
Date:       2016-09-18 10:57:01
Message-ID: E1blZmP-0001Vf-6W () code ! kde ! org
[Download RAW message or body]

Git commit 19d64627dffff9251adf1a24fec7bdfb9a931d58 by Ivan Čukić.
Committed on 18/09/2016 at 10:52.
Pushed by ivan into branch 'master'.

Sorting activities in the cache alphabetically by name

Many activities clients just show the exact list of results
that the library returns them. in the order that the library uses.

Since this order ends up being shown to the user in quite a few places,
the library now sorts the activities internally.

This affects:
 - KWin (window menu)
 - KCM for window rules
 - Activity pager
 - maybe something else as well

BUG:362774

M  +0    -10   src/common/dbus/org.kde.ActivityManager.Activities.h
M  +26   -17   src/lib/activitiescache_p.cpp
M  +27   -11   src/lib/activitiescache_p.h
M  +2    -2    src/lib/info.cpp

http://commits.kde.org/kactivities/19d64627dffff9251adf1a24fec7bdfb9a931d58

diff --git a/src/common/dbus/org.kde.ActivityManager.Activities.h \
b/src/common/dbus/org.kde.ActivityManager.Activities.h index b387529..94f0fbe 100644
--- a/src/common/dbus/org.kde.ActivityManager.Activities.h
+++ b/src/common/dbus/org.kde.ActivityManager.Activities.h
@@ -46,16 +46,6 @@ struct ActivityInfo {
         , state(state)
     {
     }
-
-    bool operator<(const ActivityInfo &other) const
-    {
-        return id < other.id;
-    }
-
-    bool operator==(const ActivityInfo &other) const
-    {
-        return id == other.id;
-    }
 };
 
 typedef QList<ActivityInfo> ActivityInfoList;
diff --git a/src/lib/activitiescache_p.cpp b/src/lib/activitiescache_p.cpp
index f392e04..09d4186 100644
--- a/src/lib/activitiescache_p.cpp
+++ b/src/lib/activitiescache_p.cpp
@@ -122,8 +122,10 @@ void ActivitiesCache::removeActivity(const QString &id)
 {
     // qDebug() << "Removing the activity";
 
-    auto where = std::lower_bound(m_activities.begin(), m_activities.end(),
-                                  ActivityInfo(id));
+    // Since we are sorting the activities by name now,
+    // we can not use lower_bound to search for an activity
+    // with a specified id
+    const auto where = find(id);
 
     if (where != m_activities.end() && where->id == id) {
         m_activities.erase(where);
@@ -166,7 +168,7 @@ void ActivitiesCache::updateActivity(const QString &id)
 
 void ActivitiesCache::updateActivityState(const QString &id, int state)
 {
-    auto where = find<Mutable>(id);
+    auto where = getInfo<Mutable>(id);
 
     if (where && where->state != state) {
         auto isInvalid = [](int state) {
@@ -234,20 +236,27 @@ void ActivitiesCache::setActivityInfo(const ActivityInfo &info)
 {
     // qDebug() << "Setting activity info" << info.id;
 
-    auto where
-        = std::lower_bound(m_activities.begin(), m_activities.end(), info);
+    // Are we updating an existing activity, or adding a new one?
+    const auto iter = find(info.id);
+    const auto present = iter != m_activities.end();
 
-    if (where == m_activities.end() || where->id != info.id) {
-        // We haven't found the activity with the specified id.
-        // This means it is a new activity.
-        m_activities.insert(where, info);
-        emit activityAdded(info.id);
-        emit activityListChanged();
+    // If there is an activity with the specified id,
+    // we are going to remove it, temporarily.
+    if (present) {
+        m_activities.erase(iter);
+    }
 
-    } else {
-        // An existing activity changed
-        *where = info;
+    // Now, we need to find where to insert the activity
+    // and keep the cache sorted by name
+    const auto where = lower_bound(info);
+
+    m_activities.insert(where, info);
+
+    if (present) {
         emit activityChanged(info.id);
+    } else {
+        emit activityAdded(info.id);
+        emit activityListChanged();
     }
 }
 
@@ -255,7 +264,7 @@ void ActivitiesCache::setActivityInfo(const ActivityInfo &info)
     void ActivitiesCache::setActivity##WHAT(const QString &id,                 \
                                             const QString &value)              \
     {                                                                          \
-        auto where = find<Mutable>(id);                                        \
+        auto where = getInfo<Mutable>(id);                                     \
                                                                                \
         if (where) {                                                           \
             where->What = value;                                               \
@@ -277,12 +286,12 @@ void ActivitiesCache::setAllActivities(const ActivityInfoList \
&_activities)  
     ActivityInfoList activities = _activities;
 
-    qSort(activities.begin(), activities.end());
-
     foreach (const ActivityInfo &info, activities) {
         m_activities << info;
     }
 
+    std::sort(m_activities.begin(), m_activities.end(), &infoLessThan);
+
     m_status = Consumer::Running;
     emit serviceStatusChanged(m_status);
     emit activityListChanged();
diff --git a/src/lib/activitiescache_p.h b/src/lib/activitiescache_p.h
index 6de318f..4afb933 100644
--- a/src/lib/activitiescache_p.h
+++ b/src/lib/activitiescache_p.h
@@ -83,25 +83,41 @@ public:
     template <typename _Result, typename _Functor>
     void passInfoFromReply(QDBusPendingCallWatcher *watcher, _Functor f);
 
-    template <int Policy = kamd::utils::Const>
-    inline typename kamd::utils::ptr_to<ActivityInfo, Policy>::type
-    find(const ActivityInfo &info)
+    static
+    bool infoLessThan(const ActivityInfo &info, const ActivityInfo &other)
     {
-        auto where
-            = std::lower_bound(m_activities.begin(), m_activities.end(), info);
+        const auto comp =
+            QString::compare(info.name, other.name, Qt::CaseInsensitive);
+        return comp < 0 || (comp == 0 && info.id < other.id);
+    }
 
-        if (where != m_activities.end() && where->id == info.id) {
-            return &(*where);
-        }
+    ActivityInfoList::iterator
+    find(const QString &id)
+    {
+        return std::find_if(m_activities.begin(), m_activities.end(),
+                            [&id] (const ActivityInfo &info) {
+                                return info.id == id;
+                            });
+    }
 
-        return Q_NULLPTR;
+    ActivityInfoList::iterator
+    lower_bound(const ActivityInfo &info)
+    {
+        return std::lower_bound(m_activities.begin(), m_activities.end(),
+                                info, &infoLessThan);
     }
 
     template <int Policy = kamd::utils::Const>
     inline typename kamd::utils::ptr_to<ActivityInfo, Policy>::type
-    find(const QString &id)
+    getInfo(const QString &id)
     {
-        return find<Policy>(ActivityInfo(id));
+        const auto where = find(id);
+
+        if (where != m_activities.end()) {
+            return &(*where);
+        }
+
+        return Q_NULLPTR;
     }
 
     template <typename TargetSlot>
diff --git a/src/lib/info.cpp b/src/lib/info.cpp
index ca3c0e4..3166055 100644
--- a/src/lib/info.cpp
+++ b/src/lib/info.cpp
@@ -161,7 +161,7 @@ Info::State Info::state() const
 {
     if (d->cache->m_status == Consumer::Unknown) return Info::Unknown;
 
-    auto info = d->cache->find(d->id);
+    auto info = d->cache->getInfo(d->id);
 
     if (!info) return Info::Invalid;
 
@@ -205,7 +205,7 @@ Info::Availability Info::availability() const
 #define CREATE_GETTER(What)                                                    \
     QString Info::What() const                                                 \
     {                                                                          \
-        auto info = d->cache->find(d->id);                                     \
+        auto info = d->cache->getInfo(d->id);                                  \
         return info ? info->What : QString();                                  \
     }
 


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic