From fcaf4383e9b0a95ccbb7d9de035155ef6c69a38c Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Thu, 28 Mar 2024 18:33:57 +0100 Subject: [PATCH] editor: Preserve scroll position when jumping from multibuffer (#9921) This is a best-effort attempt, as the target offset from the top is just an estimate; furthermore, this does not account for things like project search header (which adds a bit of vertical offset by itself and is removed once we jump into a buffer), but it still should improve the situation quite a bit. Fixes: #5296 Release Notes: - Improved target selection when jumping from multibuffer; final position in the buffer should more closely match the original position of the cursor in the multibuffer. --- crates/editor/src/editor.rs | 15 +++++++++----- crates/editor/src/element.rs | 27 ++++++++++++++++++++++++-- crates/editor/src/scroll/autoscroll.rs | 9 +++++++++ 3 files changed, 44 insertions(+), 7 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 17928bf3a8..9b6d5d6bf7 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -7670,7 +7670,7 @@ impl Editor { let range = target.range.to_offset(target.buffer.read(cx)); let range = editor.range_for_match(&range); if Some(&target.buffer) == editor.buffer.read(cx).as_singleton().as_ref() { - editor.change_selections(Some(Autoscroll::fit()), cx, |s| { + editor.change_selections(Some(Autoscroll::focused()), cx, |s| { s.select_ranges([range]); }); } else { @@ -7690,7 +7690,7 @@ impl Editor { // to avoid creating a history entry at the previous cursor location. pane.update(cx, |pane, _| pane.disable_history()); target_editor.change_selections( - Some(Autoscroll::fit()), + Some(Autoscroll::focused()), cx, |s| { s.select_ranges([range]); @@ -9504,6 +9504,7 @@ impl Editor { path: ProjectPath, position: Point, anchor: language::Anchor, + offset_from_top: u32, cx: &mut ViewContext, ) { let workspace = self.workspace(); @@ -9531,9 +9532,13 @@ impl Editor { }; let nav_history = editor.nav_history.take(); - editor.change_selections(Some(Autoscroll::newest()), cx, |s| { - s.select_ranges([cursor..cursor]); - }); + editor.change_selections( + Some(Autoscroll::top_relative(offset_from_top as usize)), + cx, + |s| { + s.select_ranges([cursor..cursor]); + }, + ); editor.nav_history = nav_history; anyhow::Ok(()) diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 58de6c9ce8..2f4a26ef48 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -1406,6 +1406,7 @@ impl EditorElement { let render_block = |block: &TransformBlock, available_space: Size, block_id: usize, + block_row_start: u32, cx: &mut ElementContext| { let mut element = match block { TransformBlock::Custom(block) => { @@ -1440,6 +1441,7 @@ impl EditorElement { buffer, range, starts_new_buffer, + height, .. } => { let include_root = self @@ -1455,6 +1457,7 @@ impl EditorElement { position: Point, anchor: text::Anchor, path: ProjectPath, + line_offset_from_top: u32, } let jump_data = project::File::from_dyn(buffer.file()).map(|file| { @@ -1466,12 +1469,29 @@ impl EditorElement { .primary .as_ref() .map_or(range.context.start, |primary| primary.start); + + let excerpt_start = range.context.start; let jump_position = language::ToPoint::to_point(&jump_anchor, buffer); + let offset_from_excerpt_start = if jump_anchor == excerpt_start { + 0 + } else { + let excerpt_start_row = + language::ToPoint::to_point(&jump_anchor, buffer).row; + jump_position.row - excerpt_start_row + }; + + let line_offset_from_top = + block_row_start + *height as u32 + offset_from_excerpt_start + - snapshot + .scroll_anchor + .scroll_position(&snapshot.display_snapshot) + .y as u32; JumpData { position: jump_position, anchor: jump_anchor, path: jump_path, + line_offset_from_top, } }); @@ -1541,6 +1561,7 @@ impl EditorElement { jump_data.path.clone(), jump_data.position, jump_data.anchor, + jump_data.line_offset_from_top, cx, ); } @@ -1599,6 +1620,7 @@ impl EditorElement { path.clone(), jump_data.position, jump_data.anchor, + jump_data.line_offset_from_top, cx, ); } @@ -1631,6 +1653,7 @@ impl EditorElement { path.clone(), jump_data.position, jump_data.anchor, + jump_data.line_offset_from_top, cx, ); } @@ -1663,7 +1686,7 @@ impl EditorElement { AvailableSpace::MinContent, AvailableSpace::Definite(block.height() as f32 * line_height), ); - let (element, element_size) = render_block(block, available_space, block_id, cx); + let (element, element_size) = render_block(block, available_space, block_id, row, cx); block_id += 1; fixed_block_max_width = fixed_block_max_width.max(element_size.width + em_width); blocks.push(BlockLayout { @@ -1691,7 +1714,7 @@ impl EditorElement { AvailableSpace::Definite(width), AvailableSpace::Definite(block.height() as f32 * line_height), ); - let (element, _) = render_block(block, available_space, block_id, cx); + let (element, _) = render_block(block, available_space, block_id, row, cx); block_id += 1; blocks.push(BlockLayout { row, diff --git a/crates/editor/src/scroll/autoscroll.rs b/crates/editor/src/scroll/autoscroll.rs index 962395e369..866eb2b070 100644 --- a/crates/editor/src/scroll/autoscroll.rs +++ b/crates/editor/src/scroll/autoscroll.rs @@ -32,6 +32,10 @@ impl Autoscroll { pub fn focused() -> Self { Self::Strategy(AutoscrollStrategy::Focused) } + /// Scrolls so that the newest cursor is roughly an n-th line from the top. + pub fn top_relative(n: usize) -> Self { + Self::Strategy(AutoscrollStrategy::TopRelative(n)) + } } #[derive(PartialEq, Eq, Default, Clone, Copy)] @@ -43,6 +47,7 @@ pub enum AutoscrollStrategy { Focused, Top, Bottom, + TopRelative(usize), } impl AutoscrollStrategy { @@ -178,6 +183,10 @@ impl Editor { scroll_position.y = (target_bottom - visible_lines).max(0.0); self.set_scroll_position_internal(scroll_position, local, true, cx); } + AutoscrollStrategy::TopRelative(lines) => { + scroll_position.y = target_top - lines as f32; + self.set_scroll_position_internal(scroll_position, local, true, cx); + } } self.scroll_manager.last_autoscroll = Some((