Merge pull request #182 from fiddlerwoaroof/fix/yf7p-cursor-is-the-only-context
yf7p: no registration can overwrite a parsed field
This commit is contained in:
+50
-19
@@ -161,7 +161,7 @@ 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
|
||||
## 8 — the interactive arm is `parse`, then the cursor; 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
|
||||
@@ -175,31 +175,59 @@ and the key from the cursor. So a script that wrote 5 to `Region/East`
|
||||
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:
|
||||
disagreement was silent. The interactive arm is now **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.
|
||||
- For `register_at_cursor` only, the registry itself then sets the command's
|
||||
**cursor** — its `CursorState`, reached through `AtCursor::cursor_mut` — to the
|
||||
live one. That is the whole of what the interactive arm adds to a parsed
|
||||
command, and no registration supplies it.
|
||||
|
||||
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.
|
||||
Three consequences hold by construction rather than by test. **Whatever `parse`
|
||||
accepts, the interactive arm accepts** — nothing after `parse` can fail — so the
|
||||
§7 gate cannot pass a segment the loop then refuses. **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. And **no field `parse` set can
|
||||
be overwritten from context**, because nothing a registration writes can name
|
||||
one: the cursor slot is typed `&mut CursorState`, and no command parses a
|
||||
`CursorState` out of its arguments. `register_pure` and `register_nullary`
|
||||
already had one arm; they are `register` with no fallback.
|
||||
|
||||
That last one is improvise-yf7p, and it is why the registry lost a parameter.
|
||||
#171 took a third function, `contextualize(C, &CmdContext) -> C`, handed the
|
||||
built command and not the args, and stated honestly that it could still
|
||||
**overwrite** a parsed field — naming the field was "a visible line in review".
|
||||
Review then injected five such overwrites on commands other than the one with a
|
||||
test (yank, paste and drill-into-cell taking their key from the cursor over the
|
||||
parsed one; toggle-panel-visibility ignoring its parsed panel; move-panel-cursor
|
||||
clamping its parsed delta to ±1) and the whole workspace stayed green. A hole
|
||||
guarded by review discipline plus one command's test is not guarded. Every
|
||||
contextualizer in the tree fell into one of two shapes, and each had an answer
|
||||
that removes the function rather than checking it:
|
||||
|
||||
- **The cursor** (`Move`, `EnterAdvance`, `CommitAndAdvance`). Always one
|
||||
`CursorState` field, always `CursorState::from_ctx`. So the registry does it,
|
||||
through a slot whose *type* is the guard: an `AtCursor` impl pointing at a
|
||||
`CellKey` or a delta is a compile error, which was tried.
|
||||
- **A read of context that belonged in `execute`** (`toggle-panel-visibility`'s
|
||||
`currently_open`, `cycle-panel-focus`'s three flags, `move-panel-cursor`'s
|
||||
cursor and bound). These copied context into the command only to read it back;
|
||||
`execute` now reads `ctx` directly, the same answer `toggle-panel-and-focus`
|
||||
got below. Those three are `register`/`register_nullary` now, so their
|
||||
interactive arm is exactly `parse`.
|
||||
|
||||
What `execute` does with a parsed field is still the command's own logic and is
|
||||
tested as such — `toggle_panel_visibility_reads_the_panel_it_names` and
|
||||
`move_panel_cursor_moves_from_the_named_panels_own_cursor` fail if the panel or
|
||||
the delta is ignored there. The difference is that the registry is no longer a
|
||||
second place such a bug can be written.
|
||||
|
||||
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
|
||||
@@ -207,9 +235,12 @@ What is still representable, stated rather than hidden:
|
||||
- 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.
|
||||
- A new command could declare a *second* `CursorState` field and parse it from
|
||||
args; `cursor_mut` could then point at the wrong one. No command has one, and
|
||||
a parsed cursor would be a new idea deserving its own argument.
|
||||
|
||||
`toggle-panel-and-focus` did not fit `contextualize`, because its pure arm
|
||||
defaulted the omitted flags to `true` and a contextualizer cannot tell an
|
||||
`toggle-panel-and-focus` did not fit #171's `contextualize` either, 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
|
||||
|
||||
@@ -2566,12 +2566,12 @@ available for the rare command that needs to enumerate all views, such as the vi
|
||||
panel.
|
||||
|
||||
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`.
|
||||
`parse`: `register` takes `parse` and an optional `fallback` reached only for
|
||||
argument lists `parse` refused, and `register_at_cursor` additionally has the
|
||||
registry set the command's `CursorState` to the live cursor. No registration
|
||||
writes any other field, so an accepted argument list is never overridden by
|
||||
context (improvise-2bac, improvise-yf7p; the argument is `context/code/cli.md`
|
||||
§8). `register_pure` is the case with 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
|
||||
|
||||
@@ -298,8 +298,9 @@ 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.
|
||||
fields — failed it. improvise-yf7p then removed `contextualize` altogether
|
||||
(`context/code/cli.md` §8), so rival (a) no longer compiles; the test stays as
|
||||
the behavioural statement of what the signature now guarantees.
|
||||
|
||||
### every_argument_list_the_gate_accepts_the_run_accepts_too
|
||||
|
||||
|
||||
@@ -3140,10 +3140,13 @@ Injected fault: hardcode the `category_panel_open` propagator to `true`. Fails.
|
||||
|
||||
### toggle_panel_visibility_reads_the_panel_it_names
|
||||
|
||||
The behavioural half, and the one that reaches the registry. Its interactive arm
|
||||
passes `currently_open` straight through, so with exactly one panel open the command
|
||||
built for each of the three names carries its own panel's flag. Both injections above
|
||||
fail here too.
|
||||
The behavioural half, and the one that reaches the registry. With exactly one panel
|
||||
open, the command built for each of the three names and executed must emit
|
||||
`SetPanelOpen` for **its own** panel, flipping **its own** flag. Both injections above
|
||||
fail here too. Since improvise-yf7p the flag is read in `execute` rather than copied
|
||||
in by a registry contextualizer, so the assertion moved from the built command's
|
||||
`Debug` to its effects; it also names the panel, so an `execute` that ignores the
|
||||
parsed panel (`ctx.panel_open(Panel::Formula)`) fails it — injected and run.
|
||||
|
||||
### a_focused_open_panel_closes_and_a_closed_one_opens
|
||||
|
||||
@@ -4728,6 +4731,18 @@ Its mirror, and the one that pins the wrap: from the first item `[` must land on
|
||||
`SetPanelCursor` carries a panel *and* a cursor and neither was checked. Both
|
||||
are now named, so moving the right cursor on the wrong panel fails.
|
||||
|
||||
### move_panel_cursor_moves_from_the_named_panels_own_cursor
|
||||
|
||||
improvise-yf7p. `move-panel-cursor formula 2` through the registry's interactive
|
||||
arm, with the formula cursor at **1** of five formulas and the category and view
|
||||
cursors at 0, must land on **3**. Before yf7p the cursor and bound were copied in
|
||||
by a registry `contextualize`, and review showed one that clamped the parsed delta
|
||||
to `delta.signum()` left the whole workspace green: every test moved by ±1, where
|
||||
a clamp is invisible. They are read in `execute` now, and this test is what sees a
|
||||
bug there. Rival `self.delta.signum()` in `execute` — injected, run, fails (lands
|
||||
on 2). Reading another panel's cursor fails too, because the fixture seats them at
|
||||
different values — the index-0 fixture could not separate them.
|
||||
|
||||
### delete_formula_at_cursor_with_formulas
|
||||
|
||||
Asserts the `RemoveFormula` names the formula the cursor is on, rather than merely
|
||||
|
||||
@@ -3,6 +3,8 @@ use std::fmt::Debug;
|
||||
|
||||
use crate::command::key::KeyCode;
|
||||
|
||||
use super::navigation::CursorState;
|
||||
|
||||
use crate::model::Model;
|
||||
use crate::model::cell::{CellKey, CellValue};
|
||||
use crate::ui::app::AppMode;
|
||||
@@ -139,13 +141,15 @@ 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>;
|
||||
|
||||
/// 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>;
|
||||
|
||||
/// A command whose one context-only field is the cursor it moves from. The
|
||||
/// registry fills it, and no registration can — `context/code/cli.md` §8.
|
||||
pub trait AtCursor: Cmd {
|
||||
fn cursor_mut(&mut self) -> &mut CursorState;
|
||||
}
|
||||
|
||||
type BoxParseFn = Box<dyn Fn(&[String]) -> Result<Box<dyn Cmd>, String>>;
|
||||
type BoxInteractiveFn = Box<dyn Fn(&[String], &CmdContext) -> Result<Box<dyn Cmd>, String>>;
|
||||
|
||||
@@ -186,25 +190,48 @@ impl CmdRegistry {
|
||||
name
|
||||
}
|
||||
|
||||
/// Register a command whose interactive arm is `parse` then `contextualize`, so
|
||||
/// args `parse` accepts are honoured whole — `context/code/cli.md` §8.
|
||||
/// Register a command whose interactive arm is `parse`, reaching `fallback`
|
||||
/// only where `parse` refused — `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.push(prototype.name(), parse, fallback, |_, _| {});
|
||||
}
|
||||
|
||||
/// As [`register`](Self::register), then the interactive arm moves the
|
||||
/// command's cursor to the live one. The cursor is the only field it writes.
|
||||
pub fn register_at_cursor<C: AtCursor + 'static>(
|
||||
&mut self,
|
||||
prototype: &C,
|
||||
parse: fn(&[String]) -> Result<C, String>,
|
||||
fallback: Option<FallbackFn<C>>,
|
||||
) {
|
||||
self.push(prototype.name(), parse, fallback, |cmd, ctx| {
|
||||
*cmd.cursor_mut() = CursorState::from_ctx(ctx);
|
||||
});
|
||||
}
|
||||
|
||||
fn push<C: Cmd + 'static>(
|
||||
&mut self,
|
||||
name: &'static str,
|
||||
parse: fn(&[String]) -> Result<C, String>,
|
||||
fallback: Option<FallbackFn<C>>,
|
||||
at_cursor: fn(&mut C, &CmdContext),
|
||||
) {
|
||||
self.entries.push(CmdEntry {
|
||||
name: prototype.name(),
|
||||
name,
|
||||
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),
|
||||
let mut built = match (parse(args), fallback) {
|
||||
(Ok(cmd), _) => cmd,
|
||||
(Err(_), Some(fallback)) => fallback(args, ctx)?,
|
||||
(Err(err), None) => return Err(err),
|
||||
};
|
||||
built.map(|c| Box::new(c) as Box<dyn Cmd>)
|
||||
at_cursor(&mut built, ctx);
|
||||
Ok(Box::new(built) as Box<dyn Cmd>)
|
||||
}),
|
||||
});
|
||||
}
|
||||
|
||||
@@ -117,6 +117,22 @@ pub(super) mod test_helpers {
|
||||
wb
|
||||
}
|
||||
|
||||
/// `two_cat_model` with one `_Measure` formula per target, in order.
|
||||
pub fn with_formulas(targets: &[&str]) -> Workbook {
|
||||
let mut wb = two_cat_model();
|
||||
for target in targets {
|
||||
wb.model.add_formula(crate::formula::ast::Formula {
|
||||
raw: format!("{target} = 0"),
|
||||
target: target.to_string(),
|
||||
target_category: "_Measure".to_string(),
|
||||
expr: crate::formula::ast::Expr::Number(0.0),
|
||||
filter: None,
|
||||
format: None,
|
||||
});
|
||||
}
|
||||
wb
|
||||
}
|
||||
|
||||
pub fn effects_debug(effects: &[Box<dyn Effect>]) -> String {
|
||||
format!("{:?}", effects)
|
||||
}
|
||||
|
||||
@@ -192,11 +192,7 @@ mod tests {
|
||||
let layout = make_layout(&m);
|
||||
let reg = make_registry();
|
||||
let ctx = make_ctx(&m, &layout, ®);
|
||||
let cmd = CyclePanelFocus {
|
||||
formula_open: false,
|
||||
category_open: false,
|
||||
view_open: false,
|
||||
};
|
||||
let cmd = CyclePanelFocus;
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert!(effects.is_empty());
|
||||
}
|
||||
@@ -208,11 +204,7 @@ mod tests {
|
||||
let reg = make_registry();
|
||||
let mut ctx = make_ctx(&m, &layout, ®);
|
||||
ctx.formula_panel_open = true;
|
||||
let cmd = CyclePanelFocus {
|
||||
formula_open: true,
|
||||
category_open: false,
|
||||
view_open: false,
|
||||
};
|
||||
let cmd = CyclePanelFocus;
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert_eq!(effects.len(), 1);
|
||||
let dbg = format!("{:?}", effects[0]);
|
||||
@@ -231,11 +223,7 @@ mod tests {
|
||||
let mut ctx = make_ctx(&m, &layout, ®);
|
||||
ctx.formula_panel_open = true;
|
||||
ctx.category_panel_open = true;
|
||||
let cmd = CyclePanelFocus {
|
||||
formula_open: true,
|
||||
category_open: true,
|
||||
view_open: false,
|
||||
};
|
||||
let cmd = CyclePanelFocus;
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert_eq!(effects.len(), 1);
|
||||
let dbg = effects_debug(&effects);
|
||||
@@ -256,11 +244,7 @@ mod tests {
|
||||
ctx.formula_panel_open = true;
|
||||
ctx.category_panel_open = true;
|
||||
ctx.mode = &AppMode::FormulaPanel;
|
||||
let cmd = CyclePanelFocus {
|
||||
formula_open: true,
|
||||
category_open: true,
|
||||
view_open: false,
|
||||
};
|
||||
let cmd = CyclePanelFocus;
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert_eq!(effects.len(), 1);
|
||||
let dbg = effects_debug(&effects);
|
||||
@@ -281,11 +265,7 @@ mod tests {
|
||||
ctx.category_panel_open = true;
|
||||
ctx.view_panel_open = true;
|
||||
ctx.mode = &AppMode::CategoryPanel;
|
||||
let cmd = CyclePanelFocus {
|
||||
formula_open: false,
|
||||
category_open: true,
|
||||
view_open: true,
|
||||
};
|
||||
let cmd = CyclePanelFocus;
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert_eq!(effects.len(), 1);
|
||||
let dbg = effects_debug(&effects);
|
||||
@@ -305,11 +285,7 @@ mod tests {
|
||||
ctx.formula_panel_open = true;
|
||||
ctx.view_panel_open = true;
|
||||
ctx.mode = &AppMode::ViewPanel;
|
||||
let cmd = CyclePanelFocus {
|
||||
formula_open: true,
|
||||
category_open: false,
|
||||
view_open: true,
|
||||
};
|
||||
let cmd = CyclePanelFocus;
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert_eq!(effects.len(), 1);
|
||||
let dbg = effects_debug(&effects);
|
||||
@@ -329,11 +305,7 @@ mod tests {
|
||||
let mut ctx = make_ctx(&m, &layout, ®);
|
||||
ctx.category_panel_open = true;
|
||||
ctx.mode = &AppMode::CategoryPanel;
|
||||
let cmd = CyclePanelFocus {
|
||||
formula_open: false,
|
||||
category_open: true,
|
||||
view_open: false,
|
||||
};
|
||||
let cmd = CyclePanelFocus;
|
||||
let effects = cmd.execute(&ctx);
|
||||
let dbg = effects_debug(&effects);
|
||||
assert!(
|
||||
@@ -344,15 +316,13 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn move_panel_cursor_down_from_zero() {
|
||||
let m = two_cat_model();
|
||||
let m = with_formulas(&["A", "B"]);
|
||||
let layout = make_layout(&m);
|
||||
let reg = make_registry();
|
||||
let ctx = make_ctx(&m, &layout, ®);
|
||||
let cmd = MovePanelCursor {
|
||||
panel: effect::Panel::Formula,
|
||||
delta: 1,
|
||||
current: 0,
|
||||
max: 5,
|
||||
};
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert_eq!(effects.len(), 1);
|
||||
@@ -363,17 +333,35 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// Evidence: `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn move_panel_cursor_moves_from_the_named_panels_own_cursor() {
|
||||
let m = with_formulas(&["A", "B", "C", "D", "E"]);
|
||||
let layout = make_layout(&m);
|
||||
let reg = make_registry();
|
||||
let mut ctx = make_ctx(&m, &layout, ®);
|
||||
ctx.formula_cursor = 1;
|
||||
let args = ["formula".to_string(), "2".to_string()];
|
||||
let effects = reg
|
||||
.interactive("move-panel-cursor", &args, &ctx)
|
||||
.expect("move-panel-cursor resolves")
|
||||
.execute(&ctx);
|
||||
let dbg = effects_debug(&effects);
|
||||
assert!(
|
||||
dbg.contains("SetPanelCursor { panel: Formula, cursor: 3 }"),
|
||||
"must move from the formula cursor (1) by the parsed delta (2), got: {dbg}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn move_panel_cursor_clamps_at_zero() {
|
||||
let m = two_cat_model();
|
||||
let m = with_formulas(&["A", "B"]);
|
||||
let layout = make_layout(&m);
|
||||
let reg = make_registry();
|
||||
let ctx = make_ctx(&m, &layout, ®);
|
||||
let cmd = MovePanelCursor {
|
||||
panel: effect::Panel::Formula,
|
||||
delta: -1,
|
||||
current: 0,
|
||||
max: 5,
|
||||
};
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert!(effects.is_empty());
|
||||
@@ -388,8 +376,6 @@ mod tests {
|
||||
let cmd = MovePanelCursor {
|
||||
panel: effect::Panel::Formula,
|
||||
delta: 1,
|
||||
current: 0,
|
||||
max: 0,
|
||||
};
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert!(effects.is_empty());
|
||||
@@ -591,16 +577,15 @@ impl Cmd for TogglePanelAndFocus {
|
||||
#[derive(Debug)]
|
||||
pub struct TogglePanelVisibility {
|
||||
pub panel: Panel,
|
||||
pub currently_open: bool,
|
||||
}
|
||||
impl Cmd for TogglePanelVisibility {
|
||||
fn name(&self) -> &'static str {
|
||||
"toggle-panel-visibility"
|
||||
}
|
||||
fn execute(&self, _ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
|
||||
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
|
||||
vec![Box::new(effect::SetPanelOpen {
|
||||
panel: self.panel,
|
||||
open: !self.currently_open,
|
||||
open: !ctx.panel_open(self.panel),
|
||||
})]
|
||||
}
|
||||
}
|
||||
@@ -608,20 +593,16 @@ impl Cmd for TogglePanelVisibility {
|
||||
/// Tab through open panels Formula → Category → View, wrapping, starting from the
|
||||
/// focused one. One open panel self-transitions; none open is a no-op.
|
||||
#[derive(Debug)]
|
||||
pub struct CyclePanelFocus {
|
||||
pub formula_open: bool,
|
||||
pub category_open: bool,
|
||||
pub view_open: bool,
|
||||
}
|
||||
pub struct CyclePanelFocus;
|
||||
impl Cmd for CyclePanelFocus {
|
||||
fn name(&self) -> &'static str {
|
||||
"cycle-panel-focus"
|
||||
}
|
||||
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
|
||||
let order = [
|
||||
(self.formula_open, AppMode::FormulaPanel),
|
||||
(self.category_open, AppMode::CategoryPanel),
|
||||
(self.view_open, AppMode::ViewPanel),
|
||||
(ctx.formula_panel_open, AppMode::FormulaPanel),
|
||||
(ctx.category_panel_open, AppMode::CategoryPanel),
|
||||
(ctx.view_panel_open, AppMode::ViewPanel),
|
||||
];
|
||||
// Index of the first candidate: the panel after the focused one,
|
||||
// or the first panel when invoked from a non-panel mode.
|
||||
@@ -646,16 +627,17 @@ impl Cmd for CyclePanelFocus {
|
||||
pub struct MovePanelCursor {
|
||||
pub panel: Panel,
|
||||
pub delta: i32,
|
||||
pub current: usize,
|
||||
pub max: usize,
|
||||
}
|
||||
impl Cmd for MovePanelCursor {
|
||||
fn name(&self) -> &'static str {
|
||||
"move-panel-cursor"
|
||||
}
|
||||
fn execute(&self, _ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
|
||||
let cursor = self.current;
|
||||
let max = self.max;
|
||||
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
|
||||
let (cursor, max) = match self.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()),
|
||||
};
|
||||
if max == 0 {
|
||||
return vec![];
|
||||
}
|
||||
|
||||
@@ -27,19 +27,21 @@ 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
|
||||
impl AtCursor for Move {
|
||||
fn cursor_mut(&mut self) -> &mut CursorState {
|
||||
&mut self.cursor
|
||||
}
|
||||
}
|
||||
|
||||
/// 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
|
||||
impl AtCursor for EnterAdvance {
|
||||
fn cursor_mut(&mut self) -> &mut CursorState {
|
||||
&mut self.cursor
|
||||
}
|
||||
}
|
||||
|
||||
impl AtCursor for CommitAndAdvance {
|
||||
fn cursor_mut(&mut self) -> &mut CursorState {
|
||||
&mut self.cursor
|
||||
}
|
||||
}
|
||||
|
||||
@@ -61,11 +63,6 @@ fn commit_at_cursor(
|
||||
})
|
||||
}
|
||||
|
||||
/// 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 {
|
||||
@@ -90,7 +87,6 @@ pub fn default_registry() -> CmdRegistry {
|
||||
key: parse_cell_key_from_args(args),
|
||||
})
|
||||
},
|
||||
as_parsed,
|
||||
Some(|_, ctx| {
|
||||
Ok(ClearCellCommand {
|
||||
key: cursor_key(ctx)?,
|
||||
@@ -121,7 +117,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
r.register_pure(&HelpPagePrevCmd(vec![]), HelpPagePrevCmd::parse);
|
||||
|
||||
// ── Navigation (unified Move) ──────────────────────────────────────
|
||||
r.register(
|
||||
r.register_at_cursor(
|
||||
&Move {
|
||||
kind: MoveKind::Relative(0, 0),
|
||||
cursor: CursorState::default(),
|
||||
@@ -137,13 +133,12 @@ pub fn default_registry() -> CmdRegistry {
|
||||
cmd_name: "move-selection",
|
||||
})
|
||||
},
|
||||
move_from_cursor,
|
||||
None,
|
||||
);
|
||||
// Jump-to-edge commands: first/last row/col
|
||||
macro_rules! reg_jump {
|
||||
($r:expr, $is_row:expr, $to_end:expr, $name:expr) => {
|
||||
$r.register(
|
||||
$r.register_at_cursor(
|
||||
&Move {
|
||||
kind: if $to_end {
|
||||
MoveKind::ToEnd($is_row)
|
||||
@@ -164,7 +159,6 @@ pub fn default_registry() -> CmdRegistry {
|
||||
cmd_name: $name,
|
||||
})
|
||||
},
|
||||
move_from_cursor,
|
||||
None,
|
||||
);
|
||||
};
|
||||
@@ -173,7 +167,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
reg_jump!(r, true, true, "jump-last-row");
|
||||
reg_jump!(r, false, false, "jump-first-col");
|
||||
reg_jump!(r, false, true, "jump-last-col");
|
||||
r.register(
|
||||
r.register_at_cursor(
|
||||
&Move {
|
||||
kind: MoveKind::Relative(0, 0),
|
||||
cursor: CursorState::default(),
|
||||
@@ -188,10 +182,9 @@ pub fn default_registry() -> CmdRegistry {
|
||||
cmd_name: "scroll-rows",
|
||||
})
|
||||
},
|
||||
move_from_cursor,
|
||||
None,
|
||||
);
|
||||
r.register(
|
||||
r.register_at_cursor(
|
||||
&Move {
|
||||
kind: MoveKind::Page(0),
|
||||
cursor: CursorState::default(),
|
||||
@@ -206,10 +199,9 @@ pub fn default_registry() -> CmdRegistry {
|
||||
cmd_name: "page-scroll",
|
||||
})
|
||||
},
|
||||
move_from_cursor,
|
||||
None,
|
||||
);
|
||||
r.register(
|
||||
r.register_at_cursor(
|
||||
&EnterAdvance {
|
||||
cursor: CursorState::default(),
|
||||
},
|
||||
@@ -227,9 +219,6 @@ pub fn default_registry() -> CmdRegistry {
|
||||
},
|
||||
})
|
||||
},
|
||||
|_, ctx| EnterAdvance {
|
||||
cursor: CursorState::from_ctx(ctx),
|
||||
},
|
||||
None,
|
||||
);
|
||||
|
||||
@@ -246,7 +235,6 @@ pub fn default_registry() -> CmdRegistry {
|
||||
key: parse_cell_key_from_args(args),
|
||||
})
|
||||
},
|
||||
as_parsed,
|
||||
Some(|_, ctx| {
|
||||
Ok(YankCell {
|
||||
key: cursor_key(ctx)?,
|
||||
@@ -265,7 +253,6 @@ pub fn default_registry() -> CmdRegistry {
|
||||
key: parse_cell_key_from_args(args),
|
||||
})
|
||||
},
|
||||
as_parsed,
|
||||
Some(|_, ctx| {
|
||||
Ok(PasteCell {
|
||||
key: cursor_key(ctx)?,
|
||||
@@ -318,7 +305,6 @@ pub fn default_registry() -> CmdRegistry {
|
||||
key: parse_cell_key_from_args(args),
|
||||
})
|
||||
},
|
||||
as_parsed,
|
||||
Some(|_, ctx| {
|
||||
Ok(DrillIntoCell {
|
||||
key: cursor_key(ctx)?,
|
||||
@@ -356,77 +342,30 @@ pub fn default_registry() -> CmdRegistry {
|
||||
});
|
||||
Ok(TogglePanelAndFocus { panel, target })
|
||||
},
|
||||
as_parsed,
|
||||
None,
|
||||
);
|
||||
r.register(
|
||||
&TogglePanelVisibility {
|
||||
panel: Panel::Formula,
|
||||
currently_open: false,
|
||||
},
|
||||
|args| {
|
||||
require_args(args, 1)?;
|
||||
let panel = parse_panel(&args[0])?;
|
||||
Ok(TogglePanelVisibility {
|
||||
panel,
|
||||
currently_open: false,
|
||||
})
|
||||
},
|
||||
|cmd, ctx| TogglePanelVisibility {
|
||||
currently_open: ctx.panel_open(cmd.panel),
|
||||
..cmd
|
||||
},
|
||||
None,
|
||||
);
|
||||
r.register(
|
||||
&CyclePanelFocus {
|
||||
formula_open: false,
|
||||
category_open: false,
|
||||
view_open: false,
|
||||
},
|
||||
|_| {
|
||||
Ok(CyclePanelFocus {
|
||||
formula_open: false,
|
||||
category_open: false,
|
||||
view_open: false,
|
||||
})
|
||||
},
|
||||
|_, ctx| CyclePanelFocus {
|
||||
formula_open: ctx.formula_panel_open,
|
||||
category_open: ctx.category_panel_open,
|
||||
view_open: ctx.view_panel_open,
|
||||
Ok(TogglePanelVisibility { panel })
|
||||
},
|
||||
None,
|
||||
);
|
||||
r.register_nullary(|| Box::new(CyclePanelFocus));
|
||||
r.register(
|
||||
&MovePanelCursor {
|
||||
panel: Panel::Formula,
|
||||
delta: 0,
|
||||
current: 0,
|
||||
max: 0,
|
||||
},
|
||||
|args| {
|
||||
require_args(args, 2)?;
|
||||
let panel = parse_panel(&args[0])?;
|
||||
let delta = args[1].parse::<i32>().map_err(|e| e.to_string())?;
|
||||
Ok(MovePanelCursor {
|
||||
panel,
|
||||
delta,
|
||||
current: 0,
|
||||
max: 0,
|
||||
})
|
||||
},
|
||||
|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()),
|
||||
};
|
||||
MovePanelCursor {
|
||||
current,
|
||||
max,
|
||||
..cmd
|
||||
}
|
||||
Ok(MovePanelCursor { panel, delta })
|
||||
},
|
||||
None,
|
||||
);
|
||||
@@ -493,7 +432,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
|
||||
// ── Commit ───────────────────────────────────────────────────────────
|
||||
// These take a mode-name arg as args[0]; the command never inspects ctx.mode.
|
||||
r.register(
|
||||
r.register_at_cursor(
|
||||
&CommitAndAdvance {
|
||||
key: CellKey::new(vec![]),
|
||||
value: String::new(),
|
||||
@@ -514,10 +453,9 @@ pub fn default_registry() -> CmdRegistry {
|
||||
edit_mode,
|
||||
})
|
||||
},
|
||||
commit_from_cursor,
|
||||
Some(|args, ctx| commit_at_cursor(args, ctx, AdvanceDir::Down)),
|
||||
);
|
||||
r.register(
|
||||
r.register_at_cursor(
|
||||
&CommitAndAdvance {
|
||||
key: CellKey::new(vec![]),
|
||||
value: String::new(),
|
||||
@@ -526,7 +464,6 @@ pub fn default_registry() -> CmdRegistry {
|
||||
edit_mode: AppMode::editing(),
|
||||
},
|
||||
|_| Err("commit-and-advance-right requires context".into()),
|
||||
commit_from_cursor,
|
||||
Some(|args, ctx| commit_at_cursor(args, ctx, AdvanceDir::Right)),
|
||||
);
|
||||
r.register(
|
||||
@@ -538,7 +475,6 @@ pub fn default_registry() -> CmdRegistry {
|
||||
let kind = parse_commit_kind(&args[0], &AppMode::Normal)?;
|
||||
Ok(CommitBuffer { kind })
|
||||
},
|
||||
as_parsed,
|
||||
Some(|args, ctx| {
|
||||
require_args(args, 1)?;
|
||||
let kind = parse_commit_kind(&args[0], ctx.mode)?;
|
||||
@@ -863,12 +799,16 @@ mod panel_open_tests {
|
||||
let ctx = app.cmd_context(KeyCode::Null, KeyModifiers::NONE);
|
||||
for asked in Panel::ALL {
|
||||
let args = [asked.name().to_string()];
|
||||
let cmd = ctx
|
||||
let effects = ctx
|
||||
.registry
|
||||
.interactive("toggle-panel-visibility", &args, &ctx)
|
||||
.expect("toggle-panel-visibility resolves");
|
||||
let dbg = format!("{cmd:?}");
|
||||
let expected = format!("currently_open: {}", asked == opened);
|
||||
.expect("toggle-panel-visibility resolves")
|
||||
.execute(&ctx);
|
||||
let dbg = format!("{effects:?}");
|
||||
let expected = format!(
|
||||
"SetPanelOpen {{ panel: {asked:?}, open: {} }}",
|
||||
asked != opened
|
||||
);
|
||||
assert!(
|
||||
dbg.contains(&expected),
|
||||
"with only {opened:?} open, toggling {asked:?} read another \
|
||||
|
||||
Reference in New Issue
Block a user