Trigger formatting effects when saving unmodified singleton buffers (#32603)

Kirill Bulatov created

Closes https://github.com/zed-industries/zed/issues/12091

Use `"save_non_dirty_buffers": false` editor settings to disable this
behavior.

Release Notes:

- Fixed formatting effects not triggered when saving unmodified
singleton buffers

Change summary

assets/settings/default.json         |  3 +
crates/editor/src/editor.rs          |  6 +++
crates/editor/src/editor_settings.rs |  7 ++++
crates/editor/src/editor_tests.rs    | 46 +++++++++++++++++++++++++----
crates/editor/src/items.rs           | 27 +++++++++--------
5 files changed, 68 insertions(+), 21 deletions(-)

Detailed changes

assets/settings/default.json 🔗

@@ -219,6 +219,9 @@
   "inline_code_actions": true,
   // Whether to allow drag and drop text selection in buffer.
   "drag_and_drop_selection": true,
+  // Whether to save singleton buffers that are not dirty.
+  // This will "touch" the file and related tools enabled, e.g. formatters.
+  "save_non_dirty_buffers": true,
   // What to do when go to definition yields no results.
   //
   // 1. Do nothing: `none`

crates/editor/src/editor.rs 🔗

@@ -15636,6 +15636,10 @@ impl Editor {
         ))
     }
 
+    fn save_non_dirty_buffers(&self, cx: &App) -> bool {
+        self.is_singleton(cx) && EditorSettings::get_global(cx).save_non_dirty_buffers
+    }
+
     fn perform_format(
         &mut self,
         project: Entity<Project>,
@@ -15648,7 +15652,7 @@ impl Editor {
         let (buffers, target) = match target {
             FormatTarget::Buffers => {
                 let mut buffers = buffer.read(cx).all_buffers();
-                if trigger == FormatTrigger::Save {
+                if trigger == FormatTrigger::Save && !self.save_non_dirty_buffers(cx) {
                     buffers.retain(|buffer| buffer.read(cx).is_dirty());
                 }
                 (buffers, LspFormatTarget::Buffers)

crates/editor/src/editor_settings.rs 🔗

@@ -50,6 +50,7 @@ pub struct EditorSettings {
     pub diagnostics_max_severity: Option<DiagnosticSeverity>,
     pub inline_code_actions: bool,
     pub drag_and_drop_selection: bool,
+    pub save_non_dirty_buffers: bool,
 }
 
 #[derive(Copy, Clone, Debug, Serialize, Deserialize, PartialEq, Eq, JsonSchema)]
@@ -502,6 +503,12 @@ pub struct EditorSettingsContent {
     ///
     /// Default: true
     pub drag_and_drop_selection: Option<bool>,
+
+    /// Whether to save singleton buffers that are not dirty.
+    /// This will "touch" the file and related tools enabled, e.g. formatters.
+    ///
+    /// Default: true
+    pub save_non_dirty_buffers: Option<bool>,
 }
 
 // Toolbar related settings

crates/editor/src/editor_tests.rs 🔗

@@ -9085,12 +9085,17 @@ async fn test_document_format_during_save(cx: &mut TestAppContext) {
         );
     }
 
-    // For non-dirty buffer, no formatting request should be sent
+    // For non-dirty buffer and the corresponding settings, no formatting request should be sent
     {
         assert!(!cx.read(|cx| editor.is_dirty(cx)));
+        cx.update_global::<SettingsStore, _>(|settings, cx| {
+            settings.update_user_settings::<EditorSettings>(cx, |settings| {
+                settings.save_non_dirty_buffers = Some(false);
+            });
+        });
 
         fake_server.set_request_handler::<lsp::request::Formatting, _, _>(move |_, _| async move {
-            panic!("Should not be invoked on non-dirty buffer");
+            panic!("Should not be invoked on non-dirty buffer when configured so");
         });
         let save = editor
             .update_in(cx, |editor, window, cx| {
@@ -9099,8 +9104,19 @@ async fn test_document_format_during_save(cx: &mut TestAppContext) {
             .unwrap();
         cx.executor().start_waiting();
         save.await;
+
+        assert_eq!(
+            editor.update(cx, |editor, cx| editor.text(cx)),
+            "one\ntwo\nthree\n"
+        );
+        assert!(!cx.read(|cx| editor.is_dirty(cx)));
     }
 
+    cx.update_global::<SettingsStore, _>(|settings, cx| {
+        settings.update_user_settings::<EditorSettings>(cx, |settings| {
+            settings.save_non_dirty_buffers = Some(false);
+        });
+    });
     // Set rust language override and assert overridden tabsize is sent to language server
     update_test_language_settings(cx, |settings| {
         settings.languages.insert(
@@ -9438,19 +9454,35 @@ async fn test_range_format_during_save(cx: &mut TestAppContext) {
     );
     assert!(!cx.read(|cx| editor.is_dirty(cx)));
 
-    // For non-dirty buffer, no formatting request should be sent
+    // For non-dirty buffer, a formatting request should be sent anyway with the default settings
+    // where non-dirty singleton buffers are saved and formatted anyway.
     let save = editor
         .update_in(cx, |editor, window, cx| {
             editor.save(true, project.clone(), window, cx)
         })
         .unwrap();
-    let _pending_format_request = fake_server
-        .set_request_handler::<lsp::request::RangeFormatting, _, _>(move |_, _| async move {
-            panic!("Should not be invoked on non-dirty buffer");
+    fake_server
+        .set_request_handler::<lsp::request::RangeFormatting, _, _>(move |params, _| async move {
+            assert_eq!(
+                params.text_document.uri,
+                lsp::Url::from_file_path(path!("/file.rs")).unwrap()
+            );
+            assert_eq!(params.options.tab_size, 4);
+            Ok(Some(vec![lsp::TextEdit::new(
+                lsp::Range::new(lsp::Position::new(0, 3), lsp::Position::new(1, 0)),
+                ", ".to_string(),
+            )]))
         })
-        .next();
+        .next()
+        .await;
+    cx.executor().advance_clock(super::FORMAT_TIMEOUT);
     cx.executor().start_waiting();
     save.await;
+    assert_eq!(
+        editor.update(cx, |editor, cx| editor.text(cx)),
+        "one, two\nthree\n"
+    );
+    assert!(!cx.read(|cx| editor.is_dirty(cx)));
 
     // Set Rust language override and assert overridden tabsize is sent to language server
     update_test_language_settings(cx, |settings| {

crates/editor/src/items.rs 🔗

@@ -816,22 +816,23 @@ impl Item for Editor {
             .into_iter()
             .map(|handle| handle.read(cx).base_buffer().unwrap_or(handle.clone()))
             .collect::<HashSet<_>>();
-        cx.spawn_in(window, async move |this, cx| {
+        let save_non_dirty_buffers = self.save_non_dirty_buffers(cx);
+        cx.spawn_in(window, async move |editor, cx| {
             if format {
-                this.update_in(cx, |editor, window, cx| {
-                    editor.perform_format(
-                        project.clone(),
-                        FormatTrigger::Save,
-                        FormatTarget::Buffers,
-                        window,
-                        cx,
-                    )
-                })?
-                .await?;
+                editor
+                    .update_in(cx, |editor, window, cx| {
+                        editor.perform_format(
+                            project.clone(),
+                            FormatTrigger::Save,
+                            FormatTarget::Buffers,
+                            window,
+                            cx,
+                        )
+                    })?
+                    .await?;
             }
 
-            if buffers.len() == 1 {
-                // Apply full save routine for singleton buffers, to allow to `touch` the file via the editor.
+            if save_non_dirty_buffers {
                 project
                     .update(cx, |project, cx| project.save_buffers(buffers, cx))?
                     .await?;