Merge #274's head (3dad3b09, improvise-3h9w) into fix/joed-waiting-commit-keeps-text

refused_here now skips the commit keys for a captured editor. The test
file was rebuilt as #278's tests plus #274's new block, because git's
hunks split a function.

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 20:14:23 -07:00
4 changed files with 105 additions and 1 deletions
+8
View File
@@ -3064,6 +3064,14 @@ held window is `None`, `settle_opened` simply tries again on the next fold, and
the capture lands once the host projects the cell. #274 took #286 by merge
rather than porting the reader; the PR body says why.
**A captured commit is not judged by the cursor's cell either (improvise-3h9w).**
#286's `refused_here` declines a commit key whose cursor cell `i` would decline,
and waits on one that is unprojected. It runs in `step()` before `commit()`,
so it must skip an editor that captured its target: that commit writes, or
basis-refuses, the captured cell, whatever now sits under the cursor. The guard
is kept for an editor that captured nothing, which is the case it was written
for.
**What the commit is follows the capture too.** Whether `Enter` writes a value
or renames a coordinate is read off `opened.target()`, not off the cell under
the cursor (ChatGPT Codex on PR #274): a relayed reframe can leave the cursor on
+22
View File
@@ -5750,3 +5750,25 @@ is a `Step::Declined`, shared by the value and rename paths.
- **Rival: `renamed_away()` is `Step::Local`** (the state before the fix). Fails
only this test. It drives the value path; the rename path reaches the same
function, so no rival can split them without also changing the code shape.
### a_captured_edit_commits_when_the_cursor_now_sits_on_an_aggregate
improvise-3h9w, a regression from merging #286 into #274, found by ChatGPT Codex
and the delta review at `8c28360f`. `step()` runs `refused_here` before
`commit()` consults the capture, and for the commit keys `refused_here` judged
the **cursor's** cell. A opens an editor on North/Cost and types `42`. B moves
Region to None, the relays reset A's cursor, and it now sits on an aggregate.
A's `Enter` was declined with "Cannot write to an aggregate" on every retry, no
frame was sent, and Esc lost the text, where `8d9de94e` wrote North/Cost. It
failed at the North/Cost assertion with `None`.
The vanished-rows test could not see this, because `unprojected` exempts an
empty grid, so no guard fired there. This case needs the cell under the cursor
to change **kind** while it stays projected.
- **Rival: `refused_here` without the `state.opened.is_some()` arm** (the state
before the fix). Fails only this test.
- **Not covered:** a captured editor whose cursor lands on a cell that is
inside the grid but outside the held window. The same arm exempts it, but no
test builds it, because every reframe here resets the cursor to the origin,
which is always inside the window.
+2 -1
View File
@@ -344,11 +344,12 @@ fn committed(state: &ClientState, name: &str, args: &[String]) -> Option<Step> {
}
/// A commit onto a cell `i` declines is refused as `committable_cell_key` refuses
/// it, out loud with the binding (ik6l); a cell op on an unprojected cell waits (7mro).
/// it (ik6l); a cell op on an unprojected cell waits (7mro). Not a captured commit (3h9w).
fn refused_here(state: &ClientState, name: &str) -> Option<String> {
let (row, col) = state.view.cursor().selected;
let waiting = unprojected(state, row, col).then(|| NOT_PROJECTED.to_string());
match name {
"commit-cell-edit" | "commit-and-advance-right" if state.opened.is_some() => None,
"commit-cell-edit" | "commit-and-advance-right" => declined_at(state, row, col).or(waiting),
"clear-cell" | "yank" | "paste" => waiting,
_ => None,
@@ -2008,3 +2008,76 @@ fn a_retry_whose_cell_is_gone_keeps_the_typed_text() {
"the text the user typed must survive the decline"
);
}
/// `host()` with `Profit = Revenue - Cost`, so a grid with Region on None
/// shows aggregates and a derived column, and no stored cell.
fn host_with_profit() -> Host {
let mut wb = Workbook::new("Test");
wb.add_category("Region").unwrap();
wb.add_item("Region", "North").unwrap();
wb.add_item("Region", "South").unwrap();
wb.add_item("_Measure", "Revenue").unwrap();
wb.add_item("_Measure", "Cost").unwrap();
wb.model.add_formula(
improvise_command::formula::parse_formula("Profit = Revenue - Cost", "_Measure").unwrap(),
);
wb.active_view_mut().set_axis("Region", Axis::Row);
wb.active_view_mut().set_axis("_Measure", Axis::Column);
wb.model.set_cell(
CellKey::new(vec![
("Region".into(), "North".into()),
("_Measure".into(), "Revenue".into()),
]),
CellValue::Number(1200.0),
);
let mut app = App::new(wb, None);
app.rebuild_layout();
Host::new(app)
}
/// improvise-3h9w: B's reframe left A's cursor on an aggregate, and Enter was
/// refused for THAT cell instead of writing the captured one. Evidence file.
#[test]
fn a_captured_edit_commits_when_the_cursor_now_sits_on_an_aggregate() {
use improvise_command::ui::reduce::caught_up;
use improvise_protocol::ModelCommand;
let mut host = host_with_profit();
let (mut a, mut b) = (Client::new(), Client::new());
let (a_id, a_snap) = host.connect(10, 10);
let (b_id, b_snap) = host.connect(10, 10);
a.set_viewport(10, 10);
b.set_viewport(10, 10);
deliver(&mut a, &[a_snap]);
deliver(&mut b, &[b_snap]);
for key in ["l", "i", "4", "2"] {
press(&mut host, a_id, &mut a, key);
}
let region_off = ModelCommand::SetAxis {
view: host.app().view_name().to_string(),
category: "Region".into(),
axis: Axis::None,
};
let relayed = host.handle(
b_id,
ClientFrame::command(caught_up(host.app(), region_off)),
);
route(&relayed, (a_id, &mut a), (b_id, &mut b));
assert!(a.state().view.mode.is_editing(), "setup: still editing");
let (row, col) = a.state().view.cursor().selected;
assert!(
matches!(
a.state().grid.cell(row, col).and_then(|c| c.id.clone()),
Some(improvise_protocol::CellId::Aggregate(_))
),
"setup: the cursor now sits on an aggregate"
);
press(&mut host, a_id, &mut a, "Enter");
assert_eq!(
cell(&host, "North", "Cost"),
Some(CellValue::Number(42.0)),
"the captured cell must be written: status {:?}",
a.state().view.status_msg
);
}