From 0849124bc2589d8d7b463e15db0b6a4abde746a0 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 6 Oct 2026 08:13:27 +0000 Subject: [PATCH] Ensure more snapshots are used after .await points (#65219) Follow-up to https://github.com/zed-industries/zed/pull/65193 Release Notes: - N/A --- crates/agent/src/tools/edit_file_tool.rs | 99 +++++++ crates/agent/src/tools/edit_session.rs | 17 +- .../edit_session/streaming_fuzzy_matcher.rs | 10 + crates/copilot/src/copilot.rs | 78 ++++- crates/editor/src/editor_tests.rs | 166 ++++++++++- crates/editor/src/navigation.rs | 18 +- crates/git_ui/src/text_diff_view.rs | 130 ++++++++- crates/language/src/buffer.rs | 49 +++- crates/multi_buffer/src/multi_buffer_tests.rs | 62 ++++ crates/multi_buffer/src/path_key.rs | 3 +- crates/project/src/git_store.rs | 181 ++++++++++-- crates/project/src/lsp_store.rs | 121 +++++--- crates/vim/src/command.rs | 245 +++++++++++++--- crates/worktree/src/worktree.rs | 275 +++++++++++++++++- 14 files changed, 1304 insertions(+), 150 deletions(-) diff --git a/crates/agent/src/tools/edit_file_tool.rs b/crates/agent/src/tools/edit_file_tool.rs index e645273d799..b46e5662d51 100644 --- a/crates/agent/src/tools/edit_file_tool.rs +++ b/crates/agent/src/tools/edit_file_tool.rs @@ -916,6 +916,105 @@ mod tests { ); } + #[gpui::test] + async fn test_streaming_edit_refreshes_match_after_buffer_change(cx: &mut TestAppContext) { + for (content, old_text, new_text, range, replacement, expected) in [ + ( + "prefix\nold value\nsuffix\n", + "old value", + "new value", + 0..0, + "馃榾\n", + "馃榾\nprefix\nnew value\nsuffix\n", + ), + ( + "prefix\nold value\nsuffix\n", + "old value", + "new value", + 0..7, + "", + "new value\nsuffix\n", + ), + ( + "prefix\n old value\n nested\nsuffix\n", + "old value\n nested", + "new value\n updated", + 0..7, + "馃榾\n\n", + "馃榾\n\n new value\n updated\nsuffix\n", + ), + ] { + let (edit_tool, project, _action_log, _fs, _thread) = + setup_test(cx, json!({"file.txt": content})).await; + let (mut sender, input) = ToolInput::::test(); + let (event_stream, _receiver) = ToolCallEventStream::test(); + let task = cx.update(|cx| edit_tool.clone().run(input, event_stream, cx)); + sender.send_partial(json!({"path": "root/file.txt"})); + cx.run_until_parked(); + sender.send_partial(json!({ + "path": "root/file.txt", + "edits": [{"old_text": old_text.split_inclusive('\n').next().unwrap()}] + })); + cx.run_until_parked(); + + let buffer = project.read_with(cx, |project, cx| { + let path = project.find_project_path("root/file.txt", cx).unwrap(); + project.get_open_buffer(&path, cx).unwrap() + }); + buffer.update(cx, |buffer, cx| { + buffer.edit([(range, replacement)], None, cx) + }); + sender.send_full(json!({ + "path": "root/file.txt", + "edits": [{"old_text": old_text, "new_text": new_text}] + })); + let EditFileToolOutput::Success { new_text, .. } = task.await.unwrap() else { + panic!("expected success"); + }; + assert_eq!(new_text, expected); + assert_eq!(buffer.read_with(cx, |buffer, _| buffer.text()), expected); + } + } + + #[gpui::test] + async fn test_streaming_edit_detects_new_ambiguous_match(cx: &mut TestAppContext) { + let (edit_tool, project, _action_log, _fs, _thread) = + setup_test(cx, json!({"file.txt": "old value\n"})).await; + let (mut sender, input) = ToolInput::::test(); + let (event_stream, _receiver) = ToolCallEventStream::test(); + let task = cx.update(|cx| edit_tool.clone().run(input, event_stream, cx)); + sender.send_partial(json!({"path": "root/file.txt"})); + cx.run_until_parked(); + sender.send_partial(json!({ + "path": "root/file.txt", + "edits": [{"old_text": "old "}] + })); + cx.run_until_parked(); + + let buffer = project.read_with(cx, |project, cx| { + let path = project.find_project_path("root/file.txt", cx).unwrap(); + project.get_open_buffer(&path, cx).unwrap() + }); + buffer.update(cx, |buffer, cx| { + buffer.edit([(0..0, "old value\n")], None, cx) + }); + sender.send_full(json!({ + "path": "root/file.txt", + "edits": [{"old_text": "old value", "new_text": "new value"}] + })); + let EditFileToolOutput::Error { error, .. } = task.await.unwrap_err() else { + panic!("expected ambiguous match error"); + }; + assert_eq!( + error, + "Edit 0 matched multiple locations in the file at lines: 1, 2. Please provide more context in old_text to uniquely identify the location." + ); + assert_eq!( + buffer.read_with(cx, |buffer, _| buffer.text()), + "old value\nold value\n" + ); + } + #[gpui::test] async fn test_streaming_no_partials_direct_final(cx: &mut TestAppContext) { let (edit_tool, _project, _action_log, _fs, _thread) = diff --git a/crates/agent/src/tools/edit_session.rs b/crates/agent/src/tools/edit_session.rs index f96f84c7f68..4d0d77b1d27 100644 --- a/crates/agent/src/tools/edit_session.rs +++ b/crates/agent/src/tools/edit_session.rs @@ -448,11 +448,18 @@ impl WritePipeline { impl EditPipeline { fn ensure_resolving_old_text(&mut self, buffer: &Entity, cx: &mut AsyncApp) { - if self.current_edit.is_none() { - let snapshot = buffer.read_with(cx, |buffer, _cx| buffer.text_snapshot()); - self.current_edit = Some(EditPipelineEntry::ResolvingOldText { - matcher: StreamingFuzzyMatcher::new(snapshot), - }); + match &mut self.current_edit { + None => { + let snapshot = buffer.read_with(cx, |buffer, _cx| buffer.text_snapshot()); + self.current_edit = Some(EditPipelineEntry::ResolvingOldText { + matcher: StreamingFuzzyMatcher::new(snapshot), + }); + } + Some(EditPipelineEntry::ResolvingOldText { matcher }) => { + let snapshot = buffer.read_with(cx, |buffer, _cx| buffer.text_snapshot()); + matcher.refresh(snapshot); + } + Some(EditPipelineEntry::StreamingNewText { .. }) => {} } } diff --git a/crates/agent/src/tools/edit_session/streaming_fuzzy_matcher.rs b/crates/agent/src/tools/edit_session/streaming_fuzzy_matcher.rs index 321fb1c3530..f99fcc1f962 100644 --- a/crates/agent/src/tools/edit_session/streaming_fuzzy_matcher.rs +++ b/crates/agent/src/tools/edit_session/streaming_fuzzy_matcher.rs @@ -143,6 +143,16 @@ impl StreamingFuzzyMatcher { SearchMatches::Fuzzy(self.matches.clone()) } + pub(super) fn refresh(&mut self, snapshot: TextBufferSnapshot) { + if self.snapshot.version == snapshot.version { + return; + } + let query = std::mem::take(&mut self.raw_query); + let line_hint = self.line_hint; + *self = Self::new(snapshot); + self.push(&query, line_hint); + } + fn resolve_location_fuzzy(&mut self) -> Vec { let new_query_line_count = self.query_lines.len(); let old_query_line_count = self.matrix.rows.saturating_sub(1); diff --git a/crates/copilot/src/copilot.rs b/crates/copilot/src/copilot.rs index 62664980de1..8491c4781b1 100644 --- a/crates/copilot/src/copilot.rs +++ b/crates/copilot/src/copilot.rs @@ -520,12 +520,20 @@ impl Copilot { #[cfg(any(test, feature = "test-support"))] pub fn fake(cx: &mut gpui::TestAppContext) -> (Entity, lsp::FakeLanguageServer) { + Self::fake_with_initializer(cx, |_| {}) + } + + #[cfg(any(test, feature = "test-support"))] + pub fn fake_with_initializer( + cx: &mut gpui::TestAppContext, + initializer: impl FnOnce(&mut lsp::FakeLanguageServer), + ) -> (Entity, lsp::FakeLanguageServer) { use fs::FakeFs; use gpui::Subscription; use lsp::FakeLanguageServer; use node_runtime::NodeRuntime; - let (server, fake_server) = FakeLanguageServer::new( + let (server, mut fake_server) = FakeLanguageServer::new( LanguageServerId(0), LanguageServerBinary { path: "path/to/copilot".into(), @@ -536,6 +544,7 @@ impl Copilot { Default::default(), &mut cx.to_async(), ); + initializer(&mut fake_server); let node_runtime = NodeRuntime::unavailable(); let send_focus_notification = Subscription::new(|| {}); let this = cx.new(|cx| Self { @@ -1031,7 +1040,6 @@ impl Copilot { let pending_snapshot = registered_buffer.report_changes(buffer, cx); let buffer = buffer.read(cx); let uri = registered_buffer.uri.clone(); - let position = position.to_point_utf16(buffer); let snapshot = buffer.snapshot(); let settings = snapshot.settings_at(0, cx); let tab_size = settings.tab_size.get(); @@ -1050,7 +1058,7 @@ impl Copilot { cx.background_spawn(async move { let (version, snapshot) = pending_snapshot.await?; - let lsp_position = point_to_lsp(position); + let lsp_position = point_to_lsp(position.to_point_utf16(&snapshot)); let nes_fut = if nes_enabled { lsp.request::( @@ -1820,6 +1828,70 @@ mod tests { }); } + #[gpui::test] + async fn test_completion_position_tracks_reported_snapshot(cx: &mut TestAppContext) { + init_test(cx); + let positions = Arc::new(Mutex::new(Vec::new())); + let (copilot, _server) = Copilot::fake_with_initializer(cx, |server| { + server.set_request_handler::({ + let positions = positions.clone(); + move |params, _| { + positions + .lock() + .push((params.text_document.version, params.position)); + async { Ok(request::NextEditSuggestionsResult { edits: Vec::new() }) } + } + }); + server.set_request_handler::({ + let positions = positions.clone(); + move |params, _| { + positions + .lock() + .push((params.text_document.version, params.position)); + async { Ok(request::InlineCompletionsResult { items: Vec::new() }) } + } + }); + }); + + for (replacement, expected_position) in [ + ("new\n茅\t", lsp::Position::new(1, 4)), + ("", lsp::Position::new(0, 2)), + ] { + let buffer = cx.new(|cx| Buffer::local("prefix\n馃榾target", cx)); + copilot.update(cx, |copilot, cx| copilot.register_buffer(&buffer, cx)); + let (release, pending) = oneshot::channel::<()>(); + copilot.update(cx, |copilot, cx| { + let registered = copilot + .server + .as_authenticated() + .unwrap() + .registered_buffers + .get_mut(&buffer.entity_id()) + .unwrap(); + registered.pending_buffer_change = + cx.background_spawn(async move { pending.await.ok() }); + }); + let position = buffer.update(cx, |buffer, cx| { + buffer.edit([(buffer.len()..buffer.len(), "!")], None, cx); + buffer.anchor_before("prefix\n馃榾".len()) + }); + let completions = + copilot.update(cx, |copilot, cx| copilot.completions(&buffer, position, cx)); + cx.run_until_parked(); + assert_eq!(positions.lock().len(), 0); + buffer.update(cx, |buffer, cx| { + buffer.edit([(0..7, replacement)], None, cx) + }); + release.send(()).unwrap(); + + assert_eq!(completions.await.unwrap().len(), 0); + assert_eq!( + positions.lock().drain(..).collect::>(), + vec![(1, expected_position); 2] + ); + } + } + fn init_test(cx: &mut TestAppContext) { zlog::init_test(); diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 0995f8fcff5..09465235150 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -18,7 +18,10 @@ use crate::{ use buffer_diff::{BufferDiff, DiffHunkSecondaryStatus, DiffHunkStatus, DiffHunkStatusKind}; use collections::{HashMap, HashSet}; use fs::Fs as _; -use futures::{StreamExt, channel::oneshot}; +use futures::{ + StreamExt, + channel::{mpsc, oneshot}, +}; use gpui::{ BackgroundExecutor, DismissEvent, Task, TaskExt, TestAppContext, UpdateGlobal, VisualTestContext, WindowBounds, WindowOptions, div, @@ -28092,6 +28095,82 @@ fn test_split_words_for_snippet_prefix() { assert_eq!(split("a.s"), &["s", ".s", "a.s"]); } +#[gpui::test] +async fn test_go_to_symbol_after_edit(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + update_test_language_settings(cx, &|settings| { + settings.defaults.document_symbols = Some(settings::DocumentSymbols::On); + }); + + for direction in [-1, 1] { + let editor = cx.add_window(|window, cx| { + let buffer = MultiBuffer::build_simple("", cx); + build_editor(buffer, window, cx) + }); + let mut cx = EditorTestContext::for_editor(editor, cx).await; + cx.set_state("padding\n藝\n"); + let (ready, pending) = oneshot::channel(); + let navigation = cx.update_editor(|editor, window, cx| { + editor.refresh_document_symbols_task = cx + .background_spawn(async move { + pending.await.expect("symbol refresh should complete"); + }) + .shared(); + editor.go_to_symbol_by_offset(window, cx, direction) + }); + cx.run_until_parked(); + cx.update_buffer(|buffer, cx| { + let end = buffer.len(); + buffer.edit( + [ + (0..0, "位 padding\nfn before() {}\n"), + (end..end, "fn after() {}\n"), + ], + None, + cx, + ); + }); + cx.update_editor(|editor, _, cx| { + let buffer = editor + .buffer + .read(cx) + .as_singleton() + .expect("singleton buffer"); + let snapshot = buffer.read(cx).snapshot(); + let text = snapshot.text(); + let items = ["fn before", "fn after"] + .into_iter() + .map(|name| { + let start = text.find(name).expect("symbol should be in buffer"); + let range = + snapshot.anchor_after(start)..snapshot.anchor_before(start + name.len()); + language::OutlineItem { + depth: 0, + range: range.clone(), + selection_range: range.clone(), + source_range_for_text: range, + text: SharedString::from(name), + highlight_ranges: Vec::new(), + name_ranges: Vec::new(), + body_range: None, + annotation_range: None, + } + }) + .collect::>(); + editor + .lsp_document_symbols + .insert(snapshot.remote_id(), items); + }); + ready.send(()).expect("symbol refresh should be pending"); + navigation.await.expect("symbol navigation should succeed"); + cx.assert_editor_state(if direction < 0 { + "位 padding\n藝fn before() {}\npadding\n\nfn after() {}\n" + } else { + "位 padding\nfn before() {}\npadding\n\n藝fn after() {}\n" + }); + } +} + #[gpui::test] async fn test_move_to_syntax_node_relative_jumps(tcx: &mut TestAppContext) { init_test(tcx, |_| {}); @@ -34292,6 +34371,91 @@ async fn test_goto_definition_with_find_all_references_fallback(cx: &mut TestApp }); } +#[gpui::test] +async fn test_find_all_references_cleanup_after_edit(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + let (requests, mut responses) = mpsc::unbounded(); + let mut cx = EditorLspTestContext::new_with_adapter( + Arc::into_inner(rust_lang()).expect("Rust language should have a single owner"), + FakeLspAdapter { + capabilities: lsp::ServerCapabilities { + references_provider: Some(lsp::OneOf::Left(true)), + ..lsp::ServerCapabilities::default() + }, + initializer: Some(Box::new(move |server| { + let requests = requests.clone(); + server.set_request_handler::(move |_, _| { + let requests = requests.clone(); + async move { + let (response, pending) = oneshot::channel(); + requests + .unbounded_send(response) + .expect("request receiver should exist"); + pending.await.expect("reference response should arrive"); + Ok(Some(Vec::new())) + } + }); + })), + ..FakeLspAdapter::default() + }, + cx, + ) + .await; + cx.set_state("fn a藝() {}\n"); + let first = cx.update_editor(|editor, window, cx| { + editor + .find_all_references(&FindAllReferences::default(), window, cx) + .expect("first request should start") + }); + let first_response = responses.next().await.expect("first request should arrive"); + cx.update_editor(|editor, window, cx| { + editor.move_to_end(&MoveToEnd, window, cx); + editor.insert("fn b() {}\n", window, cx); + editor.change_selections(SelectionEffects::no_scroll(), window, cx, |selections| { + selections.select_ranges([Point::new(1, 4)..Point::new(1, 4)]); + }); + }); + let second = cx.update_editor(|editor, window, cx| { + editor + .find_all_references(&FindAllReferences::default(), window, cx) + .expect("second request should start") + }); + let second_response = responses + .next() + .await + .expect("second request should arrive"); + first_response + .send(()) + .expect("first request should be pending"); + assert_eq!( + first.await.expect("first request should succeed"), + Navigated::No + ); + cx.run_until_parked(); + cx.update_editor(|editor, _, cx| { + let snapshot = editor.buffer.read(cx).snapshot(cx); + assert_eq!( + editor + .find_all_references_task_sources + .iter() + .map(|anchor| anchor.to_point(&snapshot)) + .collect::>(), + vec![Point::new(1, 4)], + ); + }); + second_response + .send(()) + .expect("second request should be pending"); + assert_eq!( + second.await.expect("second request should succeed"), + Navigated::No + ); + cx.run_until_parked(); + cx.update_editor(|editor, _, _| { + assert_eq!(editor.find_all_references_task_sources, Vec::new()); + }); +} + /// End-to-end regression test for ZED-79W ("cannot summarize backward"): /// language servers can return a location whose range is reversed (start /// after end). `GetReferences::response_from_lsp` used to convert the diff --git a/crates/editor/src/navigation.rs b/crates/editor/src/navigation.rs index c74e150735c..a84ce2b0fe1 100644 --- a/crates/editor/src/navigation.rs +++ b/crates/editor/src/navigation.rs @@ -1409,7 +1409,8 @@ impl Editor { let project = workspace.read(cx).project().clone(); let references = project.update(cx, |project, cx| project.references(&buffer, head, cx)); Some(cx.spawn_in(window, async move |editor, cx| { - let _cleanup = cx.on_drop(&editor, move |editor, _| { + let _cleanup = cx.on_drop(&editor, move |editor, cx| { + let multi_buffer_snapshot = editor.buffer.read(cx).snapshot(cx); if let Ok(i) = editor .find_all_references_task_sources .binary_search_by(|anchor| anchor.cmp(&head_anchor, &multi_buffer_snapshot)) @@ -2068,17 +2069,12 @@ impl Editor { cx: &mut Context, offset: i8, ) -> Task> { - let editor_snapshot = self.snapshot(window, cx); - // We don't care about multi-buffer symbols - if !editor_snapshot.is_singleton() { + if !self.buffer.read(cx).is_singleton() { return Task::ready(Ok(())); } - let cursor_offset = self - .selections - .newest::(&editor_snapshot.display_snapshot) - .head(); + let cursor = self.selections.newest_anchor().head(); cx.spawn_in(window, async move |editor, wcx| -> Result<()> { let Ok(Some(remote_id)) = editor.update(wcx, |ed, cx| { @@ -2091,12 +2087,14 @@ impl Editor { let task = editor.update(wcx, |ed, cx| ed.buffer_outline_items(remote_id, cx))?; let outline_items: Vec> = task.await; - let multi_snapshot = editor_snapshot.buffer(); + let multi_snapshot = + editor.read_with(wcx, |editor, cx| editor.buffer.read(cx).snapshot(cx))?; + let cursor_offset = cursor.to_offset(&multi_snapshot); let buffer_range = |range: &Range<_>| { Some( multi_snapshot .buffer_anchor_range_to_anchor_range(range.clone())? - .to_offset(multi_snapshot), + .to_offset(&multi_snapshot), ) }; diff --git a/crates/git_ui/src/text_diff_view.rs b/crates/git_ui/src/text_diff_view.rs index 5a1f562284d..106ff71b2a8 100644 --- a/crates/git_ui/src/text_diff_view.rs +++ b/crates/git_ui/src/text_diff_view.rs @@ -126,10 +126,14 @@ impl TextDiffView { diff }); + let expanded_selection_range = + source_buffer_snapshot.anchor_range_outside(expanded_selection_range); let task = window.spawn(cx, async move |cx| { update_diff_buffer(&diff_buffer, &source_buffer, &clipboard_buffer, cx).await; workspace.update_in(cx, |workspace, window, cx| { + let source_range = expanded_selection_range.to_point(source_buffer.read(cx)); + let project = workspace.project().clone(); let workspace_entity = cx.entity(); let diff_view = cx.new(|cx| { @@ -137,7 +141,7 @@ impl TextDiffView { clipboard_buffer, source_editor, source_buffer, - expanded_selection_range, + source_range, diff_buffer, project, workspace_entity, @@ -190,7 +194,10 @@ impl TextDiffView { splittable }); - let (buffer_changes_tx, mut buffer_changes_rx) = watch::channel(()); + let (mut buffer_changes_tx, mut buffer_changes_rx) = watch::channel(()); + if diff_buffer.read(cx).snapshot(cx).buffer_version() != &source_buffer.read(cx).version() { + buffer_changes_tx.send(()).ok(); + } cx.subscribe(&source_buffer, move |this, _, event, _| match event { language::BufferEvent::Edited { .. } @@ -849,6 +856,125 @@ mod tests { }); } + #[gpui::test] + async fn test_diffing_clipboard_tracks_selection_after_edit(cx: &mut TestAppContext) { + init_test(cx); + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let workspace = multi_workspace.read_with(cx, |workspace, _| workspace.workspace().clone()); + + for (edit_range, replacement, expected_range) in [ + (0..7, "馃榾\n\tprefix\n", Point::new(2, 0)..Point::new(3, 0)), + (0..7, "", Point::new(0, 0)..Point::new(1, 0)), + (0..21, "", Point::new(0, 0)..Point::new(0, 0)), + (0..21, "\tchanged馃榾\n", Point::new(1, 0)..Point::new(1, 0)), + ] { + let buffer = cx.new(|cx| Buffer::local("prefix\nTARGET\nsuffix\n", cx)); + let editor = cx.new_window_entity(|window, cx| { + let mut editor = Editor::for_buffer(buffer.clone(), None, window, cx); + editor.change_selections(Default::default(), window, cx, |selections| { + selections.select_ranges([MultiBufferOffset(7)..MultiBufferOffset(13)]); + }); + editor + }); + let diff_task = workspace.update_in(cx, |workspace, window, cx| { + let task = TextDiffView::open( + &DiffClipboardWithSelectionData { + clipboard_text: String::from("REPLACED"), + editor, + }, + workspace, + window, + cx, + ) + .expect("open clipboard diff"); + buffer.update(cx, |buffer, cx| { + buffer.edit([(edit_range, replacement)], None, cx); + }); + task + }); + let diff_view = diff_task.await.expect("load clipboard diff"); + diff_view.read_with(cx, |diff_view, cx| { + let editor = diff_view.diff_editor.read(cx).rhs_editor().read(cx); + let snapshot = editor.buffer().read(cx).snapshot(cx); + assert_eq!( + snapshot + .excerpts() + .map(|excerpt| excerpt.primary.to_point(buffer.read(cx))) + .collect::>(), + vec![expected_range], + ); + }); + } + } + + #[gpui::test] + async fn test_diffing_clipboard_refreshes_edits_during_initial_calculation( + cx: &mut TestAppContext, + ) { + init_test(cx); + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let workspace = multi_workspace.read_with(cx, |workspace, _| workspace.workspace().clone()); + let buffer = cx.new(|cx| Buffer::local("TARGET\n", cx)); + let editor = + cx.new_window_entity(|window, cx| Editor::for_buffer(buffer.clone(), None, window, cx)); + let clipboard_buffer = cx.new(|cx| Buffer::local("TARGET\n", cx)); + let diff_buffer = cx.new(|cx| { + BufferDiff::new_with_base_text_buffer( + &buffer.read(cx).text_snapshot(), + clipboard_buffer.clone(), + cx, + ) + }); + { + let mut async_cx = cx.to_async(); + let mut initial_update = pin!(update_diff_buffer( + &diff_buffer, + &buffer, + &clipboard_buffer, + &mut async_cx, + )); + assert!(initial_update.as_mut().now_or_never().is_none()); + buffer.update(cx, |buffer, cx| { + buffer.set_text("CHANGED\n", cx); + }); + initial_update.await; + } + assert_eq!( + diff_buffer.read_with(cx, |diff, _| diff.changed_row_counts()), + (0, 0) + ); + + let _view = cx.new_window_entity(|window, cx| { + TextDiffView::new( + clipboard_buffer, + editor, + buffer.clone(), + Point::new(0, 0)..Point::new(1, 0), + diff_buffer.clone(), + project, + workspace, + window, + cx, + ) + }); + cx.run_until_parked(); + cx.executor().advance_clock(RECALCULATE_DIFF_DEBOUNCE); + cx.run_until_parked(); + diff_buffer.read_with(cx, |diff, cx| { + assert_eq!(diff.changed_row_counts(), (1, 1)); + assert_eq!( + diff.snapshot(cx).buffer_version(), + &buffer.read(cx).version() + ); + }); + } + async fn base_test( project_root: &str, file_path: &str, diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 4557cdf5e40..9244d3e10f1 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -3103,6 +3103,7 @@ impl Buffer { return; } + self.pending_autoindent.take(); self.reparse(cx, true); cx.emit(BufferEvent::Edited { source }); let is_dirty = self.is_dirty(); @@ -3428,8 +3429,8 @@ impl Buffer { pub fn undo_operations(&mut self, counts: HashMap, cx: &mut Context) { let was_dirty = self.is_dirty(); - let operation = self.text.undo_operations(counts); let old_version = self.version.clone(); + let operation = self.text.undo_operations(counts); self.send_operation(Operation::Buffer(operation), true, cx); self.did_edit(&old_version, was_dirty, BufferEditSource::User, cx); } @@ -6335,3 +6336,49 @@ pub(crate) fn trailing_whitespace_ranges( ranges } + +#[cfg(test)] +mod tests { + use super::{AutoindentMode, Buffer}; + use crate::rust_lang; + use futures::FutureExt as _; + use gpui::{AppContext as _, TestAppContext}; + use settings::SettingsStore; + + #[gpui::test] + fn test_undo_during_async_autoindent(cx: &mut TestAppContext) { + cx.update(|cx| { + let settings_store = SettingsStore::test(cx); + cx.set_global(settings_store); + }); + for undo_operations in [false, true] { + let buffer = cx.new(|cx| Buffer::local("fn a() {}", cx).with_language(rust_lang(), cx)); + cx.run_until_parked(); + let autoindent_applied = buffer.update(cx, |buffer, cx| { + buffer.set_sync_parse_timeout(None); + let edit_id = buffer + .edit([(8..8, "\n\n")], Some(AutoindentMode::EachLine), cx) + .unwrap(); + let autoindent_applied = buffer.wait_for_autoindent_applied().unwrap(); + buffer.reparse.take(); + let snapshot = buffer.snapshot(); + let mut syntax = snapshot.syntax; + syntax.reparse(&snapshot.text, None, rust_lang()); + buffer.did_finish_parsing(syntax, None, false, cx); + assert!(buffer.pending_autoindent.is_some()); + + if undo_operations { + buffer.undo_operations([(edit_id, 1)].into_iter().collect(), cx); + } else { + assert!(buffer.undo(cx).is_some()); + } + assert!(buffer.pending_autoindent.is_none()); + assert_eq!(buffer.text(), "fn a() {}"); + autoindent_applied + }); + cx.run_until_parked(); + assert_eq!(buffer.read_with(cx, |buffer, _| buffer.text()), "fn a() {}"); + autoindent_applied.now_or_never().unwrap().unwrap(); + } + } +} diff --git a/crates/multi_buffer/src/multi_buffer_tests.rs b/crates/multi_buffer/src/multi_buffer_tests.rs index 462b646e0d0..397cf00be11 100644 --- a/crates/multi_buffer/src/multi_buffer_tests.rs +++ b/crates/multi_buffer/src/multi_buffer_tests.rs @@ -1460,6 +1460,68 @@ async fn test_set_anchored_excerpts_for_path(cx: &mut TestAppContext) { ); } +#[gpui::test] +async fn test_set_anchored_excerpts_after_edit(cx: &mut TestAppContext) { + for (replace_excerpt, edit, replacement, expected) in [ + (true, 5..5, "位x", "b位xbb"), + (false, 5..5, "位", "b位bb"), + (false, 4..7, "", ""), + ] { + let buffer = cx.new(|cx| Buffer::local("aaa\nbbb\nccc\n", cx)); + let multibuffer = cx.new(|_| MultiBuffer::new(Capability::ReadWrite)); + let update = multibuffer.update(cx, |multibuffer, cx| { + multibuffer.set_excerpts_for_path( + PathKey::sorted(0), + buffer.clone(), + [Point::new(1, 0)..Point::new(1, 3)], + 0, + cx, + ); + let snapshot = buffer.read(cx).snapshot(); + multibuffer.set_anchored_excerpts_for_path( + PathKey::sorted(0), + buffer.clone(), + vec![ + snapshot.anchor_after(Point::new(1, 0)) + ..snapshot.anchor_before(Point::new(1, 3)), + ], + 0, + cx, + ) + }); + buffer.update(cx, |buffer, cx| { + buffer.edit([(edit, replacement)], None, cx); + }); + multibuffer.update(cx, |multibuffer, cx| { + assert_eq!(multibuffer.snapshot(cx).text(), expected); + if replace_excerpt { + let snapshot = buffer.read(cx).snapshot(); + multibuffer.set_excerpt_ranges_for_path( + PathKey::sorted(0), + buffer.clone(), + &snapshot, + vec![ExcerptRange::new(Point::new(1, 3)..Point::new(1, 4))], + cx, + ); + } + }); + + let ranges = update.await; + multibuffer.read_with(cx, |multibuffer, cx| { + let snapshot = multibuffer.snapshot(cx); + assert_eq!(snapshot.text(), expected); + assert_eq!(snapshot.len(), MultiBufferOffset(expected.len())); + assert_eq!( + ranges + .iter() + .map(|range| range.to_point(&snapshot)) + .collect::>(), + vec![Point::new(0, 0)..Point::new(0, expected.len() as u32)], + ); + }); + } +} + #[gpui::test] fn test_empty_multibuffer(cx: &mut App) { let multibuffer = cx.new(|_| MultiBuffer::new(Capability::ReadWrite)); diff --git a/crates/multi_buffer/src/path_key.rs b/crates/multi_buffer/src/path_key.rs index 6e45764e73e..b5487719aff 100644 --- a/crates/multi_buffer/src/path_key.rs +++ b/crates/multi_buffer/src/path_key.rs @@ -340,8 +340,9 @@ impl MultiBuffer { ..buffer_snapshot.anchor_after(r.primary.end), }) .collect::>(); + let buffer_snapshot = buffer.read(cx).snapshot(); let inserted = - self.update_path_excerpts(path.clone(), buffer, buffer_snapshot, &anchor_ranges, cx); + self.update_path_excerpts(path.clone(), buffer, &buffer_snapshot, &anchor_ranges, cx); let path_key_index = self.get_or_create_path_key_index(&path); (inserted, path_key_index) } diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index cfc1f1bd087..f795e8b2c98 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -1693,9 +1693,6 @@ impl GitStore { let task = cx .spawn(async move |this, cx| { let result: Result> = async { - let buffer_snapshot = buffer.update(cx, |buffer, _| buffer.snapshot()); - let language_registry = - buffer.update(cx, |buffer, _| buffer.language_registry()); let content: Option> = match oid { None => None, Some(oid) => Some({ @@ -1706,6 +1703,10 @@ impl GitStore { content.into() }), }; + let (buffer_snapshot, language_registry) = buffer.read_with(cx, |buffer, _| { + (buffer.snapshot(), buffer.language_registry()) + }); + let buffer_version = buffer_snapshot.version.clone(); let buffer_diff = cx.new(|cx| { BufferDiff::new( &buffer_snapshot, @@ -1727,7 +1728,7 @@ impl GitStore { buffer_diff.set_secondary_diff(unstaged_diff); }); - this.update(cx, |this, cx| { + let recalculation = this.update(cx, |this, cx| { this.loading_diffs.remove(&(buffer_id, diff_kind)); let git_store = cx.weak_entity(); @@ -1736,15 +1737,23 @@ impl GitStore { .entry(buffer_id) .or_insert_with(|| cx.new(|cx| BufferGitState::new(git_store, cx))); - diff_state.update(cx, |state, _| { + diff_state.update(cx, |state, cx| { if let Some(oid) = oid { if let Some(content) = content { state.oid_texts.insert(oid, content); } } state.oid_diffs.insert(oid, buffer_diff.downgrade()); - }); + let buffer = buffer.read(cx); + if buffer.version() != buffer_version { + state.recalculate_diffs(buffer.text_snapshot(), cx); + } + state.wait_for_recalculation() + }) })?; + if let Some(recalculation) = recalculation { + recalculation.await; + } Ok(buffer_diff) } @@ -5909,6 +5918,31 @@ impl BufferGitState { // for a bit yield_now().await; + for (oid, oid_diff, base_text_buffer, base_text) in oid_diffs { + let base_text_snapshot = + base_text_buffer.read_with(cx, |buffer, _| buffer.snapshot()); + let new_oid_diff = cx + .update(|cx| { + oid_diff.read(cx).update_diff( + buffer.clone(), + &base_text_snapshot, + base_text.clone(), + cx, + ) + }) + .await; + + oid_diff.update(cx, |diff, cx| diff.set_snapshot(new_oid_diff, cx)); + + log::debug!( + "finished recalculating oid diff for buffer {} oid {:?}", + buffer.remote_id(), + oid + ); + + yield_now().await; + } + let cancel = this.update(cx, |this, _| { // This checks whether all pending stage/unstage operations // have quiesced (i.e. both the corresponding write and the @@ -5999,31 +6033,6 @@ impl BufferGitState { yield_now().await; - for (oid, oid_diff, base_text_buffer, base_text) in oid_diffs { - let base_text_snapshot = - base_text_buffer.read_with(cx, |buffer, _| buffer.snapshot()); - let new_oid_diff = cx - .update(|cx| { - oid_diff.read(cx).update_diff( - buffer.clone(), - &base_text_snapshot, - base_text.clone(), - cx, - ) - }) - .await; - - oid_diff.update(cx, |diff, cx| diff.set_snapshot(new_oid_diff, cx)); - - log::debug!( - "finished recalculating oid diff for buffer {} oid {:?}", - buffer.remote_id(), - oid - ); - - yield_now().await; - } - log::debug!( "finished recalculating diffs for buffer {}", buffer.remote_id() @@ -12034,6 +12043,116 @@ mod tests { (gate, repository, oids) } + #[gpui::test] + async fn test_open_diff_since_after_buffer_edit(cx: &mut TestAppContext) { + init_test(cx); + for (has_base, fail_index_write) in + [(true, false), (false, false), (true, true), (false, true)] + { + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + util::path!("/project"), + json!({ ".git": {}, "file.txt": "old\n" }), + ) + .await; + let oids = fs.set_merge_base_content_for_repo( + util::path!("/project/.git").as_ref(), + &[("file.txt", "old\n".to_owned())], + ); + let oid = *oids.first().expect("base blob"); + let gate = fs.install_blob_read_gate_for_repo(util::path!("/project/.git").as_ref()); + let project = Project::test(fs.clone(), [util::path!("/project").as_ref()], cx).await; + project + .update(cx, |project, cx| project.git_scans_complete(cx)) + .await; + let repository = project.read_with(cx, |project, cx| { + project.active_repository(cx).expect("repository") + }); + let buffer = project + .update(cx, |project, cx| { + project.open_local_buffer(util::path!("/project/file.txt"), cx) + }) + .await + .expect("open buffer"); + let git_store = project.read_with(cx, |project, _| project.git_store().clone()); + let _staged_diff = if fail_index_write { + fs.with_git_state(util::path!("/project/.git").as_ref(), false, |state| { + state.simulated_index_write_error_message = + Some("index write failed".to_owned()); + }) + .expect("configure index write failure"); + Some( + git_store + .update(cx, |git_store, cx| { + git_store.open_staged_diff(buffer.clone(), cx) + }) + .await + .expect("open staged diff"), + ) + } else { + None + }; + let (index_error_sender, index_error_receiver) = std::sync::mpsc::channel(); + let _subscription = cx.update(|cx| { + cx.subscribe(&git_store, move |_, event, _| { + if let GitStoreEvent::IndexWriteError(error) = event { + index_error_sender + .send(error.to_string()) + .expect("record index error"); + } + }) + }); + let (release_sender, release_receiver) = oneshot::channel::<()>(); + let held_job = repository.update(cx, |repository, _| { + repository.send_job("hold", None, move |_, _| async move { + release_receiver.await.expect("release job queue"); + }) + }); + let mut diff_task = git_store.update(cx, |git_store, cx| { + git_store.open_diff_since(has_base.then_some(oid), buffer.clone(), repository, cx) + }); + cx.run_until_parked(); + assert_eq!(gate.waiting(), usize::from(has_base)); + buffer.update(cx, |buffer, cx| { + buffer.set_text("new\nextra\n", cx); + }); + cx.run_until_parked(); + gate.open(); + cx.run_until_parked(); + assert!((&mut diff_task).now_or_never().is_none()); + buffer.update(cx, |buffer, cx| { + buffer.set_text("latest\nextra\nthird\n", cx); + }); + cx.run_until_parked(); + if fail_index_write { + git_store.update(cx, |git_store, cx| { + git_store.write_index_text_for_buffer_id( + buffer.read(cx).remote_id(), + Some("latest\nextra\nthird\n".to_owned()), + cx, + ); + }); + } + release_sender.send(()).expect("release job queue"); + held_job.await.expect("held job"); + let diff = diff_task.await.expect("open diff"); + diff.read_with(cx, |diff, cx| { + assert_eq!(diff.changed_row_counts(), (3, u32::from(has_base))); + assert_eq!( + diff.snapshot(cx).buffer_version(), + &buffer.read(cx).version() + ); + }); + cx.run_until_parked(); + if fail_index_write { + assert_eq!( + index_error_receiver.try_recv().expect("index write failed"), + "index write failed", + ); + } + } + } + #[gpui::test] async fn test_blob_reads_are_bounded(cx: &mut TestAppContext) { init_test(cx); diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index ec5c11c2a51..d02ef6685b9 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -16901,9 +16901,9 @@ mod tests { #[gpui::test] async fn test_fetch_inlay_hints_after_buffer_edit(cx: &mut gpui::TestAppContext) { cx.update(|cx| { - let settings_store = settings::SettingsStore::test(cx); + let settings_store = SettingsStore::test(cx); cx.set_global(settings_store); - release_channel::init(semver::Version::new(0, 0, 0), cx); + release_channel::init(Version::new(0, 0, 0), cx); }); let fs = fs::FakeFs::new(cx.executor()); fs.insert_tree( @@ -16911,17 +16911,53 @@ mod tests { serde_json::json!({ "main.rs": "fn main() {}" }), ) .await; - let project = crate::Project::test(fs, [util::path!("/the-root").as_ref()], cx).await; + let project = Project::test(fs, [util::path!("/the-root").as_ref()], cx).await; let languages = project.read_with(cx, |project, _| project.languages().clone()); languages.add(language::rust_lang()); + let (request_sender, request_receiver) = async_channel::bounded(1); + let (response_sender, response_receiver) = async_channel::bounded(1); let mut fake_servers = languages.register_fake_lsp( "Rust", language::FakeLspAdapter { capabilities: lsp::ServerCapabilities { inlay_hint_provider: Some(lsp::OneOf::Left(true)), - ..Default::default() + ..lsp::ServerCapabilities::default() }, - ..Default::default() + initializer: Some(Box::new(move |fake_server| { + let request_sender = request_sender.clone(); + let response_receiver = response_receiver.clone(); + fake_server.set_request_handler::( + move |_, _| { + let request_sender = request_sender.clone(); + let response_receiver = response_receiver.clone(); + async move { + request_sender.send(()).await?; + response_receiver.recv().await?; + Ok(Some( + [ + (0, 2, lsp::InlayHintKind::TYPE), + (0, 3, lsp::InlayHintKind::PARAMETER), + (1, 5, lsp::InlayHintKind::TYPE), + (1, 12, lsp::InlayHintKind::TYPE), + ] + .into_iter() + .map(|(row, column, kind)| lsp::InlayHint { + position: lsp::Position::new(row, column), + label: lsp::InlayHintLabel::String("hint".to_owned()), + kind: Some(kind), + text_edits: None, + tooltip: None, + padding_left: None, + padding_right: None, + data: None, + }) + .collect(), + )) + } + }, + ); + })), + ..language::FakeLspAdapter::default() }, ); let (buffer, _lsp_handle) = project @@ -16932,51 +16968,40 @@ mod tests { .expect("open buffer"); let fake_server = fake_servers.next().await.expect("start language server"); let server_id = fake_server.server.server_id(); - cx.run_until_parked(); - - let (request_sender, request_receiver) = async_channel::bounded(1); - let (response_sender, response_receiver) = async_channel::bounded(1); - fake_server.set_request_handler::(move |_, _| { - let request_sender = request_sender.clone(); - let response_receiver = response_receiver.clone(); - async move { - request_sender.send(()).await?; - response_receiver.recv().await?; - Ok(Some(vec![lsp::InlayHint { - position: lsp::Position::new(0, 1), - label: lsp::InlayHintLabel::String("hint".to_string()), - kind: Some(lsp::InlayHintKind::TYPE), - text_edits: None, - tooltip: None, - padding_left: None, - padding_right: None, - data: None, - }])) - } - }); let lsp_store = project.read_with(cx, |project, _| project.lsp_store()); - let hints_task = lsp_store.update(cx, |lsp_store, cx| { - let snapshot = buffer.read(cx).snapshot(); - lsp_store.fetch_inlay_hints( - None, - &buffer, - snapshot.anchor_before(0)..snapshot.anchor_after(snapshot.len()), - cx, - ) - }); - request_receiver.recv().await.expect("receive request"); - buffer.update(cx, |buffer, cx| { - buffer.edit([(0..0, "// edited\n")], None, cx); - }); - response_sender.send(()).await.expect("release response"); - let hints_by_server = hints_task.await.expect("fetch hints"); - let hints = hints_by_server.get(&server_id).expect("server hints"); - assert_eq!(hints.len(), 1); - buffer.read_with(cx, |buffer, _| { - let hint = hints.first().expect("hint in inserted text"); - assert_eq!(hint.position.to_offset(buffer), 1); - }); + for (range_end, expected_offsets) in [(5, vec![4, 5]), (12, vec![4, 5, 11, 18])] { + buffer.update(cx, |buffer, cx| { + buffer.edit([(0..buffer.len(), "fn main() {}")], None, cx); + }); + cx.run_until_parked(); + let hints_task = lsp_store.update(cx, |lsp_store, cx| { + let snapshot = buffer.read(cx).snapshot(); + lsp_store.fetch_inlay_hints( + None, + &buffer, + snapshot.anchor_before(0)..snapshot.anchor_after(range_end), + cx, + ) + }); + request_receiver.recv().await.expect("receive request"); + buffer.update(cx, |buffer, cx| { + buffer.edit([(0..0, "馃榾x\n")], None, cx); + }); + response_sender.send(()).await.expect("release response"); + + let hints_by_server = hints_task.await.expect("fetch hints"); + let hints = hints_by_server.get(&server_id).expect("server hints"); + buffer.read_with(cx, |buffer, _| { + assert_eq!( + hints + .iter() + .map(|hint| hint.position.to_offset(buffer)) + .collect::>(), + expected_offsets, + ); + }); + } } #[test] diff --git a/crates/vim/src/command.rs b/crates/vim/src/command.rs index a80a6fd9f01..ff044245d45 100644 --- a/crates/vim/src/command.rs +++ b/crates/vim/src/command.rs @@ -2198,22 +2198,17 @@ impl OnMatchingLines { }; vim.update_editor(cx, |_, editor, cx| { - let snapshot = editor.snapshot(window, cx); + let snapshot = editor.buffer().read(cx).snapshot(cx); let mut row = range.start.0; let point_range = Point::new(range.start.0, 0) - ..snapshot - .buffer_snapshot() - .clip_point(Point::new(range.end.0 + 1, 0), Bias::Left); + ..snapshot.clip_point(Point::new(range.end.0 + 1, 0), Bias::Left); cx.spawn_in(window, async move |editor, cx| { - let new_selections = cx + let matching_lines = cx .background_spawn(async move { let mut line = String::new(); - let mut new_selections = Vec::new(); - let chunks = snapshot - .buffer_snapshot() - .text_for_range(point_range) - .chain(["\n"]); + let mut matching_lines = Vec::new(); + let chunks = snapshot.text_for_range(point_range).chain(["\n"]); for chunk in chunks { for (newline_ix, text) in chunk.split('\n').enumerate() { @@ -2221,8 +2216,8 @@ impl OnMatchingLines { if regexes.iter().all(|(regex, should_match)| { regex.is_match(&line) == *should_match }) { - new_selections - .push(Point::new(row, 0).to_display_point(&snapshot)) + matching_lines + .push(snapshot.anchor_after(Point::new(row, 0))) } row += 1; line.clear(); @@ -2231,31 +2226,38 @@ impl OnMatchingLines { } } - new_selections + matching_lines }) .await; - if new_selections.is_empty() { - return; - } - - if let Some(vim_norm) = action.as_any().downcast_ref::() { - let mut vim_norm = vim_norm.clone(); - vim_norm.override_rows = - Some(new_selections.iter().map(|point| point.row().0).collect()); - editor - .update_in(cx, |_, window, cx| { - window.dispatch_action(vim_norm.boxed_clone(), cx); - }) - .log_err(); - return; - } - editor .update_in(cx, |editor, window, cx| { + let snapshot = editor.buffer().read(cx).snapshot(cx); + let new_selections = matching_lines + .into_iter() + .filter(|anchor| anchor.is_valid(&snapshot)) + .map(|anchor| Point::new(anchor.to_point(&snapshot).row, 0)) + .collect::>(); + if new_selections.is_empty() { + return; + } + + if let Some(vim_norm) = action.as_any().downcast_ref::() { + let mut vim_norm = vim_norm.clone(); + vim_norm.override_rows = + Some(new_selections.iter().map(|point| point.row).collect()); + window.dispatch_action(vim_norm.boxed_clone(), cx); + return; + } + editor.start_transaction_at(Instant::now(), window, cx); editor.change_selections(SelectionEffects::no_scroll(), window, cx, |s| { - s.replace_cursors_with(|_| new_selections); + s.replace_cursors_with(|map| { + new_selections + .iter() + .map(|point| point.to_display_point(map)) + .collect() + }); }); window.dispatch_action(action, cx); @@ -2274,7 +2276,7 @@ impl OnMatchingLines { editor.end_transaction_at(Instant::now(), cx); }) }) - .log_err(); + .ok(); }) .detach(); }); @@ -2630,16 +2632,23 @@ impl ShellExec { #[cfg(test)] mod test { - use std::path::{Path, PathBuf}; + use std::{ + path::{Path, PathBuf}, + slice, + }; + use super::{CommandRange, OnMatchingLines, VimNorm, WrappedAction}; use crate::{ VimAddon, state::Mode, test::{NeovimBackedTestContext, VimTestContext}, + visual::VisualDeleteLine, }; - use editor::{Editor, EditorSettings}; - use gpui::{Context, TestAppContext}; + use editor::{Editor, EditorSettings, display_map::ToDisplayPoint}; + use gpui::{Action, AppContext as _, Context, TestAppContext}; use indoc::indoc; + use language::Point; + use multi_buffer::{MultiBuffer, PathKey}; use settings::Settings; use util::path; use workspace::{OpenOptions, Workspace}; @@ -3101,6 +3110,150 @@ mod test { 藝a"}); } + #[gpui::test] + async fn test_command_matching_lines_after_edit(cx: &mut TestAppContext) { + let mut cx = VimTestContext::new(cx, true).await; + + for normal_command in [true, false] { + cx.set_state("藝match 伪\nkeep 尾\nmatch 纬\nlast 未\n", Mode::Normal); + start_matching_lines(&mut cx, normal_command); + cx.update_buffer(|buffer, cx| { + buffer.edit( + [ + (Point::new(0, 0)..Point::new(0, 0), "match 鏂癨n鍓嶇疆\n"), + (Point::new(2, 0)..Point::new(2, 0), "馃榾"), + ], + None, + cx, + ); + }); + cx.run_until_parked(); + + let expected = if normal_command { + "match 鏂癨n鍓嶇疆\nmatch 伪!\nkeep 尾\n馃榾match 纬!\nlast 未\n" + } else { + "match 鏂癨n鍓嶇疆\nkeep 尾\nlast 未\n" + }; + assert_eq!(cx.buffer_text(), expected); + + cx.set_state("藝鍓嶇疆\nmatch 伪\nkeep 尾\nmatch 纬\nlast 未\n", Mode::Normal); + start_matching_lines(&mut cx, normal_command); + cx.update_buffer(|buffer, cx| { + buffer.edit([(Point::new(0, 0)..Point::new(2, 0), "")], None, cx); + }); + cx.run_until_parked(); + + let expected = if normal_command { + "keep 尾\nmatch 纬!\nlast 未\n" + } else { + "keep 尾\nlast 未\n" + }; + assert_eq!(cx.buffer_text(), expected); + } + } + + #[gpui::test] + async fn test_command_matching_lines_after_fold_change(cx: &mut TestAppContext) { + let mut cx = VimTestContext::new(cx, true).await; + + for normal_command in [true, false] { + for (fold_before, fold_after) in [(true, true), (false, true), (true, false)] { + cx.set_state( + "藝header\nhidden 伪\nhidden 尾\nmatch 纬\nkeep 未\nmatch 蔚\nlast\n", + Mode::Normal, + ); + let fold_range = Point::new(0, 6)..Point::new(2, 9); + cx.update_editor(|editor, window, cx| { + editor.unfold_ranges(slice::from_ref(&fold_range), true, false, cx); + if fold_before { + editor.fold_ranges(vec![fold_range.clone()], false, window, cx); + } + assert_eq!( + Point::new(3, 0) + .to_display_point(&editor.snapshot(window, cx)) + .row() + .0, + if fold_before { 1 } else { 3 }, + ); + }); + start_matching_lines(&mut cx, normal_command); + cx.update_editor(|editor, window, cx| { + if fold_after { + editor.fold_ranges(vec![fold_range], false, window, cx); + } else { + editor.unfold_ranges(&[fold_range], true, false, cx); + } + }); + cx.run_until_parked(); + + let expected = if normal_command { + "header\nhidden 伪\nhidden 尾\nmatch 纬!\nkeep 未\nmatch 蔚!\nlast\n" + } else { + "header\nhidden 伪\nhidden 尾\nkeep 未\nlast\n" + }; + assert_eq!( + cx.buffer_text(), + expected, + "normal_command={normal_command}, fold_before={fold_before}, fold_after={fold_after}", + ); + } + } + } + + #[gpui::test] + async fn test_command_matching_lines_after_excerpt_removal(cx: &mut TestAppContext) { + let mut cx = VimTestContext::new(cx, true).await; + + for normal_command in [true, false] { + for keep_match in [false, true] { + let editor = cx.workspace(|workspace, window, cx| { + let (remaining_text, remaining_rows) = if keep_match { + ("keep\nmatch survives\nlast\n", 3) + } else { + ("keep\nlast\n", 2) + }; + let multibuffer = MultiBuffer::build_multi( + [ + ("match removed\n", vec![Point::row_range(0..1)]), + (remaining_text, vec![Point::row_range(0..remaining_rows)]), + ], + cx, + ); + let editor = cx.new(|cx| { + Editor::for_multibuffer( + multibuffer, + Some(workspace.project().clone()), + window, + cx, + ) + }); + workspace.add_item_to_active_pane( + Box::new(editor.clone()), + None, + true, + window, + cx, + ); + editor + }); + cx.editor = editor; + cx.run_until_parked(); + start_matching_lines(&mut cx, normal_command); + cx.update_multibuffer(|multibuffer, cx| { + multibuffer.remove_excerpts(PathKey::sorted(0), cx); + }); + cx.run_until_parked(); + + let expected = if normal_command && keep_match { + "keep\nmatch survives!\nlast\n" + } else { + "keep\nlast\n" + }; + assert_eq!(cx.buffer_text(), expected); + } + } + } + #[gpui::test] async fn test_del_marks(cx: &mut TestAppContext) { let mut cx = NeovimBackedTestContext::new(cx).await; @@ -3622,4 +3775,28 @@ mod test { Mode::VisualLine, ); } + + fn start_matching_lines(cx: &mut VimTestContext, normal_command: bool) { + let vim = cx.editor(|editor, _, _| editor.addon::().unwrap().entity.clone()); + cx.update(|window, cx| { + vim.update(cx, |vim, cx| { + OnMatchingLines { + range: CommandRange::buffer(), + search: String::from("match"), + action: WrappedAction(if normal_command { + VimNorm { + range: None, + command: String::from("A!"), + override_rows: None, + } + .boxed_clone() + } else { + VisualDeleteLine.boxed_clone() + }), + invert: false, + } + .run(vim, window, cx); + }); + }); + } } diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index 5b617447c4c..8d5477f52a7 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -6043,8 +6043,7 @@ impl BackgroundScanner { ignore_stack.append(IgnoreKind::Gitignore(job.abs_path.clone()), ignore.clone()); } - let mut entries_by_id_edits = Vec::new(); - let mut entries_by_path_edits = Vec::new(); + let mut ignore_changes = Vec::new(); let Some(path) = job .abs_path .strip_prefix(snapshot.abs_path.as_path()) @@ -6075,6 +6074,24 @@ impl BackgroundScanner { entry.is_ignored = ignore_stack.is_abs_path_ignored(&abs_path, entry.is_dir()); if entry.is_dir() { + let state = self.state.lock().await; + let Some(current_entry) = state + .snapshot + .entry_for_id(entry.id) + .filter(|current| current.path == entry.path && current.is_dir()) + else { + continue; + }; + let is_ignored = entry.is_ignored; + entry = current_entry.clone(); + if state + .snapshot + .entries_by_id + .get(&entry.id, ()) + .is_some_and(|path_entry| path_entry.scan_id <= snapshot.scan_id) + { + entry.is_ignored = is_ignored; + } let child_ignore_stack = if entry.is_ignored { IgnoreStack::all() } else { @@ -6088,7 +6105,6 @@ impl BackgroundScanner { && entry.kind.is_unloaded() && (was_ignored || ignore_stack.repo_root.is_some()) { - let state = self.state.lock().await; if self.should_scan_directory(&state, &entry, ignore_stack.repo_root.is_some()) { state @@ -6102,6 +6118,7 @@ impl BackgroundScanner { } } + drop(state); job.ignore_queue .send(UpdateIgnoreStatusJob { abs_path: abs_path.clone(), @@ -6114,21 +6131,36 @@ impl BackgroundScanner { } if entry.is_ignored != was_ignored { - let mut path_entry = snapshot.entries_by_id.get(&entry.id, ()).unwrap().clone(); - path_entry.scan_id = snapshot.scan_id; - path_entry.is_ignored = entry.is_ignored; - entries_by_id_edits.push(Edit::Insert(path_entry)); - entries_by_path_edits.push(Edit::Insert(entry)); + ignore_changes.push((entry.id, entry.path, entry.is_ignored)); } } let state = &mut self.state.lock().await; - for edit in &entries_by_path_edits { - if let Edit::Insert(entry) = edit - && let Err(ix) = state.changed_paths.binary_search(&entry.path) - { - state.changed_paths.insert(ix, entry.path.clone()); + let mut entries_by_id_edits = Vec::new(); + let mut entries_by_path_edits = Vec::new(); + for (entry_id, path, is_ignored) in ignore_changes { + let Some(path_entry) = state.snapshot.entries_by_id.get(&entry_id, ()) else { + continue; + }; + if path_entry.path != path || path_entry.scan_id > snapshot.scan_id { + continue; } + let Some(entry) = state.snapshot.entry_for_path(&path) else { + continue; + }; + if entry.id != entry_id || entry.is_ignored == is_ignored { + continue; + } + let mut path_entry = path_entry.clone(); + let mut entry = entry.clone(); + path_entry.scan_id = state.snapshot.scan_id; + path_entry.is_ignored = is_ignored; + entry.is_ignored = is_ignored; + if let Err(index) = state.changed_paths.binary_search(&path) { + state.changed_paths.insert(index, path); + } + entries_by_id_edits.push(Edit::Insert(path_entry)); + entries_by_path_edits.push(Edit::Insert(entry)); } state @@ -7394,7 +7426,21 @@ fn push_normalized(rope: &mut Rope, text: &str, scratch: &mut String) { #[cfg(test)] mod tests { - use super::*; + use super::{ + BackgroundScanner, BackgroundScannerPhase, BackgroundScannerState, IgnoreStack, + NullWatcher, RemovedEntries, STREAM_BLOCK_BYTES, ScanRequest, UpdateIgnoreStatusJob, + Worktree, stream_utf8_into_rope, + }; + use collections::{HashMap, HashSet}; + use fs::{FakeFs, Fs, RemoveOptions, RenameOptions}; + use futures::{FutureExt as _, channel::mpsc}; + use gpui::TestAppContext; + use serde_json::json; + use settings::{SettingsStore, WorktreeId}; + use smallvec::SmallVec; + use std::{path::Path, sync::Arc}; + use text::LineEnding; + use util::{path, rel_path::rel_path}; /// Streams `bytes` the way `decode_file_text_to_rope` would, returning the /// decoded text and detected line ending, or `None` if the fast path bailed. @@ -7460,6 +7506,207 @@ mod tests { } } + #[gpui::test] + async fn test_ignore_update_preserves_refreshed_entries(cx: &mut TestAppContext) { + cx.update(|cx| { + let settings_store = SettingsStore::test(cx); + cx.set_global(settings_store); + }); + for initially_ignored in [false, true] { + let fs = FakeFs::new(cx.background_executor.clone()); + fs.insert_tree( + path!("/root"), + json!({ + ".gitignore": if initially_ignored { "*.txt\n" } else { "" }, + "gone.txt": "gone", + "renamed.txt": "renamed", + "modified.txt": "old", + + "directory": { "child.txt": "child" }, + "removed": { "child.txt": "child" }, + "replaced": {}, + }), + ) + .await; + let tree = Worktree::local( + Path::new(path!("/root")), + true, + fs.clone(), + Arc::default(), + true, + WorktreeId::from_proto(0), + &mut cx.to_async(), + ) + .await + .unwrap(); + tree.read_with(cx, |tree, _| tree.as_local().unwrap().scan_complete()) + .await; + let (status_updates_tx, _status_updates_rx) = mpsc::unbounded(); + let scanner = tree.update(cx, |tree, cx| { + let tree = tree.as_local_mut().unwrap(); + tree._background_scanner_tasks.clear(); + BackgroundScanner { + state: async_lock::Mutex::new(BackgroundScannerState { + snapshot: tree.snapshot.clone(), + prev_snapshot: tree.snapshot.snapshot.clone(), + symlink_paths_by_target: HashMap::default(), + scanned_dirs: HashSet::default(), + watched_dir_abs_paths_by_entry_id: HashMap::default(), + path_prefixes_to_scan: HashSet::default(), + paths_to_scan: HashSet::default(), + removed_entries: RemovedEntries::default(), + changed_paths: Vec::new(), + scanning_enabled: true, + }), + fs: fs.clone(), + fs_case_sensitive: tree.fs_case_sensitive, + status_updates_tx, + executor: cx.background_executor().clone(), + scan_requests_rx: async_channel::unbounded().1, + path_prefixes_to_scan_rx: async_channel::unbounded().1, + next_entry_id: tree.next_entry_id.clone(), + phase: BackgroundScannerPhase::Events, + watcher: Arc::new(NullWatcher), + settings: tree.settings.clone(), + share_private_files: tree.share_private_files, + track_git_repositories: true, + is_single_file: false, + defer_watch: false, + } + }); + fs.insert_file( + path!("/root/.gitignore"), + if initially_ignored { + Vec::new() + } else { + b"*.txt\n".to_vec() + }, + ) + .await; + scanner + .process_scan_request( + ScanRequest { + relative_paths: vec![rel_path(".gitignore").into_arc()], + done: SmallVec::new(), + }, + true, + ) + .await; + let old_snapshot = scanner.state.lock().await.snapshot.clone(); + fs.remove_file(Path::new(path!("/root/gone.txt")), RemoveOptions::default()) + .await + .unwrap(); + fs.rename( + Path::new(path!("/root/renamed.txt")), + Path::new(path!("/root/moved.txt")), + RenameOptions::default(), + ) + .await + .unwrap(); + fs.insert_file(path!("/root/modified.txt"), b"new content".to_vec()) + .await; + fs.remove_dir( + Path::new(path!("/root/removed")), + RemoveOptions { + recursive: true, + ..RemoveOptions::default() + }, + ) + .await + .unwrap(); + fs.remove_dir(Path::new(path!("/root/replaced")), RemoveOptions::default()) + .await + .unwrap(); + fs.insert_file(path!("/root/replaced"), b"file".to_vec()) + .await; + scanner + .process_scan_request( + ScanRequest { + relative_paths: [ + "gone.txt", + "renamed.txt", + "moved.txt", + "modified.txt", + "directory", + "removed", + "replaced", + ] + .map(|path| rel_path(path).into_arc()) + .to_vec(), + done: SmallVec::new(), + }, + true, + ) + .await; + let refreshed_snapshot = scanner.state.lock().await.snapshot.clone(); + let expected_entries = refreshed_snapshot + .entries(true, 0) + .cloned() + .map(|mut entry| { + if entry.path.as_ref() == rel_path("directory/child.txt") { + entry.is_ignored = !initially_ignored; + } + entry + }) + .collect::>(); + assert_eq!( + refreshed_snapshot.paths().collect::>(), + [ + ".gitignore", + "directory", + "directory/child.txt", + "modified.txt", + "moved.txt", + "replaced", + ] + .map(rel_path), + ); + assert_eq!( + refreshed_snapshot + .entry_for_path(rel_path("modified.txt")) + .unwrap() + .size, + 11 + ); + let (ignore_queue, ignore_jobs) = async_channel::unbounded(); + let (scan_queue, _scan_jobs) = async_channel::unbounded(); + scanner + .update_ignore_status( + UpdateIgnoreStatusJob { + abs_path: Arc::from(Path::new(path!("/root"))), + ignore_stack: IgnoreStack::none(), + ignore_queue, + scan_queue, + }, + &old_snapshot, + ) + .await; + let child_job = ignore_jobs.try_recv().unwrap(); + assert_eq!( + child_job.abs_path.as_ref(), + Path::new(path!("/root/directory")) + ); + scanner.update_ignore_status(child_job, &old_snapshot).await; + let state = scanner.state.lock().await; + assert_eq!( + state.snapshot.entries(true, 0).cloned().collect::>(), + expected_entries + ); + assert!(ignore_jobs.is_empty()); + + state.snapshot.check_invariants(false); + for entry in &expected_entries { + let path_entry = state.snapshot.entries_by_id.get(&entry.id, ()).unwrap(); + + assert_eq!(path_entry.is_ignored, entry.is_ignored); + } + assert_eq!( + state.changed_paths, + vec![rel_path("directory/child.txt").into_arc()] + ); + } + } + #[gpui::test] async fn test_stream_utf8_falls_back_on_non_utf8() { // Each of these must bail so the caller re-reads and decodes the slow