From 6d3f53ffc619bce4e2dfa2cdd905fdfbacdb69c6 Mon Sep 17 00:00:00 2001 From: Jeff MAURY Date: Thu, 4 Jun 2026 19:33:02 +0200 Subject: [PATCH 1/2] feat(cli): add generic output formatter to eliminate --output flag duplication MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #1750 Creates a new output.rs module with generic helper functions for formatting CLI command output in JSON, YAML, or table formats. This eliminates ~38 lines of duplicated code across 4 commands, provides a single source of truth for output formatting, and makes it easier to add new output formats in the future. Changes: - crates/openshell-cli/src/output.rs: New module with 6 helper functions and 14 tests - crates/openshell-cli/src/lib.rs: Register output module - crates/openshell-cli/src/run.rs: Migrate 4 commands to use helpers - gateway_list(): 23 lines → 5 lines - sandbox_list(): 17 lines → 5 lines - provider_list_profiles(): 12 lines → 7 lines - provider_profile_export(): 11 lines → 8 lines - architecture/cli.md: New CLI architecture documentation - architecture/README.md: Add reference to CLI architecture doc Co-Authored-By: Claude Sonnet 4.5 Signed-off-by: Jeff MAURY --- architecture/README.md | 1 + architecture/cli.md | 83 +++++ crates/openshell-cli/src/lib.rs | 1 + crates/openshell-cli/src/output.rs | 505 +++++++++++++++++++++++++++++ crates/openshell-cli/src/run.rs | 76 +---- 5 files changed, 608 insertions(+), 58 deletions(-) create mode 100644 architecture/cli.md create mode 100644 crates/openshell-cli/src/output.rs diff --git a/architecture/README.md b/architecture/README.md index 5a1d82831..8a954a430 100644 --- a/architecture/README.md +++ b/architecture/README.md @@ -161,6 +161,7 @@ that crate's `README.md`. | Document | Purpose | |---|---| +| [CLI](cli.md) | CLI architecture, command implementations, and output formatting patterns. | | [Gateway](gateway.md) | Gateway control plane, auth, APIs, persistence, settings, and relay coordination. | | [Sandbox](sandbox.md) | Sandbox supervisor, child process isolation, proxy, credentials, inference, connect, and logs. | | [Security Policy](security-policy.md) | Policy model, enforcement layers, policy updates, policy advisor, and security logging. | diff --git a/architecture/cli.md b/architecture/cli.md new file mode 100644 index 000000000..a7739b577 --- /dev/null +++ b/architecture/cli.md @@ -0,0 +1,83 @@ +# CLI Architecture + +The OpenShell CLI (`openshell`) provides a command-line interface for managing sandboxes, gateways, providers, and policies. + +## Component Overview + +| Component | Path | Purpose | +|-----------|------|---------| +| Main entry point | `crates/openshell-cli/src/main.rs` | CLI argument parsing, command routing | +| Command implementations | `crates/openshell-cli/src/run.rs` | Core command logic | +| Output formatting | `crates/openshell-cli/src/output.rs` | Generic output format helpers | + +## Output Formatting + +Many CLI commands support the `--output` flag, allowing users to specify output format as JSON, YAML, or table. The `output` module provides generic helper functions to eliminate duplication across commands. + +### Design + +**Early-return pattern:** Helper functions return `Result`: +- `Ok(true)` — Format was handled (json/yaml), caller should return immediately +- `Ok(false)` — Format is "table", caller should continue to table rendering +- `Err(e)` — Unsupported format or serialization error + +**Available functions:** + +- `print_output_collection(format, items, to_json)` — Format collections +- `print_output_single(format, item, to_json)` — Format single items +- `print_output_direct(format, json_fn, yaml_fn)` — Format pre-formatted strings +- `*_to_writer()` variants — Output to custom writers instead of stdout + +### Usage Pattern + +```rust +use crate::output::print_output_collection; + +pub fn sandbox_list(output: &str) -> Result<()> { + let sandboxes = fetch_sandboxes()?; + + // Handle json/yaml output with early return + if print_output_collection(output, &sandboxes, sandbox_to_json)? { + return Ok(()); + } + + // Fall through to table rendering for "table" format + render_sandbox_table(&sandboxes); + Ok(()) +} + +fn sandbox_to_json(sandbox: &Sandbox) -> serde_json::Value { + serde_json::json!({ + "id": sandbox.id, + "name": sandbox.name, + "status": sandbox.status, + }) +} +``` + +### Behavioral Details + +- **JSON output:** Uses `println!()` — includes trailing newline +- **YAML output:** Uses `print!()` — no trailing newline (`serde_yml` includes one) +- **Error handling:** All serialization errors wrapped with `.into_diagnostic()` for miette compatibility +- **Type flexibility:** Format parameter accepts `impl AsRef` for both `&str` and enum types + +### Adding Output Support to New Commands + +1. Accept an `output: &str` parameter (or use the `OutputFormat` enum from `main.rs`) +2. Write a converter function: `fn item_to_json(item: &Item) -> serde_json::Value` +3. Call the appropriate helper with early-return pattern +4. Implement table rendering for the "table" format + +See `gateway_list()` (line 1292) and `sandbox_list()` (line 3166) in `src/run.rs` for examples. + +## Commands Using Output Formatting + +| Command | Lines | Helper Used | Converter Function | +|---------|-------|-------------|-------------------| +| `gateway list` | ~1290 | `print_output_collection` | `gateway_to_json` | +| `sandbox list` | ~3166 | `print_output_collection` | `sandbox_to_json` | +| `provider list-profiles` | ~4647 | `print_output_direct` | External crate functions | +| `provider profile export` | ~4701 | `print_output_direct` | External crate functions | + +Policy commands (`sandbox policy get`, etc.) use writer-based variants for custom output destinations. diff --git a/crates/openshell-cli/src/lib.rs b/crates/openshell-cli/src/lib.rs index 84a87acd2..156668951 100644 --- a/crates/openshell-cli/src/lib.rs +++ b/crates/openshell-cli/src/lib.rs @@ -12,6 +12,7 @@ pub mod auth; pub mod completers; pub mod edge_tunnel; pub mod oidc_auth; +pub mod output; pub(crate) mod policy_update; pub mod run; pub mod ssh; diff --git a/crates/openshell-cli/src/output.rs b/crates/openshell-cli/src/output.rs new file mode 100644 index 000000000..3a42be71d --- /dev/null +++ b/crates/openshell-cli/src/output.rs @@ -0,0 +1,505 @@ +// Copyright (C) 2025 NVIDIA Corporation +// SPDX-License-Identifier: Apache-2.0 + +//! Generic output formatting helpers for CLI commands. +//! +//! This module provides helper functions to eliminate duplication across CLI commands +//! that support `--output` flags (json, yaml, table). +//! +//! # Early-return Pattern +//! +//! All helper functions return `Result`: +//! - `Ok(true)` — Format was handled (json/yaml), caller should return immediately +//! - `Ok(false)` — Format is "table", caller should continue to table rendering +//! - `Err(e)` — Unsupported format or serialization error +//! +//! # Example +//! +//! ```ignore +//! use crate::output::print_output_collection; +//! +//! pub fn sandbox_list(output: &str) -> Result<()> { +//! let sandboxes = fetch_sandboxes()?; +//! +//! if print_output_collection(output, &sandboxes, sandbox_to_json)? { +//! return Ok(()); +//! } +//! +//! // Fall through to table rendering +//! render_sandbox_table(&sandboxes); +//! Ok(()) +//! } +//! ``` + +use miette::{IntoDiagnostic, Result}; +use std::io::Write; + +/// Print collection output in specified format (json/yaml/table). +/// +/// # Returns +/// - `Ok(true)` if format was handled (json/yaml), caller should return +/// - `Ok(false)` if format is "table", caller should continue to table rendering +/// - `Err` for unsupported formats or serialization errors +/// +/// # Behavior +/// - JSON: uses `println!()` (includes trailing newline) +/// - YAML: uses `print!()` (no trailing newline, `serde_yml` includes one) +/// +/// # Example +/// ```ignore +/// if print_output_collection(output, &sandboxes, sandbox_to_json)? { +/// return Ok(()); +/// } +/// ``` +pub fn print_output_collection( + format: impl AsRef, + items: &[T], + to_json: F, +) -> Result +where + F: Fn(&T) -> serde_json::Value, +{ + match format.as_ref() { + "json" => { + let values: Vec<_> = items.iter().map(to_json).collect(); + println!( + "{}", + serde_json::to_string_pretty(&values).into_diagnostic()? + ); + Ok(true) + } + "yaml" => { + let values: Vec<_> = items.iter().map(to_json).collect(); + print!("{}", serde_yml::to_string(&values).into_diagnostic()?); + Ok(true) + } + "table" => Ok(false), + _ => Err(miette::miette!( + "unsupported output format: {}", + format.as_ref() + )), + } +} + +/// Print single item output in specified format (json/yaml/table). +/// +/// # Returns +/// - `Ok(true)` if format was handled (json/yaml), caller should return +/// - `Ok(false)` if format is "table", caller should continue to table rendering +/// - `Err` for unsupported formats or serialization errors +/// +/// # Example +/// ```ignore +/// if print_output_single(output, &sandbox, sandbox_to_json)? { +/// return Ok(()); +/// } +/// ``` +pub fn print_output_single(format: impl AsRef, item: &T, to_json: F) -> Result +where + F: Fn(&T) -> serde_json::Value, +{ + match format.as_ref() { + "json" => { + let value = to_json(item); + println!( + "{}", + serde_json::to_string_pretty(&value).into_diagnostic()? + ); + Ok(true) + } + "yaml" => { + let value = to_json(item); + print!("{}", serde_yml::to_string(&value).into_diagnostic()?); + Ok(true) + } + "table" => Ok(false), + _ => Err(miette::miette!( + "unsupported output format: {}", + format.as_ref() + )), + } +} + +/// Print pre-formatted output in specified format (json/yaml/table). +/// +/// Use this when converter functions return `Result` instead of `serde_json::Value`. +/// +/// # Returns +/// - `Ok(true)` if format was handled (json/yaml), caller should return +/// - `Ok(false)` if format is "table", caller should continue to table rendering +/// - `Err` for unsupported formats or conversion errors +/// +/// # Example +/// ```ignore +/// if print_output_direct( +/// output, +/// || profiles_to_json(&profiles).into_diagnostic(), +/// || profiles_to_yaml(&profiles).into_diagnostic(), +/// )? { +/// return Ok(()); +/// } +/// ``` +pub fn print_output_direct(format: impl AsRef, json_fn: J, yaml_fn: Y) -> Result +where + J: FnOnce() -> Result, + Y: FnOnce() -> Result, +{ + match format.as_ref() { + "json" => { + let json = json_fn()?; + println!("{json}"); + Ok(true) + } + "yaml" => { + let yaml = yaml_fn()?; + print!("{yaml}"); + Ok(true) + } + "table" => Ok(false), + _ => Err(miette::miette!( + "unsupported output format: {}", + format.as_ref() + )), + } +} + +/// Print collection output to a custom writer in specified format (json/yaml/table). +/// +/// Writer variant for commands that need custom output destinations. +/// +/// # Returns +/// - `Ok(true)` if format was handled (json/yaml), caller should return +/// - `Ok(false)` if format is "table", caller should continue to table rendering +/// - `Err` for unsupported formats, serialization errors, or write errors +pub fn print_output_collection_to_writer( + format: impl AsRef, + writer: &mut W, + items: &[T], + to_json: F, +) -> Result +where + W: Write, + F: Fn(&T) -> serde_json::Value, +{ + match format.as_ref() { + "json" => { + let values: Vec<_> = items.iter().map(to_json).collect(); + writeln!( + writer, + "{}", + serde_json::to_string_pretty(&values).into_diagnostic()? + ) + .into_diagnostic()?; + Ok(true) + } + "yaml" => { + let values: Vec<_> = items.iter().map(to_json).collect(); + write!( + writer, + "{}", + serde_yml::to_string(&values).into_diagnostic()? + ) + .into_diagnostic()?; + Ok(true) + } + "table" => Ok(false), + _ => Err(miette::miette!( + "unsupported output format: {}", + format.as_ref() + )), + } +} + +/// Print single item output to a custom writer in specified format (json/yaml/table). +/// +/// Writer variant for commands that need custom output destinations. +/// +/// # Returns +/// - `Ok(true)` if format was handled (json/yaml), caller should return +/// - `Ok(false)` if format is "table", caller should continue to table rendering +/// - `Err` for unsupported formats, serialization errors, or write errors +pub fn print_output_single_to_writer( + format: impl AsRef, + writer: &mut W, + item: &T, + to_json: F, +) -> Result +where + W: Write, + F: Fn(&T) -> serde_json::Value, +{ + match format.as_ref() { + "json" => { + let value = to_json(item); + writeln!( + writer, + "{}", + serde_json::to_string_pretty(&value).into_diagnostic()? + ) + .into_diagnostic()?; + Ok(true) + } + "yaml" => { + let value = to_json(item); + write!( + writer, + "{}", + serde_yml::to_string(&value).into_diagnostic()? + ) + .into_diagnostic()?; + Ok(true) + } + "table" => Ok(false), + _ => Err(miette::miette!( + "unsupported output format: {}", + format.as_ref() + )), + } +} + +/// Print pre-formatted output to a custom writer in specified format (json/yaml/table). +/// +/// Writer variant for commands that need custom output destinations and use +/// pre-formatted converters returning `Result`. +/// +/// # Returns +/// - `Ok(true)` if format was handled (json/yaml), caller should return +/// - `Ok(false)` if format is "table", caller should continue to table rendering +/// - `Err` for unsupported formats, conversion errors, or write errors +pub fn print_output_direct_to_writer( + format: impl AsRef, + writer: &mut W, + json_fn: J, + yaml_fn: Y, +) -> Result +where + W: Write, + J: FnOnce() -> Result, + Y: FnOnce() -> Result, +{ + match format.as_ref() { + "json" => { + let json = json_fn()?; + writeln!(writer, "{json}").into_diagnostic()?; + Ok(true) + } + "yaml" => { + let yaml = yaml_fn()?; + write!(writer, "{yaml}").into_diagnostic()?; + Ok(true) + } + "table" => Ok(false), + _ => Err(miette::miette!( + "unsupported output format: {}", + format.as_ref() + )), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[derive(Debug, Clone)] + struct TestItem { + id: u32, + name: String, + } + + fn test_item_to_json(item: &TestItem) -> serde_json::Value { + serde_json::json!({ + "id": item.id, + "name": item.name, + }) + } + + #[test] + fn test_print_output_collection_json() { + let items = vec![ + TestItem { + id: 1, + name: "first".to_string(), + }, + TestItem { + id: 2, + name: "second".to_string(), + }, + ]; + + // Note: This test doesn't capture stdout, but verifies the function returns Ok(true) + let result = print_output_collection("json", &items, test_item_to_json); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), true); + } + + #[test] + fn test_print_output_collection_yaml() { + let items = vec![TestItem { + id: 1, + name: "test".to_string(), + }]; + + let result = print_output_collection("yaml", &items, test_item_to_json); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), true); + } + + #[test] + fn test_print_output_collection_table() { + let items = vec![TestItem { + id: 1, + name: "test".to_string(), + }]; + + let result = print_output_collection("table", &items, test_item_to_json); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), false); // Should return false for table + } + + #[test] + fn test_print_output_collection_unsupported() { + let items = vec![TestItem { + id: 1, + name: "test".to_string(), + }]; + + let result = print_output_collection("csv", &items, test_item_to_json); + assert!(result.is_err()); + assert!( + result + .unwrap_err() + .to_string() + .contains("unsupported output format") + ); + } + + #[test] + fn test_print_output_collection_empty() { + let items: Vec = vec![]; + + let result = print_output_collection("json", &items, test_item_to_json); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), true); + } + + #[test] + fn test_print_output_single_json() { + let item = TestItem { + id: 42, + name: "single".to_string(), + }; + + let result = print_output_single("json", &item, test_item_to_json); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), true); + } + + #[test] + fn test_print_output_single_yaml() { + let item = TestItem { + id: 42, + name: "single".to_string(), + }; + + let result = print_output_single("yaml", &item, test_item_to_json); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), true); + } + + #[test] + fn test_print_output_direct_json() { + let json_fn = || Ok(r#"{"test": true}"#.to_string()); + let yaml_fn = || Ok("test: true".to_string()); + + let result = print_output_direct("json", json_fn, yaml_fn); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), true); + } + + #[test] + fn test_print_output_direct_yaml() { + let json_fn = || Ok(r#"{"test": true}"#.to_string()); + let yaml_fn = || Ok("test: true".to_string()); + + let result = print_output_direct("yaml", json_fn, yaml_fn); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), true); + } + + #[test] + fn test_print_output_direct_error_propagation() { + let json_fn = || Err(miette::miette!("json conversion failed")); + let yaml_fn = || Ok("test: true".to_string()); + + let result = print_output_direct("json", json_fn, yaml_fn); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("json conversion")); + } + + #[test] + fn test_print_output_collection_to_writer_json() { + let items = vec![TestItem { + id: 1, + name: "test".to_string(), + }]; + let mut buffer = Vec::new(); + + let result = + print_output_collection_to_writer("json", &mut buffer, &items, test_item_to_json); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), true); + + let output = String::from_utf8(buffer).unwrap(); + assert!(output.contains("\"id\": 1")); + assert!(output.contains("\"name\": \"test\"")); + } + + #[test] + fn test_print_output_collection_to_writer_yaml() { + let items = vec![TestItem { + id: 1, + name: "test".to_string(), + }]; + let mut buffer = Vec::new(); + + let result = + print_output_collection_to_writer("yaml", &mut buffer, &items, test_item_to_json); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), true); + + let output = String::from_utf8(buffer).unwrap(); + assert!(output.contains("id: 1")); + assert!(output.contains("name: test")); + } + + #[test] + fn test_print_output_single_to_writer_json() { + let item = TestItem { + id: 42, + name: "single".to_string(), + }; + let mut buffer = Vec::new(); + + let result = print_output_single_to_writer("json", &mut buffer, &item, test_item_to_json); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), true); + + let output = String::from_utf8(buffer).unwrap(); + assert!(output.contains("\"id\": 42")); + assert!(output.contains("\"name\": \"single\"")); + } + + #[test] + fn test_print_output_direct_to_writer_json() { + let mut buffer = Vec::new(); + let json_fn = || Ok(r#"{"test": true}"#.to_string()); + let yaml_fn = || Ok("test: true".to_string()); + + let result = print_output_direct_to_writer("json", &mut buffer, json_fn, yaml_fn); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), true); + + let output = String::from_utf8(buffer).unwrap(); + assert!(output.contains(r#"{"test": true}"#)); + } +} diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 9988d46db..f6eb3c080 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -1289,28 +1289,8 @@ pub fn gateway_list(gateway_flag: &Option, output: &str) -> Result<()> { let gateways = list_gateways()?; let active = gateway_flag.clone().or_else(load_active_gateway); - match output { - "json" => { - let items: Vec = gateways - .iter() - .map(|g| gateway_to_json(g, &active)) - .collect(); - println!( - "{}", - serde_json::to_string_pretty(&items).into_diagnostic()? - ); - return Ok(()); - } - "yaml" => { - let items: Vec = gateways - .iter() - .map(|g| gateway_to_json(g, &active)) - .collect(); - print!("{}", serde_yml::to_string(&items).into_diagnostic()?); - return Ok(()); - } - "table" => {} - _ => return Err(miette!("unsupported output format: {output}")), + if crate::output::print_output_collection(output, &gateways, |g| gateway_to_json(g, &active))? { + return Ok(()); } if gateways.is_empty() { @@ -3182,22 +3162,8 @@ pub async fn sandbox_list( let sandboxes = response.into_inner().sandboxes; - match output { - "json" => { - let items: Vec = sandboxes.iter().map(sandbox_to_json).collect(); - println!( - "{}", - serde_json::to_string_pretty(&items).into_diagnostic()? - ); - return Ok(()); - } - "yaml" => { - let items: Vec = sandboxes.iter().map(sandbox_to_json).collect(); - print!("{}", serde_yml::to_string(&items).into_diagnostic()?); - return Ok(()); - } - "table" => {} - _ => return Err(miette!("unsupported output format: {output}")), + if crate::output::print_output_collection(output, &sandboxes, sandbox_to_json)? { + return Ok(()); } if sandboxes.is_empty() { @@ -4697,17 +4663,12 @@ pub async fn provider_list_profiles(server: &str, output: &str, tls: &TlsOptions .map(ProviderTypeProfile::from_proto) .collect::>(); - match output { - "yaml" => { - print!("{}", profiles_to_yaml(&dto_profiles).into_diagnostic()?); - return Ok(()); - } - "json" => { - println!("{}", profiles_to_json(&dto_profiles).into_diagnostic()?); - return Ok(()); - } - "table" => {} - _ => return Err(miette!("unsupported output format: {output}")), + if crate::output::print_output_direct( + output, + || profiles_to_json(&dto_profiles).into_diagnostic(), + || profiles_to_yaml(&dto_profiles).into_diagnostic(), + )? { + return Ok(()); } if profiles.is_empty() { @@ -4748,15 +4709,14 @@ pub async fn provider_profile_export( .ok_or_else(|| miette!("provider profile '{id}' not found"))?; let profile = ProviderTypeProfile::from_proto(&profile); - match output { - "yaml" => print!("{}", profile_to_yaml(&profile).into_diagnostic()?), - "json" => println!("{}", profile_to_json(&profile).into_diagnostic()?), - "table" => { - return Err(miette!( - "profile export supports '-o yaml' and '-o json'; table output is not supported" - )); - } - _ => return Err(miette!("unsupported output format: {output}")), + if !crate::output::print_output_direct( + output, + || profile_to_json(&profile).into_diagnostic(), + || profile_to_yaml(&profile).into_diagnostic(), + )? { + return Err(miette!( + "profile export supports '-o yaml' and '-o json'; table output is not supported" + )); } Ok(()) } From 991cc1825756e24d0c67687c784e7cbd72d65b03 Mon Sep 17 00:00:00 2001 From: Jeff MAURY Date: Fri, 5 Jun 2026 09:03:11 +0200 Subject: [PATCH 2/2] fix: apply recommandations from gator-agent Signed-off-by: Jeff MAURY --- architecture/README.md | 1 - architecture/cli.md | 83 ------------------------------ crates/openshell-cli/src/output.rs | 24 ++++----- 3 files changed, 12 insertions(+), 96 deletions(-) delete mode 100644 architecture/cli.md diff --git a/architecture/README.md b/architecture/README.md index 8a954a430..5a1d82831 100644 --- a/architecture/README.md +++ b/architecture/README.md @@ -161,7 +161,6 @@ that crate's `README.md`. | Document | Purpose | |---|---| -| [CLI](cli.md) | CLI architecture, command implementations, and output formatting patterns. | | [Gateway](gateway.md) | Gateway control plane, auth, APIs, persistence, settings, and relay coordination. | | [Sandbox](sandbox.md) | Sandbox supervisor, child process isolation, proxy, credentials, inference, connect, and logs. | | [Security Policy](security-policy.md) | Policy model, enforcement layers, policy updates, policy advisor, and security logging. | diff --git a/architecture/cli.md b/architecture/cli.md deleted file mode 100644 index a7739b577..000000000 --- a/architecture/cli.md +++ /dev/null @@ -1,83 +0,0 @@ -# CLI Architecture - -The OpenShell CLI (`openshell`) provides a command-line interface for managing sandboxes, gateways, providers, and policies. - -## Component Overview - -| Component | Path | Purpose | -|-----------|------|---------| -| Main entry point | `crates/openshell-cli/src/main.rs` | CLI argument parsing, command routing | -| Command implementations | `crates/openshell-cli/src/run.rs` | Core command logic | -| Output formatting | `crates/openshell-cli/src/output.rs` | Generic output format helpers | - -## Output Formatting - -Many CLI commands support the `--output` flag, allowing users to specify output format as JSON, YAML, or table. The `output` module provides generic helper functions to eliminate duplication across commands. - -### Design - -**Early-return pattern:** Helper functions return `Result`: -- `Ok(true)` — Format was handled (json/yaml), caller should return immediately -- `Ok(false)` — Format is "table", caller should continue to table rendering -- `Err(e)` — Unsupported format or serialization error - -**Available functions:** - -- `print_output_collection(format, items, to_json)` — Format collections -- `print_output_single(format, item, to_json)` — Format single items -- `print_output_direct(format, json_fn, yaml_fn)` — Format pre-formatted strings -- `*_to_writer()` variants — Output to custom writers instead of stdout - -### Usage Pattern - -```rust -use crate::output::print_output_collection; - -pub fn sandbox_list(output: &str) -> Result<()> { - let sandboxes = fetch_sandboxes()?; - - // Handle json/yaml output with early return - if print_output_collection(output, &sandboxes, sandbox_to_json)? { - return Ok(()); - } - - // Fall through to table rendering for "table" format - render_sandbox_table(&sandboxes); - Ok(()) -} - -fn sandbox_to_json(sandbox: &Sandbox) -> serde_json::Value { - serde_json::json!({ - "id": sandbox.id, - "name": sandbox.name, - "status": sandbox.status, - }) -} -``` - -### Behavioral Details - -- **JSON output:** Uses `println!()` — includes trailing newline -- **YAML output:** Uses `print!()` — no trailing newline (`serde_yml` includes one) -- **Error handling:** All serialization errors wrapped with `.into_diagnostic()` for miette compatibility -- **Type flexibility:** Format parameter accepts `impl AsRef` for both `&str` and enum types - -### Adding Output Support to New Commands - -1. Accept an `output: &str` parameter (or use the `OutputFormat` enum from `main.rs`) -2. Write a converter function: `fn item_to_json(item: &Item) -> serde_json::Value` -3. Call the appropriate helper with early-return pattern -4. Implement table rendering for the "table" format - -See `gateway_list()` (line 1292) and `sandbox_list()` (line 3166) in `src/run.rs` for examples. - -## Commands Using Output Formatting - -| Command | Lines | Helper Used | Converter Function | -|---------|-------|-------------|-------------------| -| `gateway list` | ~1290 | `print_output_collection` | `gateway_to_json` | -| `sandbox list` | ~3166 | `print_output_collection` | `sandbox_to_json` | -| `provider list-profiles` | ~4647 | `print_output_direct` | External crate functions | -| `provider profile export` | ~4701 | `print_output_direct` | External crate functions | - -Policy commands (`sandbox policy get`, etc.) use writer-based variants for custom output destinations. diff --git a/crates/openshell-cli/src/output.rs b/crates/openshell-cli/src/output.rs index 3a42be71d..cac6bae72 100644 --- a/crates/openshell-cli/src/output.rs +++ b/crates/openshell-cli/src/output.rs @@ -329,7 +329,7 @@ mod tests { // Note: This test doesn't capture stdout, but verifies the function returns Ok(true) let result = print_output_collection("json", &items, test_item_to_json); assert!(result.is_ok()); - assert_eq!(result.unwrap(), true); + assert!(result.unwrap()); } #[test] @@ -341,7 +341,7 @@ mod tests { let result = print_output_collection("yaml", &items, test_item_to_json); assert!(result.is_ok()); - assert_eq!(result.unwrap(), true); + assert!(result.unwrap()); } #[test] @@ -353,7 +353,7 @@ mod tests { let result = print_output_collection("table", &items, test_item_to_json); assert!(result.is_ok()); - assert_eq!(result.unwrap(), false); // Should return false for table + assert!(result.unwrap()); // Should return false for table } #[test] @@ -379,7 +379,7 @@ mod tests { let result = print_output_collection("json", &items, test_item_to_json); assert!(result.is_ok()); - assert_eq!(result.unwrap(), true); + assert!(result.unwrap()); } #[test] @@ -391,7 +391,7 @@ mod tests { let result = print_output_single("json", &item, test_item_to_json); assert!(result.is_ok()); - assert_eq!(result.unwrap(), true); + assert!(result.unwrap()); } #[test] @@ -403,7 +403,7 @@ mod tests { let result = print_output_single("yaml", &item, test_item_to_json); assert!(result.is_ok()); - assert_eq!(result.unwrap(), true); + assert!(result.unwrap()); } #[test] @@ -413,7 +413,7 @@ mod tests { let result = print_output_direct("json", json_fn, yaml_fn); assert!(result.is_ok()); - assert_eq!(result.unwrap(), true); + assert!(result.unwrap()); } #[test] @@ -423,7 +423,7 @@ mod tests { let result = print_output_direct("yaml", json_fn, yaml_fn); assert!(result.is_ok()); - assert_eq!(result.unwrap(), true); + assert!(result.unwrap()); } #[test] @@ -447,7 +447,7 @@ mod tests { let result = print_output_collection_to_writer("json", &mut buffer, &items, test_item_to_json); assert!(result.is_ok()); - assert_eq!(result.unwrap(), true); + assert!(result.unwrap()); let output = String::from_utf8(buffer).unwrap(); assert!(output.contains("\"id\": 1")); @@ -465,7 +465,7 @@ mod tests { let result = print_output_collection_to_writer("yaml", &mut buffer, &items, test_item_to_json); assert!(result.is_ok()); - assert_eq!(result.unwrap(), true); + assert!(result.unwrap()); let output = String::from_utf8(buffer).unwrap(); assert!(output.contains("id: 1")); @@ -482,7 +482,7 @@ mod tests { let result = print_output_single_to_writer("json", &mut buffer, &item, test_item_to_json); assert!(result.is_ok()); - assert_eq!(result.unwrap(), true); + assert!(result.unwrap()); let output = String::from_utf8(buffer).unwrap(); assert!(output.contains("\"id\": 42")); @@ -497,7 +497,7 @@ mod tests { let result = print_output_direct_to_writer("json", &mut buffer, json_fn, yaml_fn); assert!(result.is_ok()); - assert_eq!(result.unwrap(), true); + assert!(result.unwrap()); let output = String::from_utf8(buffer).unwrap(); assert!(output.contains(r#"{"test": true}"#));