From 2092c82e8ff05e5cdbdd52a004cbcc6778931c8b Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Fri, 17 Feb 2023 13:50:00 -0500 Subject: [PATCH 1/7] factor write command tests to own module --- helix-term/tests/integration.rs | 1 - helix-term/tests/test/commands.rs | 93 +----------------- helix-term/tests/test/{ => commands}/write.rs | 94 ++++++++++++++++++- 3 files changed, 93 insertions(+), 95 deletions(-) rename helix-term/tests/test/{ => commands}/write.rs (76%) diff --git a/helix-term/tests/integration.rs b/helix-term/tests/integration.rs index a378af7a9b25..be1bfc2c0efb 100644 --- a/helix-term/tests/integration.rs +++ b/helix-term/tests/integration.rs @@ -23,5 +23,4 @@ mod test { mod movement; mod prompt; mod splits; - mod write; } diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index e8d16bfaf2be..74c32c4a568e 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -1,99 +1,8 @@ -use std::ops::RangeInclusive; - -use helix_core::diagnostic::Severity; use helix_term::application::Application; use super::*; -#[tokio::test(flavor = "multi_thread")] -async fn test_write_quit_fail() -> anyhow::Result<()> { - let file = helpers::new_readonly_tempfile()?; - let mut app = helpers::AppBuilder::new() - .with_file(file.path(), None) - .build()?; - - test_key_sequence( - &mut app, - Some("ihello:wq"), - Some(&|app| { - let mut docs: Vec<_> = app.editor.documents().collect(); - assert_eq!(1, docs.len()); - - let doc = docs.pop().unwrap(); - assert_eq!(Some(file.path()), doc.path().map(PathBuf::as_path)); - assert_eq!(&Severity::Error, app.editor.get_status().unwrap().1); - }), - false, - ) - .await?; - - Ok(()) -} - -#[tokio::test(flavor = "multi_thread")] -async fn test_buffer_close_concurrent() -> anyhow::Result<()> { - test_key_sequences( - &mut helpers::AppBuilder::new().build()?, - vec![ - ( - None, - Some(&|app| { - assert_eq!(1, app.editor.documents().count()); - assert!(!app.editor.is_err()); - }), - ), - ( - Some("ihello:new"), - Some(&|app| { - assert_eq!(2, app.editor.documents().count()); - assert!(!app.editor.is_err()); - }), - ), - ( - Some(":bufferclose"), - Some(&|app| { - assert_eq!(1, app.editor.documents().count()); - assert!(!app.editor.is_err()); - }), - ), - ], - false, - ) - .await?; - - // verify if writes are queued up, it finishes them before closing the buffer - let mut file = tempfile::NamedTempFile::new()?; - let mut command = String::new(); - const RANGE: RangeInclusive = 1..=1000; - - for i in RANGE { - let cmd = format!("%c{}:w!", i); - command.push_str(&cmd); - } - - command.push_str(":bufferclose"); - - let mut app = helpers::AppBuilder::new() - .with_file(file.path(), None) - .build()?; - - test_key_sequence( - &mut app, - Some(&command), - Some(&|app| { - assert!(!app.editor.is_err(), "error: {:?}", app.editor.get_status()); - - let doc = app.editor.document_by_path(file.path()); - assert!(doc.is_none(), "found doc: {:?}", doc); - }), - false, - ) - .await?; - - helpers::assert_file_has_content(file.as_file_mut(), &RANGE.end().to_string())?; - - Ok(()) -} +mod write; #[tokio::test(flavor = "multi_thread")] async fn test_selection_duplication() -> anyhow::Result<()> { diff --git a/helix-term/tests/test/write.rs b/helix-term/tests/test/commands/write.rs similarity index 76% rename from helix-term/tests/test/write.rs rename to helix-term/tests/test/commands/write.rs index 81459b2fe846..0ea66a12df15 100644 --- a/helix-term/tests/test/write.rs +++ b/helix-term/tests/test/commands/write.rs @@ -1,5 +1,5 @@ use std::{ - io::{Read, Seek, SeekFrom, Write}, + io::{Read, Seek, Write}, ops::RangeInclusive, }; @@ -8,6 +8,96 @@ use helix_view::doc; use super::*; +#[tokio::test(flavor = "multi_thread")] +async fn test_write_quit_fail() -> anyhow::Result<()> { + let file = helpers::new_readonly_tempfile()?; + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .build()?; + + test_key_sequence( + &mut app, + Some("ihello:wq"), + Some(&|app| { + let mut docs: Vec<_> = app.editor.documents().collect(); + assert_eq!(1, docs.len()); + + let doc = docs.pop().unwrap(); + assert_eq!(Some(file.path()), doc.path().map(PathBuf::as_path)); + assert_eq!(&Severity::Error, app.editor.get_status().unwrap().1); + }), + false, + ) + .await?; + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_buffer_close_concurrent() -> anyhow::Result<()> { + test_key_sequences( + &mut helpers::AppBuilder::new().build()?, + vec![ + ( + None, + Some(&|app| { + assert_eq!(1, app.editor.documents().count()); + assert!(!app.editor.is_err()); + }), + ), + ( + Some("ihello:new"), + Some(&|app| { + assert_eq!(2, app.editor.documents().count()); + assert!(!app.editor.is_err()); + }), + ), + ( + Some(":bufferclose"), + Some(&|app| { + assert_eq!(1, app.editor.documents().count()); + assert!(!app.editor.is_err()); + }), + ), + ], + false, + ) + .await?; + + // verify if writes are queued up, it finishes them before closing the buffer + let mut file = tempfile::NamedTempFile::new()?; + let mut command = String::new(); + const RANGE: RangeInclusive = 1..=1000; + + for i in RANGE { + let cmd = format!("%c{}:w!", i); + command.push_str(&cmd); + } + + command.push_str(":bufferclose"); + + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .build()?; + + test_key_sequence( + &mut app, + Some(&command), + Some(&|app| { + assert!(!app.editor.is_err(), "error: {:?}", app.editor.get_status()); + + let doc = app.editor.document_by_path(file.path()); + assert!(doc.is_none(), "found doc: {:?}", doc); + }), + false, + ) + .await?; + + helpers::assert_file_has_content(file.as_file_mut(), &RANGE.end().to_string())?; + + Ok(()) +} + #[tokio::test(flavor = "multi_thread")] async fn test_write() -> anyhow::Result<()> { let mut file = tempfile::NamedTempFile::new()?; @@ -57,7 +147,7 @@ async fn test_overwrite_protection() -> anyhow::Result<()> { file.as_file_mut().flush()?; file.as_file_mut().sync_all()?; - file.seek(SeekFrom::Start(0))?; + file.rewind()?; let mut file_content = String::new(); file.as_file_mut().read_to_string(&mut file_content)?; From 8e07ddd8b61bbd0727012ae33c3d2555e972d08d Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sat, 22 Oct 2022 22:18:32 -0400 Subject: [PATCH 2/7] make TestCase::From more generic --- helix-term/tests/test/auto_indent.rs | 5 ++--- helix-term/tests/test/helpers.rs | 9 +++++++-- helix-term/tests/test/movement.rs | 28 ++++++++++------------------ 3 files changed, 19 insertions(+), 23 deletions(-) diff --git a/helix-term/tests/test/auto_indent.rs b/helix-term/tests/test/auto_indent.rs index 2d9082853dcf..e626acad3179 100644 --- a/helix-term/tests/test/auto_indent.rs +++ b/helix-term/tests/test/auto_indent.rs @@ -11,14 +11,13 @@ async fn auto_indent_c() -> anyhow::Result<()> { helpers::test_syntax_conf(None), // switches to append mode? ( - helpers::platform_line("void foo() {#[|}]#").as_ref(), + helpers::platform_line("void foo() {#[|}]#"), "i", helpers::platform_line(indoc! {"\ void foo() { #[|\n]#\ } - "}) - .as_ref(), + "}), ), ) .await?; diff --git a/helix-term/tests/test/helpers.rs b/helix-term/tests/test/helpers.rs index fb12ef12cff2..59d08bb6acc4 100644 --- a/helix-term/tests/test/helpers.rs +++ b/helix-term/tests/test/helpers.rs @@ -22,8 +22,13 @@ pub struct TestCase { pub out_selection: Selection, } -impl> From<(S, S, S)> for TestCase { - fn from((input, keys, output): (S, S, S)) -> Self { +impl From<(S, R, V)> for TestCase +where + S: Into, + R: Into, + V: Into, +{ + fn from((input, keys, output): (S, R, V)) -> Self { let (in_text, in_selection) = test::print(&input.into()); let (out_text, out_selection) = test::print(&output.into()); diff --git a/helix-term/tests/test/movement.rs b/helix-term/tests/test/movement.rs index e6ea3f951592..6cc89021d046 100644 --- a/helix-term/tests/test/movement.rs +++ b/helix-term/tests/test/movement.rs @@ -395,7 +395,7 @@ async fn cursor_position_append_eof() -> anyhow::Result<()> { test(( "#[foo|]#", "abar", - helpers::platform_line("#[foobar|]#\n").as_ref(), + helpers::platform_line("#[foobar|]#\n"), )) .await?; @@ -403,7 +403,7 @@ async fn cursor_position_append_eof() -> anyhow::Result<()> { test(( "#[|foo]#", "abar", - helpers::platform_line("#[foobar|]#\n").as_ref(), + helpers::platform_line("#[foobar|]#\n"), )) .await?; @@ -425,16 +425,14 @@ async fn select_mode_tree_sitter_next_function_is_union_of_objects() -> anyhow:: fn inc(x: usize) -> usize { x + 1 } /// Decrements fn dec(x: usize) -> usize { x - 1 } - "}) - .as_ref(), + "}), "]fv]f", helpers::platform_line(indoc! {"\ /// Increments #[fn inc(x: usize) -> usize { x + 1 } /// Decrements fn dec(x: usize) -> usize { x - 1 }|]# - "}) - .as_ref(), + "}), ), ) .await?; @@ -457,16 +455,14 @@ async fn select_mode_tree_sitter_prev_function_unselects_object() -> anyhow::Res #[fn inc(x: usize) -> usize { x + 1 } /// Decrements fn dec(x: usize) -> usize { x - 1 }|]# - "}) - .as_ref(), + "}), "v[f", helpers::platform_line(indoc! {"\ /// Increments #[fn inc(x: usize) -> usize { x + 1 }|]# /// Decrements fn dec(x: usize) -> usize { x - 1 } - "}) - .as_ref(), + "}), ), ) .await?; @@ -492,8 +488,7 @@ async fn select_mode_tree_sitter_prev_function_goes_backwards_to_object() -> any fn dec(x: usize) -> usize { x - 1 } /// Identity #[fn ident(x: usize) -> usize { x }|]# - "}) - .as_ref(), + "}), "v[f", helpers::platform_line(indoc! {"\ /// Increments @@ -502,8 +497,7 @@ async fn select_mode_tree_sitter_prev_function_goes_backwards_to_object() -> any #[|fn dec(x: usize) -> usize { x - 1 } /// Identity ]#fn ident(x: usize) -> usize { x } - "}) - .as_ref(), + "}), ), ) .await?; @@ -523,8 +517,7 @@ async fn select_mode_tree_sitter_prev_function_goes_backwards_to_object() -> any fn dec(x: usize) -> usize { x - 1 } /// Identity #[fn ident(x: usize) -> usize { x }|]# - "}) - .as_ref(), + "}), "v[f[f", helpers::platform_line(indoc! {"\ /// Increments @@ -533,8 +526,7 @@ async fn select_mode_tree_sitter_prev_function_goes_backwards_to_object() -> any fn dec(x: usize) -> usize { x - 1 } /// Identity ]#fn ident(x: usize) -> usize { x } - "}) - .as_ref(), + "}), ), ) .await?; From 88d2fa29706c88ae45bca18da0b761ff71ca6536 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sat, 22 Oct 2022 22:19:24 -0400 Subject: [PATCH 3/7] print doc state during tests --- helix-term/tests/test/helpers.rs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/helix-term/tests/test/helpers.rs b/helix-term/tests/test/helpers.rs index 59d08bb6acc4..811f78484967 100644 --- a/helix-term/tests/test/helpers.rs +++ b/helix-term/tests/test/helpers.rs @@ -9,7 +9,7 @@ use anyhow::bail; use crossterm::event::{Event, KeyEvent}; use helix_core::{diagnostic::Severity, test, Selection, Transaction}; use helix_term::{application::Application, args::Args, config::Config, keymap::merge_keys}; -use helix_view::{doc, editor::LspConfig, input::parse_macro, Editor}; +use helix_view::{current_ref, doc, editor::LspConfig, input::parse_macro, Editor}; use tempfile::NamedTempFile; use tokio_stream::wrappers::UnboundedReceiverStream; @@ -64,6 +64,11 @@ pub async fn test_key_sequences( let num_inputs = inputs.len(); for (i, (in_keys, test_fn)) in inputs.into_iter().enumerate() { + let (view, doc) = current_ref!(app.editor); + let state = test::plain(&doc.text().to_string(), doc.selection(view.id)); + + log::debug!("executing test with document state:\n\n-----\n\n{}", state); + if let Some(in_keys) = in_keys { for key_event in parse_macro(in_keys)?.into_iter() { let key = Event::Key(KeyEvent::from(key_event)); @@ -74,6 +79,16 @@ pub async fn test_key_sequences( let app_exited = !app.event_loop_until_idle(&mut rx_stream).await; + if !app_exited { + let (view, doc) = current_ref!(app.editor); + let state = test::plain(&doc.text().to_string(), doc.selection(view.id)); + + log::debug!( + "finished running test with document state:\n\n-----\n\n{}", + state + ); + } + // the app should not exit from any test until the last one if i < num_inputs - 1 && app_exited { bail!("application exited before test function could run"); From 923b0a7898091c5ff3bae37a37eedd981e19ee33 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Tue, 28 Feb 2023 23:57:36 -0500 Subject: [PATCH 4/7] refactor test editor config --- helix-term/tests/test/helpers.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/helix-term/tests/test/helpers.rs b/helix-term/tests/test/helpers.rs index 811f78484967..de81f758dfea 100644 --- a/helix-term/tests/test/helpers.rs +++ b/helix-term/tests/test/helpers.rs @@ -232,15 +232,19 @@ pub fn temp_file_with_contents>( /// Generates a config with defaults more suitable for integration tests pub fn test_config() -> Config { merge_keys(Config { - editor: helix_view::editor::Config { - lsp: LspConfig { - enable: false, - ..Default::default() - }, + editor: test_editor_config(), + ..Default::default() + }) +} + +pub fn test_editor_config() -> helix_view::editor::Config { + helix_view::editor::Config { + lsp: LspConfig { + enable: false, ..Default::default() }, ..Default::default() - }) + } } /// Replaces all LF chars with the system's appropriate line feed @@ -282,7 +286,7 @@ impl Default for AppBuilder { fn default() -> Self { Self { args: Args::default(), - config: Config::default(), + config: test_config(), syn_conf: test_syntax_conf(None), input: None, } From 4cb06ce1d2015c10d013359b8854663ca42d90cb Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sat, 17 Sep 2022 09:34:56 -0400 Subject: [PATCH 5/7] Allow explicit newlines in test DSL The current test DSL currently has no way to express being at the end of a line, save for putting an explicit LF or CRLF inside the `#[|]#`. The problem with this approach is that it can add unintended extra new lines if used in conjunction with raw strings, which insert newlines for you. This is a simple attempt to mitigate this problem. If there is an explicit newline character at the end of the selection, and then it is immediately followed by the same newline character at the right end of the selection, this following newline is removed. This way, one can express a cursor at the end of a line explicitly. --- Cargo.lock | 53 +++++++++++++++------------ helix-core/Cargo.toml | 1 + helix-core/src/movement.rs | 14 ++++---- helix-core/src/test.rs | 59 ++++++++++++++++++++++--------- helix-core/src/textobject.rs | 6 ++-- helix-term/tests/test/commands.rs | 44 +++++++++++------------ 6 files changed, 106 insertions(+), 71 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index af0858efe4d7..e1b53eb0df9c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -143,9 +143,9 @@ dependencies = [ [[package]] name = "chrono" -version = "0.4.24" +version = "0.4.23" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4e3c5919066adf22df73762e50cffcde3a758f2a848b113b586d1f86728b673b" +checksum = "16b0a3d9ed01224b22057780a37bb8c5dbfe1be8ba48678e7bf57ec4b385411f" dependencies = [ "iana-time-zone", "num-integer", @@ -932,9 +932,9 @@ dependencies = [ [[package]] name = "gix-tempfile" -version = "5.0.0" +version = "5.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "743bae41b5db7f085dc7acc54ed72c98853a6e5dabb355e95caa7b534f21b35c" +checksum = "aed73ef9642f779d609fd19acc332ac1597b978ee87ec11743a68eefaed65bfa" dependencies = [ "libc", "once_cell", @@ -1081,6 +1081,7 @@ dependencies = [ "hashbrown 0.13.2", "helix-loader", "imara-diff", + "indoc 1.0.9", "log", "once_cell", "quickcheck", @@ -1176,7 +1177,7 @@ dependencies = [ "helix-vcs", "helix-view", "ignore", - "indoc", + "indoc 2.0.1", "libc", "log", "once_cell", @@ -1349,6 +1350,12 @@ dependencies = [ "hashbrown 0.12.3", ] +[[package]] +name = "indoc" +version = "1.0.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bfa799dd5ed20a7e349f3b4639aa80d74549c81716d9ec4f994c9b5815598306" + [[package]] name = "indoc" version = "2.0.1" @@ -1532,6 +1539,15 @@ dependencies = [ "minimal-lexical", ] +[[package]] +name = "nom8" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ae01545c9c7fc4486ab7debaf2aad7003ac19431791868fb2e8066df97fad2f8" +dependencies = [ + "memchr", +] + [[package]] name = "num-integer" version = "0.1.45" @@ -1775,18 +1791,18 @@ checksum = "9c8132065adcfd6e02db789d9285a0deb2f3fcb04002865ab67d5fb103533898" [[package]] name = "serde" -version = "1.0.155" +version = "1.0.152" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "71f2b4817415c6d4210bfe1c7bfcf4801b2d904cb4d0e1a8fdb651013c9e86b8" +checksum = "bb7d1f0d3021d347a83e556fc4683dea2ea09d87bccdf88ff5c12545d89d5efb" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.155" +version = "1.0.152" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d071a94a3fac4aff69d023a7f411e33f40f3483f8c5190b1953822b6b76d7630" +checksum = "af487d118eecd09402d70a5d72551860e788df87b464af30e5ea6a38c75c541e" dependencies = [ "proc-macro2", "quote", @@ -2120,9 +2136,9 @@ dependencies = [ [[package]] name = "toml" -version = "0.7.3" +version = "0.7.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b403acf6f2bb0859c93c7f0d967cb4a75a7ac552100f9322faf64dc047669b21" +checksum = "f7afcae9e3f0fe2c370fd4657108972cbb2fa9db1b9f84849cefd80741b01cb6" dependencies = [ "serde", "serde_spanned", @@ -2141,15 +2157,15 @@ dependencies = [ [[package]] name = "toml_edit" -version = "0.19.6" +version = "0.19.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "08de71aa0d6e348f070457f85af8bd566e2bc452156a423ddf22861b3a953fae" +checksum = "5e6a7712b49e1775fb9a7b998de6635b299237f48b404dde71704f2e0e7f37e5" dependencies = [ "indexmap", + "nom8", "serde", "serde_spanned", "toml_datetime", - "winnow", ] [[package]] @@ -2452,15 +2468,6 @@ version = "0.42.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "447660ad36a13288b1db4d4248e857b510e8c3a225c822ba4fb748c0aafecffd" -[[package]] -name = "winnow" -version = "0.3.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ee7b2c67f962bf5042bfd8b6a916178df33a26eec343ae064cb8e069f638fa6f" -dependencies = [ - "memchr", -] - [[package]] name = "xtask" version = "0.6.0" diff --git a/helix-core/Cargo.toml b/helix-core/Cargo.toml index 62ec87b485ca..8618f5863d50 100644 --- a/helix-core/Cargo.toml +++ b/helix-core/Cargo.toml @@ -49,3 +49,4 @@ textwrap = "0.16.0" [dev-dependencies] quickcheck = { version = "1", default-features = false } +indoc = "1.0.6" diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs index 8e6b63066201..bbb37bf49df6 100644 --- a/helix-core/src/movement.rs +++ b/helix-core/src/movement.rs @@ -1474,7 +1474,7 @@ mod test { let text = Rope::from(s.as_str()); let selection = selection.transform(|r| move_prev_paragraph(text.slice(..), r, 1, Movement::Move)); - let actual = crate::test::plain(&s, selection); + let actual = crate::test::plain(&s, &selection); assert_eq!(actual, expected, "\nbefore: `{:?}`", before); } } @@ -1497,7 +1497,7 @@ mod test { let text = Rope::from(s.as_str()); let selection = selection.transform(|r| move_prev_paragraph(text.slice(..), r, 2, Movement::Move)); - let actual = crate::test::plain(&s, selection); + let actual = crate::test::plain(&s, &selection); assert_eq!(actual, expected, "\nbefore: `{:?}`", before); } } @@ -1520,7 +1520,7 @@ mod test { let text = Rope::from(s.as_str()); let selection = selection .transform(|r| move_prev_paragraph(text.slice(..), r, 1, Movement::Extend)); - let actual = crate::test::plain(&s, selection); + let actual = crate::test::plain(&s, &selection); assert_eq!(actual, expected, "\nbefore: `{:?}`", before); } } @@ -1540,7 +1540,7 @@ mod test { "a\nb\n\n#[goto\nthird\n\n|]#paragraph", ), ( - "a\nb#[\n|]#\ngoto\nsecond\n\nparagraph", + "a\nb#[\n|]#\n\ngoto\nsecond\n\nparagraph", "a\nb#[\n\n|]#goto\nsecond\n\nparagraph", ), ( @@ -1562,7 +1562,7 @@ mod test { let text = Rope::from(s.as_str()); let selection = selection.transform(|r| move_next_paragraph(text.slice(..), r, 1, Movement::Move)); - let actual = crate::test::plain(&s, selection); + let actual = crate::test::plain(&s, &selection); assert_eq!(actual, expected, "\nbefore: `{:?}`", before); } } @@ -1585,7 +1585,7 @@ mod test { let text = Rope::from(s.as_str()); let selection = selection.transform(|r| move_next_paragraph(text.slice(..), r, 2, Movement::Move)); - let actual = crate::test::plain(&s, selection); + let actual = crate::test::plain(&s, &selection); assert_eq!(actual, expected, "\nbefore: `{:?}`", before); } } @@ -1608,7 +1608,7 @@ mod test { let text = Rope::from(s.as_str()); let selection = selection .transform(|r| move_next_paragraph(text.slice(..), r, 1, Movement::Extend)); - let actual = crate::test::plain(&s, selection); + let actual = crate::test::plain(&s, &selection); assert_eq!(actual, expected, "\nbefore: `{:?}`", before); } } diff --git a/helix-core/src/test.rs b/helix-core/src/test.rs index 17523ed76107..1d967f23daf5 100644 --- a/helix-core/src/test.rs +++ b/helix-core/src/test.rs @@ -2,6 +2,7 @@ use crate::{Range, Selection}; use smallvec::SmallVec; use std::cmp::Reverse; +use unicode_segmentation::UnicodeSegmentation; /// Convert annotated test string to test string and selection. /// @@ -10,6 +11,10 @@ use std::cmp::Reverse; /// `#[` for primary selection with head after anchor followed by `|]#`. /// `#(` for secondary selection with head after anchor followed by `|)#`. /// +/// If the selection contains any LF or CRLF sequences, which are immediately +/// followed by the same grapheme, then the subsequent one is removed. This is +/// to allow representing having the cursor over the end of the line. +/// /// # Examples /// /// ``` @@ -30,23 +35,23 @@ use std::cmp::Reverse; pub fn print(s: &str) -> (String, Selection) { let mut primary_idx = None; let mut ranges = SmallVec::new(); - let mut iter = s.chars().peekable(); + let mut iter = UnicodeSegmentation::graphemes(s, true).peekable(); let mut left = String::with_capacity(s.len()); 'outer: while let Some(c) = iter.next() { let start = left.chars().count(); - if c != '#' { - left.push(c); + if c != "#" { + left.push_str(c); continue; } let (is_primary, close_pair) = match iter.next() { - Some('[') => (true, ']'), - Some('(') => (false, ')'), + Some("[") => (true, "]"), + Some("(") => (false, ")"), Some(ch) => { left.push('#'); - left.push(ch); + left.push_str(ch); continue; } None => break, @@ -56,24 +61,45 @@ pub fn print(s: &str) -> (String, Selection) { panic!("primary `#[` already appeared {:?} {:?}", left, s); } - let head_at_beg = iter.next_if_eq(&'|').is_some(); + let head_at_beg = iter.next_if_eq(&"|").is_some(); + let last_grapheme = |s: &str| { + UnicodeSegmentation::graphemes(s, true) + .last() + .map(String::from) + }; while let Some(c) = iter.next() { - if !(c == close_pair && iter.peek() == Some(&'#')) { - left.push(c); + let next = iter.peek(); + let mut prev = last_grapheme(left.as_str()); + + if !(c == close_pair && next == Some(&"#")) { + left.push_str(c); continue; } if !head_at_beg { - let prev = left.pop().unwrap(); - if prev != '|' { - left.push(prev); - left.push(c); - continue; + match &prev { + Some(p) if p != "|" => { + left.push_str(c); + continue; + } + Some(p) if p == "|" => { + left.pop().unwrap(); // pop the | + prev = last_grapheme(left.as_str()); + } + _ => (), } } iter.next(); // skip "#" + let next = iter.peek(); + + // skip explicit line end inside selection + if (prev == Some(String::from("\r\n")) || prev == Some(String::from("\n"))) + && next.map(|n| String::from(*n)) == prev + { + iter.next(); + } if is_primary { primary_idx = Some(ranges.len()); @@ -118,11 +144,11 @@ pub fn print(s: &str) -> (String, Selection) { /// use smallvec::smallvec; /// /// assert_eq!( -/// plain("abc", Selection::new(smallvec![Range::new(0, 1), Range::new(3, 2)], 0)), +/// plain("abc", &Selection::new(smallvec![Range::new(0, 1), Range::new(3, 2)], 0)), /// "#[a|]#b#(|c)#".to_owned() /// ); /// ``` -pub fn plain(s: &str, selection: Selection) -> String { +pub fn plain(s: &str, selection: &Selection) -> String { let primary = selection.primary_index(); let mut out = String::with_capacity(s.len() + 5 * selection.len()); out.push_str(s); @@ -147,6 +173,7 @@ pub fn plain(s: &str, selection: Selection) -> String { out } +#[allow(clippy::module_inception)] #[cfg(test)] #[allow(clippy::module_inception)] mod test { diff --git a/helix-core/src/textobject.rs b/helix-core/src/textobject.rs index 972a80e78a60..6e3f18cf0efd 100644 --- a/helix-core/src/textobject.rs +++ b/helix-core/src/textobject.rs @@ -437,7 +437,7 @@ mod test { let text = Rope::from(s.as_str()); let selection = selection .transform(|r| textobject_paragraph(text.slice(..), r, TextObject::Inside, 1)); - let actual = crate::test::plain(&s, selection); + let actual = crate::test::plain(&s, &selection); assert_eq!(actual, expected, "\nbefore: `{:?}`", before); } } @@ -460,7 +460,7 @@ mod test { let text = Rope::from(s.as_str()); let selection = selection .transform(|r| textobject_paragraph(text.slice(..), r, TextObject::Inside, 2)); - let actual = crate::test::plain(&s, selection); + let actual = crate::test::plain(&s, &selection); assert_eq!(actual, expected, "\nbefore: `{:?}`", before); } } @@ -491,7 +491,7 @@ mod test { let text = Rope::from(s.as_str()); let selection = selection .transform(|r| textobject_paragraph(text.slice(..), r, TextObject::Around, 1)); - let actual = crate::test::plain(&s, selection); + let actual = crate::test::plain(&s, &selection); assert_eq!(actual, expected, "\nbefore: `{:?}`", before); } } diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index 74c32c4a568e..342a849be349 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -201,12 +201,12 @@ async fn test_multi_selection_shell_commands() -> anyhow::Result<()> { .as_str(), "|echo foo", platform_line(indoc! {"\ - #[|foo - ]# - #(|foo - )# - #(|foo - )# + #[|foo\n]# + + #(|foo\n)# + + #(|foo\n)# + "}) .as_str(), )) @@ -222,12 +222,12 @@ async fn test_multi_selection_shell_commands() -> anyhow::Result<()> { .as_str(), "!echo foo", platform_line(indoc! {"\ - #[|foo - ]#lorem - #(|foo - )#ipsum - #(|foo - )#dolor + #[|foo\n]# + lorem + #(|foo\n)# + ipsum + #(|foo\n)# + dolor "}) .as_str(), )) @@ -243,12 +243,12 @@ async fn test_multi_selection_shell_commands() -> anyhow::Result<()> { .as_str(), "echo foo", platform_line(indoc! {"\ - lorem#[|foo - ]# - ipsum#(|foo - )# - dolor#(|foo - )# + lorem#[|foo\n]# + + ipsum#(|foo\n)# + + dolor#(|foo\n)# + "}) .as_str(), )) @@ -300,8 +300,8 @@ async fn test_extend_line() -> anyhow::Result<()> { platform_line(indoc! {"\ #[lorem ipsum - dolor - |]# + dolor\n|]# + "}) .as_str(), )) @@ -318,8 +318,8 @@ async fn test_extend_line() -> anyhow::Result<()> { "2x", platform_line(indoc! {"\ #[lorem - ipsum - |]# + ipsum\n|]# + "}) .as_str(), )) From df776b9b31b5e11e4090027b1dcced87c770e777 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sat, 22 Oct 2022 17:29:09 -0400 Subject: [PATCH 6/7] fix test::plain test::plain uses char indices when it should use byte indices --- helix-core/src/movement.rs | 12 ++-- helix-core/src/test.rs | 102 +++++++++++++++++++++++++++++-- helix-core/src/textobject.rs | 6 +- helix-term/tests/test/helpers.rs | 4 +- 4 files changed, 109 insertions(+), 15 deletions(-) diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs index bbb37bf49df6..60af47e5bf19 100644 --- a/helix-core/src/movement.rs +++ b/helix-core/src/movement.rs @@ -1474,7 +1474,7 @@ mod test { let text = Rope::from(s.as_str()); let selection = selection.transform(|r| move_prev_paragraph(text.slice(..), r, 1, Movement::Move)); - let actual = crate::test::plain(&s, &selection); + let actual = crate::test::plain(s.as_ref(), &selection); assert_eq!(actual, expected, "\nbefore: `{:?}`", before); } } @@ -1497,7 +1497,7 @@ mod test { let text = Rope::from(s.as_str()); let selection = selection.transform(|r| move_prev_paragraph(text.slice(..), r, 2, Movement::Move)); - let actual = crate::test::plain(&s, &selection); + let actual = crate::test::plain(s.as_ref(), &selection); assert_eq!(actual, expected, "\nbefore: `{:?}`", before); } } @@ -1520,7 +1520,7 @@ mod test { let text = Rope::from(s.as_str()); let selection = selection .transform(|r| move_prev_paragraph(text.slice(..), r, 1, Movement::Extend)); - let actual = crate::test::plain(&s, &selection); + let actual = crate::test::plain(s.as_ref(), &selection); assert_eq!(actual, expected, "\nbefore: `{:?}`", before); } } @@ -1562,7 +1562,7 @@ mod test { let text = Rope::from(s.as_str()); let selection = selection.transform(|r| move_next_paragraph(text.slice(..), r, 1, Movement::Move)); - let actual = crate::test::plain(&s, &selection); + let actual = crate::test::plain(s.as_ref(), &selection); assert_eq!(actual, expected, "\nbefore: `{:?}`", before); } } @@ -1585,7 +1585,7 @@ mod test { let text = Rope::from(s.as_str()); let selection = selection.transform(|r| move_next_paragraph(text.slice(..), r, 2, Movement::Move)); - let actual = crate::test::plain(&s, &selection); + let actual = crate::test::plain(s.as_ref(), &selection); assert_eq!(actual, expected, "\nbefore: `{:?}`", before); } } @@ -1608,7 +1608,7 @@ mod test { let text = Rope::from(s.as_str()); let selection = selection .transform(|r| move_next_paragraph(text.slice(..), r, 1, Movement::Extend)); - let actual = crate::test::plain(&s, &selection); + let actual = crate::test::plain(s.as_ref(), &selection); assert_eq!(actual, expected, "\nbefore: `{:?}`", before); } } diff --git a/helix-core/src/test.rs b/helix-core/src/test.rs index 1d967f23daf5..7183302c6c69 100644 --- a/helix-core/src/test.rs +++ b/helix-core/src/test.rs @@ -1,5 +1,6 @@ //! Test helpers. use crate::{Range, Selection}; +use ropey::Rope; use smallvec::SmallVec; use std::cmp::Reverse; use unicode_segmentation::UnicodeSegmentation; @@ -148,10 +149,12 @@ pub fn print(s: &str) -> (String, Selection) { /// "#[a|]#b#(|c)#".to_owned() /// ); /// ``` -pub fn plain(s: &str, selection: &Selection) -> String { +pub fn plain>(s: R, selection: &Selection) -> String { + let s = s.into(); let primary = selection.primary_index(); - let mut out = String::with_capacity(s.len() + 5 * selection.len()); - out.push_str(s); + let mut out = String::with_capacity(s.len_bytes() + 5 * selection.len()); + out.push_str(&s.to_string()); + let mut insertion: Vec<_> = selection .iter() .enumerate() @@ -164,7 +167,9 @@ pub fn plain(s: &str, selection: &Selection) -> String { (false, false) => [(range.anchor, ")#"), (range.head, "#(|")], } }) + .map(|(char_idx, marker)| (s.char_to_byte(char_idx), marker)) .collect(); + // insert in reverse order insertion.sort_unstable_by_key(|k| Reverse(k.0)); for (i, s) in insertion { @@ -173,7 +178,6 @@ pub fn plain(s: &str, selection: &Selection) -> String { out } -#[allow(clippy::module_inception)] #[cfg(test)] #[allow(clippy::module_inception)] mod test { @@ -289,4 +293,94 @@ mod test { print("hello #[|πŸ‘¨β€πŸ‘©β€πŸ‘§β€πŸ‘¦]# goodbye") ); } + + #[test] + fn plain_single() { + assert_eq!("#[|h]#ello", plain("hello", &Selection::single(1, 0))); + assert_eq!("#[h|]#ello", plain("hello", &Selection::single(0, 1))); + assert_eq!("#[|hell]#o", plain("hello", &Selection::single(4, 0))); + assert_eq!("#[hell|]#o", plain("hello", &Selection::single(0, 4))); + assert_eq!("#[|hello]#", plain("hello", &Selection::single(5, 0))); + assert_eq!("#[hello|]#", plain("hello", &Selection::single(0, 5))); + } + + #[test] + fn plain_multi() { + assert_eq!( + plain( + "hello", + &Selection::new( + SmallVec::from_slice(&[Range::new(1, 0), Range::new(5, 4)]), + 0 + ) + ), + String::from("#[|h]#ell#(|o)#") + ); + assert_eq!( + plain( + "hello", + &Selection::new( + SmallVec::from_slice(&[Range::new(0, 1), Range::new(4, 5)]), + 0 + ) + ), + String::from("#[h|]#ell#(o|)#") + ); + assert_eq!( + plain( + "hello", + &Selection::new( + SmallVec::from_slice(&[Range::new(2, 0), Range::new(5, 3)]), + 0 + ) + ), + String::from("#[|he]#l#(|lo)#") + ); + assert_eq!( + plain( + "hello\r\nhello\r\nhello\r\n", + &Selection::new( + SmallVec::from_slice(&[ + Range::new(7, 5), + Range::new(21, 19), + Range::new(14, 12) + ]), + 0 + ) + ), + String::from("hello#[|\r\n]#hello#(|\r\n)#hello#(|\r\n)#") + ); + } + + #[test] + fn plain_multi_byte_code_point() { + assert_eq!( + plain("β€žβ€œ", &Selection::single(1, 0)), + String::from("#[|β€ž]#β€œ") + ); + assert_eq!( + plain("β€žβ€œ", &Selection::single(2, 1)), + String::from("β€ž#[|β€œ]#") + ); + assert_eq!( + plain("β€žβ€œ", &Selection::single(0, 1)), + String::from("#[β€ž|]#β€œ") + ); + assert_eq!( + plain("β€žβ€œ", &Selection::single(1, 2)), + String::from("β€ž#[β€œ|]#") + ); + assert_eq!( + plain("they said β€žhelloβ€œ", &Selection::single(11, 10)), + String::from("they said #[|β€ž]#helloβ€œ") + ); + } + + #[test] + fn plain_multi_code_point_grapheme() { + assert_eq!( + plain("hello πŸ‘¨β€πŸ‘©β€πŸ‘§β€πŸ‘¦ goodbye", &Selection::single(13, 6)), + String::from("hello #[|πŸ‘¨β€πŸ‘©β€πŸ‘§β€πŸ‘¦]# goodbye") + ); + } } diff --git a/helix-core/src/textobject.rs b/helix-core/src/textobject.rs index 6e3f18cf0efd..bf00a4580491 100644 --- a/helix-core/src/textobject.rs +++ b/helix-core/src/textobject.rs @@ -437,7 +437,7 @@ mod test { let text = Rope::from(s.as_str()); let selection = selection .transform(|r| textobject_paragraph(text.slice(..), r, TextObject::Inside, 1)); - let actual = crate::test::plain(&s, &selection); + let actual = crate::test::plain(s.as_ref(), &selection); assert_eq!(actual, expected, "\nbefore: `{:?}`", before); } } @@ -460,7 +460,7 @@ mod test { let text = Rope::from(s.as_str()); let selection = selection .transform(|r| textobject_paragraph(text.slice(..), r, TextObject::Inside, 2)); - let actual = crate::test::plain(&s, &selection); + let actual = crate::test::plain(s.as_ref(), &selection); assert_eq!(actual, expected, "\nbefore: `{:?}`", before); } } @@ -491,7 +491,7 @@ mod test { let text = Rope::from(s.as_str()); let selection = selection .transform(|r| textobject_paragraph(text.slice(..), r, TextObject::Around, 1)); - let actual = crate::test::plain(&s, &selection); + let actual = crate::test::plain(s.as_ref(), &selection); assert_eq!(actual, expected, "\nbefore: `{:?}`", before); } } diff --git a/helix-term/tests/test/helpers.rs b/helix-term/tests/test/helpers.rs index de81f758dfea..77332fa581bb 100644 --- a/helix-term/tests/test/helpers.rs +++ b/helix-term/tests/test/helpers.rs @@ -65,7 +65,7 @@ pub async fn test_key_sequences( for (i, (in_keys, test_fn)) in inputs.into_iter().enumerate() { let (view, doc) = current_ref!(app.editor); - let state = test::plain(&doc.text().to_string(), doc.selection(view.id)); + let state = test::plain(doc.text().slice(..), doc.selection(view.id)); log::debug!("executing test with document state:\n\n-----\n\n{}", state); @@ -81,7 +81,7 @@ pub async fn test_key_sequences( if !app_exited { let (view, doc) = current_ref!(app.editor); - let state = test::plain(&doc.text().to_string(), doc.selection(view.id)); + let state = test::plain(doc.text().slice(..), doc.selection(view.id)); log::debug!( "finished running test with document state:\n\n-----\n\n{}", From 01c5382ad15d7509aa408e7b21fe08b678980206 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Thu, 10 Nov 2022 23:58:03 -0500 Subject: [PATCH 7/7] migrate test_with_config to use AppBuilder --- helix-term/tests/integration.rs | 4 ++-- helix-term/tests/test/auto_indent.rs | 7 +------ helix-term/tests/test/auto_pairs.rs | 14 ++++---------- helix-term/tests/test/helpers.rs | 17 ++++------------- helix-term/tests/test/movement.rs | 28 ++++------------------------ 5 files changed, 15 insertions(+), 55 deletions(-) diff --git a/helix-term/tests/integration.rs b/helix-term/tests/integration.rs index be1bfc2c0efb..cec374afa492 100644 --- a/helix-term/tests/integration.rs +++ b/helix-term/tests/integration.rs @@ -4,8 +4,8 @@ mod test { use std::path::PathBuf; - use helix_core::{syntax::AutoPairConfig, Position, Selection}; - use helix_term::{args::Args, config::Config}; + use helix_core::{syntax::AutoPairConfig, Selection}; + use helix_term::config::Config; use indoc::indoc; diff --git a/helix-term/tests/test/auto_indent.rs b/helix-term/tests/test/auto_indent.rs index e626acad3179..5132d44d983b 100644 --- a/helix-term/tests/test/auto_indent.rs +++ b/helix-term/tests/test/auto_indent.rs @@ -3,12 +3,7 @@ use super::*; #[tokio::test(flavor = "multi_thread")] async fn auto_indent_c() -> anyhow::Result<()> { test_with_config( - Args { - files: vec![(PathBuf::from("foo.c"), Position::default())], - ..Default::default() - }, - helpers::test_config(), - helpers::test_syntax_conf(None), + AppBuilder::new().with_file("foo.c", None), // switches to append mode? ( helpers::platform_line("void foo() {#[|}]#"), diff --git a/helix-term/tests/test/auto_pairs.rs b/helix-term/tests/test/auto_pairs.rs index e18c71195fb4..e10e0840bcb8 100644 --- a/helix-term/tests/test/auto_pairs.rs +++ b/helix-term/tests/test/auto_pairs.rs @@ -41,9 +41,7 @@ async fn insert_configured_multi_byte_chars() -> anyhow::Result<()> { for (open, close) in pairs.iter() { test_with_config( - Args::default(), - config.clone(), - helpers::test_syntax_conf(None), + AppBuilder::new().with_config(config.clone()), ( format!("#[{}|]#", LINE_END), format!("i{}", open), @@ -53,9 +51,7 @@ async fn insert_configured_multi_byte_chars() -> anyhow::Result<()> { .await?; test_with_config( - Args::default(), - config.clone(), - helpers::test_syntax_conf(None), + AppBuilder::new().with_config(config.clone()), ( format!("{}#[{}|]#{}", open, close, LINE_END), format!("i{}", close), @@ -170,15 +166,13 @@ async fn insert_before_eol() -> anyhow::Result<()> { async fn insert_auto_pairs_disabled() -> anyhow::Result<()> { for pair in DEFAULT_PAIRS { test_with_config( - Args::default(), - Config { + AppBuilder::new().with_config(Config { editor: helix_view::editor::Config { auto_pairs: AutoPairConfig::Enable(false), ..Default::default() }, ..Default::default() - }, - helpers::test_syntax_conf(None), + }), ( format!("#[{}|]#", LINE_END), format!("i{}", pair.0), diff --git a/helix-term/tests/test/helpers.rs b/helix-term/tests/test/helpers.rs index 77332fa581bb..ccd07bfa54c1 100644 --- a/helix-term/tests/test/helpers.rs +++ b/helix-term/tests/test/helpers.rs @@ -178,14 +178,11 @@ pub fn test_syntax_conf(overrides: Option) -> helix_core::syntax::Config /// document, selection, and sequence of key presses, and you just /// want to verify the resulting document and selection. pub async fn test_with_config>( - args: Args, - mut config: Config, - syn_conf: helix_core::syntax::Configuration, + app_builder: AppBuilder, test_case: T, ) -> anyhow::Result<()> { let test_case = test_case.into(); - config = helix_term::keymap::merge_keys(config); - let app = Application::new(args, config, syn_conf)?; + let app = app_builder.build()?; test_key_sequence_with_input_text( Some(app), @@ -206,13 +203,7 @@ pub async fn test_with_config>( } pub async fn test>(test_case: T) -> anyhow::Result<()> { - test_with_config( - Args::default(), - test_config(), - test_syntax_conf(None), - test_case, - ) - .await + test_with_config(AppBuilder::default(), test_case).await } pub fn temp_file_with_contents>( @@ -310,7 +301,7 @@ impl AppBuilder { // Remove this attribute once `with_config` is used in a test: #[allow(dead_code)] pub fn with_config(mut self, config: Config) -> Self { - self.config = config; + self.config = helix_term::keymap::merge_keys(config); self } diff --git a/helix-term/tests/test/movement.rs b/helix-term/tests/test/movement.rs index 6cc89021d046..e10ec6f5db3b 100644 --- a/helix-term/tests/test/movement.rs +++ b/helix-term/tests/test/movement.rs @@ -413,12 +413,7 @@ async fn cursor_position_append_eof() -> anyhow::Result<()> { #[tokio::test(flavor = "multi_thread")] async fn select_mode_tree_sitter_next_function_is_union_of_objects() -> anyhow::Result<()> { test_with_config( - Args { - files: vec![(PathBuf::from("foo.rs"), Position::default())], - ..Default::default() - }, - Config::default(), - helpers::test_syntax_conf(None), + AppBuilder::new().with_file("foo.rs", None), ( helpers::platform_line(indoc! {"\ #[/|]#// Increments @@ -443,12 +438,7 @@ async fn select_mode_tree_sitter_next_function_is_union_of_objects() -> anyhow:: #[tokio::test(flavor = "multi_thread")] async fn select_mode_tree_sitter_prev_function_unselects_object() -> anyhow::Result<()> { test_with_config( - Args { - files: vec![(PathBuf::from("foo.rs"), Position::default())], - ..Default::default() - }, - Config::default(), - helpers::test_syntax_conf(None), + AppBuilder::new().with_file("foo.rs", None), ( helpers::platform_line(indoc! {"\ /// Increments @@ -474,12 +464,7 @@ async fn select_mode_tree_sitter_prev_function_unselects_object() -> anyhow::Res async fn select_mode_tree_sitter_prev_function_goes_backwards_to_object() -> anyhow::Result<()> { // Note: the anchor stays put and the head moves back. test_with_config( - Args { - files: vec![(PathBuf::from("foo.rs"), Position::default())], - ..Default::default() - }, - Config::default(), - helpers::test_syntax_conf(None), + AppBuilder::new().with_file("foo.rs", None), ( helpers::platform_line(indoc! {"\ /// Increments @@ -503,12 +488,7 @@ async fn select_mode_tree_sitter_prev_function_goes_backwards_to_object() -> any .await?; test_with_config( - Args { - files: vec![(PathBuf::from("foo.rs"), Position::default())], - ..Default::default() - }, - Config::default(), - helpers::test_syntax_conf(None), + AppBuilder::new().with_file("foo.rs", None), ( helpers::platform_line(indoc! {"\ /// Increments