Suppress ansi with pipes #3775
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,3 +59,5 @@ do_not_version/ | |
| /ui/desktop/src/bin/goose-scheduler-executor | ||
| /ui/desktop/src/bin/goose | ||
| /.env | ||
|
|
||
| /working_dir | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| use anstream::println; | ||
| use bat::WrappingMode; | ||
| use console::{style, Color}; | ||
| use goose::config::Config; | ||
|
|
@@ -11,11 +12,10 @@ use rmcp::model::PromptArgument; | |
| use serde_json::Value; | ||
| use std::cell::RefCell; | ||
| use std::collections::HashMap; | ||
| use std::io::{Error, IsTerminal, Write}; | ||
| use std::path::{Path, PathBuf}; | ||
| use std::sync::Arc; | ||
| use std::time::Duration; | ||
| // Re-export theme for use in main | ||
| #[derive(Clone, Copy)] | ||
| pub enum Theme { | ||
|
|
@@ -131,20 +131,26 @@ thread_local! { | |
| } | ||
|
|
||
| pub fn show_thinking() { | ||
| if std::io::stdout().is_terminal() { | ||
| THINKING.with(|t| t.borrow_mut().show()); | ||
| } | ||
| } | ||
|
|
||
| pub fn hide_thinking() { | ||
| if std::io::stdout().is_terminal() { | ||
| THINKING.with(|t| t.borrow_mut().hide()); | ||
| } | ||
| } | ||
|
|
||
| #[allow(dead_code)] | ||
| pub fn set_thinking_message(s: &String) { | ||
| if std::io::stdout().is_terminal() { | ||
| THINKING.with(|t| { | ||
| if let Some(spinner) = t.borrow_mut().spinner.as_mut() { | ||
| spinner.set_message(s); | ||
| } | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| pub fn render_message(message: &Message, debug: bool) { | ||
|
|
@@ -159,7 +165,9 @@ pub fn render_message(message: &Message, debug: bool) { | |
| println!("Image: [data: {}, type: {}]", image.data, image.mime_type); | ||
| } | ||
| MessageContent::Thinking(thinking) => { | ||
| if std::env::var("GOOSE_CLI_SHOW_THINKING").is_ok() | ||
| && std::io::stdout().is_terminal() | ||
| { | ||
| println!("\n{}", style("Thinking:").dim().italic()); | ||
| print_markdown(&thinking.thinking, theme); | ||
| } | ||
|
|
@@ -183,6 +191,10 @@ pub fn render_text(text: &str, color: Option<Color>, dim: bool) { | |
| } | ||
|
|
||
| pub fn render_text_no_newlines(text: &str, color: Option<Color>, dim: bool) { | ||
| if !std::io::stdout().is_terminal() { | ||
| println!("{}", text); | ||
| return; | ||
| } | ||
| let mut styled_text = style(text); | ||
| if dim { | ||
| styled_text = styled_text.dim(); | ||
|
|
@@ -287,17 +299,18 @@ pub fn render_prompts(prompts: &HashMap<String, Vec<String>>) { | |
|
|
||
| pub fn render_prompt_info(info: &PromptInfo) { | ||
| println!(); | ||
| if let Some(ext) = &info.extension { | ||
| println!(" {}: {}", style("Extension").green(), ext); | ||
| } | ||
| println!(" Prompt: {}", style(&info.name).cyan().bold()); | ||
| if let Some(desc) = &info.description { | ||
| println!("\n {}", desc); | ||
| } | ||
| render_arguments(info); | ||
| println!(); | ||
| } | ||
|
|
||
| fn render_arguments(info: &PromptInfo) { | ||
| if let Some(args) = &info.arguments { | ||
| println!("\n Arguments:"); | ||
| for arg in args { | ||
|
|
@@ -316,7 +329,6 @@ pub fn render_prompt_info(info: &PromptInfo) { | |
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| pub fn render_extension_success(name: &str) { | ||
|
|
@@ -465,14 +477,18 @@ pub fn env_no_color() -> bool { | |
| } | ||
|
|
||
| fn print_markdown(content: &str, theme: Theme) { | ||
| if std::io::stdout().is_terminal() { | ||
| bat::PrettyPrinter::new() | ||
|
Comment on lines
+480
to
+481
Contributor
There was a problem hiding this comment. I am less familiar with
Collaborator
Author
There was a problem hiding this comment. looks cool. here we are just skipping the formatting of markdown though |
||
| .input(bat::Input::from_bytes(content.as_bytes())) | ||
| .theme(theme.as_str()) | ||
| .colored_output(env_no_color()) | ||
| .language("Markdown") | ||
| .wrapping_mode(WrappingMode::NoWrapping(true)) | ||
| .print() | ||
| .unwrap(); | ||
| } else { | ||
| print!("{}", content); | ||
| } | ||
| } | ||
|
|
||
| const INDENT: &str = " "; | ||
|
|
@@ -484,6 +500,23 @@ fn get_tool_params_max_length() -> usize { | |
| .unwrap_or(40) | ||
| } | ||
|
|
||
| fn print_value(value: &Value, debug: bool) { | ||
| let formatted = match value { | ||
| Value::String(s) => { | ||
| if !debug && s.len() > get_tool_params_max_length() { | ||
| style(format!("[REDACTED: {} chars]", s.len())).yellow() | ||
| } else { | ||
| style(s.to_string()).green() | ||
| } | ||
| } | ||
| Value::Number(n) => style(n.to_string()).yellow(), | ||
| Value::Bool(b) => style(b.to_string()).yellow(), | ||
| Value::Null => style("null".to_string()).dim(), | ||
| _ => unreachable!(), | ||
| }; | ||
| println!("{}", formatted); | ||
| } | ||
|
|
||
| fn print_params(value: &Value, depth: usize, debug: bool) { | ||
| let indent = INDENT.repeat(depth); | ||
|
|
||
|
|
@@ -502,21 +535,9 @@ fn print_params(value: &Value, depth: usize, debug: bool) { | |
| print_params(item, depth + 2, debug); | ||
| } | ||
| } | ||
| _ => { | ||
| print!("{}{}: ", indent, style(key).dim()); | ||
| print_value(val, debug); | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -527,26 +548,7 @@ fn print_params(value: &Value, depth: usize, debug: bool) { | |
| print_params(item, depth + 1, debug); | ||
| } | ||
| } | ||
| _ => print_value(value, debug), | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -775,7 +777,7 @@ pub async fn display_cost_usage( | |
| ) { | ||
| if let Some(cost) = estimate_cost_usd(provider, model, input_tokens, output_tokens).await { | ||
| use console::style; | ||
| eprintln!( | ||
| "Cost: {} USD ({} tokens: in {}, out {})", | ||
| style(format!("${:.4}", cost)).cyan(), | ||
| input_tokens + output_tokens, | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we also need to import the
eprintlnwrapper here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's a good question - this PR is fixing automations based on recipe. stderr is not useful at this point for much when it comes to recipes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the surrounding context but my understanding of the bug we're aiming to fix is "don't write ANSI to non-ttys" which is definitely a good idea globally.
This PR is fixing that for
println!but is also changing some things to useeprintln!(why?) but crucially also is not using the anstyle wrapper so we'll still output ANSI styles to stderr which seems wrong to me.IOW we should consistently
use anstream::{println, eprintln};at least right?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mostly agree, just not sure I want this to be held up because of that. eprint vs print in the code is kind of a mess right now