64bcb821da
`improvise cmd` and `improvise script` resolved every command through
`CmdRegistry::parse` — the pure arm, which builds `CursorState::default()`: row
0, col 0, and a grid of zero rows and zero columns. The interactive arm, which
builds `CursorState::from_ctx(ctx)`, was never reached from a script. So every
`Move` and `enter-advance` snapshotted a zeroed cursor at PARSE time, clamped to
(0, 0) and emitted `SetSelected(0, 0)` — worse than a no-op, since it RESETS the
cursor. Nothing could position it, so every cursor-reading command was pinned to
the origin.
Reproduced at a9cad15 on a document with Region on row carrying North, South,
East: `move-selection 2 0 . hide-selected-row-item` and a bare
`hide-selected-row-item` both wrote `hidden: Region/North`. After the fix the
first writes `hidden: Region/East`.
The repair is `parse::segments` + `parse::parse_segment_interactive`, with the
headless loop re-parsing per segment, because command n must see command n-1's
cursor. Line atomicity is preserved by keeping the existing whole-line
`parse_line` as a syntax gate, so a line that parsed before still parses and the
only behavioural change is which cursor the commands see.
Two things the ticket did not predict. The TUI's `:` line had the identical
defect — `ExecuteCommand` also used the pure arm — and is fixed the same way.
And doing that surfaced a second instance of the same shape one level down:
`register_pure` built an interactive arm that refused EMPTY args while its own
pure arm accepted them, so `w` with no path parsed one way and refused the
other, breaking `improvise cmd 'w'` and `:w` the moment either re-parsed
interactively. Both arms are now `parse`.
Evidence: context/test-evidence/cli.md; reasoning: context/code/cli.md section 7.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
320 lines
11 KiB
Rust
320 lines
11 KiB
Rust
//! 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}"
|
|
);
|
|
}
|
|
|
|
/// A document whose row axis carries three items, so a cursor move has
|
|
/// somewhere to go and the three landing places are distinguishable.
|
|
fn three_rows(scratch: &Scratch, name: &str) -> PathBuf {
|
|
let doc = scratch.path(name);
|
|
run_cmd(&["add-category Region"], &doc);
|
|
run_cmd(&["add-items Region North South East"], &doc);
|
|
doc
|
|
}
|
|
|
|
fn hidden_line(doc: &Path) -> String {
|
|
std::fs::read_to_string(doc)
|
|
.expect("the document")
|
|
.lines()
|
|
.find(|l| l.starts_with("hidden:"))
|
|
.unwrap_or("(nothing hidden)")
|
|
.to_string()
|
|
}
|
|
|
|
/// Evidence: `context/test-evidence/cli.md`.
|
|
#[test]
|
|
fn a_script_can_move_the_cursor_and_the_next_command_sees_it() {
|
|
let scratch = Scratch::new("cursor");
|
|
let moved = three_rows(&scratch, "moved.improv");
|
|
let still = three_rows(&scratch, "still.improv");
|
|
|
|
run_cmd(&["move-selection 2 0 . hide-selected-row-item"], &moved);
|
|
run_cmd(&["hide-selected-row-item"], &still);
|
|
|
|
assert_eq!(
|
|
hidden_line(&still),
|
|
"hidden: Region/North",
|
|
"without a move the cursor is at the origin; if this changes, the other \
|
|
half of this test stops meaning anything"
|
|
);
|
|
assert_eq!(
|
|
hidden_line(&moved),
|
|
"hidden: Region/East",
|
|
"`move-selection 2 0` must move the cursor two rows down before the \
|
|
next command reads it (improvise-6uva)"
|
|
);
|
|
}
|
|
|
|
/// Evidence: `context/test-evidence/cli.md`.
|
|
#[test]
|
|
fn a_script_cursor_move_survives_the_line_that_made_it() {
|
|
let scratch = Scratch::new("cursor-lines");
|
|
let doc = three_rows(&scratch, "lines.improv");
|
|
run_cmd(&["move-selection 1 0", "hide-selected-row-item"], &doc);
|
|
assert_eq!(hidden_line(&doc), "hidden: Region/South");
|
|
}
|