Skip to content

refactor(output): unify OutputFormat and standardize table library #760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
joshrotenberg merged 1 commit into from
Mar 3, 2026
Merged

refactor(output): unify OutputFormat and standardize table library #760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 0 additions & 35 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ toml = "0.9"
directories = "6.0"

# Terminal and output
comfy-table = "7.2"
serde_yaml = "0.9"
csv = "1.3"
jpx-core = "0.2.1"
Expand Down
1 change: 0 additions & 1 deletion crates/redisctl/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ flate2 = { workspace = true }
# Shared utility dependencies
thiserror = { workspace = true }
serde_yaml = { workspace = true }
comfy-table = { workspace = true }
jpx-core = { workspace = true }
config = { workspace = true }

Expand Down
10 changes: 10 additions & 0 deletions crates/redisctl/src/cli/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,16 @@ pub enum OutputFormat {
Table,
}

impl OutputFormat {
pub fn is_json(&self) -> bool {
matches!(self, Self::Json)
}

pub fn is_yaml(&self) -> bool {
matches!(self, Self::Yaml)
}
}

/// Top-level commands
#[derive(Subcommand, Debug)]
pub enum Commands {
Expand Down
18 changes: 6 additions & 12 deletions crates/redisctl/src/commands/api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,10 @@ async fn handle_cloud_api(

match result {
Ok(response) => {
// Convert CLI OutputFormat to output::OutputFormat
// Raw API responses aren't structured for tables, resolve Auto to Json
let format = match output_format {
crate::cli::OutputFormat::Auto | crate::cli::OutputFormat::Json => {
crate::output::OutputFormat::Json
}
crate::cli::OutputFormat::Yaml => crate::output::OutputFormat::Yaml,
crate::cli::OutputFormat::Table => crate::output::OutputFormat::Table,
OutputFormat::Auto => OutputFormat::Json,
other => other,
};

print_output(response, format, query.as_deref()).map_err(|e| {
Expand Down Expand Up @@ -186,13 +183,10 @@ async fn handle_enterprise_api(

match result {
Ok(response) => {
// Convert CLI OutputFormat to output::OutputFormat
// Raw API responses aren't structured for tables, resolve Auto to Json
let format = match output_format {
crate::cli::OutputFormat::Auto | crate::cli::OutputFormat::Json => {
crate::output::OutputFormat::Json
}
crate::cli::OutputFormat::Yaml => crate::output::OutputFormat::Yaml,
crate::cli::OutputFormat::Table => crate::output::OutputFormat::Table,
OutputFormat::Auto => OutputFormat::Json,
other => other,
};

print_output(response, format, query.as_deref()).map_err(|e| {
Expand Down
42 changes: 22 additions & 20 deletions crates/redisctl/src/commands/cloud/cloud_account_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ use crate::connection::ConnectionManager;
use crate::error::{RedisCtlError, Result as CliResult};

use anyhow::Context;
use comfy_table::{Cell, Color, Table};
use colored::Colorize;
use redis_cloud::CloudClient;
use serde_json::{Value, json};
use tabled::builder::Builder;
use tabled::settings::Style;

/// Parameters for cloud account operations that support async operations
pub struct CloudAccountOperationParams<'a> {
Expand Down Expand Up @@ -69,12 +71,12 @@ pub async fn handle_list(
.context("Failed to list cloud accounts")?;

// For table output, create a formatted table
if matches!(output_format, OutputFormat::Table)
if matches!(output_format, OutputFormat::Table | OutputFormat::Auto)
&& query.is_none()
&& let Some(accounts) = result.get("cloudAccounts").and_then(|a| a.as_array())
{
let mut table = Table::new();
table.set_header(vec!["ID", "Name", "Provider", "Status", "Created"]);
let mut builder = Builder::default();
builder.push_record(["ID", "Name", "Provider", "Status", "Created"]);

for account in accounts {
let id = account.get("id").and_then(|v| v.as_i64()).unwrap_or(0);
Expand All @@ -89,22 +91,22 @@ pub async fn handle_list(
.and_then(|v| v.as_str())
.unwrap_or("");

let status_cell = match status {
"active" => Cell::new(status).fg(Color::Green),
"inactive" => Cell::new(status).fg(Color::Red),
_ => Cell::new(status),
let status_str = match status {
"active" => status.green().to_string(),
"inactive" => status.red().to_string(),
_ => status.to_string(),
};

table.add_row(vec![
Cell::new(id),
Cell::new(name),
Cell::new(provider),
status_cell,
Cell::new(created_timestamp),
builder.push_record([
&id.to_string(),
name,
provider,
&status_str,
created_timestamp,
]);
}

println!("{}", table);
println!("{}", builder.build().with(Style::blank()));
return Ok(());
}

Expand All @@ -125,9 +127,9 @@ pub async fn handle_get(
.context("Failed to get cloud account")?;

// For table output, create a detailed view
if matches!(output_format, OutputFormat::Table) && query.is_none() {
let mut table = Table::new();
table.set_header(vec!["Field", "Value"]);
if matches!(output_format, OutputFormat::Table | OutputFormat::Auto) && query.is_none() {
let mut builder = Builder::default();
builder.push_record(["Field", "Value"]);

if let Some(obj) = result.as_object() {
for (key, value) in obj {
Expand All @@ -141,11 +143,11 @@ pub async fn handle_get(
_ => value.to_string(),
}
};
table.add_row(vec![Cell::new(key), Cell::new(display_value)]);
builder.push_record([key.as_str(), &display_value]);
}
}

println!("{}", table);
println!("{}", builder.build().with(Style::blank()));
return Ok(());
}

Expand Down
41 changes: 18 additions & 23 deletions crates/redisctl/src/commands/cloud/connectivity/vpc_peering.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,9 @@ fn print_vpc_peering_table(data: &Value) -> CliResult<()> {

/// Print VPC peering list in table format
fn print_vpc_peering_list_table(data: &Value) -> CliResult<()> {
use comfy_table::{Cell, Color, Table};
use colored::Colorize;
use tabled::builder::Builder;
use tabled::settings::Style;

let peerings = if let Some(arr) = data.as_array() {
arr.clone()
Expand All @@ -698,15 +700,8 @@ fn print_vpc_peering_list_table(data: &Value) -> CliResult<()> {
return Ok(());
}

let mut table = Table::new();
table.set_header(vec![
"ID",
"Status",
"VPC ID",
"Account ID",
"Region",
"CIDRs",
]);
let mut builder = Builder::default();
builder.push_record(["ID", "Status", "VPC ID", "Account ID", "Region", "CIDRs"]);

for peering in peerings {
let id = peering
Expand Down Expand Up @@ -741,23 +736,23 @@ fn print_vpc_peering_list_table(data: &Value) -> CliResult<()> {
String::new()
};

let status_cell = match status.to_lowercase().as_str() {
"active" => Cell::new(status).fg(Color::Green),
"pending" => Cell::new(status).fg(Color::Yellow),
"failed" | "error" => Cell::new(status).fg(Color::Red),
_ => Cell::new(status),
let status_str = match status.to_lowercase().as_str() {
"active" => status.green().to_string(),
"pending" => status.yellow().to_string(),
"failed" | "error" => status.red().to_string(),
_ => status.to_string(),
};

table.add_row(vec![
Cell::new(id),
status_cell,
Cell::new(vpc_id),
Cell::new(account_id),
Cell::new(region),
Cell::new(cidrs),
builder.push_record([
&id.to_string(),
&status_str,
vpc_id,
account_id,
region,
&cidrs,
]);
}

println!("{}", table);
println!("{}", builder.build().with(Style::blank()));
Ok(())
}
48 changes: 1 addition & 47 deletions crates/redisctl/src/commands/cloud/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,7 @@ use unicode_segmentation::UnicodeSegmentation;

use std::io::IsTerminal;

use crate::cli::OutputFormat;

use crate::error::{RedisCtlError, Result as CliResult};
use crate::output::print_output;

/// Row structure for vertical table display (used by get commands)
#[derive(Tabled)]
Expand Down Expand Up @@ -189,50 +186,7 @@ pub fn provider_short_name(provider: &str) -> &str {
}
}

/// Apply JMESPath query to JSON data (using extended runtime with 400+ functions)
pub fn apply_jmespath(data: &Value, query: &str) -> CliResult<Value> {
let expr = crate::output::compile_jmespath(query)
.with_context(|| format!("Invalid JMESPath expression: {}", query))?;

expr.search(data)
.with_context(|| format!("Failed to apply JMESPath query: {}", query))
.map_err(Into::into)
}

/// Handle output formatting for different formats
pub fn handle_output(
data: Value,
_output_format: OutputFormat,
query: Option<&str>,
) -> CliResult<Value> {
if let Some(q) = query {
apply_jmespath(&data, q)
} else {
Ok(data)
}
}

/// Print data in requested output format
pub fn print_formatted_output(data: Value, output_format: OutputFormat) -> CliResult<()> {
match output_format {
OutputFormat::Json => {
print_output(data, crate::output::OutputFormat::Json, None).map_err(|e| {
RedisCtlError::OutputError {
message: e.to_string(),
}
})?;
}
OutputFormat::Yaml => {
print_output(data, crate::output::OutputFormat::Yaml, None).map_err(|e| {
RedisCtlError::OutputError {
message: e.to_string(),
}
})?;
}
_ => {} // Table format handled by individual commands
}
Ok(())
}
pub use crate::output::{apply_jmespath, handle_output, print_formatted_output};

/// Prompts the user for confirmation
pub fn confirm_action(message: &str) -> CliResult<bool> {
Expand Down
Loading
Loading