Fix worktree ordering with `PathList` (#39944)

Elliot Thomas and MrSubidubi created

The recent introduction of PathList removed some of the ordering logic
resulting in paths always being alphabetised.

This change restores the previous logic for sorting worktrees in a
project using the newer PathList type.

Closes #39934

Release Notes:

- Fixed manual worktree reordering

<details>

<summary>Screen recording of it retaining the order</summary>


https://github.com/user-attachments/assets/0197d118-6ea7-4d2d-8fec-c917fcb9d277

</details>

---------

Co-authored-by: MrSubidubi <finn@zed.dev>

Change summary

crates/recent_projects/src/recent_projects.rs |  6 
crates/workspace/src/history_manager.rs       |  3 
crates/workspace/src/path_list.rs             | 85 +++++++++++++++++++-
crates/workspace/src/persistence.rs           |  4 
crates/workspace/src/workspace.rs             |  2 
5 files changed, 85 insertions(+), 15 deletions(-)

Detailed changes

crates/recent_projects/src/recent_projects.rs 🔗

@@ -311,8 +311,7 @@ impl PickerDelegate for RecentProjectsDelegate {
             .filter(|(_, (id, _, _))| !self.is_current_workspace(*id, cx))
             .map(|(id, (_, _, paths))| {
                 let combined_string = paths
-                    .paths()
-                    .iter()
+                    .ordered_paths()
                     .map(|path| path.compact().to_string_lossy().into_owned())
                     .collect::<Vec<_>>()
                     .join("");
@@ -462,8 +461,7 @@ impl PickerDelegate for RecentProjectsDelegate {
         let mut path_start_offset = 0;
 
         let (match_labels, paths): (Vec<_>, Vec<_>) = paths
-            .paths()
-            .iter()
+            .ordered_paths()
             .map(|p| p.compact())
             .map(|path| {
                 let highlighted_text =

crates/workspace/src/history_manager.rs 🔗

@@ -128,8 +128,7 @@ impl HistoryManager {
 impl HistoryManagerEntry {
     pub fn new(id: WorkspaceId, paths: &PathList) -> Self {
         let path = paths
-            .paths()
-            .iter()
+            .ordered_paths()
             .map(|path| path.compact())
             .collect::<SmallVec<[PathBuf; 2]>>();
         Self { id, path }

crates/workspace/src/path_list.rs 🔗

@@ -3,15 +3,22 @@ use std::{
     sync::Arc,
 };
 
+use itertools::Itertools;
 use util::paths::SanitizedPath;
 
 /// A list of absolute paths, in a specific order.
 ///
 /// The paths are stored in lexicographic order, so that they can be compared to
 /// other path lists without regard to the order of the paths.
+///
+/// The paths can be retrieved in the original order using `ordered_paths()`.
 #[derive(Default, PartialEq, Eq, Debug, Clone)]
 pub struct PathList {
+    /// The paths, in lexicographic order.
     paths: Arc<[PathBuf]>,
+    /// The order in which the paths were provided.
+    ///
+    /// See `ordered_paths()` for a way to get the paths in the original order.
     order: Arc<[usize]>,
 }
 
@@ -42,14 +49,25 @@ impl PathList {
         self.paths.is_empty()
     }
 
+    /// Get the paths in lexicographic order.
     pub fn paths(&self) -> &[PathBuf] {
         self.paths.as_ref()
     }
 
+    /// Get the order in which the paths were provided.
     pub fn order(&self) -> &[usize] {
         self.order.as_ref()
     }
 
+    /// Get the paths in the original order.
+    pub fn ordered_paths(&self) -> impl Iterator<Item = &PathBuf> {
+        self.order
+            .iter()
+            .zip(self.paths.iter())
+            .sorted_by_key(|(i, _)| **i)
+            .map(|(_, path)| path)
+    }
+
     pub fn is_lexicographically_ordered(&self) -> bool {
         self.order.iter().enumerate().all(|(i, &j)| i == j)
     }
@@ -109,15 +127,70 @@ mod tests {
         let list1 = PathList::new(&["a/d", "a/c"]);
         let list2 = PathList::new(&["a/c", "a/d"]);
 
-        assert_eq!(list1.paths(), list2.paths());
-        assert_ne!(list1, list2);
-        assert_eq!(list1.order(), &[1, 0]);
-        assert_eq!(list2.order(), &[0, 1]);
+        assert_eq!(list1.paths(), list2.paths(), "paths differ");
+        assert_eq!(list1.order(), &[1, 0], "list1 order incorrect");
+        assert_eq!(list2.order(), &[0, 1], "list2 order incorrect");
 
         let list1_deserialized = PathList::deserialize(&list1.serialize());
-        assert_eq!(list1_deserialized, list1);
+        assert_eq!(list1_deserialized, list1, "list1 deserialization failed");
 
         let list2_deserialized = PathList::deserialize(&list2.serialize());
-        assert_eq!(list2_deserialized, list2);
+        assert_eq!(list2_deserialized, list2, "list2 deserialization failed");
+
+        assert_eq!(
+            list1.ordered_paths().collect_array().unwrap(),
+            [&PathBuf::from("a/d"), &PathBuf::from("a/c")],
+            "list1 ordered paths incorrect"
+        );
+        assert_eq!(
+            list2.ordered_paths().collect_array().unwrap(),
+            [&PathBuf::from("a/c"), &PathBuf::from("a/d")],
+            "list2 ordered paths incorrect"
+        );
+    }
+
+    #[test]
+    fn test_path_list_ordering() {
+        let list = PathList::new(&["b", "a", "c"]);
+        assert_eq!(
+            list.paths(),
+            &[PathBuf::from("a"), PathBuf::from("b"), PathBuf::from("c")]
+        );
+        assert_eq!(list.order(), &[1, 0, 2]);
+        assert!(!list.is_lexicographically_ordered());
+
+        let serialized = list.serialize();
+        let deserialized = PathList::deserialize(&serialized);
+        assert_eq!(deserialized, list);
+
+        assert_eq!(
+            deserialized.ordered_paths().collect_array().unwrap(),
+            [
+                &PathBuf::from("b"),
+                &PathBuf::from("a"),
+                &PathBuf::from("c")
+            ]
+        );
+
+        let list = PathList::new(&["b", "c", "a"]);
+        assert_eq!(
+            list.paths(),
+            &[PathBuf::from("a"), PathBuf::from("b"), PathBuf::from("c")]
+        );
+        assert_eq!(list.order(), &[2, 0, 1]);
+        assert!(!list.is_lexicographically_ordered());
+
+        let serialized = list.serialize();
+        let deserialized = PathList::deserialize(&serialized);
+        assert_eq!(deserialized, list);
+
+        assert_eq!(
+            deserialized.ordered_paths().collect_array().unwrap(),
+            [
+                &PathBuf::from("b"),
+                &PathBuf::from("c"),
+                &PathBuf::from("a"),
+            ]
+        );
     }
 }

crates/workspace/src/persistence.rs 🔗

@@ -2493,7 +2493,7 @@ mod tests {
 
         let workspace_6 = SerializedWorkspace {
             id: WorkspaceId(6),
-            paths: PathList::new(&["/tmp6a", "/tmp6b", "/tmp6c"]),
+            paths: PathList::new(&["/tmp6c", "/tmp6b", "/tmp6a"]),
             location: SerializedWorkspaceLocation::Local,
             center_group: Default::default(),
             window_bounds: Default::default(),
@@ -2534,7 +2534,7 @@ mod tests {
         assert_eq!(locations.len(), 1);
         assert_eq!(
             locations[0].0,
-            PathList::new(&["/tmp6a", "/tmp6b", "/tmp6c"]),
+            PathList::new(&["/tmp6c", "/tmp6b", "/tmp6a"]),
         );
         assert_eq!(locations[0].1, Some(60));
     }

crates/workspace/src/workspace.rs 🔗

@@ -1539,7 +1539,7 @@ impl Workspace {
                 persistence::DB.workspace_for_roots(paths_to_open.as_slice());
 
             if let Some(paths) = serialized_workspace.as_ref().map(|ws| &ws.paths) {
-                paths_to_open = paths.paths().to_vec();
+                paths_to_open = paths.ordered_paths().cloned().collect();
                 if !paths.is_lexicographically_ordered() {
                     project_handle
                         .update(cx, |project, cx| {