fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(io): split persistence/mod.rs into focused sub-modules (improvise-nr5)

Browse Source 
Pure relocation, no behaviour change. 2783 lines in one file became:

  mod.rs              the document lifecycle (save/load/gzip/legacy-JSON over
                      Storage) plus the pub facade re-exporting the rest
  quoting.rs          CL-style |…| quoting, the one thing format and parse share
  format.rs           format_md + format_number, persisted_initial_view, coord_str
  parse.rs            parse_md + ImprovParser/Rule, parse_number_cell, and the
                      parser property/edge-case tests and grammar generator
  export.rs           export_csv / export_csv_to / render_csv
  roundtrip_tests.rs  the format_md <-> parse_md characterization suite, which
                      belongs to neither side on its own

Every function moved with its tests where ownership was clear; the 34 tests
that exercise both directions went to roundtrip_tests.rs. `coord` and
`two_cat_model`, previously copied per test module, are now one `test_helpers`
module in mod.rs.

No test changed meaning: all 97 persistence tests still pass, and each moved
function body is identical modulo indentation and the visibility keyword on the
five quoting helpers (now pub(super)).

The public API is unchanged — save, load, save_to, load_from, autosave_path,
format_md, parse_md, export_csv and export_csv_to all still resolve through
`crate::persistence`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EoW5BB87ZZdm96SvBTWawH
This commit is contained in:
Edward Langley
2026-09-19 13:35:39 -07:00
parent 2d7ff6e00c
commit 87f86d7eb5
8 changed files with 2639 additions and 2574 deletions
Download Patch File Download Diff File Expand all files Collapse all files
context/repo-map.md
+10 -3
View File
@@ -29,7 +29,7 @@ Architectural intent lives in `context/design-principles.md` — read that for t
| Change an axis label, color or cycle | `view/axis.rs` → `impl Axis` (colors resolve in `draw.rs::axis_fg`) |
| Ask "what data is visible" vs "what can the user page" | `View::filtering_cats()` vs `categories_on(Axis::Page)` |
| Change grid layout | `view/layout.rs` → `GridLayout` |
| Change `.improv` format | `persistence/improv.pest` + `persistence/mod.rs` |
| Change `.improv` format | `persistence/improv.pest` + `persistence/{format,parse}.rs` |
| Change where documents are stored | `storage.rs` → `Storage` impls (`opfs.rs` for wasm) |
| Change number display | `format.rs` → `format_f64` |
| Change CLI args | `main.rs` (clap) |
@@ -169,7 +169,7 @@ Region=West, Measure=Revenue = |pending| # pipe-quoted text
25. **`ViewState` and its field types are `Serialize`/`Deserialize`.** It is the wire payload of the client Snapshot (improvise-q08), so `AppMode`, `MinibufferConfig`, `MinibufColor`, `ViewFrame`, `KeyCode` and `KeyModifiers` all derive both (improvise-a2x), and `improvise-command` depends on `serde` for it. The constraint this puts on new state: **no borrowed-`'static` fields anywhere ViewState can reach** — `Deserialize` cannot produce one, and the error surfaces on every enclosing variant rather than at the offending field. `view_state_survives_a_json_round_trip` guards it.
26. **Axis presentation lives on `Axis`, colors resolve in `draw.rs`.** Four vocabularies hang off the enum in `view/axis.rs` — `plain_label()` ("Row"/"Col"/"Page"/"None"), `glyph()` (which `Display` appends, giving "Row ↕"), `short_label()` (three columns for the tile bar; `None` is "·", *not* a truncation), and `initial()` (one letter for the view panel; `None` is empty). Plus `color() -> AxisColor` and `next()` for the Row→Column→Page→None rotation. `AxisColor` is a semantic palette: `improvise-core` must not name a ratatui type, so `draw.rs::axis_fg` does the one mapping to a terminal color, exactly as `minibuf_fg` does for `MinibufColor` (improvise-rml). The tile bar and category panel each carried their own copy of the color table before, with nothing keeping them in agreement. Not covered here: `persistence/mod.rs` maps `Axis` to the on-disk keywords ("row"/"column"/"page"), which is a format concern and stays in the io layer.
26. **Axis presentation lives on `Axis`, colors resolve in `draw.rs`.** Four vocabularies hang off the enum in `view/axis.rs` — `plain_label()` ("Row"/"Col"/"Page"/"None"), `glyph()` (which `Display` appends, giving "Row ↕"), `short_label()` (three columns for the tile bar; `None` is "·", *not* a truncation), and `initial()` (one letter for the view panel; `None` is empty). Plus `color() -> AxisColor` and `next()` for the Row→Column→Page→None rotation. `AxisColor` is a semantic palette: `improvise-core` must not name a ratatui type, so `draw.rs::axis_fg` does the one mapping to a terminal color, exactly as `minibuf_fg` does for `MinibufColor` (improvise-rml). The tile bar and category panel each carried their own copy of the color table before, with nothing keeping them in agreement. Not covered here: `persistence/format.rs` maps `Axis` to the on-disk keywords ("row"/"column"/"page"), which is a format concern and stays in the io layer.
27. **`Axis::Filter` constrains without being navigable.** Drilling fixes its coordinates on `Filter`, not `Page` (improvise-dz8): both restrict which records are visible, but only `Page` is something the user may move or page through. The split shows up as two different questions, and picking the wrong one is the trap: **"what data is visible" is `View::filtering_cats()`** (Page ∪ Filter — used by `GridLayout` and by `add-record-row` when it builds a key from the current constraints), while **"what can the user act on" is `categories_on(Axis::Page)`** (used by `[`/`]` page cycling) or `View::tileable_cats()` (used by all three tile-select sites). `Axis::next()` maps `Filter` to itself and no navigable axis rotates into it, so tile select cannot hand one out. The `.improv` grammar accepts `filter` and round-trips a selection with it — a Filter axis only ever occurs in the never-persisted `_Drill` view, but the alternative was writing it out as some other axis.
@@ -213,7 +213,14 @@ storage.rs 288 / 12t Storage trait (read/write/list/delete), Phy
opfs.rs 253 / 0t OpfsFs: Storage over the browser's Origin Private File
System. wasm32 only. Compile-checked, not browser-tested.
persistence/improv.pest 124 PEG grammar — single source of truth
persistence/mod.rs 2783 / 97t save/load/gzip/legacy-JSON, CSV export
persistence/mod.rs 259 / 7t document lifecycle: save/load/gzip/legacy-JSON over Storage;
facade re-exporting format_md/parse_md/export_csv*
persistence/quoting.rs 63 / 0t CL-style |…| quoting, shared by format and parse
persistence/format.rs 350 / 11t format_md: Workbook → .improv text
persistence/parse.rs 1378 / 42t parse_md: pest tree-walk → Workbook (+ grammar generator)
persistence/export.rs 156 / 3t export_csv / render_csv
persistence/roundtrip_tests.rs
635 / 34t format_md ↔ parse_md characterization (tests only)
import/wizard.rs 1121 / 38t ImportPipeline + ImportWizard
import/analyzer.rs 292 / 9t Field kind detection (Category/Measure/Time/Skip)
import/csv_parser.rs 300 / 8t CSV parsing, multi-file merge
crates/improvise-formula/src/parser.rs
+1 -1
View File
@@ -862,7 +862,7 @@ mod tests {
//
// Uses pest_meta to read formula.pest at test time and walks the grammar
// AST to generate random valid formulas. Mirrors the pattern used by the
// persistence parser tests in src/persistence/mod.rs.
// persistence parser tests in src/persistence/parse.rs.
#[cfg(test)]
mod generator {
crates/improvise-io/src/persistence/export.rs
+156
View File
@@ -0,0 +1,156 @@
//! CSV export of a rendered view.
use anyhow::Result;
use std::path::Path;
use crate::storage::{PhysicalFs, Storage};
use crate::view::GridLayout;
use crate::workbook::Workbook;
/// Export a view as CSV through the real filesystem. The native entry point.
pub fn export_csv(workbook: &Workbook, view_name: &str, path: &Path) -> Result<()> {
export_csv_to(&PhysicalFs, workbook, view_name, path)
}
/// Export a view as CSV into `storage`.
pub fn export_csv_to(
storage: &dyn Storage,
workbook: &Workbook,
view_name: &str,
path: &Path,
) -> Result<()> {
storage.write(path, &render_csv(workbook, view_name)?)
}
/// Render a view as CSV bytes. Separated from writing so the bytes can go to
/// any [`Storage`] backend, not just a file (improvise-fuj).
fn render_csv(workbook: &Workbook, view_name: &str) -> Result<Vec<u8>> {
let view = workbook
.views
.get(view_name)
.ok_or_else(|| anyhow::anyhow!("View '{view_name}' not found"))?;
let layout = GridLayout::new(&workbook.model, view);
let model = &workbook.model;
let (fmt_comma, fmt_decimals) = crate::format::parse_number_format(&view.number_format);
let mut wtr = csv::WriterBuilder::new()
.flexible(true)
.from_writer(Vec::new());
// Header row
let row_header = layout.row_cats.join("/");
let page_label: Vec<String> = layout
.page_coords
.iter()
.map(|(c, v)| format!("{c}={v}"))
.collect();
let header_prefix = if page_label.is_empty() {
row_header
} else {
format!("{} ({})", row_header, page_label.join(", "))
};
let mut header: Vec<String> = Vec::new();
if !header_prefix.is_empty() {
header.push(header_prefix);
}
header.extend((0..layout.col_count()).map(|ci| layout.col_label(ci)));
wtr.write_record(&header)?;
// Data rows
for ri in 0..layout.row_count() {
let mut record: Vec<String> = Vec::new();
let row_label = layout.row_label(ri);
if !row_label.is_empty() {
record.push(row_label);
}
record.extend(
(0..layout.col_count())
.map(|ci| layout.display_text(model, ri, ci, fmt_comma, fmt_decimals)),
);
wtr.write_record(&record)?;
}
wtr.flush()?;
Ok(wtr.into_inner()?)
}
#[cfg(test)]
mod tests {
use crate::model::cell::CellValue;
use crate::persistence::test_helpers::{coord, two_cat_model};
use crate::workbook::Workbook;
#[test]
fn export_csv_produces_valid_output() {
let mut m = two_cat_model();
m.model.set_cell(
coord(&[("Type", "Food"), ("Month", "Jan")]),
CellValue::Number(100.0),
);
m.model.set_cell(
coord(&[("Type", "Gas"), ("Month", "Feb")]),
CellValue::Number(50.0),
);
let dir = tempfile::tempdir().unwrap();
let path = dir.path().join("export.csv");
super::export_csv(&m, "Default", &path).unwrap();
let content = std::fs::read_to_string(&path).unwrap();
// Should have a header and data rows
let lines: Vec<&str> = content.lines().collect();
assert!(lines.len() >= 2, "Expected header + data, got: {content}");
// Header should contain column labels
assert!(
lines[0].contains(','),
"Expected CSV header, got: {}",
lines[0]
);
}
/// Bug improvise-1cz: export_csv joined labels/values with bare commas
/// (no RFC 4180 quoting) and ignored the view's number format. With a
/// comma-grouping format, a value like 1200 renders as "1,200" and a
/// label like "Smith, Jr." splits into two fields, corrupting every row.
#[test]
fn export_csv_quotes_commas_per_rfc4180() {
let mut m = Workbook::new("T");
m.add_category("Type").unwrap();
m.add_item("Type", "Smith, Jr.").unwrap();
m.add_category("Month").unwrap();
m.add_item("Month", "Jan").unwrap();
m.model.set_cell(
coord(&[("Type", "Smith, Jr."), ("Month", "Jan")]),
CellValue::Number(1200.0),
);
m.active_view_mut().number_format = ",.2f".to_string();
let dir = tempfile::tempdir().unwrap();
let path = dir.path().join("export.csv");
super::export_csv(&m, "Default", &path).unwrap();
let mut rdr = csv::ReaderBuilder::new()
.has_headers(false)
.from_path(&path)
.unwrap();
let records: Vec<csv::StringRecord> = rdr.records().collect::<Result<_, _>>().unwrap();
// Header + one data row, each with exactly 2 fields (label + value)
assert_eq!(records.len(), 2, "expected header + 1 row: {records:?}");
for r in &records {
assert_eq!(r.len(), 2, "comma split a field: {r:?}");
}
assert_eq!(&records[1][0], "Smith, Jr.");
assert!(
records[1][1].starts_with("1,200"),
"expected comma-grouped value, got: {:?}",
&records[1][1]
);
}
#[test]
fn export_csv_unknown_view_returns_error() {
let m = two_cat_model();
let dir = tempfile::tempdir().unwrap();
let path = dir.path().join("export.csv");
assert!(super::export_csv(&m, "Nonexistent", &path).is_err());
}
}
crates/improvise-io/src/persistence/format.rs
+350
View File
@@ -0,0 +1,350 @@
//! Serializing a workbook to the `.improv` text format.
use crate::model::category::CategoryKind;
use crate::model::cell::{CellKey, CellValue};
use crate::view::Axis;
use crate::workbook::Workbook;
use super::quoting::{pipe_quote, quote_name};
fn format_number(n: f64) -> String {
if n.is_infinite() {
return if n.is_sign_positive() {
"inf".to_string()
} else {
"-inf".to_string()
};
}
if n.is_nan() {
return "nan".to_string();
}
if n.fract() == 0.0 && n.abs() < 1e15 {
format!("{}", n as i64)
} else {
let display = format!("{n}");
if display.parse::<f64>() == Ok(n) {
display
} else {
format!("{n:?}")
}
}
}
/// The view to record as `Initial View`. Saving from records or drill mode
/// makes a scratch view active, and those are not written — so fall back to
/// the first real view rather than naming one the file won't contain
/// (improvise-60z).
fn persisted_initial_view(workbook: &Workbook) -> &str {
let active_is_real = workbook
.views
.get(&workbook.active_view)
.is_some_and(|v| !v.is_virtual());
match active_is_real {
true => &workbook.active_view,
false => workbook
.views
.values()
.find(|v| !v.is_virtual())
.map(|v| v.name.as_str())
.unwrap_or(&workbook.active_view),
}
}
/// Serialize a workbook to the markdown `.improv` format.
pub fn format_md(workbook: &Workbook) -> String {
// writeln! to a String is infallible; this macro avoids .unwrap() noise.
macro_rules! w {
($dst:expr, $($arg:tt)*) => { { use std::fmt::Write; writeln!($dst, $($arg)*).ok(); } }
}
let model = &workbook.model;
let mut out = String::new();
w!(out, "v2025-04-09");
w!(out, "# {}", model.name);
w!(out, "Initial View: {}", persisted_initial_view(workbook));
// ── Views (first: typically small, orients the reader) ───────────
// `_Records`/`_Drill` are scratch views rebuilt on demand — skip them.
for view in workbook.views.values().filter(|v| !v.is_virtual()) {
w!(out, "\n## View: {}", view.name);
for (cat, axis) in &view.category_axes {
let qcat = quote_name(cat);
let axis_str = match axis {
Axis::Row => "row",
Axis::Column => "column",
Axis::Page => "page",
Axis::Filter => "filter",
Axis::None => "none",
};
// Page and Filter both carry the selected item alongside the
// axis keyword; the other axes have nothing to select.
if axis.filters_data()
&& let Some(sel) = view.page_selections.get(cat)
{
w!(out, "{qcat}: {axis_str}, {}", quote_name(sel));
continue;
}
w!(out, "{qcat}: {axis_str}");
}
if !view.number_format.is_empty() {
w!(out, "format: {}", view.number_format);
}
for (prefix, map) in [
("hidden", &view.hidden_items),
("collapsed", &view.collapsed_groups),
] {
let mut pairs: Vec<_> = map
.iter()
.flat_map(|(cat, items)| {
items.iter().map(move |item| (cat.as_str(), item.as_str()))
})
.collect();
pairs.sort();
for (cat, item) in pairs {
w!(out, "{prefix}: {}/{}", quote_name(cat), quote_name(item));
}
}
}
// ── Formulas ─────────────────────────────────────────────────────
if !model.formulas().is_empty() {
w!(out, "\n## Formulas");
for f in model.formulas() {
if CategoryKind::of_name(&f.target_category).is_measure() {
w!(out, "- {}", f.raw);
} else {
w!(out, "- {} [{}]", f.raw, quote_name(&f.target_category));
}
}
}
// ── Categories (items comma-separated on one line) ───────────────
// Collect formula targets so we can exclude them from _Measure items
let formula_targets: std::collections::HashSet<&str> = model
.formulas()
.iter()
.filter(|f| CategoryKind::of_name(&f.target_category).is_measure())
.map(|f| f.target.as_str())
.collect();
for cat in model.categories.values() {
// _Index and _Dim are fully virtual — never persisted
if cat.kind.is_index_or_dim() {
continue;
}
w!(out, "\n## Category: {}", cat.name);
let mut bare: Vec<String> = Vec::new();
let mut grouped: Vec<String> = Vec::new();
for item in cat.items.values() {
// For _Measure, skip items that are formula targets
// (they'll be recreated from the ## Formulas section)
if cat.kind == CategoryKind::VirtualMeasure
&& formula_targets.contains(item.name.as_str())
{
continue;
}
match &item.group {
Some(g) => grouped.push(format!("{}[{}]", quote_name(&item.name), quote_name(g))),
None => bare.push(quote_name(&item.name)),
}
}
if !bare.is_empty() {
w!(out, "- {}", bare.join(", "));
}
for g_item in &grouped {
w!(out, "- {g_item}");
}
for g in &cat.groups {
if let Some(parent) = &g.parent {
w!(out, "> {}[{}]", quote_name(&g.name), quote_name(parent));
}
}
}
// ── Data (last: typically the largest section) ────────────────────
let mut cells: Vec<_> = model.data.iter_cells().collect();
cells.sort_by_key(|(k, _)| coord_str(k));
if !cells.is_empty() {
w!(out, "\n## Data");
for (key, value) in cells {
let val_str = match value {
CellValue::Number(n) => format_number(*n),
CellValue::Text(s) | CellValue::Error(s) => pipe_quote(s),
};
w!(out, "{} = {}", coord_str(&key), val_str);
}
}
out
}
fn coord_str(key: &CellKey) -> String {
key.0
.iter()
.map(|(c, i)| format!("{}={}", quote_name(c), quote_name(i)))
.collect::<Vec<_>>()
.join(", ")
}
#[cfg(test)]
mod tests {
use super::format_md;
use crate::model::category::Group;
use crate::model::cell::CellValue;
use crate::persistence::test_helpers::{coord, two_cat_model};
use crate::view::Axis;
use crate::workbook::Workbook;
// ── Virtual views (improvise-60z) ────────────────────────────────────────
/// `_Records` and `_Drill` are scratch views the app builds on demand.
/// They put `_Index`/`_Dim` on axes, and those categories are never
/// written to the file — so persisting the views produces axis lines
/// referring to categories the loader will not find.
#[test]
fn virtual_views_are_not_written() {
let mut wb = two_cat_model();
wb.create_view("_Records");
wb.create_view("_Drill");
let text = format_md(&wb);
assert!(
!text.contains("## View: _Records"),
"_Records must not be persisted:\n{text}"
);
assert!(
!text.contains("## View: _Drill"),
"_Drill must not be persisted:\n{text}"
);
assert!(
text.contains("## View: Default"),
"real views still persist:\n{text}"
);
}
#[test]
fn format_md_contains_model_name() {
let m = Workbook::new("My Model");
assert!(format_md(&m).contains("# My Model"));
}
#[test]
fn format_md_contains_category_and_items() {
let m = two_cat_model();
let text = format_md(&m);
assert!(text.contains("## Category: Type"));
// Bare items are now comma-separated on one line
assert!(
text.contains("- Food, Gas"),
"expected comma-separated items:\n{text}"
);
assert!(text.contains("## Category: Month"));
assert!(text.contains("Jan"));
}
#[test]
fn format_md_item_with_group_uses_brackets() {
let mut m = Workbook::new("T");
m.add_category("Month").unwrap();
m.add_item_in_group("Month", "Jan", "Q1").unwrap();
let text = format_md(&m);
assert!(text.contains("- Jan[Q1]"), "got:\n{text}");
}
#[test]
fn format_md_group_hierarchy_uses_angle_prefix() {
let mut m = Workbook::new("T");
m.add_category("Month").unwrap();
m.add_item_in_group("Month", "Jan", "Q1").unwrap();
m.model
.category_mut("Month")
.unwrap()
.add_group(Group::new("Q1").with_parent("2025"));
let text = format_md(&m);
assert!(text.contains("> Q1[|2025|]"), "got:\n{text}");
}
#[test]
fn format_md_data_is_sorted_and_quoted() {
let mut m = two_cat_model();
m.model.set_cell(
coord(&[("Month", "Feb"), ("Type", "Food")]),
CellValue::Number(200.0),
);
m.model.set_cell(
coord(&[("Month", "Jan"), ("Type", "Gas")]),
CellValue::Text("N/A".into()),
);
let text = format_md(&m);
let data_pos = text.find("## Data").unwrap();
let feb_pos = text.find("Month=Feb").unwrap();
let jan_pos = text.find("Month=Jan").unwrap();
assert!(
data_pos < feb_pos && feb_pos < jan_pos,
"expected sorted order Feb < Jan:\n{text}"
);
assert!(text.contains("= 200"), "number not quoted:\n{text}");
assert!(
text.contains("= |N/A|"),
"text should be pipe-quoted:\n{text}"
);
}
#[test]
fn format_md_view_axes() {
let m = two_cat_model();
let text = format_md(&m);
assert!(text.contains("## View: Default"));
assert!(text.contains("Type: row"));
assert!(text.contains("Month: column"));
}
#[test]
fn format_md_page_axis_with_selection() {
let mut m = Workbook::new("T");
m.add_category("Type").unwrap();
m.add_category("Month").unwrap();
m.add_category("Region").unwrap();
m.add_item("Type", "Food").unwrap();
m.add_item("Month", "Jan").unwrap();
for r in ["East", "West"] {
m.add_item("Region", r).unwrap();
}
m.active_view_mut().set_page_selection("Region", "West");
let text = format_md(&m);
assert!(text.contains("Region: page, West"), "got:\n{text}");
}
#[test]
fn format_md_collapsed_group() {
let mut m = two_cat_model();
m.active_view_mut().toggle_group_collapse("Type", "MyGroup");
let text = format_md(&m);
assert!(text.contains("collapsed: Type/MyGroup"));
}
// ── format_md: page axis without selection ──────────────────────────
#[test]
fn format_md_page_without_selection() {
let mut m = two_cat_model();
m.active_view_mut().set_axis("Month", Axis::Page);
// Don't set a page selection
let text = format_md(&m);
assert!(text.contains("Month: page"));
// Should NOT have a comma after "page" (no selection)
let line = text.lines().find(|l| l.starts_with("Month:")).unwrap();
assert!(!line.contains(','), "Expected no selection, got: {line}");
}
// ── format_md: none axis ────────────────────────────────────────────
#[test]
fn format_md_none_axis() {
let mut m = two_cat_model();
m.active_view_mut().set_axis("Month", Axis::None);
let text = format_md(&m);
assert!(text.contains("Month: none"));
}
}
crates/improvise-io/src/persistence/mod.rs
+46 -2570
View File
File diff suppressed because it is too large Load Diff
crates/improvise-io/src/persistence/parse.rs
+1378
View File
File diff suppressed because it is too large Load Diff
crates/improvise-io/src/persistence/quoting.rs
+63
View File
@@ -0,0 +1,63 @@
//! Pipe quoting, shared between formatting and parsing.
/// Check whether a name is a valid bare identifier: `[A-Za-z_][A-Za-z0-9_-]*`
pub(super) fn is_bare_name(name: &str) -> bool {
let mut chars = name.chars();
match chars.next() {
Some(c) if c.is_ascii_alphabetic() || c == '_' => {}
_ => return false,
}
chars.all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-')
}
/// Escape a string for use inside pipe delimiters: `\|`, `\\`, `\n`.
pub(super) fn escape_pipe(s: &str) -> String {
let mut out = String::with_capacity(s.len());
for c in s.chars() {
match c {
'|' => out.push_str("\\|"),
'\\' => out.push_str("\\\\"),
'\n' => out.push_str("\\n"),
c => out.push(c),
}
}
out
}
/// Unescape a pipe-quoted body: `\|` → `|`, `\\` → `\`, `\n` → newline.
pub(super) fn unescape_pipe(s: &str) -> String {
let mut out = String::with_capacity(s.len());
let mut chars = s.chars();
while let Some(c) = chars.next() {
if c == '\\' {
match chars.next() {
Some('|') => out.push('|'),
Some('\\') => out.push('\\'),
Some('n') => out.push('\n'),
Some(other) => {
out.push('\\');
out.push(other);
}
None => out.push('\\'),
}
} else {
out.push(c);
}
}
out
}
/// CL-style `|...|` pipe quoting unless the name is a valid bare identifier.
pub(super) fn quote_name(name: &str) -> String {
if is_bare_name(name) {
name.to_string()
} else {
format!("|{}|", escape_pipe(name))
}
}
/// Pipe-quote unconditionally (for text cell values that must be distinguished
/// from numbers).
pub(super) fn pipe_quote(s: &str) -> String {
format!("|{}|", escape_pipe(s))
}
crates/improvise-io/src/persistence/roundtrip_tests.rs
+635
View File
@@ -0,0 +1,635 @@
//! Round-trip characterization of the `.improv` format: what `format_md`
//! writes, `parse_md` must read back.
use super::test_helpers::{coord, two_cat_model};
use super::{format_md, parse_md};
use crate::formula::parse_formula;
use crate::model::category::Group;
use crate::model::cell::CellValue;
use crate::view::Axis;
use crate::workbook::Workbook;
// ── Virtual views (improvise-60z) ────────────────────────────────────────
/// Saving while in records mode must not leave `Initial View: _Records`
/// pointing at a view the file no longer contains.
#[test]
fn initial_view_skips_a_virtual_active_view() {
let mut wb = two_cat_model();
wb.create_view("_Records");
wb.switch_view("_Records").unwrap();
let text = format_md(&wb);
assert!(
text.contains("Initial View: Default"),
"expected a real initial view:\n{text}"
);
let loaded = parse_md(&text).unwrap();
assert_eq!(loaded.active_view, "Default");
assert!(
loaded.views.contains_key(&loaded.active_view),
"the initial view must exist in the loaded workbook"
);
}
/// A file written by an older build may already carry the scratch views;
/// loading one must not reinstate them.
#[test]
fn virtual_views_in_an_existing_file_are_dropped_on_load() {
let text = "v2025-04-09\n\
# Budget\n\
Initial View: _Records\n\
\n## View: Default\n\
Type: row\n\
\n## View: _Records\n\
_Index: row\n\
\n## Category: Type\n\
- Food, Gas\n";
let loaded = parse_md(text).unwrap();
assert!(
!loaded.views.contains_key("_Records"),
"stale virtual view should be dropped, got {:?}",
loaded.views.keys().collect::<Vec<_>>()
);
assert_eq!(loaded.active_view, "Default");
}
// ── format_md ────────────────────────────────────────────────────────────
/// improvise-dz8: `Axis::Filter` round-trips, selection included. It
/// arises only inside the `_Drill` view, which is never written, so this
/// is defensive — but the alternative to supporting it in the format is
/// silently writing a Filter axis as something else.
#[test]
fn filter_axis_round_trips_with_its_selection() {
let mut m = Workbook::new("T");
m.add_category("Type").unwrap();
m.add_category("Region").unwrap();
m.add_item("Type", "Food").unwrap();
for r in ["East", "West"] {
m.add_item("Region", r).unwrap();
}
m.active_view_mut().set_axis("Region", Axis::Filter);
m.active_view_mut().set_page_selection("Region", "West");
let text = format_md(&m);
assert!(text.contains("Region: filter, West"), "got:\n{text}");
let back = parse_md(&text).expect("filter axis parses");
assert_eq!(back.active_view().axis_of("Region"), Axis::Filter);
assert_eq!(back.active_view().page_selection("Region"), Some("West"));
}
#[test]
fn format_md_formula_includes_category() {
let mut m = two_cat_model();
m.add_item("Type", "Total").unwrap();
m.model
.add_formula(parse_formula("Total = Food + Gas", "Type").unwrap());
let text = format_md(&m);
assert!(text.contains("- Total = Food + Gas [Type]"), "got:\n{text}");
}
// ── parse_md ─────────────────────────────────────────────────────────────
#[test]
fn parse_md_round_trips_model_name() {
let m = Workbook::new("My Budget");
assert_eq!(parse_md(&format_md(&m)).unwrap().model.name, "My Budget");
}
#[test]
fn parse_md_round_trips_categories_and_items() {
let m = two_cat_model();
let m2 = parse_md(&format_md(&m)).unwrap();
assert!(
m2.model
.category("Type")
.and_then(|c| c.items.get("Food"))
.is_some()
);
assert!(
m2.model
.category("Month")
.and_then(|c| c.items.get("Feb"))
.is_some()
);
}
#[test]
fn parse_md_round_trips_item_group() {
let mut m = Workbook::new("T");
m.add_category("Month").unwrap();
m.add_item_in_group("Month", "Jan", "Q1").unwrap();
let m2 = parse_md(&format_md(&m)).unwrap();
assert_eq!(
m2.model
.category("Month")
.and_then(|c| c.items.get("Jan"))
.and_then(|i| i.group.as_deref()),
Some("Q1")
);
}
#[test]
fn parse_md_round_trips_group_hierarchy() {
let mut m = Workbook::new("T");
m.add_category("Month").unwrap();
m.add_item_in_group("Month", "Jan", "Q1").unwrap();
m.model
.category_mut("Month")
.unwrap()
.add_group(Group::new("Q1").with_parent("2025"));
let m2 = parse_md(&format_md(&m)).unwrap();
let groups = &m2.model.category("Month").unwrap().groups;
let q1 = groups.iter().find(|g| g.name == "Q1").unwrap();
assert_eq!(q1.parent.as_deref(), Some("2025"));
}
#[test]
fn parse_md_round_trips_data_cells() {
let mut m = two_cat_model();
m.model.set_cell(
coord(&[("Month", "Jan"), ("Type", "Food")]),
CellValue::Number(100.0),
);
m.model.set_cell(
coord(&[("Month", "Feb"), ("Type", "Gas")]),
CellValue::Text("N/A".into()),
);
let m2 = parse_md(&format_md(&m)).unwrap();
assert_eq!(
m2.model
.get_cell(&coord(&[("Month", "Jan"), ("Type", "Food")])),
Some(&CellValue::Number(100.0))
);
assert_eq!(
m2.model
.get_cell(&coord(&[("Month", "Feb"), ("Type", "Gas")])),
Some(&CellValue::Text("N/A".into()))
);
}
#[test]
fn parse_md_round_trips_view_axes() {
let m = two_cat_model();
let m2 = parse_md(&format_md(&m)).unwrap();
let v = m2.active_view();
assert_eq!(v.axis_of("Type"), Axis::Row);
assert_eq!(v.axis_of("Month"), Axis::Column);
}
#[test]
fn parse_md_round_trips_page_selection() {
let mut m = Workbook::new("T");
m.add_category("Type").unwrap();
m.add_category("Month").unwrap();
m.add_category("Region").unwrap();
m.add_item("Type", "Food").unwrap();
m.add_item("Month", "Jan").unwrap();
for r in ["East", "West"] {
m.add_item("Region", r).unwrap();
}
m.active_view_mut().set_page_selection("Region", "West");
let m2 = parse_md(&format_md(&m)).unwrap();
assert_eq!(m2.active_view().page_selection("Region"), Some("West"));
assert_eq!(m2.active_view().axis_of("Region"), Axis::Page);
}
// active_view is no longer persisted — it's runtime state
#[test]
fn parse_md_round_trips_formula() {
let mut m = two_cat_model();
m.add_item("Type", "Total").unwrap();
m.model
.add_formula(parse_formula("Total = Food + Gas", "Type").unwrap());
let m2 = parse_md(&format_md(&m)).unwrap();
let f = &m2.model.formulas()[0];
assert_eq!(f.raw, "Total = Food + Gas");
assert_eq!(f.target_category, "Type");
}
/// Bug improvise-zgb (a): `rfind(" [")` mis-split a suffix whose category
/// name itself contains " [" — the writer emitted it unquoted
/// (`[Margin [%]]`) and the parser then split inside the name, producing
/// garbage raw text and category "%]". Category names in the suffix are
/// now pipe-quoted on write and parsed via the grammar's `name` rule.
#[test]
fn parse_md_round_trips_formula_category_with_special_chars() {
let mut m = two_cat_model();
m.add_category("Margin [%]").unwrap();
m.add_item("Margin [%]", "Total").unwrap();
m.model
.add_formula(parse_formula("Total = Food + Gas", "Margin [%]").unwrap());
let m2 = parse_md(&format_md(&m)).unwrap();
let f = &m2.model.formulas()[0];
assert_eq!(f.raw, "Total = Food + Gas");
assert_eq!(f.target_category, "Margin [%]");
}
#[test]
fn parse_md_round_trips_hidden_item() {
let _ = two_cat_model();
{
let m = &mut two_cat_model();
m.active_view_mut().hide_item("Type", "Gas");
let m2 = parse_md(&format_md(m)).unwrap();
assert!(m2.active_view().is_hidden("Type", "Gas"));
assert!(!m2.active_view().is_hidden("Type", "Food"));
}
}
#[test]
fn format_md_includes_number_format() {
let mut m = two_cat_model();
m.active_view_mut().number_format = ",.2f".to_string();
let text = format_md(&m);
assert!(text.contains("format: ,.2f"));
}
// ── parse_md: text values ───────────────────────────────────────────
#[test]
fn parse_md_round_trips_text_cell_values() {
let mut m = two_cat_model();
m.model.set_cell(
coord(&[("Type", "Food"), ("Month", "Jan")]),
CellValue::Text("pending".to_string()),
);
let text = format_md(&m);
let loaded = parse_md(&text).unwrap();
assert_eq!(
loaded
.model
.get_cell(&coord(&[("Type", "Food"), ("Month", "Jan")])),
Some(&CellValue::Text("pending".to_string()))
);
}
// ── parse_md: collapsed groups roundtrip ────────────────────────────
#[test]
fn parse_md_round_trips_collapsed_group() {
let mut m = two_cat_model();
m.active_view_mut().toggle_group_collapse("Type", "MyGroup");
let text = format_md(&m);
let loaded = parse_md(&text).unwrap();
assert!(loaded.active_view().is_group_collapsed("Type", "MyGroup"));
}
// ── parse_md: number format roundtrip ───────────────────────────────
#[test]
fn parse_md_round_trips_number_format() {
let mut m = two_cat_model();
m.active_view_mut().number_format = ",.2f".to_string();
let text = format_md(&m);
let loaded = parse_md(&text).unwrap();
assert_eq!(loaded.active_view().number_format, ",.2f");
}
// ── parse_md: none axis roundtrip ───────────────────────────────────
#[test]
fn parse_md_round_trips_none_axis() {
let mut m = two_cat_model();
m.active_view_mut().set_axis("Month", Axis::None);
let text = format_md(&m);
let loaded = parse_md(&text).unwrap();
assert_eq!(loaded.active_view().axis_of("Month"), Axis::None);
}
// ── parse_md: multiple views ────────────────────────────────────────
#[test]
fn parse_md_round_trips_multiple_views() {
let mut m = two_cat_model();
m.create_view("Alternate");
{
let v = m.views.get_mut("Alternate").unwrap();
v.set_axis("Type", Axis::Column);
v.set_axis("Month", Axis::Row);
}
let text = format_md(&m);
let loaded = parse_md(&text).unwrap();
assert!(loaded.views.contains_key("Default"));
assert!(loaded.views.contains_key("Alternate"));
let alt = loaded.views.get("Alternate").unwrap();
assert_eq!(alt.axis_of("Type"), Axis::Column);
assert_eq!(alt.axis_of("Month"), Axis::Row);
}
// ── Full save/load/format roundtrip with all features ───────────────
#[test]
fn full_roundtrip_preserves_all_features() {
let mut m = two_cat_model();
// Add data
m.model.set_cell(
coord(&[("Type", "Food"), ("Month", "Jan")]),
CellValue::Number(100.0),
);
m.model.set_cell(
coord(&[("Type", "Gas"), ("Month", "Feb")]),
CellValue::Text("pending".to_string()),
);
// Add formula
let f = parse_formula("Gas = Food * 2", "Type").unwrap();
m.model.add_formula(f);
// Configure view
m.active_view_mut().set_axis("Month", Axis::Page);
m.active_view_mut().set_page_selection("Month", "Jan");
m.active_view_mut().hide_item("Type", "Gas");
m.active_view_mut().toggle_group_collapse("Type", "G1");
m.active_view_mut().number_format = ",.0".to_string();
let text = format_md(&m);
let loaded = parse_md(&text).unwrap();
// Verify everything roundtripped
assert_eq!(loaded.model.name, "Budget");
assert_eq!(
loaded
.model
.get_cell(&coord(&[("Type", "Food"), ("Month", "Jan")])),
Some(&CellValue::Number(100.0))
);
assert_eq!(
loaded
.model
.get_cell(&coord(&[("Type", "Gas"), ("Month", "Feb")])),
Some(&CellValue::Text("pending".to_string()))
);
assert!(!loaded.model.formulas().is_empty());
let v = loaded.active_view();
assert_eq!(v.axis_of("Month"), Axis::Page);
assert_eq!(v.page_selection("Month"), Some("Jan"));
assert!(v.is_hidden("Type", "Gas"));
assert!(v.is_group_collapsed("Type", "G1"));
assert_eq!(v.number_format, ",.0");
}
// ── Stress tests: special characters in values ──────────────────────
#[test]
fn text_value_with_embedded_comma() {
let mut m = two_cat_model();
m.model.set_cell(
coord(&[("Type", "Food"), ("Month", "Jan")]),
CellValue::Text("Smith, Jr.".into()),
);
let text = format_md(&m);
let loaded = parse_md(&text).unwrap();
assert_eq!(
loaded
.model
.get_cell(&coord(&[("Type", "Food"), ("Month", "Jan")])),
Some(&CellValue::Text("Smith, Jr.".into())),
"Comma inside quoted text was corrupted.\n{text}"
);
}
#[test]
fn text_value_with_embedded_double_quote() {
let mut m = two_cat_model();
m.model.set_cell(
coord(&[("Type", "Food"), ("Month", "Jan")]),
CellValue::Text(r#"He said "hello""#.into()),
);
let text = format_md(&m);
let loaded = parse_md(&text).unwrap();
assert_eq!(
loaded
.model
.get_cell(&coord(&[("Type", "Food"), ("Month", "Jan")])),
Some(&CellValue::Text(r#"He said "hello""#.into())),
"Embedded double quotes were corrupted.\n{text}"
);
}
#[test]
fn text_value_with_equals_space_sequence() {
let mut m = two_cat_model();
m.model.set_cell(
coord(&[("Type", "Food"), ("Month", "Jan")]),
CellValue::Text("x = y".into()),
);
let text = format_md(&m);
let loaded = parse_md(&text).unwrap();
assert_eq!(
loaded
.model
.get_cell(&coord(&[("Type", "Food"), ("Month", "Jan")])),
Some(&CellValue::Text("x = y".into())),
"Text containing ' = ' was corrupted.\n{text}"
);
}
#[test]
fn text_value_is_single_double_quote() {
let mut m = two_cat_model();
m.model.set_cell(
coord(&[("Type", "Food"), ("Month", "Jan")]),
CellValue::Text("\"".into()),
);
let text = format_md(&m);
let loaded = parse_md(&text).unwrap();
assert_eq!(
loaded
.model
.get_cell(&coord(&[("Type", "Food"), ("Month", "Jan")])),
Some(&CellValue::Text("\"".into())),
"Single double-quote text was corrupted.\n{text}"
);
}
#[test]
fn text_value_is_empty_string() {
let mut m = two_cat_model();
m.model.set_cell(
coord(&[("Type", "Food"), ("Month", "Jan")]),
CellValue::Text("".into()),
);
let text = format_md(&m);
let loaded = parse_md(&text).unwrap();
assert_eq!(
loaded
.model
.get_cell(&coord(&[("Type", "Food"), ("Month", "Jan")])),
Some(&CellValue::Text("".into())),
"Empty string was corrupted.\n{text}"
);
}
#[test]
fn text_value_with_newline() {
let mut m = two_cat_model();
m.model.set_cell(
coord(&[("Type", "Food"), ("Month", "Jan")]),
CellValue::Text("line1\nline2".into()),
);
let text = format_md(&m);
let loaded = parse_md(&text).unwrap();
assert_eq!(
loaded
.model
.get_cell(&coord(&[("Type", "Food"), ("Month", "Jan")])),
Some(&CellValue::Text("line1\nline2".into())),
"Newline in text was corrupted.\n{text}"
);
}
#[test]
fn text_value_looks_like_number() {
let mut m = two_cat_model();
m.model.set_cell(
coord(&[("Type", "Food"), ("Month", "Jan")]),
CellValue::Text("42".into()),
);
let text = format_md(&m);
let loaded = parse_md(&text).unwrap();
assert_eq!(
loaded
.model
.get_cell(&coord(&[("Type", "Food"), ("Month", "Jan")])),
Some(&CellValue::Text("42".into())),
"Numeric-looking text was converted to Number.\n{text}"
);
}
#[test]
fn text_value_with_hash_prefix() {
let mut m = two_cat_model();
m.model.set_cell(
coord(&[("Type", "Food"), ("Month", "Jan")]),
CellValue::Text("#NotAHeader".into()),
);
let text = format_md(&m);
let loaded = parse_md(&text).unwrap();
assert_eq!(
loaded
.model
.get_cell(&coord(&[("Type", "Food"), ("Month", "Jan")])),
Some(&CellValue::Text("#NotAHeader".into())),
"Hash-prefixed text was misinterpreted.\n{text}"
);
}
#[test]
fn item_name_with_brackets_misinterpreted_as_group() {
let mut m = Workbook::new("Test");
m.add_category("Type").unwrap();
m.add_item("Type", "Item [special]").unwrap();
m.add_category("Month").unwrap();
m.add_item("Month", "Jan").unwrap();
let text = format_md(&m);
let loaded = parse_md(&text).unwrap();
let cat = loaded.model.category("Type").unwrap();
let item_names: Vec<&str> = cat.items.values().map(|i| i.name.as_str()).collect();
assert!(
item_names.contains(&"Item [special]"),
"Item name with brackets was misinterpreted as having a group.\n\
Got items: {item_names:?}\n{text}"
);
}
#[test]
fn category_name_with_comma_space_in_data() {
let mut m = Workbook::new("Test");
m.add_category("Income, Gross").unwrap();
m.add_item("Income, Gross", "A").unwrap();
m.add_category("Month").unwrap();
m.add_item("Month", "Jan").unwrap();
m.model.set_cell(
coord(&[("Income, Gross", "A"), ("Month", "Jan")]),
CellValue::Number(100.0),
);
let text = format_md(&m);
let loaded = parse_md(&text).unwrap();
assert_eq!(
loaded
.model
.get_cell(&coord(&[("Income, Gross", "A"), ("Month", "Jan")])),
Some(&CellValue::Number(100.0)),
"Category name with comma-space broke coord parsing.\n{text}"
);
}
#[test]
fn item_name_with_equals_sign_in_data() {
let mut m = Workbook::new("Test");
m.add_category("Type").unwrap();
m.add_item("Type", "A=B").unwrap();
m.add_category("Month").unwrap();
m.add_item("Month", "Jan").unwrap();
m.model.set_cell(
coord(&[("Type", "A=B"), ("Month", "Jan")]),
CellValue::Number(50.0),
);
let text = format_md(&m);
let loaded = parse_md(&text).unwrap();
assert_eq!(
loaded
.model
.get_cell(&coord(&[("Type", "A=B"), ("Month", "Jan")])),
Some(&CellValue::Number(50.0)),
"Item name with '=' broke coord parsing.\n{text}"
);
}
#[test]
fn view_name_with_parentheses() {
let mut m = Workbook::new("Test");
m.add_category("Type").unwrap();
m.add_item("Type", "A").unwrap();
m.create_view("My View (v2)");
let text = format_md(&m);
let loaded = parse_md(&text).unwrap();
assert!(
loaded.views.contains_key("My View (v2)"),
"View with parens was corrupted.\nViews: {:?}\n{text}",
loaded.views.keys().collect::<Vec<_>>()
);
}
#[test]
fn multiple_tricky_text_cells() {
let mut m = Workbook::new("EdgeCases");
m.add_category("Dim").unwrap();
for item in ["A", "B", "C", "D"] {
m.add_item("Dim", item).unwrap();
}
m.add_category("Msr").unwrap();
m.add_item("Msr", "Val").unwrap();
let cases: Vec<(&str, CellValue)> = vec![
("A", CellValue::Text("hello, world".into())),
("B", CellValue::Text(r#"a "quoted" thing"#.into())),
("C", CellValue::Text("x = y = z".into())),
("D", CellValue::Text("".into())),
];
for (item, value) in &cases {
m.model
.set_cell(coord(&[("Dim", item), ("Msr", "Val")]), value.clone());
}
let text = format_md(&m);
let loaded = parse_md(&text).unwrap();
for (item, expected) in &cases {
let got = loaded
.model
.get_cell(&coord(&[("Dim", item), ("Msr", "Val")]));
assert_eq!(
got,
Some(expected),
"Cell Dim={item} round-trip failed.\nExpected: {expected:?}\nGot: {got:?}\n{text}"
);
}
}