From d6d9c383cbc9310ae8f659ddaa4f9285db2f1b2a Mon Sep 17 00:00:00 2001 From: Smit Barmase Date: Mon, 7 Apr 2025 08:14:22 +0530 Subject: [PATCH] project_panel: Show error when file or directory already exists while renaming or creating new one (#28177) Closes #14425 image Release Notes: - Improved the project panel to show an error when a file or directory already exists while renaming or creating a new one. --- crates/project_panel/src/project_panel.rs | 127 ++++++++++++++++-- .../project_panel/src/project_panel_tests.rs | 32 ++++- 2 files changed, 143 insertions(+), 16 deletions(-) diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index ca18b417fbd7ab7e4a921f881570eb7c8816525e..641f48508c66f47bf117882a87787c899faea95a 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -22,7 +22,7 @@ use gpui::{ Hsla, InteractiveElement, KeyContext, ListHorizontalSizingBehavior, ListSizingBehavior, MouseButton, MouseDownEvent, ParentElement, Pixels, Point, PromptLevel, Render, ScrollStrategy, Stateful, Styled, Subscription, Task, UniformListScrollHandle, WeakEntity, Window, actions, - anchored, deferred, div, impl_actions, point, px, size, uniform_list, + anchored, deferred, div, impl_actions, point, px, size, transparent_black, uniform_list, }; use indexmap::IndexMap; use language::DiagnosticSeverity; @@ -53,9 +53,9 @@ use std::{ }; use theme::ThemeSettings; use ui::{ - ContextMenu, DecoratedIcon, Icon, IconDecoration, IconDecorationKind, IndentGuideColors, - IndentGuideLayout, KeyBinding, Label, ListItem, ListItemSpacing, Scrollbar, ScrollbarState, - Tooltip, prelude::*, v_flex, + Color, ContextMenu, DecoratedIcon, Icon, IconDecoration, IconDecorationKind, IndentGuideColors, + IndentGuideLayout, KeyBinding, Label, LabelSize, ListItem, ListItemSpacing, Scrollbar, + ScrollbarState, Tooltip, prelude::*, v_flex, }; use util::{ResultExt, TakeUntilExt, TryFutureExt, maybe, paths::compare_paths}; use workspace::{ @@ -128,6 +128,7 @@ struct EditState { depth: usize, processing_filename: Option, previously_focused: Option, + validation_error: bool, } impl EditState { @@ -408,7 +409,11 @@ impl ProjectPanel { cx.subscribe( &filename_editor, |project_panel, _, editor_event, cx| match editor_event { - EditorEvent::BufferEdited | EditorEvent::SelectionsChanged { .. } => { + EditorEvent::BufferEdited => { + project_panel.populate_validation_error(cx); + project_panel.autoscroll(cx); + } + EditorEvent::SelectionsChanged { .. } => { project_panel.autoscroll(cx); } EditorEvent::Blurred => { @@ -1133,14 +1138,58 @@ impl ProjectPanel { } } + fn populate_validation_error(&mut self, cx: &mut Context) { + let edit_state = match self.edit_state.as_mut() { + Some(state) => state, + None => return, + }; + let filename = self.filename_editor.read(cx).text(cx); + if !filename.is_empty() { + if let Some(worktree) = self + .project + .read(cx) + .worktree_for_id(edit_state.worktree_id, cx) + { + if let Some(entry) = worktree.read(cx).entry_for_id(edit_state.entry_id) { + if edit_state.is_new_entry() { + let new_path = entry.path.join(filename.trim_start_matches('/')); + if worktree + .read(cx) + .entry_for_path(new_path.as_path()) + .is_some() + { + edit_state.validation_error = true; + cx.notify(); + return; + } + } else { + let new_path = if let Some(parent) = entry.path.clone().parent() { + parent.join(&filename) + } else { + filename.clone().into() + }; + if let Some(existing) = worktree.read(cx).entry_for_path(new_path.as_path()) + { + if existing.id != entry.id { + edit_state.validation_error = true; + cx.notify(); + return; + } + } + }; + } + } + } + edit_state.validation_error = false; + cx.notify(); + } + fn confirm_edit( &mut self, window: &mut Window, cx: &mut Context, ) -> Option>> { let edit_state = self.edit_state.as_mut()?; - window.focus(&self.focus_handle); - let worktree_id = edit_state.worktree_id; let is_new_entry = edit_state.is_new_entry(); let filename = self.filename_editor.read(cx).text(cx); @@ -1155,7 +1204,6 @@ impl ProjectPanel { let worktree = self.project.read(cx).worktree_for_id(worktree_id, cx)?; let entry = worktree.read(cx).entry_for_id(edit_state.entry_id)?.clone(); - let path_already_exists = |path| worktree.read(cx).entry_for_path(path).is_some(); let edit_task; let edited_entry_id; if is_new_entry { @@ -1164,7 +1212,11 @@ impl ProjectPanel { entry_id: NEW_ENTRY_ID, }); let new_path = entry.path.join(filename.trim_start_matches('/')); - if path_already_exists(new_path.as_path()) { + if worktree + .read(cx) + .entry_for_path(new_path.as_path()) + .is_some() + { return None; } @@ -1178,7 +1230,10 @@ impl ProjectPanel { } else { filename.clone().into() }; - if path_already_exists(new_path.as_path()) { + if let Some(existing) = worktree.read(cx).entry_for_path(new_path.as_path()) { + if existing.id == entry.id { + window.focus(&self.focus_handle); + } return None; } edited_entry_id = entry.id; @@ -1187,6 +1242,7 @@ impl ProjectPanel { }); }; + window.focus(&self.focus_handle); edit_state.processing_filename = Some(filename); cx.notify(); @@ -1347,6 +1403,7 @@ impl ProjectPanel { processing_filename: None, previously_focused: self.selection, depth: 0, + validation_error: false, }); self.filename_editor.update(cx, |editor, cx| { editor.clear(window, cx); @@ -1396,6 +1453,7 @@ impl ProjectPanel { processing_filename: None, previously_focused: None, depth: 0, + validation_error: false, }); let file_name = entry .path @@ -3629,16 +3687,27 @@ impl ProjectPanel { item_colors.hover }; + let validation_error = + show_editor && self.edit_state.as_ref().is_some_and(|e| e.validation_error); + let border_color = if !self.mouse_down && is_active && self.focus_handle.contains_focused(window, cx) { - item_colors.focused + if validation_error { + Color::Error.color(cx) + } else { + item_colors.focused + } } else { bg_color }; let border_hover_color = if !self.mouse_down && is_active && self.focus_handle.contains_focused(window, cx) { - item_colors.focused + if validation_error { + Color::Error.color(cx) + } else { + item_colors.focused + } } else { bg_hover_color }; @@ -4108,6 +4177,40 @@ impl ProjectPanel { )) .overflow_x(), ) + .when( + + validation_error, |el| { + el + .relative() + .child( + deferred( + // Wizardry of highest order to make error border align with entry border + div() + .occlude() + .absolute() + .top_full() + .left_neg_0p5() + .right_neg_1() + .border_x_1() + .border_color(transparent_black()) + .child( + div() + .py_1() + .px_2() + .border_1() + .border_color(Color::Error.color(cx)) + .bg(cx.theme().colors().background) + .child( + Label::new(format!("{} already exists", self.filename_editor.read(cx).text(cx))) + .color(Color::Error) + .size(LabelSize::Small) + ) + ) + + ) + ) + } + ) } fn render_vertical_scrollbar(&self, cx: &mut Context) -> Option> { diff --git a/crates/project_panel/src/project_panel_tests.rs b/crates/project_panel/src/project_panel_tests.rs index 851a385e0db3fb8ab34fa85bd99cafca76208e2a..a290e8c34a252c4b61bc720bc7191ddd791075ee 100644 --- a/crates/project_panel/src/project_panel_tests.rs +++ b/crates/project_panel/src/project_panel_tests.rs @@ -1835,13 +1835,21 @@ async fn test_create_duplicate_items(cx: &mut gpui::TestAppContext) { assert!( panel.confirm_edit(window, cx).is_none(), "Should not allow to confirm on conflicting new directory name" - ) + ); + }); + cx.executor().run_until_parked(); + panel.update_in(cx, |panel, window, cx| { + assert!( + panel.edit_state.is_some(), + "Edit state should not be None after conflicting new directory name" + ); + panel.cancel(&menu::Cancel, window, cx); }); assert_eq!( visible_entries_as_strings(&panel, 0..10, cx), &[ // - "v src", + "v src <== selected", " > test" ], "File list should be unchanged after failed folder create confirmation" @@ -1880,13 +1888,21 @@ async fn test_create_duplicate_items(cx: &mut gpui::TestAppContext) { assert!( panel.confirm_edit(window, cx).is_none(), "Should not allow to confirm on conflicting new file name" - ) + ); + }); + cx.executor().run_until_parked(); + panel.update_in(cx, |panel, window, cx| { + assert!( + panel.edit_state.is_some(), + "Edit state should not be None after conflicting new file name" + ); + panel.cancel(&menu::Cancel, window, cx); }); assert_eq!( visible_entries_as_strings(&panel, 0..10, cx), &[ "v src", - " v test", + " v test <== selected", " first.rs", " second.rs", " third.rs" @@ -1930,6 +1946,14 @@ async fn test_create_duplicate_items(cx: &mut gpui::TestAppContext) { "Should not allow to confirm on conflicting file rename" ) }); + cx.executor().run_until_parked(); + panel.update_in(cx, |panel, window, cx| { + assert!( + panel.edit_state.is_some(), + "Edit state should not be None after conflicting file rename" + ); + panel.cancel(&menu::Cancel, window, cx); + }); assert_eq!( visible_entries_as_strings(&panel, 0..10, cx), &[