From 7090555daba6bce37dc6cc900387015a10a1e791 Mon Sep 17 00:00:00 2001 From: Em Zhan Date: Mon, 11 Sep 2023 19:06:25 -0500 Subject: [PATCH] Add `insert-final-newline` config option (#8157) Co-authored-by: Xalfer <64538944+Xalfer@users.noreply.github.com> --- book/src/configuration.md | 1 + helix-term/src/commands/typed.rs | 70 ++++++++----- helix-term/tests/test/commands/write.rs | 125 +++++++++++++++++++++++- helix-term/tests/test/helpers.rs | 2 +- helix-term/tests/test/splits.rs | 6 +- helix-view/src/editor.rs | 3 + 6 files changed, 173 insertions(+), 34 deletions(-) diff --git a/book/src/configuration.md b/book/src/configuration.md index eb2cf473..3b78481e 100644 --- a/book/src/configuration.md +++ b/book/src/configuration.md @@ -64,6 +64,7 @@ Its settings will be merged with the configuration directory `config.toml` and t | `text-width` | Maximum line length. Used for the `:reflow` command and soft-wrapping if `soft-wrap.wrap-at-text-width` is set | `80` | | `workspace-lsp-roots` | Directories relative to the workspace root that are treated as LSP roots. Should only be set in `.helix/config.toml` | `[]` | | `default-line-ending` | The line ending to use for new documents. Can be `native`, `lf`, `crlf`, `ff`, `cr` or `nel`. `native` uses the platform's native line ending (`crlf` on Windows, otherwise `lf`). | `native` | +| `insert-final-newline` | Whether to automatically insert a trailing line-ending on write if missing | `true` | ### `[editor.statusline]` Section diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 4237b6f6..abe6dd97 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -6,7 +6,7 @@ use crate::job::Job; use super::*; use helix_core::fuzzy::fuzzy_match; -use helix_core::{encoding, shellwords::Shellwords}; +use helix_core::{encoding, line_ending, shellwords::Shellwords}; use helix_view::document::DEFAULT_LANGUAGE_NAME; use helix_view::editor::{Action, CloseError, ConfigEvent}; use serde_json::Value; @@ -330,12 +330,16 @@ fn write_impl( path: Option<&Cow>, force: bool, ) -> anyhow::Result<()> { - let editor_auto_fmt = cx.editor.config().auto_format; + let config = cx.editor.config(); let jobs = &mut cx.jobs; let (view, doc) = current!(cx.editor); let path = path.map(AsRef::as_ref); - let fmt = if editor_auto_fmt { + if config.insert_final_newline { + insert_final_newline(doc, view); + } + + let fmt = if config.auto_format { doc.auto_format().map(|fmt| { let callback = make_format_callback( doc.id(), @@ -359,6 +363,16 @@ fn write_impl( Ok(()) } +fn insert_final_newline(doc: &mut Document, view: &mut View) { + let text = doc.text(); + if line_ending::get_line_ending(&text.slice(..)).is_none() { + let eof = Selection::point(text.len_chars()); + let insert = Transaction::insert(text, &eof, doc.line_ending.as_str().into()); + doc.apply(&insert, view.id); + doc.append_changes_to_history(view); + } +} + fn write( cx: &mut compositor::Context, args: &[Cow], @@ -658,11 +672,10 @@ pub fn write_all_impl( write_scratch: bool, ) -> anyhow::Result<()> { let mut errors: Vec<&'static str> = Vec::new(); - let auto_format = cx.editor.config().auto_format; + let config = cx.editor.config(); let jobs = &mut cx.jobs; let current_view = view!(cx.editor); - // save all documents let saves: Vec<_> = cx .editor .documents @@ -693,32 +706,35 @@ pub fn write_all_impl( current_view.id }; - let fmt = if auto_format { - doc.auto_format().map(|fmt| { - let callback = make_format_callback( - doc.id(), - doc.version(), - target_view, - fmt, - Some((None, force)), - ); - jobs.add(Job::with_callback(callback).wait_before_exiting()); - }) - } else { - None - }; + Some((doc.id(), target_view)) + }) + .collect(); - if fmt.is_none() { - return Some(doc.id()); - } + for (doc_id, target_view) in saves { + let doc = doc_mut!(cx.editor, &doc_id); + if config.insert_final_newline { + insert_final_newline(doc, view_mut!(cx.editor, target_view)); + } + + let fmt = if config.auto_format { + doc.auto_format().map(|fmt| { + let callback = make_format_callback( + doc_id, + doc.version(), + target_view, + fmt, + Some((None, force)), + ); + jobs.add(Job::with_callback(callback).wait_before_exiting()); + }) + } else { None - }) - .collect(); + }; - // manually call save for the rest of docs that don't have a formatter - for id in saves { - cx.editor.save::(id, None, force)?; + if fmt.is_none() { + cx.editor.save::(doc_id, None, force)?; + } } if !errors.is_empty() && !force { diff --git a/helix-term/tests/test/commands/write.rs b/helix-term/tests/test/commands/write.rs index f33c8aaf..376ba5e7 100644 --- a/helix-term/tests/test/commands/write.rs +++ b/helix-term/tests/test/commands/write.rs @@ -93,7 +93,7 @@ async fn test_buffer_close_concurrent() -> anyhow::Result<()> { ) .await?; - helpers::assert_file_has_content(file.as_file_mut(), &RANGE.end().to_string())?; + helpers::assert_file_has_content(file.as_file_mut(), &platform_line(&RANGE.end().to_string()))?; Ok(()) } @@ -209,7 +209,7 @@ async fn test_write_concurrent() -> anyhow::Result<()> { let mut file_content = String::new(); file.as_file_mut().read_to_string(&mut file_content)?; - assert_eq!(RANGE.end().to_string(), file_content); + assert_eq!(platform_line(&RANGE.end().to_string()), file_content); Ok(()) } @@ -424,13 +424,132 @@ async fn test_write_utf_bom_file() -> anyhow::Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread")] +async fn test_write_insert_final_newline_added_if_missing() -> anyhow::Result<()> { + let mut file = tempfile::NamedTempFile::new()?; + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .with_input_text("#[h|]#ave you tried chamomile tea?") + .build()?; + + test_key_sequence(&mut app, Some(":w"), None, false).await?; + + helpers::assert_file_has_content( + file.as_file_mut(), + &helpers::platform_line("have you tried chamomile tea?\n"), + )?; + + Ok(()) +} + +#[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 mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .with_input_text(&helpers::platform_line("#[t|]#en minutes, please\n")) + .build()?; + + 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"), + )?; + + 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 mut app = helpers::AppBuilder::new() + .with_config(Config { + editor: helix_view::editor::Config { + insert_final_newline: false, + ..Default::default() + }, + ..Default::default() + }) + .with_file(file.path(), None) + .with_input_text("#[t|]#he quiet rain continued through the night") + .build()?; + + 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", + )?; + + 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 mut app = helpers::AppBuilder::new() + .with_file(file1.path(), None) + .with_input_text("#[w|]#e don't serve time travelers here") + .build()?; + + test_key_sequence( + &mut app, + Some(&format!( + ":o {}ia time traveler walks into a bar:wa", + file2.path().to_string_lossy() + )), + None, + false, + ) + .await?; + + helpers::assert_file_has_content( + file1.as_file_mut(), + &helpers::platform_line("we don't serve time travelers here\n"), + )?; + + helpers::assert_file_has_content( + file2.as_file_mut(), + &helpers::platform_line("a time traveler walks into a bar\n"), + )?; + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_write_all_insert_final_newline_do_not_add_if_unmodified() -> anyhow::Result<()> { + let mut file = tempfile::NamedTempFile::new()?; + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .build()?; + + file.write_all(b"i lost on Jeopardy!")?; + file.rewind()?; + + test_key_sequence(&mut app, Some(":wa"), None, false).await?; + + helpers::assert_file_has_content(file.as_file_mut(), "i lost on Jeopardy!")?; + + Ok(()) +} + async fn edit_file_with_content(file_content: &[u8]) -> anyhow::Result<()> { let mut file = tempfile::NamedTempFile::new()?; file.as_file_mut().write_all(&file_content)?; helpers::test_key_sequence( - &mut helpers::AppBuilder::new().build()?, + &mut helpers::AppBuilder::new() + .with_config(Config { + editor: helix_view::editor::Config { + insert_final_newline: false, + ..Default::default() + }, + ..Default::default() + }) + .build()?, Some(&format!(":o {}:x", file.path().to_string_lossy())), None, true, diff --git a/helix-term/tests/test/helpers.rs b/helix-term/tests/test/helpers.rs index 6466bc76..e6762baf 100644 --- a/helix-term/tests/test/helpers.rs +++ b/helix-term/tests/test/helpers.rs @@ -350,7 +350,7 @@ pub fn assert_file_has_content(file: &mut File, content: &str) -> anyhow::Result let mut file_content = String::new(); file.read_to_string(&mut file_content)?; - assert_eq!(content, file_content); + assert_eq!(file_content, content); Ok(()) } diff --git a/helix-term/tests/test/splits.rs b/helix-term/tests/test/splits.rs index 1d70f24a..f010c86b 100644 --- a/helix-term/tests/test/splits.rs +++ b/helix-term/tests/test/splits.rs @@ -62,9 +62,9 @@ async fn test_split_write_quit_all() -> anyhow::Result<()> { ) .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")?; + helpers::assert_file_has_content(file1.as_file_mut(), &platform_line("hello1"))?; + helpers::assert_file_has_content(file2.as_file_mut(), &platform_line("hello2"))?; + helpers::assert_file_has_content(file3.as_file_mut(), &platform_line("hello3"))?; Ok(()) } diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 6963867a..2265633d 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -287,6 +287,8 @@ pub struct Config { pub workspace_lsp_roots: Vec, /// Which line ending to choose for new documents. Defaults to `native`. i.e. `crlf` on Windows, otherwise `lf`. pub default_line_ending: LineEndingConfig, + /// Whether to automatically insert a trailing line-ending on write if missing. Defaults to `true`. + pub insert_final_newline: bool, /// Enables smart tab pub smart_tab: Option, } @@ -842,6 +844,7 @@ impl Default for Config { completion_replace: false, workspace_lsp_roots: Vec::new(), default_line_ending: LineEndingConfig::default(), + insert_final_newline: true, smart_tab: Some(SmartTabConfig::default()), } }