Repository: helix-editor/helix (language: rust)

Issue:
Decouple the `auto-save` and `auto-format` options
I’d like to suggest that `auto-format` be triggered only by manual saving, rather than being tied to `auto-save`.

Reasons:
1. Code formatting doesn’t need to happen frequently.
2. When both `auto-save` and `auto-format` are enabled, formatting occurs while writing code, which can be disruptive.

Decoupling these options would improve the user experience and potentially resolve issues like #11237 and #11966.


Discussion / hints:
Would it be better if there was an option to configure this functionality or the behavior being hardcoded into helix?
I think an option like `format-on-auto-save` that defaults to `false` would be great.
However, I still don't think it's necessary to format every time an auto-save triggers.
> However, I still don't think it's necessary to format every time an auto-save triggers.

We'll let the user choose the behavior through config file anyway

Fix patch:
diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs
index 9c48bf72382e..c5c53acd5ce7 100644
--- a/helix-term/src/commands/typed.rs
+++ b/helix-term/src/commands/typed.rs
@@ -678,10 +678,16 @@ pub(super) fn buffers_remaining_impl(editor: &mut Editor) -> anyhow::Result<()>
     Ok(())
 }
 
+#[derive(Debug, Clone, Copy)]
+pub struct WriteAllOptions {
+    pub force: bool,
+    pub write_scratch: bool,
+    pub auto_format: bool,
+}
+
 pub fn write_all_impl(
     cx: &mut compositor::Context,
-    force: bool,
-    write_scratch: bool,
+    options: WriteAllOptions,
 ) -> anyhow::Result<()> {
     let mut errors: Vec<&'static str> = Vec::new();
     let config = cx.editor.config();
@@ -699,7 +705,7 @@ pub fn write_all_impl(
                 return None;
             }
             if doc.path().is_none() {
-                if write_scratch {
+                if options.write_scratch {
                     errors.push("cannot write a buffer without a filename");
                 }
                 return None;
@@ -722,14 +728,14 @@ pub fn write_all_impl(
         // Save an undo checkpoint for any outstanding changes.
         doc.append_changes_to_history(view);
 
-        let fmt = if config.auto_format {
+        let fmt = if options.auto_format && config.auto_format {
             doc.auto_format().map(|fmt| {
                 let callback = make_format_callback(
                     doc_id,
                     doc.version(),
                     target_view,
                     fmt,
-                    Some((None, force)),
+                    Some((None, options.force)),
                 );
                 jobs.add(Job::with_callback(callback).wait_before_exiting());
             })
@@ -738,11 +744,11 @@ pub fn write_all_impl(
         };
 
         if fmt.is_none() {
-            cx.editor.save::<PathBuf>(doc_id, None, force)?;
+            cx.editor.save::<PathBuf>(doc_id, None, options.force)?;
         }
     }
 
-    if !errors.is_empty() && !force {
+    if !errors.is_empty() && !options.force {
         bail!("{:?}", errors);
     }
 
@@ -758,7 +764,14 @@ fn write_all(
         return Ok(());
     }
 
-    write_all_impl(cx, false, true)
+    write_all_impl(
+        cx,
+        WriteAllOptions {
+            force: false,
+            write_scratch: true,
+            auto_format: true,
+        },
+    )
 }
 
 fn force_write_all(
@@ -770,7 +783,14 @@ fn force_write_all(
         return Ok(());
     }
 
-    write_all_impl(cx, true, true)
+    write_all_impl(
+        cx,
+        WriteAllOptions {
+            force: true,
+            write_scratch: true,
+            auto_format: true,
+        },
+    )
 }
 
 fn write_all_quit(
@@ -781,7 +801,14 @@ fn write_all_quit(
     if event != PromptEvent::Validate {
         return Ok(());
     }
-    write_all_impl(cx, false, true)?;
+    write_all_impl(
+        cx,
+        WriteAllOptions {
+            force: false,
+            write_scratch: true,
+            auto_format: true,
+        },
+    )?;
     quit_all_impl(cx, false)
 }
 
@@ -793,7 +820,14 @@ fn force_write_all_quit(
     if event != PromptEvent::Validate {
         return Ok(());
     }
-    let _ = write_all_impl(cx, true, true);
+    let _ = write_all_impl(
+        cx,
+        WriteAllOptions {
+            force: true,
+            write_scratch: true,
+            auto_format: true,
+        },
+    );
     quit_all_impl(cx, true)
 }
 
diff --git a/helix-term/src/handlers/auto_save.rs b/helix-term/src/handlers/auto_save.rs
index 4e154df808a7..47e2ecfdf13d 100644
--- a/helix-term/src/handlers/auto_save.rs
+++ b/helix-term/src/handlers/auto_save.rs
@@ -87,7 +87,13 @@ fn request_auto_save(editor: &mut Editor) {
         jobs: &mut Jobs::new(),
     };
 
-    if let Err(e) = commands::typed::write_all_impl(context, false, false) {
+    let options = commands::WriteAllOptions {
+        force: false,
+        write_scratch: false,
+        auto_format: false,
+    };
+
+    if let Err(e) = commands::typed::write_all_impl(context, options) {
         context.editor.set_error(format!("{}", e));
     }
 }
diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs
index 6fecd512b5a0..d8500ed4d7ea 100644
--- a/helix-term/src/ui/editor.rs
+++ b/helix-term/src/ui/editor.rs
@@ -1522,7 +1522,12 @@ impl Component for EditorView {
             }
             Event::FocusLost => {
                 if context.editor.config().auto_save.focus_lost {
-                    if let Err(e) = commands::typed::write_all_impl(context, false, false) {
+                    let options = commands::WriteAllOptions {
+                        force: false,
+                        write_scratch: false,
+                        auto_format: false,
+                    };
+                    if let Err(e) = commands::typed::write_all_impl(context, options) {
                         context.editor.set_error(format!("{}", e));
                     }
                 }


Test patch: