Prefer the same order of entries inside outline and project panels, project search multi buffer (#13044)

Kirill Bulatov created

Release Notes:

- N/A

Change summary

Cargo.lock                                |   3 
crates/multi_buffer/src/multi_buffer.rs   |   4 
crates/outline_panel/Cargo.toml           |   1 
crates/outline_panel/src/outline_panel.rs |  47 -------
crates/project/Cargo.toml                 |   1 
crates/project/src/project.rs             | 153 +++++++++++++++++++++---
crates/project_panel/Cargo.toml           |   1 
crates/project_panel/src/project_panel.rs |  51 --------
8 files changed, 140 insertions(+), 121 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -7171,7 +7171,6 @@ dependencies = [
  "serde",
  "serde_json",
  "settings",
- "unicase",
  "util",
  "workspace",
  "worktree",
@@ -7863,6 +7862,7 @@ dependencies = [
  "tempfile",
  "terminal",
  "text",
+ "unicase",
  "unindent",
  "util",
  "which 6.0.0",
@@ -7893,7 +7893,6 @@ dependencies = [
  "settings",
  "theme",
  "ui",
- "unicase",
  "util",
  "workspace",
  "worktree",

crates/multi_buffer/src/multi_buffer.rs 🔗

@@ -1122,12 +1122,12 @@ impl MultiBuffer {
                     for range in ranges.by_ref().take(range_count) {
                         let start = Anchor {
                             buffer_id: Some(buffer_id),
-                            excerpt_id: excerpt_id,
+                            excerpt_id,
                             text_anchor: range.start,
                         };
                         let end = Anchor {
                             buffer_id: Some(buffer_id),
-                            excerpt_id: excerpt_id,
+                            excerpt_id,
                             text_anchor: range.end,
                         };
                         if tx.send(start..end).await.is_err() {

crates/outline_panel/Cargo.toml 🔗

@@ -29,7 +29,6 @@ schemars.workspace = true
 serde.workspace = true
 serde_json.workspace = true
 settings.workspace = true
-unicase.workspace = true
 util.workspace = true
 worktree.workspace = true
 workspace.workspace = true

crates/outline_panel/src/outline_panel.rs 🔗

@@ -34,8 +34,7 @@ use outline_panel_settings::{OutlinePanelDockPosition, OutlinePanelSettings};
 use project::{File, Fs, Project};
 use serde::{Deserialize, Serialize};
 use settings::{Settings, SettingsStore};
-use unicase::UniCase;
-use util::{maybe, NumericPrefixWithSuffix, ResultExt, TryFutureExt};
+use util::{ResultExt, TryFutureExt};
 use workspace::{
     dock::{DockPosition, Panel, PanelEvent},
     item::ItemHandle,
@@ -1755,7 +1754,7 @@ impl OutlinePanel {
                             .into_iter()
                             .map(|(worktree_id, (worktree_snapshot, entries))| {
                                 let mut entries = entries.into_iter().collect::<Vec<_>>();
-                                sort_worktree_entries(&mut entries);
+                                project::sort_worktree_entries(&mut entries);
                                 worktree_snapshot.propagate_git_statuses(&mut entries);
                                 (worktree_id, entries)
                             })
@@ -2357,48 +2356,6 @@ fn back_to_common_visited_parent(
     None
 }
 
-fn sort_worktree_entries(entries: &mut Vec<Entry>) {
-    entries.sort_by(|entry_a, entry_b| {
-        let mut components_a = entry_a.path.components().peekable();
-        let mut components_b = entry_b.path.components().peekable();
-        loop {
-            match (components_a.next(), components_b.next()) {
-                (Some(component_a), Some(component_b)) => {
-                    let a_is_file = components_a.peek().is_none() && entry_a.is_file();
-                    let b_is_file = components_b.peek().is_none() && entry_b.is_file();
-                    let ordering = a_is_file.cmp(&b_is_file).then_with(|| {
-                        let maybe_numeric_ordering = maybe!({
-                            let num_and_remainder_a = Path::new(component_a.as_os_str())
-                                .file_stem()
-                                .and_then(|s| s.to_str())
-                                .and_then(NumericPrefixWithSuffix::from_numeric_prefixed_str)?;
-                            let num_and_remainder_b = Path::new(component_b.as_os_str())
-                                .file_stem()
-                                .and_then(|s| s.to_str())
-                                .and_then(NumericPrefixWithSuffix::from_numeric_prefixed_str)?;
-
-                            num_and_remainder_a.partial_cmp(&num_and_remainder_b)
-                        });
-
-                        maybe_numeric_ordering.unwrap_or_else(|| {
-                            let name_a = UniCase::new(component_a.as_os_str().to_string_lossy());
-                            let name_b = UniCase::new(component_b.as_os_str().to_string_lossy());
-
-                            name_a.cmp(&name_b)
-                        })
-                    });
-                    if !ordering.is_eq() {
-                        return ordering;
-                    }
-                }
-                (Some(_), None) => break cmp::Ordering::Greater,
-                (None, Some(_)) => break cmp::Ordering::Less,
-                (None, None) => break cmp::Ordering::Equal,
-            }
-        }
-    });
-}
-
 fn file_name(path: &Path) -> String {
     let mut current_path = path;
     loop {

crates/project/Cargo.toml 🔗

@@ -65,6 +65,7 @@ snippet.workspace = true
 terminal.workspace = true
 text.workspace = true
 util.workspace = true
+unicase.workspace = true
 which.workspace = true
 
 [dev-dependencies]

crates/project/src/project.rs 🔗

@@ -105,12 +105,13 @@ use task::{
 };
 use terminals::Terminals;
 use text::{Anchor, BufferId, LineEnding};
+use unicase::UniCase;
 use util::{
     debug_panic, defer, maybe, merge_json_value_into, parse_env_output,
     paths::{
         LOCAL_SETTINGS_RELATIVE_PATH, LOCAL_TASKS_RELATIVE_PATH, LOCAL_VSCODE_TASKS_RELATIVE_PATH,
     },
-    post_inc, ResultExt, TryFutureExt as _,
+    post_inc, NumericPrefixWithSuffix, ResultExt, TryFutureExt as _,
 };
 use worktree::{CreatedEntry, RemoteWorktreeClient, Snapshot, Traversal};
 
@@ -620,29 +621,11 @@ enum SearchMatchCandidate {
     Path {
         worktree_id: WorktreeId,
         is_ignored: bool,
+        is_file: bool,
         path: Arc<Path>,
     },
 }
 
-impl SearchMatchCandidate {
-    fn path(&self) -> Option<Arc<Path>> {
-        match self {
-            SearchMatchCandidate::OpenBuffer { path, .. } => path.clone(),
-            SearchMatchCandidate::Path { path, .. } => Some(path.clone()),
-        }
-    }
-
-    fn is_ignored(&self) -> bool {
-        matches!(
-            self,
-            SearchMatchCandidate::Path {
-                is_ignored: true,
-                ..
-            }
-        )
-    }
-}
-
 pub enum SearchResult {
     Buffer {
         buffer: Model<Buffer>,
@@ -7198,7 +7181,9 @@ impl Project {
             } else {
                 false
             };
-            matching_paths.sort_by_key(|candidate| (candidate.is_ignored(), candidate.path()));
+            cx.update(|cx| {
+                sort_search_matches(&mut matching_paths, cx);
+            })?;
 
             let mut range_count = 0;
             let query = Arc::new(query);
@@ -11257,6 +11242,7 @@ async fn search_snapshots(
                         worktree_id: snapshot.id(),
                         path: entry.path.clone(),
                         is_ignored: entry.is_ignored,
+                        is_file: entry.is_file(),
                     };
                     if results_tx.send(project_path).await.is_err() {
                         return;
@@ -11329,6 +11315,7 @@ async fn search_ignored_entry(
                                 .expect("scanning worktree-related files"),
                         ),
                         is_ignored: true,
+                        is_file: ignored_entry.is_file(),
                     };
                     if counter_tx.send(project_path).await.is_err() {
                         return;
@@ -11994,3 +11981,127 @@ impl DiagnosticSummary {
         }
     }
 }
+
+pub fn sort_worktree_entries(entries: &mut Vec<Entry>) {
+    entries.sort_by(|entry_a, entry_b| {
+        compare_paths(
+            (&entry_a.path, entry_a.is_file()),
+            (&entry_b.path, entry_b.is_file()),
+        )
+    });
+}
+
+fn sort_search_matches(search_matches: &mut Vec<SearchMatchCandidate>, cx: &AppContext) {
+    search_matches.sort_by(|entry_a, entry_b| match (entry_a, entry_b) {
+        (
+            SearchMatchCandidate::OpenBuffer {
+                buffer: buffer_a,
+                path: None,
+            },
+            SearchMatchCandidate::OpenBuffer {
+                buffer: buffer_b,
+                path: None,
+            },
+        ) => buffer_a
+            .read(cx)
+            .remote_id()
+            .cmp(&buffer_b.read(cx).remote_id()),
+        (
+            SearchMatchCandidate::OpenBuffer { path: None, .. },
+            SearchMatchCandidate::Path { .. }
+            | SearchMatchCandidate::OpenBuffer { path: Some(_), .. },
+        ) => Ordering::Less,
+        (
+            SearchMatchCandidate::OpenBuffer { path: Some(_), .. }
+            | SearchMatchCandidate::Path { .. },
+            SearchMatchCandidate::OpenBuffer { path: None, .. },
+        ) => Ordering::Greater,
+        (
+            SearchMatchCandidate::OpenBuffer {
+                path: Some(path_a), ..
+            },
+            SearchMatchCandidate::Path {
+                is_file: is_file_b,
+                path: path_b,
+                ..
+            },
+        ) => compare_paths((path_a.as_ref(), true), (path_b.as_ref(), *is_file_b)),
+        (
+            SearchMatchCandidate::Path {
+                is_file: is_file_a,
+                path: path_a,
+                ..
+            },
+            SearchMatchCandidate::OpenBuffer {
+                path: Some(path_b), ..
+            },
+        ) => compare_paths((path_a.as_ref(), *is_file_a), (path_b.as_ref(), true)),
+        (
+            SearchMatchCandidate::OpenBuffer {
+                path: Some(path_a), ..
+            },
+            SearchMatchCandidate::OpenBuffer {
+                path: Some(path_b), ..
+            },
+        ) => compare_paths((path_a.as_ref(), true), (path_b.as_ref(), true)),
+        (
+            SearchMatchCandidate::Path {
+                worktree_id: worktree_id_a,
+                is_file: is_file_a,
+                path: path_a,
+                ..
+            },
+            SearchMatchCandidate::Path {
+                worktree_id: worktree_id_b,
+                is_file: is_file_b,
+                path: path_b,
+                ..
+            },
+        ) => worktree_id_a.cmp(&worktree_id_b).then_with(|| {
+            compare_paths((path_a.as_ref(), *is_file_a), (path_b.as_ref(), *is_file_b))
+        }),
+    });
+}
+
+fn compare_paths(
+    (path_a, a_is_file): (&Path, bool),
+    (path_b, b_is_file): (&Path, bool),
+) -> cmp::Ordering {
+    let mut components_a = path_a.components().peekable();
+    let mut components_b = path_b.components().peekable();
+    loop {
+        match (components_a.next(), components_b.next()) {
+            (Some(component_a), Some(component_b)) => {
+                let a_is_file = components_a.peek().is_none() && a_is_file;
+                let b_is_file = components_b.peek().is_none() && b_is_file;
+                let ordering = a_is_file.cmp(&b_is_file).then_with(|| {
+                    let maybe_numeric_ordering = maybe!({
+                        let num_and_remainder_a = Path::new(component_a.as_os_str())
+                            .file_stem()
+                            .and_then(|s| s.to_str())
+                            .and_then(NumericPrefixWithSuffix::from_numeric_prefixed_str)?;
+                        let num_and_remainder_b = Path::new(component_b.as_os_str())
+                            .file_stem()
+                            .and_then(|s| s.to_str())
+                            .and_then(NumericPrefixWithSuffix::from_numeric_prefixed_str)?;
+
+                        num_and_remainder_a.partial_cmp(&num_and_remainder_b)
+                    });
+
+                    maybe_numeric_ordering.unwrap_or_else(|| {
+                        let name_a = UniCase::new(component_a.as_os_str().to_string_lossy());
+                        let name_b = UniCase::new(component_b.as_os_str().to_string_lossy());
+
+                        name_a.cmp(&name_b)
+                    })
+                });
+                if !ordering.is_eq() {
+                    return ordering;
+                }
+            }
+            (Some(_), None) => break cmp::Ordering::Greater,
+            (None, Some(_)) => break cmp::Ordering::Less,
+            (None, None) => break cmp::Ordering::Equal,
+        }
+    }
+}

crates/project_panel/Cargo.toml 🔗

@@ -31,7 +31,6 @@ serde_json.workspace = true
 settings.workspace = true
 theme.workspace = true
 ui.workspace = true
-unicase.workspace = true
 util.workspace = true
 client.workspace = true
 worktree.workspace = true

crates/project_panel/src/project_panel.rs 🔗

@@ -22,7 +22,6 @@ use project::{Entry, EntryKind, Fs, Project, ProjectEntryId, ProjectPath, Worktr
 use project_panel_settings::{ProjectPanelDockPosition, ProjectPanelSettings};
 use serde::{Deserialize, Serialize};
 use std::{
-    cmp::Ordering,
     collections::HashSet,
     ffi::OsStr,
     ops::Range,
@@ -31,8 +30,7 @@ use std::{
 };
 use theme::ThemeSettings;
 use ui::{prelude::*, v_flex, ContextMenu, Icon, KeyBinding, Label, ListItem, Tooltip};
-use unicase::UniCase;
-use util::{maybe, NumericPrefixWithSuffix, ResultExt, TryFutureExt};
+use util::{maybe, ResultExt, TryFutureExt};
 use workspace::{
     dock::{DockPosition, Panel, PanelEvent},
     notifications::{DetachAndPromptErr, NotifyTaskExt},
@@ -1623,52 +1621,7 @@ impl ProjectPanel {
             }
 
             snapshot.propagate_git_statuses(&mut visible_worktree_entries);
-
-            visible_worktree_entries.sort_by(|entry_a, entry_b| {
-                let mut components_a = entry_a.path.components().peekable();
-                let mut components_b = entry_b.path.components().peekable();
-                loop {
-                    match (components_a.next(), components_b.next()) {
-                        (Some(component_a), Some(component_b)) => {
-                            let a_is_file = components_a.peek().is_none() && entry_a.is_file();
-                            let b_is_file = components_b.peek().is_none() && entry_b.is_file();
-                            let ordering = a_is_file.cmp(&b_is_file).then_with(|| {
-                                let maybe_numeric_ordering = maybe!({
-                                    let num_and_remainder_a = Path::new(component_a.as_os_str())
-                                        .file_stem()
-                                        .and_then(|s| s.to_str())
-                                        .and_then(
-                                            NumericPrefixWithSuffix::from_numeric_prefixed_str,
-                                        )?;
-                                    let num_and_remainder_b = Path::new(component_b.as_os_str())
-                                        .file_stem()
-                                        .and_then(|s| s.to_str())
-                                        .and_then(
-                                            NumericPrefixWithSuffix::from_numeric_prefixed_str,
-                                        )?;
-
-                                    num_and_remainder_a.partial_cmp(&num_and_remainder_b)
-                                });
-
-                                maybe_numeric_ordering.unwrap_or_else(|| {
-                                    let name_a =
-                                        UniCase::new(component_a.as_os_str().to_string_lossy());
-                                    let name_b =
-                                        UniCase::new(component_b.as_os_str().to_string_lossy());
-
-                                    name_a.cmp(&name_b)
-                                })
-                            });
-                            if !ordering.is_eq() {
-                                return ordering;
-                            }
-                        }
-                        (Some(_), None) => break Ordering::Greater,
-                        (None, Some(_)) => break Ordering::Less,
-                        (None, None) => break Ordering::Equal,
-                    }
-                }
-            });
+            project::sort_worktree_entries(&mut visible_worktree_entries);
             self.visible_entries
                 .push((worktree_id, visible_worktree_entries));
         }