List space children in canonical order

Cargo.toml

@@ -50,6 +50,7 @@ memuse = "0.2.1"
 nonempty = { version = "0.8.1", features = ["serialize"] }
 signal-hook = "0.3.17"
 smallvec = "1.11.1"
+sorted-vec = "0.8.3"
 
 # Matrix
 eyeball = "0.8.7" # data structures observer returned by matrix-sdk-ui

src/buffers/mod.rs

@@ -14,13 +14,17 @@
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
 
-use crate::widgets::Prerender;
+use std::cmp::Ordering;
+use std::collections::{HashMap, HashSet};
+
 use futures::stream::FuturesUnordered;
 use futures::StreamExt;
 use matrix_sdk::async_trait;
 use nonempty::NonEmpty;
 use ratatui::text::Text;
-use std::collections::{HashMap, HashSet};
+use sorted_vec::SortedVec;
+
+use crate::widgets::Prerender;
 
 mod log;
 pub use log::LogBuffer;
@@ -35,6 +39,47 @@ pub enum BufferId {
   Room(matrix_sdk::ruma::OwnedRoomId),
 }
 
+/// Values should follow the ["Ordering of children within a space" algorithm](https://spec.matrix.org/v1.8/client-server-api/#ordering-of-children-within-a-space).
+#[derive(Clone, Debug, Default, Eq, PartialEq)]
+pub struct BufferSortKey {
+  /// Explicit `order` key in the `m.space.child` event
+  explicit_order: Option<String>,
+  origin_server_ts: Option<matrix_sdk::ruma::MilliSecondsSinceUnixEpoch>,
+}
+
+impl Ord for BufferSortKey {
+  fn cmp(&self, other: &Self) -> Ordering {
+    match (self.explicit_order.as_ref(), other.explicit_order.as_ref()) {
+      (Some(e1), Some(e2)) => e1.cmp(&e2),
+      (Some(_), None) => Ordering::Less, // children without 'order' go to the end
+      (None, Some(_)) => Ordering::Greater, // ditto
+      (None, None) => Ordering::Equal,
+    }
+    // Should we apply the same algo here? The algo from the Matrix spec does not apply
+    // because it does not consider children with m.space.parent.
+    // I'd say yes, so that all children declared by the parent are first, then all children
+    // not explicitly recognized by the parent; instead of having the latter in the middle
+    // of the list if only some children of the parent have an explicit "order" key.
+    .then(
+      match (
+        self.origin_server_ts.as_ref(),
+        other.origin_server_ts.as_ref(),
+      ) {
+        (Some(ts1), Some(ts2)) => ts1.cmp(&ts2),
+        (Some(_), None) => Ordering::Less,
+        (None, Some(_)) => Ordering::Greater,
+        (None, None) => Ordering::Equal,
+      },
+    )
+  }
+}
+
+impl PartialOrd for BufferSortKey {
+  fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+    Some(self.cmp(other))
+  }
+}
+
 #[derive(Debug, Clone)]
 pub enum BufferItemContent<'buf> {
   Text(Text<'buf>),
@@ -57,7 +102,7 @@ pub trait Buffer: Send + Sync + memuse::DynamicUsage {
   fn parent(&self) -> Option<BufferId> {
     None
   }
-  fn children(&self) -> Option<Vec<BufferId>> {
+  fn children(&self) -> Option<SortedVec<(BufferSortKey, BufferId)>> {
     None
   }
   /// Returns if there are any updates to apply.
@@ -79,7 +124,7 @@ pub trait Buffer: Send + Sync + memuse::DynamicUsage {
 pub struct Buffers {
   buffers: Vec<Box<dyn Buffer>>,
   parents: HashMap<BufferId, HashSet<BufferId>>,
-  children: HashMap<BufferId, HashSet<BufferId>>,
+  children: HashMap<BufferId, SortedVec<(BufferSortKey, BufferId)>>,
   /// Set of buffers already placed after a parent space, so other spaces should not
   /// steal them, even if they are also their child (or it would cause buffers to move
   /// every time we re-sort the buffer list)
@@ -127,7 +172,7 @@ impl Buffers {
     &self.parents
   }
 
-  pub fn children(&self) -> &HashMap<BufferId, HashSet<BufferId>> {
+  pub fn children(&self) -> &HashMap<BufferId, SortedVec<(BufferSortKey, BufferId)>> {
     &self.children
   }
 
@@ -148,15 +193,20 @@ impl Buffers {
         for child in children.iter() {
           self
             .parents
-            .entry(child.clone())
+            .entry(child.1.clone())
             .or_insert_with(HashSet::new)
             .insert(new_buffer_id.clone());
         }
-        self
+        let all_children = self
           .children
           .entry(new_buffer_id.clone())
-          .or_insert_with(HashSet::new)
+          .or_insert_with(SortedVec::new);
-          .extend(children.into_iter());
+        // all_children.extend() uses .insert() which always does a bisect search,
+        // while .push is O(1) when the element is at the end (which is usually what
+        // happens here).
+        for child in children.into_vec().into_iter() {
+          all_children.push(child);
+        }
       },
       None => {},
     };
@@ -167,11 +217,17 @@ impl Buffers {
           .entry(new_buffer_id.clone())
           .or_insert_with(HashSet::new)
           .insert(parent.clone());
-        self
+        let siblings = self
           .children
           .entry(parent.clone())
-          .or_insert_with(HashSet::new)
+          .or_insert_with(SortedVec::new);
-          .insert(new_buffer_id.clone());
+        if siblings
+          .iter()
+          .find(|(_key, buffer_id)| *buffer_id == new_buffer_id)
+          .is_none()
+        {
+          siblings.push((BufferSortKey::default(), new_buffer_id.clone()));
+        }
         Some(parent)
       },
       None => self
@@ -182,7 +238,7 @@ impl Buffers {
     let children = self
       .children
       .entry(new_buffer_id.clone())
-      .or_insert_with(HashSet::new);
+      .or_insert_with(SortedVec::new);
 
     match parent.and_then(|parent_id| self.buffers.iter().position(|buf| buf.id() == parent_id)) {
       None => self.buffers.push(new_buffer),
@@ -235,7 +291,12 @@ impl Buffers {
     let mut child_buffers = Vec::new();
     std::mem::swap(&mut all_buffers, &mut self.buffers);
     for buf in all_buffers.into_iter() {
-      if children.contains(&buf.id()) && !self.attached_to_parent.contains(&buf.id()) {
+      if children
+        .iter()
+        .find(|(_key, buf_id)| *buf_id == buf.id())
+        .is_some()
+        && !self.attached_to_parent.contains(&buf.id())
+      {
         self.attached_to_parent.insert(buf.id());
         child_buffers.push(buf);
       } else {

src/buffers/room.rs

@@ -40,9 +40,10 @@ use matrix_sdk_ui::timeline::{
 use memuse::DynamicUsage;
 use ratatui::text::Text;
 use smallvec::SmallVec;
+use sorted_vec::SortedVec;
 use tokio::sync::oneshot;
 
-use super::{Buffer, BufferId, BufferItem, BufferItemContent};
+use super::{Buffer, BufferId, BufferItem, BufferItemContent, BufferSortKey};
 use crate::widgets::Prerender;
 
 /// Like [`BufferItemContent`] but owned.
@@ -441,16 +442,18 @@ impl Buffer for RoomBuffer {
       .and_then(|roominfo| roominfo.parent.as_ref())
       .map(|parent| BufferId::Room(parent.to_owned()))
   }
-  fn children(&self) -> Option<Vec<BufferId>> {
+  fn children(&self) -> Option<SortedVec<(BufferSortKey, BufferId)>> {
     self
       .computed_roominfo
       .as_ref()
       .and_then(|roominfo| roominfo.children.as_ref())
-      .map(|children| {
+      .map(|children: &SortedVec<_>| {
-        children
+        let children = children
           .iter()
-          .map(|child| BufferId::Room(child.to_owned()))
+          .map(|(sort_key, room_id)| (sort_key.clone(), BufferId::Room(room_id.to_owned())))
-          .collect()
+          .collect();
+        // This is safe because the map above preserves order
+        unsafe { SortedVec::from_sorted(children) }
       })
   }
 
@@ -588,23 +591,33 @@ async fn compute_room_info(
         .await
       {
         Ok(child_events) => {
-          Some(
+          Some(SortedVec::from_unsorted(
             child_events
               .into_iter()
               // Extract state key (ie. the child's id) and sender
               .flat_map(|parent_event| {
                 match parent_event.deserialize() {
-                  Ok(SyncOrStrippedState::Sync(SyncStateEvent::Original(e))) => {
+                  Ok(SyncOrStrippedState::Sync(SyncStateEvent::Original(e))) => Some((
-                    Some(e.state_key.to_owned())
+                    BufferSortKey {
-                  },
+                      explicit_order: e.content.order,
+                      origin_server_ts: Some(e.origin_server_ts),
+                    },
+                    e.state_key.to_owned(),
+                  )),
                   Ok(SyncOrStrippedState::Sync(SyncStateEvent::Redacted(_))) => None,
-                  Ok(SyncOrStrippedState::Stripped(e)) => Some(e.state_key.to_owned()),
+                  Ok(SyncOrStrippedState::Stripped(e)) => Some((
+                    BufferSortKey {
+                      explicit_order: None,
+                      origin_server_ts: None, // Why don't stripped events have origin_server_ts!?
+                    },
+                    e.state_key.to_owned(),
+                  )),
 
                   Err(_) => None, // Ignore deserialization errors
                 }
               })
               .collect(),
-          )
+          ))
         },
         Err(e) => {
           tracing::error!("Failed to get child rooms of {}: {:?}", room.room_id(), e);
@@ -649,7 +662,7 @@ async fn compute_room_info(
 struct ComputedRoomInfo {
   display_name: Option<DisplayName>,
   parent: Option<OwnedRoomId>,
-  children: Option<Vec<OwnedRoomId>>,
+  children: Option<SortedVec<(BufferSortKey, OwnedRoomId)>>,
   roominfo_hash: u64,
 }