Merge pull request #131 from fiddlerwoaroof/fix/cli-honesty
CI / Build all bundles (push) Has been cancelled
CI / Test and lint (host) (push) Has been cancelled
CI / wasm32 check (browser bundle crates) (push) Has been cancelled
CI / Browser JS unit tests (node --test) (push) Has been cancelled
CI / Knowledge base consistency (push) Has been cancelled
CI / Lint the harness crate (push) Has been cancelled
CI / Browser harness (OPFS contract + shell) (push) Has been cancelled
CI / Deploy to GitHub Pages (push) Has been cancelled
CI / Build all bundles (push) Has been cancelled
CI / Test and lint (host) (push) Has been cancelled
CI / wasm32 check (browser bundle crates) (push) Has been cancelled
CI / Browser JS unit tests (node --test) (push) Has been cancelled
CI / Knowledge base consistency (push) Has been cancelled
CI / Lint the harness crate (push) Has been cancelled
CI / Browser harness (OPFS contract + shell) (push) Has been cancelled
CI / Deploy to GitHub Pages (push) Has been cancelled
fix: the headless CLI was silent whether it worked or not, and the refusal named the wrong command
This commit is contained in:
@@ -190,6 +190,30 @@ improvise cmd 'add-cat Region' 'add-item Region East' -f model.improv
|
||||
improvise script setup.txt -f model.improv
|
||||
```
|
||||
|
||||
The language is the `:` command line, exactly as typed in the TUI — not JSON.
|
||||
Arguments split on whitespace, so anything containing a space is quoted:
|
||||
|
||||
```sh
|
||||
improvise cmd 'formula Product "Total = Widget + Gadget"' -f model.improv
|
||||
```
|
||||
|
||||
### What the headless commands promise
|
||||
|
||||
- **Everything a command reports goes to stderr**, as it would appear on the
|
||||
TUI's status line, and stdout is left empty. A run that had nothing to report
|
||||
prints nothing, so `2>/dev/null` is the whole suppression story and there is
|
||||
no quiet flag.
|
||||
- **Exit 1** means a line did not parse, or an effect aborted the rest of its
|
||||
batch — a failed `:w`, say. **Exit 0 means neither of those happened**, and
|
||||
it does *not* promise every command did what it was asked: a refusal that
|
||||
only writes a status message (`add-item` naming a category that does not
|
||||
exist) is printed but still exits 0, because a status message carries no
|
||||
severity for the exit code to read. Check stderr as well as `$?` when it
|
||||
matters.
|
||||
- **A run that changed nothing does not rewrite the document.** Writing it back
|
||||
regardless silently normalises it — `1e20` becomes its twenty digits — which
|
||||
turns a hand-edited fixture into a no-op on the next run.
|
||||
|
||||
## In the browser
|
||||
|
||||
The browser page is at **<https://fiddlerwoaroof.github.io/improvise/app/>**,
|
||||
|
||||
@@ -0,0 +1,117 @@
|
||||
# The headless CLI
|
||||
|
||||
`improvise cmd` and `improvise script` are one function — `run_headless_commands`
|
||||
in `src/main.rs`; `run_headless_script` reads the file and delegates. Every claim
|
||||
below therefore holds for both, and a fix to one is a fix to both by
|
||||
construction rather than by remembering.
|
||||
|
||||
## 1 — the status line is the only channel, so it must be printed
|
||||
|
||||
`App` has no return channel for "what happened". A command's outcome — a
|
||||
refusal, a warning, a `fit-measure`'s reproducing seed — is written to
|
||||
`ViewState::status_msg` and rendered by the TUI's bottom bar. Headless, nothing
|
||||
read it, so **success and refusal were byte-identical: silence and exit 0**
|
||||
(improvise-q914, improvise-j6x8). Two things followed that are worth keeping
|
||||
in mind, because both were mistaken for correct behaviour by agents working
|
||||
above this layer:
|
||||
|
||||
- A P1's carefully-worded refusal can be written, tested at the effect, and
|
||||
still reach nobody, because the test asserts the *effect's* status and the
|
||||
surface that would show it drops it. That is this repo's characteristic
|
||||
defect one layer out, and the only cure is a test at the process boundary —
|
||||
`tests/headless_cli.rs`, which spawns the real binary.
|
||||
- `improvise cmd 'fit-measure …'` looked like it did nothing whether it worked
|
||||
or not, which made the whole GP feature read as broken from a script.
|
||||
|
||||
The loop takes the message with `std::mem::take` and clears it per command, so
|
||||
one command's status cannot be reported twice or attributed to its successor.
|
||||
|
||||
## 2 — stderr, all of it, and why no split was possible
|
||||
|
||||
The tempting design is stdout for a report and stderr for a refusal. **It
|
||||
cannot be built honestly**, because `status_msg` is a bare `String` with no
|
||||
severity, and both candidate classifiers are disproved by code in the tree:
|
||||
|
||||
- *"A refusal is a batch that aborted."* Only three effects set
|
||||
`abort_effects` (gotcha 21). `AddItem` writes `workbook.add_item`'s `Err`
|
||||
straight to the status and does not abort — and that is q914's own worked
|
||||
example of a refusal. So this classifier puts a refusal on stdout labelled a
|
||||
success.
|
||||
- *"A refusal is anything that spoke."* `cmd/fit.rs` sets a status on
|
||||
**success**, carrying the seed that reproduces the fit. So this one labels a
|
||||
deliverable a failure.
|
||||
|
||||
Guessing between them would be precisely the adjacent-check defect
|
||||
`context/design-principles.md` §7 names. So everything goes to stderr: it is
|
||||
the diagnostics stream, it leaves stdout free for a future data output, and it
|
||||
matches every other diagnostic `main.rs` already emits (`Saved to …`,
|
||||
`warning: …`, `Parse error: …`).
|
||||
|
||||
**No `--quiet` flag.** Silence is never the default and `2>/dev/null` is the
|
||||
standard suppression; a flag would be a second knob with nothing asking for it.
|
||||
|
||||
## 3 — the exit code, and the hole left open deliberately
|
||||
|
||||
Exit 1 on a parse error (pre-existing) or on `App::last_batch_aborted()`. Both
|
||||
are real flags read off state the code already keeps, so neither is a guess.
|
||||
|
||||
What this does **not** cover, stated rather than papered over: a refusal that
|
||||
only writes a status still exits 0. Closing it needs a severity beside
|
||||
`status_msg`, which lives in `ViewState` and is therefore a wire change
|
||||
(gotcha 25) reaching the browser client — out of proportion to the CLI, and
|
||||
filed as improvise-udt2, which names both rivals so the next attempt cannot
|
||||
rediscover them. Until then the README says plainly that exit 0 means "nothing
|
||||
aborted", not "everything worked", because a contract that over-promises is
|
||||
worse than a narrow one.
|
||||
|
||||
## 4 — a run that changed nothing does not write
|
||||
|
||||
`persistence::save` used to run unconditionally, so `improvise cmd` normalised
|
||||
the document on every invocation — `1e20` expanded to twenty digits. The cost
|
||||
was not cosmetic: an agent editing a fixture by string-replacing a value after
|
||||
one run got a silent no-op, and two misleading test results out of it.
|
||||
|
||||
The gate is `changed`, accumulated from `model_state.dirty` **after each
|
||||
command**, not read once at the end. Reading it at the end is wrong because
|
||||
`effect::Save` clears the flag, so `[add-item …, w other.improv]` would look
|
||||
unchanged and drop the edit from the `-f` document. `!path.exists()` is or-ed
|
||||
in so that naming a new file still creates it.
|
||||
|
||||
Over-approximation is the safe direction here, the same one `Dirty::of` takes
|
||||
(gotcha 35): a `Model(_)` effect that happens to write the value already there
|
||||
still costs one redundant save.
|
||||
|
||||
## 5 — the refusal names the word the user typed
|
||||
|
||||
`CmdRegistry::resolve` maps `formula` → `add-formula` *before* the arity check
|
||||
runs, so every validator that spelled its own name reported a command nobody
|
||||
typed (improvise-z05h). The fix moves the name to the one place that knows
|
||||
which of the two the user wrote: `require_args` no longer takes a `word`, and
|
||||
`CmdRegistry::parse` prefixes `"{typed}: "` onto any `Err`.
|
||||
|
||||
That removed 41 name literals from `cmd/`, which is the same argument gotcha 15
|
||||
makes about `registry.rs` having no name literals — a name spelled twice can be
|
||||
spelled wrong once. `interactive` deliberately does **not** prefix: `dispatch`
|
||||
in `command/keymap.rs` already formats `"{name}: {e}"` with the *bound* name
|
||||
(improvise-55l5, PR #126), and prefixing in both would double it.
|
||||
|
||||
The message states the **shape** rather than the arity, because
|
||||
"requires 1-2 argument(s), got 6" does not tell anyone to quote anything.
|
||||
|
||||
## 6 — the help's ex-commands were the unguarded surface
|
||||
|
||||
`help.rs` derives `HelpLine::Keys` rows from the keymap and drops any row whose
|
||||
command does not resolve, so a key row cannot go stale. `HelpLine::Literal` and
|
||||
`HelpLine::Accent` rows are hand-written prose and had nothing checking them —
|
||||
which is how all three documented spellings of `:formula` came to be ones the
|
||||
parser refuses.
|
||||
|
||||
`every_ex_command_the_help_pages_show_is_one_the_parser_accepts` closes that
|
||||
surface rather than the one instance: every help line whose trimmed text opens
|
||||
with `:` is split on its colons and each piece handed to `parse_line`. The
|
||||
split exists because one row shows two examples side by side.
|
||||
|
||||
What it still cannot see, and should be closed if the welcome text moves again:
|
||||
`draw.rs::draw_welcome` builds its lines as a local array inside a ratatui
|
||||
render function, so nothing can walk them. Its `:formula` line was corrected by
|
||||
hand.
|
||||
@@ -21,7 +21,7 @@ Published, in four sections:
|
||||
| Start here | `docs/design-notes.md`, `context/design-principles.md`, `context/repo-map.md` | What the project is for, the doctrine, and the single best explanation of how it is built. |
|
||||
| Decision records | `context/adr/*.md` | An ADR is written for an outside reader by construction — it states the options that were rejected, which is the part that is worth reading and the part that is nowhere else. |
|
||||
| The arguments, by area | `context/code/*.md` | This is the reasoning that used to live in code comments and was moved out under the two-line cap (design-principles §6). It is the most valuable prose in the repository and it is addressed to exactly the reader a documentation site has. |
|
||||
| Test evidence | `context/test-evidence/*.md` | 501 entries, one per test whose narrative is longer than its code. |
|
||||
| Test evidence | `context/test-evidence/*.md` | One entry per test whose narrative is longer than its code. |
|
||||
|
||||
Not published:
|
||||
|
||||
@@ -62,9 +62,9 @@ says `context/code/*.md` is published; the manifest disagreed and nothing could
|
||||
see it. That is the whole failure mode in one file.
|
||||
|
||||
**Test evidence is the one judgement call worth stating.** The argument for
|
||||
leaving it out is that 501 entries about individual tests are noise to a reader
|
||||
who wanted to know what a pivot table is. The argument for keeping it is
|
||||
stronger: every one of the seven `context/code/*.md` documents points at its
|
||||
leaving it out is that hundreds of entries about individual tests are noise
|
||||
to a reader who wanted to know what a pivot table is. The argument for keeping it is
|
||||
stronger: every one of the `context/code/*.md` documents points at its
|
||||
evidence file by name, so omitting it would break the published prose in
|
||||
exactly the way this site is otherwise careful not to; and the evidence file is
|
||||
the most distinctive artefact the repository has — a written record of the
|
||||
|
||||
@@ -0,0 +1,137 @@
|
||||
# Test evidence — the headless CLI (`src/main.rs`, `tests/headless_cli.rs`)
|
||||
|
||||
Keyed by exact test name, per design-principles §7. A renamed test orphans its
|
||||
entry; `python3 scripts/check-comment-runs.py --check-evidence` catches a
|
||||
heading that names no test.
|
||||
|
||||
---
|
||||
|
||||
## `tests/headless_cli.rs` — the contract at the process boundary
|
||||
|
||||
improvise-q914 / improvise-j6x8 / improvise-z05h. The reasoning is
|
||||
`context/code/cli.md`; this file records what broke and what each test can see.
|
||||
|
||||
**Why a spawned binary rather than a unit test.** The three things under test
|
||||
are an exit code, a choice of stream, and whether a file was written — none of
|
||||
which exists below `std::process::exit`. Per design-principles §7 this is a
|
||||
boundary contract, so it is pinned once, slowly and honestly, and the
|
||||
`command/cmd/` unit tests assume it. Measured on the orchestrator's machine the
|
||||
whole binary runs in ~0.5s, twelve process spawns included, which is well
|
||||
inside the per-binary budget.
|
||||
|
||||
**What was broken.** `run_headless_commands` applied effects and never read
|
||||
`app.view_state.status_msg`, and set `exit_code` only for parse errors. So a
|
||||
successful command and a refused one produced byte-identical output — nothing —
|
||||
and exit 0 for both. Measured at `bf83294` before the fix: six of the twelve
|
||||
tests below failed.
|
||||
|
||||
### a_refused_command_says_what_went_wrong
|
||||
|
||||
`add-item Nonexistent Thing` against a document that has no such category.
|
||||
`Workbook::add_item`'s `Err` reaches `status_msg` and the effect does **not**
|
||||
set `abort_effects`, which is what makes this the load-bearing case: it is a
|
||||
genuine refusal that no flag in the codebase classifies as one.
|
||||
|
||||
Asserts the category's name appears, not merely that something was printed —
|
||||
"something was printed" is satisfied by a fixed `ok` on every command.
|
||||
|
||||
### a_command_with_nothing_to_say_says_nothing
|
||||
|
||||
**The rival this exists to kill**, and it is the one the brief named: a fix
|
||||
that prints on the happy path and stays silent on the refusal. Injected as
|
||||
`if said.trim().is_empty() { eprintln!("ok") }` — the exact inversion — and it
|
||||
fails this test *and* `a_refused_command_says_what_went_wrong`, five tests in
|
||||
all. A test that only asked "was anything printed" passes that rival.
|
||||
|
||||
`add-category Region` is the input because it sets no status at all, so the
|
||||
assertion is about the writer rather than about this command's vocabulary.
|
||||
|
||||
### an_aborted_batch_exits_non_zero_and_says_why
|
||||
|
||||
`:w` into a directory that does not exist. `effect::Save` reports through
|
||||
`report_file_write`, which sets the status **and** `abort_effects` (gotcha 21),
|
||||
so this is the one arm the exit code can honestly read. Before the fix this
|
||||
exited 0 with no output, which is what made `improvise cmd … && next-step`
|
||||
proceed over a failed write.
|
||||
|
||||
### a_command_that_succeeds_while_reporting_still_exits_zero
|
||||
|
||||
**The rival this exists to kill**: making any emitted status a non-zero exit,
|
||||
which is the obvious way to widen the previous test and is wrong. `effect::Save`
|
||||
on **success** also writes a status, as does `cmd/fit.rs` — whose status carries
|
||||
the seed that reproduces the fit. So "it spoke" cannot mean "it failed".
|
||||
|
||||
The first assertion is a check-the-checker: if the save ever stops reporting,
|
||||
the test proves nothing and says so rather than passing quietly.
|
||||
|
||||
### a_parse_error_still_exits_non_zero
|
||||
|
||||
The pre-existing arm, kept so the widening cannot silently drop it. Also
|
||||
asserts the message names the word typed, which the `Unknown command` arm
|
||||
reports post-alias-resolution.
|
||||
|
||||
### what_a_command_says_goes_to_stderr_and_stdout_stays_empty
|
||||
|
||||
Pins the stream choice itself. Both halves are asserted: a rival that writes to
|
||||
both streams passes an stderr-only check.
|
||||
|
||||
### a_script_reports_exactly_as_cmd_does
|
||||
|
||||
`improvise script` is the same function, so this cannot fail while `cmd`
|
||||
passes — which is the point. It is here because the ticket asked whether the
|
||||
hole was shared, and a claim that two paths are one is worth one cheap
|
||||
assertion rather than a sentence. The script also carries a `#` comment, so the
|
||||
comment-skipping arm is exercised on the way through.
|
||||
|
||||
### a_run_that_changes_nothing_does_not_rewrite_the_document
|
||||
|
||||
`move-cursor down` is `ViewOnly`, so nothing is dirty. **This test passed
|
||||
before the fix** — a freshly written document re-serialises byte-identically —
|
||||
and it is kept as the control that says so. The discriminating one is below.
|
||||
|
||||
### a_no_op_run_leaves_an_unnormalised_literal_alone
|
||||
|
||||
The reported instance. A hand-written document carrying `1e20` is rewritten as
|
||||
`100000000000000000000` by a run asked to change nothing, because
|
||||
`persistence::save` ran unconditionally. It fails before the fix where its
|
||||
sibling does not, which is exactly why both are here: the sibling cannot see a
|
||||
normalisation, only a reordering.
|
||||
|
||||
The first assertion checks the fixture still contains `1e20` before the run, so
|
||||
a future grammar change that rejects the literal fails loudly instead of making
|
||||
the test vacuous.
|
||||
|
||||
### a_run_that_changed_the_document_still_writes_it
|
||||
|
||||
The regression the dirty-gate could have introduced. `effect::Save` sets
|
||||
`dirty = false`, so a gate reading the flag once at the end would see
|
||||
`[add-item East, w other.improv]` as unchanged and silently drop the edit from
|
||||
the `-f` document. Accumulating `changed` per command is what this asserts; a
|
||||
gate written the obvious way fails it.
|
||||
|
||||
### the_cmd_help_does_not_advertise_a_language_it_refuses
|
||||
|
||||
Three clap doc strings said JSON three times over. Checks both subcommands,
|
||||
because `script`'s `<PATH>` help carried the same claim and a fix to `cmd`
|
||||
alone reads as complete.
|
||||
|
||||
### the_language_the_help_names_is_the_one_it_accepts
|
||||
|
||||
The other direction, and the reason the test above is not merely a spell check:
|
||||
it demonstrates the `:` command line works end to end *and* that the JSON form
|
||||
the help advertised is genuinely refused. Without this half, deleting the word
|
||||
"JSON" from the help would satisfy the check while the CLI still accepted
|
||||
neither language.
|
||||
|
||||
#### Scratch (helper)
|
||||
|
||||
A per-test temp directory named from the test's own tag and the process id, so
|
||||
the twelve tests cannot collide, with a `Drop` that removes it. Not
|
||||
`tempfile::TempDir` only because the fixture also has to be readable by a
|
||||
spawned child at a stable path.
|
||||
|
||||
#### seeded (helper)
|
||||
|
||||
Runs `add-category Region` to create the document. It asserts the file exists
|
||||
afterwards rather than trusting the seed, because every test that follows would
|
||||
otherwise misreport as "the command did nothing" when the seed was what failed.
|
||||
@@ -4398,3 +4398,57 @@ of a view that already exists. The consequence is one level further on, in
|
||||
rather than on the applied workbook keeps the test in the `Cmd` layer where a
|
||||
command is a pure function of its context; the applied consequence is asserted
|
||||
in `lockstep.rs`, where it is also held against the browser's answer.
|
||||
|
||||
---
|
||||
|
||||
## `command/cmd/registry.rs` — a refusal names the word that was typed
|
||||
|
||||
improvise-z05h. Reasoning: `context/code/cli.md` §5 and §6.
|
||||
|
||||
### a_refusal_names_the_command_as_typed_through_an_alias
|
||||
|
||||
`CmdRegistry::resolve` rewrites `formula` to `add-formula` before the arity
|
||||
check runs, so the refusal for `formula Product Total = Widget + Gadget` named
|
||||
a command the user had not typed. Both directions are asserted — that the
|
||||
message opens with `formula:` **and** that `add-formula` does not appear in it,
|
||||
because prefixing the typed name onto a message that still spells the canonical
|
||||
one reads as fixed and is not.
|
||||
|
||||
The alias is what makes this discriminating; its sibling below covers the case
|
||||
where the two names coincide, so neither can pass by accident.
|
||||
|
||||
### a_refusal_from_a_canonical_name_is_named_as_typed
|
||||
|
||||
The control. `add-item` is not aliased, so a fix that only rewrote aliased
|
||||
names would pass the test above and fail here. Together they say the name comes
|
||||
from the registry in every case, which is what removing 41 name literals from
|
||||
`require_args` call sites bought.
|
||||
|
||||
### a_formula_refusal_states_the_shape_and_asks_for_quoting
|
||||
|
||||
The third of z05h's stacked bugs, and the one a user actually needs.
|
||||
"requires 1-2 argument(s), got 6" is true and unactionable: nothing in it
|
||||
suggests that the formula must be quoted. Asserts the message names the shape
|
||||
and contains the word `quote`, so a refusal that merely restates the arity with
|
||||
a friendlier name still fails.
|
||||
|
||||
### every_ex_command_the_help_pages_show_is_one_the_parser_accepts
|
||||
|
||||
The surface z05h identified as unguarded. `help.rs` derives `HelpLine::Keys`
|
||||
rows from the keymap and drops any whose command does not resolve, so key rows
|
||||
cannot go stale — but `Literal` and `Accent` rows are prose, and all three
|
||||
documented spellings of `:formula` were ones the parser refuses.
|
||||
|
||||
This closes the surface rather than the instance: every help line whose trimmed
|
||||
text opens with `:` is split on its colons — one row shows two `:add-cat`
|
||||
examples side by side — and each piece is handed to `parse_line`. Run against
|
||||
`bf83294` it fails naming the Welcome page and the exact line.
|
||||
|
||||
The `checked >= 8` floor is a check-the-checker: a future refactor that stops
|
||||
`pages()` emitting `Accent`/`Literal` rows, or that indents them differently,
|
||||
would otherwise leave the test scanning nothing and passing.
|
||||
|
||||
What it cannot see, stated because the gap is one edit away from mattering:
|
||||
`draw.rs::draw_welcome` builds its lines as a local array inside a ratatui
|
||||
render function, so no walk reaches them. Its `:formula` line was corrected by
|
||||
hand in the same change.
|
||||
|
||||
@@ -222,10 +222,12 @@ impl CmdRegistry {
|
||||
|
||||
/// Construct a command from text arguments (script/headless).
|
||||
pub fn parse(&self, name: &str, args: &[String]) -> Result<Box<dyn Cmd>, String> {
|
||||
let name = self.resolve(name);
|
||||
// The registry is the only thing that knows the word the user typed, so
|
||||
// it is the only thing that may name it — `context/code/command.md`.
|
||||
let canonical = self.resolve(name);
|
||||
for e in &self.entries {
|
||||
if e.name == name {
|
||||
return (e.parse)(args);
|
||||
if e.name == canonical {
|
||||
return (e.parse)(args).map_err(|err| format!("{name}: {err}"));
|
||||
}
|
||||
}
|
||||
Err(format!("Unknown command: {name}"))
|
||||
@@ -254,12 +256,9 @@ impl CmdRegistry {
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn require_args(word: &str, args: &[String], n: usize) -> Result<(), String> {
|
||||
pub(super) fn require_args(args: &[String], n: usize) -> Result<(), String> {
|
||||
if args.len() < n {
|
||||
Err(format!(
|
||||
"{word} requires {n} argument(s), got {}",
|
||||
args.len()
|
||||
))
|
||||
Err(format!("requires {n} argument(s), got {}", args.len()))
|
||||
} else {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -112,7 +112,7 @@ macro_rules! effect_cmd {
|
||||
effect_cmd!(
|
||||
AddCategoryCmd,
|
||||
"add-category",
|
||||
|args: &[String]| require_args("add-category", args, 1),
|
||||
|args: &[String]| require_args(args, 1),
|
||||
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
|
||||
vec![Box::new(effect::AddCategory(args[0].clone()))]
|
||||
}
|
||||
@@ -121,7 +121,7 @@ effect_cmd!(
|
||||
effect_cmd!(
|
||||
AddItemCmd,
|
||||
"add-item",
|
||||
|args: &[String]| require_args("add-item", args, 2),
|
||||
|args: &[String]| require_args(args, 2),
|
||||
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
|
||||
vec![Box::new(effect::AddItem {
|
||||
category: args[0].clone(),
|
||||
@@ -157,7 +157,7 @@ effect_cmd!(
|
||||
effect_cmd!(
|
||||
AddItemInGroupCmd,
|
||||
"add-item-in-group",
|
||||
|args: &[String]| require_args("add-item-in-group", args, 3),
|
||||
|args: &[String]| require_args(args, 3),
|
||||
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
|
||||
vec![Box::new(effect::AddItemInGroup {
|
||||
category: args[0].clone(),
|
||||
@@ -200,8 +200,9 @@ effect_cmd!(
|
||||
"add-formula",
|
||||
|args: &[String]| {
|
||||
if args.is_empty() || args.len() > 2 {
|
||||
// The arity alone does not tell anyone to quote — improvise-z05h.
|
||||
return Err(format!(
|
||||
"add-formula requires 1-2 argument(s), got {}",
|
||||
"expected [<category>] \"<Name> = <expr>\" — got {} word(s); quote the formula",
|
||||
args.len()
|
||||
));
|
||||
}
|
||||
@@ -225,7 +226,7 @@ effect_cmd!(
|
||||
effect_cmd!(
|
||||
ClearBufferCmd,
|
||||
"clear-buffer",
|
||||
|args: &[String]| require_args("clear-buffer", args, 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())]
|
||||
}
|
||||
@@ -234,7 +235,7 @@ effect_cmd!(
|
||||
effect_cmd!(
|
||||
RemoveFormulaCmd,
|
||||
"remove-formula",
|
||||
|args: &[String]| require_args("remove-formula", args, 2),
|
||||
|args: &[String]| require_args(args, 2),
|
||||
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
|
||||
vec![Box::new(effect::RemoveFormula {
|
||||
target_category: args[0].clone(),
|
||||
@@ -246,7 +247,7 @@ effect_cmd!(
|
||||
effect_cmd!(
|
||||
CreateViewCmd,
|
||||
"create-view",
|
||||
|args: &[String]| require_args("create-view", args, 1),
|
||||
|args: &[String]| require_args(args, 1),
|
||||
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
|
||||
vec![Box::new(effect::CreateView(args[0].clone()))]
|
||||
}
|
||||
@@ -255,7 +256,7 @@ effect_cmd!(
|
||||
effect_cmd!(
|
||||
DeleteViewCmd,
|
||||
"delete-view",
|
||||
|args: &[String]| require_args("delete-view", args, 1),
|
||||
|args: &[String]| require_args(args, 1),
|
||||
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
|
||||
vec![Box::new(effect::DeleteView(args[0].clone()))]
|
||||
}
|
||||
@@ -264,7 +265,7 @@ effect_cmd!(
|
||||
effect_cmd!(
|
||||
SwitchViewCmd,
|
||||
"switch-view",
|
||||
|args: &[String]| require_args("switch-view", args, 1),
|
||||
|args: &[String]| require_args(args, 1),
|
||||
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
|
||||
vec![Box::new(effect::SwitchView(args[0].clone()))]
|
||||
}
|
||||
@@ -273,7 +274,7 @@ effect_cmd!(
|
||||
effect_cmd!(
|
||||
SetAxisCmd,
|
||||
"set-axis",
|
||||
|args: &[String]| require_args("set-axis", args, 2),
|
||||
|args: &[String]| require_args(args, 2),
|
||||
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
|
||||
let axis = match args[1].to_lowercase().as_str() {
|
||||
"row" => Axis::Row,
|
||||
@@ -293,7 +294,7 @@ effect_cmd!(
|
||||
effect_cmd!(
|
||||
SetPageCmd,
|
||||
"set-page",
|
||||
|args: &[String]| require_args("set-page", args, 2),
|
||||
|args: &[String]| require_args(args, 2),
|
||||
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
|
||||
vec![Box::new(effect::SetPageSelection {
|
||||
view: ViewTarget::Active,
|
||||
@@ -306,7 +307,7 @@ effect_cmd!(
|
||||
effect_cmd!(
|
||||
ToggleGroupCmd,
|
||||
"toggle-group",
|
||||
|args: &[String]| require_args("toggle-group", args, 2),
|
||||
|args: &[String]| require_args(args, 2),
|
||||
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
|
||||
vec![Box::new(effect::ToggleGroup {
|
||||
view: ViewTarget::Active,
|
||||
@@ -319,7 +320,7 @@ effect_cmd!(
|
||||
effect_cmd!(
|
||||
HideItemCmd,
|
||||
"hide-item",
|
||||
|args: &[String]| require_args("hide-item", args, 2),
|
||||
|args: &[String]| require_args(args, 2),
|
||||
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
|
||||
vec![Box::new(effect::HideItem {
|
||||
view: ViewTarget::Active,
|
||||
@@ -332,7 +333,7 @@ effect_cmd!(
|
||||
effect_cmd!(
|
||||
ShowItemCmd,
|
||||
"show-item",
|
||||
|args: &[String]| require_args("show-item", args, 2),
|
||||
|args: &[String]| require_args(args, 2),
|
||||
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
|
||||
vec![Box::new(effect::ShowItem {
|
||||
view: ViewTarget::Active,
|
||||
@@ -345,7 +346,7 @@ effect_cmd!(
|
||||
effect_cmd!(
|
||||
SaveAsCmd,
|
||||
"save-as",
|
||||
|args: &[String]| require_args("save-as", args, 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])))]
|
||||
}
|
||||
@@ -354,7 +355,7 @@ effect_cmd!(
|
||||
effect_cmd!(
|
||||
SetFormatCmd,
|
||||
"set-format",
|
||||
|args: &[String]| require_args("set-format", args, 1),
|
||||
|args: &[String]| require_args(args, 1),
|
||||
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
|
||||
vec![Box::new(effect::SetNumberFormat(
|
||||
ViewTarget::Active,
|
||||
@@ -366,7 +367,7 @@ effect_cmd!(
|
||||
effect_cmd!(
|
||||
ImportCmd,
|
||||
"import",
|
||||
|args: &[String]| require_args("import", args, 1),
|
||||
|args: &[String]| require_args(args, 1),
|
||||
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
|
||||
vec![Box::new(effect::StartImportWizard(args[0].clone()))]
|
||||
}
|
||||
@@ -428,7 +429,7 @@ effect_cmd!(
|
||||
effect_cmd!(
|
||||
LoadModelCmd,
|
||||
"load",
|
||||
|args: &[String]| require_args("load", args, 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(
|
||||
&args[0],
|
||||
@@ -439,7 +440,7 @@ effect_cmd!(
|
||||
effect_cmd!(
|
||||
ExportCsvCmd,
|
||||
"export-csv",
|
||||
|args: &[String]| require_args("export-csv", args, 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(
|
||||
&args[0],
|
||||
|
||||
@@ -90,7 +90,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
cmd_name: "move-selection",
|
||||
},
|
||||
|args| {
|
||||
require_args("move-selection", args, 2)?;
|
||||
require_args(args, 2)?;
|
||||
let dr = args[0].parse::<i32>().map_err(|e| e.to_string())?;
|
||||
let dc = args[1].parse::<i32>().map_err(|e| e.to_string())?;
|
||||
Ok(Box::new(Move {
|
||||
@@ -100,7 +100,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
}))
|
||||
},
|
||||
|args, ctx| {
|
||||
require_args("move-selection", args, 2)?;
|
||||
require_args(args, 2)?;
|
||||
let dr = args[0].parse::<i32>().map_err(|e| e.to_string())?;
|
||||
let dc = args[1].parse::<i32>().map_err(|e| e.to_string())?;
|
||||
Ok(Box::new(Move {
|
||||
@@ -159,7 +159,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
cmd_name: "scroll-rows",
|
||||
},
|
||||
|args| {
|
||||
require_args("scroll-rows", args, 1)?;
|
||||
require_args(args, 1)?;
|
||||
let n = args[0].parse::<i32>().map_err(|e| e.to_string())?;
|
||||
Ok(Box::new(Move {
|
||||
kind: MoveKind::Relative(n, 0),
|
||||
@@ -168,7 +168,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
}))
|
||||
},
|
||||
|args, ctx| {
|
||||
require_args("scroll-rows", args, 1)?;
|
||||
require_args(args, 1)?;
|
||||
let n = args[0].parse::<i32>().map_err(|e| e.to_string())?;
|
||||
Ok(Box::new(Move {
|
||||
kind: MoveKind::Relative(n, 0),
|
||||
@@ -184,7 +184,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
cmd_name: "page-scroll",
|
||||
},
|
||||
|args| {
|
||||
require_args("page-scroll", args, 1)?;
|
||||
require_args(args, 1)?;
|
||||
let dir = args[0].parse::<i32>().map_err(|e| e.to_string())?;
|
||||
Ok(Box::new(Move {
|
||||
kind: MoveKind::Page(dir),
|
||||
@@ -193,7 +193,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
}))
|
||||
},
|
||||
|args, ctx| {
|
||||
require_args("page-scroll", args, 1)?;
|
||||
require_args(args, 1)?;
|
||||
let dir = args[0].parse::<i32>().map_err(|e| e.to_string())?;
|
||||
Ok(Box::new(Move {
|
||||
kind: MoveKind::Page(dir),
|
||||
@@ -280,7 +280,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
edit_mode: AppMode::Normal,
|
||||
},
|
||||
|args| {
|
||||
require_args("edit-or-drill", args, 1)?;
|
||||
require_args(args, 1)?;
|
||||
let edit_mode = parse_mode_name(&args[0])?;
|
||||
Ok(Box::new(EditOrDrill { edit_mode }))
|
||||
},
|
||||
@@ -290,7 +290,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
target_mode: AppMode::Normal,
|
||||
},
|
||||
|args| {
|
||||
require_args("enter-edit-at-cursor", args, 1)?;
|
||||
require_args(args, 1)?;
|
||||
let target_mode = parse_mode_name(&args[0])?;
|
||||
Ok(Box::new(EnterEditAtCursorCmd { target_mode }))
|
||||
},
|
||||
@@ -320,7 +320,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
|_, _| Ok(Box::new(ViewNavigate { forward: true })),
|
||||
);
|
||||
r.register_pure(&EnterMode(AppMode::Normal), |args| {
|
||||
require_args("enter-mode", args, 1)?;
|
||||
require_args(args, 1)?;
|
||||
Ok(Box::new(EnterMode(parse_mode_name(&args[0])?)))
|
||||
});
|
||||
|
||||
@@ -341,7 +341,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
},
|
||||
|args| {
|
||||
// Parse: toggle-panel-and-focus <panel> [open] [focused]
|
||||
require_args("toggle-panel-and-focus", args, 1)?;
|
||||
require_args(args, 1)?;
|
||||
let panel = parse_panel(&args[0])?;
|
||||
let open = args.get(1).map(|s| s == "true").unwrap_or(true);
|
||||
let focused = args.get(2).map(|s| s == "true").unwrap_or(open);
|
||||
@@ -352,7 +352,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
}))
|
||||
},
|
||||
|args, ctx| {
|
||||
require_args("toggle-panel-and-focus", args, 1)?;
|
||||
require_args(args, 1)?;
|
||||
let panel = parse_panel(&args[0])?;
|
||||
// Default interactive: if already open+focused -> close, else open+focus
|
||||
let currently_open = ctx.panel_open(panel);
|
||||
@@ -375,7 +375,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
currently_open: false,
|
||||
},
|
||||
|args| {
|
||||
require_args("toggle-panel-visibility", args, 1)?;
|
||||
require_args(args, 1)?;
|
||||
let panel = parse_panel(&args[0])?;
|
||||
Ok(Box::new(TogglePanelVisibility {
|
||||
panel,
|
||||
@@ -383,7 +383,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
}))
|
||||
},
|
||||
|args, ctx| {
|
||||
require_args("toggle-panel-visibility", args, 1)?;
|
||||
require_args(args, 1)?;
|
||||
let panel = parse_panel(&args[0])?;
|
||||
let currently_open = ctx.panel_open(panel);
|
||||
Ok(Box::new(TogglePanelVisibility {
|
||||
@@ -421,7 +421,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
max: 0,
|
||||
},
|
||||
|args| {
|
||||
require_args("move-panel-cursor", args, 2)?;
|
||||
require_args(args, 2)?;
|
||||
let panel = parse_panel(&args[0])?;
|
||||
let delta = args[1].parse::<i32>().map_err(|e| e.to_string())?;
|
||||
Ok(Box::new(MovePanelCursor {
|
||||
@@ -432,7 +432,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
}))
|
||||
},
|
||||
|args, ctx| {
|
||||
require_args("move-panel-cursor", args, 2)?;
|
||||
require_args(args, 2)?;
|
||||
let panel = parse_panel(&args[0])?;
|
||||
let delta = args[1].parse::<i32>().map_err(|e| e.to_string())?;
|
||||
let (current, max) = match panel {
|
||||
@@ -463,7 +463,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
|
||||
// ── Tile select ──────────────────────────────────────────────────────
|
||||
r.register_pure(&MoveTileCursor(0), |args| {
|
||||
require_args("move-tile-cursor", args, 1)?;
|
||||
require_args(args, 1)?;
|
||||
let delta = args[0].parse::<i32>().map_err(|e| e.to_string())?;
|
||||
Ok(Box::new(MoveTileCursor(delta)))
|
||||
});
|
||||
@@ -473,7 +473,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
axis: Some(crate::view::Axis::Row),
|
||||
},
|
||||
|args| {
|
||||
require_args("set-axis-for-tile", args, 1)?;
|
||||
require_args(args, 1)?;
|
||||
let axis = parse_axis(&args[0])?;
|
||||
Ok(Box::new(TileAxisOp { axis: Some(axis) }))
|
||||
},
|
||||
@@ -494,7 +494,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
buffer: String::new(),
|
||||
},
|
||||
|args| {
|
||||
require_args("append-char", args, 1)?;
|
||||
require_args(args, 1)?;
|
||||
Ok(Box::new(AppendChar {
|
||||
buffer: args[0].clone(),
|
||||
}))
|
||||
@@ -505,7 +505,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
buffer: String::new(),
|
||||
},
|
||||
|args| {
|
||||
require_args("pop-char", args, 1)?;
|
||||
require_args(args, 1)?;
|
||||
Ok(Box::new(PopChar {
|
||||
buffer: args[0].clone(),
|
||||
}))
|
||||
@@ -537,7 +537,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
}))
|
||||
},
|
||||
|args, ctx| {
|
||||
require_args("commit-cell-edit", args, 1)?;
|
||||
require_args(args, 1)?;
|
||||
let edit_mode = parse_mode_name(&args[0])?;
|
||||
let value = read_buffer(ctx, "edit");
|
||||
let key = ctx.cell_key().clone().ok_or("no cell at cursor")?;
|
||||
@@ -560,7 +560,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
},
|
||||
|_| Err("commit-and-advance-right requires context".into()),
|
||||
|args, ctx| {
|
||||
require_args("commit-and-advance-right", args, 1)?;
|
||||
require_args(args, 1)?;
|
||||
let edit_mode = parse_mode_name(&args[0])?;
|
||||
let value = read_buffer(ctx, "edit");
|
||||
let key = ctx.cell_key().clone().ok_or("no cell at cursor")?;
|
||||
@@ -578,12 +578,12 @@ pub fn default_registry() -> CmdRegistry {
|
||||
kind: CommitKind::Category,
|
||||
},
|
||||
|args| {
|
||||
require_args("commit-buffer", args, 1)?;
|
||||
require_args(args, 1)?;
|
||||
let kind = parse_commit_kind(&args[0], &AppMode::Normal)?;
|
||||
Ok(Box::new(CommitBuffer { kind }))
|
||||
},
|
||||
|args, ctx| {
|
||||
require_args("commit-buffer", args, 1)?;
|
||||
require_args(args, 1)?;
|
||||
let kind = parse_commit_kind(&args[0], ctx.mode)?;
|
||||
Ok(Box::new(CommitBuffer { kind }))
|
||||
},
|
||||
@@ -716,6 +716,83 @@ mod tests {
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// A refusal names the word the user typed, not the one the alias table
|
||||
/// resolved it to — `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn a_refusal_names_the_command_as_typed_through_an_alias() {
|
||||
let err = crate::command::parse_line("formula Product Total = Widget + Gadget")
|
||||
.expect_err("an unquoted formula is refused");
|
||||
assert!(
|
||||
err.starts_with("formula:"),
|
||||
"the refusal does not open with the word typed: {err:?}"
|
||||
);
|
||||
assert!(
|
||||
!err.contains("add-formula"),
|
||||
"the refusal names a command the user did not type: {err:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Stating the arity does not tell anyone what to do; stating the shape does.
|
||||
#[test]
|
||||
fn a_formula_refusal_states_the_shape_and_asks_for_quoting() {
|
||||
let err = crate::command::parse_line("formula Product Total = Widget + Gadget")
|
||||
.expect_err("an unquoted formula is refused");
|
||||
assert!(
|
||||
err.contains("quote"),
|
||||
"the refusal does not suggest quoting, so it is unactionable: {err:?}"
|
||||
);
|
||||
assert!(
|
||||
err.contains("<Name> = <expr>"),
|
||||
"the refusal does not state the shape it wanted: {err:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Every other refusal is named too, without the alias table in the way.
|
||||
#[test]
|
||||
fn a_refusal_from_a_canonical_name_is_named_as_typed() {
|
||||
let err = crate::command::parse_line("add-item").expect_err("add-item needs arguments");
|
||||
assert!(
|
||||
err.starts_with("add-item:"),
|
||||
"the refusal does not open with the word typed: {err:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// The ex-commands the help pages print are `HelpLine::Literal`/`Accent`
|
||||
/// rows, which no derivation checks — `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn every_ex_command_the_help_pages_show_is_one_the_parser_accepts() {
|
||||
let mut checked = 0;
|
||||
for page in improvise_protocol::help::pages() {
|
||||
for line in &page.lines {
|
||||
let text = match line {
|
||||
improvise_protocol::help::HelpLine::Accent(s) => *s,
|
||||
improvise_protocol::help::HelpLine::Literal { keys, .. } => *keys,
|
||||
_ => continue,
|
||||
};
|
||||
let text = text.trim();
|
||||
if !text.starts_with(':') {
|
||||
continue;
|
||||
}
|
||||
// One row may show two examples side by side, each opening
|
||||
// with its own colon.
|
||||
for example in text.split(':').filter(|s| !s.trim().is_empty()) {
|
||||
if let Err(e) = crate::command::parse_line(example.trim()) {
|
||||
panic!(
|
||||
"help page {:?} shows `:{}`, which the parser refuses: {e}",
|
||||
page.title,
|
||||
example.trim()
|
||||
);
|
||||
}
|
||||
checked += 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
checked >= 8,
|
||||
"only {checked} ex-command examples were found; the scan has stopped seeing them"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -412,7 +412,7 @@ fn welcome() -> Vec<HelpLine> {
|
||||
HelpLine::Text(" 3. Navigate with hjkl or arrow keys and press i to edit cells."),
|
||||
HelpLine::Blank,
|
||||
HelpLine::Text(" 4. Add formulas to compute values automatically:"),
|
||||
HelpLine::Accent(" :formula Product Total = Widget + Gadget"),
|
||||
HelpLine::Accent(" :formula Product \"Total = Widget + Gadget\""),
|
||||
HelpLine::Blank,
|
||||
HelpLine::Text(" 5. Save your work:"),
|
||||
HelpLine::Accent(" :w mymodel.improv"),
|
||||
@@ -568,7 +568,7 @@ fn editing() -> Vec<HelpLine> {
|
||||
" Example: in a Product category with items Widget and Gadget:",
|
||||
)),
|
||||
Some(HelpLine::Accent(
|
||||
" :formula Product Total = Widget + Gadget",
|
||||
" :formula Product \"Total = Widget + Gadget\"",
|
||||
)),
|
||||
Some(HelpLine::Blank),
|
||||
Some(HelpLine::Text(" Supported operators: + - * /")),
|
||||
@@ -780,7 +780,10 @@ fn commands() -> Vec<HelpLine> {
|
||||
":add-items <cat> a b c ...",
|
||||
"Add multiple items at once",
|
||||
)),
|
||||
Some(literal(":formula <cat> <Name=expr>", "Add a formula")),
|
||||
Some(literal(
|
||||
":formula <cat> \"<Name> = <expr>\"",
|
||||
"Add a formula",
|
||||
)),
|
||||
Some(HelpLine::Blank),
|
||||
Some(HelpLine::Heading("Views")),
|
||||
Some(HelpLine::Blank),
|
||||
|
||||
@@ -58,6 +58,7 @@ const SECTIONS = [
|
||||
["context/code/command.md", "code/command", "improvise-command"],
|
||||
["context/code/clients.md", "code/clients", "The clients"],
|
||||
["context/code/tui.md", "code/tui", "The TUI"],
|
||||
["context/code/cli.md", "code/cli", "The headless CLI"],
|
||||
["context/code/gp.md", "code/gp", "gp-tree and improvise-gp"],
|
||||
["context/code/kb-tool.md", "code/kb-tool", "The kb tool"],
|
||||
["context/code/tooling.md", "code/tooling", "The repository's own checkers"],
|
||||
@@ -75,6 +76,7 @@ const SECTIONS = [
|
||||
["context/test-evidence/command.md", "test-evidence/command", "improvise-command"],
|
||||
["context/test-evidence/clients.md", "test-evidence/clients", "The clients"],
|
||||
["context/test-evidence/tui.md", "test-evidence/tui", "The TUI"],
|
||||
["context/test-evidence/cli.md", "test-evidence/cli", "The headless CLI"],
|
||||
["context/test-evidence/gp.md", "test-evidence/gp", "gp-tree and improvise-gp"],
|
||||
["context/test-evidence/tooling.md", "test-evidence/tooling", "The repository's own gates"],
|
||||
],
|
||||
|
||||
+2
-2
@@ -483,11 +483,11 @@ fn draw_welcome(f: &mut Frame, area: Rect) {
|
||||
Style::default().fg(Color::Cyan),
|
||||
),
|
||||
(
|
||||
":formula <cat> <expr> Add a formula, e.g.:",
|
||||
":formula <cat> \"<Name> = <expr>\" Add a formula, e.g.:",
|
||||
Style::default().fg(Color::Cyan),
|
||||
),
|
||||
(
|
||||
" Profit = Revenue - Cost",
|
||||
" :formula _Measure \"Profit = Revenue - Cost\"",
|
||||
Style::default().fg(Color::Green),
|
||||
),
|
||||
(
|
||||
|
||||
+24
-6
@@ -43,7 +43,7 @@ trait Runnable {
|
||||
enum Commands {
|
||||
/// Import JSON or CSV data, then open TUI (or save with --output)
|
||||
Import(ImportArgs),
|
||||
/// Run a JSON command headless (repeatable)
|
||||
/// Run a command line headless, as typed after `:` (repeatable)
|
||||
Cmd(CmdArgs),
|
||||
/// Run commands from a script file headless
|
||||
Script(ScriptArgs),
|
||||
@@ -99,8 +99,9 @@ struct ImportArgs {
|
||||
|
||||
#[derive(clap::Args)]
|
||||
struct CmdArgs {
|
||||
/// JSON command strings
|
||||
json: Vec<String>,
|
||||
/// Command lines, in the `:` command language (e.g. `add-cat Region`)
|
||||
#[arg(value_name = "COMMAND")]
|
||||
commands: Vec<String>,
|
||||
|
||||
/// Model file to load/save
|
||||
#[arg(short, long)]
|
||||
@@ -109,7 +110,7 @@ struct CmdArgs {
|
||||
|
||||
#[derive(clap::Args)]
|
||||
struct ScriptArgs {
|
||||
/// Script file (one JSON command per line, # comments)
|
||||
/// Script file (one command line per line, `#` comments)
|
||||
path: PathBuf,
|
||||
|
||||
/// Model file to load/save
|
||||
@@ -155,7 +156,7 @@ impl Runnable for ImportArgs {
|
||||
|
||||
impl Runnable for CmdArgs {
|
||||
fn run(self, _model_file: Option<PathBuf>) -> Result<()> {
|
||||
run_headless_commands(&self.json, &self.file)
|
||||
run_headless_commands(&self.commands, &self.file)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -338,6 +339,9 @@ fn run_headless_commands(cmds: &[String], file: &Option<PathBuf>) -> Result<()>
|
||||
app.term_height = h;
|
||||
}
|
||||
let mut exit_code = 0;
|
||||
// `Save` clears `dirty`, so a run that edits and then `:w`s elsewhere would
|
||||
// otherwise look unchanged — `context/code/cli.md`.
|
||||
let mut changed = false;
|
||||
|
||||
for line in cmds {
|
||||
match command::parse_line(line) {
|
||||
@@ -348,6 +352,16 @@ fn run_headless_commands(cmds: &[String], file: &Option<PathBuf>) -> Result<()>
|
||||
cmd.execute(&ctx)
|
||||
};
|
||||
app.apply_effects(effects);
|
||||
changed |= app.model_state.dirty;
|
||||
// The status line is the only channel a command has to
|
||||
// report anything — `context/code/cli.md`.
|
||||
let said = std::mem::take(&mut app.view_state.status_msg);
|
||||
if !said.trim().is_empty() {
|
||||
eprintln!("{said}");
|
||||
}
|
||||
if app.last_batch_aborted() {
|
||||
exit_code = 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(e) => {
|
||||
@@ -357,7 +371,11 @@ fn run_headless_commands(cmds: &[String], file: &Option<PathBuf>) -> Result<()>
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(path) = file {
|
||||
// Writing back a document no command changed normalises it silently, which
|
||||
// turns a hand-edited fixture into a no-op — `context/code/cli.md`.
|
||||
if let Some(path) = file
|
||||
&& (changed || !path.exists())
|
||||
{
|
||||
persistence::save(&app.model_state.workbook, path)?;
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,268 @@
|
||||
//! The `improvise cmd` / `improvise script` contract, pinned at the process
|
||||
//! boundary. Why a spawned binary: `context/test-evidence/cli.md`.
|
||||
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::process::{Command, Output};
|
||||
|
||||
fn bin() -> &'static str {
|
||||
env!("CARGO_BIN_EXE_improvise")
|
||||
}
|
||||
|
||||
struct Scratch(PathBuf);
|
||||
|
||||
impl Scratch {
|
||||
fn new(tag: &str) -> Self {
|
||||
let dir = std::env::temp_dir().join(format!("improv_cli_{tag}-{}", std::process::id()));
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
std::fs::create_dir_all(&dir).expect("a scratch directory");
|
||||
Self(dir)
|
||||
}
|
||||
|
||||
fn path(&self, name: &str) -> PathBuf {
|
||||
self.0.join(name)
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for Scratch {
|
||||
fn drop(&mut self) {
|
||||
let _ = std::fs::remove_dir_all(&self.0);
|
||||
}
|
||||
}
|
||||
|
||||
fn run_cmd(args: &[&str], file: &Path) -> Output {
|
||||
Command::new(bin())
|
||||
.arg("cmd")
|
||||
.args(args)
|
||||
.arg("-f")
|
||||
.arg(file)
|
||||
.output()
|
||||
.expect("the improvise binary runs")
|
||||
}
|
||||
|
||||
fn stderr_of(out: &Output) -> String {
|
||||
String::from_utf8_lossy(&out.stderr).to_string()
|
||||
}
|
||||
|
||||
fn stdout_of(out: &Output) -> String {
|
||||
String::from_utf8_lossy(&out.stdout).to_string()
|
||||
}
|
||||
|
||||
/// A document with one category, so `add-item` has somewhere legitimate to go.
|
||||
fn seeded(scratch: &Scratch, name: &str) -> PathBuf {
|
||||
let doc = scratch.path(name);
|
||||
let out = run_cmd(&["add-category Region"], &doc);
|
||||
assert!(doc.exists(), "the seed run wrote a document: {out:?}");
|
||||
doc
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_refused_command_says_what_went_wrong() {
|
||||
let scratch = Scratch::new("refused");
|
||||
let doc = seeded(&scratch, "m.improv");
|
||||
let out = run_cmd(&["add-item Nonexistent Thing"], &doc);
|
||||
let said = stderr_of(&out);
|
||||
assert!(
|
||||
!said.trim().is_empty(),
|
||||
"a refused command said nothing at all; stdout={:?}",
|
||||
stdout_of(&out)
|
||||
);
|
||||
assert!(
|
||||
said.contains("Nonexistent"),
|
||||
"the refusal does not name the category that does not exist: {said:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// The rival this separates from: a fix that reports on the happy path and
|
||||
/// stays silent on the refusal. `context/test-evidence/cli.md`.
|
||||
#[test]
|
||||
fn a_command_with_nothing_to_say_says_nothing() {
|
||||
let scratch = Scratch::new("quiet");
|
||||
let doc = scratch.path("m.improv");
|
||||
let out = run_cmd(&["add-category Region"], &doc);
|
||||
assert!(
|
||||
stderr_of(&out).trim().is_empty(),
|
||||
"a command that succeeded with nothing to report spoke anyway: {:?}",
|
||||
stderr_of(&out)
|
||||
);
|
||||
assert!(
|
||||
out.status.success(),
|
||||
"a command that succeeded did not exit 0"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_aborted_batch_exits_non_zero_and_says_why() {
|
||||
let scratch = Scratch::new("abort");
|
||||
let doc = seeded(&scratch, "m.improv");
|
||||
let bad = scratch.path("no-such-dir").join("out.improv");
|
||||
let out = run_cmd(&[&format!("w {}", bad.display())], &doc);
|
||||
let said = stderr_of(&out);
|
||||
assert!(
|
||||
!said.trim().is_empty(),
|
||||
"a failed save said nothing: stdout={:?}",
|
||||
stdout_of(&out)
|
||||
);
|
||||
assert!(
|
||||
!out.status.success(),
|
||||
"a failed save exited 0; stderr={said:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_parse_error_still_exits_non_zero() {
|
||||
let scratch = Scratch::new("parse");
|
||||
let doc = seeded(&scratch, "m.improv");
|
||||
let out = run_cmd(&["no-such-command-at-all"], &doc);
|
||||
assert!(!out.status.success(), "an unparseable line exited 0");
|
||||
assert!(
|
||||
stderr_of(&out).contains("no-such-command-at-all"),
|
||||
"the parse error does not name the word typed: {:?}",
|
||||
stderr_of(&out)
|
||||
);
|
||||
}
|
||||
|
||||
/// 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]
|
||||
fn what_a_command_says_goes_to_stderr_and_stdout_stays_empty() {
|
||||
let scratch = Scratch::new("stream");
|
||||
let doc = seeded(&scratch, "m.improv");
|
||||
let out = run_cmd(&["add-item Nonexistent Thing"], &doc);
|
||||
assert!(
|
||||
stdout_of(&out).trim().is_empty(),
|
||||
"a diagnostic reached stdout: {:?}",
|
||||
stdout_of(&out)
|
||||
);
|
||||
assert!(!stderr_of(&out).trim().is_empty(), "stderr was empty too");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_script_reports_exactly_as_cmd_does() {
|
||||
let scratch = Scratch::new("script");
|
||||
let doc = seeded(&scratch, "m.improv");
|
||||
let script = scratch.path("s.txt");
|
||||
std::fs::write(&script, "# a comment\nadd-item Nonexistent Thing\n").expect("a script file");
|
||||
let out = Command::new(bin())
|
||||
.arg("script")
|
||||
.arg(&script)
|
||||
.arg("-f")
|
||||
.arg(&doc)
|
||||
.output()
|
||||
.expect("the improvise binary runs");
|
||||
assert!(
|
||||
stderr_of(&out).contains("Nonexistent"),
|
||||
"`script` swallowed the refusal `cmd` reports: {:?}",
|
||||
stderr_of(&out)
|
||||
);
|
||||
}
|
||||
|
||||
/// A run that changed nothing must leave the bytes alone: a fixture edited by
|
||||
/// hand between runs is otherwise silently normalised. `context/code/cli.md`.
|
||||
#[test]
|
||||
fn a_run_that_changes_nothing_does_not_rewrite_the_document() {
|
||||
let scratch = Scratch::new("noop");
|
||||
let doc = seeded(&scratch, "m.improv");
|
||||
let before = std::fs::read(&doc).expect("the seeded document");
|
||||
let out = run_cmd(&["move-cursor down"], &doc);
|
||||
let after = std::fs::read(&doc).expect("the document after a view-only command");
|
||||
assert_eq!(
|
||||
before,
|
||||
after,
|
||||
"a view-only command rewrote the document; stderr={:?}",
|
||||
stderr_of(&out)
|
||||
);
|
||||
}
|
||||
|
||||
/// The reported instance: `1e20` is expanded to its twenty digits by a run that
|
||||
/// was asked to change nothing at all.
|
||||
#[test]
|
||||
fn a_no_op_run_leaves_an_unnormalised_literal_alone() {
|
||||
let scratch = Scratch::new("literal");
|
||||
let doc = scratch.path("m.improv");
|
||||
std::fs::write(
|
||||
&doc,
|
||||
"v2025-04-09\n# Big\nInitial View: Default\n\n\
|
||||
## View: Default\nRegion: row\n_Measure: column\n\n\
|
||||
## Category: Region\n- North\n\n\
|
||||
## Category: _Measure\n- Score\n\n\
|
||||
## Data\nRegion=North, _Measure=Score = 1e20\n",
|
||||
)
|
||||
.expect("a hand-written document");
|
||||
let before = std::fs::read_to_string(&doc).expect("the document");
|
||||
assert!(before.contains("1e20"), "the fixture itself is wrong");
|
||||
let out = run_cmd(&["move-cursor down"], &doc);
|
||||
let after = std::fs::read_to_string(&doc).expect("the document after");
|
||||
assert!(
|
||||
after.contains("1e20"),
|
||||
"a no-op run normalised the literal; stderr={:?}, after={after:?}",
|
||||
stderr_of(&out)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_cmd_help_does_not_advertise_a_language_it_refuses() {
|
||||
for sub in ["cmd", "script"] {
|
||||
let out = Command::new(bin())
|
||||
.args([sub, "--help"])
|
||||
.output()
|
||||
.expect("the improvise binary runs");
|
||||
let text = format!("{}{}", stdout_of(&out), stderr_of(&out));
|
||||
assert!(
|
||||
!text.contains("JSON") && !text.contains("json"),
|
||||
"`{sub} --help` still advertises JSON, which it refuses: {text}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// The language it does accept, demonstrated end to end so the help above has
|
||||
/// something true to point at.
|
||||
#[test]
|
||||
fn the_language_the_help_names_is_the_one_it_accepts() {
|
||||
let scratch = Scratch::new("lang");
|
||||
let doc = seeded(&scratch, "m.improv");
|
||||
let out = run_cmd(&["add-item Region East"], &doc);
|
||||
assert!(out.status.success(), "the command line form was refused");
|
||||
let text = std::fs::read_to_string(&doc).expect("the document");
|
||||
assert!(text.contains("East"), "the item was not added: {text}");
|
||||
|
||||
let json = run_cmd(&[r#"{"cmd":"add-item","args":["Region","West"]}"#], &doc);
|
||||
assert!(
|
||||
!json.status.success(),
|
||||
"the JSON form the help used to advertise was accepted after all"
|
||||
);
|
||||
}
|
||||
|
||||
/// The rival this separates from: making any emitted status a non-zero exit.
|
||||
/// `fit-measure` reports its reproducing seed the same way — `context/code/cli.md`.
|
||||
#[test]
|
||||
fn a_command_that_succeeds_while_reporting_still_exits_zero() {
|
||||
let scratch = Scratch::new("reports");
|
||||
let doc = seeded(&scratch, "m.improv");
|
||||
let save = format!("w {}", doc.display());
|
||||
let out = run_cmd(&["add-item Region East", &save], &doc);
|
||||
assert!(
|
||||
!stderr_of(&out).trim().is_empty(),
|
||||
"a successful save reported nothing, so this test proves nothing"
|
||||
);
|
||||
assert!(
|
||||
out.status.success(),
|
||||
"a successful command that reported was treated as a failure: {:?}",
|
||||
stderr_of(&out)
|
||||
);
|
||||
}
|
||||
|
||||
/// A run that did change the document still writes it, even when the last thing
|
||||
/// it did was save somewhere else and clear the dirty flag.
|
||||
#[test]
|
||||
fn a_run_that_changed_the_document_still_writes_it() {
|
||||
let scratch = Scratch::new("elsewhere");
|
||||
let doc = seeded(&scratch, "m.improv");
|
||||
let other = scratch.path("other.improv");
|
||||
let save = format!("w {}", other.display());
|
||||
run_cmd(&["add-item Region East", &save], &doc);
|
||||
let text = std::fs::read_to_string(&doc).expect("the -f document");
|
||||
assert!(
|
||||
text.contains("East"),
|
||||
"the -f document lost an edit because `:w` elsewhere cleared the flag: {text}"
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user