various fixes in write-all path

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<String> = 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(

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(())
     }

helix-term/tests/integration.rs

@@ -22,5 +22,6 @@ mod test {
     mod commands;
     mod movement;
     mod prompt;
+    mod splits;
     mod write;
 }

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(())
 }

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<P: Into<PathBuf>>(path: P) -> anyhow::Result<Application> {
         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);
+    }
+}

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<esc>:sp<ret>:o {}<ret>ihello2<esc>:sp<ret>:o {}<ret>ihello3<esc>",
+                    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<ret>"),
+                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("O<esc>ihello<esc>:sp<ret>ogoodbye<esc>"),
+                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<ret>"),
+                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(())
+}