From 502d3290fb88d8a871b0824adc7987a98104933d Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Mon, 17 Jan 2022 19:04:40 -0500 Subject: improve test harness * Use new macro syntax for encoding sequences of keys * Make convenience helpers for common test pattern * Use indoc for inline indented raw strings * Add feature flag for integration testing to disable rendering --- helix-view/src/editor.rs | 1 + 1 file changed, 1 insertion(+) (limited to 'helix-view') diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index ac19def1..76ac0b51 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -769,6 +769,7 @@ impl Editor { Ok(self.new_file_from_document(action, Document::from(rope, Some(encoding)))) } + // ??? pub fn open(&mut self, path: PathBuf, action: Action) -> Result { let path = helix_core::path::get_canonicalized_path(&path)?; let id = self.document_by_path(&path).map(|doc| doc.id); -- cgit v1.2.3-70-g09d2 From 0f3c10a021bbe79e20bde1f55b87465edeec476d Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Wed, 16 Mar 2022 23:34:21 -0400 Subject: Fix initial selection of Document in new view When a new View of a Document is created, a default cursor of 0, 0 is created, and it does not get normalized to a single width cursor until at least one movement of the cursor happens. This appears to have no practical negative effect that I could find, but it makes tests difficult to work with, since the initial selection is not what you expect it to be. This changes the initial selection of a new View to be the width of the first grapheme in the text. --- .gitignore | 1 - helix-core/src/auto_pairs.rs | 14 +---- helix-core/src/selection.rs | 10 +++- helix-loader/Cargo.toml | 1 - helix-loader/src/lib.rs | 4 +- helix-term/src/application.rs | 21 ++++++++ helix-term/src/commands.rs | 17 ++++-- helix-term/tests/integration.rs | 112 ++++++++++++++++++++++++++++++---------- helix-view/src/document.rs | 34 +++++++++++- helix-view/src/editor.rs | 24 +++------ 10 files changed, 173 insertions(+), 65 deletions(-) (limited to 'helix-view') diff --git a/.gitignore b/.gitignore index 346d0946..6a6fc782 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,5 @@ target .direnv helix-term/rustfmt.toml -helix-syntax/languages/ result runtime/grammars diff --git a/helix-core/src/auto_pairs.rs b/helix-core/src/auto_pairs.rs index bcd47356..1131178e 100644 --- a/helix-core/src/auto_pairs.rs +++ b/helix-core/src/auto_pairs.rs @@ -1,9 +1,7 @@ //! When typing the opening character of one of the possible pairs defined below, //! this module provides the functionality to insert the paired closing character. -use crate::{ - graphemes, movement::Direction, Range, Rope, RopeGraphemes, Selection, Tendril, Transaction, -}; +use crate::{graphemes, movement::Direction, Range, Rope, Selection, Tendril, Transaction}; use std::collections::HashMap; use log::debug; @@ -149,14 +147,6 @@ fn prev_char(doc: &Rope, pos: usize) -> Option { doc.get_char(pos - 1) } -fn is_single_grapheme(doc: &Rope, range: &Range) -> bool { - let mut graphemes = RopeGraphemes::new(doc.slice(range.from()..range.to())); - let first = graphemes.next(); - let second = graphemes.next(); - debug!("first: {:#?}, second: {:#?}", first, second); - first.is_some() && second.is_none() -} - /// calculate what the resulting range should be for an auto pair insertion fn get_next_range( doc: &Rope, @@ -189,8 +179,8 @@ fn get_next_range( ); } - let single_grapheme = is_single_grapheme(doc, start_range); let doc_slice = doc.slice(..); + let single_grapheme = start_range.is_single_grapheme(doc_slice); // just skip over graphemes if len_inserted == 0 { diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs index 1b2416f5..83bab5e3 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -8,7 +8,7 @@ use crate::{ prev_grapheme_boundary, }, movement::Direction, - Assoc, ChangeSet, RopeSlice, + Assoc, ChangeSet, RopeGraphemes, RopeSlice, }; use smallvec::{smallvec, SmallVec}; use std::borrow::Cow; @@ -339,6 +339,14 @@ impl Range { pub fn cursor_line(&self, text: RopeSlice) -> usize { text.char_to_line(self.cursor(text)) } + + /// Returns true if this Range covers a single grapheme in the given text + pub fn is_single_grapheme(&self, doc: RopeSlice) -> bool { + let mut graphemes = RopeGraphemes::new(doc.slice(self.from()..self.to())); + let first = graphemes.next(); + let second = graphemes.next(); + first.is_some() && second.is_none() + } } impl From<(usize, usize)> for Range { diff --git a/helix-loader/Cargo.toml b/helix-loader/Cargo.toml index 20384472..3d8a697c 100644 --- a/helix-loader/Cargo.toml +++ b/helix-loader/Cargo.toml @@ -20,7 +20,6 @@ toml = "0.5" etcetera = "0.4" tree-sitter = "0.20" once_cell = "1.12" - log = "0.4" # TODO: these two should be on !wasm32 only diff --git a/helix-loader/src/lib.rs b/helix-loader/src/lib.rs index 595ac7aa..ff4414b2 100644 --- a/helix-loader/src/lib.rs +++ b/helix-loader/src/lib.rs @@ -13,7 +13,9 @@ pub fn runtime_dir() -> std::path::PathBuf { if let Ok(dir) = std::env::var("CARGO_MANIFEST_DIR") { // this is the directory of the crate being run by cargo, we need the workspace path so we take the parent - return std::path::PathBuf::from(dir).parent().unwrap().join(RT_DIR); + let path = std::path::PathBuf::from(dir).parent().unwrap().join(RT_DIR); + log::debug!("runtime dir: {}", path.to_string_lossy()); + return path; } const RT_DIR: &str = "runtime"; diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 21595eae..146194bf 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -55,8 +55,29 @@ pub struct Application { lsp_progress: LspProgressMap, } +#[cfg(feature = "integration")] +fn setup_integration_logging() { + // Separate file config so we can include year, month and day in file logs + let _ = fern::Dispatch::new() + .format(|out, message, record| { + out.finish(format_args!( + "{} {} [{}] {}", + chrono::Local::now().format("%Y-%m-%dT%H:%M:%S%.3f"), + record.target(), + record.level(), + message + )) + }) + .level(log::LevelFilter::Debug) + .chain(std::io::stdout()) + .apply(); +} + impl Application { pub fn new(args: Args, config: Config) -> Result { + #[cfg(feature = "integration")] + setup_integration_logging(); + use helix_view::editor::Action; let config_dir = helix_loader::config_dir(); diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index c9c8e6a9..6b01cbe3 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -2094,10 +2094,17 @@ fn insert_mode(cx: &mut Context) { let (view, doc) = current!(cx.editor); enter_insert_mode(doc); - let selection = doc - .selection(view.id) - .clone() - .transform(|range| Range::new(range.to(), range.from())); + log::trace!( + "entering insert mode with sel: {:?}, text: {:?}", + doc.selection(view.id), + doc.text().to_string() + ); + + let selection = doc.selection(view.id).clone().transform(|range| { + let new_range = Range::new(range.to(), range.from()); + new_range + }); + doc.set_selection(view.id, selection); } @@ -2444,8 +2451,8 @@ fn normal_mode(cx: &mut Context) { graphemes::prev_grapheme_boundary(text, range.to()), ) }); - doc.set_selection(view.id, selection); + doc.set_selection(view.id, selection); doc.restore_cursor = false; } } diff --git a/helix-term/tests/integration.rs b/helix-term/tests/integration.rs index 31a0d218..58883d40 100644 --- a/helix-term/tests/integration.rs +++ b/helix-term/tests/integration.rs @@ -2,9 +2,9 @@ mod integration { use std::path::PathBuf; - use helix_core::{syntax::AutoPairConfig, Position, Selection, Tendril, Transaction}; + use helix_core::{syntax::AutoPairConfig, Position, Selection, Transaction}; use helix_term::{application::Application, args::Args, config::Config}; - use helix_view::{current, doc, input::parse_macro}; + use helix_view::{doc, input::parse_macro}; use crossterm::event::{Event, KeyEvent}; use indoc::indoc; @@ -25,14 +25,14 @@ mod integration { let mut app = app.unwrap_or_else(|| Application::new(Args::default(), Config::default()).unwrap()); - let (view, doc) = current!(app.editor); + let (view, doc) = helix_view::current!(app.editor); + let sel = doc.selection(view.id).clone(); + // replace the initial text with the input text doc.apply( - &Transaction::insert( - doc.text(), - &Selection::single(1, 0), - Tendril::from(&test_case.in_text), - ) + &Transaction::change_by_selection(&doc.text(), &sel, |_| { + (0, doc.text().len_chars(), Some((&test_case.in_text).into())) + }) .with_selection(test_case.in_selection.clone()), view.id, ); @@ -80,12 +80,12 @@ mod integration { Args::default(), Config::default(), TestCase { - in_text: String::new(), + in_text: "\n".into(), in_selection: Selection::single(0, 1), // TODO: fix incorrect selection on new doc - in_keys: String::from("ihello worldhl"), - out_text: String::from("hello world\n"), - out_selection: Selection::single(11, 12), + in_keys: "ihello world".into(), + out_text: "hello world\n".into(), + out_selection: Selection::single(12, 11), }, )?; @@ -93,16 +93,74 @@ mod integration { } #[tokio::test] - async fn auto_pairs_basic() -> anyhow::Result<()> { + async fn insert_mode_cursor_position() -> anyhow::Result<()> { test_key_sequence_text_result( Args::default(), Config::default(), TestCase { in_text: String::new(), + in_selection: Selection::single(0, 0), + in_keys: "i".into(), + out_text: String::new(), + out_selection: Selection::single(0, 0), + }, + )?; + + test_key_sequence_text_result( + Args::default(), + Config::default(), + TestCase { + in_text: "\n".into(), + in_selection: Selection::single(0, 1), + in_keys: "i".into(), + out_text: "\n".into(), + out_selection: Selection::single(1, 0), + }, + )?; + + test_key_sequence_text_result( + Args::default(), + Config::default(), + TestCase { + in_text: "\n".into(), in_selection: Selection::single(0, 1), - in_keys: String::from("i(hl"), - out_text: String::from("()\n"), - out_selection: Selection::single(1, 2), + in_keys: "ii".into(), + out_text: "\n".into(), + out_selection: Selection::single(1, 0), + }, + )?; + + Ok(()) + } + + #[tokio::test] + async fn insert_to_normal_mode_cursor_position() -> anyhow::Result<()> { + test_key_sequence_text_result( + Args::default(), + Config::default(), + TestCase { + in_text: "\n".into(), + in_selection: Selection::single(0, 1), + in_keys: "i".into(), + out_text: "\n".into(), + out_selection: Selection::single(1, 0), + }, + )?; + + Ok(()) + } + + #[tokio::test] + async fn auto_pairs_basic() -> anyhow::Result<()> { + test_key_sequence_text_result( + Args::default(), + Config::default(), + TestCase { + in_text: "\n".into(), + in_selection: Selection::single(0, 1), + in_keys: "i(".into(), + out_text: "()\n".into(), + out_selection: Selection::single(2, 1), }, )?; @@ -116,11 +174,11 @@ mod integration { ..Default::default() }, TestCase { - in_text: String::new(), + in_text: "\n".into(), in_selection: Selection::single(0, 1), - in_keys: String::from("i(hl"), - out_text: String::from("(\n"), - out_selection: Selection::single(1, 2), + in_keys: "i(".into(), + out_text: "(\n".into(), + out_selection: Selection::single(2, 1), }, )?; @@ -136,15 +194,17 @@ mod integration { }, Config::default(), TestCase { - in_text: String::from("void foo() {}"), - in_selection: Selection::single(12, 13), - in_keys: String::from("i"), - out_text: String::from(indoc! {r#" + in_text: "void foo() {}\n".into(), + in_selection: Selection::single(13, 12), + in_keys: "i".into(), + out_text: indoc! {r#" void foo() { } - "#}), - out_selection: Selection::single(15, 16), + "#} + .trim_start() + .into(), + out_selection: Selection::single(16, 15), }, )?; diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index a2d2af77..00adaa1a 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -1,5 +1,6 @@ use anyhow::{anyhow, bail, Context, Error}; use helix_core::auto_pairs::AutoPairs; +use helix_core::Range; use serde::de::{self, Deserialize, Deserializer}; use serde::Serialize; use std::cell::Cell; @@ -83,7 +84,7 @@ impl Serialize for Mode { pub struct Document { pub(crate) id: DocumentId, text: Rope, - pub(crate) selections: HashMap, + selections: HashMap, path: Option, encoding: &'static encoding::Encoding, @@ -637,6 +638,37 @@ impl Document { .insert(view_id, selection.ensure_invariants(self.text().slice(..))); } + /// Find the origin selection of the text in a document, i.e. where + /// a single cursor would go if it were on the first grapheme. If + /// the text is empty, returns (0, 0). + pub fn origin(&self) -> Range { + if self.text().len_chars() == 0 { + return Range::new(0, 0); + } + + Range::new(0, 1).grapheme_aligned(self.text().slice(..)) + } + + /// Reset the view's selection on this document to the + /// [origin](Document::origin) cursor. + pub fn reset_selection(&mut self, view_id: ViewId) { + let origin = self.origin(); + self.set_selection(view_id, Selection::single(origin.anchor, origin.head)); + } + + /// Initializes a new selection for the given view if it does not + /// already have one. + pub fn ensure_view_init(&mut self, view_id: ViewId) { + if self.selections.get(&view_id).is_none() { + self.reset_selection(view_id); + } + } + + /// Remove a view's selection from this document. + pub fn remove_view(&mut self, view_id: ViewId) { + self.selections.remove(&view_id); + } + /// Apply a [`Transaction`] to the [`Document`] to change its text. fn apply_impl(&mut self, transaction: &Transaction, view_id: ViewId) -> bool { let old_doc = self.text().clone(); diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 76ac0b51..8607c65a 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -32,12 +32,12 @@ use anyhow::{bail, Error}; pub use helix_core::diagnostic::Severity; pub use helix_core::register::Registers; +use helix_core::Position; use helix_core::{ auto_pairs::AutoPairs, syntax::{self, AutoPairConfig}, Change, }; -use helix_core::{Position, Selection}; use helix_dap as dap; use serde::{ser::SerializeMap, Deserialize, Deserializer, Serialize, Serializer}; @@ -645,11 +645,8 @@ impl Editor { view.offset = Position::default(); let doc = self.documents.get_mut(&doc_id).unwrap(); + doc.ensure_view_init(view.id); - // initialize selection for view - doc.selections - .entry(view.id) - .or_insert_with(|| Selection::point(0)); // TODO: reuse align_view let pos = doc .selection(view.id) @@ -719,9 +716,7 @@ impl Editor { Action::Load => { let view_id = view!(self).id; let doc = self.documents.get_mut(&id).unwrap(); - if doc.selections().is_empty() { - doc.set_selection(view_id, Selection::point(0)); - } + doc.ensure_view_init(view_id); return; } Action::HorizontalSplit | Action::VerticalSplit => { @@ -736,7 +731,7 @@ impl Editor { ); // initialize selection for view let doc = self.documents.get_mut(&id).unwrap(); - doc.set_selection(view_id, Selection::point(0)); + doc.ensure_view_init(view_id); } } @@ -769,7 +764,7 @@ impl Editor { Ok(self.new_file_from_document(action, Document::from(rope, Some(encoding)))) } - // ??? + // ??? possible use for integration tests pub fn open(&mut self, path: PathBuf, action: Action) -> Result { let path = helix_core::path::get_canonicalized_path(&path)?; let id = self.document_by_path(&path).map(|doc| doc.id); @@ -791,12 +786,7 @@ impl Editor { pub fn close(&mut self, id: ViewId) { let view = self.tree.get(self.tree.focus); // remove selection - self.documents - .get_mut(&view.doc) - .unwrap() - .selections - .remove(&id); - + self.documents.get_mut(&view.doc).unwrap().remove_view(id); self.tree.remove(id); self._refresh(); } @@ -871,7 +861,7 @@ impl Editor { let view = View::new(doc_id, self.config().gutters.clone()); let view_id = self.tree.insert(view); let doc = self.documents.get_mut(&doc_id).unwrap(); - doc.set_selection(view_id, Selection::point(0)); + doc.ensure_view_init(view_id); } self._refresh(); -- cgit v1.2.3-70-g09d2 From 07fc80aece221233b4a986b0c5a03e2056cc1307 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sat, 30 Apr 2022 20:44:54 -0400 Subject: tests for serialized writes --- helix-term/tests/integration.rs | 2 + helix-term/tests/integration/auto_indent.rs | 1 + helix-term/tests/integration/auto_pairs.rs | 2 + helix-term/tests/integration/commands.rs | 25 +++++++++ helix-term/tests/integration/helpers.rs | 69 ++++++++++++++++--------- helix-term/tests/integration/movement.rs | 8 +++ helix-term/tests/integration/write.rs | 79 ++++++++++++++++++++++------- helix-view/src/editor.rs | 5 ++ 8 files changed, 147 insertions(+), 44 deletions(-) create mode 100644 helix-term/tests/integration/commands.rs (limited to 'helix-view') diff --git a/helix-term/tests/integration.rs b/helix-term/tests/integration.rs index b2b78e63..54364e12 100644 --- a/helix-term/tests/integration.rs +++ b/helix-term/tests/integration.rs @@ -17,6 +17,7 @@ mod integration { Args::default(), Config::default(), ("#[\n|]#", "ihello world", "hello world#[|\n]#"), + None, ) .await?; @@ -25,6 +26,7 @@ mod integration { mod auto_indent; mod auto_pairs; + mod commands; mod movement; mod write; } diff --git a/helix-term/tests/integration/auto_indent.rs b/helix-term/tests/integration/auto_indent.rs index 74d1ac58..fdfc7dfb 100644 --- a/helix-term/tests/integration/auto_indent.rs +++ b/helix-term/tests/integration/auto_indent.rs @@ -18,6 +18,7 @@ async fn auto_indent_c() -> anyhow::Result<()> { } "}, ), + None, ) .await?; diff --git a/helix-term/tests/integration/auto_pairs.rs b/helix-term/tests/integration/auto_pairs.rs index 52fee55e..d34cd0fd 100644 --- a/helix-term/tests/integration/auto_pairs.rs +++ b/helix-term/tests/integration/auto_pairs.rs @@ -6,6 +6,7 @@ async fn auto_pairs_basic() -> anyhow::Result<()> { Args::default(), Config::default(), ("#[\n|]#", "i(", "(#[|)]#\n"), + None, ) .await?; @@ -19,6 +20,7 @@ async fn auto_pairs_basic() -> anyhow::Result<()> { ..Default::default() }, ("#[\n|]#", "i(", "(#[|\n]#"), + None, ) .await?; diff --git a/helix-term/tests/integration/commands.rs b/helix-term/tests/integration/commands.rs new file mode 100644 index 00000000..ec60ac96 --- /dev/null +++ b/helix-term/tests/integration/commands.rs @@ -0,0 +1,25 @@ +use helix_core::diagnostic::Severity; +use helix_term::application::Application; + +use super::*; + +#[tokio::test] +async fn test_write_quit_fail() -> anyhow::Result<()> { + test_key_sequence( + &mut Application::new( + Args { + files: vec![(PathBuf::from("/foo"), Position::default())], + ..Default::default() + }, + Config::default(), + )?, + "ihello:wq", + Some(&|app| { + assert_eq!(&Severity::Error, app.editor.get_status().unwrap().1); + }), + None, + ) + .await?; + + Ok(()) +} diff --git a/helix-term/tests/integration/helpers.rs b/helix-term/tests/integration/helpers.rs index df662f07..60bfa331 100644 --- a/helix-term/tests/integration/helpers.rs +++ b/helix-term/tests/integration/helpers.rs @@ -5,7 +5,6 @@ use crossterm::event::{Event, KeyEvent}; use helix_core::{test, Selection, Transaction}; use helix_term::{application::Application, args::Args, config::Config}; use helix_view::{doc, input::parse_macro}; -use tokio::time::timeout; use tokio_stream::wrappers::UnboundedReceiverStream; #[derive(Clone, Debug)] @@ -32,35 +31,48 @@ impl> From<(S, S, S)> for TestCase { } } +#[inline] pub async fn test_key_sequence( app: &mut Application, in_keys: &str, test_fn: Option<&dyn Fn(&Application)>, + timeout: Option, ) -> anyhow::Result<()> { + test_key_sequences(app, vec![(in_keys, test_fn)], timeout).await +} + +pub async fn test_key_sequences( + app: &mut Application, + inputs: Vec<(&str, Option<&dyn Fn(&Application)>)>, + timeout: Option, +) -> anyhow::Result<()> { + let timeout = timeout.unwrap_or(Duration::from_millis(500)); let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); + let mut rx_stream = UnboundedReceiverStream::new(rx); - for key_event in parse_macro(&in_keys)?.into_iter() { - tx.send(Ok(Event::Key(KeyEvent::from(key_event))))?; - } + for (in_keys, test_fn) in inputs { + for key_event in parse_macro(&in_keys)?.into_iter() { + tx.send(Ok(Event::Key(KeyEvent::from(key_event))))?; + } - let mut rx_stream = UnboundedReceiverStream::new(rx); - let event_loop = app.event_loop(&mut rx_stream); - let result = timeout(Duration::from_millis(500), event_loop).await; + let event_loop = app.event_loop(&mut rx_stream); + let result = tokio::time::timeout(timeout, event_loop).await; - if result.is_ok() { - bail!("application exited before test function could run"); - } + if result.is_ok() { + bail!("application exited before test function could run"); + } - if let Some(test) = test_fn { - test(app); - }; + if let Some(test) = test_fn { + test(app); + }; + } for key_event in parse_macro(":q!")?.into_iter() { tx.send(Ok(Event::Key(KeyEvent::from(key_event))))?; } let event_loop = app.event_loop(&mut rx_stream); - timeout(Duration::from_millis(5000), event_loop).await?; + tokio::time::timeout(timeout, event_loop).await?; app.close().await?; Ok(()) @@ -70,6 +82,7 @@ pub async fn test_key_sequence_with_input_text>( app: Option, test_case: T, test_fn: &dyn Fn(&Application), + timeout: Option, ) -> anyhow::Result<()> { let test_case = test_case.into(); let mut app = @@ -87,7 +100,7 @@ pub async fn test_key_sequence_with_input_text>( view.id, ); - test_key_sequence(&mut app, &test_case.in_keys, Some(test_fn)).await + test_key_sequence(&mut app, &test_case.in_keys, Some(test_fn), timeout).await } /// Use this for very simple test cases where there is one input @@ -97,20 +110,26 @@ pub async fn test_key_sequence_text_result>( args: Args, config: Config, test_case: T, + timeout: Option, ) -> anyhow::Result<()> { let test_case = test_case.into(); let app = Application::new(args, config).unwrap(); - test_key_sequence_with_input_text(Some(app), test_case.clone(), &|app| { - let doc = doc!(app.editor); - assert_eq!(&test_case.out_text, doc.text()); - - let mut selections: Vec<_> = doc.selections().values().cloned().collect(); - assert_eq!(1, selections.len()); - - let sel = selections.pop().unwrap(); - assert_eq!(test_case.out_selection, sel); - }) + test_key_sequence_with_input_text( + Some(app), + test_case.clone(), + &|app| { + let doc = doc!(app.editor); + assert_eq!(&test_case.out_text, doc.text()); + + let mut selections: Vec<_> = doc.selections().values().cloned().collect(); + assert_eq!(1, selections.len()); + + let sel = selections.pop().unwrap(); + assert_eq!(test_case.out_selection, sel); + }, + timeout, + ) .await } diff --git a/helix-term/tests/integration/movement.rs b/helix-term/tests/integration/movement.rs index cac10852..a1d294c6 100644 --- a/helix-term/tests/integration/movement.rs +++ b/helix-term/tests/integration/movement.rs @@ -14,6 +14,7 @@ async fn insert_mode_cursor_position() -> anyhow::Result<()> { out_text: String::new(), out_selection: Selection::single(0, 0), }, + None, ) .await?; @@ -21,6 +22,7 @@ async fn insert_mode_cursor_position() -> anyhow::Result<()> { Args::default(), Config::default(), ("#[\n|]#", "i", "#[|\n]#"), + None, ) .await?; @@ -28,6 +30,7 @@ async fn insert_mode_cursor_position() -> anyhow::Result<()> { Args::default(), Config::default(), ("#[\n|]#", "i", "#[|\n]#"), + None, ) .await?; @@ -35,6 +38,7 @@ async fn insert_mode_cursor_position() -> anyhow::Result<()> { Args::default(), Config::default(), ("#[\n|]#", "ii", "#[|\n]#"), + None, ) .await?; @@ -48,6 +52,7 @@ async fn insert_to_normal_mode_cursor_position() -> anyhow::Result<()> { Args::default(), Config::default(), ("#[f|]#oo\n", "vll", "#[|foo]#\n"), + None, ) .await?; @@ -65,6 +70,7 @@ async fn insert_to_normal_mode_cursor_position() -> anyhow::Result<()> { #(|bar)#" }, ), + None, ) .await?; @@ -82,6 +88,7 @@ async fn insert_to_normal_mode_cursor_position() -> anyhow::Result<()> { #(ba|)#r" }, ), + None, ) .await?; @@ -99,6 +106,7 @@ async fn insert_to_normal_mode_cursor_position() -> anyhow::Result<()> { #(b|)#ar" }, ), + None, ) .await?; diff --git a/helix-term/tests/integration/write.rs b/helix-term/tests/integration/write.rs index 47e56288..27f97a45 100644 --- a/helix-term/tests/integration/write.rs +++ b/helix-term/tests/integration/write.rs @@ -1,9 +1,11 @@ use std::{ io::{Read, Write}, - ops::RangeInclusive, + time::Duration, }; +use helix_core::diagnostic::Severity; use helix_term::application::Application; +use helix_view::doc; use super::*; @@ -21,6 +23,7 @@ async fn test_write() -> anyhow::Result<()> { )?, "ii can eat glass, it will not hurt me:w", None, + Some(Duration::from_millis(1000)), ) .await?; @@ -35,35 +38,73 @@ async fn test_write() -> anyhow::Result<()> { } #[tokio::test] -async fn test_write_concurrent() -> anyhow::Result<()> { - let mut file = tempfile::NamedTempFile::new().unwrap(); - let mut command = String::new(); - const RANGE: RangeInclusive = 1..=1000; - - for i in RANGE { - let cmd = format!("%c{}:w", i); - command.push_str(&cmd); - } - - test_key_sequence( +async fn test_write_fail_mod_flag() -> anyhow::Result<()> { + test_key_sequences( &mut Application::new( Args { - files: vec![(file.path().to_path_buf(), Position::default())], + files: vec![(PathBuf::from("/foo"), Position::default())], ..Default::default() }, Config::default(), )?, - &command, + vec![ + ( + "", + Some(&|app| { + let doc = doc!(app.editor); + assert!(!doc.is_modified()); + }), + ), + ( + "ihello", + Some(&|app| { + let doc = doc!(app.editor); + assert!(doc.is_modified()); + }), + ), + ( + ":w", + Some(&|app| { + assert_eq!(&Severity::Error, app.editor.get_status().unwrap().1); + + let doc = doc!(app.editor); + assert!(doc.is_modified()); + }), + ), + ], None, ) .await?; - file.as_file_mut().flush()?; - file.as_file_mut().sync_all()?; + Ok(()) +} - let mut file_content = String::new(); - file.as_file_mut().read_to_string(&mut file_content)?; - assert_eq!(RANGE.end().to_string(), file_content); +#[tokio::test] +#[ignore] +async fn test_write_fail_new_path() -> anyhow::Result<()> { + test_key_sequences( + &mut Application::new(Args::default(), Config::default())?, + vec![ + ( + "", + Some(&|app| { + let doc = doc!(app.editor); + assert_eq!(None, app.editor.get_status()); + assert_eq!(None, doc.path()); + }), + ), + ( + ":w /foo", + Some(&|app| { + let doc = doc!(app.editor); + assert_eq!(&Severity::Error, app.editor.get_status().unwrap().1); + assert_eq!(None, doc.path()); + }), + ), + ], + Some(Duration::from_millis(1000)), + ) + .await?; Ok(()) } diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 8607c65a..d828f9ec 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -571,6 +571,11 @@ impl Editor { self.status_msg = Some((error.into(), Severity::Error)); } + #[inline] + pub fn get_status(&self) -> Option<(&Cow<'static, str>, &Severity)> { + self.status_msg.as_ref().map(|(status, sev)| (status, sev)) + } + pub fn set_theme(&mut self, theme: Theme) { // `ui.selection` is the only scope required to be able to render a theme. if theme.find_scope_index("ui.selection").is_none() { -- cgit v1.2.3-70-g09d2 From 40120967e9ba48d2e8b5fb4976a6ca1ce8993704 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sat, 30 Apr 2022 20:47:53 -0400 Subject: tests for buffer-close --- helix-term/tests/integration/commands.rs | 80 +++++++++++++++++++++++++++++++- helix-term/tests/integration/helpers.rs | 12 +++-- helix-term/tests/integration/write.rs | 48 ++++++++++++++++--- helix-view/src/editor.rs | 9 ++++ 4 files changed, 137 insertions(+), 12 deletions(-) (limited to 'helix-view') diff --git a/helix-term/tests/integration/commands.rs b/helix-term/tests/integration/commands.rs index ec60ac96..7da180b9 100644 --- a/helix-term/tests/integration/commands.rs +++ b/helix-term/tests/integration/commands.rs @@ -1,3 +1,9 @@ +use std::{ + io::{Read, Write}, + ops::RangeInclusive, + time::Duration, +}; + use helix_core::diagnostic::Severity; use helix_term::application::Application; @@ -13,7 +19,7 @@ async fn test_write_quit_fail() -> anyhow::Result<()> { }, Config::default(), )?, - "ihello:wq", + Some("ihello:wq"), Some(&|app| { assert_eq!(&Severity::Error, app.editor.get_status().unwrap().1); }), @@ -23,3 +29,75 @@ async fn test_write_quit_fail() -> anyhow::Result<()> { Ok(()) } + +#[tokio::test] +async fn test_buffer_close() -> anyhow::Result<()> { + test_key_sequences( + &mut Application::new(Args::default(), Config::default())?, + 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()); + }), + ), + ], + None, + ) + .await?; + + // verify if writes are queued up, it finishes them before closing the buffer + let mut file = tempfile::NamedTempFile::new().unwrap(); + let mut command = String::new(); + const RANGE: RangeInclusive = 1..=10; + + for i in RANGE { + let cmd = format!("%c{}:w", i); + command.push_str(&cmd); + } + + command.push_str(":bufferclose"); + + test_key_sequence( + &mut Application::new( + Args { + files: vec![(file.path().to_path_buf(), Position::default())], + ..Default::default() + }, + Config::default(), + )?, + 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); + }), + Some(Duration::from_millis(5000)), + ) + .await?; + + file.as_file_mut().flush()?; + file.as_file_mut().sync_all()?; + + let mut file_content = String::new(); + file.as_file_mut().read_to_string(&mut file_content)?; + assert_eq!(RANGE.end().to_string(), file_content); + + Ok(()) +} diff --git a/helix-term/tests/integration/helpers.rs b/helix-term/tests/integration/helpers.rs index 60bfa331..18a3517c 100644 --- a/helix-term/tests/integration/helpers.rs +++ b/helix-term/tests/integration/helpers.rs @@ -34,7 +34,7 @@ impl> From<(S, S, S)> for TestCase { #[inline] pub async fn test_key_sequence( app: &mut Application, - in_keys: &str, + in_keys: Option<&str>, test_fn: Option<&dyn Fn(&Application)>, timeout: Option, ) -> anyhow::Result<()> { @@ -43,7 +43,7 @@ pub async fn test_key_sequence( pub async fn test_key_sequences( app: &mut Application, - inputs: Vec<(&str, Option<&dyn Fn(&Application)>)>, + inputs: Vec<(Option<&str>, Option<&dyn Fn(&Application)>)>, timeout: Option, ) -> anyhow::Result<()> { let timeout = timeout.unwrap_or(Duration::from_millis(500)); @@ -51,8 +51,10 @@ pub async fn test_key_sequences( let mut rx_stream = UnboundedReceiverStream::new(rx); for (in_keys, test_fn) in inputs { - for key_event in parse_macro(&in_keys)?.into_iter() { - tx.send(Ok(Event::Key(KeyEvent::from(key_event))))?; + if let Some(in_keys) = in_keys { + for key_event in parse_macro(&in_keys)?.into_iter() { + tx.send(Ok(Event::Key(KeyEvent::from(key_event))))?; + } } let event_loop = app.event_loop(&mut rx_stream); @@ -100,7 +102,7 @@ pub async fn test_key_sequence_with_input_text>( view.id, ); - test_key_sequence(&mut app, &test_case.in_keys, Some(test_fn), timeout).await + test_key_sequence(&mut app, Some(&test_case.in_keys), Some(test_fn), timeout).await } /// Use this for very simple test cases where there is one input diff --git a/helix-term/tests/integration/write.rs b/helix-term/tests/integration/write.rs index 27f97a45..365e6b8d 100644 --- a/helix-term/tests/integration/write.rs +++ b/helix-term/tests/integration/write.rs @@ -1,5 +1,6 @@ use std::{ io::{Read, Write}, + ops::RangeInclusive, time::Duration, }; @@ -21,7 +22,7 @@ async fn test_write() -> anyhow::Result<()> { }, Config::default(), )?, - "ii can eat glass, it will not hurt me:w", + Some("ii can eat glass, it will not hurt me:w"), None, Some(Duration::from_millis(1000)), ) @@ -37,6 +38,41 @@ async fn test_write() -> anyhow::Result<()> { Ok(()) } +#[tokio::test] +async fn test_write_concurrent() -> anyhow::Result<()> { + let mut file = tempfile::NamedTempFile::new().unwrap(); + let mut command = String::new(); + const RANGE: RangeInclusive = 1..=5000; + + for i in RANGE { + let cmd = format!("%c{}:w", i); + command.push_str(&cmd); + } + + test_key_sequence( + &mut Application::new( + Args { + files: vec![(file.path().to_path_buf(), Position::default())], + ..Default::default() + }, + Config::default(), + )?, + Some(&command), + None, + Some(Duration::from_millis(10000)), + ) + .await?; + + file.as_file_mut().flush()?; + file.as_file_mut().sync_all()?; + + let mut file_content = String::new(); + file.as_file_mut().read_to_string(&mut file_content)?; + assert_eq!(RANGE.end().to_string(), file_content); + + Ok(()) +} + #[tokio::test] async fn test_write_fail_mod_flag() -> anyhow::Result<()> { test_key_sequences( @@ -49,21 +85,21 @@ async fn test_write_fail_mod_flag() -> anyhow::Result<()> { )?, vec![ ( - "", + None, Some(&|app| { let doc = doc!(app.editor); assert!(!doc.is_modified()); }), ), ( - "ihello", + Some("ihello"), Some(&|app| { let doc = doc!(app.editor); assert!(doc.is_modified()); }), ), ( - ":w", + Some(":w"), Some(&|app| { assert_eq!(&Severity::Error, app.editor.get_status().unwrap().1); @@ -86,7 +122,7 @@ async fn test_write_fail_new_path() -> anyhow::Result<()> { &mut Application::new(Args::default(), Config::default())?, vec![ ( - "", + None, Some(&|app| { let doc = doc!(app.editor); assert_eq!(None, app.editor.get_status()); @@ -94,7 +130,7 @@ async fn test_write_fail_new_path() -> anyhow::Result<()> { }), ), ( - ":w /foo", + Some(":w /foo"), Some(&|app| { let doc = doc!(app.editor); assert_eq!(&Severity::Error, app.editor.get_status().unwrap().1); diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index d828f9ec..e8603221 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -576,6 +576,15 @@ impl Editor { self.status_msg.as_ref().map(|(status, sev)| (status, sev)) } + /// Returns true if the current status is an error + #[inline] + pub fn is_err(&self) -> bool { + self.status_msg + .as_ref() + .map(|(_, sev)| *sev == Severity::Error) + .unwrap_or(false) + } + pub fn set_theme(&mut self, theme: Theme) { // `ui.selection` is the only scope required to be able to render a theme. if theme.find_scope_index("ui.selection").is_none() { -- cgit v1.2.3-70-g09d2 From ed950fcc56c480dc5a54c7e07918dca9192db200 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Tue, 26 Apr 2022 19:18:20 -0400 Subject: Add more context; Editor::open doesn't need to own path --- helix-term/src/application.rs | 17 ++++++++++------- helix-term/src/commands.rs | 4 ++-- helix-term/src/commands/lsp.rs | 6 +++--- helix-term/src/commands/typed.rs | 12 ++++++------ helix-term/src/compositor.rs | 6 +++--- helix-term/src/ui/mod.rs | 2 +- helix-view/src/editor.rs | 4 ++-- helix-view/src/handlers/dap.rs | 2 +- 8 files changed, 28 insertions(+), 25 deletions(-) (limited to 'helix-view') diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 3b96c45a..65cf4b2e 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -25,7 +25,7 @@ use std::{ time::{Duration, Instant}, }; -use anyhow::Error; +use anyhow::{Context, Error}; use crossterm::{ event::{DisableMouseCapture, EnableMouseCapture, Event}, @@ -122,7 +122,7 @@ impl Application { }); let syn_loader = std::sync::Arc::new(syntax::Loader::new(syn_loader_conf)); - let mut compositor = Compositor::new()?; + let mut compositor = Compositor::new().context("build compositor")?; let config = Arc::new(ArcSwap::from_pointee(config)); let mut editor = Editor::new( compositor.size(), @@ -141,26 +141,28 @@ impl Application { if args.load_tutor { let path = helix_loader::runtime_dir().join("tutor.txt"); - editor.open(path, Action::VerticalSplit)?; + editor.open(&path, Action::VerticalSplit)?; // Unset path to prevent accidentally saving to the original tutor file. doc_mut!(editor).set_path(None)?; } else if !args.files.is_empty() { let first = &args.files[0].0; // we know it's not empty if first.is_dir() { - std::env::set_current_dir(&first)?; + std::env::set_current_dir(&first).context("set current dir")?; editor.new_file(Action::VerticalSplit); let picker = ui::file_picker(".".into(), &config.load().editor); compositor.push(Box::new(overlayed(picker))); } else { let nr_of_files = args.files.len(); - editor.open(first.to_path_buf(), Action::VerticalSplit)?; + editor.open(first, Action::VerticalSplit)?; for (file, pos) in args.files { if file.is_dir() { return Err(anyhow::anyhow!( "expected a path to file, found a directory. (to open a directory pass it as first argument)" )); } else { - let doc_id = editor.open(file, Action::Load)?; + let doc_id = editor + .open(&file, Action::Load) + .context(format!("open '{}'", file.to_string_lossy()))?; // with Action::Load all documents have the same view let view_id = editor.tree.focus; let doc = editor.document_mut(doc_id).unwrap(); @@ -192,7 +194,8 @@ impl Application { #[cfg(windows)] let signals = futures_util::stream::empty(); #[cfg(not(windows))] - let signals = Signals::new(&[signal::SIGTSTP, signal::SIGCONT])?; + let signals = + Signals::new(&[signal::SIGTSTP, signal::SIGCONT]).context("build signal handler")?; let app = Self { compositor, diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 6b01cbe3..85dbfd56 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1024,7 +1024,7 @@ fn goto_file_impl(cx: &mut Context, action: Action) { for sel in paths { let p = sel.trim(); if !p.is_empty() { - if let Err(e) = cx.editor.open(PathBuf::from(p), action) { + if let Err(e) = cx.editor.open(&PathBuf::from(p), action) { cx.editor.set_error(format!("Open file failed: {:?}", e)); } } @@ -1849,7 +1849,7 @@ fn global_search(cx: &mut Context) { } }, move |cx, (line_num, path), action| { - match cx.editor.open(path.into(), action) { + match cx.editor.open(path, action) { Ok(_) => {} Err(e) => { cx.editor.set_error(format!( diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index b6bea8d6..ff61ee63 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -61,7 +61,7 @@ fn jump_to_location( return; } }; - let _id = editor.open(path, action).expect("editor.open failed"); + let _id = editor.open(&path, action).expect("editor.open failed"); let (view, doc) = current!(editor); let definition_pos = location.range.start; // TODO: convert inside server @@ -114,7 +114,7 @@ fn sym_picker( return; } }; - if let Err(err) = cx.editor.open(path, action) { + if let Err(err) = cx.editor.open(&path, action) { let err = format!("failed to open document: {}: {}", uri, err); log::error!("{}", err); cx.editor.set_error(err); @@ -385,7 +385,7 @@ pub fn apply_workspace_edit( }; let current_view_id = view!(editor).id; - let doc_id = match editor.open(path, Action::Load) { + let doc_id = match editor.open(&path, Action::Load) { Ok(doc_id) => doc_id, Err(err) => { let err = format!("failed to open document: {}: {}", uri, err); diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index ae3e63af..3c88b0ce 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -50,7 +50,7 @@ fn open( ensure!(!args.is_empty(), "wrong argument count"); for arg in args { let (path, pos) = args::parse_file(arg); - let _ = cx.editor.open(path, Action::Replace)?; + let _ = cx.editor.open(&path, Action::Replace)?; let (view, doc) = current!(cx.editor); let pos = Selection::point(pos_at_coords(doc.text().slice(..), pos, true)); doc.set_selection(view.id, pos); @@ -819,7 +819,7 @@ fn vsplit( } else { for arg in args { cx.editor - .open(PathBuf::from(arg.as_ref()), Action::VerticalSplit)?; + .open(&PathBuf::from(arg.as_ref()), Action::VerticalSplit)?; } } @@ -838,7 +838,7 @@ fn hsplit( } else { for arg in args { cx.editor - .open(PathBuf::from(arg.as_ref()), Action::HorizontalSplit)?; + .open(&PathBuf::from(arg.as_ref()), Action::HorizontalSplit)?; } } @@ -923,7 +923,7 @@ fn tutor( _event: PromptEvent, ) -> anyhow::Result<()> { let path = helix_loader::runtime_dir().join("tutor.txt"); - cx.editor.open(path, Action::Replace)?; + cx.editor.open(&path, Action::Replace)?; // Unset path to prevent accidentally saving to the original tutor file. doc_mut!(cx.editor).set_path(None)?; Ok(()) @@ -1150,7 +1150,7 @@ fn open_config( _event: PromptEvent, ) -> anyhow::Result<()> { cx.editor - .open(helix_loader::config_file(), Action::Replace)?; + .open(&helix_loader::config_file(), Action::Replace)?; Ok(()) } @@ -1159,7 +1159,7 @@ fn open_log( _args: &[Cow], _event: PromptEvent, ) -> anyhow::Result<()> { - cx.editor.open(helix_loader::log_file(), Action::Replace)?; + cx.editor.open(&helix_loader::log_file(), Action::Replace)?; Ok(()) } diff --git a/helix-term/src/compositor.rs b/helix-term/src/compositor.rs index e3cec643..1d421213 100644 --- a/helix-term/src/compositor.rs +++ b/helix-term/src/compositor.rs @@ -63,7 +63,7 @@ pub trait Component: Any + AnyComponent { } } -use anyhow::Error; +use anyhow::Context as AnyhowContext; use std::io::stdout; use tui::backend::{Backend, CrosstermBackend}; type Terminal = tui::terminal::Terminal>; @@ -76,9 +76,9 @@ pub struct Compositor { } impl Compositor { - pub fn new() -> Result { + pub fn new() -> anyhow::Result { let backend = CrosstermBackend::new(stdout()); - let terminal = Terminal::new(backend)?; + let terminal = Terminal::new(backend).context("build terminal")?; Ok(Self { layers: Vec::new(), terminal, diff --git a/helix-term/src/ui/mod.rs b/helix-term/src/ui/mod.rs index 23d0dca0..76ddaf89 100644 --- a/helix-term/src/ui/mod.rs +++ b/helix-term/src/ui/mod.rs @@ -175,7 +175,7 @@ pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> FilePi path.strip_prefix(&root).unwrap_or(path).to_string_lossy() }, move |cx, path: &PathBuf, action| { - if let Err(e) = cx.editor.open(path.into(), action) { + if let Err(e) = cx.editor.open(path, action) { let err = if let Some(err) = e.source() { format!("{}", err) } else { diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index e8603221..8ef4413e 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -779,8 +779,8 @@ impl Editor { } // ??? possible use for integration tests - pub fn open(&mut self, path: PathBuf, action: Action) -> Result { - let path = helix_core::path::get_canonicalized_path(&path)?; + pub fn open(&mut self, path: &Path, action: Action) -> Result { + let path = helix_core::path::get_canonicalized_path(path)?; let id = self.document_by_path(&path).map(|doc| doc.id); let id = if let Some(id) = id { diff --git a/helix-view/src/handlers/dap.rs b/helix-view/src/handlers/dap.rs index b17ca353..ae1ae64c 100644 --- a/helix-view/src/handlers/dap.rs +++ b/helix-view/src/handlers/dap.rs @@ -62,7 +62,7 @@ pub fn jump_to_stack_frame(editor: &mut Editor, frame: &helix_dap::StackFrame) { return; }; - if let Err(e) = editor.open(path, Action::Replace) { + if let Err(e) = editor.open(&path, Action::Replace) { editor.set_error(format!("Unable to jump to stack frame: {}", e)); return; } -- cgit v1.2.3-70-g09d2