From d706194597d462fbaeb1ef55e2e8fb6eae38d2f3 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sun, 10 Apr 2022 11:05:47 -0400 Subject: chore(write): serialize write operations within a Document The way that document writes are handled are by submitting them to the async job pool, which are all executed opportunistically out of order. It was discovered that this can lead to write inconsistencies when there are multiple writes to the same file in quick succession. This seeks to fix this problem by removing document writes from the general pool of jobs and into its own specialized event. Now when a user submits a write with one of the write commands, a request is simply queued up in a new mpsc channel that each Document makes to handle its own writes. This way, if multiple writes are submitted on the same document, they are executed in order, while still allowing concurrent writes for different documents. --- helix-term/tests/test/write.rs | 2 -- 1 file changed, 2 deletions(-) (limited to 'helix-term/tests/test') diff --git a/helix-term/tests/test/write.rs b/helix-term/tests/test/write.rs index 8869d881..4ac850c1 100644 --- a/helix-term/tests/test/write.rs +++ b/helix-term/tests/test/write.rs @@ -62,7 +62,6 @@ async fn test_write_quit() -> anyhow::Result<()> { } #[tokio::test] -#[ignore] async fn test_write_concurrent() -> anyhow::Result<()> { let mut file = tempfile::NamedTempFile::new()?; let mut command = String::new(); @@ -92,7 +91,6 @@ async fn test_write_concurrent() -> anyhow::Result<()> { } #[tokio::test] -#[ignore] async fn test_write_fail_mod_flag() -> anyhow::Result<()> { let file = helpers::new_readonly_tempfile()?; -- cgit v1.2.3-70-g09d2 From a5a93182cd5ccf88bc95b68044aa05d746ded35e Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sat, 23 Apr 2022 18:38:55 -0400 Subject: fix: buffer-close ensuring writes Make sure buffer-close waits for the document to finish its writes. --- helix-term/src/application.rs | 2 ++ helix-term/src/commands/typed.rs | 1 + helix-term/tests/test/commands.rs | 1 - helix-view/src/document.rs | 66 ++++++++++++++++++++++++++++++++------- helix-view/src/editor.rs | 5 +++ 5 files changed, 63 insertions(+), 12 deletions(-) (limited to 'helix-term/tests/test') diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index a9e25d08..2c1047da 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -310,6 +310,8 @@ impl Application { self.render(); } event = self.editor.wait_event() => { + log::debug!("received editor event: {:?}", event); + match event { EditorEvent::DocumentSave(event) => { self.handle_document_write(event); diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index d82dd7fe..375e7b4f 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -151,6 +151,7 @@ fn buffer_close( } let document_ids = buffer_gather_paths_impl(cx.editor, args); + log::debug!("closing buffers: {:?}", document_ids); buffer_close_by_ids_impl(cx.editor, &document_ids, false) } diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index f7ce9af0..8aea144b 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -26,7 +26,6 @@ async fn test_write_quit_fail() -> anyhow::Result<()> { } #[tokio::test] -#[ignore] async fn test_buffer_close_concurrent() -> anyhow::Result<()> { test_key_sequences( &mut Application::new(Args::default(), Config::default())?, diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index d6480b32..3045e3b7 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -616,6 +616,10 @@ impl Document { } pub async fn await_save(&mut self) -> Option { + self.await_save_impl(true).await + } + + async fn await_save_impl(&mut self, block: bool) -> Option { let mut current_save = self.current_save.lock().await; if let Some(ref mut save) = *current_save { let result = save.await; @@ -627,7 +631,15 @@ impl Document { // return early if the receiver is closed self.save_receiver.as_ref()?; - let save = match self.save_receiver.as_mut().unwrap().recv().await { + let rx = self.save_receiver.as_mut().unwrap(); + + let save_req = if block { + rx.recv().await + } else { + rx.try_recv().ok() + }; + + let save = match save_req { Some(save) => save, None => { self.save_receiver = None; @@ -648,19 +660,24 @@ impl Document { Some(result) } - /// Prepares the Document for being closed by stopping any new writes - /// and flushing through the queue of pending writes. If any fail, - /// it stops early before emptying the rest of the queue. Callers - /// should keep calling until it returns None. - pub async fn close(&mut self) -> Option { - if self.save_sender.is_some() { - self.save_sender = None; - } + /// Flushes the queue of pending writes. If any fail, + /// it stops early before emptying the rest of the queue. + pub async fn try_flush_saves(&mut self) -> Option { + self.flush_saves_impl(false).await + } + async fn flush_saves_impl(&mut self, block: bool) -> Option { let mut final_result = None; - while let Some(save_event) = self.await_save().await { - let is_err = save_event.is_err(); + while let Some(save_event) = self.await_save_impl(block).await { + let is_err = match &save_event { + Ok(event) => { + self.set_last_saved_revision(event.revision); + false + } + Err(_) => true, + }; + final_result = Some(save_event); if is_err { @@ -671,6 +688,17 @@ impl Document { final_result } + /// Prepares the Document for being closed by stopping any new writes + /// and flushing through the queue of pending writes. If any fail, + /// it stops early before emptying the rest of the queue. + pub async fn close(&mut self) -> Option { + if self.save_sender.is_some() { + self.save_sender = None; + } + + self.flush_saves_impl(true).await + } + /// Detect the programming language based on the file type. pub fn detect_language(&mut self, config_loader: Arc) { if let Some(path) = &self.path { @@ -1023,6 +1051,11 @@ impl Document { let history = self.history.take(); let current_revision = history.current_revision(); self.history.set(history); + log::debug!( + "modified - last saved: {}, current: {}", + self.last_saved_revision, + current_revision + ); current_revision != self.last_saved_revision || !self.changes.is_empty() } @@ -1036,9 +1069,20 @@ impl Document { /// Set the document's latest saved revision to the given one. pub fn set_last_saved_revision(&mut self, rev: usize) { + log::debug!( + "doc {} revision updated {} -> {}", + self.id, + self.last_saved_revision, + rev + ); self.last_saved_revision = rev; } + /// Get the document's latest saved revision. + pub fn get_last_saved_revision(&mut self) -> usize { + self.last_saved_revision + } + /// Get the current revision number pub fn get_current_revision(&mut self) -> usize { let history = self.history.take(); diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index ec6119a4..e038a82d 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1101,6 +1101,11 @@ impl Editor { None => return Err(CloseError::DoesNotExist), }; + // flush out any pending writes first to clear the modified status + if let Some(save_result) = doc.try_flush_saves().await { + save_result?; + } + if !force && doc.is_modified() { return Err(CloseError::BufferModified(doc.display_name().into_owned())); } -- cgit v1.2.3-70-g09d2 From 83b6042b97d13fca751e3d5d0c32f04e3ad04c9a Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sun, 24 Apr 2022 15:33:43 -0400 Subject: fix(write): do not set new path on document until write succeeds If a document is written with a new path, currently, in the event that the write fails, the document still gets its path changed. This fixes it so that the path is not updated unless the write succeeds. --- helix-term/src/application.rs | 26 +++++++++++------ helix-term/src/commands/typed.rs | 9 ++---- helix-term/tests/test/write.rs | 61 ++++++++++++++++++++++++++++++++++++++-- helix-view/src/document.rs | 52 ++++++++++++++++++++++++---------- 4 files changed, 116 insertions(+), 32 deletions(-) (limited to 'helix-term/tests/test') diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 2c1047da..0640de3c 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -3,6 +3,7 @@ use futures_util::Stream; use helix_core::{ config::{default_syntax_loader, user_syntax_loader}, diagnostic::{DiagnosticTag, NumberOrString}, + path::get_relative_path, pos_at_coords, syntax, Selection, }; use helix_lsp::{lsp, util::lsp_pos_to_pos, LspProgressMap}; @@ -489,17 +490,26 @@ impl Application { ); doc.set_last_saved_revision(doc_save_event.revision); + let lines = doc.text().len_lines(); let bytes = doc.text().len_bytes(); - let path_str = doc - .path() - .expect("document written without path") - .to_string_lossy() - .into_owned(); - - self.editor - .set_status(format!("'{}' written, {}L {}B", path_str, lines, bytes)); + if let Err(err) = doc.set_path(Some(&doc_save_event.path)) { + log::error!( + "error setting path for doc '{:?}': {}", + doc.path(), + err.to_string(), + ); + self.editor.set_error(err.to_string()); + } else { + // TODO: fix being overwritten by lsp + self.editor.set_status(format!( + "'{}' written, {}L {}B", + get_relative_path(&doc_save_event.path).to_string_lossy(), + lines, + bytes + )); + } } pub fn handle_terminal_events(&mut self, event: Result) { diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 375e7b4f..35c84601 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -268,11 +268,6 @@ fn write_impl( let jobs = &mut cx.jobs; let doc = doc_mut!(cx.editor); - if let Some(ref path) = path { - doc.set_path(Some(path.as_ref().as_ref())) - .context("invalid filepath")?; - } - if doc.path().is_none() { bail!("cannot write a buffer without a filename"); } @@ -292,7 +287,7 @@ fn write_impl( None }; - doc.format_and_save(fmt, force)?; + doc.format_and_save(fmt, path.map(AsRef::as_ref), force)?; if path.is_some() { let id = doc.id(); @@ -607,7 +602,7 @@ fn write_all_impl( None }; - doc.format_and_save(fmt, force)?; + doc.format_and_save::<_, PathBuf>(fmt, None, force)?; } if quit { diff --git a/helix-term/tests/test/write.rs b/helix-term/tests/test/write.rs index 4ac850c1..d2e6922f 100644 --- a/helix-term/tests/test/write.rs +++ b/helix-term/tests/test/write.rs @@ -35,7 +35,7 @@ async fn test_write() -> anyhow::Result<()> { Ok(()) } -#[tokio::test] +#[tokio::test(flavor = "multi_thread")] async fn test_write_quit() -> anyhow::Result<()> { let mut file = tempfile::NamedTempFile::new()?; @@ -129,7 +129,64 @@ async fn test_write_fail_mod_flag() -> anyhow::Result<()> { } #[tokio::test] -#[ignore] +async fn test_write_new_path() -> anyhow::Result<()> { + let mut file1 = tempfile::NamedTempFile::new().unwrap(); + let mut file2 = tempfile::NamedTempFile::new().unwrap(); + + test_key_sequences( + &mut Application::new( + Args { + files: vec![(file1.path().to_path_buf(), Position::default())], + ..Default::default() + }, + Config::default(), + )?, + vec![ + ( + Some("ii can eat glass, it will not hurt me:w"), + Some(&|app| { + let doc = doc!(app.editor); + assert!(!app.editor.is_err()); + assert_eq!(file1.path(), doc.path().unwrap()); + }), + ), + ( + Some(&format!(":w {}", file2.path().to_string_lossy())), + Some(&|app| { + let doc = doc!(app.editor); + assert!(!app.editor.is_err()); + assert_eq!(file2.path(), doc.path().unwrap()); + assert!(app.editor.document_by_path(file1.path()).is_none()); + }), + ), + ], + false, + ) + .await?; + + file1.as_file_mut().flush()?; + file1.as_file_mut().sync_all()?; + file2.as_file_mut().flush()?; + file2.as_file_mut().sync_all()?; + + let mut file1_content = String::new(); + file1.as_file_mut().read_to_string(&mut file1_content)?; + assert_eq!( + helpers::platform_line("i can eat glass, it will not hurt me\n"), + file1_content + ); + + let mut file2_content = String::new(); + file2.as_file_mut().read_to_string(&mut file2_content)?; + assert_eq!( + helpers::platform_line("i can eat glass, it will not hurt me\n"), + file2_content + ); + + Ok(()) +} + +#[tokio::test] async fn test_write_fail_new_path() -> anyhow::Result<()> { let file = helpers::new_readonly_tempfile()?; diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 3045e3b7..82d526a9 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -91,6 +91,7 @@ impl Serialize for Mode { pub struct DocumentSaveEvent { pub revision: usize, pub doc_id: DocumentId, + pub path: PathBuf, } pub type DocumentSaveEventResult = Result; @@ -512,41 +513,61 @@ impl Document { Some(fut.boxed()) } - pub fn save(&mut self, force: bool) -> Result<(), anyhow::Error> { - self.save_impl::>(None, force) + pub fn save>( + &mut self, + path: Option

, + force: bool, + ) -> Result<(), anyhow::Error> { + self.save_impl::, _>(None, path, force) } - pub fn format_and_save( + pub fn format_and_save( &mut self, - formatting: Option< - impl Future> + 'static + Send, - >, + formatting: Option, + path: Option

, force: bool, - ) -> anyhow::Result<()> { - self.save_impl(formatting, force) + ) -> anyhow::Result<()> + where + F: Future> + 'static + Send, + P: Into, + { + self.save_impl(formatting, path, force) } - // TODO: impl Drop to handle ensuring writes when closed /// The `Document`'s text is encoded according to its encoding and written to the file located /// at its `path()`. /// /// If `formatting` is present, it supplies some changes that we apply to the text before saving. - fn save_impl> + 'static + Send>( + fn save_impl( &mut self, formatting: Option, + path: Option

, force: bool, - ) -> Result<(), anyhow::Error> { + ) -> Result<(), anyhow::Error> + where + F: Future> + 'static + Send, + P: Into, + { if self.save_sender.is_none() { bail!("saves are closed for this document!"); } // we clone and move text + path into the future so that we asynchronously save the current // state without blocking any further edits. - let mut text = self.text().clone(); - let path = self.path.clone().expect("Can't save with no path set!"); - let identifier = self.identifier(); + let path = match path { + Some(path) => helix_core::path::get_canonicalized_path(&path.into())?, + None => { + if self.path.is_none() { + bail!("Can't save with no path set!"); + } + + self.path.as_ref().unwrap().clone() + } + }; + + let identifier = self.identifier(); let language_server = self.language_server.clone(); // mark changes up to now as saved @@ -586,12 +607,13 @@ impl Document { } } - let mut file = File::create(path).await?; + let mut file = File::create(&path).await?; to_writer(&mut file, encoding, &text).await?; let event = DocumentSaveEvent { revision: current_rev, doc_id, + path, }; if let Some(language_server) = language_server { -- cgit v1.2.3-70-g09d2 From e1f7bdb1d2ef68c0de38e768080291901ff4662e Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Mon, 9 May 2022 23:08:12 -0400 Subject: fix buffer-close --- helix-term/src/application.rs | 1 + helix-term/src/commands/typed.rs | 6 +++--- helix-term/tests/test/commands.rs | 2 +- helix-term/tests/test/helpers.rs | 6 ++++-- helix-term/tests/test/write.rs | 2 +- helix-view/src/document.rs | 14 ++++++-------- helix-view/src/editor.rs | 4 ++-- 7 files changed, 18 insertions(+), 17 deletions(-) (limited to 'helix-term/tests/test') diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 0640de3c..5c25e8aa 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -344,6 +344,7 @@ impl Application { #[cfg(feature = "integration")] { + log::debug!("idle handled"); idle_handled = true; } } diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 35c84601..efe693b9 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -77,9 +77,9 @@ fn buffer_close_by_ids_impl( let (modified_ids, modified_names): (Vec<_>, Vec<_>) = doc_ids .iter() .filter_map(|&doc_id| { - if let Err(CloseError::BufferModified(name)) = + if let Err(CloseError::BufferModified(name)) = tokio::task::block_in_place(|| { helix_lsp::block_on(editor.close_document(doc_id, force)) - { + }) { Some((doc_id, name)) } else { None @@ -151,7 +151,6 @@ fn buffer_close( } let document_ids = buffer_gather_paths_impl(cx.editor, args); - log::debug!("closing buffers: {:?}", document_ids); buffer_close_by_ids_impl(cx.editor, &document_ids, false) } @@ -519,6 +518,7 @@ fn write_quit( } write_impl(cx, args.first(), false)?; + // TODO: change to use document close helix_lsp::block_on(cx.jobs.finish())?; quit(cx, &[], event) } diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index 8aea144b..1f1bd6a9 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -25,7 +25,7 @@ async fn test_write_quit_fail() -> anyhow::Result<()> { Ok(()) } -#[tokio::test] +#[tokio::test(flavor = "multi_thread")] async fn test_buffer_close_concurrent() -> anyhow::Result<()> { test_key_sequences( &mut Application::new(Args::default(), Config::default())?, diff --git a/helix-term/tests/test/helpers.rs b/helix-term/tests/test/helpers.rs index 8f2501e6..bbcc6632 100644 --- a/helix-term/tests/test/helpers.rs +++ b/helix-term/tests/test/helpers.rs @@ -56,7 +56,9 @@ pub async fn test_key_sequences( for (i, (in_keys, test_fn)) in inputs.into_iter().enumerate() { 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 key = Event::Key(KeyEvent::from(key_event)); + log::trace!("sending key: {:?}", key); + tx.send(Ok(key))?; } } @@ -70,7 +72,7 @@ pub async fn test_key_sequences( // verify if it exited on the last iteration if it should have and // the inverse if i == num_inputs - 1 && app_exited != should_exit { - bail!("expected app to exit: {} != {}", app_exited, should_exit); + bail!("expected app to exit: {} != {}", should_exit, app_exited); } if let Some(test) = test_fn { diff --git a/helix-term/tests/test/write.rs b/helix-term/tests/test/write.rs index d2e6922f..544f1ba1 100644 --- a/helix-term/tests/test/write.rs +++ b/helix-term/tests/test/write.rs @@ -61,7 +61,7 @@ async fn test_write_quit() -> anyhow::Result<()> { Ok(()) } -#[tokio::test] +#[tokio::test(flavor = "multi_thread")] async fn test_write_concurrent() -> anyhow::Result<()> { let mut file = tempfile::NamedTempFile::new()?; let mut command = String::new(); diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 82d526a9..b6e42065 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -3,7 +3,6 @@ use futures_util::future::BoxFuture; use futures_util::FutureExt; use helix_core::auto_pairs::AutoPairs; use helix_core::Range; -use log::debug; use serde::de::{self, Deserialize, Deserializer}; use serde::Serialize; use std::borrow::Cow; @@ -644,16 +643,15 @@ impl Document { async fn await_save_impl(&mut self, block: bool) -> Option { let mut current_save = self.current_save.lock().await; if let Some(ref mut save) = *current_save { + log::trace!("reawaiting save of '{:?}'", self.path()); let result = save.await; *current_save = None; - debug!("save of '{:?}' result: {:?}", self.path(), result); + log::trace!("reawait save of '{:?}' result: {:?}", self.path(), result); return Some(result); } // return early if the receiver is closed - self.save_receiver.as_ref()?; - - let rx = self.save_receiver.as_mut().unwrap(); + let rx = self.save_receiver.as_mut()?; let save_req = if block { rx.recv().await @@ -672,12 +670,12 @@ impl Document { // save a handle to the future so that when a poll on this // function gets cancelled, we don't lose it *current_save = Some(save); - debug!("awaiting save of '{:?}'", self.path()); + log::trace!("awaiting save of '{:?}'", self.path()); let result = (*current_save).as_mut().unwrap().await; *current_save = None; - debug!("save of '{:?}' result: {:?}", self.path(), result); + log::trace!("save of '{:?}' result: {:?}", self.path(), result); Some(result) } @@ -715,7 +713,7 @@ impl Document { /// it stops early before emptying the rest of the queue. pub async fn close(&mut self) -> Option { if self.save_sender.is_some() { - self.save_sender = None; + self.save_sender.take(); } self.flush_saves_impl(true).await diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index e038a82d..e54aa7fa 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1102,8 +1102,8 @@ impl Editor { }; // flush out any pending writes first to clear the modified status - if let Some(save_result) = doc.try_flush_saves().await { - save_result?; + if let Some(Err(err)) = doc.try_flush_saves().await { + return Err(CloseError::SaveError(err)); } if !force && doc.is_modified() { -- cgit v1.2.3-70-g09d2 From 69c9e44ef205a81c112dfb14d5f2e67e5ce9c300 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Tue, 10 May 2022 23:41:44 -0400 Subject: update write-quit to wait for saves --- helix-term/src/commands/typed.rs | 8 ++++++-- helix-term/tests/test/commands.rs | 7 ++++++- 2 files changed, 12 insertions(+), 3 deletions(-) (limited to 'helix-term/tests/test') diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index efe693b9..bc254146 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -518,8 +518,12 @@ fn write_quit( } write_impl(cx, args.first(), false)?; - // TODO: change to use document close - helix_lsp::block_on(cx.jobs.finish())?; + let doc = doc_mut!(cx.editor); + + tokio::task::block_in_place(|| helix_lsp::block_on(doc.try_flush_saves())) + .map(|result| result.map(|_| ())) + .unwrap_or(Ok(()))?; + quit(cx, &[], event) } diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index 1f1bd6a9..b7c0f7cc 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -8,7 +8,7 @@ use helix_term::application::Application; use super::*; -#[tokio::test] +#[tokio::test(flavor = "multi_thread")] async fn test_write_quit_fail() -> anyhow::Result<()> { let file = helpers::new_readonly_tempfile()?; @@ -16,6 +16,11 @@ async fn test_write_quit_fail() -> anyhow::Result<()> { &mut helpers::app_with_file(file.path())?, 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, -- cgit v1.2.3-70-g09d2 From 57de4e62519a59aece104867569c2b9ad044af54 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Wed, 24 Aug 2022 01:13:41 -0400 Subject: various fixes in write-all path --- helix-term/src/commands/typed.rs | 25 ++++++-- helix-term/src/compositor.rs | 12 ++-- helix-term/tests/integration.rs | 1 + helix-term/tests/test/commands.rs | 12 +--- helix-term/tests/test/helpers.rs | 28 ++++++++- helix-term/tests/test/splits.rs | 122 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 175 insertions(+), 25 deletions(-) create mode 100644 helix-term/tests/test/splits.rs (limited to 'helix-term/tests/test') diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index fa2ba5e6..7fd619d9 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -573,13 +573,20 @@ fn write_all_impl( return Ok(()); } - let mut errors = String::new(); + let mut errors: Option = None; let auto_format = cx.editor.config().auto_format; let jobs = &mut cx.jobs; + // save all documents for doc in &mut cx.editor.documents.values_mut() { if doc.path().is_none() { - errors.push_str("cannot write a buffer without a filename\n"); + errors = errors + .or_else(|| Some(String::new())) + .map(|mut errs: String| { + errs.push_str("cannot write a buffer without a filename\n"); + errs + }); + continue; } @@ -591,7 +598,7 @@ fn write_all_impl( doc.auto_format().map(|fmt| { let callback = make_format_callback(doc.id(), doc.version(), fmt, Some((None, force))); - jobs.callback(callback); + jobs.add(Job::with_callback(callback).wait_before_exiting()); }) } else { None @@ -603,12 +610,12 @@ fn write_all_impl( } if quit { + cx.block_try_flush_writes()?; + if !force { buffers_remaining_impl(cx.editor)?; } - cx.block_try_flush_writes()?; - // close all views let views: Vec<_> = cx.editor.tree.views().map(|(view, _)| view.id).collect(); for view_id in views { @@ -616,7 +623,13 @@ fn write_all_impl( } } - bail!(errors) + if let Some(errs) = errors { + if !force { + bail!(errs); + } + } + + Ok(()) } fn write_all( diff --git a/helix-term/src/compositor.rs b/helix-term/src/compositor.rs index 6ef77341..5077807d 100644 --- a/helix-term/src/compositor.rs +++ b/helix-term/src/compositor.rs @@ -29,17 +29,17 @@ pub struct Context<'a> { impl<'a> Context<'a> { /// Waits on all pending jobs, and then tries to flush all pending write - /// operations for the current document. + /// operations for all documents. pub fn block_try_flush_writes(&mut self) -> anyhow::Result<()> { tokio::task::block_in_place(|| { helix_lsp::block_on(self.jobs.finish(Some(self.editor), None)) })?; - let doc = doc_mut!(self.editor); - - tokio::task::block_in_place(|| helix_lsp::block_on(doc.try_flush_saves())) - .map(|result| result.map(|_| ())) - .unwrap_or(Ok(()))?; + for doc in &mut self.editor.documents.values_mut() { + tokio::task::block_in_place(|| helix_lsp::block_on(doc.try_flush_saves())) + .map(|result| result.map(|_| ())) + .unwrap_or(Ok(()))?; + } Ok(()) } diff --git a/helix-term/tests/integration.rs b/helix-term/tests/integration.rs index 8969e976..e3754c43 100644 --- a/helix-term/tests/integration.rs +++ b/helix-term/tests/integration.rs @@ -22,5 +22,6 @@ mod test { mod commands; 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 b7c0f7cc..0279e348 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -1,7 +1,4 @@ -use std::{ - io::{Read, Write}, - ops::RangeInclusive, -}; +use std::ops::RangeInclusive; use helix_core::diagnostic::Severity; use helix_term::application::Application; @@ -86,12 +83,7 @@ async fn test_buffer_close_concurrent() -> anyhow::Result<()> { ) .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); + helpers::assert_file_has_content(file.as_file_mut(), &RANGE.end().to_string())?; Ok(()) } diff --git a/helix-term/tests/test/helpers.rs b/helix-term/tests/test/helpers.rs index bbcc6632..ed1a0331 100644 --- a/helix-term/tests/test/helpers.rs +++ b/helix-term/tests/test/helpers.rs @@ -1,10 +1,15 @@ -use std::{io::Write, path::PathBuf, time::Duration}; +use std::{ + fs::File, + io::{Read, Write}, + path::PathBuf, + time::Duration, +}; use anyhow::bail; use crossterm::event::{Event, KeyEvent}; -use helix_core::{test, Selection, Transaction}; +use helix_core::{diagnostic::Severity, test, Selection, Transaction}; use helix_term::{application::Application, args::Args, config::Config}; -use helix_view::{doc, input::parse_macro}; +use helix_view::{doc, input::parse_macro, Editor}; use tempfile::NamedTempFile; use tokio_stream::wrappers::UnboundedReceiverStream; @@ -213,3 +218,20 @@ pub fn app_with_file>(path: P) -> anyhow::Result { Config::default(), ) } + +pub fn assert_file_has_content(file: &mut File, content: &str) -> anyhow::Result<()> { + file.flush()?; + file.sync_all()?; + + let mut file_content = String::new(); + file.read_to_string(&mut file_content)?; + assert_eq!(content, file_content); + + Ok(()) +} + +pub fn assert_status_not_error(editor: &Editor) { + if let Some((_, sev)) = editor.get_status() { + assert_ne!(&Severity::Error, sev); + } +} diff --git a/helix-term/tests/test/splits.rs b/helix-term/tests/test/splits.rs new file mode 100644 index 00000000..70a517be --- /dev/null +++ b/helix-term/tests/test/splits.rs @@ -0,0 +1,122 @@ +use super::*; + +#[tokio::test(flavor = "multi_thread")] +async fn test_split_write_quit_all() -> anyhow::Result<()> { + let mut file1 = tempfile::NamedTempFile::new()?; + let mut file2 = tempfile::NamedTempFile::new()?; + let mut file3 = tempfile::NamedTempFile::new()?; + + test_key_sequences( + &mut helpers::app_with_file(file1.path())?, + vec![ + ( + Some(&format!( + "ihello1:sp:o {}ihello2:sp:o {}ihello3", + file2.path().to_string_lossy(), + file3.path().to_string_lossy() + )), + Some(&|app| { + let docs: Vec<_> = app.editor.documents().collect(); + assert_eq!(3, docs.len()); + + let doc1 = docs + .iter() + .find(|doc| doc.path().unwrap() == file1.path()) + .unwrap(); + + assert_eq!("hello1", doc1.text().to_string()); + + let doc2 = docs + .iter() + .find(|doc| doc.path().unwrap() == file2.path()) + .unwrap(); + + assert_eq!("hello2", doc2.text().to_string()); + + let doc3 = docs + .iter() + .find(|doc| doc.path().unwrap() == file3.path()) + .unwrap(); + + assert_eq!("hello3", doc3.text().to_string()); + + helpers::assert_status_not_error(&app.editor); + assert_eq!(3, app.editor.tree.views().count()); + }), + ), + ( + Some(":wqa"), + Some(&|app| { + helpers::assert_status_not_error(&app.editor); + assert_eq!(0, app.editor.tree.views().count()); + }), + ), + ], + true, + ) + .await?; + + helpers::assert_file_has_content(file1.as_file_mut(), "hello1")?; + helpers::assert_file_has_content(file2.as_file_mut(), "hello2")?; + helpers::assert_file_has_content(file3.as_file_mut(), "hello3")?; + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_split_write_quit_same_file() -> anyhow::Result<()> { + let mut file = tempfile::NamedTempFile::new()?; + + test_key_sequences( + &mut helpers::app_with_file(file.path())?, + vec![ + ( + Some("Oihello:spogoodbye"), + Some(&|app| { + assert_eq!(2, app.editor.tree.views().count()); + helpers::assert_status_not_error(&app.editor); + + let mut docs: Vec<_> = app.editor.documents().collect(); + assert_eq!(1, docs.len()); + + let doc = docs.pop().unwrap(); + + assert_eq!( + helpers::platform_line("hello\ngoodbye"), + doc.text().to_string() + ); + + assert!(doc.is_modified()); + }), + ), + ( + Some(":wq"), + Some(&|app| { + helpers::assert_status_not_error(&app.editor); + assert_eq!(1, app.editor.tree.views().count()); + + let mut docs: Vec<_> = app.editor.documents().collect(); + assert_eq!(1, docs.len()); + + let doc = docs.pop().unwrap(); + + assert_eq!( + helpers::platform_line("hello\ngoodbye"), + doc.text().to_string() + ); + + assert!(!doc.is_modified()); + }), + ), + ], + false, + ) + .await?; + + helpers::assert_file_has_content( + file.as_file_mut(), + &helpers::platform_line("hello\ngoodbye"), + )?; + + Ok(()) +} -- cgit v1.2.3-70-g09d2 From af03df3413f04ce7079d14388ce42fe70bd1397e Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Wed, 31 Aug 2022 15:08:00 -0400 Subject: fix write scratch buffer to file --- helix-term/src/commands/typed.rs | 4 --- helix-term/tests/test/write.rs | 71 ++++++++++++++++++++++++++++++---------- helix-view/src/document.rs | 18 +++++++--- 3 files changed, 67 insertions(+), 26 deletions(-) (limited to 'helix-term/tests/test') diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 7fd619d9..d312c45f 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -273,10 +273,6 @@ fn write_impl( let doc = doc_mut!(cx.editor); let path = path.map(AsRef::as_ref); - if doc.path().is_none() { - bail!("cannot write a buffer without a filename"); - } - let fmt = if editor_auto_fmt { doc.auto_format().map(|fmt| { let callback = make_format_callback( diff --git a/helix-term/tests/test/write.rs b/helix-term/tests/test/write.rs index 544f1ba1..7d105431 100644 --- a/helix-term/tests/test/write.rs +++ b/helix-term/tests/test/write.rs @@ -128,6 +128,52 @@ async fn test_write_fail_mod_flag() -> anyhow::Result<()> { Ok(()) } +#[tokio::test] +async fn test_write_scratch_to_new_path() -> anyhow::Result<()> { + let mut file = tempfile::NamedTempFile::new()?; + + test_key_sequence( + &mut Application::new(Args::default(), Config::default())?, + Some(format!("ihello:w {}", file.path().to_string_lossy()).as_ref()), + Some(&|app| { + assert!(!app.editor.is_err()); + + let mut docs: Vec<_> = app.editor.documents().collect(); + assert_eq!(1, docs.len()); + + let doc = docs.pop().unwrap(); + assert_eq!(Some(&file.path().to_path_buf()), doc.path()); + }), + false, + ) + .await?; + + helpers::assert_file_has_content(file.as_file_mut(), &helpers::platform_line("hello"))?; + + Ok(()) +} + +#[tokio::test] +async fn test_write_scratch_no_path_fails() -> anyhow::Result<()> { + helpers::test_key_sequence_with_input_text( + None, + ("#[\n|]#", "ihello:w", "hello#[\n|]#"), + &|app| { + assert!(app.editor.is_err()); + + let mut docs: Vec<_> = app.editor.documents().collect(); + assert_eq!(1, docs.len()); + + let doc = docs.pop().unwrap(); + assert_eq!(None, doc.path()); + }, + false, + ) + .await?; + + Ok(()) +} + #[tokio::test] async fn test_write_new_path() -> anyhow::Result<()> { let mut file1 = tempfile::NamedTempFile::new().unwrap(); @@ -164,24 +210,15 @@ async fn test_write_new_path() -> anyhow::Result<()> { ) .await?; - file1.as_file_mut().flush()?; - file1.as_file_mut().sync_all()?; - file2.as_file_mut().flush()?; - file2.as_file_mut().sync_all()?; + helpers::assert_file_has_content( + file1.as_file_mut(), + &helpers::platform_line("i can eat glass, it will not hurt me\n"), + )?; - let mut file1_content = String::new(); - file1.as_file_mut().read_to_string(&mut file1_content)?; - assert_eq!( - helpers::platform_line("i can eat glass, it will not hurt me\n"), - file1_content - ); - - let mut file2_content = String::new(); - file2.as_file_mut().read_to_string(&mut file2_content)?; - assert_eq!( - helpers::platform_line("i can eat glass, it will not hurt me\n"), - file2_content - ); + helpers::assert_file_has_content( + file2.as_file_mut(), + &helpers::platform_line("i can eat glass, it will not hurt me\n"), + )?; Ok(()) } diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 61bea527..ff64689e 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -574,7 +574,12 @@ impl Document { } }; - let identifier = self.identifier(); + let identifier = if self.path().is_some() { + Some(self.identifier()) + } else { + None + }; + let language_server = self.language_server.clone(); // mark changes up to now as saved @@ -628,10 +633,13 @@ impl Document { if !language_server.is_initialized() { return Ok(event); } - if let Some(notification) = - language_server.text_document_did_save(identifier, &text) - { - notification.await?; + + if let Some(identifier) = identifier { + if let Some(notification) = + language_server.text_document_did_save(identifier, &text) + { + notification.await?; + } } } -- cgit v1.2.3-70-g09d2 From 3f07885b351748c5b8225aadb165f8ef7066f047 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Fri, 16 Sep 2022 23:17:48 -0400 Subject: document should save even if formatter fails --- helix-core/src/syntax.rs | 6 +++ helix-term/src/application.rs | 15 ++---- helix-term/src/commands.rs | 27 ++++++----- helix-term/src/main.rs | 12 ++++- helix-term/tests/test/auto_indent.rs | 1 + helix-term/tests/test/auto_pairs.rs | 1 + helix-term/tests/test/commands.rs | 14 ++++-- helix-term/tests/test/helpers.rs | 90 ++++++++++++++++++++++++++++++------ helix-term/tests/test/movement.rs | 4 +- helix-term/tests/test/prompt.rs | 4 +- helix-term/tests/test/splits.rs | 11 ++++- helix-term/tests/test/write.rs | 66 ++++++++++++++++++-------- 12 files changed, 185 insertions(+), 66 deletions(-) (limited to 'helix-term/tests/test') diff --git a/helix-core/src/syntax.rs b/helix-core/src/syntax.rs index 61d382fd..f907629f 100644 --- a/helix-core/src/syntax.rs +++ b/helix-core/src/syntax.rs @@ -61,6 +61,12 @@ pub struct Configuration { pub language: Vec, } +impl Default for Configuration { + fn default() -> Self { + crate::config::default_syntax_loader() + } +} + // largely based on tree-sitter/cli/src/loader.rs #[derive(Debug, Serialize, Deserialize)] #[serde(rename_all = "kebab-case", deny_unknown_fields)] diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index fe53d73d..4fde2a66 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -1,7 +1,6 @@ use arc_swap::{access::Map, ArcSwap}; use futures_util::Stream; use helix_core::{ - config::{default_syntax_loader, user_syntax_loader}, diagnostic::{DiagnosticTag, NumberOrString}, path::get_relative_path, pos_at_coords, syntax, Selection, @@ -110,7 +109,11 @@ fn restore_term() -> Result<(), Error> { } impl Application { - pub fn new(args: Args, config: Config) -> Result { + pub fn new( + args: Args, + config: Config, + syn_loader_conf: syntax::Configuration, + ) -> Result { #[cfg(feature = "integration")] setup_integration_logging(); @@ -137,14 +140,6 @@ impl Application { }) .unwrap_or_else(|| theme_loader.default_theme(true_color)); - let syn_loader_conf = user_syntax_loader().unwrap_or_else(|err| { - eprintln!("Bad language config: {}", err); - eprintln!("Press to continue with default language config"); - use std::io::Read; - // This waits for an enter press. - let _ = std::io::stdin().read(&mut []); - default_syntax_loader() - }); let syn_loader = std::sync::Arc::new(syntax::Loader::new(syn_loader_conf)); let mut compositor = Compositor::new().context("build compositor")?; diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 6deecbe2..f6d583f5 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -2518,7 +2518,8 @@ async fn make_format_callback( format: impl Future> + Send + 'static, write: Option<(Option, bool)>, ) -> anyhow::Result { - let format = format.await?; + let format = format.await; + let call: job::Callback = Callback::Editor(Box::new(move |editor| { if !editor.documents.contains_key(&doc_id) { return; @@ -2528,19 +2529,21 @@ async fn make_format_callback( let doc = doc_mut!(editor, &doc_id); let view = view_mut!(editor); - if doc.version() == doc_version { - apply_transaction(&format, doc, view); - doc.append_changes_to_history(view.id); - doc.detect_indent_and_line_ending(); - view.ensure_cursor_in_view(doc, scrolloff); + if let Ok(format) = format { + if doc.version() == doc_version { + apply_transaction(&format, doc, view); + doc.append_changes_to_history(view.id); + doc.detect_indent_and_line_ending(); + view.ensure_cursor_in_view(doc, scrolloff); + } else { + log::info!("discarded formatting changes because the document changed"); + } + } - if let Some((path, force)) = write { - if let Err(err) = doc.save(path, force) { - editor.set_error(format!("Error saving: {}", err)); - } + if let Some((path, force)) = write { + if let Err(err) = doc.save(path, force) { + editor.set_error(format!("Error saving: {}", err)); } - } else { - log::info!("discarded formatting changes because the document changed"); } })); diff --git a/helix-term/src/main.rs b/helix-term/src/main.rs index 726bf9e3..96b695c6 100644 --- a/helix-term/src/main.rs +++ b/helix-term/src/main.rs @@ -139,8 +139,18 @@ FLAGS: Err(err) => return Err(Error::new(err)), }; + let syn_loader_conf = helix_core::config::user_syntax_loader().unwrap_or_else(|err| { + eprintln!("Bad language config: {}", err); + eprintln!("Press to continue with default language config"); + use std::io::Read; + // This waits for an enter press. + let _ = std::io::stdin().read(&mut []); + helix_core::config::default_syntax_loader() + }); + // TODO: use the thread local executor to spawn the application task separately from the work pool - let mut app = Application::new(args, config).context("unable to create new application")?; + let mut app = Application::new(args, config, syn_loader_conf) + .context("unable to create new application")?; let exit_code = app.run(&mut EventStream::new()).await?; diff --git a/helix-term/tests/test/auto_indent.rs b/helix-term/tests/test/auto_indent.rs index 2f638893..5c093a5d 100644 --- a/helix-term/tests/test/auto_indent.rs +++ b/helix-term/tests/test/auto_indent.rs @@ -8,6 +8,7 @@ async fn auto_indent_c() -> anyhow::Result<()> { ..Default::default() }, Config::default(), + helpers::test_syntax_conf(None), // switches to append mode? ( helpers::platform_line("void foo() {#[|}]#").as_ref(), diff --git a/helix-term/tests/test/auto_pairs.rs b/helix-term/tests/test/auto_pairs.rs index ec47a5b4..caf80bd4 100644 --- a/helix-term/tests/test/auto_pairs.rs +++ b/helix-term/tests/test/auto_pairs.rs @@ -13,6 +13,7 @@ async fn auto_pairs_basic() -> anyhow::Result<()> { }, ..Default::default() }, + helpers::test_syntax_conf(None), ("#[\n|]#", "i(", "(#[|\n]#"), ) .await?; diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index 0279e348..5238cc69 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -1,16 +1,18 @@ 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 helpers::app_with_file(file.path())?, + &mut app, Some("ihello:wq"), Some(&|app| { let mut docs: Vec<_> = app.editor.documents().collect(); @@ -30,7 +32,7 @@ async fn test_write_quit_fail() -> anyhow::Result<()> { #[tokio::test(flavor = "multi_thread")] async fn test_buffer_close_concurrent() -> anyhow::Result<()> { test_key_sequences( - &mut Application::new(Args::default(), Config::default())?, + &mut helpers::AppBuilder::new().build()?, vec![ ( None, @@ -70,8 +72,12 @@ async fn test_buffer_close_concurrent() -> anyhow::Result<()> { command.push_str(":bufferclose"); + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .build()?; + test_key_sequence( - &mut helpers::app_with_file(file.path())?, + &mut app, Some(&command), Some(&|app| { assert!(!app.editor.is_err(), "error: {:?}", app.editor.get_status()); diff --git a/helix-term/tests/test/helpers.rs b/helix-term/tests/test/helpers.rs index ed1a0331..c2fbe953 100644 --- a/helix-term/tests/test/helpers.rs +++ b/helix-term/tests/test/helpers.rs @@ -108,7 +108,7 @@ pub async fn test_key_sequence_with_input_text>( let test_case = test_case.into(); let mut app = match app { Some(app) => app, - None => Application::new(Args::default(), Config::default())?, + None => Application::new(Args::default(), Config::default(), test_syntax_conf(None))?, }; let (view, doc) = helix_view::current!(app.editor); @@ -132,16 +132,30 @@ pub async fn test_key_sequence_with_input_text>( .await } +/// Generates language configs that merge in overrides, like a user language +/// config. The argument string must be a raw TOML document. +pub fn test_syntax_conf(overrides: Option) -> helix_core::syntax::Configuration { + let mut lang = helix_loader::config::default_lang_config(); + + if let Some(overrides) = overrides { + let override_toml = toml::from_str(&overrides).unwrap(); + lang = helix_loader::merge_toml_values(lang, override_toml, 3); + } + + lang.try_into().unwrap() +} + /// Use this for very simple test cases where there is one input /// 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, config: Config, + syn_conf: helix_core::syntax::Configuration, test_case: T, ) -> anyhow::Result<()> { let test_case = test_case.into(); - let app = Application::new(args, config)?; + let app = Application::new(args, config, syn_conf)?; test_key_sequence_with_input_text( Some(app), @@ -162,7 +176,13 @@ pub async fn test_with_config>( } pub async fn test>(test_case: T) -> anyhow::Result<()> { - test_with_config(Args::default(), Config::default(), test_case).await + test_with_config( + Args::default(), + Config::default(), + test_syntax_conf(None), + test_case, + ) + .await } pub fn temp_file_with_contents>( @@ -207,16 +227,60 @@ pub fn new_readonly_tempfile() -> anyhow::Result { Ok(file) } -/// Creates a new Application with default config that opens the given file -/// path -pub fn app_with_file>(path: P) -> anyhow::Result { - Application::new( - Args { - files: vec![(path.into(), helix_core::Position::default())], - ..Default::default() - }, - Config::default(), - ) +#[derive(Default)] +pub struct AppBuilder { + args: Args, + config: Config, + syn_conf: helix_core::syntax::Configuration, + input: Option<(String, Selection)>, +} + +impl AppBuilder { + pub fn new() -> Self { + AppBuilder::default() + } + + pub fn with_file>( + mut self, + path: P, + pos: Option, + ) -> Self { + self.args.files.push((path.into(), pos.unwrap_or_default())); + self + } + + pub fn with_config(mut self, config: Config) -> Self { + self.config = config; + self + } + + pub fn with_input_text>(mut self, input_text: S) -> Self { + self.input = Some(test::print(&input_text.into())); + self + } + + pub fn with_lang_config(mut self, syn_conf: helix_core::syntax::Configuration) -> Self { + self.syn_conf = syn_conf; + self + } + + pub fn build(self) -> anyhow::Result { + let mut app = Application::new(self.args, self.config, self.syn_conf)?; + + if let Some((text, selection)) = self.input { + let (view, doc) = helix_view::current!(app.editor); + let sel = doc.selection(view.id).clone(); + let trans = Transaction::change_by_selection(doc.text(), &sel, |_| { + (0, doc.text().len_chars(), Some((text.clone()).into())) + }) + .with_selection(selection); + + // replace the initial text with the input text + doc.apply(&trans, view.id); + } + + Ok(app) + } } pub fn assert_file_has_content(file: &mut File, content: &str) -> anyhow::Result<()> { diff --git a/helix-term/tests/test/movement.rs b/helix-term/tests/test/movement.rs index 45aae39e..7212d026 100644 --- a/helix-term/tests/test/movement.rs +++ b/helix-term/tests/test/movement.rs @@ -70,7 +70,9 @@ async fn insert_to_normal_mode_cursor_position() -> anyhow::Result<()> { async fn cursor_position_newly_opened_file() -> anyhow::Result<()> { let test = |content: &str, expected_sel: Selection| -> anyhow::Result<()> { let file = helpers::temp_file_with_contents(content)?; - let mut app = helpers::app_with_file(file.path())?; + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .build()?; let (view, doc) = helix_view::current!(app.editor); let sel = doc.selection(view.id).clone(); diff --git a/helix-term/tests/test/prompt.rs b/helix-term/tests/test/prompt.rs index 2ab9604c..62ec03f1 100644 --- a/helix-term/tests/test/prompt.rs +++ b/helix-term/tests/test/prompt.rs @@ -1,11 +1,9 @@ use super::*; -use helix_term::application::Application; - #[tokio::test] async fn test_history_completion() -> anyhow::Result<()> { test_key_sequence( - &mut Application::new(Args::default(), Config::default())?, + &mut AppBuilder::new().build()?, Some(":asdf:theme d"), Some(&|app| { assert!(!app.editor.is_err()); diff --git a/helix-term/tests/test/splits.rs b/helix-term/tests/test/splits.rs index 70a517be..5807413a 100644 --- a/helix-term/tests/test/splits.rs +++ b/helix-term/tests/test/splits.rs @@ -6,8 +6,12 @@ async fn test_split_write_quit_all() -> anyhow::Result<()> { let mut file2 = tempfile::NamedTempFile::new()?; let mut file3 = tempfile::NamedTempFile::new()?; + let mut app = helpers::AppBuilder::new() + .with_file(file1.path(), None) + .build()?; + test_key_sequences( - &mut helpers::app_with_file(file1.path())?, + &mut app, vec![ ( Some(&format!( @@ -66,9 +70,12 @@ async fn test_split_write_quit_all() -> anyhow::Result<()> { #[tokio::test(flavor = "multi_thread")] async fn test_split_write_quit_same_file() -> anyhow::Result<()> { let mut file = tempfile::NamedTempFile::new()?; + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .build()?; test_key_sequences( - &mut helpers::app_with_file(file.path())?, + &mut app, vec![ ( Some("Oihello:spogoodbye"), diff --git a/helix-term/tests/test/write.rs b/helix-term/tests/test/write.rs index 7d105431..6aa51a31 100644 --- a/helix-term/tests/test/write.rs +++ b/helix-term/tests/test/write.rs @@ -4,7 +4,6 @@ use std::{ }; use helix_core::diagnostic::Severity; -use helix_term::application::Application; use helix_view::doc; use super::*; @@ -12,9 +11,12 @@ use super::*; #[tokio::test] async fn test_write() -> anyhow::Result<()> { let mut file = tempfile::NamedTempFile::new()?; + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .build()?; test_key_sequence( - &mut helpers::app_with_file(file.path())?, + &mut app, Some("ithe gostak distims the doshes:w"), None, false, @@ -38,9 +40,12 @@ async fn test_write() -> anyhow::Result<()> { #[tokio::test(flavor = "multi_thread")] async fn test_write_quit() -> anyhow::Result<()> { let mut file = tempfile::NamedTempFile::new()?; + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .build()?; test_key_sequence( - &mut helpers::app_with_file(file.path())?, + &mut app, Some("ithe gostak distims the doshes:wq"), None, true, @@ -66,19 +71,16 @@ async fn test_write_concurrent() -> anyhow::Result<()> { let mut file = tempfile::NamedTempFile::new()?; let mut command = String::new(); const RANGE: RangeInclusive = 1..=5000; + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .build()?; for i in RANGE { let cmd = format!("%c{}:w", i); command.push_str(&cmd); } - test_key_sequence( - &mut helpers::app_with_file(file.path())?, - Some(&command), - None, - false, - ) - .await?; + test_key_sequence(&mut app, Some(&command), None, false).await?; file.as_file_mut().flush()?; file.as_file_mut().sync_all()?; @@ -93,9 +95,12 @@ async fn test_write_concurrent() -> anyhow::Result<()> { #[tokio::test] async fn test_write_fail_mod_flag() -> anyhow::Result<()> { let file = helpers::new_readonly_tempfile()?; + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .build()?; test_key_sequences( - &mut helpers::app_with_file(file.path())?, + &mut app, vec![ ( None, @@ -133,7 +138,7 @@ async fn test_write_scratch_to_new_path() -> anyhow::Result<()> { let mut file = tempfile::NamedTempFile::new()?; test_key_sequence( - &mut Application::new(Args::default(), Config::default())?, + &mut AppBuilder::new().build()?, Some(format!("ihello:w {}", file.path().to_string_lossy()).as_ref()), Some(&|app| { assert!(!app.editor.is_err()); @@ -174,19 +179,40 @@ async fn test_write_scratch_no_path_fails() -> anyhow::Result<()> { Ok(()) } +#[tokio::test] +async fn test_write_auto_format_fails_still_writes() -> anyhow::Result<()> { + let mut file = tempfile::Builder::new().suffix(".rs").tempfile()?; + + let lang_conf = indoc! {r#" + [[language]] + name = "rust" + formatter = { command = "bash", args = [ "-c", "exit 1" ] } + "#}; + + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .with_input_text("#[l|]#et foo = 0;\n") + .with_lang_config(helpers::test_syntax_conf(Some(lang_conf.into()))) + .build()?; + + test_key_sequences(&mut app, vec![(Some(":w"), None)], false).await?; + + // file still saves + helpers::assert_file_has_content(file.as_file_mut(), "let foo = 0;\n")?; + + Ok(()) +} + #[tokio::test] async fn test_write_new_path() -> anyhow::Result<()> { let mut file1 = tempfile::NamedTempFile::new().unwrap(); let mut file2 = tempfile::NamedTempFile::new().unwrap(); + let mut app = helpers::AppBuilder::new() + .with_file(file1.path(), None) + .build()?; test_key_sequences( - &mut Application::new( - Args { - files: vec![(file1.path().to_path_buf(), Position::default())], - ..Default::default() - }, - Config::default(), - )?, + &mut app, vec![ ( Some("ii can eat glass, it will not hurt me:w"), @@ -228,7 +254,7 @@ async fn test_write_fail_new_path() -> anyhow::Result<()> { let file = helpers::new_readonly_tempfile()?; test_key_sequences( - &mut Application::new(Args::default(), Config::default())?, + &mut AppBuilder::new().build()?, vec![ ( None, -- cgit v1.2.3-70-g09d2 From bf378e71b012b4c108d11825cee6eea7b69778c2 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Tue, 4 Oct 2022 01:37:02 -0400 Subject: fix tests --- helix-term/tests/test/movement.rs | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'helix-term/tests/test') diff --git a/helix-term/tests/test/movement.rs b/helix-term/tests/test/movement.rs index 7212d026..81c66e53 100644 --- a/helix-term/tests/test/movement.rs +++ b/helix-term/tests/test/movement.rs @@ -117,6 +117,7 @@ async fn select_mode_tree_sitter_next_function_is_union_of_objects() -> anyhow:: ..Default::default() }, Config::default(), + helpers::test_syntax_conf(None), ( helpers::platform_line(indoc! {"\ #[/|]#// Increments @@ -148,6 +149,7 @@ async fn select_mode_tree_sitter_prev_function_unselects_object() -> anyhow::Res ..Default::default() }, Config::default(), + helpers::test_syntax_conf(None), ( helpers::platform_line(indoc! {"\ /// Increments @@ -180,6 +182,7 @@ async fn select_mode_tree_sitter_prev_function_goes_backwards_to_object() -> any ..Default::default() }, Config::default(), + helpers::test_syntax_conf(None), ( helpers::platform_line(indoc! {"\ /// Increments @@ -210,6 +213,7 @@ async fn select_mode_tree_sitter_prev_function_goes_backwards_to_object() -> any ..Default::default() }, Config::default(), + helpers::test_syntax_conf(None), ( helpers::platform_line(indoc! {"\ /// Increments -- cgit v1.2.3-70-g09d2 From beb3427bfbaa88bec8b4c683e342f85eb53ad77d Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Tue, 4 Oct 2022 10:35:15 -0400 Subject: improve app close failure display --- helix-term/src/application.rs | 72 +++++++++++++--------------------------- helix-term/tests/test/helpers.rs | 12 ++++++- 2 files changed, 34 insertions(+), 50 deletions(-) (limited to 'helix-term/tests/test') diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 694c55c0..2e49e6d1 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -951,28 +951,10 @@ impl Application { self.event_loop(input_stream).await; - // let mut save_errs = Vec::new(); - - // for doc in self.editor.documents_mut() { - // if let Some(Err(err)) = doc.close().await { - // save_errs.push(( - // doc.path() - // .map(|path| path.to_string_lossy().into_owned()) - // .unwrap_or_else(|| "".into()), - // err, - // )); - // } - // } - - let close_err = self.close().await.err(); + let close_errs = self.close().await; restore_term()?; - // for (path, err) in save_errs { - // self.editor.exit_code = 1; - // eprintln!("Error closing '{}': {}", path, err); - // } - - if let Some(err) = close_err { + for err in close_errs { self.editor.exit_code = 1; eprintln!("Error: {}", err); } @@ -980,49 +962,41 @@ impl Application { Ok(self.editor.exit_code) } - pub async fn close(&mut self) -> anyhow::Result<()> { + pub async fn close(&mut self) -> Vec { // [NOTE] we intentionally do not return early for errors because we // want to try to run as much cleanup as we can, regardless of // errors along the way + let mut errs = Vec::new(); - let mut result = match self + if let Err(err) = self .jobs .finish(&mut self.editor, Some(&mut self.compositor)) .await { - Ok(_) => Ok(()), - Err(err) => { - log::error!("Error executing job: {}", err); - Err(err) - } + log::error!("Error executing job: {}", err); + errs.push(err); }; for doc in self.editor.documents_mut() { - if let Some(save_result) = doc.close().await { - result = match save_result { - Ok(_) => result, - Err(err) => { - if let Some(path) = doc.path() { - log::error!( - "Error saving document '{}': {}", - path.to_string_lossy(), - err - ); - } - Err(err) - } - }; + if let Some(Err(err)) = doc.close().await { + if let Some(path) = doc.path() { + log::error!( + "Error saving document '{}': {}", + path.to_string_lossy(), + err + ); + } + errs.push(err); } } - match self.editor.close_language_servers(None).await { - Ok(_) => result, - Err(_) => { - log::error!("Timed out waiting for language servers to shutdown"); - Err(anyhow::format_err!( - "Timed out waiting for language servers to shutdown" - )) - } + if self.editor.close_language_servers(None).await.is_err() { + log::error!("Timed out waiting for language servers to shutdown"); + errs.push(anyhow::format_err!( + "Timed out waiting for language servers to shutdown" + )); } + + errs } } diff --git a/helix-term/tests/test/helpers.rs b/helix-term/tests/test/helpers.rs index c2fbe953..5adc3354 100644 --- a/helix-term/tests/test/helpers.rs +++ b/helix-term/tests/test/helpers.rs @@ -94,7 +94,17 @@ pub async fn test_key_sequences( tokio::time::timeout(TIMEOUT, event_loop).await?; } - app.close().await?; + let errs = app.close().await; + + if !errs.is_empty() { + log::error!("Errors closing app"); + + for err in errs { + log::error!("{}", err); + } + + bail!("Error closing app"); + } Ok(()) } -- cgit v1.2.3-70-g09d2