Skip to content

Commit e094b02

Browse files
fix: YAML block scalar and repeated --page-all headers in CSV/table (#37)
- YAML: only emit block scalar (|) for strings with genuine newlines; single-line strings containing '#' or ':' are now double-quoted instead, e.g. 'drive#file' renders as '"drive#file"' not a block scalar. - --page-all: CSV/table formats no longer re-emit column headers on every page; headers appear only on the first page. A new public format_value_paginated() helper replaces the now-removed format_value_compact(). - Add unit tests for both fixes (8 new test cases in formatter.rs).
1 parent ee2e216 commit e094b02

3 files changed

Lines changed: 208 additions & 21 deletions

File tree

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
---
2+
"@googleworkspace/cli": patch
3+
---
4+
5+
fix: YAML block scalar for strings with `#`/`:`, and repeated CSV/table headers with `--page-all`
6+
7+
**Bug 1 — YAML output: `drive#file` rendered as block scalar**
8+
9+
Strings containing `#` or `:` (e.g. `drive#file`, `https://…`) were
10+
incorrectly emitted as YAML block scalars (`|`), producing output like:
11+
12+
```yaml
13+
kind: |
14+
drive#file
15+
```
16+
17+
Block scalars add an implicit trailing newline which changes the string
18+
value and produces invalid-looking output. The fix restricts block
19+
scalar to strings that genuinely contain newlines; all other strings
20+
are double-quoted, which is safe for any character sequence.
21+
22+
**Bug 2 — `--page-all` with `--format csv` / `--format table` repeats headers**
23+
24+
When paginating with `--page-all`, each page printed its own header row,
25+
making the combined output unusable for downstream processing:
26+
27+
```
28+
id,kind,name ← page 1 header
29+
1,drive#file,foo.txt
30+
id,kind,name ← page 2 header (unexpected!)
31+
2,drive#file,bar.txt
32+
```
33+
34+
Column headers (and the table separator line) are now emitted only for
35+
the first page; continuation pages contain data rows only.

src/executor.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,9 +250,10 @@ async fn handle_json_response(
250250
}
251251

252252
if pagination.page_all {
253+
let is_first_page = *pages_fetched == 1;
253254
println!(
254255
"{}",
255-
crate::formatter::format_value_compact(&json_val, output_format)
256+
crate::formatter::format_value_paginated(&json_val, output_format, is_first_page)
256257
);
257258
} else {
258259
println!(

src/formatter.rs

Lines changed: 171 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,20 @@ pub fn format_value(value: &Value, format: &OutputFormat) -> String {
5555
}
5656
}
5757

58-
/// Format a JSON value as compact JSON (for NDJSON pagination mode).
59-
pub fn format_value_compact(value: &Value, format: &OutputFormat) -> String {
58+
/// Format a JSON value for a paginated page.
59+
///
60+
/// When auto-paginating with `--page-all`, CSV and table formats should only
61+
/// emit column headers on the **first** page so that each subsequent page
62+
/// contains only data rows, making the combined output machine-parseable.
63+
///
64+
/// For JSON the output is compact (one JSON object per line / NDJSON).
65+
/// For YAML the page separator is preserved as-is.
66+
pub fn format_value_paginated(value: &Value, format: &OutputFormat, is_first_page: bool) -> String {
6067
match format {
6168
OutputFormat::Json => serde_json::to_string(value).unwrap_or_default(),
62-
_ => format_value(value, format),
69+
OutputFormat::Csv => format_csv_page(value, is_first_page),
70+
OutputFormat::Table => format_table_page(value, is_first_page),
71+
OutputFormat::Yaml => format_yaml(value),
6372
}
6473
}
6574

@@ -83,13 +92,22 @@ fn extract_items(value: &Value) -> Option<(&str, &Vec<Value>)> {
8392
}
8493

8594
fn format_table(value: &Value) -> String {
95+
format_table_page(value, true)
96+
}
97+
98+
/// Format as a text table, optionally omitting the header row.
99+
///
100+
/// Pass `emit_header = false` for continuation pages when using `--page-all`
101+
/// so the combined terminal output doesn't repeat column names and separator
102+
/// lines between pages.
103+
fn format_table_page(value: &Value, emit_header: bool) -> String {
86104
// Try to extract a list of items from standard Google API response
87105
let items = extract_items(value);
88106

89107
if let Some((_key, arr)) = items {
90-
format_array_as_table(arr)
108+
format_array_as_table(arr, emit_header)
91109
} else if let Value::Array(arr) = value {
92-
format_array_as_table(arr)
110+
format_array_as_table(arr, emit_header)
93111
} else if let Value::Object(obj) = value {
94112
// Single object: key/value table
95113
let mut output = String::new();
@@ -104,7 +122,7 @@ fn format_table(value: &Value) -> String {
104122
}
105123
}
106124

107-
fn format_array_as_table(arr: &[Value]) -> String {
125+
fn format_array_as_table(arr: &[Value], emit_header: bool) -> String {
108126
if arr.is_empty() {
109127
return "(empty)\n".to_string();
110128
}
@@ -159,17 +177,19 @@ fn format_array_as_table(arr: &[Value]) -> String {
159177

160178
let mut output = String::new();
161179

162-
// Header
163-
let header: Vec<String> = columns
164-
.iter()
165-
.enumerate()
166-
.map(|(i, c)| format!("{:width$}", c, width = widths[i]))
167-
.collect();
168-
let _ = writeln!(output, "{}", header.join(" "));
180+
if emit_header {
181+
// Header
182+
let header: Vec<String> = columns
183+
.iter()
184+
.enumerate()
185+
.map(|(i, c)| format!("{:width$}", c, width = widths[i]))
186+
.collect();
187+
let _ = writeln!(output, "{}", header.join(" "));
169188

170-
// Separator
171-
let sep: Vec<String> = widths.iter().map(|w| "─".repeat(*w)).collect();
172-
let _ = writeln!(output, "{}", sep.join(" "));
189+
// Separator
190+
let sep: Vec<String> = widths.iter().map(|w| "─".repeat(*w)).collect();
191+
let _ = writeln!(output, "{}", sep.join(" "));
192+
}
173193

174194
// Rows
175195
for row in &rows {
@@ -202,7 +222,8 @@ fn json_to_yaml(value: &Value, indent: usize) -> String {
202222
Value::Bool(b) => b.to_string(),
203223
Value::Number(n) => n.to_string(),
204224
Value::String(s) => {
205-
if s.contains('\n') || s.contains(':') || s.contains('#') {
225+
if s.contains('\n') {
226+
// Genuine multi-line content: block scalar is the most readable choice.
206227
format!(
207228
"|\n{}",
208229
s.lines()
@@ -211,7 +232,12 @@ fn json_to_yaml(value: &Value, indent: usize) -> String {
211232
.join("\n")
212233
)
213234
} else {
214-
format!("\"{s}\"")
235+
// Single-line strings: always double-quote so that characters like
236+
// `#` (comment marker) and `:` (mapping indicator) are never
237+
// misinterpreted by YAML parsers. Escape backslashes and double
238+
// quotes to keep the output valid.
239+
let escaped = s.replace('\\', "\\\\").replace('"', "\\\"");
240+
format!("\"{escaped}\"")
215241
}
216242
}
217243
Value::Array(arr) => {
@@ -248,6 +274,14 @@ fn json_to_yaml(value: &Value, indent: usize) -> String {
248274
}
249275

250276
fn format_csv(value: &Value) -> String {
277+
format_csv_page(value, true)
278+
}
279+
280+
/// Format as CSV, optionally omitting the header row.
281+
///
282+
/// Pass `emit_header = false` for all pages after the first when using
283+
/// `--page-all`, so the combined output has a single header line.
284+
fn format_csv_page(value: &Value, emit_header: bool) -> String {
251285
let items = extract_items(value);
252286

253287
let arr = if let Some((_key, arr)) = items {
@@ -277,8 +311,10 @@ fn format_csv(value: &Value) -> String {
277311

278312
let mut output = String::new();
279313

280-
// Header
281-
let _ = writeln!(output, "{}", columns.join(","));
314+
// Header (omitted on continuation pages)
315+
if emit_header {
316+
let _ = writeln!(output, "{}", columns.join(","));
317+
}
282318

283319
// Rows
284320
for item in arr {
@@ -418,4 +454,119 @@ mod tests {
418454
let val = json!({"status": "ok"});
419455
assert!(extract_items(&val).is_none());
420456
}
457+
458+
// --- YAML block-scalar regression tests ---
459+
460+
#[test]
461+
fn test_format_yaml_hash_in_string_is_quoted_not_block() {
462+
// `drive#file` contains `#` which is a YAML comment marker; the
463+
// serialiser must quote it rather than emit a block scalar.
464+
let val = json!({"kind": "drive#file", "id": "123"});
465+
let output = format_value(&val, &OutputFormat::Yaml);
466+
// Must be a double-quoted string, not a block scalar (`|`).
467+
assert!(
468+
output.contains("kind: \"drive#file\""),
469+
"expected double-quoted kind, got:\n{output}"
470+
);
471+
assert!(
472+
!output.contains("kind: |"),
473+
"kind must not use block scalar, got:\n{output}"
474+
);
475+
}
476+
477+
#[test]
478+
fn test_format_yaml_colon_in_string_is_quoted() {
479+
let val = json!({"url": "https://example.com/path"});
480+
let output = format_value(&val, &OutputFormat::Yaml);
481+
assert!(
482+
output.contains("url: \"https://example.com/path\""),
483+
"expected double-quoted url, got:\n{output}"
484+
);
485+
assert!(!output.contains("url: |"), "url must not use block scalar");
486+
}
487+
488+
#[test]
489+
fn test_format_yaml_multiline_still_uses_block() {
490+
let val = json!({"body": "line one\nline two"});
491+
let output = format_value(&val, &OutputFormat::Yaml);
492+
// Multi-line content should still use block scalar.
493+
assert!(
494+
output.contains("body: |"),
495+
"multiline string must use block scalar, got:\n{output}"
496+
);
497+
}
498+
499+
// --- Paginated format tests ---
500+
501+
#[test]
502+
fn test_format_value_paginated_csv_first_page_has_header() {
503+
let val = json!({
504+
"files": [
505+
{"id": "1", "name": "a.txt"},
506+
{"id": "2", "name": "b.txt"}
507+
]
508+
});
509+
let output = format_value_paginated(&val, &OutputFormat::Csv, true);
510+
let lines: Vec<&str> = output.lines().collect();
511+
assert_eq!(lines[0], "id,name", "first page must start with header");
512+
assert_eq!(lines[1], "1,a.txt");
513+
}
514+
515+
#[test]
516+
fn test_format_value_paginated_csv_continuation_no_header() {
517+
let val = json!({
518+
"files": [
519+
{"id": "3", "name": "c.txt"}
520+
]
521+
});
522+
let output = format_value_paginated(&val, &OutputFormat::Csv, false);
523+
let lines: Vec<&str> = output.lines().collect();
524+
// The first (and only) line must be a data row, not the header.
525+
assert_eq!(lines[0], "3,c.txt", "continuation page must have no header");
526+
assert!(
527+
!output.contains("id,name"),
528+
"header must be absent on continuation pages"
529+
);
530+
}
531+
532+
#[test]
533+
fn test_format_value_paginated_table_first_page_has_header() {
534+
let val = json!({
535+
"items": [
536+
{"id": "1", "name": "foo"}
537+
]
538+
});
539+
let output = format_value_paginated(&val, &OutputFormat::Table, true);
540+
assert!(
541+
output.contains("id"),
542+
"table header must appear on first page"
543+
);
544+
assert!(output.contains("──"), "separator must appear on first page");
545+
}
546+
547+
#[test]
548+
fn test_format_value_paginated_table_continuation_no_header() {
549+
let val = json!({
550+
"items": [
551+
{"id": "2", "name": "bar"}
552+
]
553+
});
554+
let output = format_value_paginated(&val, &OutputFormat::Table, false);
555+
assert!(output.contains("bar"), "data row must be present");
556+
assert!(
557+
!output.contains("──"),
558+
"separator must be absent on continuation pages"
559+
);
560+
}
561+
562+
#[test]
563+
fn test_format_value_paginated_json_is_compact() {
564+
let val = json!({"files": [{"id": "1"}]});
565+
let output = format_value_paginated(&val, &OutputFormat::Json, true);
566+
// Compact JSON — no pretty-printed newlines inside the object
567+
assert!(
568+
!output.contains("\n "),
569+
"JSON must be compact in paginated mode"
570+
);
571+
}
421572
}

0 commit comments

Comments
 (0)