vrr/zed
2
0
Fork 0
mirror of https://github.com/zed-industries/zed.git synced 2026-07-10 00:13:29 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 18

editor: Add preserve scroll strategy for go to definition (#55036)

Browse source 
These changes attempt to expand on the work introduced by
https://github.com/zed-industries/zed/pull/54778 by introducing a new
`GoToDefinitionScrollStrategy::Preserve` variant that attempts to keep
the cursor at the same vertical offset within the viewport when
navigating to a definition.

Most of the machinery for this was already in place. To support cases
where the user's scroll position isn't snapped to an exact display row,
for example, after scrolling with the mmouse, `Autoscroll::TopRelative`
and `Autoscroll::BottomRelative` were updated from `usize` to
`ScrollOffset`, allowing fractional offsets.

When the cursor is offscreen at the moment the `editor: go to
definition` action is invoked, `Preserve` falls back to
`Autoscroll::center`, matching the existing default for
`go_to_definition_scroll_strategy`. This avoids attempting to preserve
an offset where the cursor isn't visible which would lead to the cursor
being offscreen when jumping to the definition.

Documentation has also been updated to reflect this new strategy value.

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Relates to #52173

Release Notes:

- Added a new `preserve` option to `go_to_definition_scroll_strategy`
that keeps the cursor at the same vertical position within the viewport
when navigating to a definition
This commit is contained in:
Dino 2026-04-29 08:29:15 +01:00 • committed by GitHub
parent c6c63e8a38
commit 6357a85e8f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 312 additions and 22 deletions
Download patch file Download diff file Expand all files Collapse all files

2
assets/settings/default.json
View file

@ -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.
//

49
crates/editor/src/editor.rs
View file

@ -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<Self>, Entity<Pane>) =
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<Self>) -> Option<ScrollOffset> {
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::<Point>(&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>(

238
crates/editor/src/editor_tests.rs
View file

@ -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::<lsp::request::GotoDefinition, _, _>(|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, |_| {});

4
crates/editor/src/element.rs
View file

@ -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| {

27
crates/editor/src/scroll/autoscroll.rs
View file

@ -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<ScrollOffset>, 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<SelectionEffects> for Option<Autoscroll> {
}
}
#[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 {

2
crates/repl/src/session.rs
View file

@ -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| {

3
crates/settings_content/src/editor.rs
View file

@ -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.

9
docs/src/reference/all-settings.md
View file

@ -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.