From 65f3bd360c065aca493eddf7eb5d3d8191a84b56 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Tue, 11 Nov 2025 08:31:46 +0000 Subject: fix: out of sync grasp server cli output so it shows a summary rather than a lot of lines of issues --- src/bin/git_remote_nostr/list.rs | 39 +-- src/lib/list.rs | 584 ++++++++++++++++++++++++++++++++++++++- tests/git_remote_nostr/list.rs | 5 +- 3 files changed, 595 insertions(+), 33 deletions(-) diff --git a/src/bin/git_remote_nostr/list.rs b/src/bin/git_remote_nostr/list.rs index 803cc8f..934160a 100644 --- a/src/bin/git_remote_nostr/list.rs +++ b/src/bin/git_remote_nostr/list.rs @@ -8,15 +8,16 @@ use ngit::{ fetch::fetch_from_git_server, git::{self}, git_events::{KIND_PULL_REQUEST, KIND_PULL_REQUEST_UPDATE, event_to_cover_letter, tag_value}, - list::{get_ahead_behind, list_from_remotes}, + list::{generate_remote_sync_warnings, identify_remote_sync_issues, list_from_remotes}, login::get_curent_user, repo_ref::{self}, - utils::{get_all_proposals, get_open_or_draft_proposals, get_short_git_server_name}, + utils::{get_all_proposals, get_open_or_draft_proposals}, }; use repo_ref::RepoRef; use crate::{fetch::make_commits_for_proposal, git::Repo}; +#[allow(clippy::too_many_lines)] pub async fn run_list( git_repo: &Repo, repo_ref: &RepoRef, @@ -34,33 +35,15 @@ pub async fn run_list( ); let mut state = if let Some(nostr_state) = nostr_state { - for (name, value) in &nostr_state.state { - for (url, (remote_state, _is_grasp_server)) in &remote_states { - let remote_name = get_short_git_server_name(git_repo, url); - if let Some(remote_value) = remote_state.get(name) { - if value.ne(remote_value) { - term.write_line( - format!( - "WARNING: {remote_name} {name} is {} nostr ", - if let Ok((ahead, behind)) = - get_ahead_behind(git_repo, value, remote_value) - { - format!("{} ahead {} behind", ahead.len(), behind.len()) - } else { - "out of sync with".to_string() - } - ) - .as_str(), - )?; - } - } else { - term.write_line( - format!("WARNING: {remote_name} {name} is missing but tracked on nostr") - .as_str(), - )?; - } - } + // Identify sync issues using shared abstraction + let remote_issues = identify_remote_sync_issues(git_repo, &nostr_state, &remote_states); + + // Generate and print warnings + let warnings = generate_remote_sync_warnings(git_repo, &remote_issues, &remote_states); + for warning in warnings { + term.write_line(&warning)?; } + nostr_state.state } else { let (state, _is_grasp_server) = repo_ref diff --git a/src/lib/list.rs b/src/lib/list.rs index 733936a..b4d6a5e 100644 --- a/src/lib/list.rs +++ b/src/lib/list.rs @@ -10,9 +10,40 @@ use crate::{ nostr_url::{CloneUrl, NostrUrlDecoded, ServerProtocol}, }, repo_ref::is_grasp_server_clone_url, - utils::{Direction, get_read_protocols_to_try, join_with_and, set_protocol_preference}, + repo_state::RepoState, + utils::{ + Direction, get_read_protocols_to_try, get_short_git_server_name, join_with_and, + set_protocol_preference, + }, }; +/// Sync issues identified for a single remote +#[derive(Default, Debug, Clone)] +pub struct RemoteIssues { + pub branches_out_of_sync: Vec<(String, Option<(usize, usize)>)>, // (ref, (ahead, behind)) + pub branches_missing: Vec, + pub tags_out_of_sync: Vec, + pub tags_missing: Vec, +} + +impl RemoteIssues { + /// Returns true if there are no issues + pub fn is_empty(&self) -> bool { + self.branches_out_of_sync.is_empty() + && self.branches_missing.is_empty() + && self.tags_out_of_sync.is_empty() + && self.tags_missing.is_empty() + } + + /// Returns the total count of all issues + pub fn total_count(&self) -> usize { + self.branches_out_of_sync.len() + + self.branches_missing.len() + + self.tags_out_of_sync.len() + + self.tags_missing.len() + } +} + pub fn list_from_remotes( term: &console::Term, git_repo: &Repo, @@ -182,3 +213,554 @@ pub fn get_ahead_behind( let latest = git_repo.get_commit_or_tip_of_reference(latest_ref_or_oid)?; git_repo.get_commits_ahead_behind(&base, &latest) } + +/// Identify sync discrepancies between nostr state and remote git servers +/// +/// This function analyzes the differences between the expected state (from +/// nostr) and the actual state on each remote git server, categorizing issues +/// by type (branches/tags, out of sync/missing). +/// +/// # Arguments +/// * `git_repo` - The local git repository +/// * `nostr_state` - The expected state from nostr +/// * `remote_states` - Map of remote URLs to their states and whether they're +/// grasp servers +/// +/// # Returns +/// A HashMap mapping remote names to their identified sync issues +pub fn identify_remote_sync_issues( + git_repo: &Repo, + nostr_state: &RepoState, + remote_states: &HashMap, bool)>, +) -> HashMap { + let mut remote_issues: HashMap = HashMap::new(); + + for (name, value) in &nostr_state.state { + for (url, (remote_state, _is_grasp_server)) in remote_states { + let remote_name = get_short_git_server_name(git_repo, url); + let issues = remote_issues.entry(remote_name.clone()).or_default(); + + let is_branch = name.starts_with("refs/heads/"); + let is_tag = name.starts_with("refs/tags/"); + + if let Some(remote_value) = remote_state.get(name) { + if value.ne(remote_value) { + if is_branch { + // Calculate ahead/behind for branches + let ahead_behind = get_ahead_behind(git_repo, value, remote_value) + .ok() + .map(|(ahead, behind)| (ahead.len(), behind.len())); + issues + .branches_out_of_sync + .push((name.clone(), ahead_behind)); + } else if is_tag { + issues.tags_out_of_sync.push(name.clone()); + } + } + } else if is_branch { + issues.branches_missing.push(name.clone()); + } else if is_tag { + issues.tags_missing.push(name.clone()); + } + } + } + + remote_issues +} + +/// Format a list of refs with ahead/behind information into a user-friendly +/// issue summary +/// +/// # Arguments +/// * `refs` - List of refs with optional ahead/behind counts +/// * `singular` - Singular form of the ref type (e.g., "branch") +/// * `plural` - Plural form of the ref type (e.g., "branches") +/// * `status` - Status description (e.g., "out of sync", "missing") +/// * `is_branch` - Whether these are branches (affects formatting) +/// +/// # Returns +/// A formatted string describing the issue +pub fn format_ref_issue( + refs: &[(String, Option<(usize, usize)>)], + _singular: &str, + plural: &str, + status: &str, + is_branch: bool, +) -> String { + let count = refs.len(); + + /// Helper to format branch name with ahead/behind info + fn format_branch_with_sync(name: &str, ahead_behind: &Option<(usize, usize)>) -> String { + if let Some((ahead, behind)) = ahead_behind { + if *ahead > 0 && *behind > 0 { + format!("{} ({} behind, {} ahead)", name, behind, ahead) + } else if *behind > 0 { + format!("{} ({} behind)", name, behind) + } else if *ahead > 0 { + format!("{} ({} ahead)", name, ahead) + } else { + name.to_string() + } + } else { + name.to_string() + } + } + + if count == 1 { + // Single item: name the ref with ahead/behind info + let clean_ref = refs[0] + .0 + .strip_prefix("refs/heads/") + .or_else(|| refs[0].0.strip_prefix("refs/tags/")) + .unwrap_or(&refs[0].0); + let formatted = if is_branch { + format_branch_with_sync(clean_ref, &refs[0].1) + } else { + clean_ref.to_string() + }; + format!("{} {}", formatted, status) + } else if is_branch && count <= 3 { + // For branches: list up to 3 names with ahead/behind info + let names: Vec<_> = refs + .iter() + .map(|(r, ab)| { + let clean = r.strip_prefix("refs/heads/").unwrap_or(r); + format_branch_with_sync(clean, ab) + }) + .collect(); + if count == 2 { + format!("{} and {} {}", names[0], names[1], status) + } else { + format!("{}, {} and {} {}", names[0], names[1], names[2], status) + } + } else if is_branch && count > 3 { + // For many branches: list first 2 with ahead/behind and count others + let names: Vec<_> = refs + .iter() + .take(2) + .map(|(r, ab)| { + let clean = r.strip_prefix("refs/heads/").unwrap_or(r); + format_branch_with_sync(clean, ab) + }) + .collect(); + let other_count = count - 2; + let other = if other_count == 1 { + "1 other".to_string() + } else { + format!("{} others", other_count) + }; + format!("{}, {} and {} {}", names[0], names[1], other, status) + } else { + // For tags: just count + format!("{} {} {}", count, plural, status) + } +} + +/// Format a list of refs (String only) into a user-friendly issue summary +/// +/// # Arguments +/// * `refs` - List of ref names +/// * `singular` - Singular form of the ref type (e.g., "branch") +/// * `plural` - Plural form of the ref type (e.g., "branches") +/// * `status` - Status description (e.g., "out of sync", "missing") +/// * `is_branch` - Whether these are branches (affects formatting) +/// +/// # Returns +/// A formatted string describing the issue +pub fn format_ref_issue_simple( + refs: &[String], + _singular: &str, + plural: &str, + status: &str, + is_branch: bool, +) -> String { + let count = refs.len(); + + if count == 1 { + // Single item: name the ref + let clean_ref = refs[0] + .strip_prefix("refs/heads/") + .or_else(|| refs[0].strip_prefix("refs/tags/")) + .unwrap_or(&refs[0]); + format!("{} {}", clean_ref, status) + } else if is_branch && count <= 3 { + // For branches: list up to 3 names + let names: Vec<_> = refs + .iter() + .map(|r| r.strip_prefix("refs/heads/").unwrap_or(r)) + .collect(); + if count == 2 { + format!("{} and {} {}", names[0], names[1], status) + } else { + format!("{}, {} and {} {}", names[0], names[1], names[2], status) + } + } else if is_branch && count > 3 { + // For many branches: list first 2 and count others + let names: Vec<_> = refs + .iter() + .take(2) + .map(|r| r.strip_prefix("refs/heads/").unwrap_or(r)) + .collect(); + let other_count = count - 2; + let other = if other_count == 1 { + "1 other".to_string() + } else { + format!("{} others", other_count) + }; + format!("{}, {} and {} {}", names[0], names[1], other, status) + } else { + // For tags: just count + format!("{} {} {}", count, plural, status) + } +} + +/// Generate warning messages for remote sync issues +pub fn generate_remote_sync_warnings( + git_repo: &Repo, + remote_issues: &HashMap, + remote_states: &HashMap, bool)>, +) -> Vec { + let mut warnings = Vec::new(); + + for (remote_name, issues) in remote_issues { + if issues.is_empty() { + continue; + } + + // Find remote state for this remote + let remote_state = remote_states + .iter() + .find(|(url, _)| &get_short_git_server_name(git_repo, url) == remote_name) + .map(|(_, (state, _))| state); + + if let Some(state) = remote_state { + // Check if remote is completely empty + if state.is_empty() { + warnings.push(format!("WARNING: {remote_name} has no data.")); + continue; + } + + // Check if remote only has a few branches and missing many + let remote_branches: Vec<_> = state + .keys() + .filter(|k| k.starts_with("refs/heads/")) + .map(|b| b.strip_prefix("refs/heads/").unwrap_or(b)) + .collect(); + + if remote_branches.len() <= 3 && issues.branches_missing.len() >= 5 { + let sync_status = if issues.branches_out_of_sync.is_empty() { + "" + } else { + " and they are out of sync" + }; + + warnings.push(format!( + "WARNING: {remote_name} only has {} branches{}", + remote_branches.join(", "), + sync_status + )); + continue; + } + } + + // Build summary message parts + let mut parts = Vec::new(); + + if !issues.branches_out_of_sync.is_empty() { + parts.push(format_ref_issue( + &issues.branches_out_of_sync, + "branch", + "branches", + "out of sync", + true, + )); + } + + if !issues.branches_missing.is_empty() { + parts.push(format_ref_issue_simple( + &issues.branches_missing, + "branch", + "branches", + "missing", + true, + )); + } + + if !issues.tags_out_of_sync.is_empty() { + parts.push(format_ref_issue_simple( + &issues.tags_out_of_sync, + "tag", + "tags", + "out of sync", + false, + )); + } + + if !issues.tags_missing.is_empty() { + parts.push(format_ref_issue_simple( + &issues.tags_missing, + "tag", + "tags", + "missing", + false, + )); + } + + if !parts.is_empty() { + warnings.push(format!( + "WARNING: {remote_name} is out of sync. {}", + parts.join(". ") + )); + } + } + + warnings +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_format_ref_issue_single_branch_with_ahead_behind() { + let refs = vec![("refs/heads/main".to_string(), Some((5, 3)))]; + assert_eq!( + format_ref_issue(&refs, "branch", "branches", "out of sync", true), + "main (3 behind, 5 ahead) out of sync" + ); + } + + #[test] + fn test_format_ref_issue_single_branch_only_behind() { + let refs = vec![("refs/heads/feature".to_string(), Some((0, 7)))]; + assert_eq!( + format_ref_issue(&refs, "branch", "branches", "out of sync", true), + "feature (7 behind) out of sync" + ); + } + + #[test] + fn test_format_ref_issue_single_branch_only_ahead() { + let refs = vec![("refs/heads/dev".to_string(), Some((4, 0)))]; + assert_eq!( + format_ref_issue(&refs, "branch", "branches", "out of sync", true), + "dev (4 ahead) out of sync" + ); + } + + #[test] + fn test_format_ref_issue_single_branch_no_diff() { + let refs = vec![("refs/heads/main".to_string(), Some((0, 0)))]; + assert_eq!( + format_ref_issue(&refs, "branch", "branches", "out of sync", true), + "main out of sync" + ); + } + + #[test] + fn test_format_ref_issue_single_branch_no_ahead_behind_info() { + let refs = vec![("refs/heads/main".to_string(), None)]; + assert_eq!( + format_ref_issue(&refs, "branch", "branches", "out of sync", true), + "main out of sync" + ); + } + + #[test] + fn test_format_ref_issue_two_branches() { + let refs = vec![ + ("refs/heads/main".to_string(), Some((2, 1))), + ("refs/heads/dev".to_string(), Some((0, 3))), + ]; + assert_eq!( + format_ref_issue(&refs, "branch", "branches", "out of sync", true), + "main (1 behind, 2 ahead) and dev (3 behind) out of sync" + ); + } + + #[test] + fn test_format_ref_issue_three_branches() { + let refs = vec![ + ("refs/heads/main".to_string(), Some((1, 0))), + ("refs/heads/dev".to_string(), Some((0, 2))), + ("refs/heads/feature".to_string(), None), + ]; + assert_eq!( + format_ref_issue(&refs, "branch", "branches", "out of sync", true), + "main (1 ahead), dev (2 behind) and feature out of sync" + ); + } + + #[test] + fn test_format_ref_issue_many_branches() { + let refs = vec![ + ("refs/heads/main".to_string(), Some((5, 3))), + ("refs/heads/dev".to_string(), Some((0, 1))), + ("refs/heads/feature1".to_string(), None), + ("refs/heads/feature2".to_string(), Some((2, 0))), + ]; + assert_eq!( + format_ref_issue(&refs, "branch", "branches", "out of sync", true), + "main (3 behind, 5 ahead), dev (1 behind) and 2 others out of sync" + ); + } + + #[test] + fn test_format_ref_issue_many_branches_singular_other() { + let refs = vec![ + ("refs/heads/main".to_string(), Some((1, 1))), + ("refs/heads/dev".to_string(), Some((2, 2))), + ("refs/heads/feature".to_string(), None), + ]; + // With 3 branches, it should list all 3 + assert_eq!( + format_ref_issue(&refs, "branch", "branches", "out of sync", true), + "main (1 behind, 1 ahead), dev (2 behind, 2 ahead) and feature out of sync" + ); + + // With 4 branches (show 2, then "2 others") + let refs = vec![ + ("refs/heads/main".to_string(), Some((1, 1))), + ("refs/heads/dev".to_string(), Some((2, 2))), + ("refs/heads/feature1".to_string(), None), + ("refs/heads/feature2".to_string(), None), + ]; + assert_eq!( + format_ref_issue(&refs, "branch", "branches", "out of sync", true), + "main (1 behind, 1 ahead), dev (2 behind, 2 ahead) and 2 others out of sync" + ); + } + + #[test] + fn test_format_ref_issue_single_tag() { + let refs = vec![("refs/tags/v1.0.0".to_string(), None)]; + assert_eq!( + format_ref_issue(&refs, "tag", "tags", "out of sync", false), + "v1.0.0 out of sync" + ); + } + + #[test] + fn test_format_ref_issue_multiple_tags() { + let refs = vec![ + ("refs/tags/v1.0.0".to_string(), None), + ("refs/tags/v1.0.1".to_string(), None), + ("refs/tags/v2.0.0".to_string(), None), + ]; + assert_eq!( + format_ref_issue(&refs, "tag", "tags", "out of sync", false), + "3 tags out of sync" + ); + } + + #[test] + fn test_format_ref_issue_simple_single_branch() { + let refs = vec!["refs/heads/main".to_string()]; + assert_eq!( + format_ref_issue_simple(&refs, "branch", "branches", "missing", true), + "main missing" + ); + } + + #[test] + fn test_format_ref_issue_simple_two_branches() { + let refs = vec!["refs/heads/main".to_string(), "refs/heads/dev".to_string()]; + assert_eq!( + format_ref_issue_simple(&refs, "branch", "branches", "missing", true), + "main and dev missing" + ); + } + + #[test] + fn test_format_ref_issue_simple_three_branches() { + let refs = vec![ + "refs/heads/main".to_string(), + "refs/heads/dev".to_string(), + "refs/heads/feature".to_string(), + ]; + assert_eq!( + format_ref_issue_simple(&refs, "branch", "branches", "missing", true), + "main, dev and feature missing" + ); + } + + #[test] + fn test_format_ref_issue_simple_many_branches() { + let refs = vec![ + "refs/heads/main".to_string(), + "refs/heads/dev".to_string(), + "refs/heads/feature1".to_string(), + "refs/heads/feature2".to_string(), + ]; + assert_eq!( + format_ref_issue_simple(&refs, "branch", "branches", "missing", true), + "main, dev and 2 others missing" + ); + } + + #[test] + fn test_format_ref_issue_simple_many_branches_singular_other() { + let refs = vec![ + "refs/heads/main".to_string(), + "refs/heads/dev".to_string(), + "refs/heads/feature".to_string(), + "refs/heads/hotfix".to_string(), + ]; + assert_eq!( + format_ref_issue_simple(&refs, "branch", "branches", "missing", true), + "main, dev and 2 others missing" + ); + + // Test with exactly 4 branches (2 shown + 2 others) + let refs = vec![ + "refs/heads/main".to_string(), + "refs/heads/dev".to_string(), + "refs/heads/feature".to_string(), + ]; + // With 3 branches, all should be shown + assert_eq!( + format_ref_issue_simple(&refs, "branch", "branches", "missing", true), + "main, dev and feature missing" + ); + } + + #[test] + fn test_format_ref_issue_simple_single_tag() { + let refs = vec!["refs/tags/v1.0.0".to_string()]; + assert_eq!( + format_ref_issue_simple(&refs, "tag", "tags", "missing", false), + "v1.0.0 missing" + ); + } + + #[test] + fn test_format_ref_issue_simple_multiple_tags() { + let refs = vec![ + "refs/tags/v1.0.0".to_string(), + "refs/tags/v1.0.1".to_string(), + "refs/tags/v2.0.0".to_string(), + ]; + assert_eq!( + format_ref_issue_simple(&refs, "tag", "tags", "missing", false), + "3 tags missing" + ); + } + + #[test] + fn test_format_ref_issue_without_refs_prefix() { + let refs = vec![("main".to_string(), Some((1, 0)))]; + assert_eq!( + format_ref_issue(&refs, "branch", "branches", "out of sync", true), + "main (1 ahead) out of sync" + ); + } + + #[test] + fn test_format_ref_issue_simple_without_refs_prefix() { + let refs = vec!["main".to_string()]; + assert_eq!( + format_ref_issue_simple(&refs, "branch", "branches", "missing", true), + "main missing" + ); + } +} diff --git a/tests/git_remote_nostr/list.rs b/tests/git_remote_nostr/list.rs index 530e022..8723b54 100644 --- a/tests/git_remote_nostr/list.rs +++ b/tests/git_remote_nostr/list.rs @@ -208,10 +208,7 @@ mod with_state_announcement { )?; p.expect("list: connecting...\r\n\r")?; p.expect( - format!( - "WARNING: {source_path} refs/heads/main is out of sync with nostr \r\n" - ) - .as_str(), + format!("WARNING: {source_path} is out of sync. main out of sync\r\n").as_str(), )?; // println!("{}", p.expect_eventually("\r\n\r\n")?); -- cgit v1.2.3