Merge remote-tracking branch 'gh/main' into fix/df2w-i0v9-browser-page

This commit is contained in:
Edward Langley
2026-09-22 21:15:17 -07:00
7 changed files with 506 additions and 215 deletions
+68
View File
@@ -151,7 +151,75 @@ atomicity is preserved by keeping the existing whole-line `parse_line` as a
still parses, and a line that did not still does not, so the only behavioural
change is which cursor the commands see.
**That atomicity claim was false as written** (improvise-2bac), and §8 is what
makes it true. The gate runs the pure arm; the loop runs the interactive arm. A
gate is only a gate if *every argument list the pure arm accepts, the
interactive arm accepts too* — and `commit-cell-edit` broke it:
`add-category Foo . commit-cell-edit editing 5 Foo/x` on an empty document
passed the gate, applied the first segment, and then refused the second with
`no cell at cursor`. Half the line landed.
The TUI's `:` line had the same defect and is fixed the same way, with one
limitation that is not new: `ExecuteCommand` returns one effect batch, so the
segments of `:a . b` all see the pre-batch cursor, exactly as a
`Binding::Sequence` does (gotchas 21 and 23).
## 8 — the interactive arm is `parse`, then context; it is never a second parser
#151 fixed three instances of *a registry with two construction arms demotes
whichever caller reaches the weaker one*, one patch each. A post-merge review
found the fourth (improvise-2bac): `commit-cell-edit <mode> <value> <coords>`,
whose interactive arm read only `<mode>`, took the value from the `edit` buffer
and the key from the cursor. So a script that wrote 5 to `Region/East`
**deleted the cell under the cursor** — the empty buffer, committed — with exit
0 and nothing on stderr. The fifth was already in the tree:
`toggle-panel-and-focus <panel> false false` ignored both flags and toggled.
The shape rather than the instances is what changed. `CmdRegistry::register`
used to take two free functions, `parse(args)` and `interactive(args, ctx)`,
and nothing related them; each command re-derived its arguments twice and every
disagreement was silent. It now takes three pieces, and the interactive arm is
**built from `parse`**, not beside it:
- `parse(args) -> C` — the only function that reads arguments a caller supplied.
- `contextualize(C, &CmdContext) -> C` — fills the fields that are context and
never argument (a `Move`'s origin cursor, a `MovePanelCursor`'s bound). It is
handed the built command and **not the args**, so it cannot discard them.
- `fallback: Option<(args, ctx) -> C>` — reached **only** when `parse` refused.
This is where "no coordinates means the cursor" lives: `clear-cell` bare, and
`commit-cell-edit <mode>` bare, which is what the keymap binds.
Two consequences hold by construction rather than by test. **Whatever `parse`
accepts, the interactive arm accepts** — `contextualize` is infallible — so the
§7 gate cannot pass a segment the loop then refuses. And **an accepted argument
list cannot be overridden by the cursor**, because the only function that sees
both args and context never runs on an accepted list. `register_pure` and
`register_nullary` already had one arm; they are the `contextualize = identity,
fallback = None` case.
What is still representable, stated rather than hidden:
- `contextualize` can overwrite a field `parse` set. It would have to name the
field, which is a visible line in review, where the old `|_args, ctx|` was an
invisible omission. `commit_cell_edit_honours_an_explicit_value_and_cell_over_the_cursor`
fails if `commit-cell-edit`'s does.
- A `fallback` sees the args `parse` refused and could ignore some of them.
`commit-cell-edit`'s refuses anything but a lone mode, so the malformed
`commit-cell-edit editing 5` is an error on the `:` line — which has no pure
gate — rather than a commit of the edit buffer to the cursor cell.
- The gate is **stricter** than the loop wherever a fallback exists: a bare
`clear-cell` is refused in a script. That is the safe direction — the whole
line is refused — and it is unchanged from before #151.
`toggle-panel-and-focus` did not fit `contextualize`, because its pure arm
defaulted the omitted flags to `true` and a contextualizer cannot tell an
omitted flag from an explicit one. So the omission is now in the type —
`target: Option<(bool, bool)>`, `None` meaning *toggle* — and the toggle is
resolved in `execute` against the same context. That is the honest home: the
decision depends on context, and `execute` is where context is.
Guards: `every_argument_list_the_gate_accepts_the_run_accepts_too` enumerates
**every registration** against every candidate argument list, in a context whose
cursor sits on no cell — the context in which the old `commit-cell-edit` arm
refused. It lists no commands, so a new registration is covered the day it
lands. It is backed by the process-level tests in `tests/headless_cli.rs`.
+7 -6
View File
@@ -2489,12 +2489,13 @@ line separated by `.`, coordinate pairs written `Category/Item`, and quoted stri
available for the rare command that needs to enumerate all views, such as the view
panel.
`InteractiveFn` constructs a `Cmd` from the interactive context (keymap dispatch).
It receives both the keymap args and the interactive context, so commands can
combine text arguments (a panel name) with runtime state (whether that panel is
currently open). `register_pure` registers a command that does not need interactive
context: called interactively with args it delegates to `parse`, and without args it
errors.
The interactive arm (keymap dispatch, scripts, the `:` line) is built **from**
`parse`: `register` takes `parse`, a `contextualize` that fills context-only
fields of the command `parse` built without seeing the args, and an optional
`fallback` reached only for argument lists `parse` refused. So an accepted
argument list is never overridden by context (improvise-2bac; the argument is
`context/code/cli.md` §8). `register_pure` is the case with nothing to
contextualize and no fallback: both arms are `parse`.
`write_buffer` is the effect that writes a named buffer — the mirror of
`read_buffer`, and the only place that knows "search" is not an entry in the buffers
+84 -2
View File
@@ -229,12 +229,94 @@ the syntax gate and then cleared whatever the cursor was sitting on — the orig
So improvise-6uva's own lesson had a **third** instance in its own fix: two
construction arms that mean different things demote whichever caller reaches the
weaker one, and here the "weaker" one was the arm that had the context. The four
arms now go through `key_from_args_or_cursor`: explicit coordinates win, the
arms went through `key_from_args_or_cursor`: explicit coordinates win, the
cursor is the fallback, which is also what the keymap path wants since a binding
carries no arguments.
carries no arguments. That helper is gone since improvise-2bac: the cursor is
now the registry's `fallback`, reached only when `parse` refused, so explicit
coordinates win by construction (`context/code/cli.md` §8).
The fixture writes three cells and clears the one the cursor is **not** on, so
the assertion separates "cleared the named cell" from "cleared the cursor cell"
— clearing `North` by name would have passed against the broken code. Restoring
the arg-discarding arm fails it: `the cursor cell was cleared instead of the
named one: ["Region=East = 3", "Region=South = 2"]`.
### commit_cell_edit_writes_the_named_cell_and_spares_the_cursors
improvise-2bac, found by the post-merge adversarial review of PR #151: the
fourth instance of that PR's own lesson. `commit-cell-edit` was registered with
two free arms. The pure arm read `<mode> <value> <coords>`; the interactive arm
read only `<mode>`, took the value from the `edit` buffer and the key from the
cursor. Once #151 made scripts construct through the interactive arm,
`commit-cell-edit editing 5 Region/East` passed the syntax gate and then
committed an **empty** buffer to the cursor cell — deleting it — with exit 0
and nothing on stderr.
Reproduced at `40b8913` before the fix, through the real binary: the document
came back as `["Region=East = 3", "Region=North = 1"]` — `South`, the cell the
cursor had been moved onto, was gone and `East` was unchanged.
The fixture writes three cells and **moves the cursor to `South`** in the same
line before committing to `East`, so the cursor sits on neither the target nor
the origin: a test with the cursor on the target is right by accident, and one
that only asserts the target misses the destruction. It asserts all three.
Injected rivals, run: (a) `contextualize` for `CommitAndAdvance` overwriting
`key` and `value` from the cursor and the buffer — fails this test and
`a_line_whose_gate_passes_runs_whole`; (b) the registry dispatching to the
fallback first whenever one exists (the pre-fix shape) — fails this test,
`a_line_whose_gate_passes_runs_whole`, `an_explicit_coordinate_still_beats_the_cursor`
and both registry unit tests below.
### a_line_whose_gate_passes_runs_whole
improvise-2bac's second symptom, reported on the ticket: `context/code/cli.md`
§7 said the whole-line `parse_line` gate preserved line atomicity. It did not,
wherever the interactive arm refused an argument list the pure arm accepted.
`add-category Foo . commit-cell-edit editing 5 Foo/x` on an empty document
printed `Parse error: commit-cell-edit: no cell at cursor` after `Foo` had
already been added and saved: half the line landed.
The assertion is on `Parse error` and on the committed cell, **not** on the exit
code. The line exits 1 both before #151 and after this fix, for a different and
pre-existing reason: the grid has no rows, so `commit-cell-edit`'s advance
emits `AbortChain`, and the CLI maps an aborted batch to exit 1
(`context/code/cli.md` §3). Asserting success would have made this test fail on
the correct behaviour.
### a_malformed_commit_line_is_refused_whole
The other half of atomicity, which must survive the fix: a segment the gate
refuses (`commit-cell-edit editing 5` — a value and no coordinates) refuses the
whole line, so the preceding `add-category Foo` never lands. It passed before
the fix and must keep passing; its job is to catch a "fix" that loosens the
pure arm to make the gate agree.
### commit_cell_edit_honours_an_explicit_value_and_cell_over_the_cursor
The unit-level form of the headless test, against `CmdRegistry::interactive`
directly: cursor on `Type=Food, Month=Jan`, a decoy `edit` buffer, and an
explicit `5` for `Type/Clothing Month/Feb`. It asserts the built command's key
and value. Rival (a) above — a `contextualize` that overwrites the parsed
fields — fails it; this is the one hole the new registry shape leaves
representable, so it is the one that needs a test.
### every_argument_list_the_gate_accepts_the_run_accepts_too
Enumerates **every** registration (`CmdRegistry::names`, no hand-kept list)
against every candidate argument list, and requires the interactive arm to
accept whatever the pure arm accepts, in a context whose cursor sits on **no
cell** — a category with no items. That context is what made the old
`commit-cell-edit` arm refuse, so a context with a cell under the cursor would
have passed the bug. The assertion that the cursor has no cell is checked
first; a plain `Workbook::new` does not satisfy it, which is how the fixture
came to add the empty category. Rival (b) fails it.
### explicit_open_and_focus_arguments_beat_the_toggle
The fifth instance, named on improvise-2bac as plausible and not yet run:
`toggle-panel-and-focus <panel> false false` ignored both flags and toggled, so
on a **closed** panel it opened it. The fixture closes every panel first,
because on an open-and-focused panel the toggle also closes it and the test
would pass against the bug. Rival: `execute` ignoring `target` and always
toggling — fails this test for every panel.
@@ -139,9 +139,12 @@ pub trait Cmd: Debug + Send + Sync {
/// Factory that constructs a Cmd from text arguments (headless/script).
pub type ParseFn = fn(&[String]) -> Result<Box<dyn Cmd>, String>;
/// Factory constructing a Cmd from the interactive context. It takes both the keymap
/// args and the context, so a command can combine text args with runtime state.
pub type InteractiveFn = fn(&[String], &CmdContext) -> Result<Box<dyn Cmd>, String>;
/// Fills the context-only fields of a command `parse` already built. It never
/// sees the args, so it cannot discard them — `context/code/cli.md` §8.
pub type ContextualizeFn<C> = fn(C, &CmdContext) -> C;
/// Builds a command from context, reached ONLY for args `parse` refused.
pub type FallbackFn<C> = fn(&[String], &CmdContext) -> Result<C, String>;
type BoxParseFn = Box<dyn Fn(&[String]) -> Result<Box<dyn Cmd>, String>>;
type BoxInteractiveFn = Box<dyn Fn(&[String], &CmdContext) -> Result<Box<dyn Cmd>, String>>;
@@ -183,13 +186,26 @@ impl CmdRegistry {
name
}
/// Register a command with both a text parser and an interactive constructor.
/// The name is derived from a prototype command instance.
pub fn register(&mut self, prototype: &dyn Cmd, parse: ParseFn, interactive: InteractiveFn) {
/// Register a command whose interactive arm is `parse` then `contextualize`, so
/// args `parse` accepts are honoured whole — `context/code/cli.md` §8.
pub fn register<C: Cmd + 'static>(
&mut self,
prototype: &C,
parse: fn(&[String]) -> Result<C, String>,
contextualize: ContextualizeFn<C>,
fallback: Option<FallbackFn<C>>,
) {
self.entries.push(CmdEntry {
name: prototype.name(),
parse: Box::new(parse),
interactive: Box::new(interactive),
parse: Box::new(move |args| parse(args).map(|c| Box::new(c) as Box<dyn Cmd>)),
interactive: Box::new(move |args, ctx| {
let built = match (parse(args), fallback) {
(Ok(cmd), _) => Ok(contextualize(cmd, ctx)),
(Err(_), Some(fallback)) => fallback(args, ctx),
(Err(err), None) => Err(err),
};
built.map(|c| Box::new(c) as Box<dyn Cmd>)
}),
});
}
@@ -20,8 +20,7 @@ mod tests {
let ctx = make_ctx(&m, &layout, &reg);
let cmd = TogglePanelAndFocus {
panel: effect::Panel::Formula,
open: true,
focused: true,
target: Some((true, true)),
};
let effects = cmd.execute(&ctx);
assert_eq!(effects.len(), 2);
@@ -40,8 +39,7 @@ mod tests {
let ctx = make_ctx(&m, &layout, &reg);
let cmd = TogglePanelAndFocus {
panel: effect::Panel::Formula,
open: false,
focused: false,
target: Some((false, false)),
};
let effects = cmd.execute(&ctx);
assert_eq!(effects.len(), 2);
@@ -531,20 +529,24 @@ mod tests {
#[derive(Debug)]
pub struct TogglePanelAndFocus {
pub panel: Panel,
pub open: bool,
pub focused: bool,
/// Explicit `(open, focused)`; `None` toggles: close if open and focused, else open and focus.
pub target: Option<(bool, bool)>,
}
impl Cmd for TogglePanelAndFocus {
fn name(&self) -> &'static str {
"toggle-panel-and-focus"
}
fn execute(&self, _ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
let (open, focused) = self.target.unwrap_or_else(|| {
let shown = ctx.panel_open(self.panel) && ctx.mode.panel() == Some(self.panel);
(!shown, !shown)
});
let mut effects: Vec<Box<dyn Effect>> = Vec::new();
effects.push(Box::new(effect::SetPanelOpen {
panel: self.panel,
open: self.open,
open,
}));
if self.focused {
if focused {
effects.push(effect::change_mode(self.panel.mode()));
} else {
effects.push(effect::change_mode(AppMode::Normal));
@@ -22,15 +22,50 @@ use super::text_buffer::*;
use super::tile::*;
use super::wizard::*;
/// Explicit coordinates win; the cursor is the fallback — `context/code/cli.md`.
fn key_from_args_or_cursor(args: &[String], ctx: &CmdContext) -> Result<CellKey, String> {
if args.is_empty() {
ctx.cell_key().clone().ok_or("no cell at cursor".into())
} else {
Ok(parse_cell_key_from_args(args))
/// The cell under the cursor: what a coordinate command falls back to when given none.
fn cursor_key(ctx: &CmdContext) -> Result<CellKey, String> {
ctx.cell_key().clone().ok_or("no cell at cursor".into())
}
/// A `Move` built from args takes the live cursor as its origin.
fn move_from_cursor(cmd: Move, ctx: &CmdContext) -> Move {
Move {
cursor: CursorState::from_ctx(ctx),
..cmd
}
}
/// A commit built from args still advances from the live cursor.
fn commit_from_cursor(cmd: CommitAndAdvance, ctx: &CmdContext) -> CommitAndAdvance {
CommitAndAdvance {
cursor: CursorState::from_ctx(ctx),
..cmd
}
}
/// `<mode>` alone: commit the edit buffer to the cell under the cursor.
fn commit_at_cursor(
args: &[String],
ctx: &CmdContext,
advance: AdvanceDir,
) -> Result<CommitAndAdvance, String> {
let [mode] = args else {
return Err("takes a mode alone, or a mode, a value and coordinates".into());
};
Ok(CommitAndAdvance {
edit_mode: parse_mode_name(mode)?,
value: read_buffer(ctx, "edit"),
key: cursor_key(ctx)?,
advance,
cursor: CursorState::from_ctx(ctx),
})
}
/// For a command with no context-only fields: `parse`'s result is used as built.
fn as_parsed<C>(cmd: C, _ctx: &CmdContext) -> C {
cmd
}
/// Build the default command registry with all commands.
/// Registry names MUST match the `Cmd::name()` return value.
pub fn default_registry() -> CmdRegistry {
@@ -51,14 +86,16 @@ pub fn default_registry() -> CmdRegistry {
if args.is_empty() {
return Err("clear-cell requires at least one Cat/Item coordinate".into());
}
Ok(Box::new(ClearCellCommand {
Ok(ClearCellCommand {
key: parse_cell_key_from_args(args),
}))
},
|args, ctx| {
let key = key_from_args_or_cursor(args, ctx)?;
Ok(Box::new(ClearCellCommand { key }))
})
},
as_parsed,
Some(|_, ctx| {
Ok(ClearCellCommand {
key: cursor_key(ctx)?,
})
}),
);
r.register_pure(&AddFormulaCmd(vec![]), AddFormulaCmd::parse);
r.register_pure(&FitMeasure::prototype(), FitMeasure::parse);
@@ -80,16 +117,8 @@ pub fn default_registry() -> CmdRegistry {
r.register_pure(&ExportCmd(vec![]), ExportCmd::parse);
r.register_pure(&WriteCmd(vec![]), WriteCmd::parse);
r.register_pure(&HelpCmd(vec![]), HelpCmd::parse);
r.register(
&HelpPageNextCmd(vec![]),
HelpPageNextCmd::parse,
|_args, _ctx| Ok(Box::new(HelpPageNextCmd(vec![]))),
);
r.register(
&HelpPagePrevCmd(vec![]),
HelpPagePrevCmd::parse,
|_args, _ctx| Ok(Box::new(HelpPagePrevCmd(vec![]))),
);
r.register_pure(&HelpPageNextCmd(vec![]), HelpPageNextCmd::parse);
r.register_pure(&HelpPagePrevCmd(vec![]), HelpPagePrevCmd::parse);
// ── Navigation (unified Move) ──────────────────────────────────────
r.register(
@@ -102,22 +131,14 @@ pub fn default_registry() -> CmdRegistry {
require_args(args, 2)?;
let dr = args[0].parse::<i32>().map_err(|e| e.to_string())?;
let dc = args[1].parse::<i32>().map_err(|e| e.to_string())?;
Ok(Box::new(Move {
Ok(Move {
kind: MoveKind::Relative(dr, dc),
cursor: CursorState::default(),
cmd_name: "move-selection",
}))
},
|args, ctx| {
require_args(args, 2)?;
let dr = args[0].parse::<i32>().map_err(|e| e.to_string())?;
let dc = args[1].parse::<i32>().map_err(|e| e.to_string())?;
Ok(Box::new(Move {
kind: MoveKind::Relative(dr, dc),
cursor: CursorState::from_ctx(ctx),
cmd_name: "move-selection",
}))
})
},
move_from_cursor,
None,
);
// Jump-to-edge commands: first/last row/col
macro_rules! reg_jump {
@@ -133,7 +154,7 @@ pub fn default_registry() -> CmdRegistry {
cmd_name: $name,
},
|_| {
Ok(Box::new(Move {
Ok(Move {
kind: if $to_end {
MoveKind::ToEnd($is_row)
} else {
@@ -141,19 +162,10 @@ pub fn default_registry() -> CmdRegistry {
},
cursor: CursorState::default(),
cmd_name: $name,
}))
},
|_, ctx| {
Ok(Box::new(Move {
kind: if $to_end {
MoveKind::ToEnd($is_row)
} else {
MoveKind::ToStart($is_row)
},
cursor: CursorState::from_ctx(ctx),
cmd_name: $name,
}))
})
},
move_from_cursor,
None,
);
};
}
@@ -170,21 +182,14 @@ pub fn default_registry() -> CmdRegistry {
|args| {
require_args(args, 1)?;
let n = args[0].parse::<i32>().map_err(|e| e.to_string())?;
Ok(Box::new(Move {
Ok(Move {
kind: MoveKind::Relative(n, 0),
cursor: CursorState::default(),
cmd_name: "scroll-rows",
}))
},
|args, ctx| {
require_args(args, 1)?;
let n = args[0].parse::<i32>().map_err(|e| e.to_string())?;
Ok(Box::new(Move {
kind: MoveKind::Relative(n, 0),
cursor: CursorState::from_ctx(ctx),
cmd_name: "scroll-rows",
}))
})
},
move_from_cursor,
None,
);
r.register(
&Move {
@@ -195,28 +200,21 @@ pub fn default_registry() -> CmdRegistry {
|args| {
require_args(args, 1)?;
let dir = args[0].parse::<i32>().map_err(|e| e.to_string())?;
Ok(Box::new(Move {
Ok(Move {
kind: MoveKind::Page(dir),
cursor: CursorState::default(),
cmd_name: "page-scroll",
}))
},
|args, ctx| {
require_args(args, 1)?;
let dir = args[0].parse::<i32>().map_err(|e| e.to_string())?;
Ok(Box::new(Move {
kind: MoveKind::Page(dir),
cursor: CursorState::from_ctx(ctx),
cmd_name: "page-scroll",
}))
})
},
move_from_cursor,
None,
);
r.register(
&EnterAdvance {
cursor: CursorState::default(),
},
|_| {
Ok(Box::new(EnterAdvance {
Ok(EnterAdvance {
cursor: CursorState {
row: 0,
col: 0,
@@ -227,13 +225,12 @@ pub fn default_registry() -> CmdRegistry {
visible_rows: 20,
visible_cols: 8,
},
}))
})
},
|_, ctx| {
Ok(Box::new(EnterAdvance {
cursor: CursorState::from_ctx(ctx),
}))
|_, ctx| EnterAdvance {
cursor: CursorState::from_ctx(ctx),
},
None,
);
// ── Cell operations ──────────────────────────────────────────────────
@@ -245,14 +242,16 @@ pub fn default_registry() -> CmdRegistry {
if args.is_empty() {
return Err("yank requires at least one Cat/Item coordinate".into());
}
Ok(Box::new(YankCell {
Ok(YankCell {
key: parse_cell_key_from_args(args),
}))
},
|args, ctx| {
let key = key_from_args_or_cursor(args, ctx)?;
Ok(Box::new(YankCell { key }))
})
},
as_parsed,
Some(|_, ctx| {
Ok(YankCell {
key: cursor_key(ctx)?,
})
}),
);
r.register(
&PasteCell {
@@ -262,14 +261,16 @@ pub fn default_registry() -> CmdRegistry {
if args.is_empty() {
return Err("paste requires at least one Cat/Item coordinate".into());
}
Ok(Box::new(PasteCell {
Ok(PasteCell {
key: parse_cell_key_from_args(args),
}))
},
|args, ctx| {
let key = key_from_args_or_cursor(args, ctx)?;
Ok(Box::new(PasteCell { key }))
})
},
as_parsed,
Some(|_, ctx| {
Ok(PasteCell {
key: cursor_key(ctx)?,
})
}),
);
// clear-cell is registered above (unified: ctx.cell_key() or explicit coords)
@@ -313,21 +314,19 @@ pub fn default_registry() -> CmdRegistry {
if args.is_empty() {
return Err("drill-into-cell requires Cat/Item coordinates".into());
}
Ok(Box::new(DrillIntoCell {
Ok(DrillIntoCell {
key: parse_cell_key_from_args(args),
}))
},
|args, ctx| {
let key = key_from_args_or_cursor(args, ctx)?;
Ok(Box::new(DrillIntoCell { key }))
})
},
as_parsed,
Some(|_, ctx| {
Ok(DrillIntoCell {
key: cursor_key(ctx)?,
})
}),
);
r.register_nullary(|| Box::new(ViewNavigate { forward: false }));
r.register(
&ViewNavigate { forward: true },
|_| Ok(Box::new(ViewNavigate { forward: true })),
|_, _| Ok(Box::new(ViewNavigate { forward: true })),
);
r.register_nullary(|| Box::new(ViewNavigate { forward: true }));
r.register_pure(&EnterMode(AppMode::Normal), |args| {
require_args(args, 1)?;
Ok(Box::new(EnterMode(parse_mode_name(&args[0])?)))
@@ -345,38 +344,20 @@ pub fn default_registry() -> CmdRegistry {
r.register(
&TogglePanelAndFocus {
panel: Panel::Formula,
open: true,
focused: true,
target: None,
},
|args| {
// Parse: toggle-panel-and-focus <panel> [open] [focused]
require_args(args, 1)?;
let panel = parse_panel(&args[0])?;
let open = args.get(1).map(|s| s == "true").unwrap_or(true);
let focused = args.get(2).map(|s| s == "true").unwrap_or(open);
Ok(Box::new(TogglePanelAndFocus {
panel,
open,
focused,
}))
},
|args, ctx| {
require_args(args, 1)?;
let panel = parse_panel(&args[0])?;
// Default interactive: if already open+focused -> close, else open+focus
let currently_open = ctx.panel_open(panel);
let currently_focused = ctx.mode.panel() == Some(panel);
let (open, focused) = if currently_open && currently_focused {
(false, false) // close
} else {
(true, true) // open + focus
};
Ok(Box::new(TogglePanelAndFocus {
panel,
open,
focused,
}))
let target = args.get(1).map(|s| {
let open = s == "true";
(open, args.get(2).map(|s| s == "true").unwrap_or(open))
});
Ok(TogglePanelAndFocus { panel, target })
},
as_parsed,
None,
);
r.register(
&TogglePanelVisibility {
@@ -386,20 +367,16 @@ pub fn default_registry() -> CmdRegistry {
|args| {
require_args(args, 1)?;
let panel = parse_panel(&args[0])?;
Ok(Box::new(TogglePanelVisibility {
Ok(TogglePanelVisibility {
panel,
currently_open: false,
}))
})
},
|args, ctx| {
require_args(args, 1)?;
let panel = parse_panel(&args[0])?;
let currently_open = ctx.panel_open(panel);
Ok(Box::new(TogglePanelVisibility {
panel,
currently_open,
}))
|cmd, ctx| TogglePanelVisibility {
currently_open: ctx.panel_open(cmd.panel),
..cmd
},
None,
);
r.register(
&CyclePanelFocus {
@@ -408,19 +385,18 @@ pub fn default_registry() -> CmdRegistry {
view_open: false,
},
|_| {
Ok(Box::new(CyclePanelFocus {
Ok(CyclePanelFocus {
formula_open: false,
category_open: false,
view_open: false,
}))
})
},
|_, ctx| {
Ok(Box::new(CyclePanelFocus {
formula_open: ctx.formula_panel_open,
category_open: ctx.category_panel_open,
view_open: ctx.view_panel_open,
}))
|_, ctx| CyclePanelFocus {
formula_open: ctx.formula_panel_open,
category_open: ctx.category_panel_open,
view_open: ctx.view_panel_open,
},
None,
);
r.register(
&MovePanelCursor {
@@ -433,29 +409,26 @@ pub fn default_registry() -> CmdRegistry {
require_args(args, 2)?;
let panel = parse_panel(&args[0])?;
let delta = args[1].parse::<i32>().map_err(|e| e.to_string())?;
Ok(Box::new(MovePanelCursor {
Ok(MovePanelCursor {
panel,
delta,
current: 0,
max: 0,
}))
})
},
|args, ctx| {
require_args(args, 2)?;
let panel = parse_panel(&args[0])?;
let delta = args[1].parse::<i32>().map_err(|e| e.to_string())?;
let (current, max) = match panel {
|cmd, ctx| {
let (current, max) = match cmd.panel {
Panel::Formula => (ctx.formula_cursor, ctx.model.formulas().len()),
Panel::Category => (ctx.cat_panel_cursor, ctx.cat_tree_len()),
Panel::View => (ctx.view_panel_cursor, ctx.workbook.views.len()),
};
Ok(Box::new(MovePanelCursor {
panel,
delta,
MovePanelCursor {
current,
max,
}))
..cmd
}
},
None,
);
r.register_nullary(|| Box::new(DeleteFormulaAtCursor));
r.register_nullary(|| Box::new(AddRecordRow));
@@ -490,11 +463,7 @@ pub fn default_registry() -> CmdRegistry {
// ── Grid operations ──────────────────────────────────────────────────
r.register_nullary(|| Box::new(ToggleGroupAtCursor { is_row: true }));
r.register(
&ToggleGroupAtCursor { is_row: false },
|_| Ok(Box::new(ToggleGroupAtCursor { is_row: false })),
|_, _| Ok(Box::new(ToggleGroupAtCursor { is_row: false })),
);
r.register_nullary(|| Box::new(ToggleGroupAtCursor { is_row: false }));
r.register_nullary(|| Box::new(HideSelectedRowItem));
// ── Text buffer ──────────────────────────────────────────────────────
@@ -537,27 +506,16 @@ pub fn default_registry() -> CmdRegistry {
return Err("commit-cell-edit requires a mode, value, and coords".into());
}
let edit_mode = parse_mode_name(&args[0])?;
Ok(Box::new(CommitAndAdvance {
Ok(CommitAndAdvance {
key: parse_cell_key_from_args(&args[2..]),
value: args[1].clone(),
advance: AdvanceDir::Down,
cursor: CursorState::default(),
edit_mode,
}))
},
|args, ctx| {
require_args(args, 1)?;
let edit_mode = parse_mode_name(&args[0])?;
let value = read_buffer(ctx, "edit");
let key = ctx.cell_key().clone().ok_or("no cell at cursor")?;
Ok(Box::new(CommitAndAdvance {
key,
value,
advance: AdvanceDir::Down,
cursor: CursorState::from_ctx(ctx),
edit_mode,
}))
})
},
commit_from_cursor,
Some(|args, ctx| commit_at_cursor(args, ctx, AdvanceDir::Down)),
);
r.register(
&CommitAndAdvance {
@@ -568,19 +526,8 @@ pub fn default_registry() -> CmdRegistry {
edit_mode: AppMode::editing(),
},
|_| Err("commit-and-advance-right requires context".into()),
|args, ctx| {
require_args(args, 1)?;
let edit_mode = parse_mode_name(&args[0])?;
let value = read_buffer(ctx, "edit");
let key = ctx.cell_key().clone().ok_or("no cell at cursor")?;
Ok(Box::new(CommitAndAdvance {
key,
value,
advance: AdvanceDir::Right,
cursor: CursorState::from_ctx(ctx),
edit_mode,
}))
},
commit_from_cursor,
Some(|args, ctx| commit_at_cursor(args, ctx, AdvanceDir::Right)),
);
r.register(
&CommitBuffer {
@@ -589,13 +536,14 @@ pub fn default_registry() -> CmdRegistry {
|args| {
require_args(args, 1)?;
let kind = parse_commit_kind(&args[0], &AppMode::Normal)?;
Ok(Box::new(CommitBuffer { kind }))
Ok(CommitBuffer { kind })
},
|args, ctx| {
as_parsed,
Some(|args, ctx| {
require_args(args, 1)?;
let kind = parse_commit_kind(&args[0], ctx.mode)?;
Ok(Box::new(CommitBuffer { kind }))
},
Ok(CommitBuffer { kind })
}),
);
r.register_nullary(|| Box::new(CommitExport));
r.register_nullary(|| Box::new(CommitSave));
@@ -641,6 +589,9 @@ mod tests {
vec!["1", "1"],
vec!["formula", "1"],
vec!["editing", "5", "Region", "East"],
vec!["editing", "5", "Region/East"],
vec!["formula", "false", "false"],
vec!["Region/East"],
]
.into_iter()
.map(|v| v.into_iter().map(String::from).collect())
@@ -802,6 +753,67 @@ mod tests {
"only {checked} ex-command examples were found; the scan has stopped seeing them"
);
}
/// Evidence: `context/test-evidence/cli.md`.
#[test]
fn commit_cell_edit_honours_an_explicit_value_and_cell_over_the_cursor() {
let mut app = crate::ui::app::App::new(two_cat_model(), None);
app.view_state.buffers.insert("edit".into(), "decoy".into());
let ctx = app.cmd_context(
crate::command::key::KeyCode::Null,
crate::command::key::KeyModifiers::NONE,
);
let target = CellKey::new(vec![
("Month".into(), "Feb".into()),
("Type".into(), "Clothing".into()),
]);
assert_ne!(
ctx.cell_key().as_ref(),
Some(&target),
"the cursor is elsewhere"
);
let args: Vec<String> = ["editing", "5", "Type/Clothing", "Month/Feb"]
.map(String::from)
.into();
let dbg = format!(
"{:?}",
ctx.registry
.interactive("commit-cell-edit", &args, &ctx)
.expect("commit-cell-edit resolves")
);
assert!(
dbg.contains(&format!("key: {target:?}")),
"wrong cell: {dbg}"
);
assert!(dbg.contains("value: \"5\""), "wrong value: {dbg}");
}
/// Evidence: `context/test-evidence/cli.md`.
#[test]
fn every_argument_list_the_gate_accepts_the_run_accepts_too() {
let r = default_registry();
let mut wb = crate::workbook::Workbook::new("Empty");
wb.add_category("Foo").expect("a category with no items");
let app = crate::ui::app::App::new(wb, None);
let ctx = app.cmd_context(
crate::command::key::KeyCode::Null,
crate::command::key::KeyModifiers::NONE,
);
assert!(ctx.cell_key().is_none(), "the cursor must sit on no cell");
let mut accepted = 0;
for name in r.names().collect::<Vec<_>>() {
for args in candidate_args() {
if r.parse(name, &args).is_err() {
continue;
}
accepted += 1;
if let Err(e) = r.interactive(name, &args, &ctx) {
panic!("the gate accepts `{name} {args:?}` but the run refuses it: {e}");
}
}
}
assert!(accepted > 50, "only {accepted} accepted pairs were tried");
}
}
#[cfg(test)]
@@ -880,6 +892,7 @@ mod panel_open_tests {
ctx.registry
.interactive("toggle-panel-and-focus", &args, &ctx)
.expect("toggle-panel-and-focus resolves")
.execute(&ctx)
);
assert!(
dbg.contains("open: false"),
@@ -897,6 +910,7 @@ mod panel_open_tests {
ctx.registry
.interactive("toggle-panel-and-focus", &args, &ctx)
.expect("toggle-panel-and-focus resolves")
.execute(&ctx)
);
assert!(
dbg.contains("open: true"),
@@ -905,4 +919,29 @@ mod panel_open_tests {
);
}
}
/// Evidence: `context/test-evidence/cli.md`.
#[test]
fn explicit_open_and_focus_arguments_beat_the_toggle() {
for panel in Panel::ALL {
let mut app = App::new(two_cat_model(), None);
app.view_state.formula_panel_open = false;
app.view_state.category_panel_open = false;
app.view_state.view_panel_open = false;
let ctx = app.cmd_context(KeyCode::Null, KeyModifiers::NONE);
let args = [panel.name().to_string(), "false".into(), "false".into()];
let dbg = format!(
"{:?}",
ctx.registry
.interactive("toggle-panel-and-focus", &args, &ctx)
.expect("toggle-panel-and-focus resolves")
.execute(&ctx)
);
assert!(
dbg.contains("open: false") && !dbg.contains("open: true"),
"`{} false false` on a closed panel toggled it open: {dbg}",
panel.name()
);
}
}
}
+83
View File
@@ -357,3 +357,86 @@ fn an_explicit_coordinate_still_beats_the_cursor() {
"the named cell survived: {left:?}"
);
}
/// North=1, South=2, East=3 on the row axis; the cursor starts on North.
fn three_cells(scratch: &Scratch, name: &str) -> PathBuf {
let doc = three_rows(scratch, name);
run_cmd(
&[
"set-cell 1 Region/North",
"set-cell 2 Region/South",
"set-cell 3 Region/East",
],
&doc,
);
assert_eq!(cell_lines(&doc).len(), 3, "the fixture wrote three cells");
doc
}
fn value_of(doc: &Path, item: &str) -> Option<String> {
cell_lines(doc)
.into_iter()
.find(|l| l.contains(&format!("Region={item}")))
.and_then(|l| l.rsplit(" = ").next().map(str::to_string))
}
/// Evidence: `context/test-evidence/cli.md`.
#[test]
fn commit_cell_edit_writes_the_named_cell_and_spares_the_cursors() {
let scratch = Scratch::new("commit-explicit");
let doc = three_cells(&scratch, "commit.improv");
let out = run_cmd(
&["move-selection 1 0 . commit-cell-edit editing 5 Region/East"],
&doc,
);
assert!(out.status.success(), "the line failed: {}", stderr_of(&out));
let cells = cell_lines(&doc);
assert_eq!(value_of(&doc, "East").as_deref(), Some("5"), "{cells:?}");
assert_eq!(
value_of(&doc, "South").as_deref(),
Some("2"),
"the cell under the cursor was touched: {cells:?}"
);
assert_eq!(
value_of(&doc, "North").as_deref(),
Some("1"),
"the origin cell was touched: {cells:?}"
);
}
/// Evidence: `context/test-evidence/cli.md`.
#[test]
fn a_line_whose_gate_passes_runs_whole() {
let scratch = Scratch::new("commit-atomic");
let doc = scratch.path("atomic.improv");
let out = run_cmd(
&["add-category Foo . commit-cell-edit editing 5 Foo/x"],
&doc,
);
assert!(
!stderr_of(&out).contains("Parse error"),
"a line the gate accepted was refused part-way: {}",
stderr_of(&out)
);
let text = std::fs::read_to_string(&doc).expect("the document");
assert!(
text.lines()
.any(|l| l.contains("Foo=x") && l.ends_with("= 5")),
"the commit did not land: {text}"
);
}
/// Evidence: `context/test-evidence/cli.md`.
#[test]
fn a_malformed_commit_line_is_refused_whole() {
let scratch = Scratch::new("commit-malformed");
let doc = seeded(&scratch, "malformed.improv");
let out = run_cmd(&["add-category Foo . commit-cell-edit editing 5"], &doc);
assert!(!out.status.success(), "a malformed line exited 0");
let text = std::fs::read_to_string(&doc).expect("the document");
assert!(
!text.contains("Foo"),
"the first half of a refused line landed: {text}"
);
}