From c84707261c2fd9ecdcb60b093baf327d2ab9406e Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Mon, 21 Sep 2026 14:46:15 -0700 Subject: [PATCH] fix(io): five .improv persistence defects, grammar first improvise-pnf5 (P1). A cell keyed by the empty CellKey is a legal model state -- what AddRecordRow leaves while a records row is unfinished -- and format_md wrote it as " = 0" while coord_list required at least one coordinate. Saving in records mode therefore overwrote a good document with one the application could not reopen. Given a spelling rather than discarded, because the ticket is explicit that legitimate data must not be dropped: data_coords makes the coordinate list optional and the scalar cell is written "= 42". Unambiguous under PEG, since bare_name cannot start with "=". improvise-m4rv. Workbook::new seeds a Default view and on_category_added fills it, so a file naming only Plan parsed into two views -- the phantom at index 0, and appended to the user's document on any save. parse_md now drops Default when the file named at least one view and none was Default. A file naming no view, or naming Default itself, is untouched. improvise-3yv3. format_md collected every ungrouped item and emitted the whole list before any grouped one, so an interleaved category came back reordered -- and grid axis ordering consumes item insertion order (gotcha 18). Items are now written in insertion order, a run of bare items sharing a line. improvise-qu4a. View::prune_empty never reached disk. The line is spelled with a value rather than by its presence, so the grammar generator can emit either; absent means false, the compatible reading. improvise-ppru. Model::measure_agg was lost by the roundtrip. An Aggregation section carries it, sorted -- measure_agg is a HashMap and hash order is not an order (gotcha 18), so two saves of one document must be made to agree. improvise-4lgj. The three wildcard arms in the walker silently skipped any grammar rule they did not handle, so extending improv.pest caused silent data loss rather than a loud mismatch. They now bail, and a new exhaustive match over Rule names the walker that reads each one -- a new rule stops that test compiling, which is design-principles section 2's "the compiler finds every call site" applied to a pest-derived enum. It earned its keep immediately by naming Rule::section, which the first draft had missed. That arm is also what makes the change safe: the grammar-walking generator reaches both new rules -- proved by injecting a bail into each and watching generated_file_parses fail -- so an unwalked rule is now a test failure. Un-ignores the five reproductions in tests/bug_hunt_reproductions.rs. improvise-nvq4 did NOT reproduce and is kept as a passing control. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH --- crates/improvise-io/src/persistence/format.rs | 48 +++++- .../improvise-io/src/persistence/improv.pest | 27 +++- crates/improvise-io/src/persistence/parse.rs | 135 +++++++++++++++-- .../src/persistence/roundtrip_tests.rs | 141 ++++++++++++++++++ tests/bug_hunt_reproductions.rs | 27 +++- 5 files changed, 349 insertions(+), 29 deletions(-) diff --git a/crates/improvise-io/src/persistence/format.rs b/crates/improvise-io/src/persistence/format.rs index 0ccd3dd3..1997009c 100644 --- a/crates/improvise-io/src/persistence/format.rs +++ b/crates/improvise-io/src/persistence/format.rs @@ -7,6 +7,7 @@ use crate::workbook::Workbook; // The one home of the pipe-quoting rule (improvise-i2m5); a name written here // must also survive `parse_formula`. See `context/code/formula-and-io.md`. +use crate::formula::AggFunc; use crate::formula::quote::{pipe_quote, quote_name}; /// A name as a section header spells it: the whole line, so no pipes where a @@ -22,6 +23,18 @@ fn quote_header(name: &str) -> String { } } +/// The on-disk keyword for an aggregation, exhaustively — a new `AggFunc` +/// variant fails the build here rather than being written as something else. +fn agg_keyword(agg: &AggFunc) -> &'static str { + match agg { + AggFunc::Sum => "sum", + AggFunc::Avg => "avg", + AggFunc::Min => "min", + AggFunc::Max => "max", + AggFunc::Count => "count", + } +} + fn format_number(n: f64) -> String { if n.is_infinite() { return if n.is_sign_positive() { @@ -108,6 +121,9 @@ pub fn format_md(workbook: &Workbook) -> String { if !view.number_format.is_empty() { w!(out, "format: {}", view.number_format); } + if view.prune_empty { + w!(out, "prune: true"); + } for (prefix, map) in [ ("hidden", &view.hidden_items), ("collapsed", &view.collapsed_groups), @@ -137,6 +153,19 @@ pub fn format_md(workbook: &Workbook) -> String { } } + // ── Aggregation ────────────────────────────────────────────────── + + // Sorted: `measure_agg` is a `HashMap` and hash order is not an order + // (gotcha 18) — two saves of one document must agree. + let mut aggs: Vec<_> = model.measure_agg.iter().collect(); + aggs.sort_by_key(|(name, _)| name.as_str()); + if !aggs.is_empty() { + w!(out, "\n## Aggregation"); + for (name, agg) in aggs { + w!(out, "- {}: {}", quote_name(name), agg_keyword(agg)); + } + } + // ── 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 @@ -152,8 +181,9 @@ pub fn format_md(workbook: &Workbook) -> String { continue; } w!(out, "\n## Category: {}", quote_header(&cat.name)); + // Items are written in insertion order, because that is the order the + // grid's axes consume (gotcha 18). A run of bare items shares a line. let mut bare: Vec = Vec::new(); - let mut grouped: Vec = Vec::new(); for item in cat.items.values() { // For _Measure, skip items that are formula targets // (they'll be recreated from the ## Formulas section) @@ -163,16 +193,19 @@ pub fn format_md(workbook: &Workbook) -> String { continue; } match &item.group { - Some(g) => grouped.push(format!("{}[{}]", quote_name(&item.name), quote_name(g))), None => bare.push(quote_name(&item.name)), + Some(g) => { + if !bare.is_empty() { + w!(out, "- {}", bare.join(", ")); + bare.clear(); + } + w!(out, "- {}[{}]", quote_name(&item.name), quote_name(g)); + } } } 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)); @@ -190,7 +223,10 @@ pub fn format_md(workbook: &Workbook) -> String { CellValue::Number(n) => format_number(*n), CellValue::Text(s) | CellValue::Error(s) => pipe_quote(s), }; - w!(out, "{} = {}", coord_str(&key), val_str); + match coord_str(&key) { + coords if coords.is_empty() => w!(out, "= {val_str}"), + coords => w!(out, "{coords} = {val_str}"), + } } } diff --git a/crates/improvise-io/src/persistence/improv.pest b/crates/improvise-io/src/persistence/improv.pest index 8bacdb7c..094086a3 100644 --- a/crates/improvise-io/src/persistence/improv.pest +++ b/crates/improvise-io/src/persistence/improv.pest @@ -24,6 +24,7 @@ initial_view = { "Initial View: " ~ header_name ~ NEWLINE ~ blank_lines } section = _{ category_section | formulas_section + | aggregation_section | data_section | view_section } @@ -69,6 +70,19 @@ formula_line = { formula_target = _{ " "* ~ "[" ~ name ~ "]" } formula_raw = @{ (!(formula_target? ~ NEWLINE) ~ ANY)+ } +// ── Aggregation ────────────────────────────────────────────────────────────── +// +// How a `_Measure` item aggregates over the cells a grid position covers. +// Absent means `sum`, which is `Model::agg_for`'s fallback. + +aggregation_section = { + "## Aggregation" ~ NEWLINE ~ blank_lines ~ + aggregation_line* +} + +aggregation_line = { "- " ~ name ~ ": " ~ agg_kind ~ NEWLINE ~ blank_lines } +agg_kind = @{ "sum" | "avg" | "min" | "max" | "count" } + // ── Data ───────────────────────────────────────────────────────────────────── data_section = { @@ -76,11 +90,14 @@ data_section = { data_line* } +// A cell with NO coordinates is a legal model state — the scalar cell, and the +// placeholder an unfinished records row leaves. It is spelled `= 42`. data_line = { - coord_list ~ " = " ~ cell_value ~ NEWLINE ~ blank_lines + data_coords ~ "= " ~ cell_value ~ NEWLINE ~ blank_lines } -coord_list = { coord ~ (", " ~ coord)* } +data_coords = _{ (coord_list ~ " ")? } +coord_list = { coord ~ (", " ~ coord)* } coord = { name ~ "=" ~ name } cell_value = _{ number | pipe_quoted | bare_value } @@ -98,7 +115,7 @@ view_section = { view_entry* } -view_entry = _{ format_line | hidden_line | collapsed_line | axis_line } +view_entry = _{ format_line | prune_line | hidden_line | collapsed_line | axis_line } axis_line = { name ~ ": " ~ axis_kind ~ (", " ~ name)? ~ NEWLINE ~ blank_lines @@ -107,6 +124,10 @@ axis_line = { axis_kind = @{ "row" | "column" | "page" | "filter" | "none" } format_line = { "format: " ~ rest_of_line ~ NEWLINE ~ blank_lines } +// Absent means false — the compatible reading for a file written before it +// existed, and what the writer emits for a view that does not prune. +prune_line = { "prune: " ~ bool_kw ~ NEWLINE ~ blank_lines } +bool_kw = @{ "true" | "false" } hidden_line = { "hidden: " ~ name ~ "/" ~ name ~ NEWLINE ~ blank_lines } collapsed_line = { "collapsed: " ~ name ~ "/" ~ name ~ NEWLINE ~ blank_lines } diff --git a/crates/improvise-io/src/persistence/parse.rs b/crates/improvise-io/src/persistence/parse.rs index 156cbf90..963ed3b0 100644 --- a/crates/improvise-io/src/persistence/parse.rs +++ b/crates/improvise-io/src/persistence/parse.rs @@ -6,7 +6,7 @@ use pest::Parser; use pest::iterators::{Pair, Pairs}; use pest_derive::Parser; -use crate::formula::parse_formula; +use crate::formula::{AggFunc, parse_formula}; use crate::model::category::Group; use crate::model::cell::{CellKey, CellValue}; use crate::view::Axis; @@ -35,6 +35,7 @@ struct PView { axes: Vec<(String, Axis)>, page_selections: Vec<(String, String)>, format: String, + prune_empty: bool, hidden: Vec<(String, String)>, collapsed: Vec<(String, String)>, } @@ -46,6 +47,7 @@ struct ParsedFile { initial_view: Option, categories: Vec, formulas: Vec<(String, String)>, + measure_agg: Vec<(String, AggFunc)>, data: Vec<(CellKey, CellValue)>, views: Vec, } @@ -177,12 +179,39 @@ fn walk_category_section(pair: Pair<'_, Rule>) -> Result { Rule::group_hierarchy => { pc.group_parents.push(extract_name_pair(entry)?); } - _ => {} + other => bail!("unhandled rule in category_section: {other:?}"), } } Ok(pc) } +/// The on-disk keyword for an aggregation, exhaustively — the reader's half of +/// `format.rs::agg_keyword`. +fn parse_agg_kind(kind: &str) -> Result { + match kind { + "sum" => Ok(AggFunc::Sum), + "avg" => Ok(AggFunc::Avg), + "min" => Ok(AggFunc::Min), + "max" => Ok(AggFunc::Max), + "count" => Ok(AggFunc::Count), + _ => bail!("Unknown aggregation: {kind}"), + } +} + +/// `## Aggregation`: `- Measure: avg` lines as `(item, AggFunc)` pairs. +fn walk_aggregation_section(pair: Pair<'_, Rule>) -> Result> { + let mut out = Vec::new(); + for line in pair.into_inner() { + if line.as_rule() == Rule::aggregation_line { + let mut parts = line.into_inner(); + let name = extract_name(next(&mut parts, "aggregation_line")?)?; + let kind = next(&mut parts, "aggregation_line")?; + out.push((name, parse_agg_kind(kind.as_str())?)); + } + } + Ok(out) +} + /// `## Formulas`: `(raw text, target category)` pairs. A line with no /// `[Category]` suffix targets `_Measure`. fn walk_formulas_section(pair: Pair<'_, Rule>) -> Result> { @@ -231,21 +260,28 @@ fn walk_data_section(pair: Pair<'_, Rule>) -> Result> for dl in pair.into_inner() { if dl.as_rule() == Rule::data_line { let mut dl_inner = dl.into_inner(); - let coord_list = next(&mut dl_inner, "data_line coords")?; - let value_pair = next(&mut dl_inner, "data_line value")?; - - let coords: Vec<_> = coord_list - .into_inner() - .filter(|p| p.as_rule() == Rule::coord) - .map(extract_name_pair) - .collect::>()?; - + let first = next(&mut dl_inner, "data_line")?; + // `coord_list` is optional in the grammar: a cell with no + // coordinates is the scalar cell, spelled `= 42` (improvise-pnf5). + let (coords, value_pair) = match first.as_rule() { + Rule::coord_list => (walk_coord_list(first)?, next(&mut dl_inner, "data_line")?), + _ => (Vec::new(), first), + }; out.push((CellKey::new(coords), walk_cell_value(value_pair)?)); } } Ok(out) } +/// The `Cat=Item, …` prefix of a data line. +fn walk_coord_list(coord_list: Pair<'_, Rule>) -> Result> { + coord_list + .into_inner() + .filter(|p| p.as_rule() == Rule::coord) + .map(extract_name_pair) + .collect() +} + /// The keyword after `Category:` on an axis line. fn parse_axis_kind(kind_str: &str) -> Result { match kind_str { @@ -281,6 +317,7 @@ fn walk_view_section(pair: Pair<'_, Rule>) -> Result { axes: Vec::new(), page_selections: Vec::new(), format: String::new(), + prune_empty: false, hidden: Vec::new(), collapsed: Vec::new(), }; @@ -288,9 +325,10 @@ fn walk_view_section(pair: Pair<'_, Rule>) -> Result { match entry.as_rule() { Rule::axis_line => walk_axis_line(entry, &mut pv)?, Rule::format_line => pv.format = first_str(entry)?, + Rule::prune_line => pv.prune_empty = first_str(entry)? == "true", Rule::hidden_line => pv.hidden.push(extract_name_pair(entry)?), Rule::collapsed_line => pv.collapsed.push(extract_name_pair(entry)?), - _ => {} + other => bail!("unhandled rule in view_section: {other:?}"), } } Ok(pv) @@ -310,9 +348,10 @@ fn walk_file(file: Pair<'_, Rule>) -> Result { } Rule::category_section => parsed.push_category(walk_category_section(pair)?), Rule::formulas_section => parsed.formulas.extend(walk_formulas_section(pair)?), + Rule::aggregation_section => parsed.measure_agg.extend(walk_aggregation_section(pair)?), Rule::data_section => parsed.data.extend(walk_data_section(pair)?), Rule::view_section => parsed.views.push(walk_view_section(pair)?), - _ => {} + other => bail!("unhandled rule in file: {other:?}"), } } Ok(parsed) @@ -367,6 +406,7 @@ fn build_view(wb: &mut Workbook, pv: &PView) -> Result<()> { if !pv.format.is_empty() { view.number_format = pv.format.clone(); } + view.prune_empty = pv.prune_empty; for (cat, item) in &pv.hidden { view.hide_item(cat, item); } @@ -376,6 +416,16 @@ fn build_view(wb: &mut Workbook, pv: &PView) -> Result<()> { Ok(()) } +/// `Workbook::new` seeds a `Default` view and `on_category_added` fills it, so +/// a file naming its own views gained a phantom one it never had (improvise-m4rv). +fn drop_unnamed_default(wb: &mut Workbook, named: &[&PView]) { + let default_is_a_phantom = + !named.is_empty() && !named.iter().any(|pv| pv.name == crate::view::DEFAULT_VIEW); + if default_is_a_phantom { + wb.delete_view(crate::view::DEFAULT_VIEW).ok(); + } +} + /// Turn pass 1's collectors into a [`Workbook`]. fn build_workbook(parsed: ParsedFile) -> Result { let name = parsed @@ -389,13 +439,19 @@ fn build_workbook(parsed: ParsedFile) -> Result { // A file written before improvise-60z may carry the scratch views; drop // them, and `initial_view` then falls through to the default. - for pv in parsed + let named: Vec<&PView> = parsed .views .iter() .filter(|pv| !crate::view::VIRTUAL_VIEW_NAMES.contains(&pv.name.as_str())) - { + .collect(); + for pv in &named { build_view(&mut wb, pv)?; } + drop_unnamed_default(&mut wb, &named); + + for (item, agg) in parsed.measure_agg { + wb.model.measure_agg.insert(item, agg); + } if let Some(iv) = &parsed.initial_view && wb.views.contains_key(iv) @@ -420,6 +476,55 @@ pub fn parse_md(text: &str) -> Result { build_workbook(walk_file(parse_tree(text)?)?) } +#[cfg(test)] +mod grammar_coverage { + use super::Rule; + + /// Which walker reads each grammar rule. A new rule in `improv.pest` adds a + /// `Rule` variant and this match stops compiling — improvise-4lgj. + fn walker_of(rule: Rule) -> &'static str { + match rule { + Rule::file | Rule::EOI => "walk_file", + Rule::version_line => "walk_file (skipped: the version is the literal)", + Rule::model_name | Rule::initial_view => "walk_file", + Rule::section => "silent: hoisted into file", + Rule::category_section => "walk_category_section", + Rule::item_list | Rule::grouped_item | Rule::group_hierarchy => "walk_category_section", + Rule::category_entry => "silent: hoisted into category_section", + Rule::formulas_section | Rule::formula_line | Rule::formula_raw => { + "walk_formulas_section" + } + Rule::formula_target => "silent: hoisted into formula_line", + Rule::aggregation_section | Rule::aggregation_line | Rule::agg_kind => { + "walk_aggregation_section" + } + Rule::data_section | Rule::data_line => "walk_data_section", + Rule::data_coords => "silent: hoisted into data_line", + Rule::coord_list | Rule::coord => "walk_coord_list", + Rule::cell_value => "silent: hoisted into data_line", + Rule::number | Rule::bare_value => "walk_cell_value", + Rule::view_section => "walk_view_section", + Rule::axis_line | Rule::axis_kind => "walk_axis_line", + Rule::format_line | Rule::prune_line | Rule::bool_kw => "walk_view_section", + Rule::hidden_line | Rule::collapsed_line => "walk_view_section", + Rule::view_entry => "silent: hoisted into view_section", + Rule::name => "silent: hoisted into its parent", + Rule::pipe_quoted | Rule::pipe_inner | Rule::bare_name => "extract_name", + Rule::header_name | Rule::line_name => "header_name", + Rule::rest_of_line => "first_str", + Rule::blank_lines => "silent: whitespace", + } + } + + /// The compile is the test: this call only type-checks while every `Rule` + /// variant is classified above. The assertions are a smoke check. + #[test] + fn every_grammar_rule_names_the_walker_that_reads_it() { + assert_eq!(walker_of(Rule::prune_line), "walk_view_section"); + assert_eq!(walker_of(Rule::coord_list), "walk_coord_list"); + } +} + #[cfg(test)] mod tests { use super::parse_md; diff --git a/crates/improvise-io/src/persistence/roundtrip_tests.rs b/crates/improvise-io/src/persistence/roundtrip_tests.rs index 9f4f80e1..e85b180c 100644 --- a/crates/improvise-io/src/persistence/roundtrip_tests.rs +++ b/crates/improvise-io/src/persistence/roundtrip_tests.rs @@ -920,3 +920,144 @@ fn a_bare_item_name_with_a_space_is_rejected() { "precondition: the same line quoted must parse" ); } + +// ── prune_empty (improvise-qu4a) ───────────────────────────────────────── + +/// The FLAG is asserted, not the output's stability: both sides dropped it. +/// Evidence: `context/test-evidence/formula-and-io.md`. +#[test] +fn prune_empty_survives_the_roundtrip() { + let mut wb = two_cat_model(); + wb.active_view_mut().prune_empty = true; + let text = format_md(&wb); + assert!(text.contains("prune: true"), "no prune line:\n{text}"); + assert!(parse_md(&text).unwrap().active_view().prune_empty); +} + +/// The direction the default supplies, which is why it is the discriminating +/// one to state separately: a writer that always emits `prune: true` fails it. +#[test] +fn a_view_that_does_not_prune_writes_no_prune_line() { + let mut wb = two_cat_model(); + wb.active_view_mut().prune_empty = false; + let text = format_md(&wb); + assert!(!text.contains("prune:"), "unexpected prune line:\n{text}"); + assert!(!parse_md(&text).unwrap().active_view().prune_empty); +} + +/// An older file carries no `prune:` line; `false` is the compatible reading. +#[test] +fn a_file_without_a_prune_line_does_not_prune() { + let text = "v2025-04-09\n# T\n\n## View: Default\n\n## Category: C\n- a\n"; + assert!(!parse_md(text).unwrap().active_view().prune_empty); +} + +// ── The seeded Default view (improvise-m4rv) ───────────────────────────── + +/// A file that names its own views must not gain the one `Workbook::new` +/// seeds. Evidence: `context/test-evidence/formula-and-io.md`. +#[test] +fn a_file_naming_its_own_views_gains_no_default() { + let text = "v2025-04-09\n# T\nInitial View: Plan\n\n## View: Plan\n\n## Category: C\n- a\n"; + let wb = parse_md(text).unwrap(); + assert_eq!(wb.views.keys().collect::>(), vec!["Plan"]); + assert_eq!(wb.active_view, "Plan"); +} + +/// The control the fix must not break: a file naming NO view still opens on +/// the seeded one, and a file that names `Default` itself keeps it. +#[test] +fn a_file_naming_no_view_keeps_the_seeded_default() { + let text = "v2025-04-09\n# T\n\n## Category: C\n- a\n"; + let wb = parse_md(text).unwrap(); + assert_eq!(wb.views.keys().collect::>(), vec!["Default"]); + + let named = "v2025-04-09\n# T\n\n## View: Default\n\n## Category: C\n- a\n"; + assert_eq!( + parse_md(named).unwrap().views.keys().collect::>(), + vec!["Default"] + ); +} + +// ── Aggregation (improvise-ppru) ───────────────────────────────────────── + +/// Two saves of one document must agree: `measure_agg` is a `HashMap`, and +/// hash order is not an order (gotcha 18). +#[test] +fn the_aggregation_section_is_written_in_a_stable_order() { + let mut wb = Workbook::new("T"); + for (item, agg) in [ + ("Zebra", crate::formula::AggFunc::Min), + ("Apple", crate::formula::AggFunc::Count), + ("Mango", crate::formula::AggFunc::Avg), + ] { + wb.add_item("_Measure", item).unwrap(); + wb.model.measure_agg.insert(item.into(), agg); + } + let text = format_md(&wb); + let lines: Vec<&str> = text + .lines() + .skip_while(|l| *l != "## Aggregation") + .skip(1) + .take(3) + .collect(); + assert_eq!( + lines, + vec!["- Apple: count", "- Mango: avg", "- Zebra: min"] + ); + assert_eq!(format_md(&parse_md(&text).unwrap()), text); +} + +// ── Interleaved items (improvise-3yv3) ─────────────────────────────────── + +/// The writer emits items in insertion order, splitting a bare run where a +/// grouped item interrupts it. Evidence: `context/test-evidence/formula-and-io.md`. +#[test] +fn an_interleaved_category_keeps_its_item_order() { + let mut wb = Workbook::new("T"); + wb.add_category("C").unwrap(); + for name in ["a", "b"] { + wb.add_item("C", name).unwrap(); + } + wb.add_item_in_group("C", "g1", "G").unwrap(); + wb.add_item("C", "c").unwrap(); + wb.add_item_in_group("C", "g2", "G").unwrap(); + + let text = format_md(&wb); + let body: Vec<&str> = text + .lines() + .skip_while(|l| *l != "## Category: C") + .skip(1) + .take(4) + .collect(); + assert_eq!(body, vec!["- a, b", "- g1[G]", "- c", "- g2[G]"]); + assert_eq!( + parse_md(&text) + .unwrap() + .model + .category("C") + .unwrap() + .ordered_item_names(), + wb.model.category("C").unwrap().ordered_item_names() + ); +} + +// ── The scalar cell (improvise-pnf5) ───────────────────────────────────── + +/// A cell with no coordinates is a legal model state and now has a spelling. +/// Evidence: `context/test-evidence/formula-and-io.md`. +#[test] +fn a_cell_with_no_coordinates_round_trips() { + use crate::model::cell::CellKey; + let mut wb = two_cat_model(); + wb.model + .set_cell(CellKey::new(vec![]), CellValue::Number(42.0)); + let text = format_md(&wb); + assert!(text.contains("\n= 42\n"), "expected a scalar line:\n{text}"); + let back = parse_md(&text).unwrap(); + assert_eq!( + back.model.data.get(&CellKey::new(vec![])), + Some(&CellValue::Number(42.0)) + ); + assert_eq!(format_md(&back), text); +} diff --git a/tests/bug_hunt_reproductions.rs b/tests/bug_hunt_reproductions.rs index 968d87e3..d7836ebf 100644 --- a/tests/bug_hunt_reproductions.rs +++ b/tests/bug_hunt_reproductions.rs @@ -8,7 +8,6 @@ use improvise_io::{ workbook::Workbook, }; #[test] -#[ignore = "improvise-3yv3: known bug reproduction"] fn grouped_item_order_survives_save() { let mut w = Workbook::new("test"); w.add_category("C").unwrap(); @@ -21,7 +20,6 @@ fn grouped_item_order_survives_save() { ); } #[test] -#[ignore = "improvise-ppru: known bug reproduction"] fn aggregation_survives_save() { let mut w = Workbook::new("test"); w.add_item("_Measure", "Price").unwrap(); @@ -34,7 +32,6 @@ fn aggregation_survives_save() { ); } #[test] -#[ignore = "improvise-m4rv: known bug reproduction"] fn deleted_default_stays_deleted() { let mut w = Workbook::new("test"); w.create_view("Custom"); @@ -203,7 +200,6 @@ fn literal_cr_name_survives_roundtrip() { ); } #[test] -#[ignore = "improvise-pnf5: known bug reproduction"] fn scalar_cell_survives_roundtrip() { use improvise_io::model::cell::{CellKey, CellValue}; let mut w = Workbook::new("test"); @@ -314,7 +310,6 @@ fn formula_panel_handles_zero_inner_width() { } #[test] -#[ignore = "improvise-pnf5: known bug reproduction"] fn save_with_unfinished_record_is_readable() { use improvise::{ command::{ @@ -417,3 +412,25 @@ fn ordinary_controls() { ); assert!(parse_md(&format_md(&w)).is_ok()); } + +// Passing control (improvise-nvq4): quote_name already pipe-quotes a name +// containing `/`, so hidden/collapsed round-trip. The report did not reproduce. +#[test] +fn slash_in_hidden_name_survives_roundtrip() { + let mut w = Workbook::new("test"); + w.add_category("C").unwrap(); + w.add_item("C", "A/B").unwrap(); + w.add_item_in_group("C", "x", "G/H").unwrap(); + w.active_view_mut().hide_item("C", "A/B"); + w.active_view_mut().toggle_group_collapse("C", "G/H"); + let text = format_md(&w); + let r = parse_md(&text).unwrap_or_else(|e| panic!("unparseable: {e}\n{text}")); + assert!( + r.active_view().is_hidden("C", "A/B"), + "hidden lost:\n{text}" + ); + assert!( + r.active_view().is_group_collapsed("C", "G/H"), + "collapsed lost:\n{text}" + ); +}