mirror of
https://github.com/zed-industries/zed.git
synced 2026-07-09 16:00:35 +00:00
Make editor::LineUp & editor::LineDown honor vertical_scroll_margin like vim::LineUp & vim::LineDown (#52057)
## Context This is an implementation of this feature: https://github.com/zed-industries/zed/discussions/49821 Although, I'd argue it's a bug fix, not a feature. Either way : This copies the window scroll logic from the `vim` mode versions with out all the extra logic for visual mode. Could refactor the common logic out of the vim code and make it common. But that seems like a bigger PR. Happy to take a stab at it if that's what you prefer. Could also add new commands for the new behavior if you prefer. I didn't do that, because it seems like more clutter in the commands, and my belief that the existing behavior is a bug. But happy to do that if you prefer. ## How to Review creates new function `scroll_screen_with_cursor_margin` in `scroll.rs` wires up editor::LineUp/LineDown to new function in `elements.rs` adds a test in `editor_tests.rs` ## Self-Review Checklist <!-- Check before requesting review: --> - [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 Release Notes: - editor::LineUp/LineDown commands now honor vertical_scroll_margin (same as vim::LineUp/LineDown) --------- Co-authored-by: Kirill Bulatov <mail4score@gmail.com>
This commit is contained in:
parent
7f5cf583dc
commit
029bf2f284
3 changed files with 148 additions and 7 deletions
|
|
@ -2958,6 +2958,90 @@ async fn test_scroll_page_up_page_down(cx: &mut TestAppContext) {
|
|||
});
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_scroll_line_up_down_cursor_margin(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(2, cx);
|
||||
editor
|
||||
.style(cx)
|
||||
.text
|
||||
.line_height_in_pixels(window.rem_size())
|
||||
});
|
||||
let window = cx.window;
|
||||
// 5 visible lines with margin=2: valid cursor rows are [top+2 .. top+2] (only the middle row)
|
||||
cx.simulate_window_resize(window, size(px(1000.), 5. * line_height));
|
||||
|
||||
// Cursor at row 0 — autoscroll leaves viewport at y=0.
|
||||
cx.set_state(indoc! {"
|
||||
ˇone
|
||||
two
|
||||
three
|
||||
four
|
||||
five
|
||||
six
|
||||
seven
|
||||
eight
|
||||
nine
|
||||
ten
|
||||
"});
|
||||
|
||||
cx.update_editor(|editor, window, cx| {
|
||||
assert_eq!(editor.snapshot(window, cx).scroll_position().y, 0.);
|
||||
|
||||
// Scroll down 1: top=1, visible rows 1-5, margin=2 → min_row=3, max_row=3.
|
||||
// Cursor at row 0 < min_row=3 → clamped to row 3.
|
||||
editor.scroll_screen_with_cursor_margin(&ScrollAmount::Line(1.), window, cx);
|
||||
assert_eq!(editor.snapshot(window, cx).scroll_position().y, 1.);
|
||||
let snapshot = editor.display_snapshot(cx);
|
||||
assert_eq!(
|
||||
editor.selections.newest_display(&snapshot).head().row().0,
|
||||
3,
|
||||
"cursor clamped to min_row after scrolling down"
|
||||
);
|
||||
});
|
||||
|
||||
cx.update_editor(|editor, window, cx| {
|
||||
// Scroll up 1: top=0, visible rows 0-4, margin=2 (top==0 special case) → min_row=0, max_row=2.
|
||||
// Cursor at row 3 > max_row=2 → clamped to row 2.
|
||||
editor.scroll_screen_with_cursor_margin(&ScrollAmount::Line(-1.), window, cx);
|
||||
assert_eq!(editor.snapshot(window, cx).scroll_position().y, 0.);
|
||||
let snapshot = editor.display_snapshot(cx);
|
||||
assert_eq!(
|
||||
editor.selections.newest_display(&snapshot).head().row().0,
|
||||
2,
|
||||
"cursor clamped to max_row after scrolling up"
|
||||
);
|
||||
});
|
||||
|
||||
cx.update_editor(|editor, window, cx| {
|
||||
// Half page down (2 lines): top=2, visible rows 2-6, margin=2 → min_row=4, max_row=4.
|
||||
// Cursor at row 2 < min_row=4 → clamped to row 4.
|
||||
editor.scroll_screen_with_cursor_margin(&ScrollAmount::Page(0.5), window, cx);
|
||||
assert_eq!(editor.snapshot(window, cx).scroll_position().y, 2.);
|
||||
let snapshot = editor.display_snapshot(cx);
|
||||
assert_eq!(
|
||||
editor.selections.newest_display(&snapshot).head().row().0,
|
||||
4,
|
||||
"cursor clamped to min_row after scrolling half page down"
|
||||
);
|
||||
});
|
||||
|
||||
cx.update_editor(|editor, window, cx| {
|
||||
// Half page up (2 lines): top=0, visible rows 0-4, margin=2 (top==0 special case) → min_row=0, max_row=2.
|
||||
// Cursor at row 4 > max_row=2 → clamped to row 2.
|
||||
editor.scroll_screen_with_cursor_margin(&ScrollAmount::Page(-0.5), window, cx);
|
||||
assert_eq!(editor.snapshot(window, cx).scroll_position().y, 0.);
|
||||
let snapshot = editor.display_snapshot(cx);
|
||||
assert_eq!(
|
||||
editor.selections.newest_display(&snapshot).head().row().0,
|
||||
2,
|
||||
"cursor clamped to max_row after scrolling half page up"
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_autoscroll(cx: &mut TestAppContext) {
|
||||
init_test(cx, |_| {});
|
||||
|
|
|
|||
|
|
@ -287,22 +287,22 @@ impl EditorElement {
|
|||
register_action(editor, window, Editor::scroll_cursor_bottom);
|
||||
register_action(editor, window, Editor::scroll_cursor_center_top_bottom);
|
||||
register_action(editor, window, |editor, _: &LineDown, window, cx| {
|
||||
editor.scroll_screen(&ScrollAmount::Line(1.), window, cx)
|
||||
editor.scroll_screen_with_cursor_margin(&ScrollAmount::Line(1.), window, cx)
|
||||
});
|
||||
register_action(editor, window, |editor, _: &LineUp, window, cx| {
|
||||
editor.scroll_screen(&ScrollAmount::Line(-1.), window, cx)
|
||||
editor.scroll_screen_with_cursor_margin(&ScrollAmount::Line(-1.), window, cx)
|
||||
});
|
||||
register_action(editor, window, |editor, _: &HalfPageDown, window, cx| {
|
||||
editor.scroll_screen(&ScrollAmount::Page(0.5), window, cx)
|
||||
editor.scroll_screen_with_cursor_margin(&ScrollAmount::Page(0.5), window, cx)
|
||||
});
|
||||
register_action(editor, window, |editor, _: &HalfPageUp, window, cx| {
|
||||
editor.scroll_screen(&ScrollAmount::Page(-0.5), window, cx)
|
||||
editor.scroll_screen_with_cursor_margin(&ScrollAmount::Page(-0.5), window, cx)
|
||||
});
|
||||
register_action(editor, window, |editor, _: &PageDown, window, cx| {
|
||||
editor.scroll_screen(&ScrollAmount::Page(1.), window, cx)
|
||||
editor.scroll_screen_with_cursor_margin(&ScrollAmount::Page(1.), window, cx)
|
||||
});
|
||||
register_action(editor, window, |editor, _: &PageUp, window, cx| {
|
||||
editor.scroll_screen(&ScrollAmount::Page(-1.), window, cx)
|
||||
editor.scroll_screen_with_cursor_margin(&ScrollAmount::Page(-1.), window, cx)
|
||||
});
|
||||
register_action(editor, window, Editor::move_to_previous_word_start);
|
||||
register_action(editor, window, Editor::move_to_previous_subword_start);
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@ pub(crate) mod scroll_amount;
|
|||
use crate::editor_settings::ScrollBeyondLastLine;
|
||||
use crate::{
|
||||
Anchor, DisplayPoint, DisplayRow, Editor, EditorEvent, EditorMode, EditorSettings,
|
||||
MultiBufferSnapshot, RowExt, SizingBehavior, ToPoint,
|
||||
MultiBufferSnapshot, RowExt, SelectionEffects, SizingBehavior, ToPoint,
|
||||
display_map::{DisplaySnapshot, ToDisplayPoint},
|
||||
hover_popover::hide_hover,
|
||||
persistence::EditorDb,
|
||||
|
|
@ -945,6 +945,63 @@ impl Editor {
|
|||
self.set_scroll_position(new_position, window, cx);
|
||||
}
|
||||
|
||||
pub fn scroll_screen_with_cursor_margin(
|
||||
&mut self,
|
||||
amount: &ScrollAmount,
|
||||
window: &mut Window,
|
||||
cx: &mut Context<Self>,
|
||||
) {
|
||||
self.scroll_screen(amount, window, cx);
|
||||
|
||||
let Some(visible_line_count) = self.visible_line_count() else {
|
||||
return;
|
||||
};
|
||||
let display_snapshot = self.display_map.update(cx, |map, cx| map.snapshot(cx));
|
||||
let top = self
|
||||
.scroll_manager
|
||||
.scroll_top_display_point(&display_snapshot, cx);
|
||||
let vertical_scroll_margin =
|
||||
(self.vertical_scroll_margin() as u32).min(visible_line_count as u32 / 2);
|
||||
|
||||
let max_point = display_snapshot.max_point();
|
||||
let min_row = if top.row().0 == 0 {
|
||||
DisplayRow(0)
|
||||
} else {
|
||||
DisplayRow(top.row().0 + vertical_scroll_margin)
|
||||
};
|
||||
let max_row = if top.row().0 + visible_line_count as u32 >= max_point.row().0 {
|
||||
max_point.row()
|
||||
} else {
|
||||
DisplayRow(
|
||||
(top.row().0 + visible_line_count as u32)
|
||||
.saturating_sub(1 + vertical_scroll_margin),
|
||||
)
|
||||
};
|
||||
|
||||
self.change_selections(
|
||||
SelectionEffects::no_scroll().nav_history(false),
|
||||
window,
|
||||
cx,
|
||||
|s| {
|
||||
s.move_with(&mut |map, selection| {
|
||||
let head = selection.head();
|
||||
let new_row = if head.row() < min_row {
|
||||
min_row
|
||||
} else if head.row() > max_row {
|
||||
max_row
|
||||
} else {
|
||||
head.row()
|
||||
};
|
||||
if new_row != head.row() {
|
||||
let new_head =
|
||||
map.clip_point(DisplayPoint::new(new_row, head.column()), Bias::Left);
|
||||
selection.collapse_to(new_head, selection.goal);
|
||||
}
|
||||
})
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
/// Returns an ordering. The newest selection is:
|
||||
/// Ordering::Equal => on screen
|
||||
/// Ordering::Less => above or to the left of the screen
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue