improvise-kb1d: refuse renaming a formula outside _Measure
A regular category's formula target is a real item, and replace_formula added the new target without removing the old, stranding it as an empty item and appending the new one. Until formulas have a stable identity (improvise-sboz), a target change outside _Measure is refused and the editor stays open. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
@@ -4541,4 +4541,39 @@ mod formula_edit_tests {
|
||||
vec![PROFIT, TAX, MARGIN, "Levy = Revenue * 0.3"]
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-kb1d: a Region-targeted formula at index 1. Renaming it would
|
||||
/// strand `Total` as an empty item, so the edit refuses and stays open.
|
||||
#[test]
|
||||
fn renaming_a_formula_outside_measure_refuses_and_keeps_the_editor() {
|
||||
let mut wb = Workbook::new("T");
|
||||
wb.add_item("_Measure", "Revenue").unwrap();
|
||||
wb.add_item("_Measure", "Cost").unwrap();
|
||||
wb.add_category("Region").unwrap();
|
||||
wb.add_item("Region", "East").unwrap();
|
||||
wb.add_item("Region", "West").unwrap();
|
||||
for (raw, cat) in [
|
||||
(PROFIT, "_Measure"),
|
||||
("Total = East + West", "Region"),
|
||||
(MARGIN, "_Measure"),
|
||||
] {
|
||||
wb.model.add_formula(parse_formula(raw, cat).unwrap());
|
||||
}
|
||||
let mut app = App::new(wb, None);
|
||||
app.view_state.formula_panel_open = true;
|
||||
app.view_state.mode = AppMode::FormulaPanel;
|
||||
app.view_state.formula_cursor = 1;
|
||||
let regions = |app: &App| {
|
||||
let region = app.model_state.workbook.model.category("Region").unwrap();
|
||||
region.ordered_item_names().iter().map(|s| s.to_string()).collect::<Vec<_>>()
|
||||
};
|
||||
let before = regions(&app);
|
||||
open_editor(&mut app);
|
||||
retype(&mut app, "Sum = East + West");
|
||||
press(&mut app, KeyCode::Enter);
|
||||
assert_eq!(raws(&app), vec![PROFIT, "Total = East + West", MARGIN]);
|
||||
assert_eq!(regions(&app), before, "no ghost item, nothing appended");
|
||||
assert!(in_editor(&app), "{:?}", app.view_state.mode);
|
||||
assert_eq!(buffer(&app), Some("Sum = East + West"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -425,7 +425,14 @@ impl Model {
|
||||
/// let a new target displace another formula — `context/code/core.md` §18.
|
||||
pub fn replace_formula(&mut self, id: &FormulaId, formula: Formula) -> Result<()> {
|
||||
let new_id = formula.id();
|
||||
if new_id != *id && self.formula(&new_id).is_some() {
|
||||
let renames = new_id != *id;
|
||||
// A regular category's target is a real item; renaming it is sboz's (kb1d).
|
||||
if renames && !CategoryKind::of_name(&formula.target_category).is_measure() {
|
||||
return Err(anyhow!(
|
||||
"Renaming a formula outside _Measure is not supported yet; delete and re-add it"
|
||||
));
|
||||
}
|
||||
if renames && self.formula(&new_id).is_some() {
|
||||
return Err(anyhow!("A formula for {} already exists", formula.target));
|
||||
}
|
||||
let pos = self
|
||||
@@ -1935,6 +1942,27 @@ mod formula_tests {
|
||||
assert_eq!(targets(&m), vec!["Profit", "Margin"]);
|
||||
}
|
||||
|
||||
/// improvise-kb1d: a Region-targeted formula at index 1. A rename would
|
||||
/// leave `Total` as an empty item and append `Sum`, so it is refused.
|
||||
#[test]
|
||||
fn replace_formula_refuses_to_rename_a_target_outside_measure() {
|
||||
let mut m = three_formulas();
|
||||
m.formulas.insert(1, parse_formula("Total = East + West", "Region").unwrap());
|
||||
let shape = |m: &Model| {
|
||||
let region = m.category("Region").unwrap().ordered_item_names();
|
||||
let owned: Vec<String> = region.iter().map(|s| s.to_string()).collect();
|
||||
(targets(m).iter().map(|s| s.to_string()).collect::<Vec<_>>(), owned)
|
||||
};
|
||||
let before = shape(&m);
|
||||
let id = m.formulas[1].id();
|
||||
let sum = parse_formula("Sum = East + West", "Region").unwrap();
|
||||
assert!(m.replace_formula(&id, sum).is_err());
|
||||
assert_eq!(shape(&m), before, "nothing moved, nothing added");
|
||||
let edit = parse_formula("Total = East - West", "Region").unwrap();
|
||||
m.replace_formula(&id, edit).unwrap();
|
||||
assert_eq!(targets(&m), vec!["Profit", "Total", "Tax", "Margin"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_formula_mentions_what_its_expression_or_its_where_names() {
|
||||
let margin = parse_formula("Margin = Profit / Revenue", "_Measure").unwrap();
|
||||
|
||||
Reference in New Issue
Block a user