diff --git a/crates/idx-cli/src/cli/config.rs b/crates/idx-cli/src/cli/config.rs index cd77da2..e4d7dac 100644 --- a/crates/idx-cli/src/cli/config.rs +++ b/crates/idx-cli/src/cli/config.rs @@ -1,6 +1,7 @@ use clap::{Args, Subcommand}; use crate::config::{config_path, ensure_default_config, get_config_value, set_config_value}; +use crate::output::{OutputFormat, json}; use idx_core::error::IdxError; #[derive(Debug, Args)] @@ -22,24 +23,39 @@ pub enum ConfigSubcommand { Path, } -pub fn handle(cmd: &ConfigCmd) -> Result<(), IdxError> { +pub fn handle(cmd: &ConfigCmd, format: &OutputFormat) -> Result<(), IdxError> { + // Each result as plain text, or one JSON document with -o json. + let print = |text: String, document: serde_json::Value| { + if format.is_json() { + json::print_json(&document) + } else { + println!("{text}"); + Ok(()) + } + }; match &cmd.command { ConfigSubcommand::Init => { - let path = ensure_default_config()?; - println!("{}", path.display()); + let path = ensure_default_config()?.display().to_string(); + print(path.clone(), serde_json::json!({ "path": path })) } ConfigSubcommand::Get { key } => { let value = get_config_value(key)? .ok_or_else(|| IdxError::ConfigError(format!("key not found: {key}")))?; - println!("{value}"); + print( + value.to_string(), + serde_json::json!({ "key": key, "value": value.to_string() }), + ) } ConfigSubcommand::Set { key, value } => { set_config_value(key, value)?; - println!("ok"); + print( + "ok".to_string(), + serde_json::json!({ "key": key, "value": value, "ok": true }), + ) } ConfigSubcommand::Path => { - println!("{}", config_path()?.display()); + let path = config_path()?.display().to_string(); + print(path.clone(), serde_json::json!({ "path": path })) } } - Ok(()) } diff --git a/crates/idx-cli/src/cli/ownership.rs b/crates/idx-cli/src/cli/ownership.rs index d9b4261..0d19885 100644 --- a/crates/idx-cli/src/cli/ownership.rs +++ b/crates/idx-cli/src/cli/ownership.rs @@ -1,13 +1,14 @@ use std::path::PathBuf; use clap::{Args, Subcommand}; -use comfy_table::{Cell, ContentArrangement, Table, presets::UTF8_FULL}; +use comfy_table::{ContentArrangement, Table, presets::UTF8_FULL}; use serde::Serialize; 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 idx_core::error::IdxError; use idx_core::investor::{ ConcentrationRank, HolderSource, ImportSource, Investor, parse_report_family, @@ -262,7 +263,7 @@ fn handle_discover( config: &IdxConfig, ) -> Result<(), IdxError> { if args.limit == 0 { - return Err(IdxError::ParseError( + return Err(IdxError::InvalidInput( "--limit must be greater than 0".to_string(), )); } @@ -284,17 +285,17 @@ fn handle_discover( for report in reports { table.add_row(vec![ - Cell::new(report.publish_date.split('T').next().unwrap_or("-")), - Cell::new(report.family.label()), - Cell::new(report.status.label()), - Cell::new(if report.is_attachment { + cell(report.publish_date.split('T').next().unwrap_or("-")), + cell(report.family.label()), + cell(report.status.label()), + cell(if report.is_attachment { "attachment" } else { "main" }), - Cell::new(report.original_filename.unwrap_or_else(|| "-".to_string())), - Cell::new(report.title), - Cell::new(report.pdf_url), + cell(report.original_filename.unwrap_or_else(|| "-".to_string())), + cell(report.title), + cell(report.pdf_url), ]); } @@ -334,21 +335,21 @@ fn handle_ticker( for h in &data.holders { table.add_row(vec![ - Cell::new(h.rank), - Cell::new(match h.source { + cell(h.rank), + cell(match h.source { OwnershipSource::Ksei => "KSEI", OwnershipSource::Bing => "BING", }), - Cell::new(&h.name), - Cell::new(h.investor_type.clone().unwrap_or_else(|| "-".to_string())), - Cell::new(match h.locality { + cell(&h.name), + cell(h.investor_type.clone().unwrap_or_else(|| "-".to_string())), + cell(match h.locality { Some(idx_core::ownership::types::Locality::Local) => "L", Some(idx_core::ownership::types::Locality::Foreign) => "F", None => "-", }), - Cell::new(format_idr(h.shares)), - Cell::new(format_bps(h.percentage_bps)), - Cell::new(format_signal(h.signal)), + cell(format_idr(h.shares)), + cell(format_bps(h.percentage_bps)), + cell(format_signal(h.signal)), ]); } println!("{table}"); @@ -390,14 +391,14 @@ fn handle_entity( for row in &data.holdings { table.add_row(vec![ - Cell::new(&row.ticker.code), - Cell::new(match row.source { + cell(&row.ticker.code), + cell(match row.source { OwnershipSource::Ksei => "KSEI", OwnershipSource::Bing => "BING", }), - Cell::new(format_idr(row.shares)), - Cell::new(format_bps(row.percentage_bps)), - Cell::new(row.report_date.format("%Y-%m-%d").to_string()), + cell(format_idr(row.shares)), + cell(format_bps(row.percentage_bps)), + cell(row.report_date.format("%Y-%m-%d").to_string()), ]); } println!("{table}"); @@ -430,14 +431,14 @@ fn handle_search( for entity in &entities { table.add_row(vec![ - Cell::new(&entity.canonical_name), - Cell::new( + cell(&entity.canonical_name), + cell( entity .entity_type .clone() .unwrap_or_else(|| "-".to_string()), ), - Cell::new(entity.country.clone().unwrap_or_else(|| "-".to_string())), + cell(entity.country.clone().unwrap_or_else(|| "-".to_string())), ]); } println!("{table}"); @@ -463,10 +464,10 @@ fn handle_cross_holders( for (i, row) in rows.iter().enumerate() { table.add_row(vec![ - Cell::new(i + 1), - Cell::new(&row.entity.canonical_name), - Cell::new(row.ticker_count), - Cell::new(format_bps(row.total_bps)), + cell(i + 1), + cell(&row.entity.canonical_name), + cell(row.ticker_count), + cell(format_bps(row.total_bps)), ]); } @@ -502,13 +503,13 @@ fn handle_concentration( for (idx, (ticker, m)) in rows.iter().enumerate() { table.add_row(vec![ - Cell::new(idx + 1), - Cell::new(ticker), - Cell::new(format_bps(m.top1_bps)), - Cell::new(format_bps(m.top3_bps)), - Cell::new(m.hhi), - Cell::new(format_bps(m.free_float_bps)), - Cell::new(m.holder_count), + cell(idx + 1), + cell(ticker), + cell(format_bps(m.top1_bps)), + cell(format_bps(m.top3_bps)), + cell(m.hhi), + cell(format_bps(m.free_float_bps)), + cell(m.holder_count), ]); } @@ -556,11 +557,11 @@ fn print_flow_section(title: &str, rows: &[HolderRow]) { for (i, row) in rows.iter().enumerate() { table.add_row(vec![ - Cell::new(i + 1), - Cell::new(&row.name), - Cell::new(format_idr(row.shares)), - Cell::new(format_bps(row.percentage_bps)), - Cell::new(format_signal(row.signal)), + cell(i + 1), + cell(&row.name), + cell(format_idr(row.shares)), + cell(format_bps(row.percentage_bps)), + cell(format_signal(row.signal)), ]); } println!("{table}"); @@ -583,14 +584,17 @@ fn handle_graph(investor: &Investor, args: &GraphArgs, config: &IdxConfig) -> Re match args.format.trim().to_lowercase().as_str() { "table" => { - print!("{}", graph::format_graph_text(&nodes, &edges)); + print!( + "{}", + terminal_safe(&graph::format_graph_text(&nodes, &edges)) + ); Ok(()) } "dot" => { print!("{}", graph::format_graph_dot(&nodes, &edges)); Ok(()) } - other => Err(IdxError::ParseError(format!( + other => Err(IdxError::InvalidInput(format!( "invalid --format '{other}', expected table|dot" ))), } @@ -624,20 +628,20 @@ fn handle_changes( for row in changes { table.add_row(vec![ - Cell::new(row.ticker_code), - Cell::new(row.entity_name), - Cell::new(format_change_type(row.change_type)), - Cell::new( + cell(row.ticker_code), + cell(row.entity_name), + cell(format_change_type(row.change_type)), + cell( row.old_bps .map(format_bps) .unwrap_or_else(|| "-".to_string()), ), - Cell::new( + cell( row.new_bps .map(format_bps) .unwrap_or_else(|| "-".to_string()), ), - Cell::new(format_signed_bps(row.delta_bps)), + cell(format_signed_bps(row.delta_bps)), ]); } @@ -677,11 +681,11 @@ fn handle_resolve( for row in rows { table.add_row(vec![ - Cell::new(row.raw_name), - Cell::new(row.source), - Cell::new(row.ticker_code), - Cell::new(row.current_entity.unwrap_or_else(|| "-".to_string())), - Cell::new( + cell(row.raw_name), + cell(row.source), + cell(row.ticker_code), + cell(row.current_entity.unwrap_or_else(|| "-".to_string())), + cell( row.confidence .map(|v| format!("{v:.2}")) .unwrap_or_else(|| "-".to_string()), @@ -734,13 +738,13 @@ fn handle_import( config: &IdxConfig, ) -> Result<(), IdxError> { if args.file.is_none() && args.url.is_none() && args.fetch_bing.is_none() { - return Err(IdxError::ParseError( + return Err(IdxError::InvalidInput( "provide one of: --file, --url, or --fetch-bing".to_string(), )); } if args.file.is_some() && args.url.is_some() { - return Err(IdxError::ParseError( + return Err(IdxError::InvalidInput( "--file and --url are mutually exclusive".to_string(), )); } @@ -802,11 +806,11 @@ fn handle_releases(investor: &Investor, config: &IdxConfig) -> Result<(), IdxErr for r in releases { table.add_row(vec![ - Cell::new(r.as_of_date.format("%Y-%m-%d").to_string()), - Cell::new(r.row_count), - Cell::new(r.sha256), - Cell::new(r.source_url.unwrap_or_else(|| "-".to_string())), - Cell::new(r.imported_at), + cell(r.as_of_date.format("%Y-%m-%d").to_string()), + cell(r.row_count), + cell(r.sha256), + cell(r.source_url.unwrap_or_else(|| "-".to_string())), + cell(r.imported_at), ]); } diff --git a/crates/idx-cli/src/main.rs b/crates/idx-cli/src/main.rs index 91f75a1..c984112 100644 --- a/crates/idx-cli/src/main.rs +++ b/crates/idx-cli/src/main.rs @@ -20,7 +20,10 @@ fn main() { } fn run() -> Result<(), IdxError> { - let cli = Cli::parse(); + let cli = match Cli::try_parse() { + Ok(cli) => cli, + Err(err) => return Err(argument_error(err)), + }; runtime::set_quiet(cli.quiet); let config = match IdxConfig::load_with_cli(&cli) { Ok(config) => config, @@ -30,6 +33,7 @@ fn run() -> Result<(), IdxError> { } }; output::json::configure(config.output, config::stdout_is_terminal()); + runtime::set_json(config.output.is_json()); output::style::set_color(!config.no_color); match &cli.command { @@ -58,7 +62,7 @@ fn run() -> Result<(), IdxError> { } } Commands::Config(cfg) => { - if let Err(err) = cli::config::handle(cfg) { + if let Err(err) = cli::config::handle(cfg, &config.output) { emit_error(&err, &config.output); return Err(err); } @@ -82,6 +86,57 @@ fn run() -> Result<(), IdxError> { } /// Output format for errors raised before the configuration has loaded. +/// Argument errors honor the output contract: in JSON mode they are one JSON +/// error on stderr like any other failure (clap prints plain text). Help and +/// `--version` print as usual. +fn argument_error(err: clap::Error) -> IdxError { + use clap::error::ErrorKind; + if matches!( + err.kind(), + ErrorKind::DisplayHelp + | ErrorKind::DisplayVersion + | ErrorKind::DisplayHelpOnMissingArgumentOrSubcommand + ) || !requested_output_from_args().is_json() + { + err.exit(); + } + let rendered = err.render().to_string(); + let message = rendered + .trim() + .strip_prefix("error: ") + .unwrap_or(rendered.trim()) + .to_string(); + let error = IdxError::InvalidInput(message); + emit_error(&error, &requested_output_from_args()); + error +} + +/// 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(); + let flag = args.iter().enumerate().find_map(|(i, arg)| { + if arg == "-o" || arg == "--output" { + args.get(i + 1).cloned() + } else if let Some(value) = arg.strip_prefix("--output=") { + Some(value.to_string()) + } else if arg.starts_with("-o") && arg.len() > 2 { + Some(arg[2..].trim_start_matches('=').to_string()) + } else { + None + } + }); + let setting = flag + .and_then(|value| OutputSetting::parse(&value)) + .or_else(|| { + std::env::var("IDX_OUTPUT") + .ok() + .and_then(|value| OutputSetting::parse(&value)) + }) + .unwrap_or_default(); + setting.resolve(config::stdout_is_terminal()) +} + fn bootstrap_output_format(cli: &Cli) -> OutputFormat { let setting = cli .output diff --git a/crates/idx-cli/src/output/mod.rs b/crates/idx-cli/src/output/mod.rs index 2f1b8e4..055d08f 100644 --- a/crates/idx-cli/src/output/mod.rs +++ b/crates/idx-cli/src/output/mod.rs @@ -1,6 +1,7 @@ pub mod json; pub mod style; pub mod table; +pub mod text; use std::io::IsTerminal; diff --git a/crates/idx-cli/src/output/table.rs b/crates/idx-cli/src/output/table.rs index fb5df0c..961a7a9 100644 --- a/crates/idx-cli/src/output/table.rs +++ b/crates/idx-cli/src/output/table.rs @@ -1,4 +1,5 @@ use crate::output::style::Paint; +use crate::output::text::{cell, terminal_safe, truncate_chars}; use comfy_table::{Cell, Color, ContentArrangement, Table, presets::UTF8_FULL}; use idx_core::analysis::fundamental::{ @@ -56,27 +57,27 @@ pub fn print_quotes(quotes: &[Quote], no_color: bool) -> Result<(), IdxError> { for q in quotes { let pct_cell = match q.change_pct { - None => Cell::new("-"), - Some(pct) if no_color => Cell::new(format!("{pct:+.2}%")), - Some(pct) if pct >= 0.0 => Cell::new(format!("{pct:+.2}%")).fg(Color::Green), - Some(pct) => Cell::new(format!("{pct:+.2}%")).fg(Color::Red), + None => cell("-"), + Some(pct) if no_color => cell(format!("{pct:+.2}%")), + Some(pct) if pct >= 0.0 => cell(format!("{pct:+.2}%")).fg(Color::Green), + Some(pct) => cell(format!("{pct:+.2}%")).fg(Color::Red), }; table.add_row(vec![ - Cell::new(&q.symbol), - Cell::new(format_idr(q.price)), - Cell::new( + cell(&q.symbol), + cell(format_idr(q.price)), + cell( q.change .map(|c| format!("{c:+}")) .unwrap_or_else(|| "-".to_string()), ), pct_cell, - Cell::new(q.volume.map(format_u64).unwrap_or_else(|| "-".to_string())), - Cell::new( + cell(q.volume.map(format_u64).unwrap_or_else(|| "-".to_string())), + cell( q.market_cap .map(format_u64) .unwrap_or_else(|| "-".to_string()), ), - Cell::new(format_52w_range_bar(q.week52_position)), + cell(format_52w_range_bar(q.week52_position)), ]); } @@ -114,12 +115,12 @@ pub fn print_history(symbol: &str, history: &[Ohlc]) -> Result<(), IdxError> { .set_header(vec!["DATE", "OPEN", "HIGH", "LOW", "CLOSE", "VOLUME"]); for item in history { table.add_row(vec![ - Cell::new(item.date), - Cell::new(format_idr(item.open)), - Cell::new(format_idr(item.high)), - Cell::new(format_idr(item.low)), - Cell::new(format_idr(item.close)), - Cell::new(format_u64(item.volume)), + cell(item.date), + cell(format_idr(item.open)), + cell(format_idr(item.high)), + cell(format_idr(item.low)), + cell(format_idr(item.close)), + cell(format_u64(item.volume)), ]); } println!("{table}"); @@ -143,54 +144,54 @@ pub fn print_technical(report: &TechnicalReport, no_color: bool) -> Result<(), I .set_header(vec!["METRIC", "VALUE", "SIGNAL"]); table.add_row(vec![ - Cell::new("Current Price"), - Cell::new(format_idr(report.current_price)), - Cell::new("-"), + cell("Current Price"), + cell(format_idr(report.current_price)), + cell("-"), ]); table.add_row(vec![ - Cell::new("SMA 20"), - Cell::new(format_idr_option(report.sma20)), - Cell::new("-"), + cell("SMA 20"), + cell(format_idr_option(report.sma20)), + cell("-"), ]); table.add_row(vec![ - Cell::new("SMA 50"), - Cell::new(format_idr_option(report.sma50)), - Cell::new("-"), + cell("SMA 50"), + cell(format_idr_option(report.sma50)), + cell("-"), ]); table.add_row(vec![ - Cell::new("SMA 200"), - Cell::new(format_idr_option(report.sma200)), - Cell::new("-"), + cell("SMA 200"), + cell(format_idr_option(report.sma200)), + cell("-"), ]); table.add_row(vec![ - Cell::new("RSI (14)"), - Cell::new(format_float(report.rsi14, 2)), - Cell::new(format_signal(report.signals.rsi, no_color, false)), + cell("RSI (14)"), + cell(format_float(report.rsi14, 2)), + cell(format_signal(report.signals.rsi, no_color, false)), ]); table.add_row(vec![ - Cell::new("MACD (12,26,9)"), - Cell::new(format!( + cell("MACD (12,26,9)"), + cell(format!( "{}/{}/{}", format_float(report.macd.line, 2), format_float(report.macd.signal, 2), format_float(report.macd.histogram, 2) )), - Cell::new(format_signal(report.signals.macd, no_color, false)), + cell(format_signal(report.signals.macd, no_color, false)), ]); table.add_row(vec![ - Cell::new("Trend"), - Cell::new(trend_context(report)), - Cell::new(format_signal(report.signals.trend, no_color, false)), + cell("Trend"), + cell(trend_context(report)), + cell(format_signal(report.signals.trend, no_color, false)), ]); table.add_row(vec![ - Cell::new("Volume Ratio (20)"), - Cell::new(format_volume_ratio(report)), - Cell::new("-"), + cell("Volume Ratio (20)"), + cell(format_volume_ratio(report)), + cell("-"), ]); table.add_row(vec![ - Cell::new("Overall Signal"), - Cell::new("-"), - Cell::new(format_signal(report.signals.overall, no_color, true)), + cell("Overall Signal"), + cell("-"), + cell(format_signal(report.signals.overall, no_color, true)), ]); println!("{table}"); @@ -207,19 +208,19 @@ pub fn print_growth(symbol: &str, report: &GrowthReport, no_color: bool) -> Resu .set_header(vec!["METRIC", "VALUE", "SIGNAL"]); table.add_row(vec![ - Cell::new("Revenue Growth"), - Cell::new(format_pct(report.revenue_growth_pct)), - Cell::new(format_growth_signal(&report.revenue_signal, no_color)), + cell("Revenue Growth"), + cell(format_pct(report.revenue_growth_pct)), + cell(format_growth_signal(&report.revenue_signal, no_color)), ]); table.add_row(vec![ - Cell::new("Earnings Growth"), - Cell::new(format_pct(report.earnings_growth_pct)), - Cell::new(format_growth_signal(&report.earnings_signal, no_color)), + cell("Earnings Growth"), + cell(format_pct(report.earnings_growth_pct)), + cell(format_growth_signal(&report.earnings_signal, no_color)), ]); table.add_row(vec![ - Cell::new("Overall"), - Cell::new("-"), - Cell::new(format_growth_signal(&report.overall_signal, no_color)), + cell("Overall"), + cell("-"), + cell(format_growth_signal(&report.overall_signal, no_color)), ]); println!("{table}"); @@ -240,39 +241,39 @@ pub fn print_valuation( .set_header(vec!["METRIC", "VALUE", "SIGNAL"]); table.add_row(vec![ - Cell::new("P/E (Trailing)"), - Cell::new(format_opt_f64(report.pe_trailing, 2)), - Cell::new(format_valuation_signal(&report.pe_signal, no_color)), + cell("P/E (Trailing)"), + cell(format_opt_f64(report.pe_trailing, 2)), + cell(format_valuation_signal(&report.pe_signal, no_color)), ]); table.add_row(vec![ - Cell::new("P/E (Forward)"), - Cell::new(format_opt_f64(report.pe_forward, 2)), - Cell::new("-"), + cell("P/E (Forward)"), + cell(format_opt_f64(report.pe_forward, 2)), + cell("-"), ]); table.add_row(vec![ - Cell::new("Price/Book"), - Cell::new(format_opt_f64(report.pb, 2)), - Cell::new(format_valuation_signal(&report.pb_signal, no_color)), + cell("Price/Book"), + cell(format_opt_f64(report.pb, 2)), + cell(format_valuation_signal(&report.pb_signal, no_color)), ]); table.add_row(vec![ - Cell::new("ROE"), - Cell::new(format_pct(report.roe_pct)), - Cell::new(format_valuation_signal(&report.roe_signal, no_color)), + cell("ROE"), + cell(format_pct(report.roe_pct)), + cell(format_valuation_signal(&report.roe_signal, no_color)), ]); table.add_row(vec![ - Cell::new("Net Margin"), - Cell::new(format_pct(report.net_margin_pct)), - Cell::new(format_valuation_signal(&report.margin_signal, no_color)), + cell("Net Margin"), + cell(format_pct(report.net_margin_pct)), + cell(format_valuation_signal(&report.margin_signal, no_color)), ]); table.add_row(vec![ - Cell::new("EV/EBITDA"), - Cell::new(format_opt_f64(report.ev_ebitda, 2)), - Cell::new(format_valuation_signal(&report.ev_ebitda_signal, no_color)), + cell("EV/EBITDA"), + cell(format_opt_f64(report.ev_ebitda, 2)), + cell(format_valuation_signal(&report.ev_ebitda_signal, no_color)), ]); table.add_row(vec![ - Cell::new("Overall"), - Cell::new("-"), - Cell::new(format_valuation_signal(&report.overall_signal, no_color)), + cell("Overall"), + cell("-"), + cell(format_valuation_signal(&report.overall_signal, no_color)), ]); println!("{table}"); @@ -289,24 +290,24 @@ pub fn print_risk(symbol: &str, report: &RiskReport, no_color: bool) -> Result<( .set_header(vec!["METRIC", "VALUE", "SIGNAL"]); table.add_row(vec![ - Cell::new("Debt/Equity"), - Cell::new(format_opt_f64(report.debt_to_equity, 2)), - Cell::new(format_risk_signal(&report.de_signal, no_color)), + cell("Debt/Equity"), + cell(format_opt_f64(report.debt_to_equity, 2)), + cell(format_risk_signal(&report.de_signal, no_color)), ]); table.add_row(vec![ - Cell::new("Current Ratio"), - Cell::new(format_opt_f64(report.current_ratio, 2)), - Cell::new(format_risk_signal(&report.current_ratio_signal, no_color)), + cell("Current Ratio"), + cell(format_opt_f64(report.current_ratio, 2)), + cell(format_risk_signal(&report.current_ratio_signal, no_color)), ]); table.add_row(vec![ - Cell::new("ROA"), - Cell::new(format_pct(report.roa_pct)), - Cell::new("-"), + cell("ROA"), + cell(format_pct(report.roa_pct)), + cell("-"), ]); table.add_row(vec![ - Cell::new("Overall"), - Cell::new("-"), - Cell::new(format_risk_signal(&report.overall_signal, no_color)), + cell("Overall"), + cell("-"), + cell(format_risk_signal(&report.overall_signal, no_color)), ]); println!("{table}"); @@ -341,8 +342,8 @@ pub fn print_compare(reports: &[FundamentalReport], no_color: bool) -> Result<() .load_preset(UTF8_FULL) .set_content_arrangement(ContentArrangement::Dynamic); - let mut header = vec![Cell::new("METRIC")]; - header.extend(reports.iter().map(|report| Cell::new(&report.symbol))); + let mut header = vec![cell("METRIC")]; + header.extend(reports.iter().map(|report| cell(&report.symbol))); table.set_header(header); add_compare_row( @@ -519,10 +520,11 @@ fn format_earnings_period(period: &str) -> String { return format!("FY{trimmed}"); } + // ASCII only, so the byte slices below fall on character boundaries. if trimmed.len() == 6 + && trimmed.is_ascii() && trimmed.starts_with('Q') - && trimmed[1..2].chars().all(|ch| ch.is_ascii_digit()) - && trimmed[2..].chars().all(|ch| ch.is_ascii_digit()) + && trimmed[1..].bytes().all(|byte| byte.is_ascii_digit()) { return format!("{} {}", &trimmed[..2], &trimmed[2..]); } @@ -644,7 +646,7 @@ fn format_text_signal( } fn add_compare_row(table: &mut Table, label: &str, values: Vec) { - let mut row = vec![Cell::new(label)]; + let mut row = vec![cell(label)]; row.extend(values.into_iter().map(Cell::new)); table.add_row(row); } @@ -664,7 +666,7 @@ pub fn print_profile(profile: &CompanyProfile) -> Result<(), IdxError> { let add_if_present = |t: &mut Table, label: &str, value: &str| { if !value.is_empty() { - t.add_row(vec![Cell::new(label), Cell::new(value)]); + t.add_row(vec![cell(label), cell(value)]); } }; @@ -677,24 +679,17 @@ pub fn print_profile(profile: &CompanyProfile) -> Result<(), IdxError> { add_if_present(&mut table, "City", &profile.city); add_if_present(&mut table, "Phone", &profile.phone); if profile.employees > 0 { - table.add_row(vec![ - Cell::new("Employees"), - Cell::new(profile.employees.to_string()), - ]); + table.add_row(vec![cell("Employees"), cell(profile.employees.to_string())]); } if !profile.description.is_empty() { // Truncate long descriptions for table display - let desc = if profile.description.len() > 200 { - format!("{}...", &profile.description[..200]) - } else { - profile.description.clone() - }; - table.add_row(vec![Cell::new("Description"), Cell::new(desc)]); + let desc = truncate_chars(&profile.description, 200); + table.add_row(vec![cell("Description"), cell(desc)]); } if !profile.officers.is_empty() { table.add_row(vec![ - Cell::new("Executives"), - Cell::new( + cell("Executives"), + cell( profile .officers .iter() @@ -726,7 +721,7 @@ pub fn print_financials(fin: &FinancialStatements) -> Result<(), IdxError> { entries.sort_by(|(left, _), (right, _)| left.cmp(right)); for (label, value) in entries { - t.add_row(vec![Cell::new(label), Cell::new(format_idr(*value as i64))]); + t.add_row(vec![cell(label), cell(format_idr(*value as i64))]); } println!("{t}"); }; @@ -803,10 +798,10 @@ pub fn print_sentiment(data: &SentimentData) -> Result<(), IdxError> { .set_header(vec!["RANGE", "BULLISH", "BEARISH", "NEUTRAL"]); for row in &data.statistics { table.add_row(vec![ - Cell::new(&row.time_range), - Cell::new(row.bullish), - Cell::new(row.bearish), - Cell::new(row.neutral), + cell(&row.time_range), + cell(row.bullish), + cell(row.bearish), + cell(row.neutral), ]); } println!("{table}"); @@ -814,20 +809,20 @@ pub fn print_sentiment(data: &SentimentData) -> Result<(), IdxError> { } pub fn print_insights(data: &InsightData) -> Result<(), IdxError> { - println!("{}", data.summary); + println!("{}", terminal_safe(&data.summary)); if !data.last_updated.is_empty() { - println!("Last updated: {}", data.last_updated); + println!("Last updated: {}", terminal_safe(&data.last_updated)); } if !data.highlights.is_empty() { println!("Highlights:"); for h in &data.highlights { - println!("- {h}"); + println!("- {}", terminal_safe(h)); } } if !data.risks.is_empty() { println!("Risks:"); for r in &data.risks { - println!("- {r}"); + println!("- {}", terminal_safe(r)); } } Ok(()) @@ -840,11 +835,11 @@ pub fn print_news(items: &[NewsItem]) -> Result<(), IdxError> { .set_header(vec!["TITLE", "ABOUT", "PROVIDER", "DATE", "URL"]); for item in items { table.add_row(vec![ - Cell::new(&item.title), - Cell::new(item.relevance.as_str()), - Cell::new(&item.provider), - Cell::new(&item.published_at), - Cell::new(truncate_url(&item.url)), + cell(&item.title), + cell(item.relevance.as_str()), + cell(&item.provider), + cell(&item.published_at), + cell(truncate_url(&item.url)), ]); } println!("{table}"); @@ -853,13 +848,13 @@ pub fn print_news(items: &[NewsItem]) -> Result<(), IdxError> { fn add_earnings_history_row(table: &mut Table, row: &EarningsData) { table.add_row(vec![ - Cell::new(format_earnings_period(&row.period_type)), - Cell::new(format_float(row.eps_actual, 2)), - Cell::new(format_float(row.eps_forecast, 2)), - Cell::new(format_float(row.eps_surprise, 2)), - Cell::new(format_float(row.eps_surprise_pct, 2)), - Cell::new(format_idr_option_from_f64(row.revenue_actual)), - Cell::new(format_table_date_option( + cell(format_earnings_period(&row.period_type)), + cell(format_float(row.eps_actual, 2)), + cell(format_float(row.eps_forecast, 2)), + cell(format_float(row.eps_surprise, 2)), + cell(format_float(row.eps_surprise_pct, 2)), + cell(format_idr_option_from_f64(row.revenue_actual)), + cell(format_table_date_option( row.earning_release_date.as_deref(), )), ]); @@ -867,21 +862,17 @@ fn add_earnings_history_row(table: &mut Table, row: &EarningsData) { fn add_earnings_forecast_row(table: &mut Table, row: &EarningsData) { table.add_row(vec![ - Cell::new(format_earnings_period(&row.period_type)), - Cell::new(format_float(row.eps_forecast, 2)), - Cell::new(format_idr_option_from_f64(row.revenue_forecast)), - Cell::new(format_table_date_option( + cell(format_earnings_period(&row.period_type)), + cell(format_float(row.eps_forecast, 2)), + cell(format_idr_option_from_f64(row.revenue_forecast)), + cell(format_table_date_option( row.earning_release_date.as_deref(), )), ]); } fn truncate_url(url: &str) -> String { - if url.len() > 72 { - format!("{}...", &url[..72]) - } else { - url.to_string() - } + truncate_chars(url, 72) } #[cfg(test)] diff --git a/crates/idx-cli/src/output/text.rs b/crates/idx-cli/src/output/text.rs new file mode 100644 index 0000000..c3c92d1 --- /dev/null +++ b/crates/idx-cli/src/output/text.rs @@ -0,0 +1,70 @@ +//! Text from providers and KSEI files is untrusted: before it reaches a +//! terminal, control characters (which can start escape sequences that clear +//! the screen, rewrite output, or set the window title) and bidirectional +//! overrides (which can reorder what is displayed) are replaced. + +use comfy_table::Cell; + +/// `text` with control characters other than newline, and bidirectional +/// override characters, replaced by U+FFFD. +pub fn terminal_safe(text: &str) -> String { + text.chars() + .map(|ch| { + let bidi = matches!(ch, '\u{202A}'..='\u{202E}' | '\u{2066}'..='\u{2069}'); + if (ch.is_control() && ch != '\n') || bidi { + '\u{FFFD}' + } else { + ch + } + }) + .collect() +} + +/// A table cell whose content is made [`terminal_safe`]. +pub fn cell(value: impl ToString) -> Cell { + Cell::new(terminal_safe(&value.to_string())) +} + +/// At most `max` characters of `text` (never splitting one), with `...` +/// appended when it was cut. +pub fn truncate_chars(text: &str, max: usize) -> String { + match text.char_indices().nth(max) { + Some((end, _)) => format!("{}...", &text[..end]), + None => text.to_string(), + } +} + +#[cfg(test)] +mod tests { + use super::{terminal_safe, truncate_chars}; + + #[test] + fn escape_sequences_and_bidi_overrides_are_neutralized() { + assert_eq!( + terminal_safe("\u{1b}[2J\u{1b}[Hfake"), + "\u{FFFD}[2J\u{FFFD}[Hfake" + ); + assert_eq!( + terminal_safe("\u{1b}]0;title\u{7}x"), + "\u{FFFD}]0;title\u{FFFD}x" + ); + assert_eq!(terminal_safe("a\u{9b}31mb"), "a\u{FFFD}31mb"); + assert_eq!(terminal_safe("ab\u{202E}cd"), "ab\u{FFFD}cd"); + assert_eq!(terminal_safe("line one\nline two"), "line one\nline two"); + assert_eq!( + terminal_safe("Saham Grup Djarum’s 14,81%"), + "Saham Grup Djarum’s 14,81%" + ); + } + + #[test] + fn truncation_never_splits_a_character() { + let text = format!("{}é tail", "a".repeat(199)); + assert_eq!( + truncate_chars(&text, 200), + format!("{}é...", "a".repeat(199)) + ); + assert_eq!(truncate_chars("short", 200), "short"); + assert_eq!(truncate_chars("ééé", 2), "éé..."); + } +} diff --git a/crates/idx-cli/tests/golden/config-get.golden b/crates/idx-cli/tests/golden/config-get.golden index c9ca9de..a90ab07 100644 --- a/crates/idx-cli/tests/golden/config-get.golden +++ b/crates/idx-cli/tests/golden/config-get.golden @@ -1,4 +1,4 @@ exit: 0 --- stdout -msn +{"key":"general.provider","value":"msn"} --- stderr diff --git a/crates/idx-cli/tests/golden/config-init.golden b/crates/idx-cli/tests/golden/config-init.golden index ab029c5..28a0e2f 100644 --- a/crates/idx-cli/tests/golden/config-init.golden +++ b/crates/idx-cli/tests/golden/config-init.golden @@ -1,4 +1,4 @@ exit: 0 --- stdout -/config/idx/config.toml +{"path":"/config/idx/config.toml"} --- stderr diff --git a/crates/idx-cli/tests/golden/config-path.golden b/crates/idx-cli/tests/golden/config-path.golden index ab029c5..28a0e2f 100644 --- a/crates/idx-cli/tests/golden/config-path.golden +++ b/crates/idx-cli/tests/golden/config-path.golden @@ -1,4 +1,4 @@ exit: 0 --- stdout -/config/idx/config.toml +{"path":"/config/idx/config.toml"} --- stderr diff --git a/crates/idx-cli/tests/golden/config-set-output-jsonl.golden b/crates/idx-cli/tests/golden/config-set-output-jsonl.golden index aecf1eb..02653fc 100644 --- a/crates/idx-cli/tests/golden/config-set-output-jsonl.golden +++ b/crates/idx-cli/tests/golden/config-set-output-jsonl.golden @@ -1,4 +1,4 @@ exit: 0 --- stdout -ok +{"key":"general.output","ok":true,"value":"jsonl"} --- stderr diff --git a/crates/idx-cli/tests/golden/config-set.golden b/crates/idx-cli/tests/golden/config-set.golden index aecf1eb..0529834 100644 --- a/crates/idx-cli/tests/golden/config-set.golden +++ b/crates/idx-cli/tests/golden/config-set.golden @@ -1,4 +1,4 @@ exit: 0 --- stdout -ok +{"key":"general.provider","ok":true,"value":"yahoo"} --- stderr diff --git a/crates/idx-cli/tests/golden/error-usage-missing-symbol.json.golden b/crates/idx-cli/tests/golden/error-usage-missing-symbol.json.golden index e653302..60dd4a1 100644 --- a/crates/idx-cli/tests/golden/error-usage-missing-symbol.json.golden +++ b/crates/idx-cli/tests/golden/error-usage-missing-symbol.json.golden @@ -1,9 +1,4 @@ exit: 2 --- stdout --- stderr -error: the following required arguments were not provided: - ... - -Usage: idx stocks quote ... - -For more information, try '--help'. +{"code":"INVALIDINPUT","error":true,"message":"invalid input: the following required arguments were not provided:\n ...\n\nUsage: idx stocks quote ...\n\nFor more information, try '--help'."} diff --git a/crates/idx-core/src/cache.rs b/crates/idx-core/src/cache.rs index 5e1abb5..85669f8 100644 --- a/crates/idx-core/src/cache.rs +++ b/crates/idx-core/src/cache.rs @@ -185,13 +185,11 @@ impl Cache { } let mut removed = 0usize; let mut failed = Vec::new(); - Self::walk(&self.root, &mut |p| { - if p.is_file() { - match fs::remove_file(p) { - Ok(_) => removed += 1, - Err(_) => failed.push(p.to_path_buf()), - } - } + // `walk` yields files and symlinks, never entering a linked + // directory; remove_file on a symlink removes the link, not its target. + Self::walk(&self.root, &mut |p| match fs::remove_file(p) { + Ok(_) => removed += 1, + Err(_) => failed.push(p.to_path_buf()), })?; Ok((removed, failed)) } @@ -200,7 +198,10 @@ impl Cache { for entry in fs::read_dir(dir).map_err(|e| IdxError::Io(e.to_string()))? { let entry = entry.map_err(|e| IdxError::Io(e.to_string()))?; let path = entry.path(); - if path.is_dir() { + // file_type() does not follow symlinks, so a link to a directory + // outside the cache is never walked (or emptied by `clear`). + let file_type = entry.file_type().map_err(|e| IdxError::Io(e.to_string()))?; + if file_type.is_dir() { Self::walk(&path, f)?; } else { f(&path); @@ -297,6 +298,30 @@ pub fn cache_dir() -> Result { #[cfg(test)] mod tests { + + #[cfg(unix)] + #[test] + fn clear_removes_symlinks_without_touching_their_targets() { + let base = std::env::temp_dir().join(format!("idx-cache-link-{}", fastrand::u64(..))); + let outside = base.join("documents"); + let root = base.join("cache"); + std::fs::create_dir_all(&outside).unwrap(); + std::fs::create_dir_all(root.join("quote")).unwrap(); + std::fs::write(outside.join("keep.txt"), b"mine").unwrap(); + std::fs::write(root.join("quote").join("BBCA.json"), b"{}").unwrap(); + std::os::unix::fs::symlink(&outside, root.join("link")).unwrap(); + + let (removed, failed) = super::Cache::at(root.clone()).clear().unwrap(); + + assert!(failed.is_empty(), "{failed:?}"); + assert_eq!(removed, 2, "the cache file and the link itself"); + assert!( + outside.join("keep.txt").exists(), + "the link's target is left alone" + ); + assert!(!root.join("link").exists()); + std::fs::remove_dir_all(&base).unwrap(); + } use std::fs; use std::sync::atomic::{AtomicUsize, Ordering}; diff --git a/crates/idx-core/src/investor.rs b/crates/idx-core/src/investor.rs index b5012a2..6a13ca8 100644 --- a/crates/idx-core/src/investor.rs +++ b/crates/idx-core/src/investor.rs @@ -635,7 +635,7 @@ pub fn parse_report_family(raw: &str) -> Result, I "investor-type" | "investor_type" | "investortype" => { Ok(Some(OwnershipReportFamily::InvestorTypeBreakdown)) } - _ => Err(IdxError::ParseError( + _ => Err(IdxError::InvalidInput( "invalid --family, expected: all|above1|above5|investor-type".to_string(), )), } diff --git a/crates/idx-core/src/lib.rs b/crates/idx-core/src/lib.rs index bcc2e52..a89bc2f 100644 --- a/crates/idx-core/src/lib.rs +++ b/crates/idx-core/src/lib.rs @@ -23,6 +23,7 @@ pub mod runtime { use std::sync::atomic::{AtomicBool, Ordering}; static QUIET: AtomicBool = AtomicBool::new(false); + static JSON: AtomicBool = AtomicBool::new(false); pub fn set_quiet(quiet: bool) { QUIET.store(quiet, Ordering::Relaxed); @@ -32,15 +33,29 @@ pub mod runtime { QUIET.load(Ordering::Relaxed) } - pub fn info(message: impl Display) { - if !is_quiet() { - eprintln!("info: {message}"); + /// In JSON mode diagnostics are JSON lines, so stderr stays + /// machine-readable alongside a JSON error. + pub fn set_json(json: bool) { + JSON.store(json, Ordering::Relaxed); + } + + fn emit(level: &str, message: impl Display) { + if is_quiet() { + return; + } + if JSON.load(Ordering::Relaxed) { + let line = serde_json::json!({ "level": level, "message": message.to_string() }); + eprintln!("{line}"); + } else { + eprintln!("{level}: {message}"); } } + pub fn info(message: impl Display) { + emit("info", message); + } + pub fn warn(message: impl Display) { - if !is_quiet() { - eprintln!("warning: {message}"); - } + emit("warning", message); } } diff --git a/crates/idx-core/src/ownership/graph.rs b/crates/idx-core/src/ownership/graph.rs index d51fd12..274ec81 100644 --- a/crates/idx-core/src/ownership/graph.rs +++ b/crates/idx-core/src/ownership/graph.rs @@ -14,7 +14,7 @@ pub fn query_ownership_graph( ) -> Result<(Vec, Vec), IdxError> { let root = root.trim(); if root.is_empty() { - return Err(IdxError::ParseError( + return Err(IdxError::InvalidInput( "graph root cannot be empty".to_string(), )); } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index d5c10ae..bb7d1dd 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -162,7 +162,12 @@ CLI flags > environment variables > config file > defaults ## Error Strategy - `IdxError` enum (thiserror) with structured error codes - Table mode: human-readable error on stderr -- JSON and JSONL modes: `{"error": true, "code": "...", "message": "..."}` on stderr, stdout empty +- JSON and JSONL modes: `{"error": true, "code": "...", "message": "..."}` on stderr, stdout empty. + This includes argument errors (`INVALIDINPUT`, exit 2) when `-o`/`--output`, + `IDX_OUTPUT`, or auto mode with stdout not a terminal selects JSON; `--help` + and `--version` print normally. +- Untrusted text (provider strings, KSEI names) has control characters and + bidirectional overrides replaced before it reaches a terminal table. - Exit codes by class: `0` success, `2` `INVALIDINPUT`/`CONFIGERROR` and clap usage errors, `3` `SYMBOLNOTFOUND`/`NOMARKETDATA`/`CACHEMISS`, `4` `HTTP`/`RATELIMITED`/ `PROVIDERUNAVAILABLE`/`OFFLINE`/`AUTHERROR`, `5` `UNSUPPORTED`, `1` everything else @@ -182,4 +187,6 @@ CLI flags > environment variables > config file > defaults - `stocks quote` and `stocks compare` require at least one symbol. - Offline reads warn on stderr when serving cache entries past their TTL, including the cache fetch timestamp. `--quiet` suppresses these warnings. + In JSON mode warnings and info are JSON lines, + `{"level": "warning", "message": "..."}`, so stderr stays parseable. Staleness is not yet represented by a versioned JSON result envelope.