diff --git a/assets/settings/default.json b/assets/settings/default.json index 464587e6290..02d93ece4a3 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -361,6 +361,8 @@ // 1. Vertically center the target in the viewport: `center` (default) // 2. Scroll the minimum amount needed to make the target visible: `minimum` // 3. Scroll so the target appears near the top of the viewport: `top` + // 4. Preserve the cursor's vertical position within the viewport, falling back to `center` when the cursor is + // offscreen: `preserve` "go_to_definition_scroll_strategy": "center", // Which level to use to filter out diagnostics displayed in the editor. // diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index ee6b2be18b8..42e4ebb606d 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -18340,7 +18340,11 @@ impl Editor { }; let anchor_range = range.to_anchors(&multibuffer.snapshot(cx)); self.change_selections( - SelectionEffects::scroll(Autoscroll::for_go_to_definition(cx)).nav_history(true), + SelectionEffects::scroll(Autoscroll::for_go_to_definition( + self.cursor_top_offset(cx), + cx, + )) + .nav_history(true), window, cx, |s| s.select_anchor_ranges([anchor_range]), @@ -19146,8 +19150,11 @@ impl Editor { } editor.change_selections( - SelectionEffects::scroll(Autoscroll::for_go_to_definition(cx)) - .nav_history(true), + SelectionEffects::scroll(Autoscroll::for_go_to_definition( + editor.cursor_top_offset(cx), + cx, + )) + .nav_history(true), window, cx, |s| s.select_anchor_ranges(target_ranges), @@ -19163,6 +19170,8 @@ impl Editor { return Navigated::No; }; let pane = workspace.read(cx).active_pane().clone(); + let offset = editor.cursor_top_offset(cx); + window.defer(cx, move |window, cx| { let (target_editor, target_pane): (Entity, Entity) = workspace.update(cx, |workspace, cx| { @@ -19226,8 +19235,10 @@ impl Editor { } target_editor.change_selections( - SelectionEffects::scroll(Autoscroll::for_go_to_definition(cx)) - .nav_history(true), + SelectionEffects::scroll(Autoscroll::for_go_to_definition( + offset, cx, + )) + .nav_history(true), window, cx, |s| s.select_anchor_ranges(target_ranges), @@ -19507,7 +19518,10 @@ impl Editor { let Range { start, end } = locations[destination_location_index]; editor.update_in(cx, |editor, window, cx| { - let effects = SelectionEffects::scroll(Autoscroll::for_go_to_definition(cx)); + let effects = SelectionEffects::scroll(Autoscroll::for_go_to_definition( + editor.cursor_top_offset(cx), + cx, + )); editor.unfold_ranges(&[start..end], false, false, cx); editor.change_selections(effects, window, cx, |s| { @@ -25672,7 +25686,7 @@ impl Editor { } let autoscroll = match scroll_offset { Some(scroll_offset) => { - Autoscroll::top_relative(scroll_offset as usize) + Autoscroll::top_relative(scroll_offset as ScrollOffset) } None => Autoscroll::newest(), }; @@ -26747,6 +26761,27 @@ impl Editor { self.refresh_runnables(None, window, cx); } } + + /// Returns the current cursor's vertical offset, in display rows, from the + /// top of the visible viewport. + /// Returns `None` if the cursor is not currently on screen. + pub fn cursor_top_offset(&self, cx: &mut Context) -> Option { + let visible = self.visible_line_count()?; + let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx)); + let scroll_top = self.scroll_manager.scroll_position(&display_map, cx).y; + let cursor_display_row = self + .selections + .newest::(&display_map) + .head() + .to_display_point(&display_map) + .row() + .as_f64(); + + match cursor_display_row - scroll_top { + offset if offset < 0.0 || offset >= visible => None, + offset => Some(offset), + } + } } fn edit_for_markdown_paste<'a>( diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 3a23bb1147b..b0ea1b86740 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -49,9 +49,9 @@ use project::{ use serde_json::{self, json}; use settings::{ AllLanguageSettingsContent, DelayMs, EditorSettingsContent, GlobalLspSettingsContent, - IndentGuideBackgroundColoring, IndentGuideColoring, InlayHintSettingsContent, - ProjectSettingsContent, ScrollBeyondLastLine, SearchSettingsContent, SettingsContent, - SettingsStore, + GoToDefinitionScrollStrategy, IndentGuideBackgroundColoring, IndentGuideColoring, + InlayHintSettingsContent, ProjectSettingsContent, ScrollBeyondLastLine, SearchSettingsContent, + SettingsContent, SettingsStore, }; use std::{borrow::Cow, sync::Arc}; use std::{cell::RefCell, future::Future, rc::Rc, sync::atomic::AtomicBool, time::Instant}; @@ -2962,6 +2962,102 @@ async fn test_autoscroll(cx: &mut TestAppContext) { }); } +#[gpui::test] +async fn test_autoscroll_relative(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + let mut cx = EditorTestContext::new(cx).await; + + let line_height = cx.update_editor(|editor, window, cx| { + editor.set_vertical_scroll_margin(0, cx); + editor + .style(cx) + .text + .line_height_in_pixels(window.rem_size()) + }); + let window = cx.window; + + // Resize the window such that only 6 lines of text fit on screen. + cx.simulate_window_resize(window, size(px(1000.), 6. * line_height)); + + cx.set_state( + r#"ˇone + two + three + four + five + six + seven + eight + nine + ten + eleven + twelve + thirteen + fourteen + fifteen + "#, + ); + cx.update_editor(|editor, window, cx| { + assert_eq!( + editor.snapshot(window, cx).scroll_position(), + gpui::Point::new(0., 0.0) + ); + }); + + // Placing the cursor at row 7 with a top-relative autoscroll of 2 display + // rows, should land the scroll position's y coordinate at 5.0 (7 - 2). + cx.update_editor(|editor, window, cx| { + editor.change_selections( + SelectionEffects::scroll(Autoscroll::top_relative(2.0)), + window, + cx, + |selections| selections.select_ranges([Point::new(7, 0)..Point::new(7, 0)]), + ); + }); + cx.update_editor(|editor, window, cx| { + assert_eq!( + editor.snapshot(window, cx).scroll_position(), + gpui::Point::new(0., 5.0) + ); + }); + + // Seeing as fractional offsets are supported, with the cursor at row 10 and + // a top-relative autoscroll of 2.5 display rows, the scroll position's y + // coordinate lands at 7.5 (10 - 2.5). + cx.update_editor(|editor, window, cx| { + editor.change_selections( + SelectionEffects::scroll(Autoscroll::top_relative(2.5)), + window, + cx, + |selections| selections.select_ranges([Point::new(10, 0)..Point::new(10, 0)]), + ); + }); + cx.update_editor(|editor, window, cx| { + assert_eq!( + editor.snapshot(window, cx).scroll_position(), + gpui::Point::new(0., 7.5) + ); + }); + + // When the requested offset would scroll past the top of the buffer, + // `scroll_position.y` is clamped to 0 rather than going negative. + cx.update_editor(|editor, window, cx| { + editor.change_selections( + SelectionEffects::scroll(Autoscroll::top_relative(4.0)), + window, + cx, + |selections| selections.select_ranges([Point::new(1, 0)..Point::new(1, 0)]), + ); + }); + + cx.update_editor(|editor, window, cx| { + assert_eq!( + editor.snapshot(window, cx).scroll_position(), + gpui::Point::new(0., 0.0) + ); + }); +} + #[gpui::test] async fn test_exclude_overscroll_margin_clamps_scroll_position(cx: &mut TestAppContext) { init_test(cx, |_| {}); @@ -26371,6 +26467,142 @@ async fn test_goto_definition_contained_ranges(cx: &mut TestAppContext) { assert_eq!(navigated, Navigated::Yes); } +#[gpui::test] +async fn test_goto_definition_preserve_scroll_strategy(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + update_test_editor_settings(cx, &|settings| { + settings.go_to_definition_scroll_strategy = Some(GoToDefinitionScrollStrategy::Preserve); + settings.vertical_scroll_margin = Some(0.0); + }); + + let mut cx = EditorLspTestContext::new_rust( + lsp::ServerCapabilities { + definition_provider: Some(lsp::OneOf::Left(true)), + ..lsp::ServerCapabilities::default() + }, + cx, + ) + .await; + + let window = cx.window; + let line_height = cx.update_editor(|editor, window, cx| { + editor + .style(cx) + .text + .line_height_in_pixels(window.rem_size()) + }); + cx.simulate_window_resize(window, size(px(1000.), 8. * line_height)); + + // Build a buffer where `target` is defined on row 10 and called from + // row 20, with the cursor placed on the call site. + let buffer = indoc! { " + // 0 + // 1 + // 2 + // 3 + // 4 + // 5 + // 6 + // 7 + // 8 + // 9 + fn target() // 10 + // 11 + // 12 + // 13 + // 14 + // 15 + // 16 + // 17 + // 18 + // 19 + fn caller() { ˇtarget(); } // 20 + // 21 + // 22 + // 23 + // 24 + // 25 + // 26 + // 27 + // 28 + // 29 + // 30 + "}; + + // Mock the response from the LSP server when requesting to go to a + // definition so as to always jump to the `target` function. + cx.set_request_handler::(|url, _, _| async move { + Ok(Some(lsp::GotoDefinitionResponse::Scalar(lsp::Location { + uri: url.clone(), + range: lsp::Range::new(lsp::Position::new(10, 3), lsp::Position::new(10, 9)), + }))) + }); + + let caller_row = 20.0; + let target_row = 10.0; + let offset = 1.5; + let center_offset = cx.update_editor(|editor, _, _| { + editor + .visible_line_count() + .map(|count| ((count - 1.0) / 2.0).floor()) + .expect("Visible line count should be available") + }); + + // When the cursor is visible inside the viewport, going to a definition + // should preserve that same offset value. + // In this case, with the cursor set at row 20 and the scroll position set + // to 18.5 (20 - 1.5), when going to the definition of `target` in row 10, + // the scroll position should end up at 8.5 (10 - 1.5), so as to preserve + // that same offset of 1.5. + cx.set_state(&buffer); + cx.update_editor(|editor, window, cx| { + editor.set_scroll_position(gpui::Point::new(0.0, caller_row - offset), window, cx); + }); + cx.update_editor(|editor, window, cx| editor.go_to_definition(&GoToDefinition, window, cx)) + .await + .expect("Failed to navigate to definition"); + cx.run_until_parked(); + cx.update_editor(|editor, window, cx| { + assert_eq!( + editor.snapshot(window, cx).scroll_position(), + gpui::Point::new(0.0, target_row - offset), + ); + }); + + // In the case where the cursor ends up outside of the visible viewport, the + // scroll position's offset should be ignored and the center of the viewport + // should be used instead. + // Since the cursor is jumping to row 10, the scroll position's y coordinate + // should end up at 10 minus the offset from the center of the viewport. + cx.set_state(&buffer); + cx.update_editor(|editor, window, cx| { + editor.set_scroll_position(gpui::Point::new(0.0, 0.0), window, cx); + let snapshot = editor.display_snapshot(cx); + let cursor_row = editor + .selections + .newest_display(&snapshot) + .start + .row() + .as_f64(); + let visible_lines = editor + .visible_line_count() + .expect("Visible line count should be available"); + + assert!(cursor_row >= visible_lines, "Cursor should be offscreen"); + }); + + cx.update_editor(|editor, window, cx| editor.go_to_definition(&GoToDefinition, window, cx)) + .await + .expect("Failed to navigate to definition"); + cx.run_until_parked(); + cx.update_editor(|editor, window, cx| { + assert_eq!( + editor.snapshot(window, cx).scroll_position(), + gpui::Point::new(0.0, (target_row - center_offset).max(0.0)), + ); + }); +} + #[gpui::test] async fn test_find_all_references_editor_reuse(cx: &mut TestAppContext) { init_test(cx, |_| {}); diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 4b719b1474d..a6be884058e 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -6814,7 +6814,9 @@ impl EditorElement { .display_snapshot .display_point_to_anchor(point_for_position.nearest_valid, Bias::Left); editor.change_selections( - SelectionEffects::scroll(Autoscroll::top_relative(line_index)), + SelectionEffects::scroll(Autoscroll::top_relative( + line_index as ScrollOffset, + )), window, cx, |selections| { diff --git a/crates/editor/src/scroll/autoscroll.rs b/crates/editor/src/scroll/autoscroll.rs index 38f0f4b0228..6bdb60a470e 100644 --- a/crates/editor/src/scroll/autoscroll.rs +++ b/crates/editor/src/scroll/autoscroll.rs @@ -36,11 +36,14 @@ impl Autoscroll { /// Returns the autoscroll strategy configured for navigation to definitions /// and references, based on `go_to_definition_scroll_strategy`. - pub fn for_go_to_definition(cx: &App) -> Self { + pub fn for_go_to_definition(offset: Option, cx: &App) -> Self { match EditorSettings::get_global(cx).go_to_definition_scroll_strategy { GoToDefinitionScrollStrategy::Center => Self::center(), GoToDefinitionScrollStrategy::Minimum => Self::fit(), GoToDefinitionScrollStrategy::Top => Self::focused(), + GoToDefinitionScrollStrategy::Preserve => { + offset.map(Self::top_relative).unwrap_or_else(Self::center) + } } } @@ -50,9 +53,10 @@ impl Autoscroll { Self::Strategy(AutoscrollStrategy::Focused, None) } - /// 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), None) + /// Scrolls so that the newest cursor is the given offset (in display rows) + /// from the top of the viewport. + pub fn top_relative(offset: ScrollOffset) -> Self { + Self::Strategy(AutoscrollStrategy::TopRelative(offset), None) } /// Scrolls so that the newest cursor is at the top. @@ -60,9 +64,10 @@ impl Autoscroll { Self::Strategy(AutoscrollStrategy::Top, None) } - /// Scrolls so that the newest cursor is roughly an n-th line from the bottom. - pub fn bottom_relative(n: usize) -> Self { - Self::Strategy(AutoscrollStrategy::BottomRelative(n), None) + /// Scrolls so that the newest cursor is the given offset (in display rows) + /// from the bottom of the viewport. + pub fn bottom_relative(offset: ScrollOffset) -> Self { + Self::Strategy(AutoscrollStrategy::BottomRelative(offset), None) } /// Scrolls so that the newest cursor is at the bottom. @@ -91,7 +96,7 @@ impl Into for Option { } } -#[derive(Debug, PartialEq, Eq, Default, Clone, Copy)] +#[derive(Debug, PartialEq, Default, Clone, Copy)] pub enum AutoscrollStrategy { Fit, Newest, @@ -100,10 +105,12 @@ pub enum AutoscrollStrategy { Focused, Top, Bottom, - TopRelative(usize), - BottomRelative(usize), + TopRelative(ScrollOffset), + BottomRelative(ScrollOffset), } +impl Eq for AutoscrollStrategy {} + impl AutoscrollStrategy { fn next(&self) -> Self { match self { diff --git a/crates/repl/src/session.rs b/crates/repl/src/session.rs index 38491384484..9b7bd759504 100644 --- a/crates/repl/src/session.rs +++ b/crates/repl/src/session.rs @@ -795,7 +795,7 @@ impl Session { if move_down { editor.update(cx, move |editor, cx| { editor.change_selections( - SelectionEffects::scroll(Autoscroll::top_relative(8)), + SelectionEffects::scroll(Autoscroll::top_relative(8.0)), window, cx, |selections| { diff --git a/crates/settings_content/src/editor.rs b/crates/settings_content/src/editor.rs index 18a7bd5fd49..5c0dd939688 100644 --- a/crates/settings_content/src/editor.rs +++ b/crates/settings_content/src/editor.rs @@ -831,6 +831,9 @@ pub enum GoToDefinitionScrollStrategy { Minimum, /// Scroll so the target appears near the top of the viewport. Top, + /// Preserve the cursor's vertical position within the viewport, falling + /// back to centering when the cursor is offscreen. + Preserve, } /// Determines when the mouse cursor should be hidden in an editor or input box. diff --git a/docs/src/reference/all-settings.md b/docs/src/reference/all-settings.md index c6c33b8b5ed..f175d51a398 100644 --- a/docs/src/reference/all-settings.md +++ b/docs/src/reference/all-settings.md @@ -2456,6 +2456,15 @@ Example: } ``` +4. Preserve the cursor's vertical position within the viewport, falling back to + `center` when the cursor is offscreen. + +```json [settings] +{ + "go_to_definition_scroll_strategy": "preserve" +} +``` + ## Hard Tabs - Description: Whether to indent lines using tab characters or multiple spaces.