Merge pull request #2 from cafkafk/dev

Refactored Links::link implementation to be able to deal with several possible scenarios:

    file already linked (happy case)
    link to different file exists
    non link file exists
    broken symlink exists
    no file exists (happy case)
    generic failure

Also attempted to make the code more readable than previous attempt. Admittedly, still needs some work.

src/git.rs

@@ -16,7 +16,9 @@
 
 use log::{debug, error, info, trace, warn};
 use serde::{Deserialize, Serialize};
+use std::fs::canonicalize;
 use std::os::unix::fs::symlink;
+use std::path::Path;
 use std::{fs, process::Command};
 
 /// Represents the config.toml file.
@@ -43,54 +45,46 @@ pub struct GitRepo {
     pub clone: bool,
 }
 
+fn handle_file_exists(selff: &Links, tx_path: &Path, rx_path: &Path) {
+    match rx_path.read_link() {
+        Ok(file) if file.canonicalize().unwrap() == tx_path.canonicalize().unwrap() => {
+            debug!(
+                "Linking {} -> {} failed: file already linked",
+                &selff.tx, &selff.rx
+            );
+        }
+        Ok(file) => {
+            error!(
+                "Linking {} -> {} failed: link to different file exists",
+                &selff.tx, &selff.rx
+            );
+        }
+        Err(error) => {
+            error!("Linking {} -> {} failed: file exists", &selff.tx, &selff.rx);
+        }
+    }
+}
+
 impl Links {
     /// Creates a link from a file
     fn link(&self) {
-        let rx_exists: bool = std::path::Path::new(&self.rx).exists();
+        let tx_path: &Path = std::path::Path::new(&self.tx);
-        let tx_exists: bool = std::path::Path::new(&self.tx).exists();
+        let rx_path: &Path = std::path::Path::new(&self.rx);
-        // NOTE If the file exists
+        match rx_path.try_exists() {
-        // NOTE And is not a link
+            Ok(true) => handle_file_exists(&self, &tx_path, &rx_path),
-        // NOTE We avoid using fs::read_link by returning early
+            Ok(false) if rx_path.is_symlink() => {
-        // FIXME This should be refactored
+                error!(
-        if rx_exists {
+                    "Linking {} -> {} failed: broken symlink",
-            let rx_link = fs::read_link(&self.rx);
+                    &self.tx, &self.rx
-            match rx_link {
+                );
-                Ok(_) => (),
+            }
+            Ok(false) => {
+                symlink(&self.tx, &self.rx).expect("failed to create link");
+            }
             Err(error) => {
                 error!("Linking {} -> {} failed: {}", &self.tx, &self.rx, error);
-                    return;
-                }
-            }
-        }
-        match (tx_exists, rx_exists) {
-            (true, false) => symlink(&self.tx, &self.rx).expect("failed to create link"),
-            (true, true)
-                if fs::read_link(&self.rx)
-                    .expect("failed to read link")
-                    .to_str()
-                    .unwrap()
-                    != &self.tx =>
-            {
-                error!(
-                    "link {}: can't link rx {}, file already exists",
-                    &self.name, &self.rx
-                )
-            }
-            (true, true)
-                if fs::read_link(&self.rx)
-                    .expect("failed to read link")
-                    .to_str()
-                    .unwrap()
-                    == &self.tx =>
-            {
-                warn!(
-                    "link {}: can't link rx {}, file already exists",
-                    &self.name, &self.rx
-                )
-            }
-            (false, _) => error!("link {}: could not find tx at {}", &self.name, &self.rx),
-            (_, _) => unreachable!("Should never happen!"),
             }
+        };
     }
 }