added another handler for sorting

Co-authored-by: Smit Barmase <heysmitbarmase@gmail.com>

Cargo.lock

@@ -12149,6 +12149,7 @@ dependencies = [
  "menu",
  "pretty_assertions",
  "project",
+ "rayon",
  "schemars 1.0.1",
  "search",
  "serde",

crates/project/src/project.rs

@@ -121,7 +121,7 @@ use text::{Anchor, BufferId, OffsetRangeExt, Point, Rope};
 use toolchain_store::EmptyToolchainStore;
 use util::{
     ResultExt as _, maybe,
-    paths::{PathStyle, SanitizedPath, compare_paths, is_absolute},
+    paths::{PathStyle, SanitizedPath, compare_paths, compare_rel_paths, is_absolute},
     rel_path::RelPath,
 };
 use worktree::{CreatedEntry, Snapshot, Traversal};
@@ -4041,10 +4041,7 @@ impl Project {
             (None, None) => a.read(cx).remote_id().cmp(&b.read(cx).remote_id()),
             (None, Some(_)) => std::cmp::Ordering::Less,
             (Some(_), None) => std::cmp::Ordering::Greater,
-            (Some(a), Some(b)) => compare_paths(
-                (a.path().as_std_path(), true),
-                (b.path().as_std_path(), true),
-            ),
+            (Some(a), Some(b)) => compare_rel_paths((&a.path(), true), (&b.path(), true)),
         });
         for buffer in buffers {
             tx.send_blocking(buffer.clone()).unwrap()
@@ -5539,17 +5536,6 @@ impl Completion {
     }
 }
 
-pub fn sort_worktree_entries(entries: &mut [impl AsRef<Entry>]) {
-    entries.sort_by(|entry_a, entry_b| {
-        let entry_a = entry_a.as_ref();
-        let entry_b = entry_b.as_ref();
-        compare_paths(
-            (entry_a.path.as_std_path(), entry_a.is_file()),
-            (entry_b.path.as_std_path(), entry_b.is_file()),
-        )
-    });
-}
-
 fn proto_to_prompt(level: proto::language_server_prompt_request::Level) -> gpui::PromptLevel {
     match level {
         proto::language_server_prompt_request::Level::Info(_) => gpui::PromptLevel::Info,

crates/project_panel/Cargo.toml

@@ -32,6 +32,7 @@ serde_json.workspace = true
 settings.workspace = true
 smallvec.workspace = true
 theme.workspace = true
+rayon.workspace = true
 ui.workspace = true
 util.workspace = true
 client.workspace = true

crates/project_panel/src/project_panel_tests.rs

@@ -2030,7 +2030,7 @@ async fn test_create_duplicate_items(cx: &mut gpui::TestAppContext) {
     cx.executor().run_until_parked();
     panel.update_in(cx, |panel, window, cx| {
         assert!(
-            panel.edit_state.is_some(),
+            panel.state.edit_state.is_some(),
             "Edit state should not be None after conflicting new directory name"
         );
         panel.cancel(&menu::Cancel, window, cx);
@@ -2083,7 +2083,7 @@ async fn test_create_duplicate_items(cx: &mut gpui::TestAppContext) {
     cx.executor().run_until_parked();
     panel.update_in(cx, |panel, window, cx| {
         assert!(
-            panel.edit_state.is_some(),
+            panel.state.edit_state.is_some(),
             "Edit state should not be None after conflicting new file name"
         );
         panel.cancel(&menu::Cancel, window, cx);
@@ -2139,7 +2139,7 @@ async fn test_create_duplicate_items(cx: &mut gpui::TestAppContext) {
     cx.executor().run_until_parked();
     panel.update_in(cx, |panel, window, cx| {
         assert!(
-            panel.edit_state.is_some(),
+            panel.state.edit_state.is_some(),
             "Edit state should not be None after conflicting file rename"
         );
         panel.cancel(&menu::Cancel, window, cx);
@@ -3036,7 +3036,7 @@ async fn test_rename_with_hide_root(cx: &mut gpui::TestAppContext) {
             let project = panel.project.read(cx);
             let worktree = project.visible_worktrees(cx).next().unwrap();
             let root_entry = worktree.read(cx).root_entry().unwrap();
-            panel.selection = Some(SelectedEntry {
+            panel.state.selection = Some(SelectedEntry {
                 worktree_id: worktree.read(cx).id(),
                 entry_id: root_entry.id,
             });
@@ -3045,7 +3045,7 @@ async fn test_rename_with_hide_root(cx: &mut gpui::TestAppContext) {
         panel.update_in(cx, |panel, window, cx| panel.rename(&Rename, window, cx));
 
         assert!(
-            panel.read_with(cx, |panel, _| panel.edit_state.is_none()),
+            panel.read_with(cx, |panel, _| panel.state.edit_state.is_none()),
             "Rename should be blocked when hide_root=true with single worktree"
         );
     }
@@ -3074,14 +3074,14 @@ async fn test_rename_with_hide_root(cx: &mut gpui::TestAppContext) {
 
         #[cfg(target_os = "windows")]
         assert!(
-            panel.read_with(cx, |panel, _| panel.edit_state.is_none()),
+            panel.read_with(cx, |panel, _| panel.state.edit_state.is_none()),
             "Rename should be blocked on Windows even with multiple worktrees"
         );
 
         #[cfg(not(target_os = "windows"))]
         {
             assert!(
-                panel.read_with(cx, |panel, _| panel.edit_state.is_some()),
+                panel.read_with(cx, |panel, _| panel.state.edit_state.is_some()),
                 "Rename should work with multiple worktrees on non-Windows when hide_root=true"
             );
             panel.update_in(cx, |panel, window, cx| {
@@ -3172,7 +3172,7 @@ async fn test_multiple_marked_entries(cx: &mut gpui::TestAppContext) {
     cx.update(|window, cx| {
         panel.update(cx, |this, cx| {
             let drag = DraggedSelection {
-                active_selection: this.selection.unwrap(),
+                active_selection: this.state.selection.unwrap(),
                 marked_selections: this.marked_entries.clone().into(),
             };
             let target_entry = this
@@ -3310,10 +3310,10 @@ async fn test_dragged_selection_resolve_entry(cx: &mut gpui::TestAppContext) {
     panel.update_in(cx, |panel, window, cx| {
         let drag = DraggedSelection {
             active_selection: SelectedEntry {
-                worktree_id: panel.selection.as_ref().unwrap().worktree_id,
-                entry_id: panel.resolve_entry(panel.selection.as_ref().unwrap().entry_id),
+                worktree_id: panel.state.selection.as_ref().unwrap().worktree_id,
+                entry_id: panel.resolve_entry(panel.state.selection.as_ref().unwrap().entry_id),
             },
-            marked_selections: Arc::new([*panel.selection.as_ref().unwrap()]),
+            marked_selections: Arc::new([*panel.state.selection.as_ref().unwrap()]),
         };
         let target_entry = panel
             .project
@@ -3343,10 +3343,10 @@ async fn test_dragged_selection_resolve_entry(cx: &mut gpui::TestAppContext) {
     panel.update_in(cx, |panel, window, cx| {
         let drag = DraggedSelection {
             active_selection: SelectedEntry {
-                worktree_id: panel.selection.as_ref().unwrap().worktree_id,
-                entry_id: panel.resolve_entry(panel.selection.as_ref().unwrap().entry_id),
+                worktree_id: panel.state.selection.as_ref().unwrap().worktree_id,
+                entry_id: panel.resolve_entry(panel.state.selection.as_ref().unwrap().entry_id),
             },
-            marked_selections: Arc::new([*panel.selection.as_ref().unwrap()]),
+            marked_selections: Arc::new([*panel.state.selection.as_ref().unwrap()]),
         };
         let target_entry = panel
             .project
@@ -3366,10 +3366,10 @@ async fn test_dragged_selection_resolve_entry(cx: &mut gpui::TestAppContext) {
     panel.update_in(cx, |panel, window, cx| {
         let drag = DraggedSelection {
             active_selection: SelectedEntry {
-                worktree_id: panel.selection.as_ref().unwrap().worktree_id,
-                entry_id: panel.resolve_entry(panel.selection.as_ref().unwrap().entry_id),
+                worktree_id: panel.state.selection.as_ref().unwrap().worktree_id,
+                entry_id: panel.resolve_entry(panel.state.selection.as_ref().unwrap().entry_id),
             },
-            marked_selections: Arc::new([*panel.selection.as_ref().unwrap()]),
+            marked_selections: Arc::new([*panel.state.selection.as_ref().unwrap()]),
         };
         let target_entry = panel
             .project
@@ -3395,10 +3395,10 @@ async fn test_dragged_selection_resolve_entry(cx: &mut gpui::TestAppContext) {
     panel.update_in(cx, |panel, window, cx| {
         let drag = DraggedSelection {
             active_selection: SelectedEntry {
-                worktree_id: panel.selection.as_ref().unwrap().worktree_id,
-                entry_id: panel.resolve_entry(panel.selection.as_ref().unwrap().entry_id),
+                worktree_id: panel.state.selection.as_ref().unwrap().worktree_id,
+                entry_id: panel.resolve_entry(panel.state.selection.as_ref().unwrap().entry_id),
             },
-            marked_selections: Arc::new([*panel.selection.as_ref().unwrap()]),
+            marked_selections: Arc::new([*panel.state.selection.as_ref().unwrap()]),
         };
         let target_entry = panel
             .project
@@ -3418,10 +3418,10 @@ async fn test_dragged_selection_resolve_entry(cx: &mut gpui::TestAppContext) {
     panel.update_in(cx, |panel, window, cx| {
         let drag = DraggedSelection {
             active_selection: SelectedEntry {
-                worktree_id: panel.selection.as_ref().unwrap().worktree_id,
-                entry_id: panel.resolve_entry(panel.selection.as_ref().unwrap().entry_id),
+                worktree_id: panel.state.selection.as_ref().unwrap().worktree_id,
+                entry_id: panel.resolve_entry(panel.state.selection.as_ref().unwrap().entry_id),
             },
-            marked_selections: Arc::new([*panel.selection.as_ref().unwrap()]),
+            marked_selections: Arc::new([*panel.state.selection.as_ref().unwrap()]),
         };
         let target_entry = panel
             .project
@@ -5632,7 +5632,7 @@ async fn test_create_entries_without_selection_hide_root(cx: &mut gpui::TestAppC
 
     panel.update(cx, |panel, _| {
         assert!(
-            panel.selection.is_none(),
+            panel.state.selection.is_none(),
             "Should have no selection initially"
         );
     });
@@ -5682,7 +5682,7 @@ async fn test_create_entries_without_selection_hide_root(cx: &mut gpui::TestAppC
 
     // Test 2: Create new directory when no entry is selected
     panel.update(cx, |panel, _| {
-        panel.selection = None;
+        panel.state.selection = None;
     });
 
     panel.update_in(cx, |panel, window, cx| {
@@ -6545,7 +6545,7 @@ fn select_path(panel: &Entity<ProjectPanel>, path: &str, cx: &mut VisualTestCont
             let worktree = worktree.read(cx);
             if let Ok(relative_path) = path.strip_prefix(worktree.root_name()) {
                 let entry_id = worktree.entry_for_path(relative_path).unwrap().id;
-                panel.selection = Some(crate::SelectedEntry {
+                panel.state.selection = Some(crate::SelectedEntry {
                     worktree_id: worktree.id(),
                     entry_id,
                 });
@@ -6570,7 +6570,7 @@ fn select_path_with_mark(panel: &Entity<ProjectPanel>, path: &str, cx: &mut Visu
                 if !panel.marked_entries.contains(&entry) {
                     panel.marked_entries.push(entry);
                 }
-                panel.selection = Some(entry);
+                panel.state.selection = Some(entry);
                 return;
             }
         }

crates/util/src/paths.rs

@@ -3,8 +3,10 @@ use regex::Regex;
 use serde::{Deserialize, Serialize};
 use std::cmp::Ordering;
 use std::fmt::{Display, Formatter};
+use std::iter::Peekable;
 use std::mem;
 use std::path::StripPrefixError;
+use std::str::Chars;
 use std::sync::{Arc, OnceLock};
 use std::{
     ffi::OsStr,
@@ -12,6 +14,8 @@ use std::{
     sync::LazyLock,
 };
 
+use crate::rel_path::RelPath;
+
 static HOME_DIR: OnceLock<PathBuf> = OnceLock::new();
 
 /// Returns the path to the user's home directory.
@@ -682,56 +686,54 @@ fn compare_chars(a: char, b: char) -> Ordering {
 ///
 /// The function advances both iterators past their respective numeric sequences,
 /// regardless of the comparison result.
-fn compare_numeric_segments<I>(
-    a_iter: &mut std::iter::Peekable<I>,
-    b_iter: &mut std::iter::Peekable<I>,
-) -> Ordering
-where
-    I: Iterator<Item = char>,
-{
+fn compare_numeric_segments(lhs: &mut Chars<'_>, rhs: &mut Chars<'_>) -> Ordering {
     // Collect all consecutive digits into strings
-    let mut a_num_str = String::new();
-    let mut b_num_str = String::new();
+    let lhs_bytes = lhs.as_str().as_bytes();
+    let rhs_bytes = rhs.as_str().as_bytes();
 
-    while let Some(&c) = a_iter.peek() {
-        if !c.is_ascii_digit() {
-            break;
-        }
+    let lhs_digits_len = lhs_bytes
+        .iter()
+        .position(|c| !c.is_ascii_digit())
+        .unwrap_or(lhs_bytes.len());
+    let rhs_digits_len = rhs_bytes
+        .iter()
+        .position(|c| !c.is_ascii_digit())
+        .unwrap_or(rhs_bytes.len());
+    let lhs_digits = &lhs_bytes[..lhs_digits_len];
+    let rhs_digits = &rhs_bytes[..rhs_digits_len];
 
-        a_num_str.push(c);
-        a_iter.next();
-    }
-
-    while let Some(&c) = b_iter.peek() {
-        if !c.is_ascii_digit() {
-            break;
-        }
-
-        b_num_str.push(c);
-        b_iter.next();
-    }
+    // Move the iterator forward to compensate for our reading. All that we read
+    // is single byte characters, so this is ok.
+    let _ = lhs.nth(lhs_digits_len - 1);
+    let _ = rhs.nth(rhs_digits_len - 1);
 
     // First compare lengths (handle leading zeros)
-    match a_num_str.len().cmp(&b_num_str.len()) {
+    match lhs_digits_len.cmp(&rhs_digits_len) {
         Ordering::Equal => {
             // Same length, compare digit by digit
-            match a_num_str.cmp(&b_num_str) {
-                Ordering::Equal => Ordering::Equal,
-                ordering => ordering,
-            }
+            lhs_digits.cmp(&rhs_digits)
         }
 
         // Different lengths but same value means leading zeros
         ordering => {
             // Try parsing as numbers first
-            if let (Ok(a_val), Ok(b_val)) = (a_num_str.parse::<u128>(), b_num_str.parse::<u128>()) {
+            // SAFETY: We're reinterpreting a byte slice that we know is entirely
+            // ascii digits and therefore valid utf-8.
+            let (lhs_digits, rhs_digits) = unsafe {
+                (
+                    str::from_utf8_unchecked(lhs_digits),
+                    str::from_utf8_unchecked(rhs_digits),
+                )
+            };
+            if let (Ok(a_val), Ok(b_val)) = (lhs_digits.parse::<u128>(), rhs_digits.parse::<u128>())
+            {
                 match a_val.cmp(&b_val) {
                     Ordering::Equal => ordering, // Same value, longer one is greater (leading zeros)
                     ord => ord,
                 }
             } else {
                 // If parsing fails (overflow), compare as strings
-                a_num_str.cmp(&b_num_str)
+                lhs_digits.cmp(&rhs_digits)
             }
         }
     }
@@ -757,30 +759,36 @@ where
 /// 3. Comparing numbers by their numeric value rather than lexicographically
 /// 4. For non-numeric characters, using case-sensitive comparison with lowercase priority
 fn natural_sort(a: &str, b: &str) -> Ordering {
-    let mut a_iter = a.chars().peekable();
-    let mut b_iter = b.chars().peekable();
+    // We want to operate cheaply on the underlying byte slice, so don't make this
+    // peekable (as we want to use `as_str`/`as_bytes` of Chars iter).
+    let mut a_iter = a.chars().into_iter();
+    let mut b_iter = b.chars().into_iter();
 
     loop {
-        match (a_iter.peek(), b_iter.peek()) {
-            (None, None) => return Ordering::Equal,
-            (None, _) => return Ordering::Less,
-            (_, None) => return Ordering::Greater,
-            (Some(&a_char), Some(&b_char)) => {
+        match (
+            // Should be ~free since it's just infallibly reinterpreting the value
+            a_iter.as_str().as_bytes().first(),
+            b_iter.as_str().as_bytes().first(),
+        ) {
+            (Some(a_char), Some(b_char)) => {
                 if a_char.is_ascii_digit() && b_char.is_ascii_digit() {
                     match compare_numeric_segments(&mut a_iter, &mut b_iter) {
                         Ordering::Equal => continue,
                         ordering => return ordering,
                     }
                 } else {
+                    // This could be unchecked
+                    let a_char = a_iter.next().unwrap();
+                    let b_char = b_iter.next().unwrap();
                     match compare_chars(a_char, b_char) {
                         Ordering::Equal => {
-                            a_iter.next();
-                            b_iter.next();
+                            continue;
                         }
                         ordering => return ordering,
                     }
                 }
             }
+            (lhs, rhs) => return lhs.cmp(&rhs),
         }
     }
 }
@@ -844,6 +852,65 @@ pub fn compare_paths(
     }
 }
 
+pub fn compare_rel_paths(
+    (path_a, a_is_file): (&RelPath, bool),
+    (path_b, b_is_file): (&RelPath, bool),
+) -> 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 path_a = Path::new(component_a);
+                    let path_string_a = if a_is_file {
+                        path_a.file_stem()
+                    } else {
+                        path_a.file_name()
+                    }
+                    .map(|s| s.to_string_lossy());
+
+                    let path_b = Path::new(component_b);
+                    let path_string_b = if b_is_file {
+                        path_b.file_stem()
+                    } else {
+                        path_b.file_name()
+                    }
+                    .map(|s| s.to_string_lossy());
+
+                    let compare_components = match (path_string_a, path_string_b) {
+                        (Some(a), Some(b)) => natural_sort(&a, &b),
+                        (Some(_), None) => Ordering::Greater,
+                        (None, Some(_)) => Ordering::Less,
+                        (None, None) => Ordering::Equal,
+                    };
+
+                    compare_components.then_with(|| {
+                        if a_is_file && b_is_file {
+                            let ext_a = path_a.extension().unwrap_or_default();
+                            let ext_b = path_b.extension().unwrap_or_default();
+                            ext_a.cmp(ext_b)
+                        } else {
+                            Ordering::Equal
+                        }
+                    })
+                });
+
+                if !ordering.is_eq() {
+                    return ordering;
+                }
+            }
+            (Some(_), None) => break Ordering::Greater,
+            (None, Some(_)) => break Ordering::Less,
+            (None, None) => break Ordering::Equal,
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -1300,8 +1367,8 @@ mod tests {
     fn test_compare_numeric_segments() {
         // Helper function to create peekable iterators and test
         fn compare(a: &str, b: &str) -> Ordering {
-            let mut a_iter = a.chars().peekable();
-            let mut b_iter = b.chars().peekable();
+            let mut a_iter = a.chars();
+            let mut b_iter = b.chars();
 
             let result = compare_numeric_segments(&mut a_iter, &mut b_iter);
 
@@ -1355,8 +1422,8 @@ mod tests {
         );
 
         // Iterator advancement verification
-        let mut a_iter = "123abc".chars().peekable();
-        let mut b_iter = "456def".chars().peekable();
+        let mut a_iter = "123abc".chars();
+        let mut b_iter = "456def".chars();
 
         compare_numeric_segments(&mut a_iter, &mut b_iter);