From 57de4e62519a59aece104867569c2b9ad044af54 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Wed, 24 Aug 2022 01:13:41 -0400 Subject: [PATCH] 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 diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index fa2ba5e6d..7fd619d92 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 6ef77341d..5077807d9 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 8969e976e..e3754c436 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 b7c0f7cc4..0279e348c 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 bbcc6632e..ed1a03317 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 000000000..70a517be4 --- /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(()) +}