Merge pull request #11093 from gitbutlerapp/copilot/replace-print-statements

fix: Replace println!/eprintln! with writeln! to prevent broken pipe panics

Author: Byron

crates/but/agents.md

@@ -1,7 +1,12 @@
 ### Output
-* Usable output goes to `stdout` with `wrinteln!(stdout, "…")`, with `stdout` being a re-used variable filled with `std::io::stdout()`.
-    - Use `atty::is` to print human output, otherwise print output optimised for use in shell scripts, when single values are returned.
-* Error or side-channel information goes to `stderr` with `writeln!(stderr, "…")`, with `stderr` being a re-used variable filled with `std::io::stderr()` as needed.
+* Usable output goes to `stdout` with `writeln!(stdout, …).ok()`.
+    - Obtain `stdout` once per function using `let mut stdout = std::io::stdout();`
+    - Use `stdout` when writing: `writeln!(stdout, "…").ok();`
+    - Use `atty::is` to print human output, otherwise print output optimised for use in bash scripts, when single values are returned.
+* Error or side-channel information goes to `stderr` with `writeln!(stderr, …).ok()`.
+    - Obtain `stderr` once per function using `let mut stderr = std::io::stderr();`
+    - Use `stderr` when writing: `writeln!(stderr, "…").ok();`
+* The `.ok()` at the end ignores write errors gracefully (e.g., broken pipe) instead of panicking.
 * `--json` only outputs the happy path, there are no JSON errors.
 
 ### Testing

crates/but/src/absorb/mod.rs

@@ -1,4 +1,5 @@
 use std::collections::BTreeMap;
+use std::io::Write;
 
 use bstr::{BString, ByteSlice};
 use but_api::{diff, hex_hash::HexHash, virtual_branches};
@@ -147,8 +148,9 @@ fn absorb_all(
     assignments: &[HunkAssignment],
     dependencies: &Option<HunkDependencies>,
 ) -> anyhow::Result<()> {
+    let mut stdout = std::io::stdout();
     if assignments.is_empty() {
-        println!("No uncommitted changes to absorb");
+        writeln!(stdout, "No uncommitted changes to absorb").ok();
         return Ok(());
     }
 
@@ -311,6 +313,8 @@ fn amend_commit(
     commit_id: gix::ObjectId,
     diff_specs: Vec<DiffSpec>,
 ) -> anyhow::Result<()> {
+    let mut stdout = std::io::stdout();
+    let mut stderr = std::io::stderr();
     // Convert commit_id to HexHash
     let hex_hash = HexHash::from(commit_id);
 
@@ -319,16 +323,20 @@ fn amend_commit(
     )?;
 
     if !outcome.paths_to_rejected_changes.is_empty() {
-        eprintln!(
+        writeln!(
+            stderr,
             "Warning: Failed to absorb {} file(s)",
             outcome.paths_to_rejected_changes.len()
-        );
+        )
+        .ok();
     }
 
-    println!(
+    writeln!(
+        stdout,
         "Absorbed changes into commit {}",
         &commit_id.to_hex().to_string()[..7]
-    );
+    )
+    .ok();
 
     Ok(())
 }

crates/but/src/base.rs

@@ -5,6 +5,7 @@ use gitbutler_branch_actions::upstream_integration::{
     StackStatuses::{UpToDate, UpdatesRequired},
 };
 use gitbutler_project::Project;
+use std::io::Write;
 
 #[derive(Debug, clap::Parser)]
 pub struct Platform {
@@ -20,23 +21,27 @@ pub enum Subcommands {
 }
 
 pub fn handle(cmd: Subcommands, project: &Project, json: bool) -> anyhow::Result<()> {
+    let mut stdout = std::io::stdout();
     match cmd {
         Subcommands::Check => {
             if !json {
-                println!("🔍 Checking base branch status...");
+                writeln!(stdout, "🔍 Checking base branch status...").ok();
             }
             let base_branch = but_api::virtual_branches::fetch_from_remotes(
                 project.id,
                 Some("auto".to_string()),
             )?;
-            println!("\n📍 Base branch:\t\t{}", base_branch.branch_name);
-            println!(
+            writeln!(stdout, "\n📍 Base branch:\t\t{}", base_branch.branch_name).ok();
+            writeln!(
+                stdout,
                 "⏫ Upstream commits:\t{} new commits on {}\n",
                 base_branch.behind, base_branch.branch_name
-            );
+            )
+            .ok();
             let commits = base_branch.recent_commits.iter().take(3);
             for commit in commits {
-                println!(
+                writeln!(
+                    stdout,
                     "\t{} {}",
                     &commit.id[..7],
                     &commit
@@ -46,29 +51,34 @@ pub fn handle(cmd: Subcommands, project: &Project, json: bool) -> anyhow::Result
                         .chars()
                         .take(72)
                         .collect::<String>()
-                );
+                )
+                .ok();
             }
             let hidden_commits = base_branch.behind.saturating_sub(3);
             if hidden_commits > 0 {
-                println!("\t... ({hidden_commits} more - run `but base check --all` to see all)");
+                writeln!(
+                    stdout,
+                    "\t... ({hidden_commits} more - run `but base check --all` to see all)"
+                )
+                .ok();
             }
 
             let status =
                 but_api::virtual_branches::upstream_integration_statuses(project.id, None)?;
 
             match status {
-                UpToDate => println!("\n✅ Everything is up to date"),
+                UpToDate => _ = writeln!(stdout, "\n✅ Everything is up to date").ok(),
                 UpdatesRequired {
                     worktree_conflicts,
                     statuses,
                 } => {
                     if !worktree_conflicts.is_empty() {
-                        println!(
+                        writeln!(stdout,
                             "\n❗️ There are uncommitted changes in the worktree that may conflict with the updates."
-                        );
+                        ).ok();
                     }
                     if !statuses.is_empty() {
-                        println!("\n{}", "Active Branch Status".bold());
+                        writeln!(stdout, "\n{}", "Active Branch Status".bold()).ok();
                         for (_id, status) in statuses {
                             for bs in status.branch_statuses {
                                 let status_icon = match bs.status {
@@ -95,39 +105,45 @@ pub fn handle(cmd: Subcommands, project: &Project, json: bool) -> anyhow::Result
                                     }
                                     Empty => "Nothing to do".normal(),
                                 };
-                                println!("\n{} {} ({})", status_icon, bs.name, status_text);
+                                writeln!(stdout, "\n{} {} ({})", status_icon, bs.name, status_text)
+                                    .ok();
                             }
                         }
                     }
                 }
             }
-            println!("\nRun `but base update` to update your branches");
+            writeln!(stdout, "\nRun `but base update` to update your branches").ok();
             Ok(())
         }
         Subcommands::Update => {
             let status =
                 but_api::virtual_branches::upstream_integration_statuses(project.id, None)?;
             let resolutions = match status {
                 UpToDate => {
-                    println!("✅ Everything is up to date");
+                    writeln!(stdout, "✅ Everything is up to date").ok();
                     None
                 }
                 UpdatesRequired {
                     worktree_conflicts,
                     statuses,
                 } => {
                     if !worktree_conflicts.is_empty() {
-                        println!(
+                        writeln!(stdout,
                             "❗️ There are uncommitted changes in the worktree that may conflict with
                             the updates. Please commit or stash them and try again."
-                        );
+                        )
+                        .ok();
                         None
                     } else {
-                        println!("🔄 Updating branches...");
+                        writeln!(stdout, "🔄 Updating branches...").ok();
                         let mut resolutions = vec![];
                         for (maybe_stack_id, status) in statuses {
                             let Some(stack_id) = maybe_stack_id else {
-                                println!("No stack ID, assuming we're on single-branch mode...",);
+                                writeln!(
+                                    stdout,
+                                    "No stack ID, assuming we're on single-branch mode...",
+                                )
+                                .ok();
                                 continue;
                             };
                             let approach = if status

crates/but/src/branch/list.rs

@@ -1,8 +1,11 @@
+use std::io::Write;
+
 use colored::Colorize;
 use gitbutler_branch_actions::BranchListingFilter;
 use gitbutler_project::Project;
 
 pub async fn list(project: &Project, local: bool) -> Result<(), anyhow::Error> {
+    let mut stdout = std::io::stdout();
     let filter = if local {
         Some(BranchListingFilter {
             local: Some(true),
@@ -34,7 +37,7 @@ pub async fn list(project: &Project, local: bool) -> Result<(), anyhow::Error> {
             &branch_review_map,
         );
 
-        println!("{}{}", branch.name, reviews);
+        writeln!(stdout, "{}{}", branch.name, reviews).ok();
     }
 
     for branch in remote_only_branches {
@@ -43,7 +46,7 @@ pub async fn list(project: &Project, local: bool) -> Result<(), anyhow::Error> {
             &None,
             &branch_review_map,
         );
-        println!("{} {}{}", "(remote)".dimmed(), branch.name, reviews);
+        writeln!(stdout, "{} {}{}", "(remote)".dimmed(), branch.name, reviews).ok();
     }
     Ok(())
 }
@@ -55,6 +58,7 @@ fn print_applied_branches(
         Vec<gitbutler_forge::review::ForgeReview>,
     >,
 ) {
+    let mut stdout = std::io::stdout();
     for stack in applied_stacks {
         let first_branch = stack.heads.first();
         let last_branch = stack.heads.last();
@@ -67,7 +71,7 @@ fn print_applied_branches(
                     &None,
                     branch_review_map,
                 );
-                println!("{}{}", branch_entry.green(), reviews);
+                writeln!(stdout, "{}{}", branch_entry.green(), reviews).ok();
                 continue;
             }
 
@@ -93,7 +97,7 @@ fn print_applied_branches(
                 branch_review_map,
             );
 
-            println!("{}{}", branch_entry.green(), reviews);
+            writeln!(stdout, "{}{}", branch_entry.green(), reviews).ok();
         }
     }
 }

crates/but/src/branch/mod.rs

@@ -60,6 +60,7 @@ pub enum Subcommands {
 }
 
 pub async fn handle(cmd: Option<Subcommands>, project: &Project, json: bool) -> anyhow::Result<()> {
+    let mut stdout = io::stdout();
     match cmd {
         None => {
             let local = false;
@@ -136,11 +137,11 @@ pub async fn handle(cmd: Option<Subcommands>, project: &Project, json: bool) ->
                     branch: branch_name,
                     anchor: anchor_for_json,
                 };
-                println!("{}", serde_json::to_string_pretty(&response)?);
+                writeln!(stdout, "{}", serde_json::to_string_pretty(&response)?).ok();
             } else if atty::is(Stream::Stdout) {
-                println!("Created branch {branch_name}");
+                writeln!(stdout, "Created branch {branch_name}").ok();
             } else {
-                println!("{branch_name}");
+                writeln!(stdout, "{branch_name}").ok();
             }
             Ok(())
         }
@@ -162,7 +163,7 @@ pub async fn handle(cmd: Option<Subcommands>, project: &Project, json: bool) ->
                 }
             }
 
-            println!("Branch '{}' not found in any stack", branch_name);
+            writeln!(stdout, "Branch '{}' not found in any stack", branch_name).ok();
             Ok(())
         }
         Some(Subcommands::Unapply { branch_name, force }) => {
@@ -183,7 +184,7 @@ pub async fn handle(cmd: Option<Subcommands>, project: &Project, json: bool) ->
                 }
             }
 
-            println!("Branch '{}' not found in any stack", branch_name);
+            writeln!(stdout, "Branch '{}' not found in any stack", branch_name).ok();
             Ok(())
         }
     }
@@ -195,6 +196,7 @@ fn confirm_unapply_stack(
     stack_entry: &StackEntry,
     force: bool,
 ) -> Result<(), anyhow::Error> {
+    let mut stdout = io::stdout();
     let branches = stack_entry
         .heads
         .iter()
@@ -203,27 +205,31 @@ fn confirm_unapply_stack(
         .join(", ");
 
     if !force {
-        println!(
+        writeln!(
+            stdout,
             "Are you sure you want to unapply stack with branches '{}'? [y/N]:",
             branches
-        );
+        )
+        .ok();
 
         io::stdout().flush()?;
         let mut input = String::new();
         io::stdin().read_line(&mut input)?;
 
         let input = input.trim().to_lowercase();
         if input != "y" && input != "yes" {
-            println!("Aborted unapply operation.");
+            writeln!(stdout, "Aborted unapply operation.").ok();
             return Ok(());
         }
     }
 
     but_api::virtual_branches::unapply_stack(project.id, sid)?;
-    println!(
+    writeln!(
+        stdout,
         "Unapplied stack with branches '{}' from workspace",
         branches
-    );
+    )
+    .ok();
     Ok(())
 }
 
@@ -233,24 +239,27 @@ fn confirm_branch_deletion(
     branch_name: &str,
     force: bool,
 ) -> Result<(), anyhow::Error> {
+    let mut stdout = io::stdout();
     if !force {
-        println!(
+        writeln!(
+            stdout,
             "Are you sure you want to delete branch '{}'? [y/N]:",
             branch_name
-        );
+        )
+        .ok();
 
         io::stdout().flush()?;
         let mut input = String::new();
         io::stdin().read_line(&mut input)?;
 
         let input = input.trim().to_lowercase();
         if input != "y" && input != "yes" {
-            println!("Aborted branch deletion.");
+            writeln!(stdout, "Aborted branch deletion.").ok();
             return Ok(());
         }
     }
 
     but_api::stack::remove_branch(project.id, sid, branch_name.to_owned())?;
-    println!("Deleted branch {branch_name}");
+    writeln!(stdout, "Deleted branch {branch_name}").ok();
     Ok(())
 }

crates/but/src/command.rs

@@ -1,3 +1,5 @@
+use std::io::Write;
+
 use but_action::Source;
 use but_settings::AppSettings;
 use gitbutler_command_context::CommandContext;
@@ -46,11 +48,12 @@ pub(crate) fn print<T>(this: &T, json: bool) -> anyhow::Result<()>
 where
     T: ?Sized + Serialize + std::fmt::Debug,
 {
+    let mut stdout = std::io::stdout();
     if json {
         let json = serde_json::to_string_pretty(&this)?;
-        println!("{json}");
+        writeln!(stdout, "{json}").ok();
     } else {
-        println!("{this:#?}");
+        writeln!(stdout, "{this:#?}").ok();
     }
     Ok(())
 }