From 86eb32a6df087a095d7f9b0594266bc512109eb6 Mon Sep 17 00:00:00 2001 From: kirawi <67773714+kirawi@users.noreply.github.com> Date: Thu, 4 Jan 2024 14:14:57 -0500 Subject: [PATCH] use temporary files for writes --- Cargo.lock | 1 + helix-term/tests/test/commands/write.rs | 90 ++++++++++++------------- helix-term/tests/test/helpers.rs | 8 ++- helix-view/Cargo.toml | 2 + helix-view/src/document.rs | 13 +++- 5 files changed, 64 insertions(+), 50 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9c612a7e8d2bd..df927df2d9fde 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1250,6 +1250,7 @@ dependencies = [ "serde", "serde_json", "slotmap", + "tempfile", "tokio", "tokio-stream", "toml", diff --git a/helix-term/tests/test/commands/write.rs b/helix-term/tests/test/commands/write.rs index 376ba5e7b1e78..b9245cfadb6c4 100644 --- a/helix-term/tests/test/commands/write.rs +++ b/helix-term/tests/test/commands/write.rs @@ -65,7 +65,7 @@ async fn test_buffer_close_concurrent() -> anyhow::Result<()> { .await?; // verify if writes are queued up, it finishes them before closing the buffer - let mut file = tempfile::NamedTempFile::new()?; + let file = tempfile::NamedTempFile::new()?; let mut command = String::new(); const RANGE: RangeInclusive = 1..=1000; @@ -92,15 +92,16 @@ async fn test_buffer_close_concurrent() -> anyhow::Result<()> { false, ) .await?; + let (mut file, _) = reload_file(file); - helpers::assert_file_has_content(file.as_file_mut(), &platform_line(&RANGE.end().to_string()))?; + helpers::assert_file_has_content(&mut file, &platform_line(&RANGE.end().to_string()))?; Ok(()) } #[tokio::test(flavor = "multi_thread")] async fn test_write() -> anyhow::Result<()> { - let mut file = tempfile::NamedTempFile::new()?; + let file = tempfile::NamedTempFile::new()?; let mut app = helpers::AppBuilder::new() .with_file(file.path(), None) .build()?; @@ -113,11 +114,9 @@ async fn test_write() -> anyhow::Result<()> { ) .await?; - file.as_file_mut().flush()?; - file.as_file_mut().sync_all()?; - + let (mut file, _) = reload_file(file); let mut file_content = String::new(); - file.as_file_mut().read_to_string(&mut file_content)?; + file.read_to_string(&mut file_content)?; assert_eq!( helpers::platform_line("the gostak distims the doshes"), @@ -144,12 +143,9 @@ async fn test_overwrite_protection() -> anyhow::Result<()> { test_key_sequence(&mut app, Some(":x"), None, false).await?; - file.as_file_mut().flush()?; - file.as_file_mut().sync_all()?; - - file.rewind()?; + let (mut file, _) = reload_file(file); let mut file_content = String::new(); - file.as_file_mut().read_to_string(&mut file_content)?; + file.read_to_string(&mut file_content)?; assert_eq!( helpers::platform_line("extremely important content"), @@ -161,7 +157,7 @@ async fn test_overwrite_protection() -> anyhow::Result<()> { #[tokio::test(flavor = "multi_thread")] async fn test_write_quit() -> anyhow::Result<()> { - let mut file = tempfile::NamedTempFile::new()?; + let file = tempfile::NamedTempFile::new()?; let mut app = helpers::AppBuilder::new() .with_file(file.path(), None) .build()?; @@ -174,15 +170,14 @@ async fn test_write_quit() -> anyhow::Result<()> { ) .await?; - file.as_file_mut().flush()?; - file.as_file_mut().sync_all()?; + let (mut file, _) = reload_file(file); let mut file_content = String::new(); - file.as_file_mut().read_to_string(&mut file_content)?; + file.read_to_string(&mut file_content)?; assert_eq!( helpers::platform_line("the gostak distims the doshes"), - file_content + file_content, ); Ok(()) @@ -190,7 +185,7 @@ async fn test_write_quit() -> anyhow::Result<()> { #[tokio::test(flavor = "multi_thread")] async fn test_write_concurrent() -> anyhow::Result<()> { - let mut file = tempfile::NamedTempFile::new()?; + let file = tempfile::NamedTempFile::new()?; let mut command = String::new(); const RANGE: RangeInclusive = 1..=1000; let mut app = helpers::AppBuilder::new() @@ -204,11 +199,9 @@ async fn test_write_concurrent() -> anyhow::Result<()> { test_key_sequence(&mut app, Some(&command), None, false).await?; - file.as_file_mut().flush()?; - file.as_file_mut().sync_all()?; - + let (mut file, _) = reload_file(file); let mut file_content = String::new(); - file.as_file_mut().read_to_string(&mut file_content)?; + file.read_to_string(&mut file_content)?; assert_eq!(platform_line(&RANGE.end().to_string()), file_content); Ok(()) @@ -257,7 +250,7 @@ async fn test_write_fail_mod_flag() -> anyhow::Result<()> { #[tokio::test(flavor = "multi_thread")] async fn test_write_scratch_to_new_path() -> anyhow::Result<()> { - let mut file = tempfile::NamedTempFile::new()?; + let file = tempfile::NamedTempFile::new()?; test_key_sequence( &mut AppBuilder::new().build()?, @@ -275,7 +268,8 @@ async fn test_write_scratch_to_new_path() -> anyhow::Result<()> { ) .await?; - helpers::assert_file_has_content(file.as_file_mut(), &helpers::platform_line("hello"))?; + let (mut file, _) = reload_file(file); + helpers::assert_file_has_content(&mut file, &helpers::platform_line("hello"))?; Ok(()) } @@ -303,7 +297,7 @@ async fn test_write_scratch_no_path_fails() -> anyhow::Result<()> { #[tokio::test(flavor = "multi_thread")] async fn test_write_auto_format_fails_still_writes() -> anyhow::Result<()> { - let mut file = tempfile::Builder::new().suffix(".rs").tempfile()?; + let file = tempfile::Builder::new().suffix(".rs").tempfile()?; let lang_conf = indoc! {r#" [[language]] @@ -319,16 +313,17 @@ async fn test_write_auto_format_fails_still_writes() -> anyhow::Result<()> { test_key_sequences(&mut app, vec![(Some(":w"), None)], false).await?; + let (mut file, _) = reload_file(file); // file still saves - helpers::assert_file_has_content(file.as_file_mut(), "let foo = 0;\n")?; + helpers::assert_file_has_content(&mut file, "let foo = 0;\n")?; Ok(()) } #[tokio::test(flavor = "multi_thread")] async fn test_write_new_path() -> anyhow::Result<()> { - let mut file1 = tempfile::NamedTempFile::new().unwrap(); - let mut file2 = tempfile::NamedTempFile::new().unwrap(); + let file1 = tempfile::NamedTempFile::new().unwrap(); + let file2 = tempfile::NamedTempFile::new().unwrap(); let mut app = helpers::AppBuilder::new() .with_file(file1.path(), None) .build()?; @@ -358,13 +353,15 @@ async fn test_write_new_path() -> anyhow::Result<()> { ) .await?; + let (mut file1, _) = reload_file(file1); + let (mut file2, _) = reload_file(file2); helpers::assert_file_has_content( - file1.as_file_mut(), + &mut file1, &helpers::platform_line("i can eat glass, it will not hurt me\n"), )?; helpers::assert_file_has_content( - file2.as_file_mut(), + &mut file2, &helpers::platform_line("i can eat glass, it will not hurt me\n"), )?; @@ -426,7 +423,7 @@ async fn test_write_utf_bom_file() -> anyhow::Result<()> { #[tokio::test(flavor = "multi_thread")] async fn test_write_insert_final_newline_added_if_missing() -> anyhow::Result<()> { - let mut file = tempfile::NamedTempFile::new()?; + let file = tempfile::NamedTempFile::new()?; let mut app = helpers::AppBuilder::new() .with_file(file.path(), None) .with_input_text("#[h|]#ave you tried chamomile tea?") @@ -434,8 +431,9 @@ async fn test_write_insert_final_newline_added_if_missing() -> anyhow::Result<() test_key_sequence(&mut app, Some(":w"), None, false).await?; + let (mut file, _) = reload_file(file); helpers::assert_file_has_content( - file.as_file_mut(), + &mut file, &helpers::platform_line("have you tried chamomile tea?\n"), )?; @@ -444,7 +442,7 @@ async fn test_write_insert_final_newline_added_if_missing() -> anyhow::Result<() #[tokio::test(flavor = "multi_thread")] async fn test_write_insert_final_newline_unchanged_if_not_missing() -> anyhow::Result<()> { - let mut file = tempfile::NamedTempFile::new()?; + let file = tempfile::NamedTempFile::new()?; let mut app = helpers::AppBuilder::new() .with_file(file.path(), None) .with_input_text(&helpers::platform_line("#[t|]#en minutes, please\n")) @@ -452,17 +450,15 @@ async fn test_write_insert_final_newline_unchanged_if_not_missing() -> anyhow::R test_key_sequence(&mut app, Some(":w"), None, false).await?; - helpers::assert_file_has_content( - file.as_file_mut(), - &helpers::platform_line("ten minutes, please\n"), - )?; + let (mut file, _) = reload_file(file); + helpers::assert_file_has_content(&mut file, &helpers::platform_line("ten minutes, please\n"))?; Ok(()) } #[tokio::test(flavor = "multi_thread")] async fn test_write_insert_final_newline_unchanged_if_missing_and_false() -> anyhow::Result<()> { - let mut file = tempfile::NamedTempFile::new()?; + let file = tempfile::NamedTempFile::new()?; let mut app = helpers::AppBuilder::new() .with_config(Config { editor: helix_view::editor::Config { @@ -477,18 +473,16 @@ async fn test_write_insert_final_newline_unchanged_if_missing_and_false() -> any test_key_sequence(&mut app, Some(":w"), None, false).await?; - helpers::assert_file_has_content( - file.as_file_mut(), - "the quiet rain continued through the night", - )?; + let (mut file, _) = reload_file(file); + helpers::assert_file_has_content(&mut file, "the quiet rain continued through the night")?; Ok(()) } #[tokio::test(flavor = "multi_thread")] async fn test_write_all_insert_final_newline_add_if_missing_and_modified() -> anyhow::Result<()> { - let mut file1 = tempfile::NamedTempFile::new()?; - let mut file2 = tempfile::NamedTempFile::new()?; + let file1 = tempfile::NamedTempFile::new()?; + let file2 = tempfile::NamedTempFile::new()?; let mut app = helpers::AppBuilder::new() .with_file(file1.path(), None) .with_input_text("#[w|]#e don't serve time travelers here") @@ -505,13 +499,15 @@ async fn test_write_all_insert_final_newline_add_if_missing_and_modified() -> an ) .await?; + let (mut file1, _) = reload_file(file1); + let (mut file2, _) = reload_file(file2); helpers::assert_file_has_content( - file1.as_file_mut(), + &mut file1, &helpers::platform_line("we don't serve time travelers here\n"), )?; helpers::assert_file_has_content( - file2.as_file_mut(), + &mut file2, &helpers::platform_line("a time traveler walks into a bar\n"), )?; @@ -556,7 +552,7 @@ async fn edit_file_with_content(file_content: &[u8]) -> anyhow::Result<()> { ) .await?; - file.rewind()?; + let (mut file, _) = reload_file(file); let mut new_file_content: Vec = Vec::new(); file.read_to_end(&mut new_file_content)?; diff --git a/helix-term/tests/test/helpers.rs b/helix-term/tests/test/helpers.rs index 112b5e3582df3..ec2d42610dcc2 100644 --- a/helix-term/tests/test/helpers.rs +++ b/helix-term/tests/test/helpers.rs @@ -11,7 +11,7 @@ use crossterm::event::{Event, KeyEvent}; use helix_core::{diagnostic::Severity, test, Selection, Transaction}; use helix_term::{application::Application, args::Args, config::Config, keymap::merge_keys}; use helix_view::{current_ref, doc, editor::LspConfig, input::parse_macro, Editor}; -use tempfile::NamedTempFile; +use tempfile::{NamedTempFile, TempPath}; use tokio_stream::wrappers::UnboundedReceiverStream; #[derive(Clone, Debug)] @@ -368,3 +368,9 @@ pub fn assert_status_not_error(editor: &Editor) { assert_ne!(&Severity::Error, sev); } } + +pub fn reload_file(t: NamedTempFile) -> (File, TempPath) { + let p = t.into_temp_path(); + let f = std::fs::File::open(&p).unwrap(); + (f, p) +} diff --git a/helix-view/Cargo.toml b/helix-view/Cargo.toml index db53b54cc9078..c18e296cd2b6b 100644 --- a/helix-view/Cargo.toml +++ b/helix-view/Cargo.toml @@ -26,6 +26,8 @@ bitflags = "2.4" anyhow = "1" crossterm = { version = "0.27", optional = true } +tempfile = "3.9" + # Conversion traits once_cell = "1.19" url = "2.5.0" diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index af950a3fc283e..53e50e489ef43 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -897,8 +897,17 @@ impl Document { } } - let mut file = File::create(&path).await?; - to_writer(&mut file, encoding_with_bom_info, &text).await?; + // TODO: Fail if path is read-only. + let (mut tmp_file, tmp_path) = tokio::task::spawn_blocking( + move || -> anyhow::Result<(File, tempfile::TempPath)> { + let (f, p) = tempfile::NamedTempFile::new()?.into_parts(); + Ok((tokio::fs::File::from_std(f), p)) + }, + ) + .await??; + + to_writer(&mut tmp_file, encoding_with_bom_info, &text).await?; + tokio::fs::rename(tmp_path, &path).await?; let event = DocumentSavedEvent { revision: current_rev,