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

Merge pull request #316 from fiddlerwoaroof/fix/31oe-cmdlang

Browse Source 
fix(command): pipe-quoted arguments and refusals for unknown names (improvise-31oe.2, 31oe.3)
This commit is contained in:
Ed Langley
2026-10-02 22:30:46 -07:00
committed by GitHub
parent 8da5a3101b 8fa504fbdc
commit 6693178530
14 changed files with 1201 additions and 199 deletions
Download Patch File Download Diff File Expand all files Collapse all files
context/code/cli.md
+113
View File
@@ -425,3 +425,116 @@ check. The rest come from `build_model_with_warnings` as an `Err`. All of
them reach `main` as exit 1 before `persistence::save`. So "writes nothing"
holds by statement order, not by where the refusal is raised. The headless
tests check it by asserting that the output path does not exist.
## 14 — the command language reads `|pipe quoting|`, and a surplus argument is refused (improvise-31oe.3)
`create-view |By area|` used to make a view named `|By` and drop `area|` without
a word. Two separate defects, fixed separately.
**The quoting.** `parse.rs` now has one lexer, `lex`, and everything else is
built on it: `parse_line_with`, `segments` and `parse_segment_interactive`. The
rules are:
- A `|…|` piece is read by `improvise_formula::quote::unescape_pipe_body`, the
one home of pipe quoting (gotcha 34). So `\|`, `\\` and `\n` mean exactly what
they mean in `.improv` and in formulas. The lexer only finds the closing pipe:
a backslash takes the next character with it, so `\|` does not close.
- A piece may sit **anywhere in a word**, and the pieces and bare text are
joined. So a coordinate quotes each half, `|Sales Region|/|East Coast|`, the
way a `.improv` data line quotes each name. The alternative, a pipe only at
the start of a word, would have left no way to spell a coordinate with a
space in it.
- `"…"` is **kept**, verbatim, with no escapes, and a `|` inside it stays a
`|`. It is not a workaround to retire. Formula text carries its own pipes:
`add-formula "Net = |Gross Rev| - Cost"` must reach `parse_formula` with the
pipes on. If the line had pipes only, every pipe in a formula would need
escaping. Its old leniency is kept too: a `"` that is never closed runs to
the end of the line. A `|` that is never closed is refused, naming the text
from the open pipe on, because a name is never meant to run to the end.
- The separator is a **bare** `.` word. A quoted `|.|` or `"."` is an argument,
and a `.` inside `|a . b|` does not split the line. The old `split_on_dot`
was a second scanner that knew double quotes and not pipes. With one lexer
there is no second scanner to disagree.
- A comment line (`#`, `//`) is not lexed, so an unmatched pipe in a comment is
not an error.
`segments` now returns `Result`, because an unterminated pipe is a fact about
the whole line. `main.rs` refuses the line before running any of it, as §7 does
for a parse error, and `ExecuteCommand` refuses it on the status line.
What is still not spellable: an item whose name contains `/`. The coordinate is
split on the first `/` **after** unquoting, so `|a/b|/x` is still read as
category `a` and item `b/x`. Closing that needs the lexer to hand structured
coordinates to the commands, which is a larger change than this ticket.
**The arity.** Every registration now declares `MaxArgs::AtMost(n)` or
`MaxArgs::Any` beside its prototype. `CmdRegistry::parse` and
`CmdRegistry::interactive` apply it after construction succeeds. So the rule is
written once and every command gets it, and a new registration cannot compile
without a declaration. The refusal names the command, the limit and every
surplus argument (`create-view: takes at most 1 argument(s), got 2; unexpected
"area"`). A headless run exits non-zero through the §7 parse gate, and the `:`
line refuses on the status line.
It runs **after** the command's own parse, deliberately. A command whose own
refusal states its shape keeps it: `add-formula`'s "quote the formula"
(improvise-z05h) is more use than an arity, and it is what `formula Product
Total = Widget + Gadget` still says.
`MaxArgs::Any` is a claim, and the list of commands making it is short:
`add-items`, `set-cell`, `set-format` (it joins its words), `clear-cell`,
`yank`, `paste`, `drill-into-cell` and `commit-cell-edit`. Each takes a list of
items or coordinates. `no_command_silently_drops_an_argument_it_was_given` names
that list, so widening another command to `Any` has to add it there.
## 15 — a command that names something the document lacks is refused (improvise-31oe.2)
`set-axis Regoin row`, `set-page Region Nowhere` and `set-axis Region sideways`
all exited 0 and said nothing. §10's rule applies: nothing changed, so the
request was not met.
- **An unreadable axis is an argument error.** `set-axis` now checks its axis
with `parse_axis` when it is parsed, so a script is refused before any of it
runs. `execute` still refuses an axis it cannot read rather than returning
nothing. That arm cannot be reached through the registry.
- **An unknown category, item, group or view is a refusal**, from `RequireKnown`
(below), because only the document knows what it holds. `effect::unknown_name`
asks the active view (`try_axis_of`) for a category. That is exactly the
condition under which `View::set_axis` was a silent no-op. It asks
`effective_item_names` for an item, so a formula target is a known `_Measure`
item. It asks both an item's own group and the category's `groups` for a
group. `add_item_in_group` registers no `Group`, and a parent group has no
items, so either one alone misses a real group. A view is asked of
`workbook.views`.
The siblings fixed with it: `hide-item` and `show-item` (hiding an unknown item
wrote it into `hidden_items`), `toggle-group`, and `switch-view` (the effect
ignores a view it lacks, and says nothing). `set-page` with an unknown item was
worse than a no-op: it wrote the name into `page_selections`.
The check runs at **apply** time, as an effect of its own. `execute` runs
against one context built before the line ran, and a `:` line builds every
segment up front, so a check in `execute` refused
`create-view Second . switch-view Second` and
`add-item Type Tools . set-page Type Tools` after their first halves had landed,
while `improvise cmd` (which rebuilds the context per segment) accepted them.
Each of these commands therefore returns `RequireKnown(needs)` followed by the
real effect. `RequireKnown` refuses and aborts the batch (gotcha 21), so the
real effect never applies. It is a `ViewOnly` effect that writes nothing and is
off the wire on purpose: the wire's own effects (`model_effects`, gotcha 38)
stay non-refusing, since a refusing effect there becomes a frame rejection
(`MutationRefused`). That is arguably right, but it changes the protocol, and
a browser composes these only from names it was sent. It is left for a ticket
of its own.
A refused name stops the rest of a `:` line, because the abort ends the one batch
the whole line runs as. Headless runs each segment as its own batch, so an abort
never crosses a `.` there: `:set-page Type Nowhere . add-item Type X` adds
nothing, while `improvise cmd` on the same line exits 1 and still adds `X`.
`load`/`Save` already behaved this way. The two front ends' chain semantics are
left for a ticket of their own.
The interactive cursor fallbacks of `clear-cell`, `yank`, `paste` and
`drill-into-cell` answer only for coordinates that were OMITTED
(`only_when_omitted`). Coordinates that were given and are malformed are
refused, never replaced by the cursor's cell.
context/code/command.md
+18 -2
View File
@@ -2996,8 +2996,10 @@ ride in `ViewState::transient_prefix`, so they must live where the wire types do
and the re-export keeps `crate::command::key::…` paths working.
`parse.rs` is the quasi-lisp prefix parser: `word arg1 arg2 …`, several commands per
line separated by `.`, coordinate pairs written `Category/Item`, and quoted strings
(`"Profit = Revenue - Cost"`).
line separated by `.`, coordinate pairs written `Category/Item`, verbatim quoted
strings (`"Profit = Revenue - Cost"`) and `|pipe-quoted names|` read by
`improvise_formula::quote`. One lexer serves every entry point, so the separator
and the quoting cannot disagree (improvise-31oe.3, `context/code/cli.md` §14).
### `cmd/core.rs`
@@ -3014,6 +3016,12 @@ 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`.
Every registration also declares its `MaxArgs`, and the registry refuses surplus
arguments on both arms after the command is built. `register_nullary` declares
`AtMost(0)` for itself, and an `effect_cmd!` declares its limit in the macro
call as `MAX_ARGS`. So the arity rule is written once and is total over the
registry (improvise-31oe.3, `context/code/cli.md` §14).
`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
map but its own `ViewState` field (improvise-gdl, gotcha 11). That knowledge used to
@@ -3372,6 +3380,14 @@ The parseable model-mutation commands are thin `Cmd` wrappers around effects,
constructible from string args, sharing the same execution path as
keymap-dispatched commands (gotcha 16).
A wrapper that names a category, item, group or view returns
`effect::RequireKnown(Needs)` followed by its real effect (improvise-31oe.2).
`RequireKnown` asks `unknown_name` against the live app when the batch applies
and refuses and aborts if the document lacks the name, so a `:` line sees what
its earlier segments made. The effects themselves stay silent about unknown
names, because they are also the wire's mutation path. Why the check is a
separate effect: `context/code/cli.md` §15.
### `cmd/wizard.rs` (improvise-2wm, improvise-907)
Import-wizard key handling. The wizard is a small step machine, and which key means
context/test-evidence/cli.md
+110
View File
@@ -510,6 +510,116 @@ directly imports without any `File`. The rival that merges every all-CSV
import, lone files included, fails that half ("a lone file keeps today's
shape"). So the test separates "a folder merges" from "everything merges".
### a_pipe_quoted_argument_is_one_argument_without_its_pipes
improvise-31oe.3. Red before the fix: the old tokenizer split on whitespace
only, so `create-view |By area|` built `CreateViewCmd(["|By", "area|"])`. It
asserts the whole built command, so a lexer that joined the words but kept the
pipes fails as well as one that split them. The second case has two quoted
arguments side by side, so a lexer that reads one pipe piece per line fails.
Rival: `bare_word` pushing `|` as a plain character. Fails here and in
`a_coordinate_may_pipe_quote_either_half`.
### a_pipe_quoted_argument_reads_the_escapes_improv_does
The three escapes `pipe_quote` writes, `\|`, `\\` and `\n`, each in its own
case. Red before the fix. Rival: `pipe_body` returning the raw body without
`unescape_pipe_body`. Fails here, at the first case, and nowhere else in the
crate, which is why the case exists. A lexer that treated `\|` as a closing
pipe would also fail the first case, because the pipe after `b` then opens a
piece that never closes.
### a_command_given_more_arguments_than_it_takes_is_refused
improvise-31oe.3's second half. Red before the fix: `create-view By area`
parsed and dropped `area`. It asserts the refusal opens with the command's name
and quotes the surplus word, and that the pipe-quoted spelling of the same line
is the control that parses. Rival: `MaxArgs::admit` answering `Ok` always.
Fails here, in `no_command_silently_drops_an_argument_it_was_given`, and in
`a_colon_line_naming_something_missing_refuses_on_the_status_line`.
### no_command_silently_drops_an_argument_it_was_given
The rule is one rule only if every registration obeys it, so this enumerates
every registered name against every candidate argument list, on both arms. For
each list a command accepts, it appends **four** surplus words. One would be
absorbed by an optional argument (`add-formula ["1"]` plus one more is a valid
two-argument call), and four is more than any optional span in the registry.
The last word must be named in the refusal. The exceptions are stated, not
inferred: the `Any` list, and three commands whose own shape refusal runs
first. Rivals, each run: declaring `add-item-in-group` as `Any` fails here,
naming the dropped list. Removing the check from the interactive arm alone
fails here and in the `:` line test. A test of the pure arm alone would have
missed that.
### a_command_naming_something_the_document_lacks_is_a_refusal
improvise-31oe.2 and its siblings: `set-axis`, `set-page`, `hide-item`,
`show-item`, `toggle-group` and `switch-view`, each naming a missing category,
item, group or view. Red before the fix at the first row, `set-axis Regoin
column`, with an empty status. Each row asserts the refusal flag, the missing
name in the message, and that the document's `.improv` text and the active view
are unchanged. The last two catch a refusal written after the write. The
control is `a_command_naming_what_the_document_has_is_not_refused`, whose group
row is the one that caught a group check reading only `Category::groups`:
`add_item_in_group` registers no `Group`. The checks are `effect::unknown_name`,
run by `RequireKnown` when the batch applies. Rivals, each run: the
`Needs::Item` arm answering `None` fails at `set-page ["Type", "Nowhere"]`. The
category test (`try_axis_of(..).is_none()`) never firing fails at the first
row. The `Needs::View` arm answering `None` fails at the `switch-view` row.
`RequireKnown` placed after the write, or refusing without `refuse_and_stop`'s
abort, fails the document-unchanged assertion.
### a_colon_line_naming_something_missing_refuses_on_the_status_line
The TUI path for both tickets, through `ExecuteCommand` and `App::apply_effects`,
not through the command alone. It asserts the refusal flag, the name on the
status line, and the mode back at Normal. Rivals: an unknown category accepted
fails at `:set-axis Regoin row`, and the arity check removed from the
interactive arm fails at `:create-view By area`.
### an_unknown_category_axis_or_item_exits_non_zero_and_names_it
The three repros from improvise-31oe.2, at the process boundary. Red against
the `gh/main` source with this test file: `set-axis Regoin row` exited 0 with an
empty stderr. The control, `set-page Region East`, must exit 0. Without it, a
fix that refused every `set-page` would pass.
### a_pipe_quoted_name_reaches_the_document_whole
improvise-31oe.3's repro at the process boundary: the saved document must carry
`## View: By area` and no `|By`. The second half, `create-view By area`, must
exit non-zero naming `"area"`. Red against the `gh/main` source: the document
carried the view `|By`.
### command::cmd::registry::tests::a_malformed_coordinate_is_refused_not_dropped
Codex's P2 on PR #316: `set-cell` and `parse_cell_key_from_args` (`clear-cell`,
`yank`, `paste`, `drill-into-cell`, `commit-cell-edit`) built coordinates with
`filter_map(split_once('/'))`, so `set-cell 1 Type/Food unexpected` dropped
`unexpected` without a word. The test names every caller, a half-empty
coordinate, and a control line that must still parse. Rival: the `filter_map`
restored in `parse_cell_key_from_args` fails at the first line,
`set-cell 1 Type/Food unexpected dropped "unexpected" without a word`.
### command::cmd::text_buffer::tests::colon_refuses_a_malformed_coordinate_and_leaves_the_cursor_cell_alone
The PR #316 review's blocker: on `:`, `parse_cell_key_from_args` failing sent
`clear-cell`, `yank`, `paste` and `drill-into-cell` into the cursor fallback,
which ignores its arguments. `:clear-cell Type/Clothing Month/Feb junk` cleared
the cursor's Food/Jan without a word. The test runs each line through the full
`:` path and asserts a refusal naming `"junk"`, both cells intact, nothing
yanked and no drill opened. Rival: `only_when_omitted` made unconditionally
`Ok` fails it at the first line.
### command::cmd::text_buffer::tests::colon_lets_a_later_segment_use_what_an_earlier_one_created
The review's second blocker: a `:` line is built before any segment runs, so the
name checks in `execute` refused `create-view Second . switch-view Second` and
`add-item Type Tools . set-page Type Tools` after the first half landed. The
checks now run when the effect applies. Rival: the checks back in `execute`
fails both lines with `No view named "Second"` and `Unknown item 'Tools'`.
### a_strict_import_that_skips_a_record_refuses_and_writes_nothing
improvise-8jk6.6, the user's ruling on PR #321. Written before `--strict`
crates/improvise-command/src/command/cmd/core.rs
+61 -13
View File
@@ -250,12 +250,37 @@ pub trait AtCursor: Cmd {
fn cursor_mut(&mut self) -> &mut CursorState;
}
/// How many arguments a command takes. The registry refuses any beyond it, for
/// every command in one place — `context/code/cli.md` §14.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum MaxArgs {
AtMost(usize),
Any,
}
impl MaxArgs {
fn admit(self, args: &[String]) -> Result<(), String> {
match self {
MaxArgs::AtMost(n) if args.len() > n => {
let extra: Vec<String> = args[n..].iter().map(|a| format!("{a:?}")).collect();
Err(format!(
"takes at most {n} argument(s), got {}; unexpected {}",
args.len(),
extra.join(" ")
))
}
MaxArgs::AtMost(_) | MaxArgs::Any => Ok(()),
}
}
}
type BoxParseFn = Box<dyn Fn(&[String]) -> Result<Box<dyn Cmd>, String>>;
type BoxInteractiveFn = Box<dyn Fn(&[String], &CmdContext) -> Result<Box<dyn Cmd>, String>>;
/// A registered command entry with both text and interactive constructors.
struct CmdEntry {
name: &'static str,
max: MaxArgs,
parse: BoxParseFn,
interactive: BoxInteractiveFn,
}
@@ -295,10 +320,11 @@ impl CmdRegistry {
pub fn register<C: Cmd + 'static>(
&mut self,
prototype: &C,
max: MaxArgs,
parse: fn(&[String]) -> Result<C, String>,
fallback: Option<FallbackFn<C>>,
) {
self.push(prototype.name(), parse, fallback, |_, _| {});
self.push(prototype.name(), max, parse, fallback, |_, _| {});
}
/// As [`register`](Self::register), then the interactive arm moves the
@@ -306,10 +332,11 @@ impl CmdRegistry {
pub fn register_at_cursor<C: AtCursor + 'static>(
&mut self,
prototype: &C,
max: MaxArgs,
parse: fn(&[String]) -> Result<C, String>,
fallback: Option<FallbackFn<C>>,
) {
self.push(prototype.name(), parse, fallback, |cmd, ctx| {
self.push(prototype.name(), max, parse, fallback, |cmd, ctx| {
*cmd.cursor_mut() = CursorState::from_ctx(ctx);
});
}
@@ -317,12 +344,14 @@ impl CmdRegistry {
fn push<C: Cmd + 'static>(
&mut self,
name: &'static str,
max: MaxArgs,
parse: fn(&[String]) -> Result<C, String>,
fallback: Option<FallbackFn<C>>,
at_cursor: fn(&mut C, &CmdContext),
) {
self.entries.push(CmdEntry {
name,
max,
parse: Box::new(move |args| parse(args).map(|c| Box::new(c) as Box<dyn Cmd>)),
interactive: Box::new(move |args, ctx| {
let mut built = match (parse(args), fallback) {
@@ -338,9 +367,10 @@ impl CmdRegistry {
/// Register a command that needs no interactive context: BOTH arms are
/// `parse`, so neither can demote the other — `context/code/command.md`.
pub fn register_pure(&mut self, prototype: &dyn Cmd, parse: ParseFn) {
pub fn register_pure(&mut self, prototype: &dyn Cmd, max: MaxArgs, parse: ParseFn) {
self.entries.push(CmdEntry {
name: prototype.name(),
max,
parse: Box::new(parse),
interactive: Box::new(move |args, _ctx| parse(args)),
});
@@ -352,6 +382,7 @@ impl CmdRegistry {
let name = f().name();
self.entries.push(CmdEntry {
name,
max: MaxArgs::AtMost(0),
parse: Box::new(move |_| Ok(f())),
interactive: Box::new(move |_, _| Ok(f())),
});
@@ -364,7 +395,9 @@ impl CmdRegistry {
let canonical = self.resolve(name);
for e in &self.entries {
if e.name == canonical {
return (e.parse)(args).map_err(|err| format!("{name}: {err}"));
return (e.parse)(args)
.and_then(|cmd| e.max.admit(args).map(|()| cmd))
.map_err(|err| format!("{name}: {err}"));
}
}
Err(format!("Unknown command: {name}"))
@@ -381,7 +414,7 @@ impl CmdRegistry {
let name = self.resolve(name);
for e in &self.entries {
if e.name == name {
return (e.interactive)(args, ctx);
return (e.interactive)(args, ctx).and_then(|cmd| e.max.admit(args).map(|()| cmd));
}
}
Err(format!("Unknown command: {name}"))
@@ -401,16 +434,31 @@ pub(super) fn require_args(args: &[String], n: usize) -> Result<(), String> {
}
}
/// Parse Cat/Item coordinate args into a CellKey.
pub(super) fn parse_cell_key_from_args(args: &[String]) -> crate::model::cell::CellKey {
let coords: Vec<(String, String)> = args
/// Parse Cat/Item coordinate args into a CellKey, refusing any token that is not
/// one rather than dropping it — `context/code/cli.md` §14.
pub(super) fn parse_cell_key_from_args(
args: &[String],
) -> Result<crate::model::cell::CellKey, String> {
let coords = args
.iter()
.filter_map(|s| {
let (cat, item) = s.split_once('/')?;
Some((cat.to_string(), item.to_string()))
.map(|s| match s.split_once('/') {
Some((cat, item)) if !cat.is_empty() && !item.is_empty() => {
Ok((cat.to_string(), item.to_string()))
}
_ => Err(format!("{s:?} is not a Category/Item coordinate")),
})
.collect();
crate::model::cell::CellKey::new(coords)
.collect::<Result<Vec<_>, _>>()?;
Ok(crate::model::cell::CellKey::new(coords))
}
/// A cursor fallback answers only for coordinates that were OMITTED; given and
/// wrong, they are refused, never the cursor's cell — `context/code/cli.md` §14.
pub(super) fn only_when_omitted(args: &[String]) -> Result<(), String> {
if args.is_empty() {
return Ok(());
}
parse_cell_key_from_args(args)?;
Err("takes Category/Item coordinates, or none to use the cursor".to_string())
}
/// Read the current value of a named buffer from context.
crates/improvise-command/src/command/cmd/effect_cmds.rs
+192 -48
View File
@@ -1,13 +1,13 @@
use crate::model::cell::CellValue;
use crate::ui::effect::{self, Effect, ViewTarget};
use crate::view::Axis;
use super::core::{Cmd, CmdContext, require_args};
use super::core::{Cmd, CmdContext, MaxArgs, parse_axis, parse_cell_key_from_args, require_args};
#[cfg(test)]
mod tests {
use super::*;
use crate::command::cmd::test_helpers::*;
use crate::view::Axis;
#[test]
fn add_category_cmd_produces_add_category_effect() {
@@ -59,7 +59,7 @@ mod tests {
let ctx = make_ctx(&m, &layout, &reg);
let cmd = SetAxisCmd(vec!["Type".to_string(), "col".to_string()]);
let effects = cmd.execute(&ctx);
assert_eq!(effects.len(), 1);
assert_eq!(effects.len(), 2, "the name check, then the write");
let mut app = crate::ui::app::App::new(two_cat_model(), None);
app.apply_effects(effects);
let view = app.model_state.workbook.active_view();
@@ -95,6 +95,104 @@ mod tests {
assert!(present.model_state.workbook.model.formulas().is_empty());
}
/// Run `name args` built by the pure arm against a fresh `App` on `wb`.
fn run_on(wb: &crate::workbook::Workbook, name: &str, args: &[&str]) -> crate::ui::app::App {
let args: Vec<String> = args.iter().map(|s| s.to_string()).collect();
let mut app = crate::ui::app::App::new(wb.clone(), None);
let cmd = crate::command::cmd::registry::default_registry()
.parse(name, &args)
.unwrap_or_else(|e| panic!("`{name} {args:?}` must build: {e}"));
let effects = {
let ctx = app.cmd_context(
crate::command::key::KeyCode::Null,
crate::command::key::KeyModifiers::NONE,
);
cmd.execute(&ctx)
};
app.apply_effects(effects);
app
}
/// A document where every sibling below has something real to name.
fn named_things() -> crate::workbook::Workbook {
let mut wb = two_cat_model();
wb.add_item_in_group("Type", "Tools", "Hardware").unwrap();
wb.create_view("Second");
wb
}
/// improvise-31oe.2 and its siblings: each names something missing, is
/// refused naming it, and changes nothing. Evidence: `context/test-evidence/cli.md`.
#[test]
fn a_command_naming_something_the_document_lacks_is_a_refusal() {
let wb = named_things();
let cases: [(&str, &[&str], &str); 10] = [
("set-axis", &["Regoin", "column"], "Regoin"),
("set-page", &["Regoin", "Food"], "Regoin"),
("set-page", &["Type", "Nowhere"], "Nowhere"),
("hide-item", &["Regoin", "Food"], "Regoin"),
("hide-item", &["Type", "Nowhere"], "Nowhere"),
("show-item", &["Regoin", "Food"], "Regoin"),
("show-item", &["Type", "Nowhere"], "Nowhere"),
("toggle-group", &["Regoin", "Hardware"], "Regoin"),
("toggle-group", &["Type", "Nowhere"], "Nowhere"),
("switch-view", &["Nowhere"], "Nowhere"),
];
let untouched = crate::persistence::format_md(&wb);
for (name, args, missing) in cases {
let app = run_on(&wb, name, args);
let said = &app.view_state.status_msg;
assert!(
app.last_batch_refused(),
"`{name} {args:?}` was not refused: {said:?}"
);
assert!(
said.contains(missing),
"`{name} {args:?}` did not name {missing}: {said:?}"
);
assert_eq!(
crate::persistence::format_md(&app.model_state.workbook),
untouched,
"`{name} {args:?}` changed the document"
);
assert_eq!(
app.view_state.active_view, wb.active_view,
"`{name}` moved the view"
);
}
}
/// The controls: the same commands naming what IS there are not refused.
#[test]
fn a_command_naming_what_the_document_has_is_not_refused() {
let wb = named_things();
let cases: [(&str, &[&str]); 6] = [
("set-axis", &["Type", "column"]),
("set-page", &["Type", "Clothing"]),
("hide-item", &["Type", "Clothing"]),
("show-item", &["Type", "Clothing"]),
("toggle-group", &["Type", "Hardware"]),
("switch-view", &["Second"]),
];
for (name, args) in cases {
let app = run_on(&wb, name, args);
assert!(
!app.last_batch_refused(),
"`{name} {args:?}` was refused: {:?}",
app.view_state.status_msg
);
}
}
/// An axis keyword is an argument error, caught before anything runs.
#[test]
fn set_axis_with_an_unknown_axis_is_an_argument_error() {
let err = crate::command::parse_line("set-axis Type sideways")
.expect_err("sideways is not an axis");
assert!(err.starts_with("set-axis:"), "names the command: {err}");
assert!(err.contains("sideways"), "names the axis: {err}");
}
#[test]
fn write_cmd_without_args_saves() {
let m = two_cat_model();
@@ -129,7 +227,7 @@ mod tests {
// Thin Cmd wrappers around effects, on the same path as keymap dispatch.
macro_rules! effect_cmd {
($name:ident, $cmd_name:expr, $parse:expr, $exec:expr) => {
($name:ident, $cmd_name:expr, $max:expr, $parse:expr, $exec:expr) => {
#[derive(Debug)]
pub struct $name(pub Vec<String>);
impl Cmd for $name {
@@ -143,6 +241,7 @@ macro_rules! effect_cmd {
}
}
impl $name {
pub const MAX_ARGS: MaxArgs = $max;
pub fn parse(args: &[String]) -> Result<Box<dyn Cmd>, String> {
#[allow(clippy::redundant_closure_call)]
($parse)(args)?;
@@ -152,9 +251,16 @@ macro_rules! effect_cmd {
};
}
/// `effect`, preceded by the check that what it names exists, run at APPLY time:
/// a `:` line is built before any of it runs — `context/code/cli.md` §15.
fn unless_unknown(needs: effect::Needs, effect: impl Effect + 'static) -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::RequireKnown(needs)), Box::new(effect)]
}
effect_cmd!(
AddCategoryCmd,
"add-category",
MaxArgs::AtMost(1),
|args: &[String]| require_args(args, 1),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::AddCategory(args[0].clone()))]
@@ -164,6 +270,7 @@ effect_cmd!(
effect_cmd!(
AddItemCmd,
"add-item",
MaxArgs::AtMost(2),
|args: &[String]| require_args(args, 2),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::AddItem {
@@ -176,6 +283,7 @@ effect_cmd!(
effect_cmd!(
AddItemsCmd,
"add-items",
MaxArgs::Any,
|args: &[String]| {
if args.len() < 2 {
Err("add-items requires a category and at least one item".to_string())
@@ -200,6 +308,7 @@ effect_cmd!(
effect_cmd!(
AddItemInGroupCmd,
"add-item-in-group",
MaxArgs::AtMost(3),
|args: &[String]| require_args(args, 3),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::AddItemInGroup {
@@ -213,11 +322,12 @@ effect_cmd!(
effect_cmd!(
SetCellCmd,
"set-cell",
MaxArgs::Any,
|args: &[String]| {
if args.len() < 2 {
Err("set-cell requires a value and at least one Cat/Item coordinate".to_string())
} else {
Ok(())
parse_cell_key_from_args(&args[1..]).map(|_| ())
}
},
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
@@ -226,21 +336,17 @@ effect_cmd!(
} else {
CellValue::Text(args[0].clone())
};
let coords: Vec<(String, String)> = args[1..]
.iter()
.filter_map(|s| {
let (cat, item) = s.split_once('/')?;
Some((cat.to_string(), item.to_string()))
})
.collect();
let key = crate::model::cell::CellKey::new(coords);
vec![Box::new(effect::SetCell(key, value))]
match parse_cell_key_from_args(&args[1..]) {
Ok(key) => vec![Box::new(effect::SetCell(key, value))],
Err(why) => vec![effect::refuse(why)],
}
}
);
effect_cmd!(
AddFormulaCmd,
"add-formula",
MaxArgs::AtMost(2),
|args: &[String]| {
if args.is_empty() || args.len() > 2 {
// The arity alone does not tell anyone to quote — improvise-z05h.
@@ -269,6 +375,7 @@ effect_cmd!(
effect_cmd!(
ClearBufferCmd,
"clear-buffer",
MaxArgs::AtMost(1),
|args: &[String]| require_args(args, 1),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![super::core::write_buffer(&args[0], String::new())]
@@ -278,6 +385,7 @@ effect_cmd!(
effect_cmd!(
RemoveFormulaCmd,
"remove-formula",
MaxArgs::AtMost(2),
|args: &[String]| require_args(args, 2),
|args: &Vec<String>, ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
let (category, target) = (&args[0], &args[1]);
@@ -298,6 +406,7 @@ effect_cmd!(
effect_cmd!(
CreateViewCmd,
"create-view",
MaxArgs::AtMost(1),
|args: &[String]| require_args(args, 1),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::CreateView(args[0].clone()))]
@@ -307,6 +416,7 @@ effect_cmd!(
effect_cmd!(
DeleteViewCmd,
"delete-view",
MaxArgs::AtMost(1),
|args: &[String]| require_args(args, 1),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::DeleteView(args[0].clone()))]
@@ -316,87 +426,111 @@ effect_cmd!(
effect_cmd!(
SwitchViewCmd,
"switch-view",
MaxArgs::AtMost(1),
|args: &[String]| require_args(args, 1),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::SwitchView(args[0].clone()))]
unless_unknown(
effect::Needs::View(args[0].clone()),
effect::SwitchView(args[0].clone()),
)
}
);
effect_cmd!(
SetAxisCmd,
"set-axis",
|args: &[String]| require_args(args, 2),
MaxArgs::AtMost(2),
|args: &[String]| {
require_args(args, 2)?;
parse_axis(&args[1]).map(|_| ())
},
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
let axis = match args[1].to_lowercase().as_str() {
"row" => Axis::Row,
"column" | "col" => Axis::Column,
"page" => Axis::Page,
"none" => Axis::None,
_ => return vec![], // parse step already validated
};
vec![Box::new(effect::SetAxis {
view: ViewTarget::Active,
category: args[0].clone(),
axis,
})]
match parse_axis(&args[1]) {
Ok(axis) => unless_unknown(
effect::Needs::Category(args[0].clone()),
effect::SetAxis {
view: ViewTarget::Active,
category: args[0].clone(),
axis,
},
),
Err(why) => vec![effect::refuse(why)],
}
}
);
effect_cmd!(
SetPageCmd,
"set-page",
MaxArgs::AtMost(2),
|args: &[String]| require_args(args, 2),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::SetPageSelection {
view: ViewTarget::Active,
category: args[0].clone(),
item: args[1].clone(),
})]
unless_unknown(
effect::Needs::Item(args[0].clone(), args[1].clone()),
effect::SetPageSelection {
view: ViewTarget::Active,
category: args[0].clone(),
item: args[1].clone(),
},
)
}
);
effect_cmd!(
ToggleGroupCmd,
"toggle-group",
MaxArgs::AtMost(2),
|args: &[String]| require_args(args, 2),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::ToggleGroup {
view: ViewTarget::Active,
category: args[0].clone(),
group: args[1].clone(),
})]
unless_unknown(
effect::Needs::Group(args[0].clone(), args[1].clone()),
effect::ToggleGroup {
view: ViewTarget::Active,
category: args[0].clone(),
group: args[1].clone(),
},
)
}
);
effect_cmd!(
HideItemCmd,
"hide-item",
MaxArgs::AtMost(2),
|args: &[String]| require_args(args, 2),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::HideItem {
view: ViewTarget::Active,
category: args[0].clone(),
item: args[1].clone(),
})]
unless_unknown(
effect::Needs::Item(args[0].clone(), args[1].clone()),
effect::HideItem {
view: ViewTarget::Active,
category: args[0].clone(),
item: args[1].clone(),
},
)
}
);
effect_cmd!(
ShowItemCmd,
"show-item",
MaxArgs::AtMost(2),
|args: &[String]| require_args(args, 2),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::ShowItem {
view: ViewTarget::Active,
category: args[0].clone(),
item: args[1].clone(),
})]
unless_unknown(
effect::Needs::Item(args[0].clone(), args[1].clone()),
effect::ShowItem {
view: ViewTarget::Active,
category: args[0].clone(),
item: args[1].clone(),
},
)
}
);
effect_cmd!(
SaveAsCmd,
"save-as",
MaxArgs::AtMost(1),
|args: &[String]| require_args(args, 1),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::SaveAs(std::path::PathBuf::from(&args[0])))]
@@ -406,6 +540,7 @@ effect_cmd!(
effect_cmd!(
SetFormatCmd,
"set-format",
MaxArgs::Any,
|args: &[String]| require_args(args, 1),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::SetNumberFormat(
@@ -418,6 +553,7 @@ effect_cmd!(
effect_cmd!(
ImportCmd,
"import",
MaxArgs::AtMost(1),
|args: &[String]| require_args(args, 1),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::StartImportWizard(args[0].clone()))]
@@ -427,6 +563,7 @@ effect_cmd!(
effect_cmd!(
ExportCmd,
"export",
MaxArgs::AtMost(1),
|_args: &[String]| -> Result<(), String> { Ok(()) },
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
let path = args.first().map(|s| s.as_str()).unwrap_or("export.csv");
@@ -437,6 +574,7 @@ effect_cmd!(
effect_cmd!(
WriteCmd,
"w",
MaxArgs::AtMost(1),
|_args: &[String]| -> Result<(), String> { Ok(()) },
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
if args.is_empty() {
@@ -450,6 +588,7 @@ effect_cmd!(
effect_cmd!(
HelpCmd,
"help",
MaxArgs::AtMost(0),
|_args: &[String]| -> Result<(), String> { Ok(()) },
|_args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![
@@ -462,6 +601,7 @@ effect_cmd!(
effect_cmd!(
HelpPageNextCmd,
"help-page-next",
MaxArgs::AtMost(0),
|_args: &[String]| -> Result<(), String> { Ok(()) },
|_args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![effect::help_page_next()]
@@ -471,6 +611,7 @@ effect_cmd!(
effect_cmd!(
HelpPagePrevCmd,
"help-page-prev",
MaxArgs::AtMost(0),
|_args: &[String]| -> Result<(), String> { Ok(()) },
|_args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![effect::help_page_prev()]
@@ -480,6 +621,7 @@ effect_cmd!(
effect_cmd!(
LoadModelCmd,
"load",
MaxArgs::AtMost(1),
|args: &[String]| require_args(args, 1),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::LoadModel(std::path::PathBuf::from(
@@ -491,6 +633,7 @@ effect_cmd!(
effect_cmd!(
ExportCsvCmd,
"export-csv",
MaxArgs::AtMost(1),
|args: &[String]| require_args(args, 1),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::ExportCsv(std::path::PathBuf::from(
@@ -502,6 +645,7 @@ effect_cmd!(
effect_cmd!(
ImportJsonCmd,
"import-json",
MaxArgs::AtMost(3),
|args: &[String]| {
if args.is_empty() {
Err("import-json requires a path".to_string())
crates/improvise-command/src/command/cmd/fit.rs
+3 -1
View File
@@ -4,7 +4,7 @@
use improvise_gp::fit::one_line;
use improvise_gp::{FitRequest, fit_measure, fitted_measure_name};
use crate::command::cmd::core::{Cmd, CmdContext};
use crate::command::cmd::core::{Cmd, CmdContext, MaxArgs};
use crate::ui::effect::{self, Effect};
/// A seed as the status line spells it (`0x…`, either case) or in decimal. The hex
@@ -27,6 +27,8 @@ pub struct FitMeasure {
}
impl FitMeasure {
pub const MAX_ARGS: MaxArgs = MaxArgs::AtMost(3);
/// A nameless instance for `register_pure`, which reads a command's name
/// off a prototype rather than off a string literal (gotcha 15).
pub fn prototype() -> Self {
crates/improvise-command/src/command/cmd/registry.rs
+223 -40
View File
@@ -81,52 +81,130 @@ pub fn default_registry() -> CmdRegistry {
let mut r = CmdRegistry::new();
// ── Model mutations (effect_cmd! wrappers) ───────────────────────────
r.register_pure(&AddCategoryCmd(vec![]), AddCategoryCmd::parse);
r.register_pure(&AddItemCmd(vec![]), AddItemCmd::parse);
r.register_pure(&AddItemsCmd(vec![]), AddItemsCmd::parse);
r.register_pure(&AddItemInGroupCmd(vec![]), AddItemInGroupCmd::parse);
r.register_pure(&SetCellCmd(vec![]), SetCellCmd::parse);
r.register_pure(&ClearBufferCmd(vec![]), ClearBufferCmd::parse);
r.register_pure(
&AddCategoryCmd(vec![]),
AddCategoryCmd::MAX_ARGS,
AddCategoryCmd::parse,
);
r.register_pure(&AddItemCmd(vec![]), AddItemCmd::MAX_ARGS, AddItemCmd::parse);
r.register_pure(
&AddItemsCmd(vec![]),
AddItemsCmd::MAX_ARGS,
AddItemsCmd::parse,
);
r.register_pure(
&AddItemInGroupCmd(vec![]),
AddItemInGroupCmd::MAX_ARGS,
AddItemInGroupCmd::parse,
);
r.register_pure(&SetCellCmd(vec![]), SetCellCmd::MAX_ARGS, SetCellCmd::parse);
r.register_pure(
&ClearBufferCmd(vec![]),
ClearBufferCmd::MAX_ARGS,
ClearBufferCmd::parse,
);
r.register(
&ClearCellCommand {
key: CellKey::new(vec![]),
},
MaxArgs::Any,
|args| {
if args.is_empty() {
return Err("clear-cell requires at least one Cat/Item coordinate".into());
}
Ok(ClearCellCommand {
key: parse_cell_key_from_args(args),
key: parse_cell_key_from_args(args)?,
})
},
Some(|_, ctx| {
Some(|args, ctx| {
only_when_omitted(args)?;
Ok(ClearCellCommand {
key: cursor_value_key(ctx)?,
})
}),
);
r.register_pure(&AddFormulaCmd(vec![]), AddFormulaCmd::parse);
r.register_pure(&FitMeasure::prototype(), FitMeasure::parse);
r.register_pure(&RemoveFormulaCmd(vec![]), RemoveFormulaCmd::parse);
r.register_pure(&CreateViewCmd(vec![]), CreateViewCmd::parse);
r.register_pure(&DeleteViewCmd(vec![]), DeleteViewCmd::parse);
r.register_pure(&SwitchViewCmd(vec![]), SwitchViewCmd::parse);
r.register_pure(&SetAxisCmd(vec![]), SetAxisCmd::parse);
r.register_pure(&SetPageCmd(vec![]), SetPageCmd::parse);
r.register_pure(&ToggleGroupCmd(vec![]), ToggleGroupCmd::parse);
r.register_pure(&HideItemCmd(vec![]), HideItemCmd::parse);
r.register_pure(&ShowItemCmd(vec![]), ShowItemCmd::parse);
r.register_pure(&SaveAsCmd(vec![]), SaveAsCmd::parse);
r.register_pure(&LoadModelCmd(vec![]), LoadModelCmd::parse);
r.register_pure(&ExportCsvCmd(vec![]), ExportCsvCmd::parse);
r.register_pure(&ImportJsonCmd(vec![]), ImportJsonCmd::parse);
r.register_pure(&SetFormatCmd(vec![]), SetFormatCmd::parse);
r.register_pure(&ImportCmd(vec![]), ImportCmd::parse);
r.register_pure(&ExportCmd(vec![]), ExportCmd::parse);
r.register_pure(&WriteCmd(vec![]), WriteCmd::parse);
r.register_pure(&HelpCmd(vec![]), HelpCmd::parse);
r.register_pure(&HelpPageNextCmd(vec![]), HelpPageNextCmd::parse);
r.register_pure(&HelpPagePrevCmd(vec![]), HelpPagePrevCmd::parse);
r.register_pure(
&AddFormulaCmd(vec![]),
AddFormulaCmd::MAX_ARGS,
AddFormulaCmd::parse,
);
r.register_pure(
&FitMeasure::prototype(),
FitMeasure::MAX_ARGS,
FitMeasure::parse,
);
r.register_pure(
&RemoveFormulaCmd(vec![]),
RemoveFormulaCmd::MAX_ARGS,
RemoveFormulaCmd::parse,
);
r.register_pure(
&CreateViewCmd(vec![]),
CreateViewCmd::MAX_ARGS,
CreateViewCmd::parse,
);
r.register_pure(
&DeleteViewCmd(vec![]),
DeleteViewCmd::MAX_ARGS,
DeleteViewCmd::parse,
);
r.register_pure(
&SwitchViewCmd(vec![]),
SwitchViewCmd::MAX_ARGS,
SwitchViewCmd::parse,
);
r.register_pure(&SetAxisCmd(vec![]), SetAxisCmd::MAX_ARGS, SetAxisCmd::parse);
r.register_pure(&SetPageCmd(vec![]), SetPageCmd::MAX_ARGS, SetPageCmd::parse);
r.register_pure(
&ToggleGroupCmd(vec![]),
ToggleGroupCmd::MAX_ARGS,
ToggleGroupCmd::parse,
);
r.register_pure(
&HideItemCmd(vec![]),
HideItemCmd::MAX_ARGS,
HideItemCmd::parse,
);
r.register_pure(
&ShowItemCmd(vec![]),
ShowItemCmd::MAX_ARGS,
ShowItemCmd::parse,
);
r.register_pure(&SaveAsCmd(vec![]), SaveAsCmd::MAX_ARGS, SaveAsCmd::parse);
r.register_pure(
&LoadModelCmd(vec![]),
LoadModelCmd::MAX_ARGS,
LoadModelCmd::parse,
);
r.register_pure(
&ExportCsvCmd(vec![]),
ExportCsvCmd::MAX_ARGS,
ExportCsvCmd::parse,
);
r.register_pure(
&ImportJsonCmd(vec![]),
ImportJsonCmd::MAX_ARGS,
ImportJsonCmd::parse,
);
r.register_pure(
&SetFormatCmd(vec![]),
SetFormatCmd::MAX_ARGS,
SetFormatCmd::parse,
);
r.register_pure(&ImportCmd(vec![]), ImportCmd::MAX_ARGS, ImportCmd::parse);
r.register_pure(&ExportCmd(vec![]), ExportCmd::MAX_ARGS, ExportCmd::parse);
r.register_pure(&WriteCmd(vec![]), WriteCmd::MAX_ARGS, WriteCmd::parse);
r.register_pure(&HelpCmd(vec![]), HelpCmd::MAX_ARGS, HelpCmd::parse);
r.register_pure(
&HelpPageNextCmd(vec![]),
HelpPageNextCmd::MAX_ARGS,
HelpPageNextCmd::parse,
);
r.register_pure(
&HelpPagePrevCmd(vec![]),
HelpPagePrevCmd::MAX_ARGS,
HelpPagePrevCmd::parse,
);
// ── Navigation (unified Move) ──────────────────────────────────────
r.register_at_cursor(
@@ -135,6 +213,7 @@ pub fn default_registry() -> CmdRegistry {
cursor: CursorState::default(),
cmd_name: "move-selection",
},
MaxArgs::AtMost(2),
|args| {
require_args(args, 2)?;
let dr = args[0].parse::<i32>().map_err(|e| e.to_string())?;
@@ -160,6 +239,7 @@ pub fn default_registry() -> CmdRegistry {
cursor: CursorState::default(),
cmd_name: $name,
},
MaxArgs::AtMost(0),
|_| {
Ok(Move {
kind: if $to_end {
@@ -185,6 +265,7 @@ pub fn default_registry() -> CmdRegistry {
cursor: CursorState::default(),
cmd_name: "scroll-rows",
},
MaxArgs::AtMost(1),
|args| {
require_args(args, 1)?;
let n = args[0].parse::<i32>().map_err(|e| e.to_string())?;
@@ -202,6 +283,7 @@ pub fn default_registry() -> CmdRegistry {
cursor: CursorState::default(),
cmd_name: "page-scroll",
},
MaxArgs::AtMost(1),
|args| {
require_args(args, 1)?;
let dir = args[0].parse::<i32>().map_err(|e| e.to_string())?;
@@ -217,6 +299,7 @@ pub fn default_registry() -> CmdRegistry {
&EnterAdvance {
cursor: CursorState::default(),
},
MaxArgs::AtMost(0),
|_| {
Ok(EnterAdvance {
cursor: CursorState {
@@ -239,15 +322,17 @@ pub fn default_registry() -> CmdRegistry {
&YankCell {
key: CellKey::new(vec![]),
},
MaxArgs::Any,
|args| {
if args.is_empty() {
return Err("yank requires at least one Cat/Item coordinate".into());
}
Ok(YankCell {
key: parse_cell_key_from_args(args),
key: parse_cell_key_from_args(args)?,
})
},
Some(|_, ctx| {
Some(|args, ctx| {
only_when_omitted(args)?;
Ok(YankCell {
key: cursor_key_to_read(ctx)?,
})
@@ -257,15 +342,17 @@ pub fn default_registry() -> CmdRegistry {
&PasteCell {
at: PasteAt::Spelled(CellKey::new(vec![])),
},
MaxArgs::Any,
|args| {
if args.is_empty() {
return Err("paste requires at least one Cat/Item coordinate".into());
}
Ok(PasteCell {
at: PasteAt::Spelled(parse_cell_key_from_args(args)),
at: PasteAt::Spelled(parse_cell_key_from_args(args)?),
})
},
Some(|_, ctx| {
Some(|args, ctx| {
only_when_omitted(args)?;
Ok(PasteCell {
at: PasteAt::Resolved(cursor_value_key(ctx)?),
})
@@ -288,6 +375,7 @@ pub fn default_registry() -> CmdRegistry {
&EditOrDrill {
edit_mode: AppMode::Normal,
},
MaxArgs::AtMost(1),
|args| {
require_args(args, 1)?;
let edit_mode = parse_mode_name(&args[0])?;
@@ -298,6 +386,7 @@ pub fn default_registry() -> CmdRegistry {
&EnterEditAtCursorCmd {
target_mode: AppMode::Normal,
},
MaxArgs::AtMost(1),
|args| {
require_args(args, 1)?;
let target_mode = parse_mode_name(&args[0])?;
@@ -309,15 +398,17 @@ pub fn default_registry() -> CmdRegistry {
&DrillIntoCell {
key: crate::model::cell::CellKey::new(vec![]),
},
MaxArgs::Any,
|args| {
if args.is_empty() {
return Err("drill-into-cell requires Cat/Item coordinates".into());
}
Ok(DrillIntoCell {
key: parse_cell_key_from_args(args),
key: parse_cell_key_from_args(args)?,
})
},
Some(|_, ctx| {
Some(|args, ctx| {
only_when_omitted(args)?;
Ok(DrillIntoCell {
key: cursor_key(ctx)?,
})
@@ -325,13 +416,13 @@ pub fn default_registry() -> CmdRegistry {
);
r.register_nullary(|| Box::new(ViewNavigate { forward: false }));
r.register_nullary(|| Box::new(ViewNavigate { forward: true }));
r.register_pure(&EnterMode(AppMode::Normal), |args| {
r.register_pure(&EnterMode(AppMode::Normal), MaxArgs::AtMost(1), |args| {
require_args(args, 1)?;
Ok(Box::new(EnterMode(parse_mode_name(&args[0])?)))
});
// ── Search ───────────────────────────────────────────────────────────
r.register_pure(&SearchNavigate(true), |args| {
r.register_pure(&SearchNavigate(true), MaxArgs::AtMost(1), |args| {
let forward = args.first().map(|s| s != "backward").unwrap_or(true);
Ok(Box::new(SearchNavigate(forward)))
});
@@ -344,6 +435,7 @@ pub fn default_registry() -> CmdRegistry {
panel: Panel::Formula,
target: None,
},
MaxArgs::AtMost(3),
|args| {
// Parse: toggle-panel-and-focus <panel> [open] [focused]
require_args(args, 1)?;
@@ -360,6 +452,7 @@ pub fn default_registry() -> CmdRegistry {
&TogglePanelVisibility {
panel: Panel::Formula,
},
MaxArgs::AtMost(1),
|args| {
require_args(args, 1)?;
let panel = parse_panel(&args[0])?;
@@ -373,6 +466,7 @@ pub fn default_registry() -> CmdRegistry {
panel: Panel::Formula,
delta: 0,
},
MaxArgs::AtMost(2),
|args| {
require_args(args, 2)?;
let panel = parse_panel(&args[0])?;
@@ -396,7 +490,7 @@ pub fn default_registry() -> CmdRegistry {
r.register_nullary(|| Box::new(DeleteViewAtCursor));
// ── Tile select ──────────────────────────────────────────────────────
r.register_pure(&MoveTileCursor(0), |args| {
r.register_pure(&MoveTileCursor(0), MaxArgs::AtMost(1), |args| {
require_args(args, 1)?;
let delta = args[0].parse::<i32>().map_err(|e| e.to_string())?;
Ok(Box::new(MoveTileCursor(delta)))
@@ -406,6 +500,7 @@ pub fn default_registry() -> CmdRegistry {
&TileAxisOp {
axis: Some(crate::view::Axis::Row),
},
MaxArgs::AtMost(1),
|args| {
require_args(args, 1)?;
let axis = parse_axis(&args[0])?;
@@ -423,6 +518,7 @@ pub fn default_registry() -> CmdRegistry {
&AppendChar {
buffer: String::new(),
},
MaxArgs::AtMost(1),
|args| {
require_args(args, 1)?;
Ok(Box::new(AppendChar {
@@ -434,6 +530,7 @@ pub fn default_registry() -> CmdRegistry {
&PopChar {
buffer: String::new(),
},
MaxArgs::AtMost(1),
|args| {
require_args(args, 1)?;
Ok(Box::new(PopChar {
@@ -454,13 +551,14 @@ pub fn default_registry() -> CmdRegistry {
edit_mode: AppMode::editing(),
opened: None,
},
MaxArgs::Any,
|args| {
if args.len() < 3 {
return Err("commit-cell-edit requires a mode, value, and coords".into());
}
let edit_mode = parse_mode_name(&args[0])?;
Ok(CommitAndAdvance {
key: parse_cell_key_from_args(&args[2..]),
key: parse_cell_key_from_args(&args[2..])?,
value: args[1].clone(),
advance: AdvanceDir::Down,
cursor: CursorState::default(),
@@ -479,6 +577,7 @@ pub fn default_registry() -> CmdRegistry {
edit_mode: AppMode::editing(),
opened: None,
},
MaxArgs::AtMost(1),
|_| Err("commit-and-advance-right requires context".into()),
Some(|args, ctx| commit_at_cursor(args, ctx, AdvanceDir::Right)),
);
@@ -486,6 +585,7 @@ pub fn default_registry() -> CmdRegistry {
&CommitBuffer {
kind: CommitKind::Category,
},
MaxArgs::AtMost(1),
|args| {
require_args(args, 1)?;
let kind = parse_commit_kind(&args[0], &AppMode::Normal)?;
@@ -534,6 +634,7 @@ pub(super) mod tests {
vec!["a"],
vec!["Region"],
vec!["Region", "East"],
vec!["Region", "row"],
vec!["Region", "East", "1"],
vec!["Profit = 1"],
vec![",.2f"],
@@ -544,6 +645,7 @@ pub(super) mod tests {
vec!["editing", "5", "Region/East"],
vec!["formula", "false", "false"],
vec!["Region/East"],
vec!["1", "Region/East"],
]
.into_iter()
.map(|v| v.into_iter().map(String::from).collect())
@@ -592,6 +694,87 @@ pub(super) mod tests {
);
}
/// Evidence: `context/test-evidence/cli.md`.
#[test]
fn no_command_silently_drops_an_argument_it_was_given() {
let r = default_registry();
let wb = two_cat_model();
let layout = make_layout(&wb);
let reg = make_registry();
let ctx = make_ctx(&wb, &layout, &reg);
let variadic = [
"add-items",
"set-cell",
"set-format",
"clear-cell",
"yank",
"paste",
"drill-into-cell",
"commit-cell-edit",
];
// Their own refusal states the shape, and it runs before the rule.
let own_shape = ["add-formula", "fit-measure", "commit-and-advance-right"];
let mut checked = 0;
for name in r.names().collect::<Vec<_>>() {
if variadic.contains(&name) {
continue;
}
for args in candidate_args() {
// Four more than any list accepted, so an optional argument
// cannot absorb them; the last one is surplus whatever the command.
let mut more = args.clone();
more.extend(["EXTRA1", "EXTRA2", "EXTRA3", "EXTRA4"].map(String::from));
let arms = [
r.parse(name, &args).map(|_| r.parse(name, &more)),
r.interactive(name, &args, &ctx)
.map(|_| r.interactive(name, &more, &ctx)),
];
for extended in arms.into_iter().flatten() {
checked += 1;
match extended {
Ok(_) => panic!("`{name} {more:?}` dropped arguments without a word"),
Err(e) if own_shape.contains(&name) || e.contains("\"EXTRA4\"") => {}
Err(e) => {
panic!("`{name}` refused an extra argument without naming it: {e}")
}
}
}
}
}
assert!(checked > 100, "only {checked} accepted lists were extended");
}
/// Codex on PR #316: a variadic coordinate list dropped a token that was no
/// coordinate. Evidence: `context/test-evidence/cli.md`.
#[test]
fn a_malformed_coordinate_is_refused_not_dropped() {
let lines = [
"set-cell 1 Type/Food unexpected",
"clear-cell Type/Food unexpected",
"yank Type/Food unexpected",
"paste Type/Food unexpected",
"drill-into-cell Type/Food unexpected",
"commit-cell-edit editing 5 Type/Food unexpected",
];
for line in lines {
let err = crate::command::parse_line(line)
.err()
.unwrap_or_else(|| panic!("`{line}` dropped \"unexpected\" without a word"));
assert!(
err.contains("\"unexpected\""),
"`{line}` did not name it: {err}"
);
}
for half_empty in ["set-cell 1 Type/", "clear-cell /Food"] {
assert!(
crate::command::parse_line(half_empty).is_err(),
"`{half_empty}`"
);
}
let control = "set-cell 1 Type/Food Month/Jan . clear-cell Type/Food";
assert!(crate::command::parse_line(control).is_ok(), "the control");
}
/// A variant mix-up shows up as a **collision**, not a mismatch: the wrong entry
/// takes its sibling's name and the sibling vanishes. Uniqueness catches it.
#[test]
crates/improvise-command/src/command/cmd/text_buffer.rs
+147 -3
View File
@@ -216,6 +216,110 @@ mod tests {
app.apply_effects(effects);
}
fn cell(wb: &crate::workbook::Workbook, ty: &str, month: &str) -> Option<f64> {
use crate::model::cell::{CellKey, CellValue};
let key = CellKey::new(vec![
("Type".into(), ty.into()),
("Month".into(), month.into()),
]);
match wb.model.get_cell(&key) {
Some(CellValue::Number(n)) => Some(*n),
_ => None,
}
}
/// An app whose cursor (0,0) is on Food/Jan = 1, with Clothing/Feb = 2 beside it.
fn app_with_two_cells() -> crate::ui::app::App {
use crate::model::cell::{CellKey, CellValue};
let mut app = wizard_test_app();
for (ty, month, n) in [("Food", "Jan", 1.0), ("Clothing", "Feb", 2.0)] {
let key = CellKey::new(vec![
("Type".into(), ty.into()),
("Month".into(), month.into()),
]);
app.model_state
.workbook
.model
.set_cell(key, CellValue::Number(n));
}
app
}
/// PR #316 review: a malformed coordinate on `:` fell into the cursor fallback
/// and wrote the CURSOR's cell. Evidence: `context/test-evidence/cli.md`.
#[test]
fn colon_refuses_a_malformed_coordinate_and_leaves_the_cursor_cell_alone() {
let lines = [
"clear-cell Type/Clothing Month/Feb junk",
"yank Type/Clothing Month/Feb junk",
"paste Type/Clothing Month/Feb junk",
"drill-into-cell Type/Clothing Month/Feb junk",
];
for line in lines {
let mut app = app_with_two_cells();
app.view_state.yanked = Some(crate::model::cell::CellValue::Number(9.0));
let before = app.view_state.yanked.clone();
run_colon_command(&mut app, line);
assert!(app.last_batch_refused(), "`:{line}` was not refused");
assert!(
app.view_state.status_msg.contains("\"junk\""),
"`:{line}` did not name the token: {:?}",
app.view_state.status_msg
);
let wb = &app.model_state.workbook;
assert_eq!(
cell(wb, "Food", "Jan"),
Some(1.0),
"`:{line}` wrote the cursor cell"
);
assert_eq!(
cell(wb, "Clothing", "Feb"),
Some(2.0),
"`:{line}` wrote the named cell"
);
assert_eq!(app.view_state.yanked, before, "`:{line}` yanked");
assert!(app.drill_state.is_none(), "`:{line}` opened a drill");
}
}
/// The fallback still answers for coordinates that were OMITTED.
#[test]
fn colon_clear_cell_with_no_coordinates_still_uses_the_cursor() {
let mut app = app_with_two_cells();
run_colon_command(&mut app, "clear-cell");
let wb = &app.model_state.workbook;
assert_eq!(cell(wb, "Food", "Jan"), None);
assert_eq!(cell(wb, "Clothing", "Feb"), Some(2.0));
}
/// A later `:` segment sees what an earlier one made, as `improvise cmd` does:
/// the line is built before either half runs. Evidence: `context/test-evidence/cli.md`.
#[test]
fn colon_lets_a_later_segment_use_what_an_earlier_one_created() {
let mut app = wizard_test_app();
run_colon_command(&mut app, "create-view Second . switch-view Second");
assert!(!app.last_batch_refused(), "{:?}", app.view_state.status_msg);
assert_eq!(app.view_state.active_view, "Second");
let mut app = wizard_test_app();
run_colon_command(&mut app, "add-item Type Tools . set-page Type Tools");
assert!(!app.last_batch_refused(), "{:?}", app.view_state.status_msg);
}
/// The control: a name NOTHING creates is still refused, on `:` too.
#[test]
fn colon_still_refuses_a_name_nothing_creates() {
for line in [
"switch-view Nowhere",
"set-page Type Nowhere",
"hide-item Regoin Food",
] {
let mut app = wizard_test_app();
run_colon_command(&mut app, line);
assert!(app.last_batch_refused(), "`:{line}` was not refused");
}
}
/// improvise-toc: `StartImportWizard` swaps the mode only on success, so a
/// per-type answer snapped `:import` straight back out of the wizard.
#[test]
@@ -271,6 +375,43 @@ mod tests {
);
}
/// improvise-31oe.2 on the `:` line: the refusal is on the status line,
/// naming what is missing. Evidence: `context/test-evidence/cli.md`.
#[test]
fn a_colon_line_naming_something_missing_refuses_on_the_status_line() {
for (line, missing) in [
("set-axis Regoin row", "Regoin"),
("set-axis Type sideways", "sideways"),
("set-page Type Nowhere", "Nowhere"),
("create-view By area", "\"area\""),
] {
let mut app = wizard_test_app();
run_colon_command(&mut app, line);
let said = &app.view_state.status_msg;
assert!(
app.last_batch_refused(),
"`:{line}` was not refused: {said:?}"
);
assert!(
said.contains(missing),
"`:{line}` did not name {missing}: {said:?}"
);
assert!(matches!(app.view_state.mode, AppMode::Normal), "`:{line}`");
}
}
/// improvise-31oe.3 on the `:` line.
#[test]
fn a_colon_line_reads_a_pipe_quoted_name() {
let mut app = wizard_test_app();
run_colon_command(&mut app, "create-view |By area|");
let views: Vec<&String> = app.model_state.workbook.views.keys().collect();
assert!(
app.model_state.workbook.views.contains_key("By area"),
"{views:?}"
);
}
/// A command that does not touch the mode still returns to Normal.
#[test]
fn colon_plain_command_returns_to_normal() {
@@ -372,9 +513,12 @@ impl Cmd for ExecuteCommand {
// Built against the live cursor, exactly as a script's is — improvise-6uva.
let built: Result<Vec<Box<dyn Cmd>>, String> = crate::command::parse::segments(&raw)
.into_iter()
.map(|seg| crate::command::parse::parse_segment_interactive(seg, ctx))
.collect::<Result<Vec<_>, _>>()
.and_then(|segments| {
segments
.into_iter()
.map(|seg| crate::command::parse::parse_segment_interactive(seg, ctx))
.collect::<Result<Vec<_>, _>>()
})
.map(|built| built.into_iter().flatten().collect());
match built {
Ok(cmds) => {
crates/improvise-command/src/command/parse.rs
+208 -85
View File
@@ -1,5 +1,10 @@
//! Quasi-lisp prefix command parser: `word arg1 arg2 …`, `.` between commands,
//! `Category/Item` coordinate pairs, `"quoted strings"`.
//! `Category/Item` coordinate pairs, `"quoted strings"` and `|pipe-quoted names|`.
use std::iter::Peekable;
use std::str::CharIndices;
use improvise_formula::quote::unescape_pipe_body;
use super::cmd::{Cmd, CmdContext, CmdRegistry, default_registry};
@@ -11,39 +16,19 @@ pub fn parse_line(line: &str) -> Result<Vec<Box<dyn Cmd>>, String> {
/// Parse a line into commands using a given registry.
pub fn parse_line_with(registry: &CmdRegistry, line: &str) -> Result<Vec<Box<dyn Cmd>>, String> {
let line = line.trim();
if line.is_empty() || line.starts_with('#') || line.starts_with("//") {
return Ok(vec![]);
}
let mut commands = Vec::new();
for segment in split_on_dot(line) {
let segment = segment.trim();
if segment.is_empty() {
continue;
}
let tokens = tokenize(segment);
if tokens.is_empty() {
continue;
}
let word = &tokens[0];
let args = &tokens[1..];
commands.push(registry.parse(word, args)?);
}
Ok(commands)
groups(line.trim())?
.iter()
.map(|words| registry.parse(&words[0].text, &texts(&words[1..])))
.collect()
}
/// The command segments of a line, with comments and blank segments dropped.
pub fn segments(line: &str) -> Vec<&str> {
pub fn segments(line: &str) -> Result<Vec<&str>, String> {
let line = line.trim();
if line.is_empty() || line.starts_with('#') || line.starts_with("//") {
return Vec::new();
}
split_on_dot(line)
.into_iter()
.map(str::trim)
.filter(|s| !s.is_empty())
.collect()
Ok(groups(line)?
.iter()
.map(|words| &line[words[0].start..words[words.len() - 1].end])
.collect())
}
/// Build one segment's command against a LIVE context — `context/code/cli.md`.
@@ -51,7 +36,7 @@ pub fn parse_segment_interactive(
segment: &str,
ctx: &CmdContext,
) -> Result<Option<Box<dyn Cmd>>, String> {
let tokens = tokenize(segment.trim());
let tokens = texts(&lex(segment.trim())?);
let Some(word) = tokens.first() else {
return Ok(None);
};
@@ -67,65 +52,99 @@ pub fn parse_segment_interactive(
})
}
/// Split a line on ` . ` separators (dot must be a standalone word,
/// surrounded by whitespace or at line boundaries). Respects quoted strings.
fn split_on_dot(line: &str) -> Vec<&str> {
let mut segments = Vec::new();
let mut start = 0;
let mut in_quote = false;
let bytes = line.as_bytes();
for (i, c) in line.char_indices() {
match c {
'"' => in_quote = !in_quote,
'.' if !in_quote => {
let before_ws = i == 0 || bytes[i - 1].is_ascii_whitespace();
let after_ws = i + 1 >= bytes.len() || bytes[i + 1].is_ascii_whitespace();
if before_ws && after_ws {
segments.push(&line[start..i]);
start = i + 1;
}
}
_ => {}
}
}
segments.push(&line[start..]);
segments
/// One word of a line and the byte span it was read from. A `separator` is a
/// bare `.`; a quoted one is an argument like any other.
struct Word {
text: String,
start: usize,
end: usize,
separator: bool,
}
/// Tokenize a command segment into words, handling quoted strings.
fn tokenize(input: &str) -> Vec<String> {
let mut tokens = Vec::new();
let mut chars = input.chars().peekable();
fn texts(words: &[Word]) -> Vec<String> {
words.iter().map(|w| w.text.clone()).collect()
}
while let Some(&c) = chars.peek() {
/// The words of a trimmed line, split at separators, blank commands and
/// comment lines dropped.
fn groups(line: &str) -> Result<Vec<Vec<Word>>, String> {
if line.starts_with('#') || line.starts_with("//") {
return Ok(Vec::new());
}
let mut groups: Vec<Vec<Word>> = vec![Vec::new()];
for word in lex(line)? {
match word.separator {
true => groups.push(Vec::new()),
false => groups.last_mut().expect("never empty").push(word),
}
}
groups.retain(|g| !g.is_empty());
Ok(groups)
}
/// Split `line` into words. The quoting rules: `context/code/cli.md` §14.
fn lex(line: &str) -> Result<Vec<Word>, String> {
let mut words = Vec::new();
let mut chars = line.char_indices().peekable();
while let Some(&(start, c)) = chars.peek() {
if c.is_whitespace() {
chars.next();
continue;
}
if c == '"' {
chars.next(); // consume opening quote
let mut s = String::new();
for ch in chars.by_ref() {
if ch == '"' {
break;
}
s.push(ch);
let (text, bare) = match c {
'"' => (double_quoted(&mut chars), false),
_ => bare_word(&mut chars, line)?,
};
let end = chars.peek().map_or(line.len(), |&(i, _)| i);
words.push(Word {
separator: bare && text == ".",
text,
start,
end,
});
}
Ok(words)
}
/// `"…"` read verbatim: no escapes, and a `|` inside stays a `|`.
fn double_quoted(chars: &mut Peekable<CharIndices>) -> String {
chars.next();
chars
.by_ref()
.map(|(_, c)| c)
.take_while(|&c| c != '"')
.collect()
}
/// Text up to whitespace, each `|…|` piece unquoted in place; `bare` is false
/// once a piece was quoted, so a quoted `|.|` is an argument and not a separator.
fn bare_word(chars: &mut Peekable<CharIndices>, line: &str) -> Result<(String, bool), String> {
let mut text = String::new();
let mut bare = true;
while let Some((at, c)) = chars.next_if(|&(_, c)| !c.is_whitespace()) {
match c {
'|' => {
text.push_str(&pipe_body(chars, line, at)?);
bare = false;
}
tokens.push(s);
} else {
let mut s = String::new();
while let Some(&ch) = chars.peek() {
if ch.is_whitespace() {
break;
}
s.push(ch);
chars.next();
}
tokens.push(s);
_ => text.push(c),
}
}
tokens
Ok((text, bare))
}
/// The body of the `|…|` opened at `open`, escapes applied by the one shared
/// reader (gotcha 34).
fn pipe_body(chars: &mut Peekable<CharIndices>, line: &str, open: usize) -> Result<String, String> {
let mut body = String::new();
while let Some((_, c)) = chars.next() {
match c {
'|' => return Ok(unescape_pipe_body(&body)),
'\\' => body.extend(std::iter::once(c).chain(chars.next().map(|(_, e)| e))),
_ => body.push(c),
}
}
Err(format!("unterminated |…| in `{}`", &line[open..]))
}
#[cfg(test)]
@@ -302,11 +321,14 @@ mod tests {
#[test]
fn segments_drops_comments_and_blanks() {
assert_eq!(segments("# a comment"), Vec::<&str>::new());
assert_eq!(segments("// also a comment"), Vec::<&str>::new());
assert_eq!(segments(" "), Vec::<&str>::new());
assert_eq!(segments("a . . b"), vec!["a", "b"]);
assert_eq!(segments("move-selection 2 0"), vec!["move-selection 2 0"]);
assert_eq!(segments("# a comment"), Ok(Vec::<&str>::new()));
assert_eq!(segments("// also a comment"), Ok(Vec::<&str>::new()));
assert_eq!(segments(" "), Ok(Vec::<&str>::new()));
assert_eq!(segments("a . . b"), Ok(vec!["a", "b"]));
assert_eq!(
segments("move-selection 2 0"),
Ok(vec!["move-selection 2 0"])
);
}
/// Evidence: `context/test-evidence/cli.md`.
@@ -347,4 +369,105 @@ mod tests {
.expect_err("an unknown word is refused");
assert!(err.contains("no-such-command"), "got: {err}");
}
// ── pipe quoting and arity (improvise-31oe.3) ─────────────────────────
/// The one command `line` builds, as its `Debug` shows it.
fn built(line: &str) -> String {
let cmds = parse_line(line).unwrap_or_else(|e| panic!("`{line}` must parse: {e}"));
assert_eq!(cmds.len(), 1, "`{line}` is one command");
format!("{:?}", cmds[0])
}
/// Evidence: `context/test-evidence/cli.md`.
#[test]
fn a_pipe_quoted_argument_is_one_argument_without_its_pipes() {
assert_eq!(
built("create-view |By area|"),
r#"CreateViewCmd(["By area"])"#
);
assert_eq!(
built("set-page |Sales Region| |East Coast|"),
r#"SetPageCmd(["Sales Region", "East Coast"])"#
);
}
/// Evidence: `context/test-evidence/cli.md`.
#[test]
fn a_pipe_quoted_argument_reads_the_escapes_improv_does() {
assert_eq!(built(r"create-view |a\|b|"), r#"CreateViewCmd(["a|b"])"#);
assert_eq!(
built(r"create-view |back\\slash|"),
r#"CreateViewCmd(["back\\slash"])"#
);
assert_eq!(
built(r"create-view |two\nlines|"),
r#"CreateViewCmd(["two\nlines"])"#
);
}
/// A coordinate quotes each half, as a `.improv` data line does.
#[test]
fn a_coordinate_may_pipe_quote_either_half() {
assert_eq!(
built("set-cell 5 |Sales Region|/|East Coast| Measure/Revenue"),
r#"SetCellCmd(["5", "Sales Region/East Coast", "Measure/Revenue"])"#
);
}
#[test]
fn a_dot_inside_pipes_does_not_split_the_line() {
assert_eq!(built("create-view |a . b|"), r#"CreateViewCmd(["a . b"])"#);
assert_eq!(
segments("create-view |a . b|"),
Ok(vec!["create-view |a . b|"])
);
}
/// Double quotes stay verbatim: formula text carries its own pipes.
#[test]
fn a_double_quoted_argument_keeps_the_pipes_inside_it() {
assert_eq!(
built(r#"add-formula Measure "Net = |Gross Rev| - Cost""#),
r#"AddFormulaCmd(["Measure", "Net = |Gross Rev| - Cost"])"#
);
}
/// `context/code/cli.md` §14: only a BARE `.` separates commands. A quoted one
/// is an argument, so `|.|` and `"."` name a thing called `.`.
#[test]
fn a_quoted_dot_is_an_argument_not_a_separator() {
for line in ["create-view |.|", "create-view \".\""] {
let groups = groups(line).expect("lexes");
assert_eq!(groups.len(), 1, "`{line}` split into commands");
assert_eq!(texts(&groups[0]), ["create-view", "."], "`{line}`");
}
assert_eq!(groups("create-view A . create-view B").unwrap().len(), 2);
}
#[test]
fn an_unterminated_pipe_is_refused() {
let err = parse_line("create-view |By area").expect_err("an open pipe is refused");
assert!(
err.contains("|By area"),
"the refusal names the text: {err}"
);
}
/// Evidence: `context/test-evidence/cli.md`.
#[test]
fn a_command_given_more_arguments_than_it_takes_is_refused() {
let err = parse_line("create-view By area").expect_err("two words for one argument");
assert!(err.starts_with("create-view:"), "names the command: {err}");
assert!(err.contains("\"area\""), "names the extra argument: {err}");
let err = parse_line("set-axis Type row sideways").expect_err("a third argument");
assert!(
err.contains("\"sideways\""),
"names the extra argument: {err}"
);
assert!(
parse_line("create-view |By area|").is_ok(),
"the control parses"
);
}
}
crates/improvise-command/src/ui/app.rs
+3 -1
View File
@@ -3462,7 +3462,9 @@ mod tests {
/// when any segment refused, which is what its exit code reads.
fn run_line(app: &mut App, line: &str) -> bool {
let mut refused = false;
for segment in crate::command::parse::segments(line) {
let segments = crate::command::parse::segments(line)
.unwrap_or_else(|e| panic!("`{line}` must lex: {e}"));
for segment in segments {
let effects = {
let ctx = app.cmd_context(KeyCode::Null, KeyModifiers::NONE);
crate::command::parse::parse_segment_interactive(segment, &ctx)
crates/improvise-command/src/ui/effect.rs
+57
View File
@@ -1368,6 +1368,63 @@ impl Effect for SetPanelCursor {
// ── Chain control ────────────────────────────────────────────────────────────
/// What a command's effects name that the document must have.
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum Needs {
Category(String),
Item(String, String),
Group(String, String),
View(String),
}
/// Why the document cannot satisfy `needs` in the active view, if it cannot.
fn unknown_name(app: &App, needs: &Needs) -> Option<String> {
let workbook = &app.model_state.workbook;
let (category, rest) = match needs {
Needs::View(name) => {
return (!workbook.views.contains_key(name))
.then(|| format!("No view named \"{name}\""));
}
Needs::Category(c) => (c, None),
Needs::Item(c, i) => (c, Some((i, false))),
Needs::Group(c, g) => (c, Some((g, true))),
};
let view = workbook.views.get(app.view_name())?;
if view.try_axis_of(category).is_none() {
return Some(format!("Unknown category '{category}'"));
}
let (name, is_group) = rest?;
if !is_group {
// `effective_item_names`, so a formula target is a known `_Measure` item.
let known = workbook
.model
.effective_item_names(category)
.iter()
.any(|i| i == name);
return (!known).then(|| format!("Unknown item '{name}' in '{category}'"));
}
// An item's own group registers no `Group`; a parent group has no items.
let known = workbook.model.category(category).is_some_and(|c| {
!c.items_in_group(name).is_empty() || c.groups.iter().any(|g| g.name == *name)
});
(!known).then(|| format!("Unknown group '{name}' in '{category}'"))
}
/// Refuse and stop unless the document has what the next effects name, asked at
/// apply time so a `:` line sees its earlier segments (`cli.md` §15).
#[derive(Debug)]
pub struct RequireKnown(pub Needs);
impl Effect for RequireKnown {
fn apply(&self, app: &mut App) {
if let Some(why) = unknown_name(app, &self.0) {
refuse_and_stop(app, why);
}
}
fn kind(&self) -> EffectKind {
EffectKind::ViewOnly
}
}
/// Skip the remaining effects in this `apply_effects` batch, for when a
/// sequence's premise no longer holds. The flag resets per dispatch.
#[derive(Debug)]
crates/improvise-command/src/ui/reduce/vocabulary_is_complete.rs
+4
View File
@@ -63,6 +63,10 @@ const NOT_ON_THE_WIRE: &[(&str, Verdict)] = &[
"refreshes the host's layout cache mid-batch; a client holds none (gotcha 23)",
),
),
(
"RequireKnown",
Verdict::Off("a command-side name check that writes no state (gotcha 21, cli.md §15)"),
),
(
"AbortChain",
Verdict::Off("control flow inside one apply_effects batch; it writes no state (gotcha 21)"),
src/main.rs
+10 -6
View File
@@ -432,12 +432,16 @@ fn run_headless_commands(cmds: &[String], file: &Option<PathBuf>) -> Result<()>
for line in cmds {
// The whole line is syntax-checked before any of it runs, exactly as
// before; only CONSTRUCTION moved — `context/code/cli.md`.
if let Err(e) = command::parse_line(line) {
eprintln!("Parse error: {e}");
exit_code = 1;
continue;
}
for segment in command::parse::segments(line) {
let segments = command::parse_line(line).and_then(|_| command::parse::segments(line));
let segments = match segments {
Ok(segments) => segments,
Err(e) => {
eprintln!("Parse error: {e}");
exit_code = 1;
continue;
}
};
for segment in segments {
let mut failed = None;
let effects = {
// Built against the cursor it is about to run against, so a
tests/headless_cli.rs
+52
View File
@@ -121,6 +121,58 @@ fn a_parse_error_still_exits_non_zero() {
);
}
/// improvise-31oe.2 at the process boundary, with a control that exits 0.
/// Evidence: `context/test-evidence/cli.md`.
#[test]
fn an_unknown_category_axis_or_item_exits_non_zero_and_names_it() {
let scratch = Scratch::new("unknown_names");
let doc = seeded(&scratch, "m.improv");
let out = run_cmd(&["add-item Region East"], &doc);
assert!(out.status.success(), "the seed item: {}", stderr_of(&out));
for (line, missing) in [
("set-axis Regoin row", "Regoin"),
("set-page Region Nowhere", "Nowhere"),
("set-axis Region sideways", "sideways"),
] {
let out = run_cmd(&[line], &doc);
let said = stderr_of(&out);
assert!(!out.status.success(), "`{line}` exited 0; stderr={said:?}");
assert!(
said.contains(missing),
"`{line}` did not name {missing}: {said:?}"
);
}
let out = run_cmd(&["set-page Region East"], &doc);
assert!(out.status.success(), "the control: {}", stderr_of(&out));
}
/// improvise-31oe.3: one pipe-quoted argument, and a surplus one refused.
/// Evidence: `context/test-evidence/cli.md`.
#[test]
fn a_pipe_quoted_name_reaches_the_document_whole() {
let scratch = Scratch::new("pipes");
let doc = seeded(&scratch, "m.improv");
let out = run_cmd(&["create-view |By area|"], &doc);
assert!(out.status.success(), "{}", stderr_of(&out));
let text = std::fs::read_to_string(&doc).expect("the document");
assert!(text.contains("## View: By area\n"), "{text}");
assert!(
!text.contains("|By"),
"a pipe reached the view name: {text}"
);
let out = run_cmd(&["create-view By area"], &doc);
let said = stderr_of(&out);
assert!(
!out.status.success(),
"a surplus argument exited 0: {said:?}"
);
assert!(
said.contains("\"area\""),
"the refusal does not name it: {said:?}"
);
}
/// Diagnostics go to stderr so that stdout stays free for a data output, and
/// so `2>/dev/null` is the whole suppression story. `context/code/cli.md`.
#[test]