diff --git a/helix-stdx/src/path.rs b/helix-stdx/src/path.rs index 47cca2e9d325..6793df28072e 100644 --- a/helix-stdx/src/path.rs +++ b/helix-stdx/src/path.rs @@ -214,12 +214,12 @@ fn os_str_as_bytes>(path: P) -> Vec { fn path_from_bytes(slice: &[u8]) -> Result { #[cfg(windows)] - return Ok(PathBuf::from(std::str::from_utf8(slice)?)); + let res = PathBuf::from(std::str::from_utf8(slice)?); #[cfg(unix)] - return Ok(PathBuf::from( - ::from_bytes(slice), - )); + let res = PathBuf::from(::from_bytes(slice)); + + Ok(res) } fn is_sep_byte(b: u8) -> bool { @@ -242,6 +242,20 @@ pub fn escape_path(path: &Path) -> PathBuf { path_from_bytes(&bytes).unwrap() } +pub fn add_extension<'a, S: AsRef>(p: &'a Path, extension: S) -> Cow<'a, Path> { + let new = extension.as_ref(); + if new.is_empty() { + Cow::Borrowed(p) + } else { + let Some(mut ext) = p.extension().map(std::ffi::OsStr::to_owned) else { + return Cow::Borrowed(p); + }; + ext.push("."); + ext.push(new); + Cow::Owned(p.with_extension(ext)) + } +} + #[cfg(test)] mod tests { use std::{ diff --git a/helix-stdx/tests/path.rs b/helix-stdx/tests/path.rs index cc3c15cba65c..0bf663701ead 100644 --- a/helix-stdx/tests/path.rs +++ b/helix-stdx/tests/path.rs @@ -1,10 +1,9 @@ #![cfg(windows)] -use std::{ - env::set_current_dir, - error::Error, - path::{Component, Path, PathBuf}, -}; +use std::{env::set_current_dir, error::Error, path::Component}; + +#[cfg(unix)] +use std::path::{Path, PathBuf}; use helix_stdx::path; use tempfile::Builder; diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 97015fbbe11a..3dd496602699 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -159,7 +159,7 @@ impl Backup { 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(); + let is_symlink = tokio::fs::symlink_metadata(&p).await?.is_symlink(); if is_hardlink || is_symlink { copy = true; } else { @@ -211,7 +211,9 @@ impl Backup { '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.with_extension(&ext); + + // NOTE: `escaped_p` will make dot files appear to be extensions, so we need to append + let mut backup = helix_stdx::path::add_extension(&bck_base_path, ext).into_owned(); // NOTE: Should we just overwrite regardless? // If the backup file already exists, we'll try to add a number before the extension @@ -219,11 +221,12 @@ impl Backup { // 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.with_extension(format!("{n}.{ext}")); + backup = helix_stdx::path::add_extension(&bck_base_path, format!("{n}.{ext}")) + .into_owned(); if n == u8::MAX { continue 'outer; } - n = n + 1; + n += 1; } if copy { @@ -231,11 +234,11 @@ impl Backup { // TODO: How handle error? tokio::fs::copy(&p, &backup).await?; + let from_meta = tokio::fs::metadata(&p).await?; #[cfg(unix)] { use std::os::unix::fs::{MetadataExt, PermissionsExt}; - let from_meta = tokio::fs::metadata(&p).await?; let mut perms = from_meta.permissions(); // Strip s-bit @@ -252,12 +255,11 @@ impl Backup { } std::fs::set_permissions(&backup, perms)?; - let atime = FileTime::from_last_access_time(&from_meta); - let mtime = FileTime::from_last_modification_time(&from_meta); - filetime::set_file_times(&backup, atime, mtime)?; - copy_xattr(&p, &backup)?; } + let atime = FileTime::from_last_access_time(&from_meta); + let mtime = FileTime::from_last_modification_time(&from_meta); + filetime::set_file_times(&backup, atime, mtime)?; #[cfg(windows)] { @@ -1075,7 +1077,11 @@ impl Document { } // Use a backup file - let meta = tokio::fs::metadata(&path).await?; + let meta = if path.exists() { + Some(tokio::fs::metadata(&path).await?) + } else { + None + }; let mut bck = None; let write_result: Result<(), Error> = async { if path.exists() && bck_config.kind != crate::editor::BackupKind::None { @@ -1089,6 +1095,9 @@ impl Document { if let Some(ref bck) = bck { // SECURITY: Ensure that the created file has the same perms as the original file let mut dst = if !bck.copy { + #[cfg(unix)] + let meta = meta.as_ref().unwrap(); + let mut open_opt = tokio::fs::OpenOptions::new(); open_opt.read(true).write(true).create_new(true); @@ -1164,30 +1173,29 @@ impl Document { .is_ok() { // Reset timestamps + let meta = meta.as_ref().unwrap(); let atime = FileTime::from_last_access_time(&meta); let mtime = FileTime::from_last_modification_time(&meta); filetime::set_file_times(&path, atime, mtime)?; } + } else if bck.copy { + // Restore backup from copy + let _ = tokio::fs::copy(&bck.path, &path).await.map_err(|e| { + delete_bck = false; + log::error!("Failed to restore backup on write failure: {e}") + }); } else { - if bck.copy { - // Restore backup from copy - let _ = tokio::fs::copy(&bck.path, &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, &path).await.map_err(|e| { - delete_bck = false; - log::error!("Failed to restore backup on write failure: {e}") - }); - } + delete_bck = false; + // restore backup + let _ = tokio::fs::rename(&bck.path, &path).await.map_err(|e| { + log::error!("Failed to restore backup on write failure: {e}") + }); } } // Delete backup if we're done with it if delete_bck { - tokio::fs::remove_file(bck.path).await?; + let _ = tokio::fs::remove_file(bck.path).await; } }