From e3df327f31d13019eb7d124087e023470a4423b5 Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 05:17:18 +0000 Subject: [PATCH] fix(cli): keep idx's own colors; sanitize remaining provider text; no UTF-8 panic From the Codex re-gate of the 0.3.3 fixes: - Colored signal cells (Bullish/Bearish, ratings, ownership change and flow labels) were painted before the new cell sanitizer, which turned their escape codes into U+FFFD on every color terminal; the goldens run without color and missed it. Labels idx produces itself go into cells untouched (`painted`), with the label made safe before styling. - Statement dates and currency, and the entity name in `ownership entity`, still reached the terminal unsanitized. - The JSON-mode argument handler read std::env::args(), which panics on a non-UTF-8 argument (exit 101 instead of 2); use args_os. - A missing subcommand printed help as plain text in JSON mode; it is an error (exit 2), so it is a JSON error there too. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- crates/idx-cli/src/cli/ownership.rs | 10 +++--- crates/idx-cli/src/main.rs | 13 +++++--- crates/idx-cli/src/output/table.rs | 51 ++++++++++++++++------------- crates/idx-cli/src/output/text.rs | 15 ++++++++- 4 files changed, 56 insertions(+), 33 deletions(-) diff --git a/crates/idx-cli/src/cli/ownership.rs b/crates/idx-cli/src/cli/ownership.rs index 0d19885..6abe2a6 100644 --- a/crates/idx-cli/src/cli/ownership.rs +++ b/crates/idx-cli/src/cli/ownership.rs @@ -8,7 +8,7 @@ use crate::config::IdxConfig; use crate::output::json; use crate::output::style::Paint; use crate::output::table::format_idr; -use crate::output::text::{cell, terminal_safe}; +use crate::output::text::{cell, painted, terminal_safe}; use idx_core::error::IdxError; use idx_core::investor::{ ConcentrationRank, HolderSource, ImportSource, Investor, parse_report_family, @@ -349,7 +349,7 @@ fn handle_ticker( }), cell(format_idr(h.shares)), cell(format_bps(h.percentage_bps)), - cell(format_signal(h.signal)), + painted(format_signal(h.signal)), ]); } println!("{table}"); @@ -381,7 +381,7 @@ fn handle_entity( println!( "{} {}", "Entity:".bold(), - data.entity.canonical_name.as_str().bold() + terminal_safe(&data.entity.canonical_name).bold() ); let mut table = Table::new(); table @@ -561,7 +561,7 @@ fn print_flow_section(title: &str, rows: &[HolderRow]) { cell(&row.name), cell(format_idr(row.shares)), cell(format_bps(row.percentage_bps)), - cell(format_signal(row.signal)), + painted(format_signal(row.signal)), ]); } println!("{table}"); @@ -630,7 +630,7 @@ fn handle_changes( table.add_row(vec![ cell(row.ticker_code), cell(row.entity_name), - cell(format_change_type(row.change_type)), + painted(format_change_type(row.change_type)), cell( row.old_bps .map(format_bps) diff --git a/crates/idx-cli/src/main.rs b/crates/idx-cli/src/main.rs index c984112..f94fb62 100644 --- a/crates/idx-cli/src/main.rs +++ b/crates/idx-cli/src/main.rs @@ -91,11 +91,11 @@ fn run() -> Result<(), IdxError> { /// `--version` print as usual. fn argument_error(err: clap::Error) -> IdxError { use clap::error::ErrorKind; + // A missing subcommand prints help but is an error (exit 2), so in JSON + // mode it is a JSON error too. if matches!( err.kind(), - ErrorKind::DisplayHelp - | ErrorKind::DisplayVersion - | ErrorKind::DisplayHelpOnMissingArgumentOrSubcommand + ErrorKind::DisplayHelp | ErrorKind::DisplayVersion ) || !requested_output_from_args().is_json() { err.exit(); @@ -114,7 +114,12 @@ fn argument_error(err: clap::Error) -> IdxError { /// The output format the raw arguments ask for, when they did not parse: /// `-o`/`--output` (any spelling), then `IDX_OUTPUT`, then auto. fn requested_output_from_args() -> OutputFormat { - let args: Vec = std::env::args().skip(1).collect(); + // args_os: an argument that is not UTF-8 (itself the error being + // reported) must not panic here. + let args: Vec = std::env::args_os() + .skip(1) + .map(|arg| arg.to_string_lossy().into_owned()) + .collect(); let flag = args.iter().enumerate().find_map(|(i, arg)| { if arg == "-o" || arg == "--output" { args.get(i + 1).cloned() diff --git a/crates/idx-cli/src/output/table.rs b/crates/idx-cli/src/output/table.rs index 961a7a9..8dd5be6 100644 --- a/crates/idx-cli/src/output/table.rs +++ b/crates/idx-cli/src/output/table.rs @@ -1,5 +1,5 @@ use crate::output::style::Paint; -use crate::output::text::{cell, terminal_safe, truncate_chars}; +use crate::output::text::{cell, painted, terminal_safe, truncate_chars}; use comfy_table::{Cell, Color, ContentArrangement, Table, presets::UTF8_FULL}; use idx_core::analysis::fundamental::{ @@ -166,7 +166,7 @@ pub fn print_technical(report: &TechnicalReport, no_color: bool) -> Result<(), I table.add_row(vec![ cell("RSI (14)"), cell(format_float(report.rsi14, 2)), - cell(format_signal(report.signals.rsi, no_color, false)), + painted(format_signal(report.signals.rsi, no_color, false)), ]); table.add_row(vec![ cell("MACD (12,26,9)"), @@ -176,12 +176,12 @@ pub fn print_technical(report: &TechnicalReport, no_color: bool) -> Result<(), I format_float(report.macd.signal, 2), format_float(report.macd.histogram, 2) )), - cell(format_signal(report.signals.macd, no_color, false)), + painted(format_signal(report.signals.macd, no_color, false)), ]); table.add_row(vec![ cell("Trend"), cell(trend_context(report)), - cell(format_signal(report.signals.trend, no_color, false)), + painted(format_signal(report.signals.trend, no_color, false)), ]); table.add_row(vec![ cell("Volume Ratio (20)"), @@ -191,7 +191,7 @@ pub fn print_technical(report: &TechnicalReport, no_color: bool) -> Result<(), I table.add_row(vec![ cell("Overall Signal"), cell("-"), - cell(format_signal(report.signals.overall, no_color, true)), + painted(format_signal(report.signals.overall, no_color, true)), ]); println!("{table}"); @@ -210,17 +210,17 @@ pub fn print_growth(symbol: &str, report: &GrowthReport, no_color: bool) -> Resu table.add_row(vec![ cell("Revenue Growth"), cell(format_pct(report.revenue_growth_pct)), - cell(format_growth_signal(&report.revenue_signal, no_color)), + painted(format_growth_signal(&report.revenue_signal, no_color)), ]); table.add_row(vec![ cell("Earnings Growth"), cell(format_pct(report.earnings_growth_pct)), - cell(format_growth_signal(&report.earnings_signal, no_color)), + painted(format_growth_signal(&report.earnings_signal, no_color)), ]); table.add_row(vec![ cell("Overall"), cell("-"), - cell(format_growth_signal(&report.overall_signal, no_color)), + painted(format_growth_signal(&report.overall_signal, no_color)), ]); println!("{table}"); @@ -243,7 +243,7 @@ pub fn print_valuation( table.add_row(vec![ cell("P/E (Trailing)"), cell(format_opt_f64(report.pe_trailing, 2)), - cell(format_valuation_signal(&report.pe_signal, no_color)), + painted(format_valuation_signal(&report.pe_signal, no_color)), ]); table.add_row(vec![ cell("P/E (Forward)"), @@ -253,27 +253,27 @@ pub fn print_valuation( table.add_row(vec![ cell("Price/Book"), cell(format_opt_f64(report.pb, 2)), - cell(format_valuation_signal(&report.pb_signal, no_color)), + painted(format_valuation_signal(&report.pb_signal, no_color)), ]); table.add_row(vec![ cell("ROE"), cell(format_pct(report.roe_pct)), - cell(format_valuation_signal(&report.roe_signal, no_color)), + painted(format_valuation_signal(&report.roe_signal, no_color)), ]); table.add_row(vec![ cell("Net Margin"), cell(format_pct(report.net_margin_pct)), - cell(format_valuation_signal(&report.margin_signal, no_color)), + painted(format_valuation_signal(&report.margin_signal, no_color)), ]); table.add_row(vec![ cell("EV/EBITDA"), cell(format_opt_f64(report.ev_ebitda, 2)), - cell(format_valuation_signal(&report.ev_ebitda_signal, no_color)), + painted(format_valuation_signal(&report.ev_ebitda_signal, no_color)), ]); table.add_row(vec![ cell("Overall"), cell("-"), - cell(format_valuation_signal(&report.overall_signal, no_color)), + painted(format_valuation_signal(&report.overall_signal, no_color)), ]); println!("{table}"); @@ -292,12 +292,12 @@ pub fn print_risk(symbol: &str, report: &RiskReport, no_color: bool) -> Result<( table.add_row(vec![ cell("Debt/Equity"), cell(format_opt_f64(report.debt_to_equity, 2)), - cell(format_risk_signal(&report.de_signal, no_color)), + painted(format_risk_signal(&report.de_signal, no_color)), ]); table.add_row(vec![ cell("Current Ratio"), cell(format_opt_f64(report.current_ratio, 2)), - cell(format_risk_signal(&report.current_ratio_signal, no_color)), + painted(format_risk_signal(&report.current_ratio_signal, no_color)), ]); table.add_row(vec![ cell("ROA"), @@ -307,7 +307,7 @@ pub fn print_risk(symbol: &str, report: &RiskReport, no_color: bool) -> Result<( table.add_row(vec![ cell("Overall"), cell("-"), - cell(format_risk_signal(&report.overall_signal, no_color)), + painted(format_risk_signal(&report.overall_signal, no_color)), ]); println!("{table}"); @@ -632,16 +632,18 @@ fn format_text_signal( positive: &[&str], negative: &[&str], ) -> String { + // The label comes from the analysis report; make it safe before styling. + let label = terminal_safe(signal); if no_color { - return signal.to_string(); + return label; } if positive.contains(&signal) { - signal.green().to_string() + label.green().to_string() } else if negative.contains(&signal) { - signal.red().to_string() + label.red().to_string() } else { - signal.yellow().to_string() + label.yellow().to_string() } } @@ -706,9 +708,12 @@ pub fn print_profile(profile: &CompanyProfile) -> Result<(), IdxError> { pub fn print_financials(fin: &FinancialStatements) -> Result<(), IdxError> { let print_section = |label: &str, section: &idx_core::api::types::StatementSection| { - println!("\n── {label} ({}) ──", format_table_date(§ion.end_date)); + println!( + "\n── {label} ({}) ──", + terminal_safe(&format_table_date(§ion.end_date)) + ); let mut t = Table::new(); - let value_header = format!("VALUE ({})", section.currency); + let value_header = format!("VALUE ({})", terminal_safe(§ion.currency)); t.load_preset(UTF8_FULL) .set_content_arrangement(ContentArrangement::Dynamic) .set_header(vec!["LINE ITEM", value_header.as_str()]); diff --git a/crates/idx-cli/src/output/text.rs b/crates/idx-cli/src/output/text.rs index c3c92d1..d9c64cc 100644 --- a/crates/idx-cli/src/output/text.rs +++ b/crates/idx-cli/src/output/text.rs @@ -25,6 +25,12 @@ pub fn cell(value: impl ToString) -> Cell { Cell::new(terminal_safe(&value.to_string())) } +/// A cell for text idx produced itself (labels, numbers) that may carry its +/// own color codes, which [`cell`] would strip. Never for provider text. +pub fn painted(styled: String) -> Cell { + Cell::new(styled) +} + /// At most `max` characters of `text` (never splitting one), with `...` /// appended when it was cut. pub fn truncate_chars(text: &str, max: usize) -> String { @@ -36,7 +42,7 @@ pub fn truncate_chars(text: &str, max: usize) -> String { #[cfg(test)] mod tests { - use super::{terminal_safe, truncate_chars}; + use super::{painted, terminal_safe, truncate_chars}; #[test] fn escape_sequences_and_bidi_overrides_are_neutralized() { @@ -57,6 +63,13 @@ mod tests { ); } + #[test] + fn painted_cells_keep_idx_color_codes() { + // comfy-table appends a reset; the color codes themselves survive. + let green = "\u{1b}[32mBullish\u{1b}[39m".to_string(); + assert!(painted(green.clone()).content().starts_with(&green)); + } + #[test] fn truncation_never_splits_a_character() { let text = format!("{}é tail", "a".repeat(199));