From 92647fa5b626f07ab63a842496d4fa81c3c66953 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Fri, 16 Oct 2026 23:43:33 +0000 Subject: [PATCH] Quote graph data CSV labels so they round-trip through graph.setData Found while testing crates/doc/src/graph.rs:429 (GraphSpec::to_csv) and crates/engine/src/cmd/graph.rs:85 (parse_csv) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 Co-Authored-By: Claude Opus 5.5 --- crates/doc/src/graph.rs | 19 ++++++---- crates/engine/src/cmd/graph.rs | 66 ++++++++++++++++++++++++++++------ 2 files changed, 68 insertions(+), 17 deletions(-) diff --git a/crates/doc/src/graph.rs b/crates/doc/src/graph.rs index 37329ec62..9e1df35d0 100644 --- a/crates/doc/src/graph.rs +++ b/crates/doc/src/graph.rs @@ -430,17 +430,22 @@ impl GraphSpec { let cells = self.cells(); let width = cells.iter().map(Vec::len).max().unwrap_or(0).max(self.series.len()); let mut out = String::from(","); - out.push_str(&(0..width).map(|s| self.series.get(s).cloned().unwrap_or_default()).collect::>().join(",")); + // Quote a label holding a separator, quote or line break (inner quotes doubled), or one with + // surrounding spaces; `always` also quotes blank and numeric labels so they stay labels. + let field = |c: &str, always: bool| { + let special = c.contains([',', '\t', '"', '\n', '\r']) || c != c.trim(); + if special || (always && (c.trim().is_empty() || c.trim().parse::().is_ok())) { + format!("\"{}\"", c.replace('"', "\"\"")) + } else { + c.to_string() + } + }; + out.push_str(&(0..width).map(|s| field(self.series.get(s).map(String::as_str).unwrap_or_default(), false)).collect::>().join(",")); out.push('\n'); let labelled = !self.categories.is_empty(); for (i, r) in cells.iter().enumerate() { if labelled { - let c = self.categories.get(i).map(String::as_str).unwrap_or_default(); - if c.trim().is_empty() || c.trim().parse::().is_ok() || c != c.trim() { - out.push_str(&format!("\"{c}\"")); - } else { - out.push_str(c); - } + out.push_str(&field(self.categories.get(i).map(String::as_str).unwrap_or_default(), true)); out.push(','); } out.push_str(&r.iter().map(|v| v.map(|v| v.to_string()).unwrap_or_default()).collect::>().join(",")); diff --git a/crates/engine/src/cmd/graph.rs b/crates/engine/src/cmd/graph.rs index 8ade093e8..d326e3dba 100644 --- a/crates/engine/src/cmd/graph.rs +++ b/crates/engine/src/cmd/graph.rs @@ -82,20 +82,42 @@ impl Cell { } } +/// Split `s` at each `sep` outside straight quotes (a `""` inside quotes toggles twice, so stays inside). +fn split_unquoted(s: &str, sep: char) -> Vec<&str> { + let mut parts = vec![]; + let mut quoted = false; + let mut start = 0; + for (i, ch) in s.char_indices() { + if ch == '"' { + quoted = !quoted; + } else if ch == sep && !quoted { + parts.push(s.get(start..i).unwrap_or_default()); + start = i + ch.len_utf8(); + } + } + parts.push(s.get(start..).unwrap_or_default()); + parts +} + +/// One CSV cell: a cell wholly in quotes keeps its inner text (`""` = `"`), others are trimmed. +fn cell(c: &str) -> Cell { + let c = c.trim(); + match c.strip_prefix('"').and_then(|c| c.strip_suffix('"')) { + Some(inner) => Cell { text: inner.replace("\"\"", "\""), quoted: true }, + None => Cell { text: c.trim_matches('"').to_string(), quoted: false }, + } +} + fn parse_csv(csv: &str) -> (Vec, Vec, Vec>>) { - let rows: Vec> = csv - .lines() + // Records end at line breaks outside quotes; a record is tab-separated if it has a tab outside quotes. + let rows: Vec> = split_unquoted(csv, '\n') + .into_iter() .map(|l| l.trim_end_matches('\r')) .filter(|l| !l.trim().is_empty()) .map(|l| { - let sep = if l.contains('\t') { '\t' } else { ',' }; - l.split(sep) - .map(|c| { - let c = c.trim(); - let quoted = c.len() >= 2 && c.starts_with('"') && c.ends_with('"'); - Cell { text: c.trim_matches('"').to_string(), quoted } - }) - .collect() + let tabs = split_unquoted(l, '\t'); + let parts = if tabs.len() > 1 { tabs } else { split_unquoted(l, ',') }; + parts.into_iter().map(cell).collect() }) .collect(); let Some(first) = rows.first() else { return (vec![], vec![], vec![]) }; @@ -2370,6 +2392,30 @@ mod tests { assert_eq!(cells, g.cells(), "{csv}"); } + #[test] + fn labels_with_commas_quotes_and_newlines_round_trip_through_csv() { + let mut s = Session::new(); + s.execute("file.new", &json!({"width": 800, "height": 800})).unwrap(); + let series = json!(["Revenue, USD", "Say \"hi\"", "a\tb"]); + let categories = json!(["Doe, Jane", "2024", " padded ", "two\nlines"]); + let rows = json!([[1, 2, 3], [4, null, 6], [7, 8, 9], [10, 11, 12]]); + let r = s + .execute( + "graph.create", + &json!({"type": "column", "x": 20, "y": 20, "width": 300, "height": 200, "series": series, "categories": categories, "rows": rows}), + ) + .unwrap(); + let id = r["id"].as_u64().unwrap(); + let before = s.execute("graph.setData", &json!({"id": id})).unwrap(); + assert_eq!(before["series"], series); + assert_eq!(before["categories"], categories); + s.execute("graph.setData", &json!({"id": id, "csv": before["csv"]})).unwrap(); + let after = s.execute("graph.setData", &json!({"id": id})).unwrap(); + assert_eq!(after["series"], before["series"], "{}", before["csv"]); + assert_eq!(after["categories"], before["categories"], "{}", before["csv"]); + assert_eq!(after["rows"], before["rows"], "{}", before["csv"]); + } + #[test] fn empty_cells_are_blank_and_quoted_numbers_are_labels() { // Quoted years label the rows; empty and non-numeric cells are blank values, not zeros.