Add a setting for ensuring a single final newline on save

Max Brunsfeld created

Change summary

assets/settings/default.json      |  6 +++
crates/editor/src/editor_tests.rs | 22 ++++++++++++----
crates/language/src/buffer.rs     | 26 +++++++++++++++++++
crates/project/src/project.rs     | 43 +++++++++++++++++++++++---------
crates/settings/src/settings.rs   | 11 ++++++++
5 files changed, 88 insertions(+), 20 deletions(-)

Detailed changes

assets/settings/default.json 🔗

@@ -51,8 +51,12 @@
     // 3. Position the dock full screen over the entire workspace"
     //     "default_dock_anchor": "expanded"
     "default_dock_anchor": "right",
-    // Whether or not to remove trailing whitespace from lines before saving.
+    // Whether or not to remove any trailing whitespace from lines of a buffer
+    // before saving it.
     "remove_trailing_whitespace_on_save": true,
+    // Whether or not to ensure there's a single newline at the end of a buffer
+    // when saving it.
+    "ensure_final_newline_on_save": true,
     // Whether or not to perform a buffer format before saving
     "format_on_save": "on",
     // How to perform a buffer format. This setting can take two values:

crates/editor/src/editor_tests.rs 🔗

@@ -4314,12 +4314,13 @@ async fn test_strip_whitespace_and_format_via_lsp(cx: &mut gpui::TestAppContext)
     )
     .await;
 
-    // Set up a buffer white some trailing whitespace.
+    // Set up a buffer white some trailing whitespace and no trailing newline.
     cx.set_state(
         &[
             "one ",   //
             "twoˇ",  //
             "three ", //
+            "four",   //
         ]
         .join("\n"),
     );
@@ -4348,7 +4349,8 @@ async fn test_strip_whitespace_and_format_via_lsp(cx: &mut gpui::TestAppContext)
     cx.lsp.handle_request::<lsp::request::Formatting, _, _>({
         let buffer_changes = buffer_changes.clone();
         move |_, _| {
-            // When formatting is requested, trailing whitespace has already been stripped.
+            // When formatting is requested, trailing whitespace has already been stripped,
+            // and the trailing newline has already been added.
             assert_eq!(
                 &buffer_changes.lock()[1..],
                 &[
@@ -4360,6 +4362,10 @@ async fn test_strip_whitespace_and_format_via_lsp(cx: &mut gpui::TestAppContext)
                         lsp::Range::new(lsp::Position::new(2, 5), lsp::Position::new(2, 6)),
                         "".into()
                     ),
+                    (
+                        lsp::Range::new(lsp::Position::new(3, 4), lsp::Position::new(3, 4)),
+                        "\n".into()
+                    ),
                 ]
             );
 
@@ -4379,8 +4385,9 @@ async fn test_strip_whitespace_and_format_via_lsp(cx: &mut gpui::TestAppContext)
         }
     });
 
-    // After formatting the buffer, the trailing whitespace is stripped and the
-    // edits provided by the language server have been applied.
+    // After formatting the buffer, the trailing whitespace is stripped,
+    // a newline is appended, and the edits provided by the language server
+    // have been applied.
     format.await.unwrap();
     cx.assert_editor_state(
         &[
@@ -4389,18 +4396,21 @@ async fn test_strip_whitespace_and_format_via_lsp(cx: &mut gpui::TestAppContext)
             "twoˇ", //
             "",      //
             "three", //
+            "four",  //
+            "",      //
         ]
         .join("\n"),
     );
 
-    // Undoing the formatting undoes both the trailing whitespace removal and the
-    // LSP edits.
+    // Undoing the formatting undoes the trailing whitespace removal, the
+    // trailing newline, and the LSP edits.
     cx.update_buffer(|buffer, cx| buffer.undo(cx));
     cx.assert_editor_state(
         &[
             "one ",   //
             "twoˇ",  //
             "three ", //
+            "four",   //
         ]
         .join("\n"),
     );

crates/language/src/buffer.rs 🔗

@@ -1156,6 +1156,8 @@ impl Buffer {
         })
     }
 
+    /// Spawn a background task that searches the buffer for any whitespace
+    /// at the ends of a lines, and returns a `Diff` that removes that whitespace.
     pub fn remove_trailing_whitespace(&self, cx: &AppContext) -> Task<Diff> {
         let old_text = self.as_rope().clone();
         let line_ending = self.line_ending();
@@ -1174,6 +1176,27 @@ impl Buffer {
         })
     }
 
+    /// Ensure that the buffer ends with a single newline character, and
+    /// no other whitespace.
+    pub fn ensure_final_newline(&mut self, cx: &mut ModelContext<Self>) {
+        let len = self.len();
+        let mut offset = len;
+        for chunk in self.as_rope().reversed_chunks_in_range(0..len) {
+            let non_whitespace_len = chunk
+                .trim_end_matches(|c: char| c.is_ascii_whitespace())
+                .len();
+            offset -= chunk.len();
+            offset += non_whitespace_len;
+            if non_whitespace_len != 0 {
+                if offset == len - 1 && chunk.get(non_whitespace_len..) == Some("\n") {
+                    return;
+                }
+                break;
+            }
+        }
+        self.edit([(offset..len, "\n")], None, cx);
+    }
+
     pub fn apply_diff(&mut self, diff: Diff, cx: &mut ModelContext<Self>) -> Option<TransactionId> {
         if self.version == diff.base_version {
             self.apply_non_conflicting_portion_of_diff(diff, cx)
@@ -1182,6 +1205,9 @@ impl Buffer {
         }
     }
 
+    /// Apply a diff to the buffer. If the buffer has changed since the given diff was
+    /// calculated, then adjust the diff to account for those changes, and discard any
+    /// parts of the diff that conflict with those changes.
     pub fn apply_non_conflicting_portion_of_diff(
         &mut self,
         diff: Diff,

crates/project/src/project.rs 🔗

@@ -2887,18 +2887,23 @@ impl Project {
 
                 let mut project_transaction = ProjectTransaction::default();
                 for (buffer, buffer_abs_path, language_server) in &buffers_with_paths_and_servers {
-                    let (format_on_save, remove_trailing_whitespace, formatter, tab_size) = buffer
-                        .read_with(&cx, |buffer, cx| {
-                            let settings = cx.global::<Settings>();
-                            let language_name = buffer.language().map(|language| language.name());
-                            (
-                                settings.format_on_save(language_name.as_deref()),
-                                settings
-                                    .remove_trailing_whitespace_on_save(language_name.as_deref()),
-                                settings.formatter(language_name.as_deref()),
-                                settings.tab_size(language_name.as_deref()),
-                            )
-                        });
+                    let (
+                        format_on_save,
+                        remove_trailing_whitespace,
+                        ensure_final_newline,
+                        formatter,
+                        tab_size,
+                    ) = buffer.read_with(&cx, |buffer, cx| {
+                        let settings = cx.global::<Settings>();
+                        let language_name = buffer.language().map(|language| language.name());
+                        (
+                            settings.format_on_save(language_name.as_deref()),
+                            settings.remove_trailing_whitespace_on_save(language_name.as_deref()),
+                            settings.ensure_final_newline_on_save(language_name.as_deref()),
+                            settings.formatter(language_name.as_deref()),
+                            settings.tab_size(language_name.as_deref()),
+                        )
+                    });
 
                     let whitespace_transaction_id = if remove_trailing_whitespace {
                         let diff = buffer
@@ -2906,7 +2911,19 @@ impl Project {
                             .await;
                         buffer.update(&mut cx, move |buffer, cx| {
                             buffer.finalize_last_transaction();
-                            buffer.apply_non_conflicting_portion_of_diff(diff, cx)
+                            buffer.start_transaction();
+                            buffer.apply_non_conflicting_portion_of_diff(diff, cx);
+                            if ensure_final_newline {
+                                buffer.ensure_final_newline(cx);
+                            }
+                            buffer.end_transaction(cx)
+                        })
+                    } else if ensure_final_newline {
+                        buffer.update(&mut cx, move |buffer, cx| {
+                            buffer.finalize_last_transaction();
+                            buffer.start_transaction();
+                            buffer.ensure_final_newline(cx);
+                            buffer.end_transaction(cx)
                         })
                     } else {
                         None

crates/settings/src/settings.rs 🔗

@@ -95,6 +95,7 @@ pub struct EditorSettings {
     pub preferred_line_length: Option<u32>,
     pub format_on_save: Option<FormatOnSave>,
     pub remove_trailing_whitespace_on_save: Option<bool>,
+    pub ensure_final_newline_on_save: Option<bool>,
     pub formatter: Option<Formatter>,
     pub enable_language_server: Option<bool>,
 }
@@ -365,6 +366,9 @@ impl Settings {
                 remove_trailing_whitespace_on_save: required(
                     defaults.editor.remove_trailing_whitespace_on_save,
                 ),
+                ensure_final_newline_on_save: required(
+                    defaults.editor.ensure_final_newline_on_save,
+                ),
                 format_on_save: required(defaults.editor.format_on_save),
                 formatter: required(defaults.editor.formatter),
                 enable_language_server: required(defaults.editor.enable_language_server),
@@ -470,6 +474,12 @@ impl Settings {
         })
     }
 
+    pub fn ensure_final_newline_on_save(&self, language: Option<&str>) -> bool {
+        self.language_setting(language, |settings| {
+            settings.ensure_final_newline_on_save.clone()
+        })
+    }
+
     pub fn format_on_save(&self, language: Option<&str>) -> FormatOnSave {
         self.language_setting(language, |settings| settings.format_on_save.clone())
     }
@@ -569,6 +579,7 @@ impl Settings {
                 soft_wrap: Some(SoftWrap::None),
                 preferred_line_length: Some(80),
                 remove_trailing_whitespace_on_save: Some(true),
+                ensure_final_newline_on_save: Some(true),
                 format_on_save: Some(FormatOnSave::On),
                 formatter: Some(Formatter::LanguageServer),
                 enable_language_server: Some(true),