Fix: Improve merge commit information retrieval

src/main.rs

@@ -64,7 +64,7 @@
     }
 
     fn merge_conflict(branch: String, upstream: String, message: Option<String>) -> Self {
         let mut error_msg = format!("Merge conflict between {} and {}", upstream, branch);
         if let Some(details) = message {
             error_msg.push('\n');
             error_msg.push_str(&details);
@@ -73,7 +73,7 @@
     }
 
     fn git_command_failed(command: String, status: i32, stdout: String, stderr: String) -> Self {
         let error_msg = format!(
             "Git command failed: {}\nStatus: {}\nStdout: {}\nStderr: {}",
             command, status, stdout, stderr
         );
@@ -132,21 +132,21 @@
     if name.starts_with("git-") && name.len() > 4 {
         let tmp: Vec<String> = name.split("git-").map(|x| x.to_string()).collect();
         let git_cmd = &tmp[1];
         return format!("git {}", git_cmd);
     }
     name
 }
 
 fn chain_name_key(branch_name: &str) -> String {
     format!("branch.{}.chain-name", branch_name)
 }
 
 fn chain_order_key(branch_name: &str) -> String {
     format!("branch.{}.chain-order", branch_name)
 }
 
 fn root_branch_key(branch_name: &str) -> String {
     format!("branch.{}.root-branch", branch_name)
 }
 
 fn generate_chain_order() -> String {
@@ -195,7 +195,7 @@
         branch.bold(),
         upstream_branch.bold()
     );
     eprintln!(
         "⚠️  Resolve any rebase merge conflicts, and then run {} rebase",
         executable_name
     );
@@ -532,7 +532,7 @@
         let mut branches = Chain::get_branches_for_chain(git_chain, chain_name)?;
 
         if branches.is_empty() {
             return Err(Error::from_str(&format!(
                 "Unable to get branches attached to chain: {}",
                 chain_name
             )));
@@ -579,10 +579,10 @@
         let status = match ahead_behind {
             (0, 0) => "".to_string(),
             (ahead, 0) => {
                 format!("{} ahead", ahead)
             }
             (0, behind) => {
                 format!("{} behind", behind)
             }
             (ahead, behind) => {
                 format!("{} ahead ⦁ {} behind", ahead, behind)
@@ -1725,9 +1725,9 @@
         parent_branch: &str,
         branch_name: &str,
     ) -> Result<Vec<MergeCommitInfo>, Error> {
-        // Get the latest commit on the branch
+        // 1. Fetch all commit hashes for the given `branch_name`.
         let mut command = Command::new("git");
-        command.args(["log", "--oneline", "-1", branch_name]);
+        command.args(["log", "--format=%H", branch_name]); // Get only commit hashes
         let output = match command.output() {
             Ok(output) => output,
             Err(_) => return Ok(vec![]), // Return empty vec on error
@@ -1737,92 +1737,101 @@
             return Ok(vec![]);
         }
 
-        let latest_commit = String::from_utf8_lossy(&output.stdout).trim().to_string();
-        if latest_commit.is_empty() {
-            return Ok(vec![]);
-        }
+        let commit_hashes_str = String::from_utf8_lossy(&output.stdout);
+        let commit_hashes: Vec<&str> = commit_hashes_str.trim().lines().collect();
 
-        // Check if it's a merge commit by looking for parent commits
-        let commit_hash = latest_commit.split_whitespace().next().unwrap_or("");
-        if commit_hash.is_empty() {
+        if commit_hashes.is_empty() {
             return Ok(vec![]);
         }
 
-        // Get commit information
-        let mut command = Command::new("git");
-        command.args(["show", "--stat", commit_hash]);
-        let output = match command.output() {
-            Ok(output) => output,
-            Err(_) => return Ok(vec![]),
-        };
+        let mut merge_commits_info = Vec::new();
 
-        if !output.status.success() {
-            return Ok(vec![]);
-        }
+        // 2. Iterate through each commit hash.
+        for commit_hash in commit_hashes {
+            if commit_hash.is_empty() {
+                continue;
+            }
+
+            // 3. For each commit, use `git show --stat <commit_hash>` to get its information.
+            let mut show_command = Command::new("git");
+            show_command.args(["show", "--stat", commit_hash]);
+            let show_output = match show_command.output() {
+                Ok(output) => output,
+                Err(_) => continue, // Skip this commit on error
+            };
+
+            if !show_output.status.success() {
+                continue; // Skip this commit on error
+            }
 
-        let commit_info = String::from_utf8_lossy(&output.stdout).to_string();
+            let commit_info_str = String::from_utf8_lossy(&show_output.stdout).to_string();
+            let commit_lines: Vec<&str> = commit_info_str.lines().collect();
 
-        // Check if it's a merge commit, which typically contains "Merge" in the commit message
-        if commit_info.contains(&format!("Merge branch '{}'", parent_branch))
-            || commit_info.contains("Merge branch")
-        {
-            // Extract commit message (first line after commit hash)
-            let commit_lines: Vec<&str> = commit_info.lines().collect();
-            let message = commit_lines
+            // 4. If the commit is a merge commit (contains "Merge branch" in its message AND refers to `parent_branch`),
+            //    extract its message and stats.
+            let merge_line_prefix = format!("Merge branch '{}'", parent_branch);
+            let is_merge_from_parent = commit_lines
                 .iter()
-                .position(|line| line.trim().starts_with("Merge branch"))
-                .map(|idx| commit_lines[idx].trim().to_string());
+                .any(|line| line.trim().starts_with(&merge_line_prefix));
 
-            // Extract stats
-            let stats_line = commit_lines
+            // Also check for generic "Merge branch" if parent_branch is empty (e.g. for initial merge to main)
+            // or if the specific format isn't exact.
+            let is_generic_merge = commit_lines
                 .iter()
-                .find(|line| line.contains("files changed") || line.contains("file changed"));
+                .any(|line| line.trim().starts_with("Merge branch"));
 
-            let stats = stats_line.map(|line| {
-                let mut files_changed = 0;
-                let mut insertions = 0;
-                let mut deletions = 0;
+            // A merge commit must have a line starting with "Merge: "
+            let is_actual_merge_commit = commit_lines.iter().any(|line| line.starts_with("Merge: "));
 
-                if let Some(files_idx) = line.find("file changed") {
-                    if let Some(files_num) = line[..files_idx].split_whitespace().last() {
-                        files_changed = files_num.parse().unwrap_or(0);
-                    }
-                } else if let Some(files_idx) = line.find("files changed") {
-                    if let Some(files_num) = line[..files_idx].split_whitespace().last() {
-                        files_changed = files_num.parse().unwrap_or(0);
-                    }
-                }
+            if is_actual_merge_commit && (is_merge_from_parent || (parent_branch.is_empty() && is_generic_merge)) {
+                // Extract commit message (the line starting with "Merge branch ...")
+                let message = commit_lines
+                    .iter()
+                    .find(|line| line.trim().starts_with("Merge branch"))
+                    .map(|line| line.trim().to_string());
 
-                if let Some(ins_idx) = line.find("insertion") {
-                    if let Some(ins_end) = line[..ins_idx].rfind(' ') {
-                        if let Some(ins_start) = line[..ins_end].rfind(' ') {
-                            let ins_str = &line[ins_start + 1..ins_end];
-                            insertions = ins_str.parse().unwrap_or(0);
+                // Extract stats (preserve existing logic, with slight refinement for parsing)
+                let stats_line = commit_lines
+                    .iter()
+                    .find(|line| line.contains("files changed") || line.contains("file changed"));
+
+                let stats = stats_line.map(|line| {
+                    let mut files_changed = 0;
+                    let mut insertions = 0;
+                    let mut deletions = 0;
+
+                    // Example line: "1 file changed, 5 insertions(+), 2 deletions(-)"
+                    // More robust parsing:
+                    let parts: Vec<&str> = line.split(',').map(|s| s.trim()).collect();
+                    for part in parts {
+                        if part.contains("file changed") || part.contains("files changed") {
+                            if let Some(num_str) = part.split_whitespace().next() {
+                                files_changed = num_str.parse().unwrap_or(0);
+                            }
+                        } else if part.contains("insertion") {
+                            if let Some(num_str) = part.split_whitespace().next() {
+                                insertions = num_str.parse().unwrap_or(0);
+                            }
+                        } else if part.contains("deletion") {
+                            if let Some(num_str) = part.split_whitespace().next() {
+                                deletions = num_str.parse().unwrap_or(0);
+                            }
                         }
                     }
-                }
-
-                if let Some(del_idx) = line.find("deletion") {
-                    if let Some(del_end) = line[..del_idx].rfind(' ') {
-                        if let Some(del_start) = line[..del_end].rfind(' ') {
-                            let del_str = &line[del_start + 1..del_end];
-                            deletions = del_str.parse().unwrap_or(0);
-                        }
+                    MergeStats {
+                        files_changed,
+                        insertions,
+                        deletions,
                     }
-                }
+                });
 
-                MergeStats {
-                    files_changed,
-                    insertions,
-                    deletions,
-                }
-            });
-
-            return Ok(vec![MergeCommitInfo { message, stats }]);
+                merge_commits_info.push(MergeCommitInfo { message, stats });
+            }
         }
 
-        // It's not a merge commit
-        Ok(vec![])
+        // 5. Collect all `MergeCommitInfo` structs for the identified merge commits.
+        // 6. Return `Ok(Vec<MergeCommitInfo>)`
+        Ok(merge_commits_info)
     }
 
     fn report_merge_results(

tests/merge.rs

@@ -296,6 +296,106 @@
     teardown_git_repo(repo_name);
 }
 
+#[test]
+fn test_get_merge_commit_info_multiple_merges() {
+    let repo_name = "test_get_merge_commit_info_multiple_merges";
+    let repo = setup_git_repo(repo_name);
+    let path_to_repo = generate_path_to_repo(repo_name);
+    let original_cwd = std::env::current_dir().unwrap(); // Save CWD
+    std::env::set_current_dir(&path_to_repo).unwrap(); // Change CWD to repo path
+
+    // 1. Set up a Git repository with a `main` branch and a feature branch (e.g., `feat/test-merge-info`).
+    create_new_file(&path_to_repo, "initial.txt", "Initial content on main");
+    first_commit_all(&repo, "Initial commit on main");
+
+    create_branch(&repo, "feat/test-merge-info");
+    checkout_branch(&repo, "feat/test-merge-info");
+    create_new_file(&path_to_repo, "feature_initial.txt", "Initial content on feature branch");
+    commit_all(&repo, "Initial commit on feature branch");
+
+    // 2. Create a few commits on the `main` branch.
+    checkout_branch(&repo, "main");
+    create_new_file(&path_to_repo, "main_commit1.txt", "Main commit 1");
+    commit_all(&repo, "Main C1");
+    create_new_file(&path_to_repo, "main_commit2.txt", "Main commit 2");
+    commit_all(&repo, "Main C2");
+
+    // 3. Create a few commits on the `feat/test-merge-info` branch.
+    checkout_branch(&repo, "feat/test-merge-info");
+    create_new_file(&path_to_repo, "feat_commit1.txt", "Feature commit 1");
+    commit_all(&repo, "Feat C1");
+    create_new_file(&path_to_repo, "feat_commit2.txt", "Feature commit 2");
+    commit_all(&repo, "Feat C2");
+
+    // 4. Merge `main` into `feat/test-merge-info` (this will be the first merge commit).
+    // Add some file changes in this merge.
+    // The stats will reflect files from main brought into feat/test-merge-info.
+    // For this merge, main_commit1.txt and main_commit2.txt are new to feat/test-merge-info.
+    let merge_msg1 = "Merge main into feat/test-merge-info (1st)";
+    run_git_command(&path_to_repo, vec!["merge", "main", "--no-ff", "-m", merge_msg1]);
+
+    // 5. Create more commits on `main`.
+    checkout_branch(&repo, "main");
+    create_new_file(&path_to_repo, "main_commit3.txt", "Main commit 3");
+    commit_all(&repo, "Main C3");
+    create_new_file(&path_to_repo, "main_commit4.txt", "Main commit 4");
+    commit_all(&repo, "Main C4");
+
+    // 6. Create more commits on `feat/test-merge-info`.
+    checkout_branch(&repo, "feat/test-merge-info");
+    create_new_file(&path_to_repo, "feat_commit3.txt", "Feature commit 3");
+    commit_all(&repo, "Feat C3");
+
+    // 7. Merge `main` into `feat/test-merge-info` again (this will be the second merge commit).
+    // Add different file changes in this merge.
+    // For this merge, main_commit3.txt and main_commit4.txt are new to feat/test-merge-info.
+    let merge_msg2 = "Merge main into feat/test-merge-info (2nd)";
+    run_git_command(&path_to_repo, vec!["merge", "main", "--no-ff", "-m", merge_msg2]);
+
+    // 8. Call `get_merge_commit_info` with `parent_branch` as "main" and `branch_name` as "feat/test-merge-info".
+    // Need to instantiate GitChain for the current repo.
+    // The GitChain::init() discovers the repo from the current directory.
+    let git_chain_path = Path::new("."); // Relative to current_dir which is path_to_repo
+    let git_chain = git_chain::GitChain::init(git_chain_path).expect("Failed to init GitChain");
+
+    let merge_infos_result = git_chain.get_merge_commit_info("main", "feat/test-merge-info");
+
+    // 9. Assert that the function returns `Ok` with a vector containing two `MergeCommitInfo` structs.
+    assert!(merge_infos_result.is_ok(), "get_merge_commit_info should return Ok");
+    let merge_infos = merge_infos_result.unwrap();
+    assert_eq!(merge_infos.len(), 2, "Should find 2 merge commits from main");
+
+    // 10. Assert that the extracted information (message, stats) for both merge commits is correct.
+    // Merge commits are typically listed in reverse chronological order by `git log`.
+    // The function as implemented iterates `git log` and then `git show` for each,
+    // so the order in `merge_infos` should be reverse chronological.
+
+    // Second merge (most recent)
+    let info2 = &merge_infos[0];
+    assert_eq!(info2.message, Some(merge_msg2.to_string()));
+    assert!(info2.stats.is_some(), "Stats should be present for the 2nd merge");
+    if let Some(stats) = &info2.stats {
+        // main_commit3.txt, main_commit4.txt are introduced.
+        // Exact insertions/deletions depend on line count, but files_changed should be 2.
+        assert_eq!(stats.files_changed, 2, "2nd merge: files_changed should be 2");
+        assert!(stats.insertions > 0, "2nd merge: insertions should be > 0");
+        // Deletions could be 0 if only new files are added.
+    }
+
+    // First merge
+    let info1 = &merge_infos[1];
+    assert_eq!(info1.message, Some(merge_msg1.to_string()));
+    assert!(info1.stats.is_some(), "Stats should be present for the 1st merge");
+    if let Some(stats) = &info1.stats {
+        // main_commit1.txt, main_commit2.txt are introduced.
+        assert_eq!(stats.files_changed, 2, "1st merge: files_changed should be 2");
+        assert!(stats.insertions > 0, "1st merge: insertions should be > 0");
+    }
+
+    std::env::set_current_dir(original_cwd).unwrap(); // Restore CWD
+    teardown_git_repo(repo_name);
+}
+
 #[test]
 fn merge_subcommand_with_ahead_behind() {
     // Test that merge command works with branches that are ahead and behind