diff --git a/crates/editor/src/bookmarks.rs b/crates/editor/src/bookmarks.rs index bcfaafe78e..3f32073632 100644 --- a/crates/editor/src/bookmarks.rs +++ b/crates/editor/src/bookmarks.rs @@ -1,21 +1,33 @@ -use std::ops::Range; +use std::{cmp::Reverse, ops::Range}; -use gpui::Entity; +use collections::{HashMap, HashSet}; +use futures::{StreamExt as _, channel::mpsc}; +use gpui::{AppContext as _, Entity, EventEmitter, Subscription, Task}; use language::Buffer; -use multi_buffer::{Anchor, MultiBufferOffset, MultiBufferSnapshot, ToOffset as _}; -use project::{Project, bookmark_store::BookmarkStore}; +use multi_buffer::{ + Anchor, Event as MultiBufferEvent, MultiBuffer, MultiBufferOffset, MultiBufferSnapshot, + PathKey, ToOffset as _, ToPoint as _, +}; +use project::{ + Project, + bookmark_store::{BookmarkStore, BookmarkStoreEvent}, +}; use rope::Point; use text::Bias; +use theme::ActiveTheme as _; use ui::{Context, Window}; use util::ResultExt as _; use workspace::{Workspace, searchable::Direction}; use crate::display_map::DisplayRow; use crate::{ - EditBookmark, Editor, GoToNextBookmark, GoToPreviousBookmark, MultibufferSelectionMode, - SelectionEffects, ToggleBookmark, ToggleBookmarkWithLabel, ViewBookmarks, scroll::Autoscroll, + EditBookmark, Editor, GoToNextBookmark, GoToPreviousBookmark, RowHighlightOptions, + SelectionEffects, ToggleBookmark, ToggleBookmarkWithLabel, ViewBookmarks, + multibuffer_context_lines, scroll::Autoscroll, }; +pub(crate) enum BookmarkRowHighlights {} + #[derive(Clone, Debug)] struct BookmarkTarget { buffer: Entity, @@ -338,32 +350,146 @@ impl Editor { cx: &mut Context, ) { let bookmark_store = workspace.project().read(cx).bookmark_store(); + bookmark_store.update(cx, |store, cx| store.forget_failed_paths(cx)); + + if Self::activate_bookmarks_tab(workspace, window, cx) { + return; + } + + if bookmark_store.read(cx).is_empty() { + return; + } + cx.spawn_in(window, async move |workspace, cx| { - let Some(locations) = BookmarkStore::all_bookmark_locations(bookmark_store, cx) + let Some(locations) = BookmarkStore::all_bookmark_locations(bookmark_store.clone(), cx) .await .log_err() else { return; }; + if locations.is_empty() { + return; + } workspace .update_in(cx, |workspace, window, cx| { - Editor::open_locations_in_multibuffer( - workspace, - locations, - "Bookmarks".into(), - false, - false, - MultibufferSelectionMode::First, - window, - cx, - ); + if Self::activate_bookmarks_tab(workspace, window, cx) { + return; + } + + let capability = workspace.project().read(cx).capability(); + let excerpt_buffer = + cx.new(|_cx| MultiBuffer::new(capability).with_title("Bookmarks".into())); + let bookmarks_tab_state = cx.new(|cx| { + BookmarksTabState::new( + excerpt_buffer.clone(), + bookmark_store, + locations, + cx, + ) + }); + let first_range = bookmarks_tab_state + .read(cx) + .populated_ranges + .first() + .cloned(); + let editor = cx.new(|cx| { + let mut editor = Editor::for_multibuffer( + excerpt_buffer, + Some(workspace.project().clone()), + window, + cx, + ); + editor.set_bookmarks_tab_state(bookmarks_tab_state, cx); + if let Some(first_range) = first_range { + editor.change_selections( + SelectionEffects::no_scroll(), + window, + cx, + |s| { + s.clear_disjoint(); + s.select_anchor_ranges(std::iter::once(first_range)); + }, + ); + } + editor + }); + + workspace.add_item_to_active_pane(Box::new(editor), None, true, window, cx); }) .log_err(); }) .detach(); } + fn activate_bookmarks_tab( + workspace: &mut Workspace, + window: &mut Window, + cx: &mut Context, + ) -> bool { + let activation_history = workspace.recently_activated_items(cx); + let existing = workspace + .items_of_type::(cx) + .filter(|editor| editor.read(cx).bookmarks_tab_state.is_some()) + .max_by_key(|editor| { + ( + activation_history + .get(&editor.entity_id()) + .copied() + .unwrap_or(0), + editor.entity_id(), + ) + }); + existing.is_some_and(|existing| workspace.activate_item(&existing, true, true, window, cx)) + } + + pub(super) fn set_bookmarks_tab_state( + &mut self, + bookmarks_tab_state: Entity, + cx: &mut Context, + ) { + self.bookmarks_tab_subscription = Some(cx.subscribe( + &bookmarks_tab_state, + |editor, _bookmarks_tab_state, event, cx| { + let BookmarksTabEvent::Populated(ranges) = event; + editor.on_bookmarks_populated(ranges, cx); + }, + )); + + let populated_ranges = bookmarks_tab_state.read(cx).populated_ranges.clone(); + self.bookmarks_tab_state = Some(bookmarks_tab_state); + self.on_bookmarks_populated(&populated_ranges, cx); + } + + fn on_bookmarks_populated(&mut self, ranges: &[Range], cx: &mut Context) { + self.clear_row_highlights::(); + let snapshot = self.buffer.read(cx).snapshot(cx); + for range in ranges { + if !snapshot.can_resolve(&range.start) { + continue; + } + let row = range.start.to_point(&snapshot).row; + let start = snapshot.anchor_before(Point::new(row, 0)); + let max_point = snapshot.max_point(); + let end = if row >= max_point.row { + if max_point.column == 0 { + Anchor::Max + } else { + snapshot.anchor_before(max_point) + } + } else { + snapshot.anchor_before(Point::new(row + 1, 0)) + }; + self.highlight_rows::( + start..end, + |cx| cx.theme().colors().editor_highlighted_line_background, + RowHighlightOptions::default(), + cx, + ); + } + cx.notify(); + } + fn bookmarks_in_range( range: Range, multi_buffer_snapshot: &MultiBufferSnapshot, @@ -398,3 +524,147 @@ impl Editor { .collect() } } + +pub(crate) struct BookmarksTabState { + multibuffer: Entity, + populated_ranges: Vec>, + populated_emit_scheduled: bool, + refresh_tx: mpsc::UnboundedSender<()>, + _refresh_task: Task<()>, + _store_subscription: Subscription, + _multibuffer_subscription: Subscription, +} + +pub(crate) enum BookmarksTabEvent { + Populated(Vec>), +} + +impl EventEmitter for BookmarksTabState {} + +impl BookmarksTabState { + fn new( + multibuffer: Entity, + bookmark_store: Entity, + locations: HashMap, Vec>>, + cx: &mut Context, + ) -> Self { + let store_subscription = cx.subscribe( + &bookmark_store, + |this, _bookmark_store, event: &BookmarkStoreEvent, _cx| { + if *event == BookmarkStoreEvent::BookmarksChanged { + this.schedule_refresh(); + } + }, + ); + let multibuffer_subscription = cx.subscribe( + &multibuffer, + |this, _multibuffer, event: &MultiBufferEvent, cx| { + if let MultiBufferEvent::Edited { .. } = event { + this.schedule_populated_emit(cx); + } + }, + ); + let context_lines = multibuffer_context_lines(cx); + let populated_ranges = multibuffer.update(cx, |multibuffer, cx| { + set_bookmark_excerpts(multibuffer, locations, context_lines, cx) + }); + let (refresh_tx, mut refresh_rx) = mpsc::unbounded::<()>(); + let refresh_task = cx.spawn(async move |this, cx| { + while refresh_rx.next().await.is_some() { + while refresh_rx.try_recv().is_ok() {} + + let Some(locations) = + BookmarkStore::all_bookmark_locations(bookmark_store.clone(), cx) + .await + .log_err() + else { + continue; + }; + + let applied = this.update(cx, |this, cx| { + let context_lines = multibuffer_context_lines(cx); + this.populated_ranges = this.multibuffer.update(cx, |multibuffer, cx| { + set_bookmark_excerpts(multibuffer, locations, context_lines, cx) + }); + this.schedule_populated_emit(cx); + }); + if applied.is_err() { + break; + } + } + }); + Self { + multibuffer, + populated_ranges, + populated_emit_scheduled: false, + refresh_tx, + _refresh_task: refresh_task, + _store_subscription: store_subscription, + _multibuffer_subscription: multibuffer_subscription, + } + } + + fn schedule_refresh(&self) { + self.refresh_tx.unbounded_send(()).ok(); + } + + fn schedule_populated_emit(&mut self, cx: &mut Context) { + if self.populated_emit_scheduled { + return; + } + self.populated_emit_scheduled = true; + let this = cx.weak_entity(); + cx.defer(move |cx| { + this.update(cx, |this, cx| { + this.populated_emit_scheduled = false; + cx.emit(BookmarksTabEvent::Populated(this.populated_ranges.clone())); + }) + .ok(); + }); + } +} + +fn set_bookmark_excerpts( + multibuffer: &mut MultiBuffer, + locations: impl IntoIterator, Vec>)>, + context_line_count: u32, + cx: &mut Context, +) -> Vec> { + let mut stale_paths = multibuffer + .snapshot(cx) + .buffers_with_paths() + .map(|(_, path_key)| path_key.clone()) + .collect::>(); + + let mut anchor_ranges = >>::new(); + for (buffer, mut ranges) in locations { + ranges.sort_by_key(|range| (range.start, Reverse(range.end))); + let path_key = PathKey::for_buffer(&buffer, cx); + stale_paths.remove(&path_key); + multibuffer.set_excerpts_for_path( + path_key, + buffer.clone(), + ranges.clone(), + context_line_count, + cx, + ); + let snapshot = multibuffer.snapshot(cx); + let buffer_snapshot = buffer.read(cx).snapshot(); + anchor_ranges.extend(ranges.into_iter().filter_map(|range| { + let text_range = buffer_snapshot.anchor_range_inside(range); + let start = snapshot.anchor_in_buffer(text_range.start)?; + let end = snapshot.anchor_in_buffer(text_range.end)?; + Some(start..end) + })); + } + + for path_key in stale_paths { + multibuffer.remove_excerpts(path_key, cx); + } + + let snapshot = multibuffer.snapshot(cx); + anchor_ranges + .retain(|range| snapshot.can_resolve(&range.start) && snapshot.can_resolve(&range.end)); + anchor_ranges.sort_by(|a, b| a.start.cmp(&b.start, &snapshot)); + anchor_ranges +} diff --git a/crates/editor/src/display_map/block_map.rs b/crates/editor/src/display_map/block_map.rs index 1a7b9d27d0..22fb21e4a1 100644 --- a/crates/editor/src/display_map/block_map.rs +++ b/crates/editor/src/display_map/block_map.rs @@ -487,7 +487,7 @@ impl Block { } } - fn is_header(&self) -> bool { + pub(crate) fn is_header(&self) -> bool { match self { Block::Custom(_) => false, Block::FoldedBuffer { .. } => true, diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 3f94f90d61..c9fa740931 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -273,6 +273,7 @@ pub use zed_actions::editor::RevealInFileManager; use zed_actions::editor::{MoveDown, MoveUp}; use crate::{ + bookmarks::BookmarksTabState, code_context_menus::CompletionsMenuSource, editor_settings::MultiCursorModifier, hover_links::{find_url, find_url_from_range}, @@ -1130,6 +1131,8 @@ pub struct Editor { expect_bounds_change: Option>, runnables: RunnableData, bookmark_store: Option>, + bookmarks_tab_state: Option>, + bookmarks_tab_subscription: Option, breakpoint_store: Option>, gutter_hover_button: (Option, Option>), pub(crate) gutter_diff_review_indicator: (Option, Option>), @@ -1854,6 +1857,9 @@ impl Editor { clone.needs_initial_data_update = self.enable_lsp_data; clone.enable_runnables = self.enable_runnables; clone.enable_code_lens = self.enable_code_lens; + if let Some(bookmarks_tab_state) = self.bookmarks_tab_state.clone() { + clone.set_bookmarks_tab_state(bookmarks_tab_state, cx); + } clone } @@ -2450,6 +2456,8 @@ impl Editor { pending_blame_hover_observation: None, bookmark_store, + bookmarks_tab_state: None, + bookmarks_tab_subscription: None, breakpoint_store, gutter_hover_button: (None, None), gutter_diff_review_indicator: (None, None), @@ -9227,7 +9235,26 @@ impl Editor { } else { end.row().0 }; + let mut header_rows = snapshot + .blocks_in_range( + DisplayRow(start_row)..DisplayRow(end_row.saturating_add(1)), + ) + .filter(|(_, block)| block.is_header()) + .map(|(block_row, block)| { + block_row.0..block_row.0.saturating_add(block.height()) + }) + .peekable(); for row in start_row..=end_row { + while header_rows + .next_if(|header_range| header_range.end <= row) + .is_some() + {} + if header_rows + .peek() + .is_some_and(|header_range| header_range.contains(&row)) + { + continue; + } let used_index = used_highlight_orders.entry(row).or_insert(highlight.index); if highlight.index >= *used_index { diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 5de63628ce..c718e9541a 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -45,7 +45,7 @@ use parking_lot::Mutex; use pretty_assertions::{assert_eq, assert_ne}; use project::{ FakeFs, Project, ProjectPath, - bookmark_store::SerializedBookmark, + bookmark_store::{BookmarkStore, BookmarkStoreEvent, SerializedBookmark}, debugger::breakpoint_store::{BreakpointState, SourceBreakpoint}, project_settings::LspSettings, trusted_worktrees::{PathTrust, TrustedWorktrees}, @@ -74,7 +74,7 @@ use util::{ }; use workspace::{ CloseActiveItem, CloseAllItems, CloseOtherItems, MultiWorkspace, NavigationEntry, OpenOptions, - ToolbarItemLocation, ViewId, + Pane, SplitDirection, ToolbarItemLocation, ViewId, Workspace, item::{FollowEvent, FollowableItem, Item, ItemHandle, SaveOptions}, register_project_item, }; @@ -34185,6 +34185,707 @@ async fn test_empty_rename_is_no_op(cx: &mut TestAppContext) { } } +#[gpui::test] +async fn test_bookmarks_tab_auto_refresh(cx: &mut TestAppContext) { + let (workspace, _pane, project, editor, mut cx) = init_bookmarks_tab_test( + cx, + json!({ + "main.rs": "line0\nline1\nline2\nline3\nline4\nline5\nline6\nline7\nline8\nline9\n", + "other.rs": "other0\nother1\nother2\nother3\n", + }), + ) + .await; + + toggle_bookmark_on_row(&editor, 0, &mut cx); + cx.run_until_parked(); + + let bookmarks_editor = open_and_find_bookmarks_tab(&workspace, &mut cx); + + assert_eq!( + excerpt_count(&bookmarks_editor, &mut cx), + 1, + "bookmarks editor should have 1 excerpt after first bookmark" + ); + + toggle_bookmark_on_row(&editor, 7, &mut cx); + cx.run_until_parked(); + assert_eq!( + excerpt_count(&bookmarks_editor, &mut cx), + 2, + "bookmarks editor should auto-refresh to 2 excerpts after second bookmark" + ); + + toggle_bookmark_on_row(&editor, 7, &mut cx); + cx.run_until_parked(); + assert_eq!( + excerpt_count(&bookmarks_editor, &mut cx), + 1, + "bookmarks editor should auto-refresh back to 1 excerpt after removing a bookmark" + ); + assert_eq!( + cursor_line_text(&bookmarks_editor, &mut cx), + "line0", + "refreshing must keep the surviving excerpt intact, preserving selections in it" + ); + + let other_editor = open_bookmarks_test_editor(&workspace, &project, "other.rs", &mut cx).await; + toggle_bookmark_on_row(&other_editor, 1, &mut cx); + cx.run_until_parked(); + assert_eq!( + bookmarked_paths(&bookmarks_editor, &mut cx), + vec!["main.rs", "other.rs"], + "bookmarks editor should hold one path per bookmarked file" + ); + + toggle_bookmark_on_row(&editor, 0, &mut cx); + cx.run_until_parked(); + assert_eq!( + bookmarked_paths(&bookmarks_editor, &mut cx), + vec!["other.rs"], + "unbookmarking a file's last line should drop its path from the bookmarks editor" + ); +} + +#[gpui::test] +async fn test_bookmarks_tab_selects_first_bookmark_despite_concurrent_change( + cx: &mut TestAppContext, +) { + let (workspace, _pane, _project, editor, mut cx) = init_bookmarks_tab_test( + cx, + json!({ + "main.rs": "line0\nline1\nline2\nline3\nline4\nline5\nline6\nline7\nline8\nline9\n", + }), + ) + .await; + + toggle_bookmark_on_row(&editor, 5, &mut cx); + cx.run_until_parked(); + + open_bookmarks_tab(&workspace, &mut cx); + + toggle_bookmark_on_row(&editor, 9, &mut cx); + cx.run_until_parked(); + + let bookmarks_editor = find_bookmarks_editor(&workspace, &mut cx) + .expect("bookmarks editor should exist after view_bookmarks"); + assert_eq!( + cursor_line_text(&bookmarks_editor, &mut cx), + "line5", + "a bookmark change racing the tab creation must not lose the initial selection" + ); +} + +#[gpui::test] +async fn test_view_bookmarks_without_resolvable_bookmarks_opens_no_tab(cx: &mut TestAppContext) { + let (workspace, _pane, project, _editor, mut cx) = init_bookmarks_tab_test( + cx, + json!({ + "main.rs": "line0\nline1\n", + "other.rs": "other0\n", + "third.rs": "third0\n", + }), + ) + .await; + + let bookmark_store = cx.update(|_window, cx| project.read(cx).bookmark_store()); + load_test_bookmarks( + &bookmark_store, + vec![(bookmark_path(path!("/a/third.rs")), vec![99])], + &mut cx, + ) + .await; + + let worktree_id = project.update(&mut cx, |project, cx| { + project + .worktrees(cx) + .next() + .expect("project should have at least one worktree") + .read(cx) + .id() + }); + let other_buffer = project + .update(&mut cx, |project, cx| { + project.open_buffer((worktree_id, rel_path("other.rs")), cx) + }) + .await + .expect("other.rs buffer should open"); + let probe_anchor = other_buffer.update(&mut cx, |buffer, _| { + buffer.text_snapshot().anchor_before(Point::new(0, 0)) + }); + bookmark_store.update(&mut cx, |store, cx| { + assert_eq!( + store + .find_bookmark(&other_buffer, probe_anchor, cx) + .map(|bookmark| bookmark.label.clone()), + None, + "a probe on a bookmark-less file should find nothing" + ); + }); + + open_bookmarks_tab(&workspace, &mut cx); + cx.run_until_parked(); + + assert_eq!( + find_bookmarks_editor(&workspace, &mut cx), + None, + "a store whose bookmarks all fail to resolve must not open an empty tab, even after a find_bookmark probe" + ); +} + +#[gpui::test] +async fn test_bookmarks_tab_survives_split(cx: &mut TestAppContext) { + let (workspace, pane, _project, editor, mut cx) = init_bookmarks_tab_test( + cx, + json!({ + "main.rs": "line0\nline1\nline2\nline3\nline4\nline5\nline6\nline7\nline8\nline9\n", + }), + ) + .await; + + toggle_bookmark_on_row(&editor, 0, &mut cx); + cx.run_until_parked(); + + let bookmarks_editor = open_and_find_bookmarks_tab(&workspace, &mut cx); + let split_editor = bookmarks_editor.update_in(&mut cx, |editor, window, cx| { + cx.new(|cx| editor.clone(window, cx)) + }); + assert!( + split_editor.read_with(&cx, |editor, _| editor.bookmarks_tab_state.is_some()), + "split clone should share the bookmarks tab state" + ); + assert!( + split_editor.read_with(&cx, |editor, cx| editor + .highlighted_rows::(cx) + .next() + .is_some()), + "split clone should carry over the bookmark line highlights" + ); + + toggle_bookmark_on_row(&editor, 7, &mut cx); + close_bookmarks_editor(&pane, bookmarks_editor, &mut cx); + assert_eq!( + excerpt_count(&split_editor, &mut cx), + 2, + "split should refresh itself when the original tab closes with a refresh in flight" + ); + + toggle_bookmark_on_row(&editor, 7, &mut cx); + cx.run_until_parked(); + assert_eq!( + excerpt_count(&split_editor, &mut cx), + 1, + "split clone's subscription should keep refreshing after the original tab closes" + ); +} + +#[gpui::test] +async fn test_bookmark_resolution_drop_emits_store_event(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/a"), + json!({ + "main.rs": "line0\nline1\n", + }), + ) + .await; + let project = Project::test(fs, [path!("/a").as_ref()], cx).await; + let worktree_id = project.read_with(cx, |project, cx| { + project + .worktrees(cx) + .next() + .expect("project should have at least one worktree") + .read(cx) + .id() + }); + let buffer = project + .update(cx, |project, cx| { + project.open_buffer((worktree_id, rel_path("main.rs")), cx) + }) + .await + .expect("main.rs buffer should open"); + + let bookmark_store = project.read_with(cx, |project, _| project.bookmark_store()); + let abs_path = cx + .update(|cx| BookmarkStore::abs_path_from_buffer(&buffer, cx)) + .expect("buffer should have an absolute path"); + + bookmark_store + .update(cx, |store, cx| { + store.load_serialized_bookmarks( + BTreeMap::from([(abs_path, serialized_rows(&[0, 99]))]), + cx, + ) + }) + .await + .expect("loading serialized bookmarks should succeed"); + + let events = Rc::new(RefCell::new(Vec::::new())); + let _subscription = cx.update({ + let events = events.clone(); + |cx| { + cx.subscribe(&bookmark_store, move |_, event: &BookmarkStoreEvent, _| { + events.borrow_mut().push(event.clone()); + }) + } + }); + + let (buffer_id, snapshot) = + buffer.read_with(cx, |buffer, _| (buffer.remote_id(), buffer.text_snapshot())); + let resolved_count = bookmark_store.update(cx, |store, cx| { + store + .bookmarks_for_buffer( + buffer.clone(), + text::Anchor::min_max_range_for_buffer(buffer_id), + &snapshot, + cx, + ) + .len() + }); + assert_eq!( + resolved_count, 1, + "only the in-range bookmark should survive resolution" + ); + assert_eq!( + *events.borrow(), + vec![BookmarkStoreEvent::BookmarksChanged], + "dropping the out-of-range bookmark during resolution should emit BookmarksChanged" + ); + + let resolved_count = bookmark_store.update(cx, |store, cx| { + store + .bookmarks_for_buffer( + buffer.clone(), + text::Anchor::min_max_range_for_buffer(buffer_id), + &snapshot, + cx, + ) + .len() + }); + assert_eq!( + resolved_count, 1, + "an already-resolved entry should keep its bookmark" + ); + assert_eq!( + *events.borrow(), + vec![BookmarkStoreEvent::BookmarksChanged], + "resolution of an already-loaded entry should not emit again" + ); + + bookmark_store.update(cx, |store, cx| { + store.edit_bookmark( + &buffer, + snapshot.anchor_before(Point::new(0, 0)), + "renamed".to_string(), + cx, + ) + }); + assert_eq!( + *events.borrow(), + vec![ + BookmarkStoreEvent::BookmarksChanged, + BookmarkStoreEvent::LabelChanged + ], + "editing a bookmark's label should emit LabelChanged rather than a set change" + ); +} + +#[gpui::test] +async fn test_bookmarks_tab_highlights_bookmark_on_trailing_empty_line(cx: &mut TestAppContext) { + let (workspace, _pane, _project, editor, mut cx) = init_bookmarks_tab_test( + cx, + json!({ + "main.rs": "line0\nline1\n", + }), + ) + .await; + + toggle_bookmark_on_row(&editor, 2, &mut cx); + cx.run_until_parked(); + + let bookmarks_editor = open_and_find_bookmarks_tab(&workspace, &mut cx); + assert_eq!( + highlighted_display_rows_of(&bookmarks_editor, &mut cx), + vec![last_display_row(&bookmarks_editor, &mut cx)], + "a bookmark on the trailing empty line should still highlight its row" + ); +} + +#[gpui::test] +async fn test_bookmarks_tab_keeps_unopenable_paths(cx: &mut TestAppContext) { + let (workspace, _pane, project, editor, mut cx) = init_bookmarks_tab_test( + cx, + json!({ + "main.rs": "line0\nline1\nline2\nline3\nline4\nline5\nline6\nline7\nline8\nline9\n", + "unopenable.rs": {}, + }), + ) + .await; + + let bookmark_store = cx.update(|_window, cx| project.read(cx).bookmark_store()); + let unopenable_path = bookmark_path(path!("/a/unopenable.rs")); + load_test_bookmarks( + &bookmark_store, + vec![ + (bookmark_path(path!("/a/main.rs")), vec![0]), + (Arc::clone(&unopenable_path), vec![0]), + ], + &mut cx, + ) + .await; + + let bookmarks_editor = open_and_find_bookmarks_tab(&workspace, &mut cx); + assert_eq!( + excerpt_count(&bookmarks_editor, &mut cx), + 1, + "only the openable file should produce an excerpt" + ); + assert_eq!( + failed_paths(&bookmark_store, &mut cx), + vec![Arc::clone(&unopenable_path)], + "the unopenable path should be recorded to avoid retrying it on every refresh" + ); + + toggle_bookmark_on_row(&editor, 7, &mut cx); + cx.run_until_parked(); + assert_eq!( + excerpt_count(&bookmarks_editor, &mut cx), + 2, + "refreshes should keep working while an unopenable path is recorded" + ); + + let serialized = bookmark_store.read_with(&cx, |store, cx| store.all_serialized_bookmarks(cx)); + assert_eq!( + serialized.get(&unopenable_path).map(|rows| rows.len()), + Some(1), + "an unopenable path must keep its serialized bookmark instead of losing it" + ); +} + +#[gpui::test] +async fn test_view_bookmarks_activates_most_recent_tab(cx: &mut TestAppContext) { + let (workspace, pane, _project, editor, mut cx) = init_bookmarks_tab_test( + cx, + json!({ + "main.rs": "line0\nline1\nline2\nline3\nline4\nline5\nline6\nline7\nline8\nline9\n", + }), + ) + .await; + + toggle_bookmark_on_row(&editor, 0, &mut cx); + cx.run_until_parked(); + + let bookmarks_editor = open_and_find_bookmarks_tab(&workspace, &mut cx); + let split_editor = bookmarks_editor.update_in(&mut cx, |editor, window, cx| { + cx.new(|cx| editor.clone(window, cx)) + }); + let second_pane = workspace.update_in(&mut cx, |workspace, window, cx| { + workspace.split_pane(pane.clone(), SplitDirection::Right, window, cx) + }); + second_pane.update_in(&mut cx, |pane, window, cx| { + pane.add_item(Box::new(split_editor.clone()), true, true, None, window, cx); + }); + cx.run_until_parked(); + + open_bookmarks_tab(&workspace, &mut cx); + cx.run_until_parked(); + + let active_editor = workspace.update(&mut cx, |workspace, cx| { + workspace.active_item_as::(cx) + }); + assert_eq!( + active_editor, + Some(split_editor), + "view_bookmarks should activate the most recently used bookmarks tab" + ); + let bookmark_editor_count = workspace.update(&mut cx, |workspace, cx| { + workspace + .items_of_type::(cx) + .filter(|editor| editor.read(cx).bookmarks_tab_state.is_some()) + .count() + }); + assert_eq!( + bookmark_editor_count, 2, + "activating an existing tab must not create another one" + ); +} + +#[gpui::test] +async fn test_bookmarks_tab_highlight_does_not_grow_past_last_row(cx: &mut TestAppContext) { + let (workspace, _pane, _project, editor, mut cx) = init_bookmarks_tab_test( + cx, + json!({ + "main.rs": "line0\nline1\nline2", + }), + ) + .await; + + toggle_bookmark_on_row(&editor, 2, &mut cx); + cx.run_until_parked(); + + let bookmarks_editor = open_and_find_bookmarks_tab(&workspace, &mut cx); + + editor.update_in(&mut cx, |editor, window, cx| { + editor.change_selections(SelectionEffects::no_scroll(), window, cx, |s| { + s.select_ranges([Point::new(2, 5)..Point::new(2, 5)]) + }); + editor.insert("\nline3", window, cx); + }); + cx.run_until_parked(); + + assert_eq!( + highlighted_display_rows_of(&bookmarks_editor, &mut cx), + vec![display_row_for( + &bookmarks_editor, + Point::new(2, 0), + &mut cx + )], + "appending lines after a last-row bookmark must not stretch its highlight over the new rows" + ); +} + +#[gpui::test] +async fn test_bookmarks_tab_trailing_line_highlight_does_not_grow_on_edit(cx: &mut TestAppContext) { + let (workspace, _pane, _project, editor, mut cx) = init_bookmarks_tab_test( + cx, + json!({ + "main.rs": "line0\nline1\n", + }), + ) + .await; + + toggle_bookmark_on_row(&editor, 2, &mut cx); + cx.run_until_parked(); + + let bookmarks_editor = open_and_find_bookmarks_tab(&workspace, &mut cx); + + bookmarks_editor.update_in(&mut cx, |editor, window, cx| { + let max_point = editor.buffer.read(cx).snapshot(cx).max_point(); + editor.change_selections(SelectionEffects::no_scroll(), window, cx, |s| { + s.select_ranges([max_point..max_point]) + }); + editor.insert("new0\nnew1", window, cx); + }); + cx.run_until_parked(); + + assert_eq!( + highlighted_display_rows_of(&bookmarks_editor, &mut cx), + vec![last_display_row(&bookmarks_editor, &mut cx)], + "typing at a trailing-line bookmark must move its single-row highlight, not stretch it" + ); +} + +#[gpui::test] +async fn test_bookmarks_tab_last_line_highlight_does_not_cover_next_file_header( + cx: &mut TestAppContext, +) { + let (workspace, _pane, project, editor, mut cx) = init_bookmarks_tab_test( + cx, + json!({ + "main.rs": "line0\nline1\nline2", + "other.rs": "other0\nother1\n", + }), + ) + .await; + + toggle_bookmark_on_row(&editor, 2, &mut cx); + let other_editor = open_bookmarks_test_editor(&workspace, &project, "other.rs", &mut cx).await; + toggle_bookmark_on_row(&other_editor, 0, &mut cx); + cx.run_until_parked(); + + let bookmarks_editor = open_and_find_bookmarks_tab(&workspace, &mut cx); + assert_eq!( + highlighted_display_rows_of(&bookmarks_editor, &mut cx), + vec![ + display_row_for(&bookmarks_editor, Point::new(2, 0), &mut cx), + display_row_for(&bookmarks_editor, Point::new(3, 0), &mut cx), + ], + "a bookmark on a file's last line must not highlight the next file's header rows" + ); +} + +#[gpui::test] +async fn test_bookmarks_tab_trailing_line_highlight_does_not_cover_next_file_header( + cx: &mut TestAppContext, +) { + let (workspace, _pane, project, editor, mut cx) = init_bookmarks_tab_test( + cx, + json!({ + "main.rs": "line0\nline1\n", + "other.rs": "other0\n", + }), + ) + .await; + + toggle_bookmark_on_row(&editor, 2, &mut cx); + let other_editor = open_bookmarks_test_editor(&workspace, &project, "other.rs", &mut cx).await; + toggle_bookmark_on_row(&other_editor, 0, &mut cx); + cx.run_until_parked(); + + let bookmarks_editor = open_and_find_bookmarks_tab(&workspace, &mut cx); + assert_eq!( + highlighted_display_rows_of(&bookmarks_editor, &mut cx), + vec![ + display_row_for(&bookmarks_editor, Point::new(2, 0), &mut cx), + display_row_for(&bookmarks_editor, Point::new(3, 0), &mut cx), + ], + "a bookmark on a trailing empty line must not highlight the next file's header rows" + ); +} + +#[gpui::test] +async fn test_worktree_add_retries_only_contained_failed_paths(cx: &mut TestAppContext) { + let (workspace, _pane, project, _editor, mut cx) = init_bookmarks_tab_test( + cx, + json!({ + "main.rs": "line0\nline1\n", + }), + ) + .await; + + let bookmark_store = cx.update(|_window, cx| project.read(cx).bookmark_store()); + let elsewhere_path = bookmark_path(path!("/elsewhere/file.rs")); + load_test_bookmarks( + &bookmark_store, + vec![ + (bookmark_path(path!("/a/main.rs")), vec![0]), + (Arc::clone(&elsewhere_path), vec![0]), + ], + &mut cx, + ) + .await; + + let bookmarks_editor = open_and_find_bookmarks_tab(&workspace, &mut cx); + assert_eq!( + excerpt_count(&bookmarks_editor, &mut cx), + 1, + "the path outside all worktrees should not produce an excerpt yet" + ); + assert_eq!( + failed_paths(&bookmark_store, &mut cx), + vec![Arc::clone(&elsewhere_path)], + "the path outside all worktrees should be recorded as failed to open" + ); + + let events = Rc::new(RefCell::new(Vec::::new())); + let _subscription = cx.update({ + let events = events.clone(); + |_window, cx| { + cx.subscribe(&bookmark_store, move |_, event: &BookmarkStoreEvent, _| { + events.borrow_mut().push(event.clone()); + }) + } + }); + + let fs = project.read_with(&cx, |project, _| project.fs().clone()); + fs.as_fake() + .insert_tree(path!("/unrelated"), json!({ "other.rs": "other0\n" })) + .await; + project + .update(&mut cx, |project, cx| { + project.create_worktree(path!("/unrelated"), true, cx) + }) + .await + .expect("creating the unrelated worktree should succeed"); + cx.run_until_parked(); + + assert_eq!( + *events.borrow(), + Vec::::new(), + "a worktree containing no failed paths must not trigger a retry" + ); + assert_eq!( + failed_paths(&bookmark_store, &mut cx), + vec![Arc::clone(&elsewhere_path)], + "an unrelated worktree must not clear the failed path cache" + ); + + fs.as_fake() + .insert_tree(path!("/elsewhere"), json!({ "file.rs": "else0\n" })) + .await; + project + .update(&mut cx, |project, cx| { + project.create_worktree(path!("/elsewhere"), true, cx) + }) + .await + .expect("creating the containing worktree should succeed"); + cx.run_until_parked(); + + assert_eq!( + failed_paths(&bookmark_store, &mut cx), + Vec::new(), + "adding a worktree containing a failed path must retry it" + ); + assert_eq!( + excerpt_count(&bookmarks_editor, &mut cx), + 2, + "the bookmarks tab should refresh and show the failed path's bookmark" + ); +} + +#[gpui::test] +async fn test_bookmarks_tab_retries_failed_path_when_file_appears(cx: &mut TestAppContext) { + let (workspace, _pane, project, _editor, mut cx) = init_bookmarks_tab_test( + cx, + json!({ + "main.rs": "line0\nline1\n", + "late.rs": {}, + }), + ) + .await; + + let bookmark_store = cx.update(|_window, cx| project.read(cx).bookmark_store()); + let late_path = bookmark_path(path!("/a/late.rs")); + load_test_bookmarks( + &bookmark_store, + vec![ + (bookmark_path(path!("/a/main.rs")), vec![0]), + (Arc::clone(&late_path), vec![0]), + ], + &mut cx, + ) + .await; + + let bookmarks_editor = open_and_find_bookmarks_tab(&workspace, &mut cx); + assert_eq!( + excerpt_count(&bookmarks_editor, &mut cx), + 1, + "the missing file should not produce an excerpt yet" + ); + assert_eq!( + failed_paths(&bookmark_store, &mut cx), + vec![Arc::clone(&late_path)], + "the missing path should be recorded as failed to open" + ); + + let fs = project.read_with(&cx, |project, _| project.fs().clone()); + fs.remove_dir( + std::path::Path::new(path!("/a/late.rs")), + fs::RemoveOptions { + recursive: true, + ignore_if_not_exists: false, + }, + ) + .await + .expect("removing the placeholder directory should succeed"); + fs.as_fake() + .insert_file(path!("/a/late.rs"), "late0\n".as_bytes().to_vec()) + .await; + cx.run_until_parked(); + + assert_eq!( + failed_paths(&bookmark_store, &mut cx), + Vec::new(), + "a failed path must be retried once its file appears on disk" + ); + assert_eq!( + excerpt_count(&bookmarks_editor, &mut cx), + 2, + "the bookmarks tab should refresh and show the late file's bookmark" + ); +} + #[gpui::test] async fn test_rename_with_duplicate_edits(cx: &mut TestAppContext) { init_test(cx, |_| {}); @@ -44375,6 +45076,232 @@ async fn assert_range_format_merge( assert!(!cx.read(|cx| editor.is_dirty(cx))); } +async fn init_bookmarks_tab_test( + cx: &mut TestAppContext, + files: serde_json::Value, +) -> ( + Entity, + Entity, + Entity, + Entity, + VisualTestContext, +) { + init_test(cx, |_| {}); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree(path!("/a"), files).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 workspace = window + .read_with(cx, |multi_workspace, _| multi_workspace.workspace().clone()) + .expect("window should still be open"); + let mut cx = VisualTestContext::from_window(*window, cx); + let editor = open_bookmarks_test_editor(&workspace, &project, "main.rs", &mut cx).await; + let pane = workspace.update(&mut cx, |workspace, _| workspace.active_pane().clone()); + (workspace, pane, project, editor, cx) +} + +async fn open_bookmarks_test_editor( + workspace: &Entity, + project: &Entity, + path: &str, + cx: &mut VisualTestContext, +) -> Entity { + let worktree_id = project.update(cx, |project, cx| { + project + .worktrees(cx) + .next() + .expect("project should have at least one worktree") + .read(cx) + .id() + }); + let buffer = project + .update(cx, |project, cx| { + project.open_buffer((worktree_id, rel_path(path)), cx) + }) + .await + .expect("buffer should open"); + let editor = workspace.update_in(cx, |_workspace, window, cx| { + let multibuffer = MultiBuffer::build_from_buffer(buffer, cx); + cx.new(|cx| { + Editor::new( + EditorMode::full(), + multibuffer, + Some(project.clone()), + window, + cx, + ) + }) + }); + let pane = workspace.update(cx, |workspace, _| workspace.active_pane().clone()); + pane.update_in(cx, |pane, window, cx| { + pane.add_item(Box::new(editor.clone()), true, true, None, window, cx); + }); + editor +} + +fn open_bookmarks_tab(workspace: &Entity, cx: &mut VisualTestContext) { + workspace.update_in(cx, |workspace, window, cx| { + Editor::view_bookmarks(workspace, &actions::ViewBookmarks, window, cx); + }); +} + +fn toggle_bookmark_on_row(editor: &Entity, row: u32, cx: &mut VisualTestContext) { + editor.update_in(cx, |editor, window, cx| { + editor.change_selections(SelectionEffects::no_scroll(), window, cx, |s| { + s.select_ranges([Point::new(row, 0)..Point::new(row, 0)]) + }); + editor.toggle_bookmark(&actions::ToggleBookmark, window, cx); + }); +} + +fn find_bookmarks_editor( + workspace: &Entity, + cx: &mut VisualTestContext, +) -> Option> { + workspace.update(cx, |workspace, cx| { + workspace + .items_of_type::(cx) + .find(|editor| editor.read(cx).bookmarks_tab_state.is_some()) + }) +} + +fn excerpt_count(editor: &Entity, cx: &mut VisualTestContext) -> usize { + editor.update(cx, |editor, cx| { + editor.buffer.read(cx).snapshot(cx).excerpts().count() + }) +} + +fn bookmarked_paths(editor: &Entity, cx: &mut VisualTestContext) -> Vec { + editor.update(cx, |editor, cx| { + let mut paths = editor + .buffer + .read(cx) + .snapshot(cx) + .buffers_with_paths() + .map(|(_, path_key)| path_key.path.as_unix_str().to_string()) + .collect::>(); + paths.sort(); + paths + }) +} + +fn cursor_line_text(editor: &Entity, cx: &mut VisualTestContext) -> String { + editor.update(cx, |editor, cx| { + let snapshot = editor.buffer.read(cx).snapshot(cx); + let head = editor + .selections + .newest::(&editor.display_snapshot(cx)) + .head(); + snapshot + .text_for_range(Point::new(head.row, 0)..Point::new(head.row + 1, 0)) + .collect::() + .trim_end() + .to_string() + }) +} + +fn close_bookmarks_editor( + pane: &Entity, + bookmarks_editor: Entity, + cx: &mut VisualTestContext, +) { + let weak_bookmarks_editor = bookmarks_editor.downgrade(); + pane.update_in(cx, |pane, window, cx| { + pane.remove_item(weak_bookmarks_editor.entity_id(), false, false, window, cx); + }); + drop(bookmarks_editor); + cx.executor() + .advance_clock(workspace::SERIALIZATION_THROTTLE_TIME); + cx.run_until_parked(); + cx.update(|_window, _cx| {}); + weak_bookmarks_editor.assert_released(); +} + +fn open_and_find_bookmarks_tab( + workspace: &Entity, + cx: &mut VisualTestContext, +) -> Entity { + open_bookmarks_tab(workspace, cx); + cx.run_until_parked(); + find_bookmarks_editor(workspace, cx) + .expect("bookmarks editor should exist after view_bookmarks") +} + +fn bookmark_path(path: &str) -> Arc { + Arc::from(std::path::Path::new(path)) +} + +fn serialized_rows(rows: &[u32]) -> Vec { + rows.iter() + .map(|&row| SerializedBookmark { + row, + label: String::new(), + }) + .collect() +} + +async fn load_test_bookmarks( + bookmark_store: &Entity, + entries: Vec<(Arc, Vec)>, + cx: &mut VisualTestContext, +) { + let entries = entries + .into_iter() + .map(|(path, rows)| (path, serialized_rows(&rows))) + .collect::>(); + bookmark_store + .update(cx, |store, cx| store.load_serialized_bookmarks(entries, cx)) + .await + .expect("loading serialized bookmarks should succeed"); +} + +fn failed_paths( + bookmark_store: &Entity, + cx: &mut VisualTestContext, +) -> Vec> { + bookmark_store.read_with(cx, |store, _| { + let mut paths = store + .paths_failed_to_open() + .iter() + .cloned() + .collect::>(); + paths.sort(); + paths + }) +} + +fn highlighted_display_rows_of( + editor: &Entity, + cx: &mut VisualTestContext, +) -> Vec { + editor.update_in(cx, |editor, window, cx| { + editor + .highlighted_display_rows(window, cx) + .into_keys() + .collect() + }) +} + +fn display_row_for( + editor: &Entity, + point: Point, + cx: &mut VisualTestContext, +) -> DisplayRow { + editor.update(cx, |editor, cx| { + editor + .display_snapshot(cx) + .point_to_display_point(point, text::Bias::Left) + .row() + }) +} + +fn last_display_row(editor: &Entity, cx: &mut VisualTestContext) -> DisplayRow { + let last_point = editor.update(cx, |editor, cx| { + editor.buffer.read(cx).snapshot(cx).max_point() + }); + display_row_for(editor, last_point, cx) +} #[gpui::test] async fn test_scroll_range_hold_freezes_before_first_settled_frame(cx: &mut TestAppContext) { init_test(cx, |_| {}); diff --git a/crates/editor/src/navigation.rs b/crates/editor/src/navigation.rs index 2c1607e71e..1c075e72bd 100644 --- a/crates/editor/src/navigation.rs +++ b/crates/editor/src/navigation.rs @@ -2207,7 +2207,9 @@ impl Editor { let excerpt_buffer = cx.new(|cx| { let key = &mut key.1; let mut multibuffer = MultiBuffer::new(capability); - for (buffer, mut ranges_for_buffer) in locations { + let mut sorted_locations = locations.into_iter().collect::>(); + sorted_locations.sort_by_key(|(buffer, _)| buffer.read(cx).remote_id()); + for (buffer, mut ranges_for_buffer) in sorted_locations { ranges_for_buffer.sort_by_key(|range| (range.start, Reverse(range.end))); key.push((buffer.read(cx).remote_id(), ranges_for_buffer.clone())); multibuffer.set_excerpts_for_path( diff --git a/crates/project/src/bookmark_store.rs b/crates/project/src/bookmark_store.rs index 25c462359e..7b23d85382 100644 --- a/crates/project/src/bookmark_store.rs +++ b/crates/project/src/bookmark_store.rs @@ -1,14 +1,25 @@ use std::{collections::BTreeMap, ops::Range, path::Path, sync::Arc}; use anyhow::Result; -use futures::{StreamExt, TryFutureExt, TryStreamExt, stream::FuturesUnordered}; -use gpui::{App, AppContext, Context, Entity, Subscription, Task}; +use collections::{HashMap, HashSet}; +use futures::{StreamExt, TryFutureExt, stream::FuturesUnordered}; +use gpui::{App, AppContext, Context, Entity, EventEmitter, Subscription, Task}; use itertools::Itertools; use language::{Buffer, BufferEvent}; -use std::collections::HashMap; use text::{BufferSnapshot, Point}; +use worktree::PathChange; -use crate::{ProjectPath, buffer_store::BufferStore, worktree_store::WorktreeStore}; +use crate::{ + ProjectPath, + buffer_store::BufferStore, + worktree_store::{WorktreeStore, WorktreeStoreEvent}, +}; + +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum BookmarkStoreEvent { + BookmarksChanged, + LabelChanged, +} #[derive(Clone, Debug)] pub struct Bookmark { @@ -83,23 +94,116 @@ pub struct BookmarkStore { buffer_store: Entity, worktree_store: Entity, bookmarks: BTreeMap, BookmarkEntry>, + paths_failed_to_open: HashSet>, + _worktree_store_subscription: Subscription, } +impl EventEmitter for BookmarkStore {} + impl BookmarkStore { - pub fn new(worktree_store: Entity, buffer_store: Entity) -> Self { + pub fn new( + worktree_store: Entity, + buffer_store: Entity, + cx: &mut Context, + ) -> Self { + let worktree_store_subscription = + cx.subscribe(&worktree_store, Self::handle_worktree_store_event); Self { buffer_store, worktree_store, bookmarks: BTreeMap::new(), + paths_failed_to_open: HashSet::default(), + _worktree_store_subscription: worktree_store_subscription, } } + fn handle_worktree_store_event( + &mut self, + _worktree_store: Entity, + event: &WorktreeStoreEvent, + cx: &mut Context, + ) { + if self.paths_failed_to_open.is_empty() { + return; + } + match event { + WorktreeStoreEvent::WorktreeAdded(worktree) => { + let worktree_abs_path = worktree.read(cx).abs_path(); + let previous_len = self.paths_failed_to_open.len(); + self.paths_failed_to_open + .retain(|path| !path.starts_with(&worktree_abs_path)); + if self.paths_failed_to_open.len() < previous_len { + Self::emit_bookmarks_changed(cx); + } + } + WorktreeStoreEvent::WorktreeUpdatedEntries(worktree_id, entries) => { + let Some(worktree) = self + .worktree_store + .read(cx) + .worktree_for_id(*worktree_id, cx) + else { + return; + }; + let worktree_abs_path = worktree.read(cx).abs_path(); + let failed_by_rel_path = self + .paths_failed_to_open + .iter() + .filter_map(|abs_path| { + abs_path + .strip_prefix(&worktree_abs_path) + .ok() + .map(|rel_path| (rel_path.to_owned(), abs_path.clone())) + }) + .collect::>(); + if failed_by_rel_path.is_empty() { + return; + } + let mut removed_any = false; + for (rel_path, _, change) in entries.iter() { + if *change == PathChange::Removed { + continue; + } + if let Some(abs_path) = failed_by_rel_path.get(rel_path.as_std_path()) { + removed_any |= self.paths_failed_to_open.remove(abs_path.as_ref()); + } + } + if removed_any { + Self::emit_bookmarks_changed(cx); + } + } + _ => {} + } + } + + fn emit_bookmarks_changed(cx: &mut Context) { + cx.emit(BookmarkStoreEvent::BookmarksChanged); + cx.notify(); + } + + #[cfg(any(test, feature = "test-support"))] + pub fn paths_failed_to_open(&self) -> &HashSet> { + &self.paths_failed_to_open + } + + pub fn forget_failed_paths(&mut self, cx: &mut Context) { + if self.paths_failed_to_open.is_empty() { + return; + } + self.paths_failed_to_open.clear(); + Self::emit_bookmarks_changed(cx); + } + + pub fn is_empty(&self) -> bool { + self.bookmarks.values().all(BookmarkEntry::is_empty) + } + pub fn load_serialized_bookmarks( &mut self, bookmark_rows: BTreeMap, Vec>, cx: &mut Context, ) -> Task> { self.bookmarks.clear(); + self.paths_failed_to_open.clear(); for (path, rows) in bookmark_rows { if rows.is_empty() { @@ -112,7 +216,7 @@ impl BookmarkStore { self.bookmarks.insert(path, BookmarkEntry::Unloaded(rows)); } - cx.notify(); + Self::emit_bookmarks_changed(cx); Task::ready(Ok(())) } @@ -126,6 +230,7 @@ impl BookmarkStore { return; }; + let unresolved_count = bookmarks.len(); let snapshot = buffer.read(cx).snapshot(); let max_point = snapshot.max_point(); @@ -152,6 +257,9 @@ impl BookmarkStore { }) .collect(); + let was_failed_to_open = self.paths_failed_to_open.remove(abs_path); + + let resolved_count = bookmarks.len(); if bookmarks.is_empty() { self.bookmarks.remove(abs_path); } else { @@ -160,6 +268,10 @@ impl BookmarkStore { self.bookmarks .insert(abs_path.clone(), BookmarkEntry::Loaded(buffer_bookmarks)); } + + if was_failed_to_open || resolved_count < unresolved_count { + Self::emit_bookmarks_changed(cx); + } } pub fn abs_path_from_buffer(buffer: &Entity, cx: &App) -> Option> { @@ -209,7 +321,7 @@ impl BookmarkStore { buffer_bookmarks.bookmarks.push(Bookmark { anchor, label }); } - cx.notify(); + Self::emit_bookmarks_changed(cx); } pub fn find_bookmark( @@ -218,19 +330,12 @@ impl BookmarkStore { anchor: text::Anchor, cx: &mut Context, ) -> Option<&Bookmark> { - let Some(abs_path) = Self::abs_path_from_buffer(buffer, cx) else { - return None; - }; + let abs_path = Self::abs_path_from_buffer(buffer, cx)?; self.resolve_anchors_if_needed(&abs_path, buffer, cx); - let entry = self - .bookmarks - .entry(abs_path.clone()) - .or_insert_with(|| BookmarkEntry::Loaded(BufferBookmarks::new(buffer.clone(), cx))); - - let BookmarkEntry::Loaded(buffer_bookmarks) = entry else { - unreachable!("resolve_if_needed should have converted to Loaded"); + let BookmarkEntry::Loaded(buffer_bookmarks) = self.bookmarks.get(&abs_path)? else { + return None; }; let snapshot = buffer.read(cx).text_snapshot(); @@ -268,6 +373,7 @@ impl BookmarkStore { .find(|existing| existing.anchor.summary::(&snapshot).row == row) { bookmark.label = label; + cx.emit(BookmarkStoreEvent::LabelChanged); cx.notify(); } } @@ -327,7 +433,7 @@ impl BookmarkStore { } BookmarkEntry::Unloaded(_) => true, }); - cx.notify(); + Self::emit_bookmarks_changed(cx); return; } @@ -355,7 +461,7 @@ impl BookmarkStore { return; }; self.bookmarks.insert(new_abs_path, entry); - cx.notify(); + Self::emit_bookmarks_changed(cx); } } } @@ -408,8 +514,11 @@ impl BookmarkStore { Self::resolve_all(&this, cx).await?; cx.read_entity(&this, |this, cx| { - let mut locations: HashMap<_, Vec<_>> = HashMap::new(); + let mut locations: HashMap<_, Vec<_>> = HashMap::default(); for bookmarks in this.bookmarks.values().filter_map(BookmarkEntry::loaded) { + if bookmarks.bookmarks().is_empty() { + continue; + } let snapshot = cx.read_entity(bookmarks.buffer(), |b, _| b.snapshot()); let ranges: Vec> = bookmarks .bookmarks() @@ -436,8 +545,10 @@ impl BookmarkStore { this.bookmarks .iter() .filter_map(|(path, entry)| match entry { - BookmarkEntry::Unloaded(_) => Some(path.clone()), - BookmarkEntry::Loaded(_) => None, + BookmarkEntry::Unloaded(_) if !this.paths_failed_to_open.contains(path) => { + Some(path.clone()) + } + _ => None, }) .collect_vec() }); @@ -458,20 +569,22 @@ impl BookmarkStore { }) .collect(); - let opened: Vec<_> = open_tasks - .inspect_err(|(path, error)| { - log::warn!( - "Could not open buffer for bookmarked path {}: {error}", - path.display() - ) - }) - .filter_map(|res| async move { res.ok() }) - .collect() - .await; + let results: Vec<_> = open_tasks.collect().await; cx.update_entity(&this, |this, cx| { - for (path, buffer) in opened { - this.resolve_anchors_if_needed(&path, &buffer, cx); + for result in results { + match result { + Ok((path, buffer)) => this.resolve_anchors_if_needed(path, &buffer, cx), + Err((path, error)) => { + log::warn!( + "Could not open buffer for bookmarked path {}: {error}", + path.display() + ); + if this.bookmarks.contains_key(path) { + this.paths_failed_to_open.insert(path.clone()); + } + } + } } cx.notify(); }); @@ -481,7 +594,8 @@ impl BookmarkStore { pub fn clear_bookmarks(&mut self, cx: &mut Context) { self.bookmarks.clear(); - cx.notify(); + self.paths_failed_to_open.clear(); + Self::emit_bookmarks_changed(cx); } } @@ -508,5 +622,14 @@ async fn open_path( }) .await?; + let exists_on_disk = cx.read_entity(&buffer, |buffer, _| { + buffer.file().is_some_and(|file| file.disk_state().exists()) + }); + anyhow::ensure!( + exists_on_disk, + "bookmarked file {} does not exist on disk", + path.display() + ); + Ok(buffer) } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index d2d55aaac9..f4c4ef2718 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -1262,7 +1262,7 @@ impl Project { .detach(); let bookmark_store = - cx.new(|_| BookmarkStore::new(worktree_store.clone(), buffer_store.clone())); + cx.new(|cx| BookmarkStore::new(worktree_store.clone(), buffer_store.clone(), cx)); let breakpoint_store = cx.new(|_| BreakpointStore::local(worktree_store.clone(), buffer_store.clone())); @@ -1517,7 +1517,7 @@ impl Project { cx.subscribe(&lsp_store, Self::on_lsp_store_event).detach(); let bookmark_store = - cx.new(|_| BookmarkStore::new(worktree_store.clone(), buffer_store.clone())); + cx.new(|cx| BookmarkStore::new(worktree_store.clone(), buffer_store.clone(), cx)); let breakpoint_store = cx.new(|_| { BreakpointStore::remote( @@ -1780,7 +1780,7 @@ impl Project { cx.new(|cx| ProjectEnvironment::new(None, worktree_store.downgrade(), None, true, cx)); let bookmark_store = - cx.new(|_| BookmarkStore::new(worktree_store.clone(), buffer_store.clone())); + cx.new(|cx| BookmarkStore::new(worktree_store.clone(), buffer_store.clone(), cx)); let breakpoint_store = cx.new(|_| { BreakpointStore::remote(