From 1a29180185466e37e17e752d8c2ac7e83138094e Mon Sep 17 00:00:00 2001 From: Keith Simmons Date: Fri, 25 Mar 2022 20:05:46 -0700 Subject: [PATCH] Fixed issue with enabling and disabling vim mode dynamically Also added indoc and marked text utility to vim tests to improve readability --- Cargo.lock | 15 +++- crates/editor/src/display_map.rs | 7 +- crates/editor/src/editor.rs | 3 +- crates/editor/src/test.rs | 49 +------------ crates/util/src/test.rs | 51 +++++++++++++- crates/vim/Cargo.toml | 2 + crates/vim/src/editor_events.rs | 8 +-- crates/vim/src/editor_utils.rs | 97 ------------------------- crates/vim/src/insert.rs | 6 +- crates/vim/src/vim.rs | 31 ++++---- crates/vim/src/vim_tests.rs | 117 +++++++++++++++++++++++++++---- 11 files changed, 192 insertions(+), 194 deletions(-) delete mode 100644 crates/vim/src/editor_utils.rs diff --git a/Cargo.lock b/Cargo.lock index 65d690db23..74cfa42bdf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2527,6 +2527,15 @@ dependencies = [ "hashbrown 0.9.1", ] +[[package]] +name = "indoc" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e7906a9fababaeacb774f72410e497a1d18de916322e33797bb2cd29baa23c9e" +dependencies = [ + "unindent", +] + [[package]] name = "infer" version = "0.2.3" @@ -5553,9 +5562,9 @@ checksum = "39ec24b3121d976906ece63c9daad25b85969647682eee313cb5779fdd69e14e" [[package]] name = "unindent" -version = "0.1.7" +version = "0.1.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f14ee04d9415b52b3aeab06258a3f07093182b88ba0f9b8d203f211a7a7d41c7" +checksum = "514672a55d7380da379785a4d70ca8386c8883ff7eaae877be4d2081cebe73d8" [[package]] name = "universal-hash" @@ -5680,9 +5689,11 @@ dependencies = [ "collections", "editor", "gpui", + "indoc", "language", "log", "project", + "util", "workspace", ] diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index c3fef6e013..7aa4cdd891 100644 --- a/crates/editor/src/display_map.rs +++ b/crates/editor/src/display_map.rs @@ -498,17 +498,14 @@ impl ToDisplayPoint for Anchor { #[cfg(test)] pub mod tests { use super::*; - use crate::{ - movement, - test::{marked_display_snapshot, marked_text_ranges}, - }; + use crate::{movement, test::marked_display_snapshot}; use gpui::{color::Color, elements::*, test::observe, MutableAppContext}; use language::{Buffer, Language, LanguageConfig, RandomCharIter, SelectionGoal}; use rand::{prelude::*, Rng}; use smol::stream::StreamExt; use std::{env, sync::Arc}; use theme::SyntaxTheme; - use util::test::sample_text; + use util::test::{marked_text_ranges, sample_text}; use Bias::*; #[gpui::test(iterations = 100)] diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 1d1910e386..f45ea0fa2e 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -6167,7 +6167,6 @@ pub fn styled_runs_for_code_label<'a>( #[cfg(test)] mod tests { - use crate::test::marked_text_by; use super::*; use gpui::{ @@ -6181,7 +6180,7 @@ mod tests { use std::{cell::RefCell, rc::Rc, time::Instant}; use text::Point; use unindent::Unindent; - use util::test::sample_text; + use util::test::{marked_text_by, sample_text}; use workspace::FollowableItem; #[gpui::test] diff --git a/crates/editor/src/test.rs b/crates/editor/src/test.rs index 3c3e22ff3b..e80547c9dd 100644 --- a/crates/editor/src/test.rs +++ b/crates/editor/src/test.rs @@ -1,6 +1,4 @@ -use std::ops::Range; - -use collections::HashMap; +use util::test::marked_text; use crate::{ display_map::{DisplayMap, DisplaySnapshot, ToDisplayPoint}, @@ -15,51 +13,6 @@ fn init_logger() { } } -pub fn marked_text_by( - marked_text: &str, - markers: Vec, -) -> (String, HashMap>) { - let mut extracted_markers: HashMap> = Default::default(); - let mut unmarked_text = String::new(); - - for char in marked_text.chars() { - if markers.contains(&char) { - let char_offsets = extracted_markers.entry(char).or_insert(Vec::new()); - char_offsets.push(unmarked_text.len()); - } else { - unmarked_text.push(char); - } - } - - (unmarked_text, extracted_markers) -} - -pub fn marked_text(marked_text: &str) -> (String, Vec) { - let (unmarked_text, mut markers) = marked_text_by(marked_text, vec!['|']); - (unmarked_text, markers.remove(&'|').unwrap_or_else(Vec::new)) -} - -pub fn marked_text_ranges( - marked_text: &str, - range_markers: Vec<(char, char)>, -) -> (String, Vec>) { - let mut marker_chars = Vec::new(); - for (start, end) in range_markers.iter() { - marker_chars.push(*start); - marker_chars.push(*end); - } - let (unmarked_text, markers) = marked_text_by(marked_text, marker_chars); - let ranges = range_markers - .iter() - .map(|(start_marker, end_marker)| { - let start = markers.get(start_marker).unwrap()[0]; - let end = markers.get(end_marker).unwrap()[0]; - start..end - }) - .collect(); - (unmarked_text, ranges) -} - // Returns a snapshot from text containing '|' character markers with the markers removed, and DisplayPoints for each one. pub fn marked_display_snapshot( text: &str, diff --git a/crates/util/src/test.rs b/crates/util/src/test.rs index 71b847df69..b4cf25274e 100644 --- a/crates/util/src/test.rs +++ b/crates/util/src/test.rs @@ -1,4 +1,8 @@ -use std::path::{Path, PathBuf}; +use std::{ + collections::HashMap, + ops::Range, + path::{Path, PathBuf}, +}; use tempdir::TempDir; pub fn temp_tree(tree: serde_json::Value) -> TempDir { @@ -48,3 +52,48 @@ pub fn sample_text(rows: usize, cols: usize, start_char: char) -> String { } text } + +pub fn marked_text_by( + marked_text: &str, + markers: Vec, +) -> (String, HashMap>) { + let mut extracted_markers: HashMap> = Default::default(); + let mut unmarked_text = String::new(); + + for char in marked_text.chars() { + if markers.contains(&char) { + let char_offsets = extracted_markers.entry(char).or_insert(Vec::new()); + char_offsets.push(unmarked_text.len()); + } else { + unmarked_text.push(char); + } + } + + (unmarked_text, extracted_markers) +} + +pub fn marked_text(marked_text: &str) -> (String, Vec) { + let (unmarked_text, mut markers) = marked_text_by(marked_text, vec!['|']); + (unmarked_text, markers.remove(&'|').unwrap_or_else(Vec::new)) +} + +pub fn marked_text_ranges( + marked_text: &str, + range_markers: Vec<(char, char)>, +) -> (String, Vec>) { + let mut marker_chars = Vec::new(); + for (start, end) in range_markers.iter() { + marker_chars.push(*start); + marker_chars.push(*end); + } + let (unmarked_text, markers) = marked_text_by(marked_text, marker_chars); + let ranges = range_markers + .iter() + .map(|(start_marker, end_marker)| { + let start = markers.get(start_marker).unwrap()[0]; + let end = markers.get(end_marker).unwrap()[0]; + start..end + }) + .collect(); + (unmarked_text, ranges) +} diff --git a/crates/vim/Cargo.toml b/crates/vim/Cargo.toml index a223172749..28ee7de872 100644 --- a/crates/vim/Cargo.toml +++ b/crates/vim/Cargo.toml @@ -16,8 +16,10 @@ workspace = { path = "../workspace" } log = "0.4" [dev-dependencies] +indoc = "1.0.4" editor = { path = "../editor", features = ["test-support"] } gpui = { path = "../gpui", features = ["test-support"] } project = { path = "../project", features = ["test-support"] } language = { path = "../language", features = ["test-support"] } +util = { path = "../util", features = ["test-support"] } workspace = { path = "../workspace", features = ["test-support"] } \ No newline at end of file diff --git a/crates/vim/src/editor_events.rs b/crates/vim/src/editor_events.rs index 388de1a2ca..2f8a60ef33 100644 --- a/crates/vim/src/editor_events.rs +++ b/crates/vim/src/editor_events.rs @@ -13,9 +13,7 @@ pub fn init(cx: &mut MutableAppContext) { fn editor_created(EditorCreated(editor): &EditorCreated, cx: &mut MutableAppContext) { cx.update_default_global(|vim_state: &mut VimState, cx| { vim_state.editors.insert(editor.id(), editor.downgrade()); - if vim_state.enabled { - VimState::update_cursor_shapes(cx); - } + VimState::sync_editor_options(cx); }) } @@ -40,9 +38,7 @@ fn editor_blurred(EditorBlurred(editor): &EditorBlurred, cx: &mut MutableAppCont } } }); - editor.update(cx, |editor, _| { - editor.remove_keymap_context_layer::(); - }) + VimState::sync_editor_options(cx); } fn editor_released(EditorReleased(editor): &EditorReleased, cx: &mut MutableAppContext) { diff --git a/crates/vim/src/editor_utils.rs b/crates/vim/src/editor_utils.rs deleted file mode 100644 index 21dc498436..0000000000 --- a/crates/vim/src/editor_utils.rs +++ /dev/null @@ -1,97 +0,0 @@ -use editor::{display_map::DisplaySnapshot, Bias, DisplayPoint, Editor}; -use gpui::ViewContext; -use language::{Selection, SelectionGoal}; - -pub trait VimEditorExt { - fn clip_selections(self: &mut Self, cx: &mut ViewContext); - fn clipped_move_selections( - self: &mut Self, - cx: &mut ViewContext, - move_selection: impl Fn(&DisplaySnapshot, &mut Selection), - ); - fn clipped_move_selection_heads( - &mut self, - cx: &mut ViewContext, - update_head: impl Fn( - &DisplaySnapshot, - DisplayPoint, - SelectionGoal, - ) -> (DisplayPoint, SelectionGoal), - ); - fn clipped_move_cursors( - self: &mut Self, - cx: &mut ViewContext, - update_cursor_position: impl Fn( - &DisplaySnapshot, - DisplayPoint, - SelectionGoal, - ) -> (DisplayPoint, SelectionGoal), - ); -} - -pub fn clip_display_point(map: &DisplaySnapshot, mut display_point: DisplayPoint) -> DisplayPoint { - let next_char = map.chars_at(display_point).next(); - if next_char == Some('\n') || next_char == None { - *display_point.column_mut() = display_point.column().saturating_sub(1); - display_point = map.clip_point(display_point, Bias::Left); - } - display_point -} - -impl VimEditorExt for Editor { - fn clip_selections(self: &mut Self, cx: &mut ViewContext) { - self.move_selections(cx, |map, selection| { - if selection.is_empty() { - let adjusted_cursor = clip_display_point(map, selection.start); - selection.collapse_to(adjusted_cursor, selection.goal); - } else { - let adjusted_head = clip_display_point(map, selection.head()); - selection.set_head(adjusted_head, selection.goal); - } - }) - } - - fn clipped_move_selections( - self: &mut Self, - cx: &mut ViewContext, - move_selection: impl Fn(&DisplaySnapshot, &mut Selection), - ) { - self.move_selections(cx, |map, selection| { - move_selection(map, selection); - let adjusted_head = clip_display_point(map, selection.head()); - selection.set_head(adjusted_head, selection.goal); - }) - } - - fn clipped_move_selection_heads( - &mut self, - cx: &mut ViewContext, - update_head: impl Fn( - &DisplaySnapshot, - DisplayPoint, - SelectionGoal, - ) -> (DisplayPoint, SelectionGoal), - ) { - self.clipped_move_selections(cx, |map, selection| { - let (new_head, new_goal) = update_head(map, selection.head(), selection.goal); - let adjusted_head = clip_display_point(map, new_head); - selection.set_head(adjusted_head, new_goal); - }); - } - - fn clipped_move_cursors( - self: &mut Self, - cx: &mut ViewContext, - update_cursor_position: impl Fn( - &DisplaySnapshot, - DisplayPoint, - SelectionGoal, - ) -> (DisplayPoint, SelectionGoal), - ) { - self.move_selections(cx, |map, selection| { - let (cursor, new_goal) = update_cursor_position(map, selection.head(), selection.goal); - let adjusted_cursor = clip_display_point(map, cursor); - selection.collapse_to(adjusted_cursor, new_goal); - }); - } -} diff --git a/crates/vim/src/insert.rs b/crates/vim/src/insert.rs index d87dcb2445..50fd53b37b 100644 --- a/crates/vim/src/insert.rs +++ b/crates/vim/src/insert.rs @@ -3,7 +3,7 @@ use gpui::{action, keymap::Binding, MutableAppContext, ViewContext}; use language::SelectionGoal; use workspace::Workspace; -use crate::{editor_utils::VimEditorExt, mode::Mode, SwitchMode, VimState}; +use crate::{mode::Mode, SwitchMode, VimState}; action!(NormalBefore); @@ -18,11 +18,11 @@ pub fn init(cx: &mut MutableAppContext) { } fn normal_before(_: &mut Workspace, _: &NormalBefore, cx: &mut ViewContext) { - VimState::switch_mode(&SwitchMode(Mode::Normal), cx); VimState::update_active_editor(cx, |editor, cx| { - editor.clipped_move_cursors(cx, |map, mut cursor, _| { + editor.move_cursors(cx, |map, mut cursor, _| { *cursor.column_mut() = cursor.column().saturating_sub(1); (map.clip_point(cursor, Bias::Left), SelectionGoal::None) }); }); + VimState::switch_mode(&SwitchMode(Mode::Normal), cx); } diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs index 1aec087d95..68ecd4e003 100644 --- a/crates/vim/src/vim.rs +++ b/crates/vim/src/vim.rs @@ -1,5 +1,4 @@ mod editor_events; -mod editor_utils; mod insert; mod mode; mod normal; @@ -8,7 +7,6 @@ mod vim_tests; use collections::HashMap; use editor::{CursorShape, Editor}; -use editor_utils::VimEditorExt; use gpui::{action, MutableAppContext, ViewContext, WeakViewHandle}; use mode::Mode; @@ -49,21 +47,11 @@ impl VimState { } fn switch_mode(SwitchMode(mode): &SwitchMode, cx: &mut MutableAppContext) { - let active_editor = cx.update_default_global(|this: &mut Self, _| { + cx.update_default_global(|this: &mut Self, _| { this.mode = *mode; - this.active_editor.clone() }); - if let Some(active_editor) = active_editor.and_then(|e| e.upgrade(cx)) { - active_editor.update(cx, |active_editor, cx| { - active_editor.set_keymap_context_layer::(mode.keymap_context_layer()); - active_editor.set_input_enabled(*mode == Mode::Insert); - if *mode != Mode::Insert { - active_editor.clip_selections(cx); - } - }); - } - VimState::update_cursor_shapes(cx); + VimState::sync_editor_options(cx); } fn settings_changed(cx: &mut MutableAppContext) { @@ -76,20 +64,29 @@ impl VimState { } else { Mode::Insert }; - Self::update_cursor_shapes(cx); + Self::sync_editor_options(cx); } }); } - fn update_cursor_shapes(cx: &mut MutableAppContext) { + fn sync_editor_options(cx: &mut MutableAppContext) { cx.defer(move |cx| { cx.update_default_global(|this: &mut VimState, cx| { - let cursor_shape = this.mode.cursor_shape(); + let mode = this.mode; + let cursor_shape = mode.cursor_shape(); + let keymap_layer_active = this.enabled; for editor in this.editors.values() { if let Some(editor) = editor.upgrade(cx) { editor.update(cx, |editor, cx| { editor.set_cursor_shape(cursor_shape, cx); editor.set_clip_at_line_ends(cursor_shape == CursorShape::Block, cx); + editor.set_input_enabled(mode == Mode::Insert); + if keymap_layer_active { + let context_layer = mode.keymap_context_layer(); + editor.set_keymap_context_layer::(context_layer); + } else { + editor.remove_keymap_context_layer::(); + } }); } } diff --git a/crates/vim/src/vim_tests.rs b/crates/vim/src/vim_tests.rs index f898809138..e35ed9a122 100644 --- a/crates/vim/src/vim_tests.rs +++ b/crates/vim/src/vim_tests.rs @@ -1,8 +1,10 @@ +use indoc::indoc; use std::ops::Deref; use editor::{display_map::ToDisplayPoint, DisplayPoint}; use gpui::{json::json, keymap::Keystroke, ViewHandle}; use language::{Point, Selection}; +use util::test::marked_text; use workspace::{WorkspaceHandle, WorkspaceParams}; use crate::*; @@ -10,46 +12,98 @@ use crate::*; #[gpui::test] async fn test_insert_mode(cx: &mut gpui::TestAppContext) { let mut cx = VimTestAppContext::new(cx, "").await; - assert_eq!(cx.mode(), Mode::Normal); cx.simulate_keystroke("i"); assert_eq!(cx.mode(), Mode::Insert); cx.simulate_keystrokes(&["T", "e", "s", "t"]); - assert_eq!(cx.editor_text(), "Test".to_owned()); + cx.assert_newest_selection_head("Test|"); cx.simulate_keystroke("escape"); assert_eq!(cx.mode(), Mode::Normal); + cx.assert_newest_selection_head("Tes|t"); } #[gpui::test] async fn test_normal_hjkl(cx: &mut gpui::TestAppContext) { let mut cx = VimTestAppContext::new(cx, "Test\nTestTest\nTest").await; - assert_eq!(cx.mode(), Mode::Normal); cx.simulate_keystroke("l"); - assert_eq!(cx.newest_selection().head(), DisplayPoint::new(0, 1)); + cx.assert_newest_selection_head(indoc! {" + T|est + TestTest + Test"}); cx.simulate_keystroke("h"); - assert_eq!(cx.newest_selection().head(), DisplayPoint::new(0, 0)); + cx.assert_newest_selection_head(indoc! {" + |Test + TestTest + Test"}); cx.simulate_keystroke("j"); - assert_eq!(cx.newest_selection().head(), DisplayPoint::new(1, 0)); + cx.assert_newest_selection_head(indoc! {" + Test + |TestTest + Test"}); cx.simulate_keystroke("k"); - assert_eq!(cx.newest_selection().head(), DisplayPoint::new(0, 0)); - + cx.assert_newest_selection_head(indoc! {" + |Test + TestTest + Test"}); cx.simulate_keystroke("j"); - assert_eq!(cx.newest_selection().head(), DisplayPoint::new(1, 0)); + cx.assert_newest_selection_head(indoc! {" + Test + |TestTest + Test"}); // When moving left, cursor does not wrap to the previous line cx.simulate_keystroke("h"); - assert_eq!(cx.newest_selection().head(), DisplayPoint::new(1, 0)); + cx.assert_newest_selection_head(indoc! {" + Test + |TestTest + Test"}); // When moving right, cursor does not reach the line end or wrap to the next line for _ in 0..9 { cx.simulate_keystroke("l"); } - assert_eq!(cx.newest_selection().head(), DisplayPoint::new(1, 7)); + cx.assert_newest_selection_head(indoc! {" + Test + TestTes|t + Test"}); // Goal column respects the inability to reach the end of the line cx.simulate_keystroke("k"); - assert_eq!(cx.newest_selection().head(), DisplayPoint::new(0, 3)); + cx.assert_newest_selection_head(indoc! {" + Tes|t + TestTest + Test"}); cx.simulate_keystroke("j"); - assert_eq!(cx.newest_selection().head(), DisplayPoint::new(1, 7)); + cx.assert_newest_selection_head(indoc! {" + Test + TestTes|t + Test"}); +} + +#[gpui::test] +async fn test_toggle_through_settings(cx: &mut gpui::TestAppContext) { + let mut cx = VimTestAppContext::new(cx, "").await; + + // Editor acts as though vim is disabled + cx.disable_vim(); + assert_eq!(cx.mode(), Mode::Insert); + cx.simulate_keystrokes(&["h", "j", "k", "l"]); + cx.assert_newest_selection_head("hjkl|"); + + // Enabling dynamically sets vim mode again + cx.enable_vim(); + assert_eq!(cx.mode(), Mode::Normal); + cx.simulate_keystrokes(&["h", "h", "h", "l"]); + assert_eq!(cx.editor_text(), "hjkl".to_owned()); + cx.assert_newest_selection_head("hj|kl"); + cx.simulate_keystrokes(&["i", "T", "e", "s", "t"]); + cx.assert_newest_selection_head("hjTest|kl"); + + // Disabling and enabling resets to normal mode + assert_eq!(cx.mode(), Mode::Insert); + cx.disable_vim(); + assert_eq!(cx.mode(), Mode::Insert); + cx.enable_vim(); + assert_eq!(cx.mode(), Mode::Normal); } struct VimTestAppContext<'a> { @@ -68,6 +122,13 @@ impl<'a> VimTestAppContext<'a> { crate::init(cx); }); let params = cx.update(WorkspaceParams::test); + + cx.update(|cx| { + cx.update_global(|settings: &mut Settings, _| { + settings.vim_mode = true; + }); + }); + params .fs .as_fake() @@ -107,6 +168,22 @@ impl<'a> VimTestAppContext<'a> { } } + fn enable_vim(&mut self) { + self.cx.update(|cx| { + cx.update_global(|settings: &mut Settings, _| { + settings.vim_mode = true; + }); + }) + } + + fn disable_vim(&mut self) { + self.cx.update(|cx| { + cx.update_global(|settings: &mut Settings, _| { + settings.vim_mode = false; + }); + }) + } + fn newest_selection(&mut self) -> Selection { self.editor.update(self.cx, |editor, cx| { let snapshot = editor.snapshot(cx); @@ -141,6 +218,20 @@ impl<'a> VimTestAppContext<'a> { self.simulate_keystroke(keystroke_text); } } + + fn assert_newest_selection_head(&mut self, text: &str) { + let (unmarked_text, markers) = marked_text(&text); + assert_eq!( + self.editor_text(), + unmarked_text, + "Unmarked text doesn't match editor text" + ); + let newest_selection = self.newest_selection(); + let expected_head = self.editor.update(self.cx, |editor, cx| { + markers[0].to_display_point(&editor.snapshot(cx)) + }); + assert_eq!(newest_selection.head(), expected_head) + } } impl<'a> Deref for VimTestAppContext<'a> {