diff --git a/.changeset/fix-yaml-block-scalar-and-page-all-headers.md b/.changeset/fix-yaml-block-scalar-and-page-all-headers.md new file mode 100644 index 00000000..f8bf33a3 --- /dev/null +++ b/.changeset/fix-yaml-block-scalar-and-page-all-headers.md @@ -0,0 +1,35 @@ +--- +"@googleworkspace/cli": patch +--- + +fix: YAML block scalar for strings with `#`/`:`, and repeated CSV/table headers with `--page-all` + +**Bug 1 — YAML output: `drive#file` rendered as block scalar** + +Strings containing `#` or `:` (e.g. `drive#file`, `https://…`) were +incorrectly emitted as YAML block scalars (`|`), producing output like: + +```yaml +kind: | + drive#file +``` + +Block scalars add an implicit trailing newline which changes the string +value and produces invalid-looking output. The fix restricts block +scalar to strings that genuinely contain newlines; all other strings +are double-quoted, which is safe for any character sequence. + +**Bug 2 — `--page-all` with `--format csv` / `--format table` repeats headers** + +When paginating with `--page-all`, each page printed its own header row, +making the combined output unusable for downstream processing: + +``` +id,kind,name ← page 1 header +1,drive#file,foo.txt +id,kind,name ← page 2 header (unexpected!) +2,drive#file,bar.txt +``` + +Column headers (and the table separator line) are now emitted only for +the first page; continuation pages contain data rows only. diff --git a/src/executor.rs b/src/executor.rs index f985c1e7..9f4e4497 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -250,9 +250,10 @@ async fn handle_json_response( } if pagination.page_all { + let is_first_page = *pages_fetched == 1; println!( "{}", - crate::formatter::format_value_compact(&json_val, output_format) + crate::formatter::format_value_paginated(&json_val, output_format, is_first_page) ); } else { println!( diff --git a/src/formatter.rs b/src/formatter.rs index de134df8..22d75a01 100644 --- a/src/formatter.rs +++ b/src/formatter.rs @@ -55,11 +55,20 @@ pub fn format_value(value: &Value, format: &OutputFormat) -> String { } } -/// Format a JSON value as compact JSON (for NDJSON pagination mode). -pub fn format_value_compact(value: &Value, format: &OutputFormat) -> String { +/// Format a JSON value for a paginated page. +/// +/// When auto-paginating with `--page-all`, CSV and table formats should only +/// emit column headers on the **first** page so that each subsequent page +/// contains only data rows, making the combined output machine-parseable. +/// +/// For JSON the output is compact (one JSON object per line / NDJSON). +/// For YAML the page separator is preserved as-is. +pub fn format_value_paginated(value: &Value, format: &OutputFormat, is_first_page: bool) -> String { match format { OutputFormat::Json => serde_json::to_string(value).unwrap_or_default(), - _ => format_value(value, format), + OutputFormat::Csv => format_csv_page(value, is_first_page), + OutputFormat::Table => format_table_page(value, is_first_page), + OutputFormat::Yaml => format_yaml(value), } } @@ -83,13 +92,22 @@ fn extract_items(value: &Value) -> Option<(&str, &Vec)> { } fn format_table(value: &Value) -> String { + format_table_page(value, true) +} + +/// Format as a text table, optionally omitting the header row. +/// +/// Pass `emit_header = false` for continuation pages when using `--page-all` +/// so the combined terminal output doesn't repeat column names and separator +/// lines between pages. +fn format_table_page(value: &Value, emit_header: bool) -> String { // Try to extract a list of items from standard Google API response let items = extract_items(value); if let Some((_key, arr)) = items { - format_array_as_table(arr) + format_array_as_table(arr, emit_header) } else if let Value::Array(arr) = value { - format_array_as_table(arr) + format_array_as_table(arr, emit_header) } else if let Value::Object(obj) = value { // Single object: key/value table let mut output = String::new(); @@ -104,7 +122,7 @@ fn format_table(value: &Value) -> String { } } -fn format_array_as_table(arr: &[Value]) -> String { +fn format_array_as_table(arr: &[Value], emit_header: bool) -> String { if arr.is_empty() { return "(empty)\n".to_string(); } @@ -159,17 +177,19 @@ fn format_array_as_table(arr: &[Value]) -> String { let mut output = String::new(); - // Header - let header: Vec = columns - .iter() - .enumerate() - .map(|(i, c)| format!("{:width$}", c, width = widths[i])) - .collect(); - let _ = writeln!(output, "{}", header.join(" ")); + if emit_header { + // Header + let header: Vec = columns + .iter() + .enumerate() + .map(|(i, c)| format!("{:width$}", c, width = widths[i])) + .collect(); + let _ = writeln!(output, "{}", header.join(" ")); - // Separator - let sep: Vec = widths.iter().map(|w| "─".repeat(*w)).collect(); - let _ = writeln!(output, "{}", sep.join(" ")); + // Separator + let sep: Vec = widths.iter().map(|w| "─".repeat(*w)).collect(); + let _ = writeln!(output, "{}", sep.join(" ")); + } // Rows for row in &rows { @@ -202,7 +222,8 @@ fn json_to_yaml(value: &Value, indent: usize) -> String { Value::Bool(b) => b.to_string(), Value::Number(n) => n.to_string(), Value::String(s) => { - if s.contains('\n') || s.contains(':') || s.contains('#') { + if s.contains('\n') { + // Genuine multi-line content: block scalar is the most readable choice. format!( "|\n{}", s.lines() @@ -211,7 +232,12 @@ fn json_to_yaml(value: &Value, indent: usize) -> String { .join("\n") ) } else { - format!("\"{s}\"") + // Single-line strings: always double-quote so that characters like + // `#` (comment marker) and `:` (mapping indicator) are never + // misinterpreted by YAML parsers. Escape backslashes and double + // quotes to keep the output valid. + let escaped = s.replace('\\', "\\\\").replace('"', "\\\""); + format!("\"{escaped}\"") } } Value::Array(arr) => { @@ -248,6 +274,14 @@ fn json_to_yaml(value: &Value, indent: usize) -> String { } fn format_csv(value: &Value) -> String { + format_csv_page(value, true) +} + +/// Format as CSV, optionally omitting the header row. +/// +/// Pass `emit_header = false` for all pages after the first when using +/// `--page-all`, so the combined output has a single header line. +fn format_csv_page(value: &Value, emit_header: bool) -> String { let items = extract_items(value); let arr = if let Some((_key, arr)) = items { @@ -277,8 +311,10 @@ fn format_csv(value: &Value) -> String { let mut output = String::new(); - // Header - let _ = writeln!(output, "{}", columns.join(",")); + // Header (omitted on continuation pages) + if emit_header { + let _ = writeln!(output, "{}", columns.join(",")); + } // Rows for item in arr { @@ -418,4 +454,119 @@ mod tests { let val = json!({"status": "ok"}); assert!(extract_items(&val).is_none()); } + + // --- YAML block-scalar regression tests --- + + #[test] + fn test_format_yaml_hash_in_string_is_quoted_not_block() { + // `drive#file` contains `#` which is a YAML comment marker; the + // serialiser must quote it rather than emit a block scalar. + let val = json!({"kind": "drive#file", "id": "123"}); + let output = format_value(&val, &OutputFormat::Yaml); + // Must be a double-quoted string, not a block scalar (`|`). + assert!( + output.contains("kind: \"drive#file\""), + "expected double-quoted kind, got:\n{output}" + ); + assert!( + !output.contains("kind: |"), + "kind must not use block scalar, got:\n{output}" + ); + } + + #[test] + fn test_format_yaml_colon_in_string_is_quoted() { + let val = json!({"url": "https://example.com/path"}); + let output = format_value(&val, &OutputFormat::Yaml); + assert!( + output.contains("url: \"https://example.com/path\""), + "expected double-quoted url, got:\n{output}" + ); + assert!(!output.contains("url: |"), "url must not use block scalar"); + } + + #[test] + fn test_format_yaml_multiline_still_uses_block() { + let val = json!({"body": "line one\nline two"}); + let output = format_value(&val, &OutputFormat::Yaml); + // Multi-line content should still use block scalar. + assert!( + output.contains("body: |"), + "multiline string must use block scalar, got:\n{output}" + ); + } + + // --- Paginated format tests --- + + #[test] + fn test_format_value_paginated_csv_first_page_has_header() { + let val = json!({ + "files": [ + {"id": "1", "name": "a.txt"}, + {"id": "2", "name": "b.txt"} + ] + }); + let output = format_value_paginated(&val, &OutputFormat::Csv, true); + let lines: Vec<&str> = output.lines().collect(); + assert_eq!(lines[0], "id,name", "first page must start with header"); + assert_eq!(lines[1], "1,a.txt"); + } + + #[test] + fn test_format_value_paginated_csv_continuation_no_header() { + let val = json!({ + "files": [ + {"id": "3", "name": "c.txt"} + ] + }); + let output = format_value_paginated(&val, &OutputFormat::Csv, false); + let lines: Vec<&str> = output.lines().collect(); + // The first (and only) line must be a data row, not the header. + assert_eq!(lines[0], "3,c.txt", "continuation page must have no header"); + assert!( + !output.contains("id,name"), + "header must be absent on continuation pages" + ); + } + + #[test] + fn test_format_value_paginated_table_first_page_has_header() { + let val = json!({ + "items": [ + {"id": "1", "name": "foo"} + ] + }); + let output = format_value_paginated(&val, &OutputFormat::Table, true); + assert!( + output.contains("id"), + "table header must appear on first page" + ); + assert!(output.contains("──"), "separator must appear on first page"); + } + + #[test] + fn test_format_value_paginated_table_continuation_no_header() { + let val = json!({ + "items": [ + {"id": "2", "name": "bar"} + ] + }); + let output = format_value_paginated(&val, &OutputFormat::Table, false); + assert!(output.contains("bar"), "data row must be present"); + assert!( + !output.contains("──"), + "separator must be absent on continuation pages" + ); + } + + #[test] + fn test_format_value_paginated_json_is_compact() { + let val = json!({"files": [{"id": "1"}]}); + let output = format_value_paginated(&val, &OutputFormat::Json, true); + // Compact JSON — no pretty-printed newlines inside the object + assert!( + !output.contains("\n "), + "JSON must be compact in paginated mode" + ); + } }