add test to check if ownership stays the same

helix-stdx/src/path.rs

@@ -242,7 +242,7 @@ pub fn escape_path(path: &Path) -> PathBuf {
     path_from_bytes(&bytes).unwrap()
 }
 
-pub fn add_extension<'a, S: AsRef<std::ffi::OsStr>>(p: &'a Path, extension: S) -> Cow<'a, Path> {
+pub fn add_extension<S: AsRef<std::ffi::OsStr>>(p: &Path, extension: S) -> Cow<'_, Path> {
     let new = extension.as_ref();
     if new.is_empty() {
         Cow::Borrowed(p)

helix-term/tests/test/commands/write.rs

@@ -687,6 +687,25 @@ async fn test_hardlink_write() -> anyhow::Result<()> {
     Ok(())
 }
 
+#[tokio::test(flavor = "multi_thread")]
+#[cfg(unix)]
+async fn test_write_ownership() -> anyhow::Result<()> {
+    let mut file = tempfile::NamedTempFile::new()?;
+    let mut app = helpers::AppBuilder::new()
+        .with_file(file.path(), None)
+        .build()?;
+
+    let old_meta = file.as_file().metadata()?;
+
+    test_key_sequence(&mut app, Some("hello:w<ret>"), None, false).await?;
+    reload_file(&mut file).unwrap();
+
+    let new_meta = file.as_file().metadata()?;
+    assert!(old_meta.uid() == new_meta.uid() && old_meta.gid() == new_meta.gid());
+
+    Ok(())
+}
+
 async fn edit_file_with_content(file_content: &[u8]) -> anyhow::Result<()> {
     let mut file = tempfile::NamedTempFile::new()?;
 

helix-view/src/document.rs

@@ -213,7 +213,7 @@ impl Backup {
             let bck_base_path = &dir.join(&escaped_p);
 
             // 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();
+            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
@@ -221,7 +221,7 @@ 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 = helix_stdx::path::add_extension(&bck_base_path, format!("{n}.{ext}"))
+                backup = helix_stdx::path::add_extension(bck_base_path, format!("{n}.{ext}"))
                     .into_owned();
                 if n == u8::MAX {
                     continue 'outer;
@@ -1155,12 +1155,9 @@ impl Document {
             };
 
             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;
+
+                // Attempt to restore backup
                 if write_result.is_err() {
                     // If original file no longer exists, then backup is renamed to original file
                     if !path.exists() {
@@ -1174,8 +1171,8 @@ impl Document {
                         {
                             // Reset timestamps
                             let meta = meta.as_ref().unwrap();
-                            let atime = FileTime::from_last_access_time(&meta);
-                            let mtime = FileTime::from_last_modification_time(&meta);
+                            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 {