fixed get_group_priority, fixed clippy suggestion and minor refactor

src/dotfiles.rs

@@ -29,15 +29,22 @@ pub const VALID_TARGETS: &[&str] = &[
     "_windows",
 ];
 
-pub fn get_target_priority(target: impl AsRef<str>) -> usize {
-    let target = target.as_ref();
+/// Returns the priority number for the group
+/// A higher number means a higher priority
+pub fn get_group_priority(group: impl AsRef<str>) -> usize {
+    let group = group.as_ref();
+    let target = group.split('_').last().unwrap_or(group);
+
+    // priority is in order of specificity
+    // the more os specific target has higher priority
     match target {
-        "_unix" | "_windows" => 2,
-        _ if !group_ends_with_target_name(target) => 0,
-        _ => 1,
+        "unix" | "windows" => 1,
+        _ if !group_ends_with_target_name(group) => 0,
+        _ => 2,
     }
 }
 
+/// Returns the index of the group with the highest priority in the `targets`
 pub fn get_highest_priority_target_idx(targets: &[impl AsRef<str>]) -> Option<usize> {
     if targets.is_empty() {
         return None;
@@ -47,7 +54,7 @@ pub fn get_highest_priority_target_idx(targets: &[impl AsRef<str>]) -> Option<us
     let mut highest_idx = 0;
 
     for (idx, target) in targets.iter().enumerate() {
-        let target_priority = get_target_priority(target);
+        let target_priority = get_group_priority(target);
 
         if target_priority >= highest_priority {
             highest_priority = target_priority;
@@ -58,9 +65,9 @@ pub fn get_highest_priority_target_idx(targets: &[impl AsRef<str>]) -> Option<us
     Some(highest_idx)
 }
 
-// Exit codes
-/// Couldn't find the dotfiles directory
+/// Exit codes
 pub enum ReturnCode {
+    /// Couldn't find the dotfiles directory
     CouldntFindDotfiles = 2,
     /// No Configs/Hooks/Secrets folder setup
     NoSetupFolder = 3,
@@ -107,6 +114,7 @@ pub struct Dotfile {
 impl TryFrom<path::PathBuf> for Dotfile {
     type Error = String;
 
+    /// Returns Ok if the path is pointing to a group within $TUCKR_HOME
     fn try_from(value: path::PathBuf) -> Result<Self, Self::Error> {
         /// returns the path for the group the file belongs to.
         /// an error is returned if the file does not belong to dotfiles
@@ -504,4 +512,20 @@ mod tests {
         assert_eq!(super::get_dotfile_profile_from_path(no_profile_dir), None,);
         assert_eq!(super::get_dotfile_profile_from_path(invalid_dir), None,);
     }
+
+    #[test]
+    fn group_priority() {
+        let groups = [
+            ("first_group_windows", 1),
+            ("second_linux", 2),
+            ("anotherone_here_macos", 2),
+            ("another_unix", 1),
+            ("priority_is_zero", 0),
+            ("no_priority", 0),
+        ];
+
+        for (group, expected_priority) in groups {
+            assert_eq!(super::get_group_priority(group), expected_priority);
+        }
+    }
 }

src/symlinks.rs

@@ -313,7 +313,6 @@ impl SymlinkHandler {
     }
 
     /// returns a cache with files in dotfiles that already exist in $HOME
-    /// todo: test and somethingsomething group not being marked as conflict when both have $HOME/test file
     fn get_conflicts_in_cache(&self) -> HashCache {
         let mut conflicts = HashCache::new();
 
@@ -328,6 +327,9 @@ impl SymlinkHandler {
             }
         }
 
+        // doesn't mark not owned dotfiles as conflicts if a higher priority dotfile
+        // with the same file is already symlinked. this allows dotfile fallbacks to
+        // work properly instead of falsely flagged as conflicts
         for files in self.not_owned.values() {
             for file in files {
                 conflicts.entry(file.group_name.clone()).or_default();
@@ -339,8 +341,8 @@ impl SymlinkHandler {
                     continue;
                 };
 
-                let target_has_higher_priority = dotfiles::get_target_priority(&dotfile.group_name)
-                    > dotfiles::get_target_priority(&file.group_name);
+                let target_has_higher_priority = dotfiles::get_group_priority(&dotfile.group_name)
+                    > dotfiles::get_group_priority(&file.group_name);
                 let not_same_base_group = dotfiles::group_without_target(&file.group_name)
                     != dotfiles::group_without_target(&dotfile.group_name);
 
@@ -367,7 +369,7 @@ impl SymlinkHandler {
             };
 
             let group = &groups[idx];
-            let group = Dotfile::try_from(self.dotfiles_dir.join("Configs").join(&group)).unwrap();
+            let group = Dotfile::try_from(self.dotfiles_dir.join("Configs").join(group)).unwrap();
             if group.path.exists() {
                 // iterate through all the files in group_dir
                 group.try_iter().unwrap().for_each(|f| symlink_file(f.path))
@@ -487,7 +489,6 @@ fn foreach_group<F: Fn(&SymlinkHandler, &String)>(
         valid_groups
     };
 
-    // handles wildcard
     if groups.contains(&"*".to_string()) {
         let symgroups = if symlinked {
             &sym.not_symlinked
@@ -496,15 +497,10 @@ fn foreach_group<F: Fn(&SymlinkHandler, &String)>(
         };
 
         for group in symgroups.keys() {
-            // Takes the name of the group to be passed the function
-            // Ignore groups in the excludes array
             if exclude.contains(group) {
                 continue;
             }
 
-            // Ignore conditional groups for other platforms.
-            // To force linking a group of other target_os/target_family, use
-            // explict argument passing instead of wildcard.
             if !dotfiles::group_is_valid_target(group) {
                 continue;
             }
@@ -689,8 +685,6 @@ fn print_global_status(sym: &SymlinkHandler) -> Result<(), ExitCode> {
 
     // --- detect conflicts ---
     let conflicts = sym.get_conflicts_in_cache();
-    // whether a conflict is a symlink or a pre-existing file does not matter for global status
-    // so we just add them together
     let conflicts: HashSet<_> = conflicts.keys().collect();
 
     // --- Creates all the tables and prints them ---