From 5a4473f92ae0ca0c47f6e0ceb7abb31bb816b372 Mon Sep 17 00:00:00 2001 From: Austin Cummings Date: Mon, 24 Aug 2026 20:01:46 +0000 Subject: [PATCH] editor: Repaint gutter immediately when bookmarks change (#62847) Closes #61005 The bookmark store notified on every change, but nothing was listening. Editors only repainted their bookmark icons because the toggle actions happened to notify the editor directly - so workspace::ClearBookmarks, which never touches an editor, left stale icons in the gutter until something else caused a repaint. Now editors observe the bookmark store, so any change repaints them. This also fixes toggling a bookmark in one split not updating another split showing the same file. Release Notes: - Fixed bookmarks not disappearing from the gutter immediately when running `workspace::ClearBookmarks`. --- crates/editor/src/bookmarks.rs | 4 -- crates/editor/src/editor.rs | 6 ++ crates/editor/src/editor_tests.rs | 114 ++++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 4 deletions(-) diff --git a/crates/editor/src/bookmarks.rs b/crates/editor/src/bookmarks.rs index 12a899f398..c28db615cd 100644 --- a/crates/editor/src/bookmarks.rs +++ b/crates/editor/src/bookmarks.rs @@ -116,8 +116,6 @@ impl Editor { // Only add new (unnamed) bookmarks and leave existing ones unchanged. self.toggle_bookmarks(absent_targets, String::new(), cx); } - - cx.notify(); } pub fn toggle_bookmark_at_row(&mut self, row: DisplayRow, cx: &mut Context) { @@ -145,8 +143,6 @@ impl Editor { bookmark_store.update(cx, |bookmark_store, cx| { bookmark_store.toggle_bookmark(buffer, position, String::new(), cx); }); - - cx.notify(); } pub fn edit_bookmark(&mut self, _: &EditBookmark, window: &mut Window, cx: &mut Context) { diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 20e1c665cc..3d4c27d96f 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -2174,6 +2174,12 @@ impl Editor { _ => {} }, )); + project_subscriptions.push(cx.observe( + &project.read(cx).bookmark_store(), + |_, _, cx| { + cx.notify(); + }, + )); let git_store = project.read(cx).git_store().clone(); let project = project.clone(); project_subscriptions.push(cx.subscribe( diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index d3f9dc2282..cd220d28e2 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -33538,6 +33538,120 @@ async fn test_active_bookmarks(cx: &mut TestAppContext) { assert!(!active.contains(&DisplayRow(8))); } +#[gpui::test] +async fn test_clearing_bookmark_store_notifies_editor(cx: &mut TestAppContext) { + let mut ctx = BookmarkTestContext::new("Line 0\nLine 1\nLine 2\nLine 3", cx).await; + + ctx.toggle_bookmarks_at_rows(&[1, 3]); + + let active = ctx + .editor + .update_in(&mut ctx.cx, |editor: &mut Editor, window, cx| { + editor.active_bookmarks(DisplayRow(0)..DisplayRow(4), window, cx) + }); + assert!(active.contains(&DisplayRow(1))); + assert!(active.contains(&DisplayRow(3))); + + let editor = ctx.editor.clone(); + let editor_notified = Rc::new(RefCell::new(false)); + let _subscription = ctx.cx.update(|_, cx| { + cx.observe(&editor, { + let editor_notified = editor_notified.clone(); + move |_, _| *editor_notified.borrow_mut() = true + }) + }); + + // `workspace::ClearBookmarks` mutates only the store, so the editor's gutter + // repaint relies entirely on its observation of the bookmark store. + ctx.project.update(&mut ctx.cx, |project, cx| { + project.bookmark_store().update(cx, |bookmark_store, cx| { + bookmark_store.clear_bookmarks(cx); + }); + }); + ctx.cx.run_until_parked(); + + assert!( + *editor_notified.borrow(), + "clearing the bookmark store should notify the editor so its gutter repaints" + ); + let active = ctx + .editor + .update_in(&mut ctx.cx, |editor: &mut Editor, window, cx| { + editor.active_bookmarks(DisplayRow(0)..DisplayRow(4), window, cx) + }); + assert!(active.is_empty()); +} + +#[gpui::test] +async fn test_clear_bookmarks_action_repaints_workspace_editor(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/a"), + json!({ "main.rs": "Line 0\nLine 1\nLine 2\nLine 3" }), + ) + .await; + let project = Project::test(fs, [path!("/a").as_ref()], cx).await; + let window = cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let mut cx = VisualTestContext::from_window(*window, cx); + let workspace = window + .read_with(&mut cx, |mw, _| mw.workspace().clone()) + .unwrap(); + + let worktree_id = workspace.update_in(&mut cx, |workspace, _, cx| { + workspace.project().update(cx, |project, cx| { + project.worktrees(cx).next().unwrap().read(cx).id() + }) + }); + + let editor = workspace + .update_in(&mut cx, |workspace, window, cx| { + workspace.open_path((worktree_id, rel_path("main.rs")), None, true, window, cx) + }) + .await + .unwrap() + .downcast::() + .unwrap(); + cx.run_until_parked(); + + editor.update_in(&mut cx, |editor, window, cx| { + editor.change_selections(SelectionEffects::no_scroll(), window, cx, |s| { + s.select_ranges([Point::new(1, 0)..Point::new(1, 0)]) + }); + editor.toggle_bookmark(&actions::ToggleBookmark, window, cx); + }); + + let active = editor.update_in(&mut cx, |editor, window, cx| { + editor.active_bookmarks(DisplayRow(0)..DisplayRow(4), window, cx) + }); + assert!(active.contains(&DisplayRow(1))); + + // Drain the effects still pending from the toggle above, so that the only + // thing that can notify the editor below is the action dispatch. + cx.run_until_parked(); + + let editor_notified = Rc::new(RefCell::new(false)); + let _subscription = cx.update(|_, cx| { + cx.observe(&editor, { + let editor_notified = editor_notified.clone(); + move |_, _| *editor_notified.borrow_mut() = true + }) + }); + + cx.dispatch_action(workspace::ClearBookmarks); + cx.run_until_parked(); + + assert!( + *editor_notified.borrow(), + "workspace::ClearBookmarks should notify the editor so its gutter repaints" + ); + let active = editor.update_in(&mut cx, |editor, window, cx| { + editor.active_bookmarks(DisplayRow(0)..DisplayRow(4), window, cx) + }); + assert!(active.is_empty()); +} + #[gpui::test] async fn test_bookmark_not_available_in_single_line_editor(cx: &mut TestAppContext) { init_test(cx, |_| {});