Add insert-final-newline config option (#8157)

Co-authored-by: Xalfer <64538944+Xalfer@users.noreply.github.com>

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
 

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<str>>,
     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<str>],
@@ -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,10 +706,21 @@ pub fn write_all_impl(
                 current_view.id
             };
 
-            let fmt = if auto_format {
+            Some((doc.id(), target_view))
+        })
+        .collect();
+
+    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_id,
                     doc.version(),
                     target_view,
                     fmt,
@@ -709,16 +733,8 @@ pub fn write_all_impl(
         };
 
         if fmt.is_none() {
-                return Some(doc.id());
+            cx.editor.save::<PathBuf>(doc_id, None, force)?;
         }
-
-            None
-        })
-        .collect();
-
-    // manually call save for the rest of docs that don't have a formatter
-    for id in saves {
-        cx.editor.save::<PathBuf>(id, None, force)?;
     }
 
     if !errors.is_empty() && !force {

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<ret>"), 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<ret>"), 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<ret>"), 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 {}<ret>ia time traveler walks into a bar<esc>:wa<ret>",
+            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<ret>"), 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 {}<ret>:x<ret>", file.path().to_string_lossy())),
         None,
         true,

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

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

helix-view/src/editor.rs

@@ -287,6 +287,8 @@ pub struct Config {
     pub workspace_lsp_roots: Vec<PathBuf>,
     /// 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<SmartTabConfig>,
 }
@@ -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()),
         }
     }