From a8481099ca9d2f294d8179a9b7fe7ead8a117f40 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 14 Jun 2024 18:33:36 +0300 Subject: [PATCH] Prefer the same order of entries inside outline and project panels, project search multi buffer (#13044) Release Notes: - N/A --- 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(-) diff --git a/Cargo.lock b/Cargo.lock index 74bf2c5100..3aeb7dede6 100644 --- a/Cargo.lock +++ b/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", diff --git a/crates/multi_buffer/src/multi_buffer.rs b/crates/multi_buffer/src/multi_buffer.rs index a51133cb25..ff05aac6e8 100644 --- a/crates/multi_buffer/src/multi_buffer.rs +++ b/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() { diff --git a/crates/outline_panel/Cargo.toml b/crates/outline_panel/Cargo.toml index ed510ac7ff..9dfdba92c5 100644 --- a/crates/outline_panel/Cargo.toml +++ b/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 diff --git a/crates/outline_panel/src/outline_panel.rs b/crates/outline_panel/src/outline_panel.rs index 7b068d85ff..aae355a697 100644 --- a/crates/outline_panel/src/outline_panel.rs +++ b/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::>(); - 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) { - 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 { diff --git a/crates/project/Cargo.toml b/crates/project/Cargo.toml index a35d2f73c4..fe8a7b444f 100644 --- a/crates/project/Cargo.toml +++ b/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] diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 8a57ae30f2..4f07ef2e50 100644 --- a/crates/project/src/project.rs +++ b/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, }, } -impl SearchMatchCandidate { - fn path(&self) -> Option> { - 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, @@ -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) { + 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, 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, + } + } +} diff --git a/crates/project_panel/Cargo.toml b/crates/project_panel/Cargo.toml index 33b57bbf33..5ce14b43e1 100644 --- a/crates/project_panel/Cargo.toml +++ b/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 diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 417af04575..0ca5b6c595 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/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)); }