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

WIP improvise-tej1: compose delete-formula-at-cursor in the browser

Browse Source 
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:
Edward Langley
2026-09-23 07:47:29 -07:00
parent 77f059c852
commit 3a56daa509
9 changed files with 134 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/improvise-command/src/command/cmd/effect_cmds.rs
+11 -5
View File
@@ -251,11 +251,17 @@ effect_cmd!(
RemoveFormulaCmd,
"remove-formula",
|args: &[String]| require_args(args, 2),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::RemoveFormula {
target_category: args[0].clone(),
target: args[1].clone(),
})]
|args: &Vec<String>, ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
let (category, target) = (&args[0], &args[1]);
let named = ctx
.model
.formulas()
.iter()
.find(|f| &f.target_category == category && &f.target == target);
match named {
Some(f) => vec![Box::new(effect::RemoveFormula { id: f.id() })],
None => vec![effect::refuse(format!("No formula for {target} in {category}"))],
}
}
);
crates/improvise-command/src/command/cmd/panel.rs
+3 -5
View File
@@ -402,7 +402,7 @@ mod tests {
let effects = DeleteFormulaAtCursor.execute(&ctx);
let dbg = effects_debug(&effects);
assert!(
dbg.contains("RemoveFormula { target: \"Tax\", target_category: \"Type\" }"),
dbg.contains("target: \"Tax\", target_category: \"Type\""),
"must remove the formula AT THE CURSOR (Tax, index 1), got: {dbg}"
);
assert!(
@@ -668,10 +668,8 @@ impl Cmd for DeleteFormulaAtCursor {
let cursor = ctx.formula_cursor.min(formulas.len().saturating_sub(1));
if cursor < formulas.len() {
let f = &formulas[cursor];
let mut effects: Vec<Box<dyn Effect>> = vec![Box::new(effect::RemoveFormula {
target: f.target.clone(),
target_category: f.target_category.clone(),
})];
let mut effects: Vec<Box<dyn Effect>> =
vec![Box::new(effect::RemoveFormula { id: f.id() })];
if cursor > 0 {
effects.push(Box::new(effect::SetPanelCursor {
panel: Panel::Formula,
crates/improvise-command/src/ui/effect.rs
+9 -12
View File
@@ -231,17 +231,17 @@ impl Effect for AddFormula {
}
}
/// Remove the formula `id` names. A formula already gone is refused and stops
/// the batch, so a frame that named it is rolled back (improvise-tej1).
#[derive(Debug)]
pub struct RemoveFormula {
pub target: String,
pub target_category: String,
pub id: FormulaId,
}
impl Effect for RemoveFormula {
fn apply(&self, app: &mut App) {
app.model_state
.workbook
.model
.remove_formula(&self.target, &self.target_category);
if let Err(e) = app.model_state.workbook.model.delete_formula(&self.id) {
refuse_and_stop(app, e.to_string());
}
}
fn kind(&self) -> EffectKind {
EffectKind::Model(ModelChange::CellData)
@@ -1914,8 +1914,7 @@ mod tests {
.apply(&mut app);
assert!(!app.model_state.workbook.model.formulas().is_empty());
RemoveFormula {
target: "Clothing".to_string(),
target_category: "Type".to_string(),
id: crate::formula::parse_formula("Clothing = Food * 2", "Type").unwrap().id(),
}
.apply(&mut app);
assert!(app.model_state.workbook.model.formulas().is_empty());
@@ -2890,8 +2889,7 @@ mod tests {
}),
("RemoveFormula", || {
Box::new(RemoveFormula {
target: "Profit".into(),
target_category: "_Measure".into(),
id: crate::formula::parse_formula("Profit = Revenue - Cost", "_Measure").unwrap().id(),
})
}),
];
@@ -2914,8 +2912,7 @@ mod tests {
}
if label == "AddFormula" {
app.apply_effects(vec![Box::new(RemoveFormula {
target: "Profit".into(),
target_category: "_Measure".into(),
id: crate::formula::parse_formula("Profit = Revenue - Cost", "_Measure").unwrap().id(),
})]);
}
app.model_state.dirty = false;
crates/improvise-command/src/ui/project.rs
+1 -2
View File
@@ -3067,8 +3067,7 @@ mod tests {
target_category: "_Measure".into(),
},
ModelCommand::RemoveFormula {
target: "Profit".into(),
target_category: "_Measure".into(),
id: crate::formula::parse_formula("Profit = Revenue", "_Measure").unwrap().id(),
},
]
}
crates/improvise-command/src/ui/reduce.rs
+2 -9
View File
@@ -171,13 +171,7 @@ pub fn model_effects(app: &App, cmd: &ModelCommand) -> Vec<Box<dyn Effect>> {
raw: raw.clone(),
target_category: target_category.clone(),
}),
ModelCommand::RemoveFormula {
target,
target_category,
} => Box::new(effect::RemoveFormula {
target: target.clone(),
target_category: target_category.clone(),
}),
ModelCommand::RemoveFormula { id } => Box::new(effect::RemoveFormula { id: id.clone() }),
ModelCommand::ReplaceFormula { id, was, raw } => Box::new(effect::ReplaceFormula {
id: id.clone(),
was: was.clone(),
@@ -687,8 +681,7 @@ pub(crate) mod tests {
target_category: "_Measure".into(),
},
ModelCommand::RemoveFormula {
target: "Profit".into(),
target_category: "_Measure".into(),
id: crate::formula::parse_formula("Profit = Revenue", "_Measure").unwrap().id(),
},
ModelCommand::ReplaceFormula {
id: crate::formula::parse_formula("Profit = Revenue", "_Measure")
crates/improvise-core/src/model/types.rs
+22
View File
@@ -417,6 +417,18 @@ impl Model {
&self.formulas
}
/// Remove the formula `id` names, refusing if none does — a delete that
/// finds nothing is a stale claim, not a no-op (improvise-tej1).
pub fn delete_formula(&mut self, id: &FormulaId) -> Result<()> {
let pos = self
.formulas
.iter()
.position(|f| f.id() == *id)
.ok_or_else(|| anyhow!("That formula no longer exists"))?;
self.formulas.remove(pos);
Ok(())
}
pub fn formula(&self, id: &FormulaId) -> Option<&Formula> {
self.formulas.iter().find(|f| f.id() == *id)
}
@@ -1935,6 +1947,16 @@ mod formula_tests {
assert_eq!(targets(&m), vec!["Profit", "Margin"]);
}
#[test]
fn delete_formula_removes_the_one_its_id_names_and_refuses_a_stale_id() {
let mut m = three_formulas();
let tax = m.formulas[1].id();
m.delete_formula(&tax).unwrap();
assert_eq!(targets(&m), vec!["Profit", "Margin"]);
assert!(m.delete_formula(&tax).is_err(), "a second delete is stale");
assert_eq!(targets(&m), vec!["Profit", "Margin"]);
}
#[test]
fn a_formula_mentions_what_its_expression_or_its_where_names() {
let margin = parse_formula("Margin = Profit / Revenue", "_Measure").unwrap();
crates/improvise-protocol/src/reduce.rs
+3 -6
View File
@@ -103,9 +103,9 @@ pub enum ModelCommand {
raw: String,
target_category: String,
},
/// Remove the formula `id` names; refused if it is already gone.
RemoveFormula {
target: String,
target_category: String,
id: FormulaId,
},
/// Edit the formula `id` names in place, if it still reads `was` — an edit
/// of that formula, not a remove-then-add (improvise-km71).
@@ -628,10 +628,7 @@ mod tests {
raw,
target_category,
} => name(raw) + name(target_category),
ModelCommand::RemoveFormula {
target,
target_category,
} => name(target) + name(target_category),
ModelCommand::RemoveFormula { id } => formula(id),
ModelCommand::ReplaceFormula { id, was, raw } => formula(id) + name(was) + name(raw),
ModelCommand::CleanEmptyRecords | ModelCommand::SortData => 0,
ModelCommand::AddCategory(category)
crates/improvise-protocol/src/resolve.rs
+24 -2
View File
@@ -339,7 +339,6 @@ fn declined(name: &str, args: &[String]) -> Option<String> {
"toggle-prune-empty" => "hiding empty rows and columns",
"toggle-records-mode" => "records mode",
"add-record-row" => "adding a record row",
"delete-formula-at-cursor" => "deleting a formula",
// Reachable only behind a prefix, and a chord must be answered or it
// strands its own disarm — improvise-hs0u, `context/code/clients.md`.
"toggle-col-group-under-cursor" => "collapsing a column group",
@@ -474,6 +473,7 @@ fn composed(
}),
// ── the formula panel ────────────────────────────────────────
"edit-formula-at-cursor" => Some(edit_formula_at(state, state.view.formula_cursor)),
"delete-formula-at-cursor" => delete_formula_at_cursor(state),
"open-item-add-at-cursor" => Some(match cat_at_cursor(state) {
Some(cat) => vec![Command::ChangeMode(AppMode::item_add(cat))],
@@ -1229,6 +1229,29 @@ fn switch_view_at(state: &ClientState, index: usize) -> Option<Vec<Command>> {
])
}
/// `DeleteFormulaAtCursor::execute`: the cursor clamped to the list, the
/// formula named by its id, and the step back only away from the top.
fn delete_formula_at_cursor(state: &ClientState) -> Option<Vec<Command>> {
let formulas = &state.formulas.formulas;
let cursor = state.view.formula_cursor.min(formulas.len().saturating_sub(1));
let Some(entry) = formulas.get(cursor) else {
return Some(Vec::new());
};
let mut commands = vec![model(
state,
ModelCommand::RemoveFormula {
id: entry.id.clone(),
},
)?];
if cursor > 0 {
commands.push(Command::SetPanelCursor {
panel: Panel::Formula,
cursor: cursor - 1,
});
}
Some(commands)
}
/// `DeleteViewAtCursor::execute`, including the cursor step back that only
/// happens away from the top of the list.
fn delete_view_at_cursor(state: &ClientState) -> Option<Vec<Command>> {
@@ -2364,7 +2387,6 @@ mod tests {
"NORMAL: n search-navigate Silent",
"NORMAL: z toggle-group-under-cursor Silent",
// Honestly unavailable, and saying so in `declined`'s words.
"FORMULAS: Del/d delete-formula-at-cursor Declines",
"NORMAL: > drill-into-cell Declines",
"NORMAL: : enter-mode Declines",
"NORMAL: P toggle-prune-empty Declines",
crates/improvise-wasm-client/tests/lockstep.rs
+59
View File
@@ -2043,3 +2043,62 @@ fn renaming_a_formula_from_the_panel_lands_the_same_document_as_the_host() {
assert_eq!(client.view.status_msg, "Formula updated");
assert_eq!(app.view_state.status_msg, "Renamed Tax to Levy");
}
/// `d` in the formula panel, at a NONZERO cursor over three formulas: the
/// client names the formula by its id, and both ends step the cursor back.
#[test]
fn deleting_a_formula_from_the_panel_agrees_with_the_host() {
let mut app = furnished_host();
add_formulas(&mut app);
app.view_state.formula_panel_open = true;
app.view_state.mode = AppMode::FormulaPanel;
app.view_state.formula_cursor = 1;
let mut twin = twin_of(&app);
add_formulas(&mut twin);
let mut client = furnished_client(&app, (2, 1));
client.formulas = improvise_command::ui::project::formulas_of(&app);
both_ends(&mut app, &mut twin, &mut client, "d");
assert_eq!(
raws(&app),
vec![FORMULAS[0], FORMULAS[2]],
"`d` must remove the formula under cursor 1, not the first or the last"
);
assert_eq!(app.view_state.formula_cursor, 0, "and step the cursor back");
}
/// The id the client names was deleted by someone else first: the frame is
/// refused whole and the document keeps what it had.
#[test]
fn deleting_a_formula_another_client_already_removed_is_refused_cleanly() {
let mut app = furnished_host();
add_formulas(&mut app);
app.view_state.formula_panel_open = true;
app.view_state.mode = AppMode::FormulaPanel;
app.view_state.formula_cursor = 1;
let mut client = furnished_client(&app, (2, 1));
client.formulas = improvise_command::ui::project::formulas_of(&app);
let event = BrowserKey::new("d").to_key_event().unwrap();
let KeyOutcome::Local(cmds) = resolve(&client, event) else {
panic!("`d` in the formula panel resolves here");
};
let mut host = improvise_command::ui::host::Host::new(app);
let (id, _) = host.connect(2, 1);
let model = &mut host.app_mut().model_state.workbook.model;
model.remove_formula("Tax", "_Measure");
let before = document_text(host.app());
let replies = host.handle(id, ClientFrame::Commands(cmds));
assert!(
replies.iter().any(|a| matches!(
a.msg,
improvise_protocol::ServerFrame::Rejected(
improvise_protocol::Rejection::MutationRefused
)
)),
"a delete of a formula that no longer exists must be refused: {replies:?}"
);
assert_eq!(document_text(host.app()), before, "nothing else was removed");
assert_eq!(raws(host.app()), vec![FORMULAS[0], FORMULAS[2]]);
}