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/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" +} 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 new file mode 100644 index 000000000..70a605c94 --- /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 blank", &["--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:?}"); +} 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. 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" +}