Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

vim: Fix scrolling

Browse source 
After #2641 we noticed that scrolling didn't take a count parameter.

The PageDown/PageUp logic was also broken by an additional -1 (for both
vim mode and not).
This commit is contained in:
Conrad Irwin 2023-06-26 12:23:58 -06:00
parent 3f4a06f576
commit 02fc5dd6c9
7 changed files with 195 additions and 106 deletions
Download patch file Download diff file Expand all files Collapse all files

32
assets/keymaps/vim.json
View file

@ -58,10 +58,6 @@
}
],
"%": "vim::Matching",
"ctrl-y": [
"vim::Scroll",
"LineUp"
],
"f": [
"vim::PushOperator",
{
@ -197,26 +193,14 @@
"focus": true
}
],
"ctrl-f": [
"vim::Scroll",
"PageDown"
],
"ctrl-b": [
"vim::Scroll",
"PageUp"
],
"ctrl-d": [
"vim::Scroll",
"HalfPageDown"
],
"ctrl-u": [
"vim::Scroll",
"HalfPageUp"
],
"ctrl-e": [
"vim::Scroll",
"LineDown"
],
"ctrl-f": "vim::PageDown",
"pagedown": "vim::PageDown",
"ctrl-b": "vim::PageUp",
"pageup": "vim::PageUp",
"ctrl-d": "vim::ScrollDown",
"ctrl-u": "vim::ScrollUp",
"ctrl-e": "vim::LineDown",
"ctrl-y": "vim::LineUp",
"r": [
"vim::PushOperator",
"Replace"

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

@ -1,5 +1,6 @@
use super::*;
use crate::{
scroll::scroll_amount::ScrollAmount,
test::{
assert_text_with_selections, build_editor, editor_lsp_test_context::EditorLspTestContext,
editor_test_context::EditorTestContext, select_ranges,
@ -1359,6 +1360,43 @@ async fn test_move_start_of_paragraph_end_of_paragraph(cx: &mut gpui::TestAppCon
);
}
#[gpui::test]
async fn test_scroll_page_up_page_down(cx: &mut gpui::TestAppContext) {
init_test(cx, |_| {});
let mut cx = EditorTestContext::new(cx).await;
let line_height = cx.editor(|editor, cx| editor.style(cx).text.line_height(cx.font_cache()));
cx.simulate_window_resize(cx.window_id, vec2f(1000., 4. * line_height + 0.5));
cx.set_state(
&r#"ˇone
two
three
four
five
six
seven
eight
nine
ten
"#,
);
cx.update_editor(|editor, cx| {
assert_eq!(editor.snapshot(cx).scroll_position(), vec2f(0., 0.));
editor.scroll_screen(&ScrollAmount::Page(1.), cx);
assert_eq!(editor.snapshot(cx).scroll_position(), vec2f(0., 3.));
editor.scroll_screen(&ScrollAmount::Page(1.), cx);
assert_eq!(editor.snapshot(cx).scroll_position(), vec2f(0., 6.));
editor.scroll_screen(&ScrollAmount::Page(-1.), cx);
assert_eq!(editor.snapshot(cx).scroll_position(), vec2f(0., 3.));
editor.scroll_screen(&ScrollAmount::Page(-0.5), cx);
assert_eq!(editor.snapshot(cx).scroll_position(), vec2f(0., 2.));
editor.scroll_screen(&ScrollAmount::Page(0.5), cx);
assert_eq!(editor.snapshot(cx).scroll_position(), vec2f(0., 3.));
});
}
#[gpui::test]
async fn test_move_page_up_page_down(cx: &mut gpui::TestAppContext) {
init_test(cx, |_| {});

2
crates/editor/src/scroll.rs
View file

@ -368,7 +368,7 @@ impl Editor {
}
let cur_position = self.scroll_position(cx);
let new_pos = cur_position + vec2f(0., amount.lines(self) - 1.);
let new_pos = cur_position + vec2f(0., amount.lines(self));
self.set_scroll_position(new_pos, cx);
}

12
crates/editor/src/scroll/actions.rs
View file

@ -27,22 +27,22 @@ pub fn init(cx: &mut AppContext) {
cx.add_action(Editor::scroll_cursor_center);
cx.add_action(Editor::scroll_cursor_bottom);
cx.add_action(|this: &mut Editor, _: &LineDown, cx| {
this.scroll_screen(&ScrollAmount::LineDown, cx)
this.scroll_screen(&ScrollAmount::Line(1.), cx)
});
cx.add_action(|this: &mut Editor, _: &LineUp, cx| {
this.scroll_screen(&ScrollAmount::LineUp, cx)
this.scroll_screen(&ScrollAmount::Line(-1.), cx)
});
cx.add_action(|this: &mut Editor, _: &HalfPageDown, cx| {
this.scroll_screen(&ScrollAmount::HalfPageDown, cx)
this.scroll_screen(&ScrollAmount::Page(0.5), cx)
});
cx.add_action(|this: &mut Editor, _: &HalfPageUp, cx| {
this.scroll_screen(&ScrollAmount::HalfPageUp, cx)
this.scroll_screen(&ScrollAmount::Page(-0.5), cx)
});
cx.add_action(|this: &mut Editor, _: &PageDown, cx| {
this.scroll_screen(&ScrollAmount::PageDown, cx)
this.scroll_screen(&ScrollAmount::Page(1.), cx)
});
cx.add_action(|this: &mut Editor, _: &PageUp, cx| {
this.scroll_screen(&ScrollAmount::PageUp, cx)
this.scroll_screen(&ScrollAmount::Page(-1.), cx)
});
}

32
crates/editor/src/scroll/scroll_amount.rs
View file

@ -6,12 +6,10 @@ use crate::Editor;
#[derive(Clone, PartialEq, Deserialize)]
pub enum ScrollAmount {
LineUp,
LineDown,
HalfPageUp,
HalfPageDown,
PageUp,
PageDown,
// Scroll N lines (positive is towards the end of the document)
Line(f32),
// Scroll N pages (positive is towards the end of the document)
Page(f32),
}
impl ScrollAmount {
@ -24,10 +22,10 @@ impl ScrollAmount {
let context_menu = editor.context_menu.as_mut()?;
match self {
Self::LineDown | Self::HalfPageDown => context_menu.select_next(cx),
Self::LineUp | Self::HalfPageUp => context_menu.select_prev(cx),
Self::PageDown => context_menu.select_last(cx),
Self::PageUp => context_menu.select_first(cx),
Self::Line(c) if *c > 0. => context_menu.select_next(cx),
Self::Line(_) => context_menu.select_prev(cx),
Self::Page(c) if *c > 0. => context_menu.select_last(cx),
Self::Page(_) => context_menu.select_first(cx),
}
.then_some(())
})
@ -36,13 +34,13 @@ impl ScrollAmount {
pub fn lines(&self, editor: &mut Editor) -> f32 {
match self {
Self::LineDown => 1.,
Self::LineUp => -1.,
Self::HalfPageDown => editor.visible_line_count().map(|l| l / 2.).unwrap_or(1.),
Self::HalfPageUp => -editor.visible_line_count().map(|l| l / 2.).unwrap_or(1.),
// Minus 1. here so that there is a pivot line that stays on the screen
Self::PageDown => editor.visible_line_count().unwrap_or(1.) - 1.,
Self::PageUp => -editor.visible_line_count().unwrap_or(1.) - 1.,
Self::Line(count) => *count,
Self::Page(count) => editor
.visible_line_count()
// subtract one to leave an anchor line
// round towards zero (so page-up and page-down are symmetric)
.map(|l| ((l - 1.) * count).trunc())
.unwrap_or(0.),
}
}
}

65
crates/vim/src/normal.rs
View file

@ -1,9 +1,10 @@
mod change;
mod delete;
mod scroll;
mod substitute;
mod yank;
use std::{borrow::Cow, cmp::Ordering, sync::Arc};
use std::{borrow::Cow, sync::Arc};
use crate::{
motion::Motion,
@ -13,14 +14,12 @@ use crate::{
};
use collections::{HashMap, HashSet};
use editor::{
display_map::ToDisplayPoint,
scroll::{autoscroll::Autoscroll, scroll_amount::ScrollAmount},
Anchor, Bias, ClipboardSelection, DisplayPoint, Editor,
display_map::ToDisplayPoint, scroll::autoscroll::Autoscroll, Anchor, Bias, ClipboardSelection,
DisplayPoint,
};
use gpui::{actions, impl_actions, AppContext, ViewContext, WindowContext};
use gpui::{actions, AppContext, ViewContext, WindowContext};
use language::{AutoindentMode, Point, SelectionGoal};
use log::error;
use serde::Deserialize;
use workspace::Workspace;
use self::{
@ -30,9 +29,6 @@ use self::{
yank::{yank_motion, yank_object},
};
#[derive(Clone, PartialEq, Deserialize)]
struct Scroll(ScrollAmount);
actions!(
vim,
[
@ -51,8 +47,6 @@ actions!(
]
);
impl_actions!(vim, [Scroll]);
pub fn init(cx: &mut AppContext) {
cx.add_action(insert_after);
cx.add_action(insert_first_non_whitespace);
@ -90,13 +84,8 @@ pub fn init(cx: &mut AppContext) {
})
});
cx.add_action(paste);
cx.add_action(|_: &mut Workspace, Scroll(amount): &Scroll, cx| {
Vim::update(cx, |vim, cx| {
vim.update_active_editor(cx, |editor, cx| {
scroll(editor, amount, cx);
})
})
});
scroll::init(cx);
}
pub fn normal_motion(
@ -393,46 +382,6 @@ fn paste(_: &mut Workspace, _: &Paste, cx: &mut ViewContext<Workspace>) {
});
}
fn scroll(editor: &mut Editor, amount: &ScrollAmount, cx: &mut ViewContext<Editor>) {
let should_move_cursor = editor.newest_selection_on_screen(cx).is_eq();
editor.scroll_screen(amount, cx);
if should_move_cursor {
let selection_ordering = editor.newest_selection_on_screen(cx);
if selection_ordering.is_eq() {
return;
}
let visible_rows = if let Some(visible_rows) = editor.visible_line_count() {
visible_rows as u32
} else {
return;
};
let scroll_margin_rows = editor.vertical_scroll_margin() as u32;
let top_anchor = editor.scroll_manager.anchor().anchor;
editor.change_selections(None, cx, |s| {
s.replace_cursors_with(|snapshot| {
let mut new_point = top_anchor.to_display_point(&snapshot);
match selection_ordering {
Ordering::Less => {
*new_point.row_mut() += scroll_margin_rows;
new_point = snapshot.clip_point(new_point, Bias::Right);
}
Ordering::Greater => {
*new_point.row_mut() += visible_rows - scroll_margin_rows as u32;
new_point = snapshot.clip_point(new_point, Bias::Left);
}
Ordering::Equal => unreachable!(),
}
vec![new_point]
})
});
}
}
pub(crate) fn normal_replace(text: Arc<str>, cx: &mut WindowContext) {
Vim::update(cx, |vim, cx| {
vim.update_active_editor(cx, |editor, cx| {

120
crates/vim/src/normal/scroll.rs Normal file
View file

@ -0,0 +1,120 @@
use std::cmp::Ordering;
use crate::Vim;
use editor::{display_map::ToDisplayPoint, scroll::scroll_amount::ScrollAmount, Editor};
use gpui::{actions, AppContext, ViewContext};
use language::Bias;
use workspace::Workspace;
actions!(
vim,
[LineUp, LineDown, ScrollUp, ScrollDown, PageUp, PageDown,]
);
pub fn init(cx: &mut AppContext) {
cx.add_action(|_: &mut Workspace, _: &LineDown, cx| {
scroll(cx, |c| ScrollAmount::Line(c.unwrap_or(1.)))
});
cx.add_action(|_: &mut Workspace, _: &LineUp, cx| {
scroll(cx, |c| ScrollAmount::Line(-c.unwrap_or(1.)))
});
cx.add_action(|_: &mut Workspace, _: &PageDown, cx| {
scroll(cx, |c| ScrollAmount::Page(c.unwrap_or(1.)))
});
cx.add_action(|_: &mut Workspace, _: &PageUp, cx| {
scroll(cx, |c| ScrollAmount::Page(-c.unwrap_or(1.)))
});
cx.add_action(|_: &mut Workspace, _: &ScrollDown, cx| {
scroll(cx, |c| {
if let Some(c) = c {
ScrollAmount::Line(c)
} else {
ScrollAmount::Page(0.5)
}
})
});
cx.add_action(|_: &mut Workspace, _: &ScrollUp, cx| {
scroll(cx, |c| {
if let Some(c) = c {
ScrollAmount::Line(-c)
} else {
ScrollAmount::Page(-0.5)
}
})
});
}
fn scroll(cx: &mut ViewContext<Workspace>, by: fn(c: Option<f32>) -> ScrollAmount) {
Vim::update(cx, |vim, cx| {
let amount = by(vim.pop_number_operator(cx).map(|c| c as f32));
vim.update_active_editor(cx, |editor, cx| scroll_editor(editor, &amount, cx));
})
}
fn scroll_editor(editor: &mut Editor, amount: &ScrollAmount, cx: &mut ViewContext<Editor>) {
let should_move_cursor = editor.newest_selection_on_screen(cx).is_eq();
editor.scroll_screen(amount, cx);
if should_move_cursor {
let selection_ordering = editor.newest_selection_on_screen(cx);
if selection_ordering.is_eq() {
return;
}
let visible_rows = if let Some(visible_rows) = editor.visible_line_count() {
visible_rows as u32
} else {
return;
};
let top_anchor = editor.scroll_manager.anchor().anchor;
editor.change_selections(None, cx, |s| {
s.replace_cursors_with(|snapshot| {
let mut new_point = top_anchor.to_display_point(&snapshot);
match selection_ordering {
Ordering::Less => {
new_point = snapshot.clip_point(new_point, Bias::Right);
}
Ordering::Greater => {
*new_point.row_mut() += visible_rows - 1;
new_point = snapshot.clip_point(new_point, Bias::Left);
}
Ordering::Equal => unreachable!(),
}
vec![new_point]
})
});
}
}
#[cfg(test)]
mod test {
use crate::{state::Mode, test::VimTestContext};
use gpui::geometry::vector::vec2f;
use indoc::indoc;
#[gpui::test]
async fn test_scroll(cx: &mut gpui::TestAppContext) {
let mut cx = VimTestContext::new(cx, true).await;
cx.set_state(indoc! {"ˇa\nb\nc\nd\ne\n"}, Mode::Normal);
cx.update_editor(|editor, cx| {
assert_eq!(editor.snapshot(cx).scroll_position(), vec2f(0., 0.))
});
cx.simulate_keystrokes(["ctrl-e"]);
cx.update_editor(|editor, cx| {
assert_eq!(editor.snapshot(cx).scroll_position(), vec2f(0., 1.))
});
cx.simulate_keystrokes(["2", "ctrl-e"]);
cx.update_editor(|editor, cx| {
assert_eq!(editor.snapshot(cx).scroll_position(), vec2f(0., 3.))
});
cx.simulate_keystrokes(["ctrl-y"]);
cx.update_editor(|editor, cx| {
assert_eq!(editor.snapshot(cx).scroll_position(), vec2f(0., 2.))
});
}
}