feat(cli): add colorized output and improved formatting#101
Conversation
WalkthroughAdds five runtime dependencies and replaces the simple CLI with a clap-based REPL featuring colored output, progress spinners, JSON output option, ASK/EXPLAIN commands, SQL execution, formatted/tabled result rendering, and minor core changes to Value formatting. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as CLI (nexum_cli)
participant Storage as Storage/Engine
participant Executor as Executor/Catalog
participant NL as NLTranslator
participant Explainer as QueryExplainer
User->>CLI: Launch CLI
CLI->>CLI: print_banner()
CLI->>Storage: initialize engine
Storage-->>CLI: ready
CLI->>Executor: init executor & catalog
Executor-->>CLI: ready
CLI->>NL: load translator
NL-->>CLI: loaded
CLI->>Explainer: load explainer
Explainer-->>CLI: loaded
loop REPL
User->>CLI: command (ASK / EXPLAIN / SQL / HELP / EXIT)
alt ASK
CLI->>NL: translate question -> SQL
NL-->>CLI: SQL
CLI->>Executor: execute SQL
Executor-->>CLI: ExecutionResult
CLI-->>User: show SQL + results (JSON or table)
else EXPLAIN
CLI->>Explainer: explain SQL
Explainer-->>CLI: plan
CLI-->>User: show plan
else SQL
CLI->>Executor: execute SQL
Executor-->>CLI: ExecutionResult
CLI-->>User: show results
else HELP
CLI-->>User: display help
else EXIT
CLI-->>User: exit message
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🎉 Thanks for opening your first pull request in NexumDB!
We appreciate your contribution! Here's what happens next:
- ✅ Automated checks will run (CI, tests, linting)
- 👀 A maintainer will review your changes
- 💬 We may suggest some improvements or ask questions
- 🚀 Once approved, we'll merge your contribution!
Before we can merge:
- Make sure all CI checks pass
- Sign your commits with DCO (
git commit -s) - Address any review feedback
Check out our Contributing Guide for more details.
Thanks for making NexumDB better! 🙌
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nexum_cli/src/main.rs (1)
99-106:⚠️ Potential issue | 🔴 CriticalInfinite loop on EOF when stdin is piped or closed.
read_linereturnsOk(0)on EOF, leavinginputempty. Thecontinueon Line 104 causes an infinite busy-loop when the CLI is used non-interactively (e.g.,echo "SELECT 1" | nexum).🐛 Proposed fix
let mut input = String::new(); - io::stdin().read_line(&mut input)?; + let bytes_read = io::stdin().read_line(&mut input)?; + if bytes_read == 0 { + // EOF reached + println!("{}", "Goodbye! 👋".green().bold()); + break; + } let input = input.trim();
🤖 Fix all issues with AI agents
In `@nexum_cli/src/main.rs`:
- Around line 52-61: The startup currently hardcodes the data path when
constructing StorageEngine in main; add a --data-dir field to the Args struct
and parse it in Args::parse(), use that value (falling back to the current
"./nexumdb_data" default) when calling StorageEngine::new(...) and when passing
storage into Executor::new(...) and Catalog::new(...). Update any related
docs/usage text and ensure the new Args field name (e.g., data_dir) is used to
build the spinner message created by create_spinner(...) so the chosen path is
visible on startup.
- Around line 118-119: Replace fragile byte-index slicing (input[4..] and
input[8..]) with the safe get(...) accessor and handle the Option: e.g. in the
ASK branch use input.get(4..).map(str::trim).unwrap_or("") to produce
natural_query, and do the same for the EXPLAIN branch with get(8..) (trim and
unwrap_or). Update the code paths that currently use input[4..] and input[8..]
(the ASK/EXPLAIN handling in main.rs) so they no longer panic on non-ASCII
input.
- Around line 382-401: The JSON builder for ExecutionResult::Selected currently
skips inserting keys when a row has fewer values than columns, producing
inconsistent objects; update the mapping in the Selected arm (where columns,
rows, and value_to_json are used) to insert serde_json::Value::Null for any
missing value (i.e., when i >= row.values.len()) so every column key is present
for each row and consider adding an optional debug/warn log mentioning the row
index and column name to surface mismatches.
- Around line 535-536: The match arm handling
nexum_core::sql::types::Value::Float(f) currently uses serde_json::json!(f),
which panics for NaN/Infinity; replace that conversion by calling
serde_json::Value::from_f64(f) and handle the Option result—when from_f64
returns Some(value) use that JSON value, and when it returns None (NaN/Infinity)
return a safe fallback such as serde_json::Value::Null (or a string/explicit
sentinel) so the code no longer panics at runtime.
- Line 442: The call to serde_json::to_string_pretty(&json).unwrap() can panic
on NaN/Infinity floats (e.g., if ExecutionResult contains such values); replace
the direct unwrap in main.rs by handling the Result from
serde_json::to_string_pretty (referencing serde_json::to_string_pretty and the
ExecutionResult/json variable) — either use unwrap_or_else to convert the Err
into a safe fallback string and log the error, or match on the Result and print
a clear error message (or sanitized/fallback JSON) instead of panicking.
- Around line 12-50: The version string is duplicated between the Args command
attribute and the print_banner function; replace the hard-coded "0.4.0" by a
single source of truth (e.g., define const VERSION: &str =
env!("CARGO_PKG_VERSION") or use env!("CARGO_PKG_VERSION") directly) and
reference that constant/variable in the #[command(version = ...)] on struct Args
and inside print_banner so both locations use the same value.
- Around line 199-219: In print_banner(), the ASCII art raw string literal
contains a double backslash sequence ("\_\\__") that renders as two backslashes
and misaligns the banner; edit the raw string inside print_banner() to replace
the double backslash with a single backslash sequence ("\_\__") so the art uses
one backslash consistently and restores correct alignment.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@nexum_cli/src/main.rs`:
- Around line 526-534: The CLI duplicates the private helper format_value
(identical to the one in nexum_core::executor); remove the copied function from
nexum_cli/src/main.rs and instead expose a single implementation from the core
crate: either make the existing format_value in nexum_core::executor public (pub
fn format_value) and import and call nexum_core::executor::format_value from
main, or implement std::fmt::Display for nexum_core::sql::types::Value in the
core crate and update main to use value.to_string() / format!("{}", value);
update usages in main (and executor if you choose Display) to reference the
shared symbol (format_value or Display) and delete the duplicated function in
main.rs.
- Line 445: The fallback in the
serde_json::to_string_pretty(...).unwrap_or_else(|e| format!(...)) closure
builds JSON by interpolating the error string unescaped, producing invalid JSON
if e contains quotes/backslashes; change the unwrap_or_else to return a properly
escaped JSON value by serializing a small structure or map (e.g., a HashMap or
serde_json::json! object) containing the error under an "error" key and then
calling serde_json::to_string (or to_string_pretty) on that value instead of
using format!, updating the closure that captures e from
serde_json::to_string_pretty to perform this safe serialization.
- Line 118: The ASK/EXPLAIN matching is currently case-sensitive because it uses
input.strip_prefix("ASK ") / strip_prefix("ask "), causing mixed-case inputs to
fall through; change the logic to perform a case-insensitive prefix check (e.g.,
use input.to_ascii_uppercase().starts_with("ASK ") / "EXPLAIN ") and then obtain
the original-case remainder by slicing the original input with the matched
prefix length (so keep using the original `input` string for the returned
`natural_query`/explain text). Replace uses of strip_prefix("ASK ") /
strip_prefix("ask ") and the analogous EXPLAIN handling with this
case-insensitive check + slicing so "Ask", "aSk", etc. are accepted.
| fn format_value(value: &nexum_core::sql::types::Value) -> String { | ||
| match value { | ||
| nexum_core::sql::types::Value::Integer(i) => i.to_string(), | ||
| nexum_core::sql::types::Value::Float(f) => f.to_string(), | ||
| nexum_core::sql::types::Value::Text(t) => t.clone(), | ||
| nexum_core::sql::types::Value::Boolean(b) => b.to_string(), | ||
| nexum_core::sql::types::Value::Null => "NULL".to_string(), | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Duplicated logic: format_value exists identically in nexum_core::executor (lines 653–661).
This is a direct copy of the private format_value in the core crate. Consider either making the core version pub or implementing std::fmt::Display for nexum_core::sql::types::Value so both the CLI and executor can share it without duplication.
🤖 Prompt for AI Agents
In `@nexum_cli/src/main.rs` around lines 526 - 534, The CLI duplicates the private
helper format_value (identical to the one in nexum_core::executor); remove the
copied function from nexum_cli/src/main.rs and instead expose a single
implementation from the core crate: either make the existing format_value in
nexum_core::executor public (pub fn format_value) and import and call
nexum_core::executor::format_value from main, or implement std::fmt::Display for
nexum_core::sql::types::Value in the core crate and update main to use
value.to_string() / format!("{}", value); update usages in main (and executor if
you choose Display) to reference the shared symbol (format_value or Display) and
delete the duplicated function in main.rs.
aviralgarg05
left a comment
There was a problem hiding this comment.
Pls fix the failing tests
2c68293 to
119c7d2
Compare
119c7d2 to
85f6aad
Compare
- Add colored output for different message types (errors red, success green, warnings yellow) - Improve table formatting for SELECT results using comfy-table - Add progress indicators for long operations using indicatif - Add --json output format option via clap - Improve help messages with ASCII banner and organized command reference Closes aviralgarg05#92
- Use env!(CARGO_PKG_VERSION) instead of hardcoded version string - Fix double backslash in ASCII art banner - Handle NaN/Infinity floats safely in JSON output - Use unwrap_or_else for JSON serialization to prevent panics - Insert null for missing column values in JSON output - Use strip_prefix for safer string slicing (avoids byte offset issues)
- 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>
85f6aad to
19e86ed
Compare
Description
Add colorized output and better formatting to the CLI as requested in #92.
Changes Made
Files Modified
exum_cli/Cargo.toml\ - Added 5 new dependencies
exum_cli/src/main.rs\ - Complete rewrite (~480 lines)
Testing
Closes #92
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Documentation