Skip to content

Commit 85f6aad

Browse files
committed
fix(cli): address remaining CodeRabbit review feedback
- Handle EOF on stdin to prevent infinite loop when piped - Implement Display trait for Value to deduplicate format_value - Use serde_json for safe JSON error fallback (proper escaping) - Support case-insensitive ASK/EXPLAIN command matching Signed-off-by: hargunsiingh <hsnarang45@gmail.com>
1 parent 2c68293 commit 85f6aad

3 files changed

Lines changed: 27 additions & 20 deletions

File tree

nexum_cli/src/main.rs

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,12 @@ fn main() -> anyhow::Result<()> {
9797
io::stdout().flush()?;
9898

9999
let mut input = String::new();
100-
io::stdin().read_line(&mut input)?;
100+
let bytes_read = io::stdin().read_line(&mut input)?;
101+
if bytes_read == 0 {
102+
// EOF reached (e.g., piped input exhausted)
103+
println!("{}", "Goodbye! 👋".green().bold());
104+
break;
105+
}
101106

102107
let input = input.trim();
103108

@@ -115,8 +120,8 @@ fn main() -> anyhow::Result<()> {
115120
continue;
116121
}
117122

118-
if let Some(natural_query) = input.strip_prefix("ASK ").or_else(|| input.strip_prefix("ask ")) {
119-
let natural_query = natural_query.trim();
123+
if input.len() >= 4 && input[..4].eq_ignore_ascii_case("ASK ") {
124+
let natural_query = input[4..].trim();
120125

121126
if let Some(ref translator) = nl_translator {
122127
let schema = get_schema_context(&catalog);
@@ -158,8 +163,8 @@ fn main() -> anyhow::Result<()> {
158163
}
159164

160165
// Handle EXPLAIN command
161-
if let Some(query_to_explain) = input.strip_prefix("EXPLAIN ").or_else(|| input.strip_prefix("explain ")) {
162-
let query_to_explain = query_to_explain.trim();
166+
if input.len() >= 8 && input[..8].eq_ignore_ascii_case("EXPLAIN ") {
167+
let query_to_explain = input[8..].trim();
163168

164169
if let Some(ref explainer) = query_explainer {
165170
println!();
@@ -442,7 +447,9 @@ fn print_result_json(result: &ExecutionResult) {
442447
})
443448
}
444449
};
445-
println!("{}", serde_json::to_string_pretty(&json).unwrap_or_else(|e| format!("{{\"error\": \"{}\"}}", e)));
450+
println!("{}", serde_json::to_string_pretty(&json).unwrap_or_else(|e| {
451+
serde_json::to_string(&serde_json::json!({ "error": e.to_string() })).unwrap_or_default()
452+
}));
446453
}
447454

448455
fn print_result_formatted(result: &ExecutionResult) {
@@ -524,13 +531,7 @@ fn print_result_formatted(result: &ExecutionResult) {
524531
}
525532

526533
fn format_value(value: &nexum_core::sql::types::Value) -> String {
527-
match value {
528-
nexum_core::sql::types::Value::Integer(i) => i.to_string(),
529-
nexum_core::sql::types::Value::Float(f) => f.to_string(),
530-
nexum_core::sql::types::Value::Text(t) => t.clone(),
531-
nexum_core::sql::types::Value::Boolean(b) => b.to_string(),
532-
nexum_core::sql::types::Value::Null => "NULL".to_string(),
533-
}
534+
value.to_string()
534535
}
535536

536537
fn value_to_json(value: &nexum_core::sql::types::Value) -> serde_json::Value {

nexum_core/src/executor/mod.rs

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -652,13 +652,7 @@ impl Executor {
652652
}
653653

654654
fn format_value(value: &Value) -> String {
655-
match value {
656-
Value::Integer(i) => i.to_string(),
657-
Value::Float(f) => f.to_string(),
658-
Value::Text(t) => t.clone(),
659-
Value::Boolean(b) => b.to_string(),
660-
Value::Null => "NULL".to_string(),
661-
}
655+
value.to_string()
662656
}
663657

664658
fn build_cache_key(

nexum_core/src/sql/types.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,18 @@ impl Value {
3131
}
3232
}
3333

34+
impl std::fmt::Display for Value {
35+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
36+
match self {
37+
Value::Integer(i) => write!(f, "{}", i),
38+
Value::Float(v) => write!(f, "{}", v),
39+
Value::Text(t) => write!(f, "{}", t),
40+
Value::Boolean(b) => write!(f, "{}", b),
41+
Value::Null => write!(f, "NULL"),
42+
}
43+
}
44+
}
45+
3446
#[derive(Debug, Clone)]
3547
pub struct Column {
3648
pub name: String,

0 commit comments

Comments
 (0)