feat: choose writing strategy

helix-view/src/document.rs

@@ -66,6 +66,15 @@ pub enum Mode {
     Insert = 2,
 }
 
+#[derive(Debug, Default, Copy, Clone, PartialEq, Eq, Serialize, serde::Deserialize)]
+#[serde(rename_all = "kebab-case", deny_unknown_fields)]
+pub enum WritingStrategy {
+    #[default]
+    MoveBackup,
+    CopyBackup,
+    Overwrite,
+}
+
 impl Display for Mode {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
@@ -952,6 +961,7 @@ impl Document {
         let encoding_with_bom_info = (self.encoding, self.has_bom);
         let last_saved_time = self.last_saved_time;
 
+        let writing_strategy_config = self.config.load().writing_strategy;
         // We encode the file according to the `Document`'s encoding.
         let future = async move {
             use tokio::fs;
@@ -995,9 +1005,19 @@ impl Document {
                 ));
             }
 
+            let writing_strategy = {
                 // Assume it is a hardlink to prevent data loss if the metadata cant be read (e.g. on certain Windows configurations)
                 let is_hardlink = helix_stdx::faccess::hardlink_count(&write_path).unwrap_or(2) > 1;
-            let backup = if path.exists() {
+
+                if is_hardlink {
+                    WritingStrategy::CopyBackup
+                } else {
+                    writing_strategy_config
+                }
+            };
+
+            let backup = match writing_strategy {
+                WritingStrategy::MoveBackup | WritingStrategy::CopyBackup if path.exists() => {
                     let path_ = write_path.clone();
                     // hacks: we use tempfile to handle the complex task of creating
                     // non clobbered temporary path for us we don't want
@@ -1005,28 +1025,31 @@ impl Document {
                     // since the path doesn't exist yet, we just want
                     // the path
                     tokio::task::spawn_blocking(move || -> Option<PathBuf> {
-                    let mut builder = tempfile::Builder::new();
+                        let backup_path = tempfile::Builder::new()
-                    builder.prefix(path_.file_name()?).suffix(".bck");
+                            .prefix(path_.file_name()?)
-
+                            .suffix(".bck")
-                    let backup_path = if is_hardlink {
+                            .make_in(path_.parent()?, |backup| {
-                        builder
+                                match writing_strategy {
-                            .make_in(path_.parent()?, |backup| std::fs::copy(&path_, backup))
+                                    WritingStrategy::CopyBackup => {
+                                        std::fs::copy(&path_, backup)?;
+                                    }
+                                    WritingStrategy::MoveBackup => {
+                                        std::fs::rename(&path_, backup)?;
+                                    }
+                                    WritingStrategy::Overwrite => unreachable!(),
+                                }
+                                Ok(())
+                            })
                             .ok()?
-                            .into_temp_path()
+                            .into_temp_path();
-                    } else {
-                        builder
-                            .make_in(path_.parent()?, |backup| std::fs::rename(&path_, backup))
-                            .ok()?
-                            .into_temp_path()
-                    };
 
                         backup_path.keep().ok()
                     })
                     .await
                     .ok()
                     .flatten()
-            } else {
+                }
-                None
+                _ => None,
             };
 
             let write_result: anyhow::Result<_> = async {
@@ -1037,46 +1060,49 @@ impl Document {
             }
             .await;
 
-            let save_time = match fs::metadata(&write_path).await {
+            let save_time = fs::metadata(&write_path)
-                Ok(metadata) => metadata.modified().map_or(SystemTime::now(), |mtime| mtime),
+                .await
-                Err(_) => SystemTime::now(),
+                .and_then(|metadata| metadata.modified())
-            };
+                .unwrap_or_else(|_| SystemTime::now());
 
+            if let Err(err) = write_result {
                 if let Some(backup) = backup {
-                if is_hardlink {
+                    match writing_strategy {
-                    let mut delete = true;
+                        WritingStrategy::CopyBackup => {
-                    if write_result.is_err() {
                             // Restore backup
-                        let _ = tokio::fs::copy(&backup, &write_path).await.map_err(|e| {
+                            if let Err(e) = tokio::fs::copy(&backup, &write_path).await {
-                            delete = false;
                                 log::error!("Failed to restore backup on write failure: {e}")
-                        });
                             }
-
-                    if delete {
-                        // Delete backup
-                        let _ = tokio::fs::remove_file(backup)
-                            .await
-                            .map_err(|e| log::error!("Failed to remove backup file on write: {e}"));
                         }
-                } else if write_result.is_err() {
+                        WritingStrategy::MoveBackup => {
                             // restore backup
-                    let _ = tokio::fs::rename(&backup, &write_path)
+                            if let Err(e) = tokio::fs::rename(&backup, &write_path).await {
-                        .await
+                                log::error!("Failed to restore backup on write failure: {e}");
-                        .map_err(|e| log::error!("Failed to restore backup on write failure: {e}"));
+                            }
+                        }
+                        WritingStrategy::Overwrite => unreachable!(),
+                    }
                 } else {
-                    // copy metadata and delete backup
+                    log::error!(
+                        "Failed to restore backup on write failure (backup doesn't exist) for write error: {err}"
+                    );
+                }
+            } else if let Some(backup) = backup {
+                // backup exists & successfully saved. delete backup
+                if writing_strategy == WritingStrategy::MoveBackup {
+                    // the file is newly created one, therefore the metadata must be copied
+                    let backup = backup.clone();
                     let _ = tokio::task::spawn_blocking(move || {
-                        let _ = copy_metadata(&backup, &write_path)
+                        if let Err(e) = copy_metadata(&backup, &write_path) {
-                            .map_err(|e| log::error!("Failed to copy metadata on write: {e}"));
+                            log::error!("Failed to copy metadata on write: {e}");
-                        let _ = std::fs::remove_file(backup)
+                        }
-                            .map_err(|e| log::error!("Failed to remove backup file on write: {e}"));
                     })
                     .await;
                 }
+                if let Err(e) = tokio::fs::remove_file(backup).await {
+                    log::error!("Failed to remove backup file on write: {e}");
+                }
             }
-
-            write_result?;
 
             let event = DocumentSavedEvent {
                 revision: current_rev,

helix-view/src/editor.rs

@@ -3,6 +3,7 @@ use crate::{
     clipboard::ClipboardProvider,
     document::{
         DocumentOpenError, DocumentSavedEventFuture, DocumentSavedEventResult, Mode, SavePoint,
+        WritingStrategy,
     },
     events::{DocumentDidClose, DocumentDidOpen, DocumentFocusLost},
     graphics::{CursorKind, Rect},
@@ -370,6 +371,7 @@ pub struct Config {
     /// Whether to read settings from [EditorConfig](https://editorconfig.org) files. Defaults to
     /// `true`.
     pub editor_config: bool,
+    pub writing_strategy: WritingStrategy,
 }
 
 #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, Eq, PartialOrd, Ord)]
@@ -1017,6 +1019,7 @@ impl Default for Config {
             end_of_line_diagnostics: DiagnosticFilter::Disable,
             clipboard_provider: ClipboardProvider::default(),
             editor_config: true,
+            writing_strategy: WritingStrategy::default(),
         }
     }
 }