From 8e1bbf32be02c16c6cb9959948f1380a42a89dce Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 26 Sep 2023 14:49:08 -0600 Subject: [PATCH] vim: Fix ctrl-u/ctrl-d They should work by exactly half a screen, and also move the cursor. --- crates/editor/src/editor_tests.rs | 2 +- crates/editor/src/scroll/scroll_amount.rs | 10 ++- crates/vim/src/normal/scroll.rs | 88 +++++++++++++++---- .../src/test/neovim_backed_test_context.rs | 21 ++++- crates/vim/test_data/test_ctrl_d_u.json | 22 +++++ 5 files changed, 122 insertions(+), 21 deletions(-) create mode 100644 crates/vim/test_data/test_ctrl_d_u.json diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 7acf0c652f..72df7b7874 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -1429,7 +1429,7 @@ async fn test_scroll_page_up_page_down(cx: &mut gpui::TestAppContext) { 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.)); + assert_eq!(editor.snapshot(cx).scroll_position(), vec2f(0., 1.)); editor.scroll_screen(&ScrollAmount::Page(0.5), cx); assert_eq!(editor.snapshot(cx).scroll_position(), vec2f(0., 3.)); }); diff --git a/crates/editor/src/scroll/scroll_amount.rs b/crates/editor/src/scroll/scroll_amount.rs index 0edab2bdfc..2cb22d1516 100644 --- a/crates/editor/src/scroll/scroll_amount.rs +++ b/crates/editor/src/scroll/scroll_amount.rs @@ -15,9 +15,13 @@ impl ScrollAmount { 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 * count).trunc() - count.signum()) + .map(|mut l| { + // for full pages subtract one to leave an anchor line + if count.abs() == 1.0 { + l -= 1.0 + } + (l * count).trunc() + }) .unwrap_or(0.), } } diff --git a/crates/vim/src/normal/scroll.rs b/crates/vim/src/normal/scroll.rs index 877fff328b..e113df2265 100644 --- a/crates/vim/src/normal/scroll.rs +++ b/crates/vim/src/normal/scroll.rs @@ -15,19 +15,19 @@ actions!( pub fn init(cx: &mut AppContext) { cx.add_action(|_: &mut Workspace, _: &LineDown, cx| { - scroll(cx, |c| ScrollAmount::Line(c.unwrap_or(1.))) + scroll(cx, false, |c| ScrollAmount::Line(c.unwrap_or(1.))) }); cx.add_action(|_: &mut Workspace, _: &LineUp, cx| { - scroll(cx, |c| ScrollAmount::Line(-c.unwrap_or(1.))) + scroll(cx, false, |c| ScrollAmount::Line(-c.unwrap_or(1.))) }); cx.add_action(|_: &mut Workspace, _: &PageDown, cx| { - scroll(cx, |c| ScrollAmount::Page(c.unwrap_or(1.))) + scroll(cx, false, |c| ScrollAmount::Page(c.unwrap_or(1.))) }); cx.add_action(|_: &mut Workspace, _: &PageUp, cx| { - scroll(cx, |c| ScrollAmount::Page(-c.unwrap_or(1.))) + scroll(cx, false, |c| ScrollAmount::Page(-c.unwrap_or(1.))) }); cx.add_action(|_: &mut Workspace, _: &ScrollDown, cx| { - scroll(cx, |c| { + scroll(cx, true, |c| { if let Some(c) = c { ScrollAmount::Line(c) } else { @@ -36,7 +36,7 @@ pub fn init(cx: &mut AppContext) { }) }); cx.add_action(|_: &mut Workspace, _: &ScrollUp, cx| { - scroll(cx, |c| { + scroll(cx, true, |c| { if let Some(c) = c { ScrollAmount::Line(-c) } else { @@ -46,15 +46,27 @@ pub fn init(cx: &mut AppContext) { }); } -fn scroll(cx: &mut ViewContext, by: fn(c: Option) -> ScrollAmount) { +fn scroll( + cx: &mut ViewContext, + move_cursor: bool, + by: fn(c: Option) -> ScrollAmount, +) { Vim::update(cx, |vim, cx| { let amount = by(vim.take_count(cx).map(|c| c as f32)); - vim.update_active_editor(cx, |editor, cx| scroll_editor(editor, &amount, cx)); + vim.update_active_editor(cx, |editor, cx| { + scroll_editor(editor, move_cursor, &amount, cx) + }); }) } -fn scroll_editor(editor: &mut Editor, amount: &ScrollAmount, cx: &mut ViewContext) { +fn scroll_editor( + editor: &mut Editor, + preserve_cursor_position: bool, + amount: &ScrollAmount, + cx: &mut ViewContext, +) { let should_move_cursor = editor.newest_selection_on_screen(cx).is_eq(); + let old_top_anchor = editor.scroll_manager.anchor().anchor; editor.scroll_screen(amount, cx); if should_move_cursor { @@ -68,8 +80,14 @@ fn scroll_editor(editor: &mut Editor, amount: &ScrollAmount, cx: &mut ViewContex editor.change_selections(None, cx, |s| { s.move_with(|map, selection| { - let head = selection.head(); + let mut head = selection.head(); let top = top_anchor.to_display_point(map); + + if preserve_cursor_position { + let old_top = old_top_anchor.to_display_point(map); + let new_row = top.row() + selection.head().row() - old_top.row(); + head = map.clip_point(DisplayPoint::new(new_row, head.column()), Bias::Left) + } let min_row = top.row() + VERTICAL_SCROLL_MARGIN as u32; let max_row = top.row() + visible_rows - VERTICAL_SCROLL_MARGIN as u32 - 1; @@ -92,7 +110,10 @@ fn scroll_editor(editor: &mut Editor, amount: &ScrollAmount, cx: &mut ViewContex #[cfg(test)] mod test { - use crate::{state::Mode, test::VimTestContext}; + use crate::{ + state::Mode, + test::{NeovimBackedTestContext, VimTestContext}, + }; use gpui::geometry::vector::vec2f; use indoc::indoc; use language::Point; @@ -148,10 +169,10 @@ mod test { }); cx.simulate_keystrokes(["ctrl-d"]); cx.update_editor(|editor, cx| { - assert_eq!(editor.snapshot(cx).scroll_position(), vec2f(0., 2.0)); + assert_eq!(editor.snapshot(cx).scroll_position(), vec2f(0., 3.0)); assert_eq!( editor.selections.newest(cx).range(), - Point::new(5, 0)..Point::new(5, 0) + Point::new(6, 0)..Point::new(6, 0) ) }); @@ -162,11 +183,48 @@ mod test { }); cx.simulate_keystrokes(["v", "ctrl-d"]); cx.update_editor(|editor, cx| { - assert_eq!(editor.snapshot(cx).scroll_position(), vec2f(0., 2.0)); + assert_eq!(editor.snapshot(cx).scroll_position(), vec2f(0., 3.0)); assert_eq!( editor.selections.newest(cx).range(), - Point::new(0, 0)..Point::new(5, 1) + Point::new(0, 0)..Point::new(6, 1) ) }); } + #[gpui::test] + async fn test_ctrl_d_u(cx: &mut gpui::TestAppContext) { + let mut cx = NeovimBackedTestContext::new(cx).await; + + cx.set_scroll_height(10).await; + + pub fn sample_text(rows: usize, cols: usize, start_char: char) -> String { + let mut text = String::new(); + for row in 0..rows { + let c: char = (start_char as u32 + row as u32) as u8 as char; + let mut line = c.to_string().repeat(cols); + if row < rows - 1 { + line.push('\n'); + } + text += &line; + } + text + } + let content = "ˇ".to_owned() + &sample_text(26, 2, 'a'); + cx.set_shared_state(&content).await; + + // skip over the scrolloff at the top + // test ctrl-d + cx.simulate_shared_keystrokes(["4", "j", "ctrl-d"]).await; + cx.assert_state_matches().await; + cx.simulate_shared_keystrokes(["ctrl-d"]).await; + cx.assert_state_matches().await; + cx.simulate_shared_keystrokes(["g", "g", "ctrl-d"]).await; + cx.assert_state_matches().await; + + // test ctrl-u + cx.simulate_shared_keystrokes(["ctrl-u"]).await; + cx.assert_state_matches().await; + cx.simulate_shared_keystrokes(["ctrl-d", "ctrl-d", "4", "j", "ctrl-u", "ctrl-u"]) + .await; + cx.assert_state_matches().await; + } } diff --git a/crates/vim/src/test/neovim_backed_test_context.rs b/crates/vim/src/test/neovim_backed_test_context.rs index 227d39bb63..53aa536c2f 100644 --- a/crates/vim/src/test/neovim_backed_test_context.rs +++ b/crates/vim/src/test/neovim_backed_test_context.rs @@ -1,9 +1,10 @@ +use editor::scroll::VERTICAL_SCROLL_MARGIN; use indoc::indoc; use settings::SettingsStore; use std::ops::{Deref, DerefMut, Range}; use collections::{HashMap, HashSet}; -use gpui::ContextHandle; +use gpui::{geometry::vector::vec2f, ContextHandle}; use language::{ language_settings::{AllLanguageSettings, SoftWrap}, OffsetRangeExt, @@ -132,7 +133,9 @@ impl<'a> NeovimBackedTestContext<'a> { panic!("nvim doesn't support columns < 12") } self.neovim.set_option("wrap").await; - self.neovim.set_option("columns=12").await; + self.neovim + .set_option(&format!("columns={}", columns)) + .await; self.update(|cx| { cx.update_global(|settings: &mut SettingsStore, cx| { @@ -144,6 +147,20 @@ impl<'a> NeovimBackedTestContext<'a> { }) } + pub async fn set_scroll_height(&mut self, rows: u32) { + // match Zed's scrolling behavior + self.neovim + .set_option(&format!("scrolloff={}", VERTICAL_SCROLL_MARGIN)) + .await; + // +2 to account for the vim command UI at the bottom. + self.neovim.set_option(&format!("lines={}", rows + 2)).await; + let window = self.window; + let line_height = + self.editor(|editor, cx| editor.style(cx).text.line_height(cx.font_cache())); + + window.simulate_resize(vec2f(1000., (rows as f32) * line_height), &mut self.cx); + } + pub async fn set_neovim_option(&mut self, option: &str) { self.neovim.set_option(option).await; } diff --git a/crates/vim/test_data/test_ctrl_d_u.json b/crates/vim/test_data/test_ctrl_d_u.json new file mode 100644 index 0000000000..4235d8d278 --- /dev/null +++ b/crates/vim/test_data/test_ctrl_d_u.json @@ -0,0 +1,22 @@ +{"SetOption":{"value":"scrolloff=3"}} +{"SetOption":{"value":"lines=12"}} +{"Put":{"state":"ˇaa\nbb\ncc\ndd\nee\nff\ngg\nhh\nii\njj\nkk\nll\nmm\nnn\noo\npp\nqq\nrr\nss\ntt\nuu\nvv\nww\nxx\nyy\nzz"}} +{"Key":"4"} +{"Key":"j"} +{"Key":"ctrl-d"} +{"Get":{"state":"aa\nbb\ncc\ndd\nee\nff\ngg\nhh\nii\nˇjj\nkk\nll\nmm\nnn\noo\npp\nqq\nrr\nss\ntt\nuu\nvv\nww\nxx\nyy\nzz","mode":"Normal"}} +{"Key":"ctrl-d"} +{"Get":{"state":"aa\nbb\ncc\ndd\nee\nff\ngg\nhh\nii\njj\nkk\nll\nmm\nnn\nˇoo\npp\nqq\nrr\nss\ntt\nuu\nvv\nww\nxx\nyy\nzz","mode":"Normal"}} +{"Key":"g"} +{"Key":"g"} +{"Key":"ctrl-d"} +{"Get":{"state":"aa\nbb\ncc\ndd\nee\nff\ngg\nhh\nˇii\njj\nkk\nll\nmm\nnn\noo\npp\nqq\nrr\nss\ntt\nuu\nvv\nww\nxx\nyy\nzz","mode":"Normal"}} +{"Key":"ctrl-u"} +{"Get":{"state":"aa\nbb\ncc\nˇdd\nee\nff\ngg\nhh\nii\njj\nkk\nll\nmm\nnn\noo\npp\nqq\nrr\nss\ntt\nuu\nvv\nww\nxx\nyy\nzz","mode":"Normal"}} +{"Key":"ctrl-d"} +{"Key":"ctrl-d"} +{"Key":"4"} +{"Key":"j"} +{"Key":"ctrl-u"} +{"Key":"ctrl-u"} +{"Get":{"state":"aa\nbb\ncc\ndd\nee\nff\ngg\nˇhh\nii\njj\nkk\nll\nmm\nnn\noo\npp\nqq\nrr\nss\ntt\nuu\nvv\nww\nxx\nyy\nzz","mode":"Normal"}}