avoid fast fail in post_feedback()

ensures the patch is printed before an error short-circuits the
function.

Author: 2bndy5

cpp-linter/src/error.rs

@@ -70,6 +70,10 @@ pub enum ClientError {
         #[source]
         source: std::io::Error,
     },
+
+    /// Error when failing to write to stdout.
+    #[error("Failed to write to stdout: {0}")]
+    StdoutWriteFailed(#[source] std::io::Error),
 }
 
 /// Errors related to invoking clang tools and processing their output.

cpp-linter/src/rest_client.rs

@@ -2,7 +2,7 @@
 
 use std::{
     env,
-    io::BufRead,
+    io::{BufRead, Write, stdout},
     path::PathBuf,
     sync::{Arc, Mutex},
 };
@@ -147,21 +147,21 @@ impl RestClient {
             if feedback_inputs.step_summary {
                 self.client.append_step_summary(&summary)?;
             }
-            if let Some(summary_output_file) = feedback_inputs.summary_output_file {
+            if let Some(summary_output_file) = &feedback_inputs.summary_output_file {
                 let output_file = if summary_output_file.is_absolute() {
                     summary_output_file
                 } else {
-                    feedback_inputs.repo_root.join(&summary_output_file)
+                    &feedback_inputs.repo_root.join(summary_output_file)
                 };
                 if let Some(parent) = output_file.parent() {
                     std::fs::create_dir_all(parent).map_err(|e| ClientError::MkDirFailed {
                         file_path: output_file.clone(),
                         source: e,
                     })?;
                 }
-                std::fs::write(&output_file, &summary).map_err(|e| {
+                std::fs::write(output_file, &summary).map_err(|e| {
                     ClientError::SummaryOutputFileWriteFailed {
-                        file_path: output_file,
+                        file_path: output_file.to_owned(),
                         source: e,
                     }
                 })?;
@@ -184,35 +184,6 @@ impl RestClient {
         ];
         self.client.write_output_variables(&output_vars)?;
 
-        if feedback_inputs.thread_comments != ThreadComments::Off {
-            // post thread comment for PR or push event
-            if comment.as_ref().is_none_or(|c| c.len() > u16::MAX as usize) {
-                comment = Some(Self::make_comment(
-                    files,
-                    format_checks_failed,
-                    tidy_checks_failed,
-                    &clang_versions,
-                    Some(u16::MAX as u64),
-                )?);
-            }
-            let options = ThreadCommentOptions {
-                policy: if feedback_inputs.thread_comments == ThreadComments::Update {
-                    CommentPolicy::Update
-                } else {
-                    // feedback_inputs.thread_comments is not Off and not Update, so it must be just On.
-                    CommentPolicy::Anew
-                },
-                comment: comment.unwrap_or_default(),
-                kind: if format_checks_failed == 0 && tidy_checks_failed == 0 {
-                    CommentKind::Lgtm
-                } else {
-                    CommentKind::Concerns
-                },
-                marker: COMMENT_MARKER.to_string(),
-                no_lgtm: feedback_inputs.no_lgtm,
-            };
-            self.client.post_thread_comment(options).await?;
-        }
         if self.client.is_pr_event() && feedback_inputs.pr_review {
             let summary_only = ["true", "on", "1"].contains(
                 &env::var("CPP_LINTER_PR_REVIEW_SUMMARY_ONLY")
@@ -230,6 +201,7 @@ impl RestClient {
                     &feedback_inputs.repo_root,
                 )?;
             }
+            self.show_patch(&feedback_inputs)?;
 
             let mut options = ReviewOptions {
                 marker: COMMENT_MARKER.to_string(),
@@ -267,32 +239,37 @@ impl RestClient {
                     .map_err(|e| ClientError::MutexPoisoned(e.to_string()))?;
                 file.maybe_append_patch(&feedback_inputs.repo_root)?;
             }
+            self.show_patch(&feedback_inputs)?;
         }
-        let auto_fix_patch_path = feedback_inputs
-            .repo_root
-            .join(ClangParams::CACHE_DIR)
-            .join(ClangParams::AUTO_FIX_PATCH);
-        if let Ok(patch_file) = std::fs::File::open(&auto_fix_patch_path) {
-            self.client.write_output_variables(&[OutputVariable {
-                name: "fix-patch-path".to_string(),
-                value: auto_fix_patch_path.to_string_lossy().replace("\\", "/"),
-            }])?;
 
-            log::info!("Generated patch content:");
-            let mut reader = std::io::BufReader::new(patch_file);
-            let mut line_buf = String::new();
-            while let Ok(bytes_read) = reader.read_line(&mut line_buf)
-                && bytes_read > 0
-            {
-                if line_buf.starts_with('+') {
-                    print!("{}", line_buf.green()); // Green for additions
-                } else if line_buf.starts_with('-') {
-                    print!("{}", line_buf.red()); // Red for deletions
-                } else {
-                    print!("{line_buf}"); // Default style for context lines
-                }
-                line_buf.clear();
+        if feedback_inputs.thread_comments != ThreadComments::Off {
+            // post thread comment for PR or push event
+            if comment.as_ref().is_none_or(|c| c.len() > u16::MAX as usize) {
+                comment = Some(Self::make_comment(
+                    files,
+                    format_checks_failed,
+                    tidy_checks_failed,
+                    &clang_versions,
+                    Some(u16::MAX as u64),
+                )?);
             }
+            let options = ThreadCommentOptions {
+                policy: if feedback_inputs.thread_comments == ThreadComments::Update {
+                    CommentPolicy::Update
+                } else {
+                    // feedback_inputs.thread_comments is not Off and not Update, so it must be just On.
+                    CommentPolicy::Anew
+                },
+                comment: comment.unwrap_or_default(),
+                kind: if format_checks_failed == 0 && tidy_checks_failed == 0 {
+                    CommentKind::Lgtm
+                } else {
+                    CommentKind::Concerns
+                },
+                marker: COMMENT_MARKER.to_string(),
+                no_lgtm: feedback_inputs.no_lgtm,
+            };
+            self.client.post_thread_comment(options).await?;
         }
         Ok(format_checks_failed + tidy_checks_failed)
     }
@@ -427,6 +404,50 @@ impl RestClient {
         comment.push_str(USER_OUTREACH);
         Ok(comment)
     }
+
+    /// Show the generated patch content in the console output.
+    ///
+    /// And also write the patch path to the output variable `fix-patch-path` for CI platforms.
+    fn show_patch(&self, feedback_inputs: &FeedbackInput) -> Result<(), ClientError> {
+        let auto_fix_patch_path = feedback_inputs
+            .repo_root
+            .join(ClangParams::CACHE_DIR)
+            .join(ClangParams::AUTO_FIX_PATCH);
+        if let Ok(patch_file) = std::fs::File::open(&auto_fix_patch_path) {
+            self.client.write_output_variables(&[OutputVariable {
+                name: "fix-patch-path".to_string(),
+                value: auto_fix_patch_path.to_string_lossy().replace("\\", "/"),
+            }])?;
+
+            log::info!("Generated patch content:");
+            let stdout_handle = stdout();
+            let mut reader = std::io::BufReader::new(patch_file);
+            let mut line_buf = String::new();
+            let mut stdout_lock = stdout_handle.lock();
+            while let Ok(bytes_read) = reader.read_line(&mut line_buf)
+                && bytes_read > 0
+            {
+                if line_buf.starts_with('+') {
+                    // Green for additions
+                    stdout_lock
+                        .write_all(line_buf.green().to_string().as_bytes())
+                        .map_err(ClientError::StdoutWriteFailed)?;
+                } else if line_buf.starts_with('-') {
+                    // Red for deletions
+                    stdout_lock
+                        .write_all(line_buf.red().to_string().as_bytes())
+                        .map_err(ClientError::StdoutWriteFailed)?;
+                } else {
+                    // Default style for context lines
+                    stdout_lock
+                        .write_all(line_buf.as_bytes())
+                        .map_err(ClientError::StdoutWriteFailed)?;
+                }
+                line_buf.clear();
+            }
+        }
+        Ok(())
+    }
 }
 
 /// A closing tag for details blocks in markdown comments.