From 10eedfacc1fba607fa153d3686a2d0335d3b2146 Mon Sep 17 00:00:00 2001 From: Mason Mac <67773714+kirawi@users.noreply.github.com> Date: Tue, 30 Jul 2024 12:21:17 -0400 Subject: [PATCH] Ensure that temporary files written have the same metadata as the original --- Cargo.lock | 1 + helix-loader/src/lib.rs | 15 ++ helix-stdx/src/faccess.rs | 196 +++++++++++++++++++++----- helix-stdx/src/path.rs | 41 ++++++ helix-view/Cargo.toml | 1 + helix-view/src/document.rs | 279 ++++++++++++++++++++++++++++--------- helix-view/src/editor.rs | 30 ++++ 7 files changed, 466 insertions(+), 97 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7e80230d..7e445ba6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1457,6 +1457,7 @@ dependencies = [ "once_cell", "parking_lot", "rustix", + "same-file", "serde", "serde_json", "slotmap", diff --git a/helix-loader/src/lib.rs b/helix-loader/src/lib.rs index 0e7c134d..3d754cb8 100644 --- a/helix-loader/src/lib.rs +++ b/helix-loader/src/lib.rs @@ -132,6 +132,21 @@ pub fn cache_dir() -> PathBuf { path } +pub fn state_dir() -> PathBuf { + #[cfg(unix)] + { + let strategy = choose_base_strategy().expect("Unable to find the state directory!"); + let mut path = strategy.state_dir(); + path.push("helix"); + path + } + + #[cfg(windows)] + { + cache_dir() + } +} + pub fn config_file() -> PathBuf { CONFIG_FILE.get().map(|path| path.to_path_buf()).unwrap() } diff --git a/helix-stdx/src/faccess.rs b/helix-stdx/src/faccess.rs index e4c3daf2..1b305767 100644 --- a/helix-stdx/src/faccess.rs +++ b/helix-stdx/src/faccess.rs @@ -1,5 +1,6 @@ //! From +use std::fs::File; use std::io; use std::path::Path; @@ -24,7 +25,10 @@ bitflags! { mod imp { use super::*; - use rustix::fs::Access; + use rustix::{ + fd::AsFd, + fs::{Access, OpenOptionsExt}, + }; use std::os::unix::fs::{MetadataExt, PermissionsExt}; pub fn access(p: &Path, mode: AccessMode) -> io::Result<()> { @@ -50,13 +54,20 @@ mod imp { Ok(()) } - fn chown(p: &Path, uid: Option, gid: Option) -> io::Result<()> { + pub fn chown(p: &Path, uid: Option, gid: Option) -> io::Result<()> { let uid = uid.map(|n| unsafe { rustix::fs::Uid::from_raw(n) }); let gid = gid.map(|n| unsafe { rustix::fs::Gid::from_raw(n) }); rustix::fs::chown(p, uid, gid)?; Ok(()) } + pub fn fchown(fd: impl AsFd, uid: Option, gid: Option) -> io::Result<()> { + let uid = uid.map(|n| unsafe { rustix::fs::Uid::from_raw(n) }); + let gid = gid.map(|n| unsafe { rustix::fs::Gid::from_raw(n) }); + rustix::fs::fchown(fd, uid, gid)?; + Ok(()) + } + pub fn copy_metadata(from: &Path, to: &Path) -> io::Result<()> { let from_meta = std::fs::metadata(from)?; let to_meta = std::fs::metadata(to)?; @@ -79,15 +90,34 @@ mod imp { let metadata = p.metadata()?; Ok(metadata.nlink()) } + + pub fn create_copy_mode(from: &Path, to: &Path) -> io::Result { + let from_meta = std::fs::metadata(from)?; + let mode = from_meta.permissions().mode(); + let file = std::fs::OpenOptions::new() + .mode(mode) + .read(true) + .write(true) + .create_new(true) + .open(to)?; + + // Change ownership + let from_meta = std::fs::metadata(from)?; + let uid = from_meta.uid(); + let gid = from_meta.gid(); + fchown(file.as_fd(), Some(uid), Some(gid))?; + Ok(file) + } } -// Licensed under MIT from faccess except for `chown`, `copy_metadata` and `is_acl_inherited` #[cfg(windows)] mod imp { - use windows_sys::Win32::Foundation::{CloseHandle, LocalFree, ERROR_SUCCESS, HANDLE}; + use windows_sys::Win32::Foundation::{ + CloseHandle, LocalFree, ERROR_SUCCESS, GENERIC_READ, GENERIC_WRITE, HANDLE, + }; use windows_sys::Win32::Security::Authorization::{ - GetNamedSecurityInfoW, SetNamedSecurityInfoW, SE_FILE_OBJECT, + GetNamedSecurityInfoW, SetSecurityInfo, SE_FILE_OBJECT, }; use windows_sys::Win32::Security::{ AccessCheck, AclSizeInformation, GetAce, GetAclInformation, GetSidIdentifierAuthority, @@ -100,7 +130,8 @@ mod imp { }; use windows_sys::Win32::Storage::FileSystem::{ GetFileInformationByHandle, BY_HANDLE_FILE_INFORMATION, FILE_ACCESS_RIGHTS, - FILE_ALL_ACCESS, FILE_GENERIC_EXECUTE, FILE_GENERIC_READ, FILE_GENERIC_WRITE, + FILE_ALL_ACCESS, FILE_GENERIC_EXECUTE, FILE_GENERIC_READ, FILE_GENERIC_WRITE, WRITE_DAC, + WRITE_OWNER, }; use windows_sys::Win32::System::Threading::{GetCurrentThread, OpenThreadToken}; @@ -110,6 +141,7 @@ mod imp { use std::os::windows::{ffi::OsStrExt, fs::OpenOptionsExt, io::AsRawHandle}; + // Licensed under MIT from faccess struct SecurityDescriptor { sd: PSECURITY_DESCRIPTOR, owner: PSID, @@ -128,6 +160,7 @@ mod imp { } impl SecurityDescriptor { + // Licensed under MIT from faccess fn for_path(p: &Path) -> io::Result { let path = std::fs::canonicalize(p)?; let pathos = path.into_os_string(); @@ -202,6 +235,7 @@ mod imp { } } + // Licensed under MIT from faccess struct ThreadToken(HANDLE); impl Drop for ThreadToken { fn drop(&mut self) { @@ -211,6 +245,7 @@ mod imp { } } + // Licensed under MIT from faccess impl ThreadToken { fn new() -> io::Result { unsafe { @@ -237,6 +272,7 @@ mod imp { } } + // Licensed under MIT from faccess // Based roughly on Tcl's NativeAccess() // https://github.com/tcltk/tcl/blob/2ee77587e4dc2150deb06b48f69db948b4ab0584/win/tclWinFile.c fn eaccess(p: &Path, mut mode: FILE_ACCESS_RIGHTS) -> io::Result<()> { @@ -330,6 +366,7 @@ mod imp { } } + // Licensed under MIT from faccess pub fn access(p: &Path, mode: AccessMode) -> io::Result<()> { let mut imode = 0; @@ -356,13 +393,8 @@ mod imp { } } - fn chown(p: &Path, sd: SecurityDescriptor) -> io::Result<()> { - let path = std::fs::canonicalize(p)?; - let pathos = path.as_os_str(); - let mut pathw = Vec::with_capacity(pathos.len() + 1); - pathw.extend(pathos.encode_wide()); - pathw.push(0); - + // SAFETY: It is the caller's responsibility to close the handle + fn chown(handle: HANDLE, sd: SecurityDescriptor) -> io::Result<()> { let mut owner = std::ptr::null_mut(); let mut group = std::ptr::null_mut(); let mut dacl = std::ptr::null(); @@ -387,8 +419,8 @@ mod imp { } let err = unsafe { - SetNamedSecurityInfoW( - pathw.as_ptr(), + SetSecurityInfo( + handle, SE_FILE_OBJECT, si, owner, @@ -405,9 +437,18 @@ mod imp { } } - pub fn copy_metadata(from: &Path, to: &Path) -> io::Result<()> { + pub fn copy_ownership(from: &Path, to: &Path) -> io::Result<()> { let sd = SecurityDescriptor::for_path(from)?; - chown(to, sd)?; + let to_file = std::fs::OpenOptions::new() + .read(true) + .access_mode(GENERIC_READ | GENERIC_WRITE | WRITE_OWNER | WRITE_DAC) + .open(to)?; + chown(to_file.as_raw_handle(), sd)?; + Ok(()) + } + + pub fn copy_metadata(from: &Path, to: &Path) -> io::Result<()> { + copy_ownership(from, to)?; let meta = std::fs::metadata(from)?; let perms = meta.permissions(); @@ -417,17 +458,42 @@ mod imp { Ok(()) } - pub fn hardlink_count(p: &Path) -> std::io::Result { - let file = std::fs::File::open(p)?; + fn file_info(p: &Path) -> io::Result { + let file = File::open(p)?; let handle = file.as_raw_handle(); let mut info: BY_HANDLE_FILE_INFORMATION = unsafe { std::mem::zeroed() }; if unsafe { GetFileInformationByHandle(handle, &mut info) } == 0 { - Err(std::io::Error::last_os_error()) + Err(io::Error::last_os_error()) } else { - Ok(info.nNumberOfLinks as u64) + Ok(info) } } + + pub fn hardlink_count(p: &Path) -> io::Result { + let n = file_info(p)?.nNumberOfLinks as u64; + Ok(n) + } + + pub fn create_copy_mode(from: &Path, to: &Path) -> io::Result { + let sd = SecurityDescriptor::for_path(from)?; + + // read/write still need to be set to true or `create_new` returns an error + let to_file = std::fs::OpenOptions::new() + .read(true) + .write(true) + .access_mode(GENERIC_READ | GENERIC_WRITE | WRITE_OWNER | WRITE_DAC) + .create_new(true) + .open(to)?; + + // Necessary because `security_attributes` is not exposed: https://github.com/rust-lang/libs-team/issues/314 + chown(to_file.as_raw_handle(), sd)?; + + let meta = std::fs::metadata(from)?; + let perms = meta.permissions(); + std::fs::set_permissions(to, perms)?; + Ok(to_file) + } } // Licensed under MIT from faccess except for `copy_metadata` @@ -453,14 +519,6 @@ mod imp { Err(io::Error::new(io::ErrorKind::NotFound, "Path not found")) } } - - pub fn copy_metadata(from: &path, to: &Path) -> io::Result<()> { - let meta = std::fs::metadata(from)?; - let perms = meta.permissions(); - std::fs::set_permissions(to, perms)?; - - Ok(()) - } } pub fn readonly(p: &Path) -> bool { @@ -471,10 +529,86 @@ pub fn readonly(p: &Path) -> bool { } } +pub fn hardlink_count(p: &Path) -> io::Result { + imp::hardlink_count(p) +} + pub fn copy_metadata(from: &Path, to: &Path) -> io::Result<()> { imp::copy_metadata(from, to) } -pub fn hardlink_count(p: &Path) -> io::Result { - imp::hardlink_count(p) +// /// Create a file copying permissions, uid, and gid of `from` at the the target destination `to` +// pub fn create_copy_mode(from: &Path, to: &Path) -> io::Result { +// imp::create_copy_mode(from, to) +// } + +#[cfg(windows)] +pub fn copy_ownership(from: &Path, to: &Path) -> io::Result<()> { + imp::copy_ownership(from, to) } + +/* +Neovim backup path function: +- If a backup is desired (would be disabled by a user if using a file watcher): + - Checks if user explicitly requested a copy + - Or automatically choose whether to copy or rename + - Offers options for: + - Breaking symlinks or hardlinks (not offered in Helix) + - Offers the ability to have a list of directories where the backup file is written: + - Default is: ".,$XDG_STATE_HOME/nvim/backup//" + - Offers ability to set backup extension +- For copy backup: + - If the file is a link, then the backup will have the name of the link +- Auto backup: + - Immediately set copy if: + - Is hardlink or symlink + - Then, tries to: + - Create a temporary file with the same permissions as the file to test if its ok to rename later + - If it fails, then set copy + - fchown created file + - If it fails or perms weren't copied, then set copy + - Delete test file + - Otherwise, will rename +- Break symlink/hardlink if requested +- Copy backup: + - If there is an error while creating the file, it will be propogated unless force write is true + - Try to create backup path in bdir: + - Tries first directory where this is possible + - If no directory exists, the last directory is created + - Filename is escaped and extension applied + - Check if backup already exists: + - Check if pre-existing file is a symlink to the original file (and don't attempt to create one) + - Dunno what p_bk is, but if false, it tries to create a different backup file path where each character before the extension is changed (if all attempts fail, then error) + - Copies file with UV_FS_COPYFILE_FICLONE + - Sets perm as os_setperm(*backupp, perm & 0777); + - On Unix: + - Attempts to set gid via chown: + - os_setperm(*backupp, (perm & 0707) | ((perm & 07) << 3) if fails + - Sets file time: + os_file_settime(*backupp, + (double)file_info_old->stat.st_atim.tv_sec, + (double)file_info_old->stat.st_mtim.tv_sec); + - On Windows, sets ACL + - Attempts to copy xattr if exists +- Rename backup: + - Backup is created by renaming original file: + - Don't if file is read-only and cpoptions has "W" flag + - Tries to find backup file name w/ bdir (similar to copy) + - Checks if a file with that name already exists: + - Attempts same method as copy backup to create a different filename + +Neovim write: +- On Unix: + - If using :w! and file was read-only, make it writable (if process uid is same as file): +- Reset read-only flag if overwriting +- Executes fsync (will not propogate error if storage does not support op) +- Copies xattr for non-copy backups +- If a rename backup is being performed: + - Check if uid and gid are same as original file, and set if they aren't + - Set perms +- Either way, copy perms from old file to new file +- Either way, if not a backup copy, also set ACL (method seems to not do anything?) +- On failure: + - If a copy, copy contents from copy to original file + - Otherwise, rename backup back to original path +*/ diff --git a/helix-stdx/src/path.rs b/helix-stdx/src/path.rs index 72b233cc..74390212 100644 --- a/helix-stdx/src/path.rs +++ b/helix-stdx/src/path.rs @@ -8,6 +8,7 @@ use std::{ ffi::OsString, ops::Range, path::{Component, Path, PathBuf, MAIN_SEPARATOR_STR}, + str::Utf8Error, }; use crate::env::current_working_dir; @@ -295,6 +296,46 @@ pub fn expand + ?Sized>(path: &T) -> Cow<'_, Path> { } } +fn os_str_as_bytes>(path: P) -> Vec { + let path = path.as_ref(); + + #[cfg(windows)] + return path.to_str().unwrap().into(); + + #[cfg(unix)] + return std::os::unix::ffi::OsStrExt::as_bytes(path).to_vec(); +} + +fn path_from_bytes(slice: &[u8]) -> Result { + #[cfg(windows)] + return Ok(PathBuf::from(std::str::from_utf8(slice)?)); + + #[cfg(unix)] + return Ok(PathBuf::from( + ::from_bytes(slice), + )); +} + +fn is_sep_byte(b: u8) -> bool { + if cfg!(windows) { + b == b'/' || b == b'\\' + } else { + b == b'/' + } +} + +/// Replaces all path separators in a path with % +pub fn escape_path(path: &Path) -> PathBuf { + let s = path.as_os_str().to_os_string(); + let mut bytes = os_str_as_bytes(&s); + for b in bytes.iter_mut() { + if is_sep_byte(*b) { + *b = b'%'; + } + } + path_from_bytes(&bytes).unwrap() +} + #[cfg(test)] mod tests { use std::{ diff --git a/helix-view/Cargo.toml b/helix-view/Cargo.toml index 6f71fa05..58ab50f6 100644 --- a/helix-view/Cargo.toml +++ b/helix-view/Cargo.toml @@ -29,6 +29,7 @@ anyhow = "1" crossterm = { version = "0.28", optional = true } tempfile = "3.14" +same-file = "1.0.1" # Conversion traits once_cell = "1.20" diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index fa089cda..d398b930 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -1,5 +1,5 @@ use anyhow::{anyhow, bail, Error}; -use arc_swap::access::DynAccess; +use arc_swap::access::{Access, DynAccess}; use arc_swap::ArcSwap; use futures_util::future::BoxFuture; use futures_util::FutureExt; @@ -10,7 +10,7 @@ use helix_core::encoding::Encoding; use helix_core::syntax::{Highlight, LanguageServerFeature}; use helix_core::text_annotations::{InlineAnnotation, Overlay}; use helix_lsp::util::lsp_pos_to_pos; -use helix_stdx::faccess::{copy_metadata, readonly}; +use helix_stdx::faccess::readonly; use helix_vcs::{DiffHandle, DiffProviderRegistry}; use thiserror; @@ -37,6 +37,7 @@ use helix_core::{ ChangeSet, Diagnostic, LineEnding, Range, Rope, RopeBuilder, Selection, Syntax, Transaction, }; +use crate::editor::BackupConfig; use crate::{ editor::Config, events::{DocumentDidChange, SelectionDidChange}, @@ -130,6 +131,132 @@ pub enum DocumentOpenError { IoError(#[from] io::Error), } +struct Backup { + copy: bool, + path: PathBuf, +} + +impl Backup { + async fn from(p: PathBuf, config: &BackupConfig) -> Result { + use crate::editor::BackupKind; + use helix_stdx::faccess::*; + + // This fn won't be called if it's none or it doesn't exist + if config.kind == BackupKind::None || !p.exists() { + unreachable!(); + } + + let mut copy = config.kind == BackupKind::Copy; + + // Do not rename iff: + // - it is a hardlink + // - it is a symlink + // - we don't have file create perms for the dir + // TODO: also set copy when perms can't be set or metadata can't be read + if !copy { + // Conservatively assume it is a hardlink if we can't read metadata + let is_hardlink = { + let p_ = p.clone(); + tokio::task::spawn_blocking(move || hardlink_count(&p_).unwrap_or(2)).await? > 1 + }; + let is_symlink = tokio::fs::metadata(&p).await?.is_symlink(); + if is_hardlink || is_symlink { + copy = true; + } else { + // Check if we have write permissions by creating a temporary file + let mut builder = tempfile::Builder::new(); + // TODO: Need a way to create cross-platform perms with more granularity + // builder.permissions() + if let Ok(f) = builder.tempfile() { + // Check if we have perms to set perms + if copy_metadata(&p, f.path()).is_err() { + copy = true; + } + } else { + copy = true; + } + } + } + + // Look for valid backup directory + // Check if: + // - directory is not writable + // - path is a directory + // - path exists + let mut dir_exists = false; + let escaped_p = helix_stdx::path::escape_path(&p); + 'outer: for dir in config.directories.iter().filter(|p| p.is_dir()) { + let ext = config.extension.as_str(); + let bck_base_path = &dir.join(&escaped_p); + let mut backup = bck_base_path.join(&ext); + + // NOTE: Should we just overwrite regardless? + // If the backup file already exists, we'll try to add a number before the extension + // until we're done + // NOTE: u8 since if we need more than 256, there might be an issue + let mut n: u8 = 1; + while backup.exists() { + backup = bck_base_path.join(n.to_string()).join(&ext); + let Some(n) = n.checked_add(1) else { + continue 'outer; + }; + } + + if copy { + // Create the copy backup + // TODO: How handle error? + tokio::fs::copy(&p, &backup).await?; + + #[cfg(unix)] + { + let mut meta = tokio::fs::metadata(&p).await?; + let mut perms = meta.permissions(); + + // Strip s-bit + perms.set_mode(perms.mode() & 0o0777); + + let from_gid = from_meta.gid(); + let to_gid = to_meta.gid(); + + // If chown fails, se the protection bits for the roup the same as the perm bits for others + if from_gid != to_gid && chown(to, None, Some(from_gid)).is_err() { + let new_perms = (perms.mode() & 0o0707) | ((perms.mode() & 0o07) << 3); + perms.set_mode(new_perms); + } + std::fs::set_permissions(&backup, perms)?; + // TODO: Set time + // TODO: set xattr via rustix + } + + #[cfg(windows)] + { + let backup_ = backup.clone(); + tokio::task::spawn_blocking(move || -> std::io::Result<()> { + copy_ownership(&p, &backup_)?; + Ok(()) + }) + .await?; + } + + return Ok(Self { + copy: true, + path: backup, + }); + } else { + tokio::fs::rename(p, &backup).await?; + return Ok(Self { + copy: false, + path: backup, + }); + } + } + // TODO: Try to initialize last dir if none of the dirs exist + + // TODO + bail!("err"); + } +} + pub struct Document { pub(crate) id: DocumentId, text: Rope, @@ -892,6 +1019,8 @@ impl Document { let encoding_with_bom_info = (self.encoding, self.has_bom); let last_saved_time = self.last_saved_time; + let bck_config = self.config.clone().load().backup.clone(); + // We encode the file according to the `Document`'s encoding. let future = async move { use tokio::fs; @@ -935,84 +1064,102 @@ impl Document { )); } - // 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() { - 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 - // the whole automatically delete path on drop thing - // since the path doesn't exist yet, we just want - // the path - tokio::task::spawn_blocking(move || -> Option { - let mut builder = tempfile::Builder::new(); - builder.prefix(path_.file_name()?).suffix(".bck"); + // Use a backup file + let mut bck = None; + let write_result = if bck_config.kind != crate::editor::BackupKind::None { + bck = Some(Backup::from(write_path.clone(), &bck_config).await?); + let bck = bck.unwrap(); - let backup_path = if is_hardlink { - builder - .make_in(path_.parent()?, |backup| std::fs::copy(&path_, backup)) - .ok()? - .into_temp_path() - } else { - builder - .make_in(path_.parent()?, |backup| std::fs::rename(&path_, backup)) - .ok()? - .into_temp_path() - }; + // SECURITY: Ensure that the created file has the same perms as the original file + let dst = if !bck.copy { + let from_meta = tokio::fs::metadata(&write_path).await?; + let mut open_opt = tokio::fs::OpenOptions::new() + .read(true) + .write(true) + .create_new(true); - backup_path.keep().ok() - }) - .await - .ok() - .flatten() - } else { - None - }; + #[cfg(unix)] + { + let mode = from_meta.permissions().mode(); + open_opt.mode(mode); + } + + let file = open_opt.open(&bck.path).await?; + let to_meta = file.metadata().await?; + + #[cfg(unix)] + { + // TODO: set gid/uid via fchown + } + + #[cfg(windows)] + { + let from = write_path.clone(); + let to = bck.path.clone(); + tokio::task::spawn_blocking(move || { + helix_stdx::faccess::copy_ownership(&from, &to)?; + Ok(()) + }) + .await?; + } + file + } else { + // SECURITY: Backup copy already exists + tokio::fs::File::create(&write_path).await? + }; - let write_result: anyhow::Result<_> = async { - let mut dst = tokio::fs::File::create(&write_path).await?; to_writer(&mut dst, encoding_with_bom_info, &text).await?; dst.sync_all().await?; Ok(()) - } - .await; + } else { + let dst = tokio::fs::File::create(&write_path).await?; + to_writer(&mut dst, encoding_with_bom_info, &text).await?; + dst.sync_all().await?; + Ok(()) + }; let save_time = match fs::metadata(&write_path).await { Ok(metadata) => metadata.modified().map_or(SystemTime::now(), |mtime| mtime), Err(_) => SystemTime::now(), }; - if let Some(backup) = backup { - if is_hardlink { - let mut delete = true; - if write_result.is_err() { - // Restore backup - let _ = tokio::fs::copy(&backup, &write_path).await.map_err(|e| { - delete = false; - log::error!("Failed to restore backup on write failure: {e}") - }); - } - - if delete { - // Delete backup - let _ = tokio::fs::remove_file(backup) + if let Some(bck) = bck { + /* + - If original file no longer exists, then backup is renamed to original file + - And the timestamp is preserved by setting timestmaps to prior to write + - Then backup is deleted + */ + let mut delete_bck = true; + if write_result.is_err() { + // If original file no longer exists, then backup is renamed to original file + if !write_path.exists() { + if !tokio::fs::rename(&bck.path, &write_path) .await - .map_err(|e| log::error!("Failed to remove backup file on write: {e}")); + .map_err(|e| { + delete_bck = false; + log::error!("Failed to restore backup on write failure: {e}") + }) + .is_err() + { + // TODO: Set timestamps to prior to write + } + } else { + if bck.copy { + // Restore backup from copy + let _ = tokio::fs::copy(&bck.path, &write_path).await.map_err(|e| { + delete_bck = false; + log::error!("Failed to restore backup on write failure: {e}") + }); + } else { + // restore backup + let _ = tokio::fs::rename(&bck.path, &write_path) + .await + .map_err(|e| { + delete_bck = false; + log::error!("Failed to restore backup on write failure: {e}") + }); + } } - } else if write_result.is_err() { - // restore backup - let _ = tokio::fs::rename(&backup, &write_path) - .await - .map_err(|e| log::error!("Failed to restore backup on write failure: {e}")); - } else { - // copy metadata and delete backup - let _ = tokio::task::spawn_blocking(move || { - let _ = copy_metadata(&backup, &write_path) - .map_err(|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; } } diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 174190e5..25c06b21 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -355,6 +355,35 @@ pub struct Config { pub end_of_line_diagnostics: DiagnosticFilter, // Set to override the default clipboard provider pub clipboard_provider: ClipboardProvider, + pub backup: BackupConfig, +} + +#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, Eq, PartialOrd, Ord)] +#[serde(rename_all = "kebab-case", default)] +pub struct BackupConfig { + pub kind: BackupKind, + pub directories: Vec, + pub extension: String, +} + +impl Default for BackupConfig { + fn default() -> Self { + Self { + kind: BackupKind::Auto, + // TODO: Prevent empty vector + directories: vec![helix_loader::state_dir().join("backup")], + // TODO: Prevent empty strings + extension: String::from("bck"), + } + } +} + +#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, Eq, PartialOrd, Ord)] +#[serde(rename_all = "kebab-case")] +pub enum BackupKind { + None, + Copy, + Auto, } #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, Eq, PartialOrd, Ord)] @@ -995,6 +1024,7 @@ impl Default for Config { inline_diagnostics: InlineDiagnosticsConfig::default(), end_of_line_diagnostics: DiagnosticFilter::Disable, clipboard_provider: ClipboardProvider::default(), + backup: BackupConfig::default(), } } }