From 34dae654bc27f7b533b9cd79bd15963e0f73f44e Mon Sep 17 00:00:00 2001 From: Tim Vermeulen Date: Mon, 20 Apr 2026 14:31:48 +0200 Subject: [PATCH] editor: Cancel blame popover task when opening modal (#52525) Update all direct calls to `Editor::inline_blame_popover::take` to now be calls to `Editor::hide_blame_popover` as this ensures that the popover is also not shown in case its task still hasn't finished. This fixes a bug where the inline git blame popover could be shown even after the user had opened a modal. Release Notes: - Fixed the inline Git blame popover sometimes appearing after opening a modal. --------- Co-authored-by: dino --- crates/editor/src/editor.rs | 39 +++++++---- crates/editor/src/editor_tests.rs | 109 +++++++++++++++++++++++++----- crates/editor/src/items.rs | 4 +- 3 files changed, 119 insertions(+), 33 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index b2d3d354660c796cdecf34b796f9eb0d49f4fbd6..4842e861efd02e071fbefdc41feecfacd4f0e65e 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -2664,7 +2664,7 @@ impl Editor { EditorEvent::ScrollPositionChanged { local, .. } => { if *local { editor.hide_signature_help(cx, SignatureHelpHiddenBy::Escape); - editor.inline_blame_popover.take(); + editor.hide_blame_popover(true, cx); let snapshot = editor.snapshot(window, cx); let new_anchor = editor .scroll_manager @@ -3800,7 +3800,7 @@ impl Editor { self.refresh_matching_bracket_highlights(&display_map, cx); self.refresh_outline_symbols_at_cursor(cx); self.update_visible_edit_prediction(window, cx); - self.inline_blame_popover.take(); + self.hide_blame_popover(true, cx); if self.git_blame_inline_enabled { self.start_inline_blame_timer(window, cx); } @@ -7598,23 +7598,36 @@ impl Editor { self.mouse_context_menu.is_some() } + /// Hides the inline blame popover element, in case it's already visible, or + /// interrupts the task meant to show it, in case the task is running. + /// + /// When `ignore_timeout` is set to `true`, the popover is hidden + /// immediately, otherwise it'll be hidden after a short delay. + /// + /// Returns `true` if the popover was visible and was hidden, `false` + /// otherwise. pub fn hide_blame_popover(&mut self, ignore_timeout: bool, cx: &mut Context) -> bool { self.inline_blame_popover_show_task.take(); + if let Some(state) = &mut self.inline_blame_popover { - let hide_task = cx.spawn(async move |editor, cx| { - if !ignore_timeout { + if ignore_timeout { + self.inline_blame_popover.take(); + cx.notify(); + } else { + state.hide_task = Some(cx.spawn(async move |editor, cx| { cx.background_executor() .timer(std::time::Duration::from_millis(100)) .await; - } - editor - .update(cx, |editor, cx| { - editor.inline_blame_popover.take(); - cx.notify(); - }) - .ok(); - }); - state.hide_task = Some(hide_task); + + editor + .update(cx, |editor, cx| { + editor.inline_blame_popover.take(); + cx.notify(); + }) + .ok(); + })); + } + true } else { false diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 20eb17a2da9ba848030f1fbf26d22bf72498d68b..297a943455f0b3b6acb26af7879c949d6b308afe 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -28936,29 +28936,36 @@ println!("5"); }); } -#[gpui::test] -async fn test_hide_mouse_context_menu_on_modal_opened(cx: &mut TestAppContext) { - struct EmptyModalView { - focus_handle: gpui::FocusHandle, - } - impl EventEmitter for EmptyModalView {} - impl Render for EmptyModalView { - fn render(&mut self, _: &mut Window, _: &mut Context<'_, Self>) -> impl IntoElement { - div() - } +struct EmptyModalView { + focus_handle: gpui::FocusHandle, +} + +impl EventEmitter for EmptyModalView {} + +impl Render for EmptyModalView { + fn render(&mut self, _: &mut Window, _: &mut Context<'_, Self>) -> impl IntoElement { + div() } - impl Focusable for EmptyModalView { - fn focus_handle(&self, _cx: &App) -> gpui::FocusHandle { - self.focus_handle.clone() - } +} + +impl Focusable for EmptyModalView { + fn focus_handle(&self, _cx: &App) -> gpui::FocusHandle { + self.focus_handle.clone() } - impl workspace::ModalView for EmptyModalView {} - fn new_empty_modal_view(cx: &App) -> EmptyModalView { - EmptyModalView { +} + +impl workspace::ModalView for EmptyModalView {} + +impl EmptyModalView { + fn new(cx: &App) -> Self { + Self { focus_handle: cx.focus_handle(), } } +} +#[gpui::test] +async fn test_hide_mouse_context_menu_on_modal_opened(cx: &mut TestAppContext) { init_test(cx, |_| {}); let fs = FakeFs::new(cx.executor()); @@ -28987,7 +28994,7 @@ async fn test_hide_mouse_context_menu_on_modal_opened(cx: &mut TestAppContext) { assert!(editor.mouse_context_menu.is_some()); }); workspace.update_in(cx, |workspace, window, cx| { - workspace.toggle_modal(window, cx, |_, cx| new_empty_modal_view(cx)); + workspace.toggle_modal(window, cx, |_, cx| EmptyModalView::new(cx)); }); cx.read(|cx| { @@ -28995,6 +29002,72 @@ async fn test_hide_mouse_context_menu_on_modal_opened(cx: &mut TestAppContext) { }); } +#[gpui::test] +async fn test_hide_pending_blame_popover_when_modal_opens(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let window = cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let workspace = window + .read_with(cx, |multi_workspace, _| multi_workspace.workspace().clone()) + .unwrap(); + let multi_buffer = cx.update(|cx| MultiBuffer::build_simple("Buffer Contents!", cx)); + let buffer_id = multi_buffer.read_with(cx, |multi_buffer, cx| { + multi_buffer + .all_buffers_iter() + .next() + .expect("Should have at least one buffer") + .read(cx) + .remote_id() + }); + let cx = &mut VisualTestContext::from_window(*window, cx); + let editor = cx.new_window_entity(|window, cx| { + Editor::new( + EditorMode::full(), + multi_buffer, + Some(project.clone()), + window, + cx, + ) + }); + + workspace.update_in(cx, |workspace, window, cx| { + workspace.add_item_to_active_pane(Box::new(editor.clone()), None, true, window, cx); + }); + + editor.update_in(cx, |editor, _, cx| { + editor.blame = Some( + cx.new(|cx| GitBlame::new(editor.buffer.clone(), project.clone(), false, true, cx)), + ); + editor.show_blame_popover( + buffer_id, + &::git::blame::BlameEntry { + sha: "1b1b1b".parse().unwrap(), + range: 0..1, + ..Default::default() + }, + gpui::point(gpui::px(0.), gpui::px(0.)), + false, + cx, + ); + + assert!(editor.inline_blame_popover_show_task.is_some()); + assert!(editor.inline_blame_popover.is_none()); + }); + + workspace.update_in(cx, |workspace, window, cx| { + workspace.toggle_modal(window, cx, |_, cx| EmptyModalView::new(cx)); + }); + + // Toggling a modal while the blame popover task is still pending should + // clear both the task and any rendered popover. + editor.update_in(cx, |editor, _, _| { + assert!(editor.inline_blame_popover.is_none()); + assert!(editor.inline_blame_popover_show_task.is_none()); + }); +} + fn set_linked_edit_ranges( opening: (Point, Point), closing: (Point, Point), diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 5ad584f1a4233a92ed1f04cde4db57bb9dc470f9..5dc0563300ed20d15aa94a2b7d9e80187cd7c4a1 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -1017,10 +1017,10 @@ impl Item for Editor { if let Some(workspace_entity) = &workspace.weak_handle().upgrade() { cx.subscribe( workspace_entity, - |editor, _, event: &workspace::Event, _cx| { + |editor, _, event: &workspace::Event, cx| { if let workspace::Event::ModalOpened = event { editor.mouse_context_menu.take(); - editor.inline_blame_popover.take(); + editor.hide_blame_popover(true, cx); } }, )