diff --git a/src/cli/operation/mod.rs b/src/cli/operation/mod.rs index b1083b1..26a2b7c 100644 --- a/src/cli/operation/mod.rs +++ b/src/cli/operation/mod.rs @@ -29,14 +29,7 @@ impl AnimationTerminal { } pub fn render(&mut self, frame: &str) -> io::Result<()> { - if self.rendered_line_count > 0 { - write!(self.stdout, "\r")?; - - if self.rendered_line_count > 1 { - write!(self.stdout, "\x1b[{}A", self.rendered_line_count - 1)?; - } - } - + self.reposition_cursor()?; write!(self.stdout, "{ANSI_CLEAR_TO_END}{frame}")?; self.stdout .flush() @@ -50,6 +43,27 @@ impl AnimationTerminal { self.active = false; Ok(()) } + + pub fn finish_and_clear(&mut self) -> io::Result<()> { + self.reposition_cursor()?; + write!(self.stdout, "{ANSI_CLEAR_TO_END}{ANSI_SHOW_CURSOR}")?; + self.stdout.flush()?; + self.rendered_line_count = 0; + self.active = false; + Ok(()) + } + + fn reposition_cursor(&mut self) -> io::Result<()> { + if self.rendered_line_count > 0 { + write!(self.stdout, "\r")?; + + if self.rendered_line_count > 1 { + write!(self.stdout, "\x1b[{}A", self.rendered_line_count - 1)?; + } + } + + Ok(()) + } } impl Drop for AnimationTerminal { diff --git a/src/cli/sync/mod.rs b/src/cli/sync/mod.rs index aee039e..9388bf0 100644 --- a/src/cli/sync/mod.rs +++ b/src/cli/sync/mod.rs @@ -15,7 +15,7 @@ use crate::core::merge; use crate::core::store::{PendingOperationKind, load_operation, open_initialized}; use crate::core::sync::{ self, RemotePushActionKind, RemotePushOutcome, SyncCompletion, SyncEvent, SyncOptions, - SyncStage, + SyncStage, SyncStatus, }; use crate::core::tree::{self, TreeOptions, TreeView}; @@ -201,15 +201,31 @@ pub fn execute(args: SyncArgs) -> io::Result { } if final_status.success() { - let pull_request_update_plan = - sync::plan_pull_request_updates(&restacked_branch_names)?; + let pull_request_update_plan = if animate { + plan_pull_request_updates_with_animation( + runtime + .as_ref() + .expect("animation runtime should exist for TTY sync"), + &restacked_branch_names, + )? + } else { + sync::plan_pull_request_updates(&restacked_branch_names)? + }; if !pull_request_update_plan.actions.is_empty() { if printed_output { println!(); } - let updated_pull_requests = - sync::execute_pull_request_update_plan(&pull_request_update_plan)?; + let updated_pull_requests = if animate { + execute_pull_request_update_plan_with_animation( + runtime + .as_ref() + .expect("animation runtime should exist for TTY sync"), + pull_request_update_plan.clone(), + )? + } else { + sync::execute_pull_request_update_plan(&pull_request_update_plan)? + }; let output = format_pull_request_update_success_output(&updated_pull_requests); if !output.is_empty() { @@ -235,7 +251,16 @@ pub fn execute(args: SyncArgs) -> io::Result { } else { println!(); - let push_outcome = sync::execute_remote_push_plan(&push_plan)?; + let push_outcome = if animate { + execute_remote_push_plan_with_animation( + runtime + .as_ref() + .expect("animation runtime should exist for TTY sync"), + push_plan.clone(), + )? + } else { + sync::execute_remote_push_plan(&push_plan)? + }; final_status = push_outcome.status; if push_outcome.status.success() { @@ -322,6 +347,8 @@ enum WorkerMessage { struct SyncAnimationSession { terminal: Option, stage: Option, + status: Option, + status_frame_index: usize, initial_local_view: Option, } @@ -335,12 +362,30 @@ impl SyncAnimationSession { Self { terminal: None, stage: None, + status: None, + status_frame_index: 0, initial_local_view, } } fn apply(&mut self, event: SyncEvent) -> io::Result<()> { match event { + SyncEvent::StatusChanged(status) => { + if self.status.as_ref() == Some(&status) { + return Ok(()); + } + + self.status = Some(status); + self.status_frame_index = 0; + + if self.stage.is_some() || self.terminal.is_some() { + self.render_active()?; + } else { + self.start_terminal_and_render_active()?; + } + + Ok(()) + } SyncEvent::StageStarted(SyncStage::LocalSync { .. }) => { if let Some(ActiveSyncStage::Local(animation)) = self.stage.as_mut() { if animation.apply_event(&event) { @@ -356,11 +401,8 @@ impl SyncAnimationSession { let mut animation = render::SyncAnimation::new(&view); animation.apply_event(&event); - let mut terminal = render::AnimationTerminal::start()?; - terminal.render(&animation.render_active())?; - self.terminal = Some(terminal); self.stage = Some(ActiveSyncStage::Local(animation)); - Ok(()) + self.start_terminal_and_render_active() } SyncEvent::StageStarted(SyncStage::CleanupResume { plan, @@ -376,18 +418,28 @@ impl SyncAnimationSession { &untracked_branches, &active_branch_name, ); - let mut terminal = render::AnimationTerminal::start()?; - terminal.render(&animation.render_active())?; - self.terminal = Some(terminal); self.stage = Some(ActiveSyncStage::Cleanup(animation)); - Ok(()) + self.start_terminal_and_render_active() } SyncEvent::Cleanup(clean_event) => { let Some(ActiveSyncStage::Cleanup(animation)) = self.stage.as_mut() else { return Ok(()); }; - if animation.apply_event(&clean_event) { + let status_changed = + if let Some(status) = render::status_from_clean_event(&clean_event) { + if self.status.as_ref() == Some(&status) { + false + } else { + self.status = Some(status); + self.status_frame_index = 0; + true + } + } else { + false + }; + + if animation.apply_event(&clean_event) || status_changed { self.render_active()?; } @@ -408,14 +460,19 @@ impl SyncAnimationSession { } fn tick(&mut self) -> io::Result<()> { - let Some(stage) = self.stage.as_mut() else { - return Ok(()); - }; + let mut changed = false; - let changed = match stage { - ActiveSyncStage::Local(animation) => animation.tick(), - ActiveSyncStage::Cleanup(animation) => animation.tick(), - }; + if self.status.is_some() { + self.status_frame_index = self.status_frame_index.wrapping_add(1); + changed = true; + } + + if let Some(stage) = self.stage.as_mut() { + changed |= match stage { + ActiveSyncStage::Local(animation) => animation.tick(), + ActiveSyncStage::Cleanup(animation) => animation.tick(), + }; + } if changed { self.render_active()?; @@ -429,7 +486,7 @@ impl SyncAnimationSession { return Ok(()); }; let Some(stage) = self.stage.take() else { - return Ok(()); + return terminal.finish_and_clear(); }; let frame = match stage { @@ -444,7 +501,7 @@ impl SyncAnimationSession { return Ok(()); }; let Some(stage) = self.stage.take() else { - return Ok(()); + return terminal.finish_and_clear(); }; let frame = match stage { @@ -459,15 +516,79 @@ impl SyncAnimationSession { return Ok(()); }; let Some(stage) = self.stage.as_ref() else { + if let Some(status) = self.status.as_ref() { + let frame = + render::render_active_frame(Some((status, self.status_frame_index)), None); + return terminal.render(&frame); + } + return Ok(()); }; - let frame = match stage { + let body = match stage { ActiveSyncStage::Local(animation) => animation.render_active(), ActiveSyncStage::Cleanup(animation) => animation.render_active(), }; + let frame = render::render_active_frame( + self.status + .as_ref() + .map(|status| (status, self.status_frame_index)), + Some(&body), + ); terminal.render(&frame) } + + fn start_terminal_and_render_active(&mut self) -> io::Result<()> { + if self.terminal.is_none() { + self.terminal = Some(render::AnimationTerminal::start()?); + } + + self.render_active() + } +} + +struct SyncStatusSession { + terminal: render::AnimationTerminal, + status: SyncStatus, + frame_index: usize, +} + +impl SyncStatusSession { + fn start(status: SyncStatus) -> io::Result { + let mut session = Self { + terminal: render::AnimationTerminal::start()?, + status, + frame_index: 0, + }; + session.render()?; + Ok(session) + } + + fn apply(&mut self, status: SyncStatus) -> io::Result<()> { + if self.status == status { + return Ok(()); + } + + self.status = status; + self.frame_index = 0; + self.render() + } + + fn tick(&mut self) -> io::Result<()> { + self.frame_index = self.frame_index.wrapping_add(1); + self.render() + } + + fn finish_clear(&mut self) -> io::Result<()> { + self.terminal.finish_and_clear() + } + + fn render(&mut self) -> io::Result<()> { + self.terminal.render(&render::render_active_frame( + Some((&self.status, self.frame_index)), + None, + )) + } } fn execute_sync_with_animation( @@ -528,6 +649,148 @@ async fn drive_sync_animation( } } +fn plan_pull_request_updates_with_animation( + runtime: &Runtime, + restacked_branch_names: &[String], +) -> io::Result { + let restacked_branch_names = restacked_branch_names.to_vec(); + execute_status_task_with_animation( + runtime, + SyncStatus::InspectingPullRequestUpdates, + move |_| sync::plan_pull_request_updates(&restacked_branch_names), + ) +} + +fn execute_pull_request_update_plan_with_animation( + runtime: &Runtime, + plan: sync::PullRequestUpdatePlan, +) -> io::Result> { + execute_status_task_with_animation( + runtime, + SyncStatus::InspectingPullRequestUpdates, + move |sender| { + sync::execute_pull_request_update_plan_with_reporter(&plan, &mut |status| { + let _ = sender.blocking_send(WorkerMessage::Event(status)); + Ok(()) + }) + }, + ) +} + +fn execute_remote_push_plan_with_animation( + runtime: &Runtime, + plan: sync::RemotePushPlan, +) -> io::Result { + let initial_status = plan + .actions + .first() + .map(|action| SyncStatus::PushingRemoteBranch { + branch_name: action.target.branch_name.clone(), + remote_name: action.target.remote_name.clone(), + kind: action.kind, + }) + .expect("remote push animation requires at least one action"); + + execute_status_task_with_animation(runtime, initial_status, move |sender| { + sync::execute_remote_push_plan_with_reporter(&plan, &mut |status| { + let _ = sender.blocking_send(WorkerMessage::Event(status)); + Ok(()) + }) + }) +} + +fn execute_status_task_with_animation( + runtime: &Runtime, + initial_status: SyncStatus, + task: Task, +) -> io::Result +where + Outcome: Send + 'static, + Task: FnOnce(mpsc::Sender>) -> io::Result + + Send + + 'static, +{ + runtime.block_on(execute_status_task_with_animation_async( + initial_status, + task, + )) +} + +async fn execute_status_task_with_animation_async( + initial_status: SyncStatus, + task: Task, +) -> io::Result +where + Outcome: Send + 'static, + Task: FnOnce(mpsc::Sender>) -> io::Result + + Send + + 'static, +{ + let (sender, mut receiver) = mpsc::channel::>(64); + let worker = tokio::task::spawn_blocking(move || { + let task_sender = sender.clone(); + let _ = sender.blocking_send(WorkerMessage::Event(initial_status)); + let outcome = task(task_sender); + let _ = sender.blocking_send(WorkerMessage::Finished(outcome)); + }); + + let mut animation = None; + let outcome = drive_status_animation(&mut animation, &mut receiver).await; + let worker_result = worker.await; + + if let Some(animation) = animation.as_mut() { + animation.finish_clear()?; + } + + if let Err(err) = worker_result { + return Err(io::Error::other(err.to_string())); + } + + outcome +} + +async fn drive_status_animation( + animation: &mut Option, + receiver: &mut mpsc::Receiver>, +) -> io::Result { + loop { + match time::timeout(Duration::from_millis(80), receiver.recv()).await { + Ok(Some(WorkerMessage::Event(status))) => { + if let Some(animation) = animation.as_mut() { + animation.apply(status)?; + } else { + *animation = Some(SyncStatusSession::start(status)?); + } + } + Ok(Some(WorkerMessage::Finished(outcome))) => { + return drain_worker_messages( + receiver, + |status| { + if let Some(animation) = animation.as_mut() { + animation.apply(status)?; + } else { + *animation = Some(SyncStatusSession::start(status)?); + } + + Ok(()) + }, + outcome, + ); + } + Ok(None) => { + return Err(io::Error::other( + "sync status animation worker ended unexpectedly", + )); + } + Err(_) => { + if let Some(animation) = animation.as_mut() { + animation.tick()?; + } + } + } + } +} + fn execute_cleanup_with_animation( runtime: &Runtime, plan: &clean::CleanPlan, @@ -539,8 +802,13 @@ async fn execute_cleanup_with_animation_async( plan: clean::CleanPlan, ) -> io::Result { let mut animation = super::clean::render::CleanAnimation::new(&plan); + let mut status = initial_cleanup_status(&plan); + let mut status_frame_index = 0; let mut terminal = render::AnimationTerminal::start()?; - terminal.render(&animation.render_active())?; + terminal.render(&render::render_active_frame( + status.as_ref().map(|status| (status, status_frame_index)), + Some(&animation.render_active()), + ))?; let (sender, mut receiver) = mpsc::channel::>(64); @@ -552,7 +820,14 @@ async fn execute_cleanup_with_animation_async( let _ = sender.blocking_send(WorkerMessage::Finished(outcome)); }); - let outcome = drive_cleanup_animation(&mut terminal, &mut animation, &mut receiver).await; + let outcome = drive_cleanup_animation( + &mut terminal, + &mut animation, + &mut status, + &mut status_frame_index, + &mut receiver, + ) + .await; let worker_result = worker.await; if let Err(err) = worker_result { @@ -562,9 +837,15 @@ async fn execute_cleanup_with_animation_async( let outcome = outcome?; if outcome.status.success() { - terminal.finish(&animation.render_final())?; + terminal.finish(&render::render_active_frame( + None, + Some(&animation.render_final()), + ))?; } else { - terminal.finish(&animation.render_active())?; + terminal.finish(&render::render_active_frame( + None, + Some(&animation.render_active()), + ))?; if outcome.paused { common::print_restack_pause_guidance(outcome.failure_output.as_deref()); } else { @@ -578,21 +859,32 @@ async fn execute_cleanup_with_animation_async( async fn drive_cleanup_animation( terminal: &mut render::AnimationTerminal, animation: &mut super::clean::render::CleanAnimation, + status: &mut Option, + status_frame_index: &mut usize, receiver: &mut mpsc::Receiver>, ) -> io::Result { loop { match time::timeout(Duration::from_millis(80), receiver.recv()).await { Ok(Some(WorkerMessage::Event(event))) => { - if animation.apply_event(&event) { - terminal.render(&animation.render_active())?; + let status_changed = apply_cleanup_status(status, status_frame_index, &event); + if animation.apply_event(&event) || status_changed { + terminal.render(&render::render_active_frame( + status.as_ref().map(|status| (status, *status_frame_index)), + Some(&animation.render_active()), + ))?; } } Ok(Some(WorkerMessage::Finished(outcome))) => { return drain_worker_messages( receiver, |event| { - if animation.apply_event(&event) { - terminal.render(&animation.render_active())?; + let status_changed = + apply_cleanup_status(status, status_frame_index, &event); + if animation.apply_event(&event) || status_changed { + terminal.render(&render::render_active_frame( + status.as_ref().map(|status| (status, *status_frame_index)), + Some(&animation.render_active()), + ))?; } Ok(()) @@ -606,14 +898,67 @@ async fn drive_cleanup_animation( )); } Err(_) => { + let mut changed = false; + + if status.is_some() { + *status_frame_index = (*status_frame_index).wrapping_add(1); + changed = true; + } + if animation.tick() { - terminal.render(&animation.render_active())?; + changed = true; + } + + if changed { + terminal.render(&render::render_active_frame( + status.as_ref().map(|status| (status, *status_frame_index)), + Some(&animation.render_active()), + ))?; } } } } } +fn initial_cleanup_status(plan: &clean::CleanPlan) -> Option { + let candidate = plan.candidates.first()?; + + if let Some(restack_branch) = candidate.restack_plan.first() { + return Some(SyncStatus::RestackingBranch { + branch_name: restack_branch.branch_name.clone(), + onto_branch: restack_branch.onto_branch.clone(), + }); + } + + if candidate.is_deleted_locally() { + Some(SyncStatus::ArchivingBranch { + branch_name: candidate.branch_name.clone(), + }) + } else { + Some(SyncStatus::DeletingBranch { + branch_name: candidate.branch_name.clone(), + }) + } +} + +fn apply_cleanup_status( + status: &mut Option, + status_frame_index: &mut usize, + event: &clean::CleanEvent, +) -> bool { + let Some(next_status) = render::status_from_clean_event(event) else { + return false; + }; + + if status.as_ref() == Some(&next_status) { + return false; + } + + *status = Some(next_status); + *status_frame_index = 0; + true +} + fn drain_worker_messages( receiver: &mut mpsc::Receiver>, mut apply_event: impl FnMut(Event) -> io::Result<()>, diff --git a/src/cli/sync/render.rs b/src/cli/sync/render.rs index f8a13dc..777e907 100644 --- a/src/cli/sync/render.rs +++ b/src/cli/sync/render.rs @@ -1,12 +1,65 @@ use crate::cli::common; +use crate::core::clean::CleanEvent; use crate::core::restack::RestackPreview; -use crate::core::sync::{SyncEvent, SyncStage}; +use crate::core::sync::{RemotePushActionKind, SyncEvent, SyncStage, SyncStatus}; use crate::core::tree::{TreeNode, TreeView}; use crate::ui::markers; use crate::ui::palette::Accent; pub use super::super::operation::AnimationTerminal; +pub fn render_active_frame(status: Option<(&SyncStatus, usize)>, body: Option<&str>) -> String { + let Some((status, frame_index)) = status else { + return body.unwrap_or_default().to_string(); + }; + + let header = render_status_header(status, frame_index); + + match body.filter(|body| !body.is_empty()) { + Some(body) => format!("{header}\n\n{body}"), + None => header, + } +} + +pub fn render_status_header(status: &SyncStatus, frame_index: usize) -> String { + let label = format_status_text(status); + let throbber = markers::THROBBER_FRAMES[frame_index % markers::THROBBER_FRAMES.len()]; + + format!( + "{} {}", + Accent::SyncInFlight.paint_ansi(&label), + Accent::SyncInFlight.paint_ansi(throbber) + ) +} + +pub fn status_from_clean_event(event: &CleanEvent) -> Option { + match event { + CleanEvent::RebaseStarted { + branch_name, + onto_branch, + } + | CleanEvent::RebaseProgress { + branch_name, + onto_branch, + .. + } => Some(SyncStatus::RestackingBranch { + branch_name: branch_name.clone(), + onto_branch: onto_branch.clone(), + }), + CleanEvent::DeleteStarted { branch_name } => Some(SyncStatus::DeletingBranch { + branch_name: branch_name.clone(), + }), + CleanEvent::ArchiveStarted { branch_name } => Some(SyncStatus::ArchivingBranch { + branch_name: branch_name.clone(), + }), + CleanEvent::SwitchingToTrunk { .. } + | CleanEvent::SwitchedToTrunk { .. } + | CleanEvent::RebaseCompleted { .. } + | CleanEvent::DeleteCompleted { .. } + | CleanEvent::ArchiveCompleted { .. } => None, + } +} + pub struct SyncAnimation { root_label: Option, roots: Vec, @@ -25,6 +78,7 @@ impl SyncAnimation { pub fn apply_event(&mut self, event: &SyncEvent) -> bool { match event { + SyncEvent::StatusChanged(_) => false, SyncEvent::StageStarted(SyncStage::LocalSync { active_branch_name, deleted_branches, @@ -277,6 +331,43 @@ fn format_branch_text(branch_name: &str, pull_request_number: Option) -> St } } +fn format_status_text(status: &SyncStatus) -> String { + match status { + SyncStatus::FetchingRemotes => "Fetching remotes".to_string(), + SyncStatus::RepairingClosedPullRequests => "Repairing closed pull requests".to_string(), + SyncStatus::RemovingMergedLocalBranches => "Removing merged local branches".to_string(), + SyncStatus::ReconcilingDeletedLocalBranch { step_branch_name } => { + format!("Reconciling deleted local branch {step_branch_name}") + } + SyncStatus::PreparingRestack { step_branch_name } => { + format!("Preparing restack for {step_branch_name}") + } + SyncStatus::RestackingBranch { + branch_name, + onto_branch, + } => format!("Restacking {branch_name} onto {onto_branch}"), + SyncStatus::InspectingPullRequestUpdates => "Inspecting pull request updates".to_string(), + SyncStatus::UpdatingPullRequestBase { + branch_name, + pull_request_number, + } => format!("Updating pull request #{pull_request_number} base for {branch_name}"), + SyncStatus::PushingRemoteBranch { + branch_name, + remote_name, + kind, + } => format!( + "{} {branch_name} on {remote_name}", + match kind { + RemotePushActionKind::CreateRemoteBranch => "Creating remote branch", + RemotePushActionKind::UpdateRemoteBranch => "Pushing", + RemotePushActionKind::ForceUpdateRemoteBranch => "Force-pushing", + } + ), + SyncStatus::DeletingBranch { branch_name } => format!("Deleting {branch_name}"), + SyncStatus::ArchivingBranch { branch_name } => format!("Archiving {branch_name}"), + } +} + fn format_completed_label(branch_name: &str, pull_request_number: Option) -> String { let label = format_branch_text(branch_name, pull_request_number); format!( @@ -332,10 +423,14 @@ fn format_branch_label(node: &VisualTreeNode) -> String { #[cfg(test)] mod tests { - use super::{SyncAnimation, render_completed_tree}; + use super::{ + SyncAnimation, render_active_frame, render_completed_tree, render_status_header, + status_from_clean_event, + }; + use crate::core::clean::CleanEvent; use crate::core::restack::RestackPreview; use crate::core::store::PendingSyncPhase; - use crate::core::sync::{SyncEvent, SyncStage}; + use crate::core::sync::{SyncEvent, SyncStage, SyncStatus}; use crate::core::tree::{TreeLabel, TreeNode, TreeView}; fn sample_view() -> TreeView { @@ -429,6 +524,40 @@ mod tests { ); } + #[test] + fn renders_header_only_status_bar() { + assert_eq!( + render_active_frame(Some((&SyncStatus::FetchingRemotes, 0)), None), + "\u{1b}[38;5;208mFetching remotes\u{1b}[0m \u{1b}[38;5;208m|\u{1b}[0m" + ); + } + + #[test] + fn renders_status_bar_above_existing_tree_body() { + let animation = SyncAnimation::new(&sample_view()); + + assert_eq!( + render_active_frame( + Some(( + &SyncStatus::PreparingRestack { + step_branch_name: "feat/auth".into(), + }, + 1 + )), + Some(&animation.render_active()), + ), + concat!( + "\u{1b}[38;5;208mPreparing restack for feat/auth\u{1b}[0m ", + "\u{1b}[38;5;208m/\u{1b}[0m\n\n", + "main\n", + "└── feat/auth (#42)\n", + " ├── feat/auth-api\n", + " │ └── feat/auth-api-tests\n", + " └── feat/auth-ui" + ) + ); + } + #[test] fn keeps_completed_branches_green_and_prunes_archived_nodes_from_final_view() { let mut animation = SyncAnimation::new(&sample_view()); @@ -503,4 +632,100 @@ mod tests { assert!(after.contains("[2/5]")); assert!(after.contains("\u{1b}[38;5;208mfeat/auth-api\u{1b}[0m")); } + + #[test] + fn header_throbber_advances_without_changing_tree_body() { + let mut animation = SyncAnimation::new(&sample_view()); + + animation.apply_event(&SyncEvent::StageStarted(SyncStage::LocalSync { + phase: PendingSyncPhase::RestackOutdatedLocalStacks, + step_branch_name: "feat/auth".into(), + active_branch_name: "feat/auth-api".into(), + deleted_branches: Vec::new(), + restacked_branches: Vec::new(), + })); + animation.apply_event(&SyncEvent::RestackProgress { + branch_name: "feat/auth-api".into(), + onto_branch: "feat/auth".into(), + current_commit: 2, + total_commits: 5, + }); + + let body = animation.render_active(); + let before = render_active_frame(Some((&SyncStatus::FetchingRemotes, 0)), Some(&body)); + let after = render_active_frame(Some((&SyncStatus::FetchingRemotes, 1)), Some(&body)); + + assert!(before.contains("Fetching remotes")); + assert!(after.contains("Fetching remotes")); + assert!(before.contains("\u{1b}[38;5;208m|\u{1b}[0m")); + assert!(after.contains("\u{1b}[38;5;208m/\u{1b}[0m")); + assert_eq!( + before.split_once("\n\n").map(|(_, body)| body), + after.split_once("\n\n").map(|(_, body)| body) + ); + assert!(after.contains("[2/5]")); + } + + #[test] + fn final_frames_omit_status_header() { + let animation = SyncAnimation::new(&sample_view()); + + assert_eq!( + render_active_frame(None, Some(&animation.render_active())), + concat!( + "main\n", + "└── feat/auth (#42)\n", + " ├── feat/auth-api\n", + " │ └── feat/auth-api-tests\n", + " └── feat/auth-ui" + ) + ); + } + + #[test] + fn maps_cleanup_events_to_sync_status_bar_actions() { + assert_eq!( + status_from_clean_event(&CleanEvent::DeleteStarted { + branch_name: "feat/auth".into(), + }), + Some(SyncStatus::DeletingBranch { + branch_name: "feat/auth".into(), + }) + ); + assert_eq!( + status_from_clean_event(&CleanEvent::RebaseProgress { + branch_name: "feat/auth-ui".into(), + onto_branch: "main".into(), + current_commit: 1, + total_commits: 2, + }), + Some(SyncStatus::RestackingBranch { + branch_name: "feat/auth-ui".into(), + onto_branch: "main".into(), + }) + ); + assert_eq!( + status_from_clean_event(&CleanEvent::ArchiveCompleted { + branch_name: "feat/auth".into(), + }), + None + ); + } + + #[test] + fn renders_status_header_with_right_hand_throbber() { + assert_eq!( + render_status_header( + &SyncStatus::UpdatingPullRequestBase { + branch_name: "feat/auth".into(), + pull_request_number: 42, + }, + 2, + ), + concat!( + "\u{1b}[38;5;208mUpdating pull request #42 base for feat/auth\u{1b}[0m ", + "\u{1b}[38;5;208m-\u{1b}[0m" + ) + ); + } } diff --git a/src/core/sync.rs b/src/core/sync.rs index eb4a4df..354bd1d 100644 --- a/src/core/sync.rs +++ b/src/core/sync.rs @@ -38,8 +38,42 @@ pub enum SyncStage { }, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SyncStatus { + FetchingRemotes, + RepairingClosedPullRequests, + RemovingMergedLocalBranches, + ReconcilingDeletedLocalBranch { + step_branch_name: String, + }, + PreparingRestack { + step_branch_name: String, + }, + RestackingBranch { + branch_name: String, + onto_branch: String, + }, + InspectingPullRequestUpdates, + UpdatingPullRequestBase { + branch_name: String, + pull_request_number: u64, + }, + PushingRemoteBranch { + branch_name: String, + remote_name: String, + kind: RemotePushActionKind, + }, + DeletingBranch { + branch_name: String, + }, + ArchivingBranch { + branch_name: String, + }, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub enum SyncEvent { + StatusChanged(SyncStatus), StageStarted(SyncStage), BranchArchived { branch_name: String, @@ -257,6 +291,10 @@ where let mut restacked_branches = payload.restacked_branches.clone(); restacked_branches.extend(pending_operation.completed_branches().iter().cloned()); let active_action = pending_operation.active_action().clone(); + reporter(SyncEvent::StatusChanged(SyncStatus::RestackingBranch { + branch_name: active_action.branch_name.clone(), + onto_branch: active_action.new_base.branch_name.clone(), + }))?; reporter(SyncEvent::StageStarted(SyncStage::CleanupResume { plan: clean::plan_for_resume(&payload)?, active_branch_name: active_action.branch_name.clone(), @@ -357,14 +395,21 @@ where workflow::ensure_ready_for_operation(&session.repo, "sync")?; workflow::ensure_no_pending_operation(&session.paths, "sync")?; clean::reconcile_branch_divergence_state(&mut session)?; + reporter(SyncEvent::StatusChanged(SyncStatus::FetchingRemotes))?; let remote_sync_enabled = fetch_sync_remotes(&session)?; let repaired_pull_requests = if remote_sync_enabled { + reporter(SyncEvent::StatusChanged( + SyncStatus::RepairingClosedPullRequests, + ))?; repair_closed_pull_requests_for_deleted_parent_branches(&session)? } else { Vec::new() }; let original_branch = git::current_branch_name()?; if remote_sync_enabled { + reporter(SyncEvent::StatusChanged( + SyncStatus::RemovingMergedLocalBranches, + ))?; delete_local_branches_merged_into_deleted_parent_branches(&session, &original_branch)?; } let outcome = execute_local_sync( @@ -440,6 +485,10 @@ fn report_resumed_full_sync_stage_started( where F: FnMut(SyncEvent) -> io::Result<()>, { + reporter(SyncEvent::StatusChanged(status_for_local_sync_phase( + payload.phase, + &payload.step_branch_name, + )))?; let mut resumed_restacked_branches = payload.restacked_branches.clone(); resumed_restacked_branches.extend(pending_operation.completed_branches().iter().cloned()); @@ -460,6 +509,10 @@ fn report_sync_continue_progress( where F: FnMut(SyncEvent) -> io::Result<()>, { + reporter(SyncEvent::StatusChanged(SyncStatus::RestackingBranch { + branch_name: action.branch_name.clone(), + onto_branch: action.new_base.branch_name.clone(), + }))?; reporter(SyncEvent::RestackProgress { branch_name: action.branch_name.clone(), onto_branch: action.new_base.branch_name.clone(), @@ -476,6 +529,10 @@ fn report_cleanup_continue_progress( where F: FnMut(SyncEvent) -> io::Result<()>, { + reporter(SyncEvent::StatusChanged(SyncStatus::RestackingBranch { + branch_name: action.branch_name.clone(), + onto_branch: action.new_base.branch_name.clone(), + }))?; reporter(SyncEvent::Cleanup(clean::CleanEvent::RebaseProgress { branch_name: action.branch_name.clone(), onto_branch: action.new_base.branch_name.clone(), @@ -813,6 +870,10 @@ fn report_local_sync_stage_started( where F: FnMut(SyncEvent) -> io::Result<()>, { + reporter(SyncEvent::StatusChanged(status_for_local_sync_phase( + phase, + step_branch_name, + )))?; reporter(SyncEvent::StageStarted(SyncStage::LocalSync { phase, step_branch_name: step_branch_name.to_string(), @@ -822,6 +883,19 @@ where })) } +fn status_for_local_sync_phase(phase: PendingSyncPhase, step_branch_name: &str) -> SyncStatus { + match phase { + PendingSyncPhase::ReconcileDeletedLocalBranches => { + SyncStatus::ReconcilingDeletedLocalBranch { + step_branch_name: step_branch_name.to_string(), + } + } + PendingSyncPhase::RestackOutdatedLocalStacks => SyncStatus::PreparingRestack { + step_branch_name: step_branch_name.to_string(), + }, + } +} + fn report_restack_event( reporter: &mut F, event: workflow::RestackExecutionEvent<'_>, @@ -830,11 +904,21 @@ where F: FnMut(SyncEvent) -> io::Result<()>, { match event { - workflow::RestackExecutionEvent::Started(action) => reporter(SyncEvent::RestackStarted { - branch_name: action.branch_name.clone(), - onto_branch: action.new_base.branch_name.clone(), - }), + workflow::RestackExecutionEvent::Started(action) => { + reporter(SyncEvent::StatusChanged(SyncStatus::RestackingBranch { + branch_name: action.branch_name.clone(), + onto_branch: action.new_base.branch_name.clone(), + }))?; + reporter(SyncEvent::RestackStarted { + branch_name: action.branch_name.clone(), + onto_branch: action.new_base.branch_name.clone(), + }) + } workflow::RestackExecutionEvent::Progress { action, progress } => { + reporter(SyncEvent::StatusChanged(SyncStatus::RestackingBranch { + branch_name: action.branch_name.clone(), + onto_branch: action.new_base.branch_name.clone(), + }))?; reporter(SyncEvent::RestackProgress { branch_name: action.branch_name.clone(), onto_branch: action.new_base.branch_name.clone(), @@ -1264,9 +1348,24 @@ pub fn plan_remote_pushes( } pub fn execute_remote_push_plan(plan: &RemotePushPlan) -> io::Result { + execute_remote_push_plan_with_reporter(plan, &mut |_| Ok(())) +} + +pub fn execute_remote_push_plan_with_reporter( + plan: &RemotePushPlan, + reporter: &mut F, +) -> io::Result +where + F: FnMut(SyncStatus) -> io::Result<()>, +{ let mut pushed_actions = Vec::new(); for action in &plan.actions { + reporter(SyncStatus::PushingRemoteBranch { + branch_name: action.target.branch_name.clone(), + remote_name: action.target.remote_name.clone(), + kind: action.kind, + })?; let push_output = match action.kind { RemotePushActionKind::CreateRemoteBranch | RemotePushActionKind::UpdateRemoteBranch => { git::push_branch_to_remote(&action.target)? @@ -1344,9 +1443,23 @@ pub fn plan_pull_request_updates( pub fn execute_pull_request_update_plan( plan: &PullRequestUpdatePlan, ) -> io::Result> { + execute_pull_request_update_plan_with_reporter(plan, &mut |_| Ok(())) +} + +pub fn execute_pull_request_update_plan_with_reporter( + plan: &PullRequestUpdatePlan, + reporter: &mut F, +) -> io::Result> +where + F: FnMut(SyncStatus) -> io::Result<()>, +{ let mut updated_actions = Vec::new(); for action in &plan.actions { + reporter(SyncStatus::UpdatingPullRequestBase { + branch_name: action.branch_name.clone(), + pull_request_number: action.pull_request_number, + })?; gh::retarget_pull_request_base(action.pull_request_number, &action.new_base_branch_name) .map_err(|err| { io::Error::other(format!( @@ -1431,19 +1544,89 @@ fn plan_remote_push_action( #[cfg(test)] mod tests { use super::{ - RemotePushActionKind, SyncEvent, SyncOptions, SyncStage, plan_remote_pushes, - pull_request_needs_repair, run, run_with_reporter, + PullRequestUpdatePlan, RemotePushActionKind, RemotePushPlan, SyncEvent, SyncOptions, + SyncStage, SyncStatus, execute_pull_request_update_plan_with_reporter, + execute_remote_push_plan_with_reporter, plan_remote_pushes, pull_request_needs_repair, run, + run_with_reporter, }; use crate::core::gh::{PullRequestState, PullRequestStatus}; + use crate::core::git::BranchPushTarget; use crate::core::test_support::{ - append_file, commit_file, create_tracked_branch, git_ok, initialize_main_repo, + append_file, commit_file, create_tracked_branch, git_ok, git_output, initialize_main_repo, with_temp_repo, }; + use std::env; + use std::fs; + use std::path::Path; + + #[cfg(unix)] + use std::os::unix::fs::PermissionsExt; fn initialize_origin_remote(repo: &std::path::Path) { - git_ok(repo, &["init", "--bare", "origin.git"]); - git_ok(repo, &["remote", "add", "origin", "origin.git"]); + git_ok(repo, &["init", "--bare", ".git/origin.git"]); + git_ok(repo, &["remote", "add", "origin", ".git/origin.git"]); git_ok(repo, &["push", "-u", "origin", "main"]); + git_ok( + repo, + &[ + "--git-dir=.git/origin.git", + "symbolic-ref", + "HEAD", + "refs/heads/main", + ], + ); + } + + fn install_fake_executable(bin_dir: &Path, name: &str, script: &str) { + fs::create_dir_all(bin_dir).unwrap(); + let path = bin_dir.join(name); + fs::write(&path, script).unwrap(); + #[cfg(unix)] + { + let mut permissions = fs::metadata(&path).unwrap().permissions(); + permissions.set_mode(0o755); + fs::set_permissions(path, permissions).unwrap(); + } + } + + fn path_with_prepend(dir: &Path) -> String { + let existing_path = env::var("PATH").unwrap_or_default(); + if existing_path.is_empty() { + dir.display().to_string() + } else { + format!("{}:{existing_path}", dir.display()) + } + } + + struct EnvVarGuard { + key: &'static str, + original_value: Option, + } + + impl EnvVarGuard { + fn set(key: &'static str, value: String) -> Self { + let original_value = env::var(key).ok(); + unsafe { + env::set_var(key, value); + } + Self { + key, + original_value, + } + } + } + + impl Drop for EnvVarGuard { + fn drop(&mut self) { + match &self.original_value { + Some(value) => unsafe { + env::set_var(self.key, value); + }, + None => unsafe { + env::remove_var(self.key); + }, + } + } } #[test] @@ -1588,6 +1771,190 @@ mod tests { )); } + #[test] + fn emits_preflight_status_events_before_first_local_sync_stage() { + with_temp_repo("dgr-sync-core", |repo| { + initialize_main_repo(repo); + initialize_origin_remote(repo); + crate::core::init::run(&crate::core::init::InitOptions::default()).unwrap(); + create_tracked_branch("feat/auth"); + commit_file(repo, "auth.txt", "auth\n", "feat: auth"); + git_ok(repo, &["push", "-u", "origin", "feat/auth"]); + git_ok(repo, &["checkout", "main"]); + commit_file(repo, "README.md", "root\nmain\n", "feat: trunk follow-up"); + git_ok(repo, &["checkout", "feat/auth"]); + + let mut events = Vec::new(); + let outcome = run_with_reporter(&SyncOptions::default(), &mut |event| { + events.push(event.clone()); + Ok(()) + }) + .unwrap(); + + assert!(outcome.status.success()); + let fetch_index = events + .iter() + .position(|event| { + matches!(event, SyncEvent::StatusChanged(SyncStatus::FetchingRemotes)) + }) + .unwrap(); + let repair_index = events + .iter() + .position(|event| { + matches!( + event, + SyncEvent::StatusChanged(SyncStatus::RepairingClosedPullRequests) + ) + }) + .unwrap(); + let prune_index = events + .iter() + .position(|event| { + matches!( + event, + SyncEvent::StatusChanged(SyncStatus::RemovingMergedLocalBranches) + ) + }) + .unwrap(); + let stage_index = events + .iter() + .position(|event| { + matches!(event, SyncEvent::StageStarted(SyncStage::LocalSync { .. })) + }) + .unwrap(); + + assert!(fetch_index < repair_index); + assert!(repair_index < prune_index); + assert!(prune_index < stage_index); + }); + } + + #[test] + fn reports_pull_request_base_updates_before_each_retarget() { + with_temp_repo("dgr-sync-core", |repo| { + initialize_main_repo(repo); + + let bin_dir = repo.join("fake-bin"); + let log_path = repo.join("gh.log"); + install_fake_executable( + &bin_dir, + "gh", + &format!( + "#!/bin/sh\nset -eu\nprintf '%s\\n' \"$*\" >> \"{}\"\n", + log_path.display() + ), + ); + fs::write(&log_path, "").unwrap(); + let _path_guard = EnvVarGuard::set("PATH", path_with_prepend(&bin_dir)); + + let plan = PullRequestUpdatePlan { + actions: vec![ + super::PullRequestUpdateAction { + branch_name: "feat/auth".into(), + pull_request_number: 42, + new_base_branch_name: "main".into(), + }, + super::PullRequestUpdateAction { + branch_name: "feat/auth-ui".into(), + pull_request_number: 43, + new_base_branch_name: "feat/auth".into(), + }, + ], + }; + let mut statuses = Vec::new(); + + let updated = execute_pull_request_update_plan_with_reporter(&plan, &mut |status| { + statuses.push(status); + Ok(()) + }) + .unwrap(); + + assert_eq!(updated, plan.actions); + assert_eq!( + statuses, + vec![ + SyncStatus::UpdatingPullRequestBase { + branch_name: "feat/auth".into(), + pull_request_number: 42, + }, + SyncStatus::UpdatingPullRequestBase { + branch_name: "feat/auth-ui".into(), + pull_request_number: 43, + }, + ] + ); + + let log = fs::read_to_string(log_path).unwrap(); + assert!(log.contains("pr edit 42 --base main")); + assert!(log.contains("pr edit 43 --base feat/auth")); + }); + } + + #[test] + fn reports_remote_push_status_before_each_push() { + with_temp_repo("dgr-sync-core", |repo| { + initialize_main_repo(repo); + initialize_origin_remote(repo); + crate::core::init::run(&crate::core::init::InitOptions::default()).unwrap(); + create_tracked_branch("feat/auth"); + commit_file(repo, "auth.txt", "auth\n", "feat: auth"); + create_tracked_branch("feat/auth-ui"); + commit_file(repo, "ui.txt", "ui\n", "feat: auth ui"); + + let plan = RemotePushPlan { + actions: vec![ + super::RemotePushAction { + target: BranchPushTarget { + remote_name: "origin".into(), + branch_name: "feat/auth".into(), + }, + kind: RemotePushActionKind::CreateRemoteBranch, + }, + super::RemotePushAction { + target: BranchPushTarget { + remote_name: "origin".into(), + branch_name: "feat/auth-ui".into(), + }, + kind: RemotePushActionKind::CreateRemoteBranch, + }, + ], + }; + let mut statuses = Vec::new(); + + let outcome = execute_remote_push_plan_with_reporter(&plan, &mut |status| { + statuses.push(status); + Ok(()) + }) + .unwrap(); + + assert!(outcome.status.success()); + assert_eq!(outcome.pushed_actions, plan.actions); + assert_eq!( + statuses, + vec![ + SyncStatus::PushingRemoteBranch { + branch_name: "feat/auth".into(), + remote_name: "origin".into(), + kind: RemotePushActionKind::CreateRemoteBranch, + }, + SyncStatus::PushingRemoteBranch { + branch_name: "feat/auth-ui".into(), + remote_name: "origin".into(), + kind: RemotePushActionKind::CreateRemoteBranch, + }, + ] + ); + assert!( + git_output(repo, &["ls-remote", "--heads", "origin", "feat/auth"]) + .contains("refs/heads/feat/auth") + ); + assert!( + git_output(repo, &["ls-remote", "--heads", "origin", "feat/auth-ui"]) + .contains("refs/heads/feat/auth-ui") + ); + }); + } + #[test] fn emits_local_sync_restack_events_for_outdated_branch_restack() { with_temp_repo("dgr-sync-core", |repo| { @@ -1611,12 +1978,29 @@ mod tests { assert!(outcome.status.success()); assert!(matches!( &events[0], + SyncEvent::StatusChanged(SyncStatus::FetchingRemotes) + )); + assert!(matches!( + &events[1], + SyncEvent::StatusChanged(SyncStatus::PreparingRestack { + step_branch_name, + }) if step_branch_name == "feat/auth-ui" + )); + assert!(matches!( + &events[2], SyncEvent::StageStarted(SyncStage::LocalSync { step_branch_name, active_branch_name, .. }) if step_branch_name == "feat/auth-ui" && active_branch_name == "feat/auth-ui" )); + assert!(events.iter().any(|event| matches!( + event, + SyncEvent::StatusChanged(SyncStatus::RestackingBranch { + branch_name, + onto_branch, + }) if branch_name == "feat/auth-ui" && onto_branch == "feat/auth" + ))); assert!(events.iter().any(|event| matches!( event, SyncEvent::RestackStarted { @@ -1724,6 +2108,12 @@ mod tests { assert!(paused.paused); assert!(matches!( &events[0], + SyncEvent::StatusChanged(SyncStatus::PreparingRestack { + step_branch_name, + }) if step_branch_name == "feat/auth" + )); + assert!(matches!( + &events[1], SyncEvent::StageStarted(SyncStage::LocalSync { active_branch_name, restacked_branches,