Merge pull request #236 from fiddlerwoaroof/fix/i3o5-set-status-refusals

The collapsed-group and three paste declines refuse, so they exit 1 and reject a frame (improvise-i3o5)

context/code/cli.md

@@ -313,9 +313,15 @@ still possible later, and would want the severity *on the message*; that is a
 separate change with a separate staleness problem.
 
 **The classification is per writer, and it is not total.** Every refusal in
-`ui/effect.rs` and in `cmd/{commit,fit,grid,mode,search,text_buffer}.rs` goes
-through `refuse`. Those still written with `set_status` exit 0 as before, and
-are listed in improvise-i3o5. `q` on a dirty document is deliberately left a
+`ui/effect.rs` and in `cmd/{cell,commit,fit,grid,mode,search,text_buffer}.rs`
+goes through `refuse`. `EditOrDrill`'s collapsed-group arm and the three
+spelled-paste declines in `cell.rs` (aggregate, coordinate column, inside a
+drill) joined them with improvise-i3o5. Those still written with `set_status`
+exit 0 as before, and are listed on that bead: three in `cmd/panel.rs`, and
+`command/keymap.rs::dispatch`'s arg-error arm. The sweeps that pin refusals
+revert `refuse(` calls, so they **cannot see a refusal written as
+`set_status`**. Finding one takes a read of every `set_status` on a decline
+path, and that is how these were found. `q` on a dirty document is deliberately left a
 report: headless, its text is false — the CLI saves anyway (improvise-ep5o).
 The two rivals udt2 named both stay green: a successful `:w` and a successful
 `fit-measure` report through `set_status` and exit 0.

context/test-evidence/command.md

@@ -6048,3 +6048,32 @@ from the landed cell and untouched.
 flag. This test fails alone. Every other test stays green, which was the gap.
 Once improvise-fy9o re-seeds the reopened editor, a stale `touched` would make
 every untouched Tab or Enter write its seed.
+
+#### improvise-i3o5: four declines that `set_status` hid from every sweep
+
+`every_refusal_site_refuses_its_failure_and_only_its_failure` gained four cases.
+PR #213's review found them by reading, because the sweeps behind this table
+revert `refuse(` calls and cannot see a refusal written as `set_status`. Each
+exited 0 headless and would not reject a frame:
+
+- `EditOrDrill`'s collapsed-group arm (`folded_quarter`, cursor on Q2's fold at
+  row 2). The succeeding twin is on Feb at row 1. Its two sibling arms already
+  refused.
+- `paste_into`'s aggregate decline. The spelled key `Row/A Col/A` omits the
+  `Hidden` category on None. The twin spells all three coordinates.
+- `paste_into_record`'s coordinate-column decline: `_Index/1 _Dim/0` is Region.
+  The twin, `_Dim/2`, is a measure.
+- `paste_into_record`'s inside-a-drill decline. The same measure key, under a
+  frozen drill. The twin is the same key with no drill.
+
+**Red at `e406edc`**, in the order the table reaches the cases. Each failed with
+its own text and `refused=false`, and each went green only when its own site was
+converted. The collapsed-group case failed first. Then, one conversion at a
+time, the three paste cases failed with the aggregate, coordinate-column and
+inside-a-drill text.
+
+**Per-site revert:** `/tmp/refuse-fu/sweep.py`, not committed, restricted to the
+four sites. It reverts each `effect::refuse(` to `effect::set_status(` alone and
+runs `cargo test --workspace --no-fail-fast`. Four of four fail this test, and
+nothing else fails. None of the four needs an abort: each is the only effect its
+command emits, and `p`/`i` are single bindings, not sequences.

crates/improvise-command/src/command/cmd/cell.rs

@@ -626,7 +626,7 @@ impl Cmd for PasteCell {
 fn paste_into(ctx: &CmdContext, key: &CellKey, value: CellValue) -> Box<dyn Effect> {
     match synthetic_record_info(key) {
         Some((row, col)) => paste_into_record(ctx, row, col, value),
-        None if aggregates(ctx, key) => effect::set_status(AGGREGATE_REFUSAL),
+        None if aggregates(ctx, key) => effect::refuse(AGGREGATE_REFUSAL),
         None => Box::new(effect::SetCell(key.clone(), value)),
     }
 }
@@ -638,9 +638,9 @@ fn paste_into_record(
     value: CellValue,
 ) -> Box<dyn Effect> {
     match (ctx.layout.record_cell_key(row, col), ctx.has_drill_state) {
-        (Some(_), true) => effect::set_status(INSIDE_A_DRILL_REFUSAL),
+        (Some(_), true) => effect::refuse(INSIDE_A_DRILL_REFUSAL),
         (Some(real), false) => Box::new(effect::SetCell(real, value)),
-        (None, _) => effect::set_status(NO_VALUE_REFUSAL),
+        (None, _) => effect::refuse(NO_VALUE_REFUSAL),
     }
 }
 

crates/improvise-command/src/command/cmd/mode.rs

@@ -359,7 +359,7 @@ impl Cmd for EditOrDrill {
     fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
         // Neither edit nor drill: no record carries a group's name (core.md §14).
         if ctx.layout.folds_a_group(ctx.selected.0, ctx.selected.1) {
-            return vec![effect::set_status(
+            return vec![effect::refuse(
                 "Collapsed group — press z to expand it, then edit an item",
             )];
         }

crates/improvise-command/src/ui/app.rs

@@ -3285,6 +3285,36 @@ mod tests {
         app
     }
 
+    /// Months grouped into quarters with Q2 collapsed: row 2 is the fold.
+    fn folded_quarter() -> App {
+        let mut wb = Workbook::new("T");
+        wb.add_category("Month").unwrap();
+        wb.add_category("Type").unwrap();
+        for (month, quarter) in [("Jan", "Q1"), ("Feb", "Q1"), ("Apr", "Q2"), ("May", "Q2")] {
+            wb.add_item_in_group("Month", month, quarter).unwrap();
+        }
+        wb.add_item("Type", "Food").unwrap();
+        wb.active_view_mut().toggle_group_collapse("Month", "Q2");
+        let mut app = App::new(wb, None);
+        app.rebuild_layout();
+        app
+    }
+
+    fn yanked(mut app: App) -> App {
+        app.view_state.yanked = Some(CellValue::Number(5.5));
+        app
+    }
+
+    /// A drill frozen over the records the grid already shows.
+    fn in_a_drill(mut app: App) -> App {
+        let records = app.layout.records.clone().expect("records mode");
+        app.drill_state = Some(DrillState {
+            records,
+            pending_edits: HashMap::new(),
+        });
+        app
+    }
+
     fn with_a_second_view(mut app: App) -> App {
         app.model_state.workbook.create_view("Second");
         app
@@ -3486,6 +3516,45 @@ mod tests {
                 two_col_model,
                 |app| commit_in(app, AppMode::command_mode(), "command", "add-category Z"),
             ),
+            // improvise-i3o5: these four wrote their decline with `set_status`.
+            (
+                "edit-or-drill on a collapsed group's row",
+                folded_quarter,
+                |app| {
+                    app.cursor_mut().selected = (2, 0);
+                    run_line(app, "edit-or-drill editing")
+                },
+                "Collapsed group",
+                folded_quarter,
+                |app| {
+                    app.cursor_mut().selected = (1, 0);
+                    run_line(app, "edit-or-drill editing")
+                },
+            ),
+            (
+                "a spelled paste onto an aggregate",
+                || yanked(aggregated_grid(true)),
+                |app| run_line(app, "paste Row/A Col/A"),
+                crate::command::cmd::core::AGGREGATE_REFUSAL,
+                || yanked(aggregated_grid(true)),
+                |app| run_line(app, "paste Row/A Col/A Hidden/A"),
+            ),
+            (
+                "a spelled paste onto a records coordinate column",
+                || yanked(records_model_with_two_rows()),
+                |app| run_line(app, "paste _Index/1 _Dim/0"),
+                crate::command::cmd::core::NO_VALUE_REFUSAL,
+                || yanked(records_model_with_two_rows()),
+                |app| run_line(app, "paste _Index/1 _Dim/2"),
+            ),
+            (
+                "a spelled paste inside a drill",
+                || yanked(in_a_drill(records_model_with_two_rows())),
+                |app| run_line(app, "paste _Index/1 _Dim/2"),
+                crate::command::cmd::core::INSIDE_A_DRILL_REFUSAL,
+                || yanked(records_model_with_two_rows()),
+                |app| run_line(app, "paste _Index/1 _Dim/2"),
+            ),
         ];
         for (what, fail_setup, fails, says, ok_setup, succeeds) in cases {
             let mut app = fail_setup();