From 6f693fdb1ee7d4d14dcbf3fbb947a77d39f01980 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 21:43:54 +0000 Subject: [PATCH 1/4] test: pin `--format` and the table rendering (RED) #989 asks for a table when rows are read with eyes and JSON when they are piped to `jq`, and leaves open whether the user can choose. These tests answer that with a `--format {auto,table,json}` flag -- the issue's own preferred option, and the one that stays in flag-space rather than reopening the dot-command question #953 closed. Nine fail today; three pass as regression guards. The guards are the point of the shape: `dirsql "" | jq` must keep printing exactly the bytes it prints now, and the server must keep rejecting a rendering flag its transport cannot honour. Every case forces the format or runs through a pipe, so none of it needs a terminal. Refs #989 skip-changelog: test-only commit; the fragment lands with the implementation Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GU6WPGs4bbkad8QhQfyXDD --- packages/rust/tests/repl_format_e2e.rs | 257 +++++++++++++++++++++++++ 1 file changed, 257 insertions(+) create mode 100644 packages/rust/tests/repl_format_e2e.rs diff --git a/packages/rust/tests/repl_format_e2e.rs b/packages/rust/tests/repl_format_e2e.rs new file mode 100644 index 000000000..de456c724 --- /dev/null +++ b/packages/rust/tests/repl_format_e2e.rs @@ -0,0 +1,257 @@ +//! End-to-end tests for how result rows are rendered (#989). +//! +//! These spawn the real compiled `dirsql` binary over a fixture directory and +//! assert what reaches stdout. Nothing is mocked (real process, real +//! filesystem, real SQLite). +//! +//! The contract under test: a JSON array is right for `dirsql query "…" | jq` +//! and wrong for a human reading a REPL, so the rendering follows the +//! **destination**. `--format` decides, defaulting to `auto`, which reads +//! stdout: a table when it is a terminal, JSON when it is redirected. The +//! flag is valid in one-shot mode and the REPL alike; `auto` in a pipe must +//! stay byte-identical to what `dirsql query` printed before this existed, +//! which is the regression that matters most. +//! +//! Every case here forces the format explicitly or runs through a pipe, so +//! none of it needs a terminal. +//! +//! Gated behind `--features cli`: the `dirsql` bin target is +//! `required-features = ["cli"]`. + +#![cfg(feature = "cli")] + +use std::io::Write; +use std::process::{Output, Stdio}; + +use assert_cmd::prelude::*; +use tempfile::TempDir; + +/// Two files with known, distinct sizes, so a rendering can be pinned exactly. +fn fixture() -> TempDir { + let root = TempDir::new().unwrap(); + std::fs::write(root.path().join("a.md"), "alpha\n").unwrap(); + std::fs::write(root.path().join("bb.md"), "beta beta\n").unwrap(); + root +} + +/// Run a one-shot query with the given extra argv. +fn query(root: &TempDir, sql: &str, args: &[&str]) -> Output { + std::process::Command::cargo_bin("dirsql") + .expect("binary must exist") + .arg(sql) + .args(args) + .current_dir(root.path()) + .output() + .expect("spawning `dirsql` failed") +} + +/// Run the REPL over a pipe with the given extra argv. +fn repl(root: &TempDir, stdin: &str, args: &[&str]) -> Output { + let mut child = std::process::Command::cargo_bin("dirsql") + .expect("binary must exist") + .args(args) + .current_dir(root.path()) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawning `dirsql` failed"); + child + .stdin + .take() + .expect("stdin was piped") + .write_all(stdin.as_bytes()) + .expect("writing to the REPL's stdin failed"); + child + .wait_with_output() + .expect("waiting on `dirsql` failed") +} + +fn stdout_of(out: &Output) -> String { + String::from_utf8_lossy(&out.stdout).into_owned() +} + +const SQL: &str = "SELECT basename, size FROM './' ORDER BY basename"; + +#[test] +fn a_piped_one_shot_query_still_prints_the_json_array() { + // The regression that matters most: `dirsql "…" | jq` must not change. + let root = fixture(); + + let out = query(&root, SQL, &[]); + + assert_eq!( + stdout_of(&out).trim(), + r#"[{"basename":"a.md","size":6},{"basename":"bb.md","size":10}]"#, + "{out:?}" + ); +} + +#[test] +fn auto_in_a_pipe_is_byte_identical_to_explicit_json() { + // `auto` reads the destination; a pipe is not a terminal, so the two must + // produce the same bytes rather than merely similar ones. + let root = fixture(); + + let inferred = query(&root, SQL, &[]); + let explicit = query(&root, SQL, &["--format", "json"]); + + assert_eq!( + inferred.stdout, explicit.stdout, + "{inferred:?} {explicit:?}" + ); +} + +#[test] +fn the_table_format_can_be_asked_for_in_a_pipe() { + // The point of a flag rather than TTY-only inference: the user decides, + // in both directions. + let root = fixture(); + + let out = query(&root, SQL, &["--format", "table"]); + let stdout = stdout_of(&out); + + assert!(stdout.contains("basename"), "a header row, got {stdout:?}"); + assert!(stdout.contains("a.md"), "got {stdout:?}"); + assert!(stdout.contains("bb.md"), "got {stdout:?}"); + assert!( + !stdout.contains(r#"{"basename""#), + "no JSON in table mode, got {stdout:?}" + ); +} + +#[test] +fn the_table_aligns_its_columns() { + // A table whose columns do not line up is just noisier JSON. Both value + // rows must place their second column at the same offset. + let root = fixture(); + + let out = query(&root, SQL, &["--format", "table"]); + let stdout = stdout_of(&out); + let offsets: Vec> = stdout + .lines() + .filter(|line| line.contains("a.md") || line.contains("bb.md")) + .map(|line| line.rfind("6").or_else(|| line.rfind("10"))) + .collect(); + + assert_eq!(offsets.len(), 2, "two value rows, got {stdout:?}"); + assert_eq!(offsets[0], offsets[1], "columns must align, got {stdout:?}"); +} + +#[test] +fn the_table_reports_how_many_rows_there_were() { + // Reading rows with eyes means wanting the count without counting. + let root = fixture(); + + let out = query(&root, SQL, &["--format", "table"]); + + assert!( + stdout_of(&out).contains("2 rows"), + "got {}", + stdout_of(&out) + ); +} + +#[test] +fn an_empty_table_says_so_rather_than_printing_nothing() { + let root = fixture(); + + let out = query( + &root, + "SELECT basename FROM './' WHERE 0", + &["--format", "table"], + ); + + assert!( + stdout_of(&out).contains("no rows"), + "got {}", + stdout_of(&out) + ); +} + +#[test] +fn a_null_is_named_rather_than_rendered_as_a_blank() { + // A blank cell is indistinguishable from an empty string. + let root = fixture(); + + let out = query(&root, "SELECT NULL AS nothing", &["--format", "table"]); + + assert!(stdout_of(&out).contains("NULL"), "got {}", stdout_of(&out)); +} + +#[test] +fn the_json_format_can_be_asked_for_explicitly() { + let root = fixture(); + + let out = query(&root, SQL, &["--format", "json"]); + + assert_eq!( + stdout_of(&out).trim(), + r#"[{"basename":"a.md","size":6},{"basename":"bb.md","size":10}]"#, + "{out:?}" + ); +} + +#[test] +fn the_repl_takes_the_same_flag() { + // `--format` is valid wherever rows are printed, not just in one-shot + // mode -- the REPL is the surface that motivated it. + let root = fixture(); + + let out = repl( + &root, + "SELECT basename FROM './' ORDER BY basename\n", + &["--format", "table"], + ); + let stdout = stdout_of(&out); + + assert!(stdout.contains("basename"), "a header row, got {stdout:?}"); + assert!( + !stdout.contains(r#"{"basename""#), + "no JSON in table mode, got {stdout:?}" + ); +} + +#[test] +fn the_piped_repl_defaults_to_json() { + // `auto` keys on stdout, and the REPL's stdout here is a pipe. + let root = fixture(); + + let out = repl(&root, "SELECT basename FROM './' ORDER BY basename\n", &[]); + + assert_eq!( + stdout_of(&out).trim(), + r#"[{"basename":"a.md"},{"basename":"bb.md"}]"#, + "{out:?}" + ); +} + +#[test] +fn an_unknown_format_is_a_usage_error() { + // Silently falling back would hide a typo until someone read the output. + let root = fixture(); + + let out = query(&root, SQL, &["--format", "yaml"]); + + assert_eq!(out.status.code(), Some(2), "{out:?}"); + assert!( + String::from_utf8_lossy(&out.stderr).contains("yaml"), + "the error names the value, got {out:?}" + ); +} + +#[test] +fn the_server_does_not_take_the_flag() { + // The server speaks JSON over HTTP; a rendering flag there would be a + // promise the transport cannot keep. + let root = fixture(); + + let out = std::process::Command::cargo_bin("dirsql") + .expect("binary must exist") + .args(["server", "--format", "table"]) + .current_dir(root.path()) + .output() + .expect("spawning `dirsql` failed"); + + assert_eq!(out.status.code(), Some(2), "{out:?}"); +} From 060576f12a666b12e3256dccfbc17257870ae52e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 22:19:05 +0000 Subject: [PATCH 2/4] feat(cli): render rows as a table when they are read rather than piped A JSON array is right for `dirsql "" | jq` and wrong for a person: `SELECT * FROM './'` over a few thousand files puts a few thousand JSON objects on one line. Rows now follow their destination -- a table when stdout is a terminal, the same JSON array when it is not. #989 left open whether the user gets a say. They do, through `--format {auto,table,json}`, which is the issue's own preferred option and the one that stays in flag-space: `.mode` would mean reintroducing the dot-command namespace #953 closed, and a flag serves `dirsql query` too. `auto` is the default and reads **stdout**, not stdin -- `dirsql > rows.json` typed at a terminal is still headed for a file, and the file wants JSON. `server` does not take the flag; its transport is JSON over HTTP and could not honour it. Piped output is unchanged byte for byte, which is the regression that mattered most here and is asserted directly rather than by eye. The renderer is deliberately modest, in `sqlite3`'s `.mode column` style: aligned columns, a rule under the header, a row count, `NULL` spelled out so it cannot be read as an empty string, and every column any row carries so a ragged result drops nothing. Two things happen to a value on its way into a cell, and a `content` column is the reason for both. Its newlines are escaped, because the manual run showed a single unescaped one collapsing the whole layout -- one row spans as many lines as its longest value and no column lines up with any other. And it is truncated past 60 characters, because a cell holding a whole file otherwise sets the width of every row. Tabs, carriage returns and other control characters are escaped alongside the newlines; a bare `\r` would rewind the cursor over a row already drawn. `--format json` returns values unaltered. Terminal-width-aware layout and paging are out of scope, said so in the docs rather than silently omitted. Slice 2's pty cases now ask for `--format json`: on a terminal they would otherwise read rows through a table layout, making every one of them a rendering test as well as a termination test. One new case covers the default they gave up. Fixes #989 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GU6WPGs4bbkad8QhQfyXDD --- docs/reference/cli.md | 63 ++- .../changelog.d/2026-08-20-format-flag.md | 3 + .../migrations.d/2026-08-20-format-flag.md | 63 +++ packages/rust/src/cli/mod.rs | 2 + packages/rust/src/cli/repl.rs | 68 ++- packages/rust/src/cli/run.rs | 142 ++++++- packages/rust/src/cli/table.rs | 392 ++++++++++++++++++ packages/rust/tests/repl_format_e2e.rs | 2 +- packages/rust/tests/repl_tty_e2e.rs | 43 +- 9 files changed, 763 insertions(+), 15 deletions(-) create mode 100644 packages/rust/changelog.d/2026-08-20-format-flag.md create mode 100644 packages/rust/migrations.d/2026-08-20-format-flag.md create mode 100644 packages/rust/src/cli/table.rs diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 811980838..d6676bbbe 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -61,7 +61,11 @@ dirsql # `exit`, `quit`, or Ctrl-D to leave. # # dirsql> SELECT count(*) AS files FROM './'; -# [{"files":128}] +# files +# ----- +# 128 +# +# 1 row # dirsql> ``` @@ -79,6 +83,51 @@ rather than re-walking the directory each time, and the live watcher keeps it fresh between them. Files the scan had to skip are named on stderr once, up front. +### Output format + +Rows go where they are useful: a **table** when stdout is a terminal, the +**JSON array** when it is piped or redirected. `SELECT * FROM './'` in a +5000-file tree should not put a 5000-element JSON array in front of a person, +and `dirsql "…" | jq` should not have to parse a table. + +`--format` overrides that, in both directions, and is valid in the REPL and in +[`dirsql query`](#dirsql-query) alike: + +| Value | Renders | +|---|---| +| `auto` (default) | Table if stdout is a terminal, JSON otherwise. | +| `table` | Always a table — including into a pipe or a file. | +| `json` | Always the JSON array — including at a terminal. | + +```bash +dirsql "SELECT basename, size FROM './' ORDER BY basename" --format table +# basename size +# -------- ---- +# a.md 6 +# bb.md 10 +# +# 2 rows +``` + +There is no `.mode`: dirsql has no dot-commands to extend (see +[Leaving](#leaving)), and a flag serves the one-shot query too. `dirsql server` +does not take `--format` — its transport is JSON over HTTP. + +**`auto` keys on stdout, not stdin.** `dirsql > rows.json` typed at a terminal +is still headed for a file, and the file gets JSON. + +Table rendering is deliberately plain: aligned columns, a rule under the +header, a row count, and `NULL` spelled out so it cannot be confused with an +empty string. Two things happen to a value on its way into a cell, both +because a `content` column holds a whole file: **newlines, tabs and other +control characters are escaped** (`\n`, `\t`, `\u{…}`) so one row cannot span +several lines, and **anything longer than 60 characters is truncated with +`…`** so one column cannot set the width of every row. `--format json` +returns the values unaltered. + +Laying the table out to the terminal's width, and paging a long result, are +both out of scope; pipe to `less` for the latter. + ### Where a statement ends At its semicolon — the same rule `sqlite3` uses, and **SQLite's own tokenizer** @@ -157,7 +206,11 @@ failure: dirsql> SELECT nope FROM missing; dirsql: SQLite error: no such table: missing dirsql> SELECT 1 AS n; -[{"n":1}] +n +- +1 + +1 row ``` A config that cannot be loaded is different in kind: it fails identically for @@ -331,6 +384,12 @@ Errors print the same diagnostic the HTTP `{"error": …}` body carries — config failures, SQL errors, rejected reads, hook failures, timeouts — to stderr, with exit code `1`. +#### `--format {auto,table,json}` + +How to render the result rows — the same flag [the REPL](#output-format) +takes, with the same `auto` default. A one-shot query is usually piped, so +`auto` usually means JSON; `--format table` is there for the times it is not. + ### Exit codes | Code | Meaning | diff --git a/packages/rust/changelog.d/2026-08-20-format-flag.md b/packages/rust/changelog.d/2026-08-20-format-flag.md new file mode 100644 index 000000000..c9314cd97 --- /dev/null +++ b/packages/rust/changelog.d/2026-08-20-format-flag.md @@ -0,0 +1,3 @@ +**Added** + +- **`--format {auto,table,json}` decides how result rows are rendered**, in the REPL and `dirsql query` alike. `auto` (the default) reads the **destination**: a table when stdout is a terminal, the JSON array when it is piped or redirected — so `SELECT * FROM './'` in a large tree lays itself out to be read instead of putting a few thousand JSON objects in front of a person, while `dirsql "…" | jq` keeps receiving byte-identical JSON. It keys on stdout, not stdin, so `dirsql > rows.json` typed at a terminal still writes JSON. `--format table` and `--format json` override in either direction, including into a pipe. There is no `.mode`: dirsql has no dot-commands to extend, and a flag serves the one-shot query too; `dirsql server` does not accept it, since its transport is JSON over HTTP. Tables are aligned columns with a rule under the header, a row count, `NULL` spelled out so it cannot be read as an empty string, control characters escaped (`\n`, `\t`, `\u{…}`) so one row cannot span several lines, and cells truncated past 60 characters so one `content` column cannot set the width of every row. `--format json` returns values unaltered. (#989) diff --git a/packages/rust/migrations.d/2026-08-20-format-flag.md b/packages/rust/migrations.d/2026-08-20-format-flag.md new file mode 100644 index 000000000..2b7d03244 --- /dev/null +++ b/packages/rust/migrations.d/2026-08-20-format-flag.md @@ -0,0 +1,63 @@ +### Result rows render as a table at a terminal (#989) + +#### Summary + +Every surface printed a JSON array. Rows now follow their **destination**: a +table when stdout is a terminal, the same JSON array when it is piped or +redirected. A new `--format {auto,table,json}` overrides that in either +direction, defaulting to `auto`. + +**Piped output is unchanged, byte for byte.** `dirsql "" | jq` and +`dirsql "" > rows.json` produce exactly what they produced before. + +#### Required changes + +| Before | After | Fix | +| ------ | ----- | --- | +| `dirsql ""` at a terminal printed JSON you could copy | Prints a table | `dirsql "" --format json`, or redirect it — `auto` already picks JSON for any non-terminal destination. | +| A script capturing output **through a pty** (`script`, `expect`, a CI terminal emulator) parsed JSON | Sees a table | Pass `--format json` explicitly. A pty is a terminal, so `auto` cannot tell it from a human. | +| `dirsql server --format …` | Not applicable | The flag was never accepted there and still is not: the server speaks JSON over HTTP. | + +#### Deprecations removed + +_None._ + +#### Behavior changes without code changes + +- Rows printed to a terminal are a table rather than a JSON array. Nothing + changes for a pipe, a file, or any other non-terminal destination. +- The decision keys on **stdout**, not stdin. `dirsql > rows.json` typed + interactively writes JSON; `dirsql < script.sql` with stdout on a terminal + writes a table. +- Table cells are altered to fit a grid: newlines, tabs and other control + characters are escaped (`\n`, `\t`, `\u{…}`), and anything longer than 60 + characters is truncated with `…`. A `content` column is the reason for both; + use `--format json` when you need the value unaltered. +- `NULL` is spelled out in a table, so it is distinguishable from an empty + string. JSON is unaffected — it still carries `null`. + +#### Verification + +```bash +# A pipe is not a terminal, so nothing changes there: +dirsql "SELECT basename FROM './'" | jq -r '.[].basename' +# a.md +# bb.md + +# The same query at a terminal: +dirsql "SELECT basename, size FROM './' ORDER BY basename" --format table +# basename size +# -------- ---- +# a.md 6 +# bb.md 10 +# +# 2 rows + +# Forcing JSON at a terminal: +dirsql "SELECT basename FROM './'" --format json +# [{"basename":"a.md"},{"basename":"bb.md"}] + +# An unknown value is a usage error rather than a silent fallback: +dirsql "SELECT 1" --format yaml; echo $? +# 2 +``` diff --git a/packages/rust/src/cli/mod.rs b/packages/rust/src/cli/mod.rs index a1e465973..1ba36f585 100644 --- a/packages/rust/src/cli/mod.rs +++ b/packages/rust/src/cli/mod.rs @@ -19,6 +19,7 @@ //! - [`execute`] — the transport-agnostic query pipeline shared by the //! HTTP handler and the one-shot `dirsql query` subcommand. //! - [`repl`] — the read-eval-print loop behind bare `dirsql`. +//! - [`table`] — result rows rendered for reading rather than piping. //! - [`serialize`] — row + event → JSON. use std::net::SocketAddr; @@ -37,6 +38,7 @@ pub mod router; pub mod run; pub mod serialize; pub mod server; +pub mod table; pub use run::run_cli; pub use server::{serve, serve_with_state}; diff --git a/packages/rust/src/cli/repl.rs b/packages/rust/src/cli/repl.rs index 41fe86b5c..e012db19f 100644 --- a/packages/rust/src/cli/repl.rs +++ b/packages/rust/src/cli/repl.rs @@ -38,7 +38,7 @@ use serde_json::Value; use super::AppState; use super::execute::{QueryFailure, execute_query}; -use super::run::query_body; +use super::run::{Format, query_body, render_rows}; /// The interactive prompt, split as reedline renders it: the left half, then /// the indicator that follows it. @@ -97,6 +97,7 @@ trait Lines { /// line for every statement the user types. pub(super) async fn run_repl( state: &AppState, + format: Format, input: R, out: &mut W, err: &mut E, @@ -114,9 +115,17 @@ where let statements = StateStatements(state); if interactive { - repl_loop(&statements, EditorLines::new(), out, err, true).await + repl_loop(&statements, EditorLines::new(), out, err, format, true).await } else { - repl_loop(&statements, PipedLines(Some(input)), out, err, false).await + repl_loop( + &statements, + PipedLines(Some(input)), + out, + err, + format, + false, + ) + .await } } @@ -127,6 +136,7 @@ async fn repl_loop( mut lines: L, out: &mut W, err: &mut E, + format: Format, interactive: bool, ) -> u8 where @@ -166,7 +176,7 @@ where // A closed stdout (`dirsql < script.sql | head -1`) means // nothing downstream is listening; keep reading and there is // a whole script left to execute into a broken pipe. - if writeln!(out, "{value}").is_err() { + if write!(out, "{}", render_rows(&value, format)).is_err() { return 0; } } @@ -524,7 +534,15 @@ mod tests { async fn drive(entries: Vec) -> (FakeStatements, String, String, u8) { let fake = FakeStatements::default(); let (mut out, mut err) = (Vec::new(), Vec::new()); - let code = repl_loop(&fake, FakeLines::of(entries), &mut out, &mut err, false).await; + let code = repl_loop( + &fake, + FakeLines::of(entries), + &mut out, + &mut err, + Format::Json, + false, + ) + .await; (fake, text(&out), text(&err), code) } @@ -538,6 +556,7 @@ mod tests { FakeLines::statements(&["SELECT 1", "SELECT 2"]), &mut out, &mut err, + Format::Json, false, ) .await; @@ -557,6 +576,7 @@ mod tests { FakeLines::statements(&[" SELECT 1 \n"]), &mut out, &mut err, + Format::Json, false, ) .await; @@ -633,6 +653,7 @@ mod tests { FakeLines::statements(&["SELECT nope", "SELECT 1"]), &mut out, &mut err, + Format::Json, false, ) .await; @@ -657,7 +678,15 @@ mod tests { let fake = FakeStatements::default(); let (mut out, mut err) = (Vec::new(), Vec::new()); - repl_loop(&fake, FakeLines::of(vec![]), &mut out, &mut err, true).await; + repl_loop( + &fake, + FakeLines::of(vec![]), + &mut out, + &mut err, + Format::Json, + true, + ) + .await; assert!(text(&out).ends_with("\n\n\n"), "{:?}", text(&out)); } @@ -667,7 +696,15 @@ mod tests { let fake = FakeStatements::default(); let (mut out, mut err) = (Vec::new(), Vec::new()); - repl_loop(&fake, FakeLines::of(vec![]), &mut out, &mut err, true).await; + repl_loop( + &fake, + FakeLines::of(vec![]), + &mut out, + &mut err, + Format::Json, + true, + ) + .await; assert!(text(&out).starts_with(&banner()), "{:?}", text(&out)); } @@ -682,6 +719,7 @@ mod tests { FakeLines::statements(&["SELECT 1"]), &mut BrokenPipe, &mut err, + Format::Json, true, ) .await; @@ -703,6 +741,7 @@ mod tests { FakeLines::statements(&["SELECT 1"]), &mut FlushFails, &mut err, + Format::Json, true, ) .await; @@ -716,7 +755,15 @@ mod tests { let fake = FakeStatements::default(); let (mut out, mut err) = (Vec::new(), Vec::new()); - let code = repl_loop(&fake, FakeLines::failing(), &mut out, &mut err, false).await; + let code = repl_loop( + &fake, + FakeLines::failing(), + &mut out, + &mut err, + Format::Json, + false, + ) + .await; assert_eq!(code, 1); assert_eq!( @@ -737,6 +784,7 @@ mod tests { FakeLines::statements(&["SELECT 1", "SELECT 2"]), &mut BrokenPipe, &mut err, + Format::Json, false, ) .await; @@ -757,6 +805,7 @@ mod tests { PipedLines(Some(input("SELECT 1\nSELECT 2\n"))), &mut out, &mut err, + Format::Json, false, ) .await; @@ -775,6 +824,7 @@ mod tests { PipedLines(Some(input("SELECT 1"))), &mut out, &mut err, + Format::Json, false, ) .await; @@ -792,6 +842,7 @@ mod tests { PipedLines(Some(BrokenReader)), &mut out, &mut err, + Format::Json, false, ) .await; @@ -821,6 +872,7 @@ mod tests { let code = run_repl( &state, + Format::Json, input("SELECT 1\nSELECT 2\n"), &mut out, &mut err, diff --git a/packages/rust/src/cli/run.rs b/packages/rust/src/cli/run.rs index 6adc96c2d..8fc1b2dd7 100644 --- a/packages/rust/src/cli/run.rs +++ b/packages/rust/src/cli/run.rs @@ -19,10 +19,11 @@ use std::path::PathBuf; use super::{ AppState, ServerConfig, execute::execute_query, init::InitOptions, repl::run_repl, - serve_with_state, + serve_with_state, table, }; use crate::{DirSQL, Extension, Row, Table}; -use clap::{Args, Parser, Subcommand}; +use clap::{Args, Parser, Subcommand, ValueEnum}; +use serde_json::Value; #[derive(Debug, Parser)] #[command( @@ -64,6 +65,11 @@ struct Cli { #[arg(long = "on-file")] on_file: Vec, + /// How to render result rows — the same `--format` the `query` subcommand + /// takes (see [`Format`]). Applies to the REPL too. + #[arg(long, value_enum, default_value_t = Format::Auto)] + format: Format, + /// Config flags for the default (query) mode. They are subcommand-local, /// not global: for the `query`/`server` subcommands the same flags are /// passed AFTER the subcommand (`dirsql query -c `); a config @@ -195,10 +201,53 @@ struct QueryArgs { #[arg(long = "on-file")] on_file: Vec, + /// How to render result rows. See [`Format`]. + #[arg(long, value_enum, default_value_t = Format::Auto)] + format: Format, + #[command(flatten)] common: ConfigArgs, } +/// How result rows are rendered. +/// +/// Deliberately a flag rather than a `.mode` meta-command: dirsql has no +/// dot-commands to extend (#953), and a flag serves the one-shot query as +/// well as the REPL. It is **not** on `server`, whose transport is JSON over +/// HTTP and cannot honour it. +#[derive(Debug, Clone, Copy, PartialEq, Eq, ValueEnum)] +pub(super) enum Format { + /// Pick by destination: a table when stdout is a terminal, JSON when it + /// is redirected or piped. + Auto, + /// An aligned table with a row count, for reading. + Table, + /// The JSON array every other surface returns, for `jq`. + Json, +} + +impl Format { + /// Settle `Auto` against the destination, yielding `Table` or `Json`. + /// + /// Keys on **stdout**, not stdin: `dirsql > rows.json` typed at a + /// terminal is still headed for a file, and the file wants JSON. + fn resolve(self, stdout_is_terminal: bool) -> Self { + match self { + Self::Auto if stdout_is_terminal => Self::Table, + Self::Auto => Self::Json, + settled => settled, + } + } +} + +/// Render one result the way `format` asks. `Auto` must already be settled. +pub(super) fn render_rows(value: &Value, format: Format) -> String { + match format { + Format::Table => table::render(value.as_array().map_or(&[][..], Vec::as_slice)), + _ => format!("{value}\n"), + } +} + /// Reduce the repeatable `--on-file` occurrences to at most one parser command. /// /// `clap` collects repeats into a `Vec` so the error can name config files @@ -300,6 +349,7 @@ async fn run_default(cli: Cli) -> u8 { run_query(QueryArgs { sql, on_file: cli.on_file, + format: cli.format, common: cli.common, }) .await @@ -319,6 +369,7 @@ async fn run_default(cli: Cli) -> u8 { report_scan_failures(&state); run_repl( &state, + cli.format.resolve(std::io::stdout().is_terminal()), // `StdinLock` is not `Send`, so it cannot cross into the // blocking read; `BufReader` locks per call and can. std::io::BufReader::new(std::io::stdin()), @@ -350,9 +401,11 @@ async fn run_query(args: QueryArgs) -> u8 { // Unbounded: the process IS the query, so `timeout(1)` expresses any cap // natively; only the long-lived server enforces `query_timeout` (408). + let format = args.format.resolve(std::io::stdout().is_terminal()); + match execute_query(&state, query_body(&args.sql), None).await { Ok(value) => { - println!("{value}"); + print!("{}", render_rows(&value, format)); // The query ran and its rows are on stdout, so this is not a // failure -- but the index behind them is missing files, and a // caller piping into `jq` under `set -e` has no other way to find @@ -704,6 +757,89 @@ mod tests { assert!(cli.sql.is_none()); } + /// One result row, built from `Value` alone so the test reaches for + /// nothing outside the module. + fn one_row() -> Value { + Value::Array(vec![Value::from_iter([("n".to_string(), Value::from(1))])]) + } + + #[test] + fn format_defaults_to_auto() { + assert_eq!(Cli::parse_from(["dirsql", "SELECT 1"]).format, Format::Auto); + } + + #[test] + fn format_parses_for_the_repl_and_the_default_query() { + assert_eq!( + Cli::parse_from(["dirsql", "--format", "table"]).format, + Format::Table + ); + assert_eq!( + Cli::parse_from(["dirsql", "SELECT 1", "--format", "json"]).format, + Format::Json + ); + } + + #[test] + fn format_parses_after_the_query_subcommand() { + match Cli::parse_from(["dirsql", "query", "SELECT 1", "--format", "table"]).command { + Some(Command::Query(args)) => assert_eq!(args.format, Format::Table), + other => panic!("expected a query subcommand, got {other:?}"), + } + } + + #[test] + fn the_server_takes_no_format_flag() { + // Its transport is JSON over HTTP; a rendering flag there would be a + // promise the transport cannot keep. + assert!(Cli::try_parse_from(["dirsql", "server", "--format", "table"]).is_err()); + } + + #[test] + fn an_unknown_format_is_rejected_rather_than_falling_back() { + // Silently defaulting would hide a typo until someone read the output. + assert!(Cli::try_parse_from(["dirsql", "SELECT 1", "--format", "yaml"]).is_err()); + } + + #[test] + fn auto_becomes_a_table_at_a_terminal() { + assert_eq!(Format::Auto.resolve(true), Format::Table); + } + + #[test] + fn auto_becomes_json_anywhere_else() { + assert_eq!(Format::Auto.resolve(false), Format::Json); + } + + #[test] + fn an_explicit_format_ignores_the_destination() { + // The point of the flag: a table into a pipe, JSON at a terminal. + assert_eq!(Format::Table.resolve(false), Format::Table); + assert_eq!(Format::Json.resolve(true), Format::Json); + } + + #[test] + fn json_rendering_is_the_value_and_a_newline() { + // Byte-identical to what the one-shot query printed before `--format` + // existed, which is the regression that matters most. + assert_eq!(render_rows(&one_row(), Format::Json), "[{\"n\":1}]\n"); + } + + #[test] + fn table_rendering_takes_the_other_route() { + let rendered = render_rows(&one_row(), Format::Table); + + assert!(rendered.contains("1 row"), "{rendered:?}"); + assert!(!rendered.contains('{'), "no JSON in a table, {rendered:?}"); + } + + #[test] + fn a_result_that_is_not_an_array_renders_as_an_empty_table() { + // The pipeline only ever returns an array; rendering must still not + // panic if that ever stops being true. + assert_eq!(render_rows(&Value::Null, Format::Table), "(no rows)\n"); + } + #[test] fn server_subcommand_parses_host_and_port() { // #662: the server moved behind `dirsql server`; `--host`/`--port` are diff --git a/packages/rust/src/cli/table.rs b/packages/rust/src/cli/table.rs new file mode 100644 index 000000000..06d7cd5d3 --- /dev/null +++ b/packages/rust/src/cli/table.rs @@ -0,0 +1,392 @@ +//! Rendering result rows as a table, for rows that are read rather than piped. +//! +//! The JSON array every other surface prints is right for `dirsql "…" | jq` +//! and wrong for a human: `SELECT * FROM './'` over a few thousand files puts +//! a few thousand JSON objects on one line. A table is the same rows laid out +//! to be read. +//! +//! Deliberately modest, in the `sqlite3` `.mode column` style: aligned +//! columns, a rule under the header, a row count. **Long cells are truncated +//! rather than wrapped**, which is what keeps one `content` column from +//! swallowing the table; laying the table out to the terminal's width, and +//! paging a long result, are both out of scope. + +use std::collections::BTreeSet; + +use serde_json::Value; + +/// Printed instead of an empty table. A query that matched nothing and a +/// query whose output was swallowed look identical otherwise. +const NO_ROWS: &str = "(no rows)"; + +/// How a SQL `NULL` is shown. Spelled out because a blank cell cannot be told +/// apart from an empty string. +const NULL: &str = "NULL"; + +/// Longest a cell may be before it is cut short. A `content` column holds +/// whole files; without a cap it decides the width of every row. +const MAX_CELL: usize = 60; + +/// Marks a cell the cap cut short. +const ELLIPSIS: char = '…'; + +/// Spaces between columns. +const GUTTER: &str = " "; + +/// Render `rows` -- the JSON array the shared pipeline produces -- as a table. +pub(super) fn render(rows: &[Value]) -> String { + let columns = columns(rows); + if columns.is_empty() { + return format!("{NO_ROWS}\n"); + } + + let grid: Vec> = rows.iter().map(|row| cells(row, &columns)).collect(); + let widths = widths(&columns, &grid); + + let mut out = String::new(); + out.push_str(&line(&columns, &widths)); + out.push_str(&rule(&widths)); + for row in &grid { + out.push_str(&line(row, &widths)); + } + out.push_str(&format!("\n{}\n", count(rows.len()))); + out +} + +/// Every column any row carries. Rows are JSON objects, so the keys arrive +/// sorted and stay in step with the JSON rendering of the same result. +fn columns(rows: &[Value]) -> Vec { + let mut names = BTreeSet::new(); + for row in rows { + if let Value::Object(map) = row { + names.extend(map.keys().cloned()); + } + } + names.into_iter().collect() +} + +/// One row's cells, in column order. A row missing a column renders it as +/// `NULL`, which is what a missing value is. +fn cells(row: &Value, columns: &[String]) -> Vec { + columns + .iter() + .map(|column| cell(row.get(column).unwrap_or(&Value::Null))) + .collect() +} + +/// One value as text: flattened onto a single line, then cut short if it +/// would dominate the table. +fn cell(value: &Value) -> String { + let text = match value { + Value::Null => NULL.to_string(), + Value::String(s) => s.clone(), + other => other.to_string(), + }; + truncate(&flatten(&text)) +} + +/// Escape the control characters that would break the layout. +/// +/// A `content` cell holds a whole file, newlines and all. Left as they are, +/// one row spans as many lines as its longest value and no column lines up +/// with any other -- the table stops being a table. `--format json` is the +/// way to get the bytes back unaltered. +fn flatten(text: &str) -> String { + let mut out = String::with_capacity(text.len()); + for ch in text.chars() { + match ch { + '\n' => out.push_str("\\n"), + '\r' => out.push_str("\\r"), + '\t' => out.push_str("\\t"), + other if other.is_control() => out.push_str(&format!("\\u{{{:x}}}", other as u32)), + other => out.push(other), + } + } + out +} + +/// Cut `text` to [`MAX_CELL`] characters, marking that something was dropped. +/// Counts characters rather than bytes so a multi-byte value is not split +/// mid-character. +fn truncate(text: &str) -> String { + let mut chars = text.chars(); + let head: String = chars.by_ref().take(MAX_CELL).collect(); + match chars.next() { + Some(_) => format!("{head}{ELLIPSIS}"), + None => head, + } +} + +/// How wide each column has to be to hold its header and every cell under it. +fn widths(columns: &[String], grid: &[Vec]) -> Vec { + columns + .iter() + .enumerate() + .map(|(index, column)| { + grid.iter() + .filter_map(|row| row.get(index)) + .map(|cell| cell.chars().count()) + .chain(std::iter::once(column.chars().count())) + .max() + .unwrap_or(0) + }) + .collect() +} + +/// One rendered row. The last column is not padded, so nothing carries +/// trailing whitespace. +fn line(cells: &[String], widths: &[usize]) -> String { + let padded: Vec = cells + .iter() + .zip(widths) + .enumerate() + .map(|(index, (cell, width))| match index == cells.len() - 1 { + true => cell.clone(), + false => pad(cell, *width), + }) + .collect(); + format!("{}\n", padded.join(GUTTER)) +} + +/// The rule under the header. +fn rule(widths: &[usize]) -> String { + let dashes: Vec = widths.iter().map(|width| "-".repeat(*width)).collect(); + format!("{}\n", dashes.join(GUTTER)) +} + +fn pad(cell: &str, width: usize) -> String { + let spaces = width.saturating_sub(cell.chars().count()); + format!("{cell}{}", " ".repeat(spaces)) +} + +/// The footer. Reading rows with eyes means wanting the count without +/// counting them. +fn count(rows: usize) -> String { + match rows { + 1 => "1 row".to_string(), + other => format!("{other} rows"), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Build a row object from its columns. + /// + /// `Value` builds an object from any iterator of pairs, which keeps this + /// to the module's own imports -- a unit test reaching for + /// `serde_json::json!` would be reaching outside the unit. + fn row(columns: [(&str, Value); N]) -> Value { + Value::from_iter(columns.map(|(name, value)| (name.to_string(), value))) + } + + fn rendered(rows: [Value; N]) -> String { + render(&rows) + } + + #[test] + fn an_empty_result_says_so() { + // Nothing at all is indistinguishable from output that went missing. + assert_eq!(render(&[]), "(no rows)\n"); + } + + #[test] + fn rows_with_no_columns_say_so_too() { + // A `SELECT` of nothing is still nothing to lay out. + assert_eq!(rendered([row([]), row([])]), "(no rows)\n"); + } + + #[test] + fn a_single_row_renders_header_rule_and_value() { + assert_eq!( + rendered([row([("n", Value::from(1))])]), + "n\n-\n1\n\n1 row\n", + "header, rule, value, then the count" + ); + } + + #[test] + fn columns_are_padded_to_their_widest_cell() { + let table = rendered([ + row([("basename", Value::from("a.md")), ("size", Value::from(6))]), + row([ + ("basename", Value::from("bb.md")), + ("size", Value::from(10)), + ]), + ]); + + assert_eq!( + table, + "basename size\n-------- ----\na.md 6\nbb.md 10\n\n2 rows\n" + ); + } + + #[test] + fn a_header_wider_than_its_values_sets_the_width() { + let table = rendered([row([("basename", Value::from("a"))])]); + + assert_eq!(table, "basename\n--------\na\n\n1 row\n"); + } + + #[test] + fn the_last_column_carries_no_trailing_padding() { + // Padding the final column leaves invisible whitespace on every line. + let table = rendered([ + row([("a", Value::from("xx")), ("b", Value::from("y"))]), + row([("a", Value::from("x")), ("b", Value::from("yy"))]), + ]); + + for line in table.lines() { + assert_eq!(line, line.trim_end(), "{line:?} has trailing whitespace"); + } + } + + #[test] + fn every_column_any_row_carries_is_shown() { + // A ragged result must not silently drop the columns only some rows + // have. + let table = rendered([row([("a", Value::from(1))]), row([("b", Value::from(2))])]); + + assert!(table.starts_with("a b\n"), "{table:?}"); + } + + #[test] + fn a_row_missing_a_column_renders_it_as_null() { + let table = rendered([ + row([("a", Value::from(1))]), + row([("a", Value::from(2)), ("b", Value::from(3))]), + ]); + + assert!(table.contains("1 NULL"), "{table:?}"); + } + + #[test] + fn a_null_is_named_rather_than_blank() { + // A blank cell cannot be told apart from an empty string. + let table = rendered([row([("a", Value::Null)])]); + + assert!(table.contains(NULL), "{table:?}"); + } + + #[test] + fn an_empty_string_stays_empty() { + // The other half of the same distinction. + let table = rendered([row([("a", Value::from(""))])]); + + assert!(!table.contains(NULL), "{table:?}"); + } + + #[test] + fn a_string_renders_unquoted() { + // JSON quotes are noise once the shape is a table. + let table = rendered([row([("a", Value::from("hello"))])]); + + assert!(table.contains("hello"), "{table:?}"); + assert!(!table.contains('"'), "{table:?}"); + } + + #[test] + fn numbers_and_booleans_render_as_themselves() { + let table = rendered([row([ + ("i", Value::from(42)), + ("f", Value::from(1.5)), + ("b", Value::from(true)), + ])]); + + assert!(table.contains("42"), "{table:?}"); + assert!(table.contains("1.5"), "{table:?}"); + assert!(table.contains("true"), "{table:?}"); + } + + #[test] + fn a_nested_value_falls_back_to_its_json() { + // Nothing in a result should render as a debug placeholder. + let table = rendered([row([("a", Value::from(vec![1, 2]))])]); + + assert!(table.contains("[1,2]"), "{table:?}"); + } + + #[test] + fn a_long_cell_is_cut_short() { + // One `content` column holding a whole file would otherwise set the + // width of every row. + let long = "x".repeat(MAX_CELL + 10); + let table = rendered([row([("content", Value::from(long))])]); + + assert!(table.contains(ELLIPSIS), "{table:?}"); + for line in table.lines() { + assert!( + line.chars().count() <= MAX_CELL + 1, + "{line:?} is wider than the cap" + ); + } + } + + #[test] + fn a_cell_exactly_at_the_cap_is_left_alone() { + // Off by one here marks an untouched value as truncated. + let exact = "x".repeat(MAX_CELL); + let table = rendered([row([("a", Value::from(exact))])]); + + assert!(!table.contains(ELLIPSIS), "{table:?}"); + } + + #[test] + fn truncation_counts_characters_not_bytes() { + // Cutting at a byte offset would split a multi-byte character and + // produce invalid UTF-8 in the middle of the table. + let wide = "é".repeat(MAX_CELL + 5); + + let cut = truncate(&wide); + + assert_eq!(cut.chars().count(), MAX_CELL + 1, "cap plus the ellipsis"); + } + + #[test] + fn a_newline_in_a_cell_is_escaped_rather_than_wrapping() { + // A `content` cell holds a whole file. Left alone, its newlines make + // one row span several lines and nothing lines up. + let table = rendered([row([("a", Value::from("one\ntwo")), ("b", Value::from(1))])]); + + assert_eq!( + table.lines().count(), + 5, + "header, rule, one row, blank, count: {table:?}" + ); + assert!(table.contains(r"one\ntwo"), "{table:?}"); + } + + #[test] + fn carriage_returns_and_tabs_are_escaped_too() { + // A bare `\r` would rewind the cursor to the start of the line and + // overwrite the row that was already drawn. + let table = rendered([row([("a", Value::from("x\ry\tz"))])]); + + assert!(table.contains(r"x\ry\tz"), "{table:?}"); + } + + #[test] + fn other_control_characters_are_escaped_by_code_point() { + // An escape byte in a value would otherwise be handed to the terminal + // as an instruction. + assert_eq!(flatten("a\u{1b}b"), r"a\u{1b}b"); + } + + #[test] + fn ordinary_text_passes_through_flattening_unchanged() { + assert_eq!(flatten("héllo wörld"), "héllo wörld"); + } + + #[test] + fn one_row_is_singular() { + // "1 rows" is the kind of detail that makes output look unfinished. + assert_eq!(count(1), "1 row"); + } + + #[test] + fn every_other_count_is_plural() { + assert_eq!(count(0), "0 rows"); + assert_eq!(count(2), "2 rows"); + } +} diff --git a/packages/rust/tests/repl_format_e2e.rs b/packages/rust/tests/repl_format_e2e.rs index de456c724..70a605c94 100644 --- a/packages/rust/tests/repl_format_e2e.rs +++ b/packages/rust/tests/repl_format_e2e.rs @@ -174,7 +174,7 @@ fn a_null_is_named_rather_than_rendered_as_a_blank() { // A blank cell is indistinguishable from an empty string. let root = fixture(); - let out = query(&root, "SELECT NULL AS nothing", &["--format", "table"]); + let out = query(&root, "SELECT NULL AS blank", &["--format", "table"]); assert!(stdout_of(&out).contains("NULL"), "got {}", stdout_of(&out)); } diff --git a/packages/rust/tests/repl_tty_e2e.rs b/packages/rust/tests/repl_tty_e2e.rs index ecc147e03..e35cc3118 100644 --- a/packages/rust/tests/repl_tty_e2e.rs +++ b/packages/rust/tests/repl_tty_e2e.rs @@ -66,13 +66,26 @@ impl Session { struct Terminal { home: TempDir, root: TempDir, + args: String, } impl Terminal { + /// A session that asks for JSON, so a case can assert on rows without + /// also asserting on a table layout. fn new() -> Self { + Self::with_args("--format json") + } + + /// A session with no `--format`, so the default rendering applies. + fn default_format() -> Self { + Self::with_args("") + } + + fn with_args(args: &str) -> Self { Self { home: TempDir::new().unwrap(), root: TempDir::new().unwrap(), + args: args.to_string(), } } @@ -83,7 +96,16 @@ impl Terminal { fn type_in(&self, keys: &str) -> Session { let mut child = Command::new("script") .arg("-qec") - .arg(cargo_bin("dirsql").to_str().expect("a UTF-8 binary path")) + // `--format json` keeps these cases about *where a statement + // ends*: on a terminal the REPL renders a table by default + // (#989), and asserting rows through a table layout would make + // every one of them a rendering test too. `repl_format_e2e.rs` + // covers the rendering. + .arg(format!( + "{} {}", + cargo_bin("dirsql").to_str().expect("a UTF-8 binary path"), + self.args + )) .arg("/dev/null") .current_dir(self.root.path()) // Keep history inside the test's own tree: a suite that wrote to @@ -388,6 +410,25 @@ fn ctrl_d_leaves_the_session() { ); } +#[test] +fn a_terminal_session_renders_a_table_by_default() { + // The other half of #989's rule, and the only place it can be seen: with + // no `--format`, a session whose stdout is a terminal lays its rows out + // to be read rather than piped. + let session = Terminal::default_format().type_in("SELECT 1 AS n;\nquit\n"); + + assert!( + session.shows("1 row"), + "a table footer, got:\n{}", + session.screen + ); + assert!( + !session.shows(r#"[{"n":1}]"#), + "not the JSON array, got:\n{}", + session.screen + ); +} + #[test] fn a_failed_statement_still_leaves_the_session_running() { // Slice 1's continue-on-error survives multi-line entry. From 8d830a373acc03a88601acb844b4e0eb2908d834 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 22:40:23 +0000 Subject: [PATCH 3/4] e2e attestation for claude/epic-953-slice3-format --- .../e2e-attestations/claude-epic-953-slice3-format.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 packages/python/e2e-attestations/claude-epic-953-slice3-format.json diff --git a/packages/python/e2e-attestations/claude-epic-953-slice3-format.json b/packages/python/e2e-attestations/claude-epic-953-slice3-format.json new file mode 100644 index 000000000..47e08e7eb --- /dev/null +++ b/packages/python/e2e-attestations/claude-epic-953-slice3-format.json @@ -0,0 +1,7 @@ +{ + "command": "uv run python -m pytest tests/e2e/ -x -q", + "ran_at": 1787265623, + "exit_code": 0, + "commit": "060576f12a666b12e3256dccfbc17257870ae52e", + "branch": "claude/epic-953-slice3-format" +} From a81e07540583afcee689122ce8895b07350880db Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 22:40:40 +0000 Subject: [PATCH 4/4] e2e attestation for claude/epic-953-slice3-format --- .../ts/e2e-attestations/claude-epic-953-slice3-format.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 packages/ts/e2e-attestations/claude-epic-953-slice3-format.json diff --git a/packages/ts/e2e-attestations/claude-epic-953-slice3-format.json b/packages/ts/e2e-attestations/claude-epic-953-slice3-format.json new file mode 100644 index 000000000..35d6e9be5 --- /dev/null +++ b/packages/ts/e2e-attestations/claude-epic-953-slice3-format.json @@ -0,0 +1,7 @@ +{ + "command": "pnpm test:e2e", + "ran_at": 1787265640, + "exit_code": 0, + "commit": "8d830a373acc03a88601acb844b4e0eb2908d834", + "branch": "claude/epic-953-slice3-format" +}