diff --git a/CHANGELOG.md b/CHANGELOG.md index ba6a5b51..1810e698 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,76 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.9.0] - 2026-08-22 + +### Changed + +- **`--rm` is docker's `--rm`: the workspace goes when the session ends.** + `dl --rm` and `dl --rm -- ` hand over a session and delete the + workspace and its clone once it ends — which is what `--autorm` did, under the + name docker gives it. The `rm` verb is unchanged and is the only way to delete a + workspace *now*, so the whole grammar is `docker rm` beside `docker run --rm`, + and neither spelling has to be read twice to work out which was meant: + + ```bash + dl kinisi/repo@fix/x rm # delete it now + dl kinisi/repo@fix/x --rm # shell; it goes when you exit + dl kinisi/repo@fix/x --rm -- make test # one command, then it goes + aid kinisi/repo@fix/x 'fix the flaky test' --rm # the agent runs, then it goes + ``` + + Everything the removal already promised is unchanged: it stops at work that is + nowhere else and leaves the workspace standing, it collects a build that died in + `postCreateCommand`, and the exit code is the session's and never the removal's. + `--force` still does not compose with it — that is `dl rm --force`, which is + where docker keeps its `-f` too. + +### Removed + +- **`--autorm` is now spelled `--rm`.** A rename and nothing else; the behaviour + above is what it always did. The old spelling is recognised and refused with the + new one rather than dropped, so a line recalled from history says what happened + instead of quietly doing nothing: + + ``` + $ dl --autorm + --autorm is now spelled --rm: 'dl --rm' opens the workspace and deletes + it when the session ends, the way 'docker run --rm' does. Use 'dl rm' to + delete one now. + ``` + +- **`--stop` is retired, and so is appending `--rm` to cancel a line.** Both were + the *suffix* form of a verb: typed at the end of a line that already asked for + something and winning over it, so `aid 'review this pr' --rm` deleted the + workspace and printed `--rm overrode the rest of the line`. That shape cannot + survive a `--rm` that means "delete when the session ends" — the two spellings + look like a pair, and one cancelling the line while the other runs it is exactly + the pair nobody can keep straight. So `aid 'review this pr' --rm` now runs + the review and deletes afterwards, and `--stop` refuses with the word to use: + + ``` + $ dl --stop + --stop is no longer a flag: the flag spellings now modify a session (--rm deletes + the workspace once one ends) rather than name a verb. Use 'dl stop' to + stop a workspace. + ``` + + For "I am done with this workspace", `dl rm` names it and `dl rm` picks it — + and the pick marks several with TAB, which for a long `aid` prompt line is fewer + keystrokes than recalling it to type at the end. What is genuinely gone is + deleting a workspace without naming or picking it, by appending to whatever the + last line happened to be. + + One consequence worth knowing: `dl prune --rm` used to remove ``, and + is now the `prune` retirement's refusal, since nothing overrides a line any more. + Add `--force` to that line and the `--force`-beside-`--rm` refusal is what you + get, because the pair is the more confused half and is named first. + `dl --prune` is unchanged. + +- **`dl rm --rm` is refused as the two requests it is**, rather than being + quietly treated as one of them. The sentence names the verb, which is the + spelling that already does what such a line most likely meant. + ## [0.8.0] - 2026-08-22 ### Added diff --git a/README.md b/README.md index 255be25d..9ec50d47 100644 --- a/README.md +++ b/README.md @@ -134,79 +134,61 @@ transport, which has no terminal; `dl restart` republishes the alias. Set | `dl reset` | Clean slate (remove all, recreate) | | `dl dotfiles` | Refresh dotfiles in the running workspace (`chezmoi update`) | | `dl -- ` | Run shell command in workspace (with a terminal, when `dl` has one) | -| `dl --autorm` | Attach, and [delete the workspace when the session ends](#--autorm-the-throwaway-workspace) | +| `dl --rm` | Attach, and [delete the workspace when the session ends](#--rm-the-throwaway-workspace) | Every verb in that table also takes the workspace second — `dl stop ` — and with no workspace at all it opens the selector and applies itself to what you pick — everything you pick, -for the verbs the selector lets TAB mark several of. `stop` and `rm` answer to -`--stop` and `--rm` as well, since the flag spellings were documented long before they worked. +for the verbs the selector lets TAB mark several of. -### `--stop` and `--rm` can be appended to a line that says something else +`rm` is a word and `--rm` is a flag, and they are two different requests — docker's +split, described next. `--stop` is not a spelling of anything: it was the flag form of +the `stop` verb and is [retired](#--stop-and---autorm-are-retired). -The flag spellings do one thing the words cannot: they may be typed at the *end* of a -line that already asked for something, and they win over it. +### `--rm`: the throwaway workspace -```bash -aid kinisi/repo@fix/x 'review this pr' # work happens -aid kinisi/repo@fix/x 'review this pr' --rm --force # ↑, with `--rm --force` appended -``` - -Both `dl` and `aid` accept it, and a leading verb word on the recalled line is not -mistaken for the workspace, so `dl prune --force` recalled with `--rm` appended -still removes ``. This exists because a shell makes appending to the previous line -cheap and rewriting the front of it expensive — deleting the workspace you were just -working in should not cost an edit in the middle of a long prompt. - -What the suffix beat is named on stderr before anything is removed: - -``` ---rm overrode the rest of the line: 'review this pr' was not acted on. -``` - -That notice is the price of the convenience and is not optional: the line is now -allowed to carry an instruction it will not carry out, so a deliberate `--rm` and a -slip have to be told apart. Note that a `--` command tail cannot be overridden this -way — everything after `--` belongs to the workspace's command, so a `--rm` typed -there is an argument to that command and not a verb. - -### `--autorm`: the throwaway workspace - -`--autorm` deletes the workspace once the session ends, the way `docker run --rm` does. -It applies to the two forms that hand a session over and come back from it: +`--rm` deletes the workspace once the session ends, the way `docker run --rm` does. It +applies to the two forms that hand a session over and come back from it: ```bash -dl kinisi/repo@fix/x --autorm # shell; the workspace goes when you exit -dl kinisi/repo@fix/x --autorm -- make test # one command, then the workspace goes -aid kinisi/repo@fix/x 'fix the flaky test' --autorm +dl kinisi/repo@fix/x --rm # shell; the workspace goes when you exit +dl kinisi/repo@fix/x --rm -- make test # one command, then the workspace goes +aid kinisi/repo@fix/x 'fix the flaky test' --rm ``` +**The word and the flag are docker's two commands, not two spellings of one.** +`docker rm` deletes a container now; `docker run --rm` deletes one when what it ran +has finished; and no docker subcommand takes a `--rm` meaning the first of those. Here +too: `dl rm` deletes now, `dl --rm` deletes after, and neither has to be read +twice to work out which was meant. `--force` follows docker as well — it is `dl rm +--force`'s, never `--rm`'s. + **It stops at work that is nowhere else.** The removal is `dl rm`'s, guard included, so a clone holding uncommitted or unpushed work — or one git could not read to find out — refuses, says which, and leaves the workspace standing: ``` ---autorm: the session has ended, removing kinisi/repo@fix/x. +--rm: the session has ended, removing kinisi/repo@fix/x. kinisi-repo-fix-x-1a2b holds 1 uncommitted change(s) (scratch.txt). Push or commit it, or run: dl kinisi/repo@fix/x rm --force ``` That is what makes it safe to leave on a line you recall: the flag never decides that your work was disposable. For the same reason `--force` does not compose with it — a -`--force` habitually appended to a recalled `--autorm` line would destroy work hours +`--force` habitually appended to a recalled `--rm` line would destroy work hours later, unattended, with nobody reading the sentence explaining it. Run `dl rm --force` when that is what you mean. **A build that failed is collected too.** The removal runs whenever the launch got as far as asking devpod for the workspace — including when `devpod up` died in `postCreateCommand`, which leaves the container *running* and the clone cut. That is -the case an unattended `dl owner/repo --autorm -- make test` in CI most needs covered. +the case an unattended `dl owner/repo --rm -- make test` in CI most needs covered. A launch that stopped earlier — an unknown workspace, a branch that could not be named, a devpod that would not run — created nothing, so nothing is removed and nothing is said about it. Three more things it does not promise: -- **The exit code is the launch's.** `dl repo --autorm -- make test` exits with the +- **The exit code is the launch's.** `dl repo --rm -- make test` exits with the test's status, and a failed build exits with devpod's; a removal that refused is never what the code reports. The refusal is on stderr and the workspace is still there. @@ -214,12 +196,16 @@ Three more things it does not promise: of the gaps. See "How you exit decides whether it fires" below. - **It does not know about your other shells.** Nothing serialises two sessions on one workspace — the launch lock covers the build, not the session — so a second - `dl ` in another terminal is attached to the same container, and the `--autorm` - run exiting first removes it from under that one. Use `--autorm` for the workspace + `dl ` in another terminal is attached to the same container, and the `--rm` + run exiting first removes it from under that one. Use `--rm` for the workspace you opened to throw away, not for one you may already be sitting in elsewhere. -On an `aid` line it is appendable like `--rm`, and unlike `--rm` it **keeps the prompt** — -the agent still runs, and the workspace goes when it is done. +On an `aid` line it is **appendable**, and it keeps the prompt: recall the line, type +`--rm` at the end, and the agent still runs — the workspace goes when it is done. That +is the shape a shell makes cheap, appending to the previous line rather than editing +the front of it. Note that a `--` command tail is not appendable this way: everything +after `--` belongs to the workspace's command, so a `--rm` typed there is an argument +to that command. ### How you exit decides whether it fires @@ -230,7 +216,7 @@ the session or kills `dl`. a pty — a bare `dl ` runs `devpod ssh `, and `dl -- ` on a terminal runs `ssh -t` — which puts your local terminal in raw mode and clears `ISIG`. Ctrl-C is then a byte travelling to the remote pty, not a signal to `dl`: the program *inside* the -container gets the interrupt. So `aid repo 'fix it' --autorm` and Ctrl-C twice to leave +container gets the interrupt. So `aid repo 'fix it' --rm` and Ctrl-C twice to leave Claude Code ends the remote command, ends the session, and the workspace goes. In an interactive shell Ctrl-C just hands you a fresh prompt — `exit` or Ctrl-D is what ends that session, and either fires the removal. @@ -241,17 +227,55 @@ removal (a signal handler may not allocate or lock, and this one `_exit`s): - Ctrl-C during the clone or the container build, before any pty exists. - Closing the terminal window — SIGHUP, which `dl` does not handle at all. -One-line check for your own setup: start `dl --autorm` and press Ctrl-C once. A +One-line check for your own setup: start `dl --rm` and press Ctrl-C once. A fresh prompt *inside* the container means Ctrl-C is being forwarded and the removal will fire when you leave. Landing back on the host means it reached `dl`, and it will not. Those two forms and no others. Every verb word refuses the flag rather than ignoring it, and `code` is the one worth knowing about: it returns while VS Code is still connecting, -so honouring `--autorm` there would delete the container out from under a window that is +so honouring `--rm` there would delete the container out from under a window that is still opening. `restart`, `recreate` and `reset` do end in a session and would work, but -they are out too, because `--autorm` is the throwaway workspace and not a cleanup modifier +they are out too, because `--rm` is the throwaway workspace and not a cleanup modifier on every verb that ends in a shell. +### `--stop` and `--autorm` are retired + +Both moved because `--rm` changed meaning, and both are still recognised so that a +line recalled from history says what happened instead of quietly doing something else: + +``` +$ dl --autorm +--autorm is now spelled --rm: 'dl --rm' opens the workspace and deletes it +when the session ends, the way 'docker run --rm' does. Use 'dl rm' to +delete one now. + +$ dl --stop +--stop is no longer a flag: the flag spellings now modify a session (--rm deletes the +workspace once one ends) rather than name a verb. Use 'dl stop' to stop a +workspace. +``` + +`--autorm` is a rename and nothing else: the behaviour above is what it always did. + +`--stop` is a genuine withdrawal, and so is the thing `--rm` used to do. Both were the +*suffix* form of a verb — appended to a line that already asked for something, and +winning over it, so that `aid 'review this pr' --rm` deleted the workspace and +printed `--rm overrode the rest of the line`. That shape cannot survive `--rm` meaning +"delete when the session ends": the two spellings look alike, and one cancelling the +line while the other runs it is the one pair a person cannot keep straight. + +What replaces it, for "I am done with this workspace": + +```bash +dl rm # the workspace named +dl rm # or pick it — TAB marks several, and rm takes each in turn +``` + +For a long `aid` prompt line that is the cheaper edit anyway: `dl rm` and a pick +beats recalling the line to type at the end of it. What is genuinely gone is deleting +a workspace *without naming or picking it*, by appending to whatever the last line +happened to be. + ### `prune` is no longer a spelling of the `rm` verb `dl prune` used to delete one workspace and `dl --prune` removes clone @@ -267,10 +291,10 @@ $ dl prune or 'dl --prune' to remove the clone directories no workspace opens any more. ``` -`dl --prune` is unchanged. The word is still *recognised* rather than forgotten, so -it is never read as a workspace name: a `dl prune --force` line recalled with -`--rm` appended still removes ``, and a workspace that really is called `prune` -is still reachable as `dl stop prune`. Use `dl rm` from now on. +`dl --prune` is unchanged. The word is still *recognised* rather than forgotten, so it +is never read as a workspace name — `dl prune ` says what moved instead of +reporting an unknown workspace called `prune` — and a workspace that really is called +`prune` is still reachable as `dl stop prune`. Use `dl rm` from now on. ## Global Commands @@ -350,8 +374,7 @@ are unaffected, and `dl -- claude` still runs exactly what you typed. |--------|-------------| | `--claude`, `--codex`, `--gemini` | Pick the agent (default: `claude`) | | `--devcontainer ` | Passed through to `dl` | -| `--rm`, `--stop` | Do that to the workspace instead of starting an agent; appendable to a recalled line, `--force` with `--rm` deletes despite unsaved work | -| `--autorm` | Run the agent, then delete the workspace when the session ends | +| `--rm` | Run the agent, then [delete the workspace when the session ends](#--rm-the-throwaway-workspace). Appendable to a recalled line, prompt and all. To delete one *now* instead, that is `dl rm` | | `DEVLAUNCH_AID_AGENT=` | Change the default agent | | `DEVLAUNCH_NO_TTY=1` | No prompt question, no pty: the old one-shot behaviour | | `DEVLAUNCH_NO_HERDR=1` | Do not report agent state to a host-side [herdr](#agent-state-in-a-herdr-session) session | @@ -1052,7 +1075,7 @@ Removing a workspace removes three things: the devpod workspace, the local clone (unless it holds work that exists nowhere else — see [cleaning up workspaces](#cleaning-up-workspaces)), and **the named Docker volumes that workspace's devcontainer created**. Every path that removes a workspace does all three: -`dl rm`, the appended `--rm`, `--autorm`, and `--purge`. +`dl rm`, `dl --rm`, and `--purge`. Two volumes per workspace, both named from what devpod recorded substituting into the devcontainer: diff --git a/docs/rust-rewrite-plan.md b/docs/rust-rewrite-plan.md index b0326c71..78cec9be 100644 --- a/docs/rust-rewrite-plan.md +++ b/docs/rust-rewrite-plan.md @@ -175,7 +175,7 @@ Nothing diverges silently. | 12 | Non-UTF-8 clone paths render lossily (U+FFFD) where Python round-tripped the bytes via surrogateescape. | Rust has no surrogateescape; only undecodable paths differ. | | 13 | The primary failure survives compound failures: a failed clone reports its git reason even when the debris cleanup also fails (Python raised the cleanup `OSError`, masking the clone's), and a migration that cannot write its orphan/unmigrated listing files says so in the migration report instead of aborting. | #251 §5 error shaping — no cause is masked. | | 14 | CLI usage errors (unknown flags, malformed combinations clap itself rejects) exit 2, clap's convention, where Python exited 1. Refusals Python also made keep exit 1 and Python's wording. Inputs: `dl --nope`; `dl myws echo hi` (three positional words); `dl -leading-dash` (Python probed it as a workspace name, one `devpod status` round trip); `dl --ls --ls` (Python's argparse accepted the repeat). | Generated parser owns its usage errors (companion to rows 2–3). | -| 15 | Grammar completions and tightenings: `--stop`/`--rm` flag spellings work as verbs (dl.py's docstring promised them; its dispatch parsed them as a spec and failed); `--json`/`--size` require `--ls`; a global command refuses a stray target, a meaningless `-y`/`--force`, or `--devcontainer`; a trailing `-- ` beside a non-attach verb is refused; two global commands together (`dl --refresh --ls`, `dl --repos --ls`) are refused where Python ran whichever argparse happened to check first; no-target verb forms (`dl --stop`, `dl -- `) open the selector; the `Ignoring --devcontainer: it does not apply` diagnostic names the resolved verb (`rm` for the `prune` spelling). Python silently discarded all of these. | Documented grammar made real; silent discards made refusals (rows 1–2's aim). | +| 15 | Grammar completions and tightenings: `--stop`/`--rm` flag spellings work as verbs (dl.py's docstring promised them; its dispatch parsed them as a spec and failed); `--json`/`--size` require `--ls`; a global command refuses a stray target, a meaningless `-y`/`--force`, or `--devcontainer`; a trailing `-- ` beside a non-attach verb is refused; two global commands together (`dl --refresh --ls`, `dl --repos --ls`) are refused where Python ran whichever argparse happened to check first; no-target verb forms (`dl --stop`, `dl -- `) open the selector; the `Ignoring --devcontainer: it does not apply` diagnostic names the resolved verb (`rm` for the `prune` spelling). Python silently discarded all of these. | Documented grammar made real; silent discards made refusals (rows 1–2's aim). **The `--stop`/`--rm` half is superseded by row 32**: `--rm` is now docker's session flag and `--stop` is retired, both refused by name. Everything else in this row stands. | | 16 | `--version` always prints the bare version; Python appended `(dev, editable from )` for editable installs. A compiled binary has no editable-install metadata. | PEP 610 provenance has no Rust analogue; released builds printed identically anyway. | | 17 | Provisioning never kills a launch devpod already ran: an unparsable probe stage line (`failed ²`, a bignum status) reads as not-reached where Python raised inside `stage_outcomes`; a trampoline sidecar with a non-string `exe` lends nothing where Python's `Path(2)` raised `TypeError`; a host with no home directory still runs the setup pass and network install where Python's `Path.home()` raised after `devpod up`. | Typed reads at the probe/sidecar boundary (rows 8/11's aim); every arm ends in the network-install fallback Python promised. | | 18 | The host-tools bundle streams as plain ustar, not Python's PAX default; an archive member name ≥100 bytes is refused (`NameTooLong`) and provisioning falls through to the network install, where Python wrote a PAX long-name header. | Hand-rolled writer (no tar dependency); `tar xf -` observes identical members for every real payload; the fallback is the path Python took for any bundle failure. | @@ -191,9 +191,11 @@ Nothing diverges silently. | 28 | The `metadata.json` reader takes strict JSON where Python's `json.loads` took its extensions: `NaN`/`Infinity` literals, numbers beyond f64 range (`1e400`, which Python reads as `inf`), and lone-surrogate `\uD800` escapes all read as corruption, so a file carrying one is quarantined to `metadata.json.corrupt` (bytes intact, single slot) and the run starts with an empty cache where Python loaded the records around the value. | serde_json cannot represent a lone surrogate in a Rust `String` and f64 cannot hold `1e400`, so accepting them would mean values the store could not round-trip; no build of dl (Python or Rust) ever writes any of them — only a hand-edited or third-party-written file can carry one — and the quarantine keeps the bytes recoverable. Pinned by `pythons_json_extensions_read_as_corruption_here` in `domain/metadata.rs`. | | 29 | A clone the host refused as not-found gets a second line naming the same repository under an owner the completion cache knows: `Did you mean 'kinisi-robotics/kinisi_ros'? …`. Python printed git's stderr and stopped. The line is added only for a host's own not-found wording (GitHub, GitLab, Bitbucket), only from a clone step, and only when the cache holds the same repo name under another owner *and* not under the owner typed; at most three candidates are named and any remainder is counted. The git text above it is unchanged in every case. | A mistyped owner and a revoked permission produce the same six lines from git, so git's words cannot tell them apart — and devlaunch already holds the answer in the list its own shell completion offers. Additive: nothing is removed or reworded, and a machine with no cache entry sees exactly what it saw before. The suggestion names a *spec*, not a `dl …` command line, because `aid` renders through the same function and its own invocation carries an agent prompt. Pinned by the `wrong_owner_hint` tests in `dl/tests/launch.rs` and `dl/src/render.rs`. | -| 30 | `--stop` and `--rm` are **appendable**: on a line that already said something else they win, and the words they displace are reported rather than refused. `dl 'review this pr' --rm` was `Unknown command 'review this pr'` (exit 1) in both Python and earlier Rust; `dl prune --force --rm` took `prune` as the *workspace* and reported it unknown. A leading verb word is now skipped when finding the target, a word spelling the flag's own verb is absorbed silently, and everything else displaced is named on stderr (`--rm overrode the rest of the line: 'review this pr' was not acted on.`) before anything is removed. `aid` peels the same two flags — plus a `--force` in their company — off the end of its argv before the prompt join, so `aid --rm --force` removes the workspace and hands no agent command to dl; a verb flag before the spec is collected the same way rather than passed through beside a `--` tail dl would refuse. The peel is bounded to the exact words at the very end of the line, so a prompt that mentions `--rm` or ends in a bare `--force` is untouched, and a `--` command tail cannot be overridden at all. | A shell makes appending to a recalled line cheap and rewriting its front expensive, so "and now delete it" has to be spellable as a suffix. The notice is what makes it safe rather than a silent discard: the line may now carry an instruction it will not carry out, so a deliberate suffix and a slip have to be distinguishable. Row 15 made the flag spellings work; this makes them the one form that can be typed last. Pinned by the `suffix_verb` tests in `dl/src/cli.rs` and `aid/src/rewrite.rs`, and by the `--aid -- --help` allowance in `rust/tools/parity_cases.txt` — aid's help text is hand-written and was byte-compared against Python's, so documenting a flag Python's aid has not got is where this row becomes visible to the parity harness. | +| 30 | `--stop` and `--rm` are **appendable**: on a line that already said something else they win, and the words they displace are reported rather than refused. `dl 'review this pr' --rm` was `Unknown command 'review this pr'` (exit 1) in both Python and earlier Rust; `dl prune --force --rm` took `prune` as the *workspace* and reported it unknown. A leading verb word is now skipped when finding the target, a word spelling the flag's own verb is absorbed silently, and everything else displaced is named on stderr (`--rm overrode the rest of the line: 'review this pr' was not acted on.`) before anything is removed. `aid` peels the same two flags — plus a `--force` in their company — off the end of its argv before the prompt join, so `aid --rm --force` removes the workspace and hands no agent command to dl; a verb flag before the spec is collected the same way rather than passed through beside a `--` tail dl would refuse. The peel is bounded to the exact words at the very end of the line, so a prompt that mentions `--rm` or ends in a bare `--force` is untouched, and a `--` command tail cannot be overridden at all. | **Superseded by row 32**, which takes both spellings back: `--rm` became docker's `--rm` and `--stop` was retired, so no flag overrides a line any more. A shell makes appending to a recalled line cheap and rewriting its front expensive, so "and now delete it" has to be spellable as a suffix. The notice is what makes it safe rather than a silent discard: the line may now carry an instruction it will not carry out, so a deliberate suffix and a slip have to be distinguishable. Row 15 made the flag spellings work; this makes them the one form that can be typed last. Pinned by the `suffix_verb` tests in `dl/src/cli.rs` and `aid/src/rewrite.rs`, and by the `--aid -- --help` allowance in `rust/tools/parity_cases.txt` — aid's help text is hand-written and was byte-compared against Python's, so documenting a flag Python's aid has not got is where this row becomes visible to the parity harness. | -| 31 | `prune` is retired as a workspace verb. `dl prune` / `dl prune ` / `dl prune` were Python's second spelling of `rm` and now refuse with `'prune' is no longer a workspace verb. Use 'dl rm' to delete a workspace, or 'dl --prune' to remove the clone directories no workspace opens any more.` (exit 1). The word stays in a `RETIRED` table rather than being dropped, so it is still never read as a workspace name: `dl prune --force --rm` removes `` (row 30's suffix form, with `prune` absorbed as the same verb the flag asks for), and a live verb beside it still wins from either position, so `dl stop prune` stops a workspace named `prune` as before. `dl --prune` is unchanged. Row 15's parenthetical about the `Ignoring --devcontainer` diagnostic naming `rm` for the `prune` spelling is now unreachable. | One word meant two unrelated commands, separated only by two dashes: `dl prune` deleted a workspace and `dl --prune` deletes clone directories and no workspace. The two failure modes were losing a workspace meant to be kept, and being refused by `--prune takes no workspace` for a reason that sentence cannot explain. `rm` has no such twin, and the retirement is a refusal that names both meanings rather than a silent removal. Pinned by the `prune_is_refused_as_a_verb_from_either_position` and `a_retired_word_is_never_read_as_the_workspace` tests in `dl/src/cli.rs`. | +| 31 | `prune` is retired as a workspace verb. `dl prune` / `dl prune ` / `dl prune` were Python's second spelling of `rm` and now refuse with `'prune' is no longer a workspace verb. Use 'dl rm' to delete a workspace, or 'dl --prune' to remove the clone directories no workspace opens any more.` (exit 1). The word stays in a `RETIRED` table rather than being dropped, so it is still never read as a workspace name: `dl prune --force --rm` removes `` (row 30's suffix form, with `prune` absorbed as the same verb the flag asks for), and a live verb beside it still wins from either position, so `dl stop prune` stops a workspace named `prune` as before. `dl --prune` is unchanged. Row 15's parenthetical about the `Ignoring --devcontainer` diagnostic naming `rm` for the `prune` spelling is now unreachable. | **Superseded in part by row 32**, which retires row 30's suffix form: `dl prune --rm` is now this row's refusal rather than a removal of ``, and the word is still in `RETIRED` and still never read as a workspace name. One word meant two unrelated commands, separated only by two dashes: `dl prune` deleted a workspace and `dl --prune` deletes clone directories and no workspace. The two failure modes were losing a workspace meant to be kept, and being refused by `--prune takes no workspace` for a reason that sentence cannot explain. `rm` has no such twin, and the retirement is a refusal that names both meanings rather than a silent removal. Pinned by the `prune_is_refused_as_a_verb_from_either_position`, `a_retired_word_is_never_read_as_the_workspace` and `a_recalled_prune_line_is_the_retirement_and_the_pair_is_named_ahead_of_it` tests in `dl/src/cli.rs`. | + +| 32 | **`--rm` is docker's `--rm`, and the flag-spelled verbs are retired.** `dl --rm` and `dl --rm -- ` now hand over a session and delete the workspace once it ends — what row 30's `--autorm` did, under the name docker gives it. The word `rm` is unchanged and is the only way to delete one *now*, so `docker rm` / `docker run --rm` is the whole of the grammar and no spelling has to be read twice. Three withdrawals pay for it, each refused at exit 1 rather than reinterpreted: `--autorm` (`--autorm is now spelled --rm: …`); `--stop`, whose only reason to exist was being a flag (rows 15 and 30) and which cannot stay a *cancelling* suffix beside a `--rm` that runs the line; and row 30's suffix override itself, so `aid 'review this pr' --rm` now runs the review and deletes afterwards where it used to delete instead, and `dl prune --rm` is row 31's refusal rather than a removal (with `--force` also on the line it is the `--force`-beside-`--rm` refusal instead, named first because the pair is the more confused half). `--force` still does not compose with `--rm` — docker keeps `-f` on `rm` too — and `dl rm --rm` is refused as the two requests it is. Retired flags are answered *before* anything else the line got wrong, since both moved on account of `--rm`'s new meaning. aid peels the retired spellings so dl refuses them by name instead of joining them into a prompt, and builds no agent command for such a line, so nothing is booted on the way to exit 1. `Overridden`, `pick_target` and the `--rm overrode the rest of the line` notice are gone with the override. | Row 30 bought "and now delete it" as a suffix and paid for it with a flag whose meaning was unguessable from its spelling: `--rm` cancelled the line, `--autorm` ran it, and the two looked like a pair. docker had already split the same problem the other way — a verb for now, a `run` flag for after — and taking that split makes the common line (`aid --rm`: send the agent in, get the disk back) the one the short spelling names, at the cost of the rarer one, which `dl rm` and a pick does in fewer keystrokes than recalling a long prompt to append to it. The withdrawals are recognised rather than deleted, for row 31's reason: a flag dropped from `Cli` is clap's `unexpected argument` at exit 2, naming the spelling and not the replacement — and the spelling is exactly what cannot explain a line that stopped working because a *different* flag changed meaning. Pinned by the `retired_flag` / `rm_` tests in `dl/src/cli.rs`, `the_retired_flag_spellings_name_the_words_that_replaced_them` and `autorm_is_refused_with_the_spelling_that_replaced_it` in `dl/tests/lifecycle.rs`, the `rm_on_exit_` tests in `dl/tests/launch.rs`, and `a_retired_spelling_starts_no_agent_and_is_handed_to_dl_to_refuse` in `aid/src/rewrite.rs`. | Additions require a PR that updates this table; the row number is cited by any per-binary harness branch. diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 9c5f054a..b6d90d15 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -13,7 +13,7 @@ dependencies = [ [[package]] name = "aid" -version = "0.8.0" +version = "0.9.0" dependencies = [ "devlaunch-test-support", "dl", @@ -452,7 +452,7 @@ dependencies = [ [[package]] name = "devlaunch-core" -version = "0.8.0" +version = "0.9.0" dependencies = [ "devlaunch-runner", "devlaunch-test-support", @@ -470,7 +470,7 @@ dependencies = [ [[package]] name = "devlaunch-runner" -version = "0.8.0" +version = "0.9.0" dependencies = [ "libc", "tempfile", @@ -478,7 +478,7 @@ dependencies = [ [[package]] name = "devlaunch-test-support" -version = "0.8.0" +version = "0.9.0" dependencies = [ "devlaunch-runner", "serde_json", @@ -525,7 +525,7 @@ dependencies = [ [[package]] name = "dl" -version = "0.8.0" +version = "0.9.0" dependencies = [ "assert_cmd", "clap", diff --git a/rust/Cargo.toml b/rust/Cargo.toml index 3cdd4e84..b1820759 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -11,7 +11,7 @@ members = [ # The single source of the version (docs/rust-rewrite-plan.md: cutover ships # 0.1.0, version read from Cargo.toml). [workspace.package] -version = "0.8.0" +version = "0.9.0" edition = "2024" license = "MIT" repository = "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/blooop/devlaunch" diff --git a/rust/aid/src/main.rs b/rust/aid/src/main.rs index 8c78f162..234e7e9a 100644 --- a/rust/aid/src/main.rs +++ b/rust/aid/src/main.rs @@ -118,21 +118,6 @@ fn run(argv: &[String]) -> i32 { ); return 1; }; - // A `--rm`/`--stop` appended to a recalled line beat a prompt, and the prompt is - // the thing aid would otherwise have handed an agent — so aid, not dl, is the one - // that swallowed it and the one that owes the sentence. dl's wording, through - // dl, for the same reason `python_repr` is: one sentence, not two spellings of - // one. Printed before the `aid -> dl` line so the reason to reach for Ctrl-C - // comes before the command that needs interrupting. - if let (rewrite::Task::Verb { overridden, .. }, Some(flag)) = - (&parsed.task, parsed.task.verb_flag()) - && !overridden.is_empty() - { - eprintln!( - "{}", - dl::overridden_notice(flag, std::slice::from_ref(overridden)) - ); - } // Python's `logging.info("aid -> dl %s", shlex.join(dl_args))`, which lands on // stderr as the bare message. It is how a person sees what aid actually asked // for, and the quoting is what makes it a line they can paste. @@ -210,15 +195,14 @@ Options: {agents} Pick the agent (default: {default}) --devcontainer Passed through to dl - --rm, --stop Do that to the workspace instead of starting - an agent. Appendable: recall the line and type - it at the end, prompt and all. Add --force to - --rm to delete despite unsaved work. - --autorm Delete the workspace once the agent's session - ends. Also appendable, and unlike --rm it keeps - the prompt: the agent runs, then the workspace - goes. Stops at work that is nowhere else and - says so, leaving the workspace standing. + --rm Delete the workspace once the agent's session + ends, the way docker run --rm does. Appendable: + recall the line and type it at the end, prompt + and all — the agent still runs, and the workspace + goes when it is done. Stops at work that is + nowhere else and says so, leaving the workspace + standing. To delete one now instead, that is + dl's rm verb: dl rm. --help, -h Show this help --version Show version @@ -229,12 +213,10 @@ Examples: aid blooop/devlaunch # Start {default} in the workspace aid blooop/devlaunch@fix/42 fix the bug # Open the branch, hand over the prompt aid --gemini ./my-project explain this # Pick a different agent - aid blooop/devlaunch@fix/42 fix the bug --rm --force + aid blooop/devlaunch@fix/42 fix the bug --rm # The line above, recalled, with the - # workspace deleted instead - aid blooop/devlaunch@fix/42 fix the bug --autorm - # Same line, but the agent still runs - # and the workspace goes afterwards + # workspace deleted once the agent is + # done with it Everything else — listing, stopping, deleting, VS Code — is dl's job: dl --help\n\n" diff --git a/rust/aid/src/rewrite.rs b/rust/aid/src/rewrite.rs index 4c901520..3297de79 100644 --- a/rust/aid/src/rewrite.rs +++ b/rust/aid/src/rewrite.rs @@ -111,17 +111,7 @@ const AGENTS: &[(&str, Agent)] = &[ /// here it is the one thing aid knows about dl's grammar. const DL_VALUE_OPTIONS: &[&str] = &["--devcontainer"]; -/// The dl workspace verbs a line may end with, and win with. -/// -/// aid's rule is that everything after the spec is prompt, flags and all, which is -/// what lets a prompt go unquoted. These two are the one exception, and it is -/// bounded to earn it — see [`peel_suffix`]. They exist because appending to a -/// recalled line is the cheap edit a shell offers and rewriting the front of one is -/// not, so "and now delete it" has to be spellable as a suffix or it is not -/// spellable at all. -const SUFFIX_VERBS: &[&str] = &["--rm", "--stop"]; - -/// The modifier those verbs take, peeled only in their company. +/// The modifier the suffix options take, peeled only in their company. /// /// On its own at the end of a line `--force` is prompt text. It is also the one /// word that must not reach dl as a *leading* option on its own: dl reads argv @@ -129,20 +119,35 @@ const SUFFIX_VERBS: &[&str] = &["--rm", "--stop"]; /// name, and a `--force` in the workspace slot is a workspace called `--force`. const SUFFIX_MODIFIERS: &[&str] = &["--force"]; -/// dl options a line may end with that ride *with* the prompt instead of beating it. +/// dl options a line may end with, which ride *with* the prompt. +/// +/// aid's rule is that everything after the spec is prompt, flags and all, which is +/// what lets a prompt go unquoted. These are the one exception, and it is bounded to +/// earn it — see [`peel_suffix`]. They exist because appending to a recalled line is +/// the cheap edit a shell offers and rewriting the front of one is not, so "and then +/// delete it" has to be spellable as a suffix or it is not spellable at all. +/// +/// **The prompt survives it, which is the whole difference from what `--rm` used to +/// be.** It means "run the prompt *and then* delete the workspace" — the commonest +/// thing anybody wants of an agent line: send it in, let it work, get the disk back. +/// +/// Bounded three ways to earn the exception: only at the very end of the line, only +/// these exact words, and only as whole argv words. `aid explain the --rm flag` +/// ends on `flag` and is untouched, and a quoted `aid 'why --rm'` is one +/// argument that is not `--rm`. Divergence row 32. +const SUFFIX_OPTIONS: &[&str] = &["--rm"]; + +/// The retired spellings, peeled for one reason: so dl refuses them by name. /// -/// The difference from [`SUFFIX_VERBS`] is the whole reason this is a third list and -/// not a fourth entry in that one. `--rm` appended to a recalled line means "forget -/// the prompt, delete it instead", so it displaces the prompt and owes a sentence -/// saying so. `--autorm` means "run the prompt *and then* delete it", which is the -/// commonest thing anybody wants of an agent line — send it in, let it work, get the -/// disk back — so the prompt has to survive the flag. +/// Left out of the peel they would fall into the *prompt* — aid joins every +/// post-spec word — and `aid 'review this pr' --autorm` would quietly ask an +/// agent to read `--autorm` where it used to delete the workspace. Peeled, they ride +/// to dl ahead of nothing at all: [`Task::Retired`] builds no agent command, so dl +/// refuses the spelling before a workspace is booted or a prompt is asked for. /// -/// Bounded the same three ways to earn the same exception: only at the very end of -/// the line, only these exact words, and only as whole argv words. `aid explain -/// the --autorm flag` ends on `flag` and is untouched, and a quoted `aid 'why -/// --autorm'` is one argument that is not `--autorm`. -const SUFFIX_OPTIONS: &[&str] = &["--autorm"]; +/// They are not [`SUFFIX_OPTIONS`] because that list is what a *running* line may +/// carry, and both of these end the line instead. Divergence row 32. +const SUFFIX_RETIRED: &[&str] = &["--stop", "--autorm"]; /// The names a `--flag` can pick an agent by, sorted — for the help and for the /// refusal that lists them. @@ -188,60 +193,40 @@ pub(crate) struct AidArgs { /// What the line asks dl to do with the workspace. /// -/// A sum rather than a prompt beside a list of flags, because the two are -/// exclusive and each carries what the other has no use for: a `--rm` line starts -/// no agent and so has no prompt to give one, and an agent line has no verb flags. -/// The prompt the verb beat is *kept* on that arm rather than dropped, because a -/// line that deletes a workspace owes the person the sentence naming the words it -/// did not act on. +/// A sum rather than a prompt beside a bool, because the second arm has no use for +/// a prompt and building one for it would be building a thing to ignore: a line +/// carrying a retired spelling starts no agent, so there is nothing to prompt. #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) enum Task { /// Start an agent, with this prompt. Empty is a session with no prompt rather /// than a prompt that is empty. Agent { agent: String, prompt: String }, - /// A dl workspace verb, spelled as a flag, which wins over the prompt. - Verb { - /// The verb and any modifier it came with, handed to dl as they were - /// typed. Never empty: no flags is no verb, which is the other arm. - flags: Vec, - /// The prompt it overrode, or empty when the line carried none. - overridden: String, - }, -} - -impl Task { - /// The verb flag this line won with, for the notice that names it. + /// A line spelling a flag this build has retired ([`SUFFIX_RETIRED`]). /// - /// `Some` for every [`Task::Verb`] by construction — the arm is only built - /// from a peel that found one. - pub(crate) fn verb_flag(&self) -> Option<&str> { - match self { - Self::Agent { .. } => None, - Self::Verb { flags, .. } => flags - .iter() - .map(String::as_str) - .find(|word| SUFFIX_VERBS.contains(word)), - } - } + /// Carries nothing: what it asks dl for is the *refusal*, which is dl's sentence + /// to write and needs no argument from here. Naming it as its own arm rather + /// than letting the line through as an agent is what keeps the interactive flow + /// from booting a workspace and asking for a prompt on the way to exit 1. + Retired, } impl AidArgs { /// The agent this line starts, when it starts one. /// - /// `None` is a verb line, which starts none — the distinction the caller needs - /// when reporting an agent name the environment invented. + /// `None` is a retired-spelling line, which starts none — the distinction the + /// caller needs when reporting an agent name the environment invented. pub(crate) fn agent(&self) -> Option<&str> { match &self.task { Task::Agent { agent, .. } => Some(agent), - Task::Verb { .. } => None, + Task::Retired => None, } } /// The same line with the prompt the interactive editor collected. /// - /// A verb line is returned unchanged — it has no prompt to replace, and the - /// interactive flow never reaches one — so this cannot turn "delete the - /// workspace" back into "start an agent". + /// A retired-spelling line is returned unchanged — it has no prompt to replace, + /// and the interactive flow never reaches one — so this cannot turn a refusal + /// back into a launch. pub(crate) fn with_prompt(mut self, typed: String) -> Self { if let Task::Agent { prompt, .. } = &mut self.task { *prompt = typed; @@ -255,9 +240,9 @@ impl AidArgs { /// `[
…, , "up"]` — dl's own warm-up verb, which is idempotent /// and hands over no session. This is what the interactive flow runs in the /// background while the prompt is being typed, so it deliberately carries **no -/// suffix options**: `--autorm` beside `up` is a pair dl refuses by name, `up` -/// takes no automatic removal anyway, and the flag still rides on the foreground -/// attach line where it means what it always meant. +/// suffix options**: `--rm` beside `up` is a pair dl refuses by name, `up` hands +/// over no session for a removal to wait on anyway, and the flag still rides on the +/// foreground attach line where it means what it means. pub(crate) fn build_boot_args(parsed: &AidArgs) -> Vec { let mut args = parsed.dl_options.clone(); args.push(parsed.spec.clone()); @@ -279,34 +264,29 @@ pub(crate) fn default_agent(environment: Option<&str>) -> Result, - /// dl options that leave the prompt standing. - options: Vec, +/// Derived from the run rather than carried beside it: a `retired` field would be a +/// second copy of a fact the words already state, and the two could disagree. +fn names_a_retired_spelling(options: &[String]) -> bool { + options + .iter() + .any(|word| SUFFIX_RETIRED.contains(&word.as_str())) } /// Split a trailing run of dl flags off the end of a command line. /// /// The one exception to "everything after the spec is prompt", and bounded three /// ways to earn it: only at the very *end* of the line, only the exact words in -/// [`SUFFIX_VERBS`], [`SUFFIX_MODIFIERS`] and [`SUFFIX_OPTIONS`], and only when a -/// verb or an option is among them. A prompt whose last word happens to be `--force` -/// is untouched, and so is one that merely mentions `--rm` inside it — `aid fix -/// the --rm flag` ends on `flag`, and a quoted `aid 'drop --rm'` is one argument -/// that is not `--rm`. +/// [`SUFFIX_OPTIONS`], [`SUFFIX_RETIRED`] and [`SUFFIX_MODIFIERS`], and only when an +/// option or a retired spelling is among them. A prompt whose last word happens to +/// be `--force` is untouched, and so is one that merely mentions `--rm` inside it — +/// `aid fix the --rm flag` ends on `flag`, and a quoted `aid 'drop --rm'` +/// is one argument that is not `--rm`. /// -/// A `--force` in the run goes to [`Suffix::verbs`] when a verb is there to modify -/// and to [`Suffix::options`] when there is not. That second case is `aid -/// --autorm --force`, which is a pair dl refuses by name — and handing it -/// to dl whole is what gets the person that sentence instead of a `--force` silently -/// glued onto their prompt. +/// A `--force` in the run is peeled with them, which is what gets `aid +/// --rm --force` the sentence dl has for that pair instead of a `--force` silently +/// glued onto the prompt. /// /// **Options come out ahead of modifiers, whichever order they were typed in**, and /// that is not cosmetic. dl recovers `--force`'s meaning from its *position* in the @@ -324,54 +304,48 @@ struct Suffix { /// **Divergence row 30**, aid's half: Python joined every post-spec word into the /// prompt with no exception, so `aid --rm` asked an agent to read /// `--rm`. -fn peel_suffix(argv: &[String]) -> Option<(&[String], Suffix)> { +fn peel_suffix(argv: &[String]) -> Option<(&[String], Vec)> { let is_suffix = |word: &str| { - SUFFIX_VERBS.contains(&word) + SUFFIX_OPTIONS.contains(&word) + || SUFFIX_RETIRED.contains(&word) || SUFFIX_MODIFIERS.contains(&word) - || SUFFIX_OPTIONS.contains(&word) }; let mut at = argv.len(); while at > 0 && is_suffix(argv[at - 1].as_str()) { at -= 1; } let run = &argv[at..]; - let has_verb = run.iter().any(|word| SUFFIX_VERBS.contains(&word.as_str())); - let has_option = run - .iter() - .any(|word| SUFFIX_OPTIONS.contains(&word.as_str())); + let names = |list: &[&str]| run.iter().any(|word| list.contains(&word.as_str())); // A run of nothing but modifiers is prompt text, which is the rule `--force` // alone has always been read by. - if !has_verb && !has_option { + if !names(SUFFIX_OPTIONS) && !names(SUFFIX_RETIRED) { return None; } - let mut suffix = Suffix::default(); // Modifiers held back and appended, so an option always precedes one — see the // positional argument above. Typed order is kept *within* each group. + let mut options: Vec = Vec::new(); let mut modifiers: Vec = Vec::new(); for word in run { - if SUFFIX_OPTIONS.contains(&word.as_str()) { - suffix.options.push(word.clone()); - } else if has_verb { - suffix.verbs.push(word.clone()); - } else { + if SUFFIX_MODIFIERS.contains(&word.as_str()) { modifiers.push(word.clone()); + } else { + options.push(word.clone()); } } - suffix.options.append(&mut modifiers); - Some((&argv[..at], suffix)) + options.append(&mut modifiers); + Some((&argv[..at], options)) } /// Split an aid command line into agent, dl options, workspace spec, and the task. /// /// The first argument that is neither an agent flag nor a dl option is the workspace /// spec; everything after it is the prompt, flags and all, so a prompt never has to -/// be quoted to protect it from aid's own parsing — except a trailing verb flag, -/// which [`peel_suffix`] takes off first and which wins over the prompt. +/// be quoted to protect it from aid's own parsing — except a trailing dl option, +/// which [`peel_suffix`] takes off first and which rides beside the prompt. /// -/// A verb flag can also arrive *before* the spec (`aid --rm owner/repo`), which is -/// the same request written the other way round. It is collected as a verb rather -/// than passed through as an ordinary dl option, because the pass-through would -/// hand dl a verb and an agent command at once and dl refuses that pair. +/// A `--rm` can also arrive *before* the spec (`aid --rm owner/repo fix it`), which +/// is the same request written the other way round: an unrecognised leading flag is +/// passed through to dl, and dl takes it in either position. pub(crate) fn parse_aid_args( argv: &[String], environment: Option<&str>, @@ -382,9 +356,8 @@ pub(crate) fn parse_aid_args( let mut agent = default_agent(environment)?; let (line, trailing) = match peel_suffix(argv) { Some((line, trailing)) => (line, trailing), - None => (argv, Suffix::default()), + None => (argv, Vec::new()), }; - let mut leading: Vec = Vec::new(); let mut dl_options: Vec = Vec::new(); let mut spec: Option = None; let mut at = 0; @@ -401,11 +374,6 @@ pub(crate) fn parse_aid_args( at += 2; continue; } - if SUFFIX_VERBS.contains(&word) { - leading.push(word.to_owned()); - at += 1; - continue; - } if word.starts_with('-') { // aid does not need to know every dl flag to stay out of its way. dl_options.push(word.to_owned()); @@ -420,18 +388,15 @@ pub(crate) fn parse_aid_args( return Err(UsageError::NoWorkspace); }; let prompt = line[at.min(line.len())..].join(" "); - let flags: Vec = leading.into_iter().chain(trailing.verbs).collect(); + let retired = names_a_retired_spelling(&trailing); Ok(AidArgs { spec, dl_options, - spec_options: trailing.options, - task: if flags.is_empty() { - Task::Agent { agent, prompt } + spec_options: trailing, + task: if retired { + Task::Retired } else { - Task::Verb { - flags, - overridden: prompt, - } + Task::Agent { agent, prompt } }, }) } @@ -494,14 +459,14 @@ pub(crate) fn build_agent_command( /// joining everything after `--` with spaces, which is why the agent command is one /// argument and its quoting lives inside it. /// -/// `--autorm` lands between the spec and the `--`, so the agent still gets its prompt -/// and dl still gets the flag: `[…, , "--autorm", "--", ]`. +/// `--rm` lands between the spec and the `--`, so the agent still gets its prompt and +/// dl still gets the flag: `[…, , "--rm", "--", ]`. /// -/// A verb line is `[
…, , …]` and has **no `--` tail**: -/// dl refuses a command beside a verb, and rightly, because the point of the suffix -/// is that no agent is being started. The verb flags go *after* the spec so that a -/// `--force` among them lands where dl reads it as the modifier rather than as the -/// workspace's name. +/// A retired-spelling line is `[
…, , …]` and has **no `--` +/// tail**: there is no agent to start, and a command tail would only give dl a second +/// thing to complain about ahead of the spelling that is the actual problem. The +/// options still go *after* the spec so that a `--force` among them lands where dl +/// reads it as the modifier rather than as the workspace's name. pub(crate) fn build_dl_args( parsed: &AidArgs, herdr: Option<&herdr::Session>, @@ -516,7 +481,7 @@ pub(crate) fn build_dl_args( args.push("--".to_owned()); args.push(build_agent_command(agent, prompt, herdr)?); } - Task::Verb { flags, .. } => args.extend(flags.iter().cloned()), + Task::Retired => {} } Some(args) } @@ -537,20 +502,12 @@ mod tests { parse_aid_args(&words(argv), None).expect("a usable command line") } - /// The prompt an agent line carries. Panics on a verb line, which has none — - /// the tests that mean to look at one use [`overridden`]. + /// The prompt an agent line carries. Panics on a retired-spelling line, which + /// carries none. fn prompt(parsed: &AidArgs) -> &str { match &parsed.task { Task::Agent { prompt, .. } => prompt, - Task::Verb { .. } => panic!("a verb line has no prompt"), - } - } - - /// The verb flags a line ends with, and the prompt they beat. - fn overridden(argv: &[&str]) -> (Vec, String) { - match parsed(argv).task { - Task::Agent { .. } => panic!("an agent line overrides nothing"), - Task::Verb { flags, overridden } => (flags, overridden), + Task::Retired => panic!("a retired-spelling line has no prompt"), } } @@ -664,60 +621,36 @@ mod tests { } } - // --------------------------------------------------- the suffix verb + // ------------------------------------------------- the retired spellings #[test] - fn a_trailing_verb_flag_wins_over_the_prompt_and_keeps_it() { - // The line this exists for: a recalled `aid` command with `--rm --force` - // typed at the end. The prompt is kept rather than dropped, because the - // notice has to be able to name it. - let (flags, beaten) = overridden(&[ - "kinisi-robotics/kinisi_ros@fix/x", - "review this pr", - "--rm", - "--force", - ]); - - assert_eq!(flags, ["--rm", "--force"]); - assert_eq!(beaten, "review this pr"); - } - - #[test] - fn a_verb_line_asks_dl_for_the_verb_and_starts_no_agent() { - // No `--` tail at all: dl refuses a command beside a verb, and there is no - // agent to run one. `--force` lands after the spec, where dl reads it as - // the modifier rather than as the workspace's name. + fn a_retired_spelling_starts_no_agent_and_is_handed_to_dl_to_refuse() { + // The transitional line: `--autorm` recalled from history, now spelled + // `--rm`. Peeled rather than joined into the prompt, so the person gets dl's + // sentence naming the new spelling instead of an agent asked to read + // `--autorm` — and with no `--` tail, so dl refuses before a workspace is + // booted or an agent runs. assert_eq!( - build_dl_args( - &parsed(&["owner/repo@fix/x", "review this pr", "--rm", "--force"]), - None - ) - .expect("a verb line needs no agent"), - ["owner/repo@fix/x", "--rm", "--force"] + parsed(&["owner/repo", "fix the bug", "--autorm"]).task, + Task::Retired + ); + assert_eq!( + build_dl_args(&parsed(&["owner/repo", "fix the bug", "--autorm"]), None) + .expect("a retired-spelling line needs no agent"), + ["owner/repo", "--autorm"] ); assert_eq!( build_dl_args( &parsed(&["--devcontainer", "robot", "owner/repo", "hi", "--stop"]), None ) - .expect("a verb line needs no agent"), + .expect("a retired-spelling line needs no agent"), ["--devcontainer", "robot", "owner/repo", "--stop"] ); } #[test] - fn a_verb_flag_before_the_spec_is_the_same_request() { - // `aid --rm owner/repo` used to hand dl a verb *and* an agent command, a - // pair dl refuses. It is collected as the verb instead. - assert_eq!( - build_dl_args(&parsed(&["--rm", "owner/repo"]), None).expect("a verb line"), - ["owner/repo", "--rm"] - ); - assert_eq!(overridden(&["--rm", "owner/repo"]).1, ""); - } - - #[test] - fn a_prompt_that_merely_mentions_a_verb_flag_is_still_a_prompt() { + fn a_prompt_that_merely_mentions_a_suffix_flag_is_still_a_prompt() { // The bound the peel is worth having: only the exact word, only at the very // end. Everything else stays what aid has always made it. assert_eq!( @@ -728,7 +661,7 @@ mod tests { prompt(&parsed(&["owner/repo", "explain --rm"])), "explain --rm" ); - // `--force` alone is not a verb, so it is not a suffix — and a `--force` + // `--force` alone names no option, so it is not a suffix — and a `--force` // handed to dl as a leading option would be read as the workspace's name. assert_eq!( prompt(&parsed(&["owner/repo", "do it", "--force"])), @@ -737,7 +670,7 @@ mod tests { } #[test] - fn a_verb_flag_with_no_workspace_is_still_no_workspace() { + fn a_suffix_flag_with_no_workspace_is_still_no_workspace() { for argv in [vec!["--rm"], vec!["--stop", "--force"]] { assert_eq!( parse_aid_args(&words(&argv), None), @@ -747,18 +680,6 @@ mod tests { } } - #[test] - fn the_notice_names_the_verb_that_won() { - let removing = parsed(&["owner/repo", "review this pr", "--rm", "--force"]); - - assert_eq!(removing.task.verb_flag(), Some("--rm")); - assert_eq!( - parsed(&["owner/repo", "hi"]).task.verb_flag(), - None, - "an agent line won nothing" - ); - } - // --------------------------------------------- the agent's command #[test] @@ -942,30 +863,30 @@ mod tests { // ------------------------------------------------- the suffix option #[test] - fn a_trailing_autorm_keeps_the_prompt_and_rides_beside_it() { - // The difference from `--rm` in one assertion: this is the line somebody - // actually types — send the agent in, and have the workspace go when it is - // done — so the prompt has to survive the flag rather than lose to it. - let chosen = parsed(&["owner/repo@fix/x", "fix the flaky test", "--autorm"]); + fn a_trailing_rm_keeps_the_prompt_and_rides_beside_it() { + // The line somebody actually types — send the agent in, and have the + // workspace go when it is done — so the prompt has to survive the flag + // rather than lose to it, which under row 30 it did not. + let chosen = parsed(&["owner/repo@fix/x", "fix the flaky test", "--rm"]); assert_eq!(prompt(&chosen), "fix the flaky test"); - assert_eq!(chosen.spec_options, ["--autorm"]); + assert_eq!(chosen.spec_options, ["--rm"]); assert_eq!(chosen.agent(), Some("claude")); } #[test] - fn autorm_lands_between_the_spec_and_the_agent_command() { + fn rm_lands_between_the_spec_and_the_agent_command() { // Behind the spec, because that is where dl reads a flag as a modifier // rather than as the workspace's name, and ahead of the `--`, because // everything after that belongs to the workspace's command. - let built = build_dl_args(&parsed(&["owner/repo", "fix it", "--autorm"]), None) + let built = build_dl_args(&parsed(&["owner/repo", "fix it", "--rm"]), None) .expect("an agent line builds"); assert_eq!( built, [ "owner/repo", - "--autorm", + "--rm", "--", "CLAUDE_CODE_DISABLE_TERMINAL_TITLE=1 IS_SANDBOX=1 claude \ --dangerously-skip-permissions 'fix it'" @@ -974,38 +895,38 @@ mod tests { } #[test] - fn autorm_before_the_spec_is_the_same_request() { - // An unknown leading flag is passed through to dl, which is all `--autorm` + fn rm_before_the_spec_is_the_same_request() { + // An unknown leading flag is passed through to dl, which is all `--rm` // needs: dl accepts it in any position, unlike `--force`. - let built = build_dl_args(&parsed(&["--autorm", "owner/repo", "fix it"]), None) - .expect("an agent line"); + let built = + build_dl_args(&parsed(&["--rm", "owner/repo", "fix it"]), None).expect("an agent line"); - assert_eq!(built[0], "--autorm"); + assert_eq!(built[0], "--rm"); assert_eq!(built[1], "owner/repo"); } #[test] - fn autorm_with_no_prompt_is_a_session_that_cleans_up_after_itself() { - let chosen = parsed(&["owner/repo", "--autorm"]); + fn rm_with_no_prompt_is_a_session_that_cleans_up_after_itself() { + let chosen = parsed(&["owner/repo", "--rm"]); assert_eq!(prompt(&chosen), ""); - assert_eq!(chosen.spec_options, ["--autorm"]); + assert_eq!(chosen.spec_options, ["--rm"]); } #[test] - fn a_prompt_that_merely_mentions_autorm_is_still_a_prompt() { - // The same bound the verb peel is worth having: only the exact word, only at - // the very end, only as a whole argv word. + fn a_prompt_that_merely_mentions_rm_is_still_a_prompt() { + // The bound the peel is worth having: only the exact word, only at the very + // end, only as a whole argv word. assert_eq!( - prompt(&parsed(&["owner/repo", "explain the --autorm flag"])), - "explain the --autorm flag" + prompt(&parsed(&["owner/repo", "explain the --rm flag"])), + "explain the --rm flag" ); assert_eq!( - prompt(&parsed(&["owner/repo", "explain", "--autorm", "please"])), - "explain --autorm please" + prompt(&parsed(&["owner/repo", "explain", "--rm", "please"])), + "explain --rm please" ); assert!( - parsed(&["owner/repo", "explain the --autorm flag"]) + parsed(&["owner/repo", "explain the --rm flag"]) .spec_options .is_empty() ); @@ -1013,8 +934,8 @@ mod tests { #[test] fn force_alone_at_the_end_of_a_line_is_still_prompt_text() { - // Unchanged by the third list: a run of nothing but modifiers is not a - // suffix, which is the rule that keeps `--force` out of a prompt's way. + // A run of nothing but modifiers is not a suffix, which is the rule that + // keeps `--force` out of a prompt's way. assert_eq!( prompt(&parsed(&["owner/repo", "use the --force"])), "use the --force" @@ -1026,14 +947,14 @@ mod tests { } #[test] - fn autorm_and_force_are_handed_to_dl_whole_so_dl_can_refuse_the_pair() { - // dl refuses `--force` beside `--autorm` by name. Peeling both is what gets + fn rm_and_force_are_handed_to_dl_whole_so_dl_can_refuse_the_pair() { + // dl refuses `--force` beside `--rm` by name. Peeling both is what gets // the person that sentence, where leaving `--force` in the prompt would glue // it silently onto what the agent reads. - let chosen = parsed(&["owner/repo", "fix it", "--autorm", "--force"]); + let chosen = parsed(&["owner/repo", "fix it", "--rm", "--force"]); assert_eq!(prompt(&chosen), "fix it"); - assert_eq!(chosen.spec_options, ["--autorm", "--force"]); + assert_eq!(chosen.spec_options, ["--rm", "--force"]); } #[test] @@ -1044,9 +965,9 @@ mod tests { // *verb* — answering `Unknown command '--force'` about a line whose real // problem is the pair. An option ahead of it puts it at index 2, where dl // reads it as the modifier and refuses the pair by name. - let typed_backwards = parsed(&["owner/repo", "fix it", "--force", "--autorm"]); + let typed_backwards = parsed(&["owner/repo", "fix it", "--force", "--rm"]); - assert_eq!(typed_backwards.spec_options, ["--autorm", "--force"]); + assert_eq!(typed_backwards.spec_options, ["--rm", "--force"]); assert_eq!(prompt(&typed_backwards), "fix it"); let built = build_dl_args(&typed_backwards, None).expect("an agent line"); assert_eq!( @@ -1057,32 +978,29 @@ mod tests { } #[test] - fn a_verb_beside_autorm_keeps_the_verbs_own_reading() { - // `--rm --autorm` is a contradiction, and it is dl's to name rather than - // aid's: the verb still beats the prompt, the option still travels, and dl - // answers "--autorm means nothing for rm". + fn a_retired_spelling_beside_the_live_one_is_still_dls_to_refuse() { + // `--rm --autorm` is one request written twice, half of it in a spelling + // this build has retired — and naming which half is dl's job, not aid's. + // So the whole run travels and no agent command is built. let chosen = parsed(&["owner/repo", "review this pr", "--rm", "--autorm"]); - match &chosen.task { - Task::Verb { flags, overridden } => { - assert_eq!(flags, &["--rm"]); - assert_eq!(overridden, "review this pr"); - } - Task::Agent { .. } => panic!("the verb should still have won"), - } - assert_eq!(chosen.spec_options, ["--autorm"]); + assert_eq!(chosen.task, Task::Retired); + assert_eq!(chosen.spec_options, ["--rm", "--autorm"]); assert_eq!( - build_dl_args(&chosen, None).expect("a verb line"), - ["owner/repo", "--autorm", "--rm"] + build_dl_args(&chosen, None).expect("a retired-spelling line"), + ["owner/repo", "--rm", "--autorm"] ); } #[test] - fn the_help_names_autorm_and_says_it_keeps_the_prompt() { + fn the_help_names_rm_and_says_the_agent_still_runs() { let help = crate::help(); - assert!(help.contains("--autorm"), "{help}"); - assert!(help.contains("it keeps"), "{help}"); + assert!(help.contains("--rm"), "{help}"); + assert!(help.contains("the agent still runs"), "{help}"); + // And it names the verb, because "delete it now" no longer has an aid + // spelling at all and the help is where somebody looks for where it went. + assert!(help.contains("dl rm"), "{help}"); } // ------------------------------------------------ the interactive line @@ -1123,14 +1041,14 @@ mod tests { // The whole point of `with_prompt`: the editor's text goes through the // same quoting and the same prompt flags as a prompt typed on the command // line, so the two spellings cannot drift. - let chosen = parsed(&["owner/repo", "--autorm"]).with_prompt("fix the bug".to_owned()); + let chosen = parsed(&["owner/repo", "--rm"]).with_prompt("fix the bug".to_owned()); assert_eq!(prompt(&chosen), "fix the bug"); assert_eq!( build_dl_args(&chosen, None).expect("an agent line"), [ "owner/repo", - "--autorm", + "--rm", "--", "CLAUDE_CODE_DISABLE_TERMINAL_TITLE=1 IS_SANDBOX=1 claude \ --dangerously-skip-permissions 'fix the bug'" diff --git a/rust/devlaunch-core/completions/dl.bash b/rust/devlaunch-core/completions/dl.bash index 330ce3a0..fec3fafa 100644 --- a/rust/devlaunch-core/completions/dl.bash +++ b/rust/devlaunch-core/completions/dl.bash @@ -61,9 +61,11 @@ _dl_completion() { fi # Workspace subcommands - # `--autorm` is not a verb: it rides beside `dl ` and `dl -- `, - # which is exactly the position this list is offered in. - local ws_cmds="up stop rm code restart recreate reset dotfiles --autorm --" + # `--rm` is not a verb — the `rm` beside it is: the flag rides on `dl ` and + # `dl -- ` and deletes the workspace when that session ends, which is + # exactly the position this list is offered in. Both are here because they are + # two different requests, docker's `rm` and `run --rm`. + local ws_cmds="up stop rm code restart recreate reset dotfiles --rm --" # Options that take a value; a variant name or a path follows them. local value_opts="--devcontainer" diff --git a/rust/devlaunch-core/src/flows/lifecycle.rs b/rust/devlaunch-core/src/flows/lifecycle.rs index efd1447b..0d77b8c7 100644 --- a/rust/devlaunch-core/src/flows/lifecycle.rs +++ b/rust/devlaunch-core/src/flows/lifecycle.rs @@ -316,7 +316,7 @@ impl<'a> Refresh<'a> { /// The latch it clears is not a rate limit; it is "one command, one child", and /// what it protects against is a command that warmed the cache on its way in /// spawning a second child on its way out to describe the same world. That is not - /// this. `dl --autorm` is the one command that changes the workspace list + /// this. `dl --rm` is the one command that changes the workspace list /// **twice** — the launch may create a workspace and force the refresh that /// records it, and the removal then deletes it — so the child already spawned is /// indexing a world with a workspace in it that is about to be gone. Leaving the diff --git a/rust/dl/src/cli.rs b/rust/dl/src/cli.rs index d703e454..1da78197 100644 --- a/rust/dl/src/cli.rs +++ b/rust/dl/src/cli.rs @@ -50,12 +50,35 @@ const VERBS: [(&str, VerbWord); 8] = [ /// /// **Recognised rather than forgotten, and that is the whole point of the table.** /// A retired word simply dropped from [`VERBS`] would be read as a *workspace -/// name* — `dl prune ` would report an unknown workspace called `prune`, and -/// `dl prune --force --rm` would delete a workspace called `prune` instead of -/// ``. Kept here, the word still cannot be a target, still says what to type -/// instead, and a suffix verb meaning the same thing can absorb it silently. +/// name* — `dl prune ` would report an unknown workspace called `prune`, naming +/// the word that moved as if it were the thing being looked for. Kept here, the word +/// still cannot be a target, and still says what to type instead. const RETIRED: [(&str, RetiredWord); 1] = [("prune", RetiredWord::Prune)]; +/// The flags that were commands once and are not any more. +/// +/// **Recognised rather than removed, for the same reason [`RETIRED`] keeps `prune`.** +/// A flag simply deleted from [`Cli`] is clap's `unexpected argument` at exit 2, which +/// names the spelling and nothing else — and both of these moved *because* `--rm` +/// changed meaning, so the spelling is exactly what cannot explain itself. Kept here, +/// each one is refused at exit 1 by a sentence that says what to type. +/// +/// Both are hidden on [`Cli`]: they are not options this build offers, only spellings +/// it still answers for. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum RetiredFlag { + /// `--stop`, the flag spelling of the `stop` verb. + /// + /// It went with row 30's suffix form rather than on its own account. The whole + /// point of a flag-spelled verb was that it could be *appended* to a recalled + /// line and beat it; once `--rm` means "delete when the session ends", a flag + /// that instead cancels the line is the one thing that pair must not be — + /// `--rm` and `--stop` look alike, and would then act oppositely. + Stop, + /// `--autorm`, which is now spelled `--rm`. + Autorm, +} + /// A word this build no longer accepts as a verb. /// /// An enum rather than a string, so the sentence each one needs is an exhaustive @@ -84,17 +107,6 @@ impl RetiredWord { .map(|(_, retired)| *retired) } - /// The verb this word used to mean. - /// - /// Not for carrying out — the word is refused — but so a *suffix* flag asking - /// for the same thing can absorb it instead of reporting that it displaced - /// something. `dl prune --force --rm` says delete twice and means it once. - fn was(self) -> VerbWord { - match self { - Self::Prune => VerbWord::Remove, - } - } - /// The word to type instead. pub(crate) fn instead(self) -> &'static str { match self { @@ -152,21 +164,28 @@ impl VerbWord { /// Whether the workspace is deleted once the session it handed over has ended. /// /// A named pair rather than a bool, and carried by the two arms of [`Verb`] the flag -/// is *defined* for — which is what makes `dl code --autorm` unwritable rather -/// than merely refused, and what stops every other arm carrying a field the -/// dispatcher has to remember to ignore. +/// is *defined* for — which is what makes `dl code --rm` unwritable rather than +/// merely refused, and what stops every other arm carrying a field the dispatcher has +/// to remember to ignore. +/// +/// **This is `--rm`, and [`Verb::Remove`] is `rm`** — docker's split, which is where +/// the spelling comes from: `docker rm` deletes now, `docker run --rm` deletes once +/// what it ran has finished, and no docker subcommand takes a `--rm` meaning the +/// first of those. So here too the word is the only way to say "delete it now", the +/// flag the only way to say "delete it after", and neither spelling has to be read +/// twice to find out which was meant. Divergence row 32. /// /// **Two arms and not five, and that is a scope decision rather than a mechanical /// one.** `restart`, `recreate` and `reset` also end in a session /// (`LaunchVerb::attaches`), so the removal would work behind them; they are out -/// because `--autorm` means *the throwaway workspace* — open one, use it, let it go — +/// because `--rm` means *the throwaway workspace* — open one, use it, let it go — /// and not *clean up after whichever verb this was*. Widening it later is adding /// arms here; the refusal names the two forms rather than claiming those verbs hand /// over nothing, because they do. #[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub(crate) enum Autorm { - /// `--autorm`: remove the workspace when the session ends, guard and all. - OnExit, +pub(crate) enum RmOnExit { + /// `--rm`: remove the workspace when the session ends, guard and all. + Yes, /// The default: the workspace outlives the session, as it always has. No, } @@ -181,10 +200,10 @@ pub(crate) enum Autorm { pub(crate) enum Verb { /// `dl ` — bring it up and hand over an interactive shell. Attach { - autorm: Autorm, + rm: RmOnExit, }, /// `dl -- ` — one command, inside the workspace. - Run(NonEmpty, Autorm), + Run(NonEmpty, RmOnExit), Up, Stop, /// `rm`. `force` is `--force`: delete despite unsaved work, and count an @@ -288,45 +307,6 @@ pub(crate) enum Command { }, } -/// Words a flag-spelled verb overrode. -/// -/// `--rm` and `--stop` are the *suffix* form of their verbs: appended to a line -/// that already said something else, they win. What they displaced is carried -/// here rather than dropped, because a line that deletes a workspace must not also -/// swallow an instruction it did not carry out — the point of typing the suffix is -/// that the rest of the line is stale, and the risk is that it is not. -#[derive(Clone, Debug, PartialEq, Eq)] -pub(crate) struct Overridden { - /// The flag that won, spelled as it was typed. - pub(crate) flag: &'static str, - /// The words it displaced, in the order they were typed. [`NonEmpty`] - /// because nothing displaced is no override at all, not an override of - /// nothing — which is what the `None` beside it says. - pub(crate) words: NonEmpty, -} - -/// One resolved command line: what to do, and what saying so overrode. -/// -/// A pair rather than a field on [`Command`], because only the two flag-spelled -/// verbs can override anything and every other arm would carry an `Option` that is -/// always `None`. -#[derive(Clone, Debug, PartialEq, Eq)] -pub(crate) struct Resolved { - pub(crate) command: Command, - pub(crate) overridden: Option, -} - -impl Command { - /// This command, having overridden nothing — every arm but the flag-spelled - /// verbs'. - fn alone(self) -> Resolved { - Resolved { - command: self, - overridden: None, - } - } -} - /// A command line clap accepted but `dl` cannot make a command of. /// /// Kept apart from clap's own errors because the two exit differently: clap's @@ -338,6 +318,8 @@ pub(crate) enum GrammarError { UnknownVerb { target: String, word: String }, /// A word that was a verb in an earlier build and is not one now. RetiredVerb(RetiredWord), + /// A flag that was a command in an earlier build and is not one now. + RetiredFlag(RetiredFlag), /// A global command was given a workspace to act on. TargetNotAllowed { command: &'static str }, /// A modifier that means nothing for the command it was given to. @@ -349,22 +331,29 @@ pub(crate) enum GrammarError { CommandNotAllowed { verb: &'static str }, /// `--devcontainer` on a command that opens no workspace. DevcontainerNotAllowed { command: &'static str }, - /// `--autorm` on a command the flag is not defined for. + /// `--rm` on a command the flag is not defined for. /// - /// Refused rather than ignored, and `code` is why. `dl code --autorm` + /// Refused rather than ignored, and `code` is why. `dl code --rm` /// returns the moment devpod has told VS Code where to connect — *before* the /// editor is attached — so honouring it there would delete the container out /// from under a window that is still opening. A silently-dropped flag would /// read as "it ran and kept the workspace", which is the one reading that /// makes somebody type it again. - AutormNotAllowed { command: &'static str }, - /// `--force` beside `--autorm`. + /// + /// `dl rm --rm` reaches this too, and the sentence is the useful one there: + /// the verb alone already deletes the workspace. + RmNotAllowed { command: &'static str }, + /// `--force` beside `--rm`. /// /// The pair looks like it should compose and must not: the unsaved-work guard - /// is the whole reason `--autorm` is safe to leave on a recalled line, and a + /// is the whole reason `--rm` is safe to leave on a recalled line, and a /// `--force` habitually appended to that line would destroy work hours later, /// unattended, with nobody watching the sentence that explained it. - AutormForced, + /// + /// docker draws the line in the same place, which is worth knowing because it + /// makes this a shape rather than a special case: `-f` is `docker rm`'s, and + /// `docker run --rm` has no forcing flag at all. + RmForced, /// The `--devcontainer` value cannot be a path. Devcontainer { raw: String, @@ -456,14 +445,15 @@ pub(crate) struct Cli { #[arg(long, group = "what")] version: bool, - /// Stop a workspace without deleting it. May be appended to a line that - /// already said something else, and wins over it. - #[arg(long, group = "what")] + /// Retired: the flag spelling of the `stop` verb. Recognised so it can be + /// refused with the word to use instead. Deliberately outside the `what` group, + /// so `dl --ls --stop` reports the retirement rather than a group conflict about + /// a flag that no longer names a command. + #[arg(long, hide = true)] stop: bool, - /// Delete a workspace. Refuses if its clone holds work that is nowhere else. - /// May be appended to a line that already said something else, and wins. - #[arg(long, group = "what")] - rm: bool, + /// Retired: what `--rm` is now called. + #[arg(long, hide = true)] + autorm: bool, /// The known `owner/repo` strings, one per line (for shell completion). #[arg(long, group = "what", hide = true)] @@ -489,9 +479,9 @@ pub(crate) struct Cli { devcontainer: Option, /// Delete the workspace once the session ends, like `docker run --rm`. Only /// for the two forms that hand one over: `dl ` and `dl -- `. - /// Stops at work that is nowhere else, exactly as `rm` does. + /// Stops at work that is nowhere else, exactly as the `rm` verb does. #[arg(long)] - autorm: bool, + rm: bool, } /// The half of the grammar clap's own argument list cannot show: the verbs are @@ -508,7 +498,7 @@ const GRAMMAR: &str = "Examples: dl blooop/devlaunch -- make test Run one command inside the workspace dl blooop/devlaunch stop Stop it dl stop blooop-devlaunch-main-1a2b Stop it by workspace id - dl blooop/devlaunch --autorm Open it, and delete it when the shell exits + dl blooop/devlaunch --rm Open it, and delete it when the shell exits dl --ls --json Every workspace, machine-readable Workspace commands (dl , or dl ): @@ -532,32 +522,27 @@ dotfiles, TAB marks several rows and the verb applies to each in turn — dl rm clear five workspaces in one visit. The forms that end in a session (attach, --, restart, recreate, reset) take exactly one. ---stop and --rm are the same two verbs as flags, and unlike the words they may be -appended to a line that already says something else, which then loses: - - dl owner/repo 'review this pr' --rm --force - dl stop owner/repo --rm - -Both act on owner/repo. It is the shape for recalling the previous line and typing -at the end of it rather than rewriting its front. Whatever the suffix beat is named -on stderr before anything is removed, so a deliberate one and a slip do not look -alike. - 'prune' was a second spelling of the rm verb and is retired: it collided with the --prune flag below, which removes clone directories and no workspace at all. Typing it says so and names both. ---autorm is the throwaway workspace: dl --autorm and dl --autorm -- -delete the workspace and its clone once the session ends, the way docker run --rm -does. It stops at work that is nowhere else — the same check dl rm makes — and -then leaves the workspace standing and says so, which is what makes it safe to leave -on a recalled line. The exit code is the session's, not the removal's. +--rm is the throwaway workspace, and it is docker's --rm rather than another spelling +of the rm verb above: dl --rm and dl --rm -- hand over a session and +delete the workspace and its clone once it ends, exactly as docker run --rm does, +where the rm verb deletes one now. It stops at work that is nowhere else — the same +check the verb makes — and then leaves the workspace standing and says so, which is +what makes it safe to leave on a line you recall. The exit code is the session's, not +the removal's. -Those two forms and no others: every verb above refuses --autorm rather than ignoring -it, and 'code' is the one worth knowing about, since it returns while VS Code is still +Those two forms and no others: every verb above refuses --rm rather than ignoring it, +and 'code' is the one worth knowing about, since it returns while VS Code is still connecting. --force does not compose with it either — use dl rm --force when you -mean to delete despite the work. Best-effort by nature: a Ctrl-C during the build, or -a closed terminal, ends dl before the session does and leaves the workspace behind."; +mean to delete despite the work, which is where docker keeps its -f too. Best-effort +by nature: a Ctrl-C during the build, or a closed terminal, ends dl before the session +does and leaves the workspace behind. + +--stop is retired, and --autorm is what --rm is now called. Both are still recognised +and say so."; /// What no flag and no verb names: the variables that change a launch. /// @@ -583,8 +568,6 @@ enum Chosen { Reconcile, Purge, Version, - Stop, - Remove, Repos, CompletionData, UpdateCache, @@ -602,8 +585,6 @@ impl Cli { (self.reconcile, Chosen::Reconcile), (self.purge, Chosen::Purge), (self.version, Chosen::Version), - (self.stop, Chosen::Stop), - (self.rm, Chosen::Remove), (self.repos, Chosen::Repos), (self.completion_data, Chosen::CompletionData), (self.update_cache, Chosen::UpdateCache), @@ -611,22 +592,33 @@ impl Cli { .into_iter() .find_map(|(given, chosen)| given.then_some(chosen)) } + + /// The retired spelling this line used, if it used one. + fn retired_flag(&self) -> Option { + [ + (self.stop, RetiredFlag::Stop), + (self.autorm, RetiredFlag::Autorm), + ] + .into_iter() + .find_map(|(given, retired)| given.then_some(retired)) + } } /// The command `cli` asks for. /// /// Total over [`Cli`]: every accepted command line is either a [`Command`] or a /// named [`GrammarError`], and nothing is left to be worked out later. -pub(crate) fn resolve(cli: Cli, argv: &[String]) -> Result { +pub(crate) fn resolve(cli: Cli, argv: &[String]) -> Result { + // First, because both retired spellings moved on account of what `--rm` means + // now: a line carrying one of them is asking for a grammar this build has not + // got, and every other diagnostic would describe the line it typed instead of + // the reason it stopped working. + if let Some(retired) = cli.retired_flag() { + return Err(GrammarError::RetiredFlag(retired)); + } match cli.chosen() { - // The two flag-spelled verbs are the verb-first grammar under another - // name, so they go through the same words handling and inherit its - // target-or-selector rule — and, being flags, they are also the one form - // that can be appended to a line that already said something else. - Some(Chosen::Stop) => verb_command(&cli, VerbWord::Stop, flag_of(Chosen::Stop)), - Some(Chosen::Remove) => verb_command(&cli, VerbWord::Remove, flag_of(Chosen::Remove)), - Some(global) => global_command(&cli, global).map(Command::alone), - None => workspace_command(cli, argv).map(Command::alone), + Some(global) => global_command(&cli, global), + None => workspace_command(cli, argv), } } @@ -682,8 +674,8 @@ fn global_command(cli: &Cli, chosen: Chosen) -> Result { if cli.devcontainer.is_some() { return Err(GrammarError::DevcontainerNotAllowed { command: name }); } - if cli.autorm { - return Err(GrammarError::AutormNotAllowed { command: name }); + if cli.rm { + return Err(GrammarError::RmNotAllowed { command: name }); } if !cli.command.is_empty() { return Err(GrammarError::CommandNotAllowed { verb: name }); @@ -735,11 +727,6 @@ fn global_command(cli: &Cli, chosen: Chosen) -> Result { Chosen::Repos => Command::Repos, Chosen::CompletionData => Command::CompletionData, Chosen::UpdateCache => Command::UpdateCache { force: cli.force }, - // Handled by `resolve` before this function is reached: they are verbs, - // not global commands, and the compiler is the reason that stays true. - Chosen::Stop | Chosen::Remove => { - return Err(GrammarError::TargetNotAllowed { command: name }); - } }) } @@ -752,94 +739,12 @@ fn flag_of(chosen: Chosen) -> &'static str { Chosen::Reconcile => "--reconcile", Chosen::Purge => "--purge", Chosen::Version => "--version", - Chosen::Stop => "--stop", - Chosen::Remove => "--rm", Chosen::Repos => "--repos", Chosen::CompletionData => "--completion-data", Chosen::UpdateCache => "--update-cache", } } -/// `dl --stop []` and `dl --rm []`: the verb-first grammar, and -/// the suffix form of it. -/// -/// Unlike the bare words, these two may be *appended* to a line that already said -/// something else, and they win. That is the whole reason they are flags: a shell -/// makes recalling the previous line and typing at the end of it cheap, and -/// rewriting the front of a recalled line expensive, so the only shape that can -/// mean "and now delete it" is a suffix. `dl 'review this pr' --rm` and -/// `dl prune --force --rm` both remove ``, where the first used to be -/// [`GrammarError::UnknownVerb`] and the second took `prune` as the *workspace*. -/// -/// The displaced words are named rather than dropped in silence — see -/// [`Overridden`] — with one exception: a word spelling the same verb the flag -/// does displaced nothing, because it is the line saying one thing twice, which is -/// exactly what appending `--rm` to a `prune` line produces. -/// -/// **Divergence row 30.** Row 15 made the flag spellings work at all; this is the -/// one thing they do that the words cannot. -fn verb_command(cli: &Cli, word: VerbWord, flag: &'static str) -> Result { - let verb = word.with(cli.force); - if !cli.command.is_empty() { - return Err(GrammarError::CommandNotAllowed { verb: verb.word() }); - } - // Before `--yes`, because a `--stop --autorm` line is the more confused of the - // two and the flag it is confused about is the one worth naming. - if cli.autorm { - return Err(GrammarError::AutormNotAllowed { - command: verb.word(), - }); - } - if cli.yes { - return Err(GrammarError::ModifierNotAllowed { - modifier: "--yes", - command: verb.word(), - }); - } - let devcontainer = devcontainer_of(cli)?; - let (target, displaced) = pick_target(&cli.words, word); - Ok(Resolved { - command: match target { - None => Command::Select { verb, devcontainer }, - Some(target) => Command::Workspace { - target, - verb, - devcontainer, - }, - }, - overridden: NonEmpty::of(displaced).map(|words| Overridden { flag, words }), - }) -} - -/// The workspace on a `--rm`/`--stop` line, and the words that flag overrode. -/// -/// The target is the first word that is *not* a verb, so a verb word standing ahead -/// of the workspace is not mistaken for it. Everything after that first word is -/// displaced, and so is any verb word naming a different verb — except a word -/// spelling `flag`'s own verb, which is redundant rather than displaced. -/// -/// No target at all is the selector, as it is for a bare verb: `dl --rm prune` is -/// the remove verb spelled twice and names no workspace, so it picks one. -fn pick_target(words: &[String], flag: VerbWord) -> (Option, Vec) { - let mut target = None; - let mut displaced = Vec::new(); - for word in words { - // A retired verb word stands for what it used to mean, so it is not a - // workspace name here either. `dl prune --force --rm` is the line this - // whole suffix form exists for, and `prune` is exactly the word in it that - // is no longer a verb — reading it as the target would delete the wrong - // thing, and displacing it would report noise about a line that asked for - // one thing twice. - match VerbWord::of(word).or_else(|| RetiredWord::of(word).map(RetiredWord::was)) { - Some(spelled) if spelled == flag => {} - Some(_) => displaced.push(word.clone()), - None if target.is_none() => target = Some(word.clone()), - None => displaced.push(word.clone()), - } - } - (target, displaced) -} - /// The workspace-first and verb-first grammar: up to two words, plus `-- `. fn workspace_command(cli: Cli, argv: &[String]) -> Result { let devcontainer = devcontainer_of(&cli)?; @@ -851,21 +756,20 @@ fn workspace_command(cli: Cli, argv: &[String]) -> Result } // Before `force_placement`, and that ordering is the whole of the fix it is. // `--force`'s meaning is recovered from its *position* in the word stream, and - // `--autorm` is a word in that stream — so `dl --autorm --force` reads - // `--force` as trailing (the pair, correctly) while `dl --autorm --force ` - // reads it as the *verb* slot and answers `Unknown command '--force'` about a - // workspace called `--autorm`, which explains nothing about a line whose real - // problem is the pair. Asked here, every spelling of the pair gets the sentence - // that names it. + // `--rm` is a word in that stream — so `dl --rm --force` reads `--force` as + // trailing (the pair, correctly) while `dl --rm --force ` reads it as the + // *verb* slot and answers `Unknown command '--force'` about a workspace called + // `--rm`, which explains nothing about a line whose real problem is the pair. + // Asked here, every spelling of the pair gets the sentence that names it. // - // What this gives up is `dl --force --autorm`, which used to be an attach on a + // What this gives up is `dl --force --rm`, which used to be an attach on a // workspace *named* `--force` (the slot-0 reading Python's parity preserves) and // is now the refused pair. That is the better answer: a line carrying both flags // is somebody expecting `--force` to license the removal, not somebody who named // a workspace `--force`. The parity reading is untouched for every line that does - // not also say `--autorm`. - if cli.autorm && cli.force { - return Err(GrammarError::AutormForced); + // not also say `--rm`. + if cli.rm && cli.force { + return Err(GrammarError::RmForced); } // `--force` only means force where Python read it — after the workspace and the // verb. Anywhere earlier it is the word in that slot, and the refusal is @@ -878,10 +782,10 @@ fn workspace_command(cli: Cli, argv: &[String]) -> Result // unknown name does. return Ok(Command::Workspace { target: "--force".to_owned(), - // `Autorm::No` by the check above, not by choice: a line holding - // both flags was refused before it got here. + // `RmOnExit::No` by the check above, not by choice: a line + // holding both flags was refused before it got here. verb: Verb::Attach { - autorm: autorm_of(&cli), + rm: rm_on_exit_of(&cli), }, devcontainer, }); @@ -901,14 +805,14 @@ fn workspace_command(cli: Cli, argv: &[String]) -> Result }); } // Where `--force` really is the modifier, and so the one place the pair - // with `--autorm` can be told from a workspace that happens to be + // with `--rm` can be told from a workspace that happens to be // called `--force`. ForcePlace::Trailing => {} } } - let autorm = autorm_of(&cli); - let run = NonEmpty::of(cli.command.iter().cloned()).map(|words| Verb::Run(words, autorm)); - let attach = || Verb::Attach { autorm }; + let rm = rm_on_exit_of(&cli); + let run = NonEmpty::of(cli.command.iter().cloned()).map(|words| Verb::Run(words, rm)); + let attach = || Verb::Attach { rm }; let (target, verb) = match cli.words.as_slice() { [] => (None, run.unwrap_or_else(attach)), [only] => match VerbWord::of(only) { @@ -957,12 +861,14 @@ fn workspace_command(cli: Cli, argv: &[String]) -> Result if !cli.command.is_empty() && !matches!(verb, Verb::Run(..)) { return Err(GrammarError::CommandNotAllowed { verb: verb.word() }); } - // The verb words the grammar just resolved carry no [`Autorm`] — only the two - // attach spellings can — so a `--autorm` that reached one of them was written - // and could not be honoured. Read off the *verb* rather than the words, so - // `dl up --autorm` and `dl up --autorm` are refused alike. - if cli.autorm && !matches!(verb, Verb::Attach { .. } | Verb::Run(..)) { - return Err(GrammarError::AutormNotAllowed { + // The verb words the grammar just resolved carry no [`RmOnExit`] — only the two + // attach spellings can — so a `--rm` that reached one of them was written and + // could not be honoured. Read off the *verb* rather than the words, so + // `dl up --rm` and `dl up --rm` are refused alike. `dl rm --rm` + // lands here too: the word and the flag are two different requests, and saying so + // beats quietly treating the pair as one of them. + if cli.rm && !matches!(verb, Verb::Attach { .. } | Verb::Run(..)) { + return Err(GrammarError::RmNotAllowed { command: verb.word(), }); } @@ -977,12 +883,8 @@ fn workspace_command(cli: Cli, argv: &[String]) -> Result } /// Whether this line asked for the workspace to go when the session does. -fn autorm_of(cli: &Cli) -> Autorm { - if cli.autorm { - Autorm::OnExit - } else { - Autorm::No - } +fn rm_on_exit_of(cli: &Cli) -> RmOnExit { + if cli.rm { RmOnExit::Yes } else { RmOnExit::No } } fn devcontainer_of(cli: &Cli) -> Result, GrammarError> { @@ -1041,28 +943,14 @@ mod tests { use super::*; - /// The command a line resolves to, with what it overrode set aside — which is - /// `None` for everything but the two flag-spelled verbs, and what - /// [`full`] is for when it is not. + /// The command a line resolves to. fn parse(argv: &[&str]) -> Result { - full(argv).map(|resolved| resolved.command) - } - - fn full(argv: &[&str]) -> Result { let cli = Cli::try_parse_from(std::iter::once("dl").chain(argv.iter().copied())) .unwrap_or_else(|error| panic!("clap refused {argv:?}: {error}")); let raw: Vec = argv.iter().map(|word| (*word).to_owned()).collect(); resolve(cli, &raw) } - /// The words a line overrode, in order, or `None` when it overrode nothing. - fn overridden(argv: &[&str]) -> Option<(&'static str, Vec)> { - let resolved = full(argv).expect("a usable command line"); - resolved - .overridden - .map(|had| (had.flag, had.words.iter().cloned().collect())) - } - fn refused(argv: &[&str]) -> clap::error::ErrorKind { Cli::try_parse_from(std::iter::once("dl").chain(argv.iter().copied())) .expect_err("clap should refuse this") @@ -1071,17 +959,17 @@ mod tests { /// `dl ` — the plain attach, which is what almost every case below wants. fn attach() -> Verb { - Verb::Attach { autorm: Autorm::No } + Verb::Attach { rm: RmOnExit::No } } fn run(words: &[&str]) -> Verb { - run_with(words, Autorm::No) + run_with(words, RmOnExit::No) } - fn run_with(words: &[&str], autorm: Autorm) -> Verb { + fn run_with(words: &[&str], rm: RmOnExit) -> Verb { Verb::Run( NonEmpty::of(words.iter().map(|word| word.to_string())).expect("a command"), - autorm, + rm, ) } @@ -1155,22 +1043,36 @@ mod tests { } } + #[test] + fn a_recalled_prune_line_is_the_retirement_and_the_pair_is_named_ahead_of_it() { + // The two readings a `dl prune --force` line recalled with `--rm` + // appended can have, and they are not the same one. Row 30 made the whole + // line remove ``; with the override gone, the words are read again and + // `prune` is the retirement — *unless* `--force` is there too, in which case + // the pair is what the line got wrong first and the more confused half. + assert_eq!( + parse(&["prune", "ws", "--rm"]), + Err(GrammarError::RetiredVerb(RetiredWord::Prune)) + ); + assert_eq!( + parse(&["prune", "ws", "--force", "--rm"]), + Err(GrammarError::RmForced) + ); + } + #[test] fn a_retired_word_is_never_read_as_the_workspace() { // The failure mode the RETIRED table exists to prevent: dropped from the // verbs and nothing else, `prune` would be an ordinary word — a workspace - // name in every one of these lines. + // name here, and an unknown-workspace refusal naming the word that moved + // instead of saying that it moved. assert_eq!( - parse(&["prune", "ws", "--force", "--rm"]), - Ok(workspace("ws", Verb::Remove { force: true })), - "the recalled line still removes ws, not a workspace called prune" + parse(&["prune", "ws"]), + Err(GrammarError::RetiredVerb(RetiredWord::Prune)) ); - // And it displaced nothing: `prune` and `--rm` asked for the same thing. - assert_eq!(overridden(&["prune", "ws", "--rm"]), None); - // Asking for something else with it does displace it, and says so. assert_eq!( - overridden(&["prune", "ws", "--stop"]), - Some(("--stop", vec!["prune".to_owned()])) + parse(&["ws", "prune", "--force"]), + Err(GrammarError::RetiredVerb(RetiredWord::Prune)) ); } @@ -1393,14 +1295,71 @@ mod tests { } #[test] - fn the_flag_spelled_verbs_are_the_verb_first_grammar() { - assert_eq!(parse(&["--stop", "ws"]), Ok(workspace("ws", Verb::Stop))); + fn the_flag_spelled_stop_verb_is_retired_from_every_position() { + // Row 15 made `dl --stop ` work and row 30 made it appendable; row 32 + // takes the spelling back, because `--rm` now modifies a session and a + // look-alike flag that cancels the line instead is the one thing the pair + // must not be. Every position, so no line finds a corner where it survives. + for argv in [ + &["--stop"][..], + &["--stop", "ws"][..], + &["ws", "--stop"][..], + &["ws", "--stop", "--force"][..], + ] { + assert_eq!( + parse(argv), + Err(GrammarError::RetiredFlag(RetiredFlag::Stop)), + "dl {}", + argv.join(" ") + ); + } + } + + #[test] + fn autorm_is_retired_in_favour_of_the_rm_it_became() { + // The same behaviour under a new spelling, so the refusal is the only thing + // standing between a recalled `--autorm` line and a silent no-op. + for argv in [ + &["--autorm"][..], + &["ws", "--autorm"][..], + &["--autorm", "ws"][..], + &["ws", "--autorm", "--", "make", "test"][..], + ] { + assert_eq!( + parse(argv), + Err(GrammarError::RetiredFlag(RetiredFlag::Autorm)), + "dl {}", + argv.join(" ") + ); + } + } + + #[test] + fn a_retired_flag_is_answered_before_anything_else_the_line_got_wrong() { + // Both spellings moved *because* `--rm` changed meaning, so the retirement + // is the reason the line stopped working and every other diagnostic would + // describe something the person did not do. Asked first for that reason, + // and these are the lines that would otherwise answer something else: a + // group conflict, and a `--force` in the workspace slot. + assert_eq!( + parse(&["--ls", "--stop"]), + Err(GrammarError::RetiredFlag(RetiredFlag::Stop)) + ); + assert_eq!( + parse(&["--force", "--autorm"]), + Err(GrammarError::RetiredFlag(RetiredFlag::Autorm)) + ); + } + + #[test] + fn the_words_the_retired_flags_stood_for_still_work() { + assert_eq!(parse(&["stop", "ws"]), Ok(workspace("ws", Verb::Stop))); assert_eq!( - parse(&["--rm", "ws", "--force"]), + parse(&["rm", "ws", "--force"]), Ok(workspace("ws", Verb::Remove { force: true })) ); assert_eq!( - parse(&["--stop"]), + parse(&["stop"]), Ok(Command::Select { verb: Verb::Stop, devcontainer: None @@ -1505,82 +1464,23 @@ mod tests { // ============================================ the cache-refresh argv stripping - // ================================================ the flag-spelled verbs as a suffix - - #[test] - fn a_suffix_verb_overrides_a_stale_word_rather_than_refusing_the_line() { - // The shape this exists for: an `aid`-style line recalled from history, - // with `--rm` typed at the end of it. Before, two words neither of which - // is a verb was `UnknownVerb` and the line did nothing. - assert_eq!( - parse(&["owner/repo@fix/x", "review this pr", "--rm", "--force"]), - Ok(workspace("owner/repo@fix/x", Verb::Remove { force: true })) - ); - assert_eq!( - overridden(&["owner/repo@fix/x", "review this pr", "--rm"]), - Some(("--rm", vec!["review this pr".to_owned()])) - ); - } + // ============================================ a stale word beside the --rm flag #[test] - fn a_leading_verb_word_is_not_mistaken_for_the_workspace() { - // What `dl prune --force` + `--rm --force` recalls to. `--rm` used to - // read `prune` as the target and report an unknown workspace called that. + fn a_word_the_grammar_cannot_read_is_still_refused_beside_the_flag() { + // The line row 30 taught `--rm` to rescue, and row 32 hands back: an + // `aid`-style line recalled with the flag typed at the end. `--rm` no + // longer overrides anything, so the two words that are not a verb are + // what they were before — a refusal that names the word and how to run it. assert_eq!( - parse(&["prune", "ws", "--force", "--rm"]), - Ok(workspace("ws", Verb::Remove { force: true })) - ); - // And it displaced nothing: `prune` and `--rm` are one verb spelled twice. - assert_eq!(overridden(&["prune", "ws", "--rm"]), None); - assert_eq!(overridden(&["ws", "--rm"]), None); - } - - #[test] - fn a_suffix_verb_overrides_a_different_verb_and_says_so() { - // `dl stop` recalled with `--rm` appended: the flag wins, and the - // verb it beat is named, because the two are not the same request. - assert_eq!( - parse(&["ws", "stop", "--rm"]), - Ok(workspace("ws", Verb::Remove { force: false })) - ); - assert_eq!( - overridden(&["ws", "stop", "--rm"]), - Some(("--rm", vec!["stop".to_owned()])) - ); - assert_eq!( - overridden(&["ws", "rm", "--stop"]), - Some(("--stop", vec!["rm".to_owned()])) - ); - } - - #[test] - fn a_suffix_verb_with_only_its_own_verb_word_picks_a_workspace() { - // `dl --rm prune` names no workspace at all — it is the verb twice — so it - // opens the selector, as the bare `dl prune` does. - assert_eq!( - parse(&["--rm", "prune"]), - Ok(Command::Select { - verb: Verb::Remove { force: false }, - devcontainer: None + parse(&["owner/repo@fix/x", "review this pr", "--rm"]), + Err(GrammarError::UnknownVerb { + target: "owner/repo@fix/x".to_owned(), + word: "review this pr".to_owned() }) ); } - #[test] - fn the_notice_names_every_word_the_suffix_beat() { - assert_eq!( - crate::render::overridden_notice("--rm", &["review this pr".to_owned()]), - "--rm overrode the rest of the line: 'review this pr' was not acted on." - ); - assert_eq!( - crate::render::overridden_notice( - "--stop", - &["code".to_owned(), "and the rest".to_owned()] - ), - "--stop overrode the rest of the line: 'code', 'and the rest' were not acted on." - ); - } - #[test] fn the_refresh_predicate_sees_argv_with_devcontainer_removed() { let cases: [(&[&str], &[&str]); 5] = [ @@ -1626,44 +1526,32 @@ mod tests { assert!(help.contains("Usage: dl "), "{help}"); } - // ================================================================== --autorm + // ======================================================================= --rm #[test] - fn autorm_is_carried_by_the_two_forms_that_hand_over_a_session() { + fn rm_is_carried_by_the_two_forms_that_hand_over_a_session() { assert_eq!( - parse(&["ws", "--autorm"]), - Ok(workspace( - "ws", - Verb::Attach { - autorm: Autorm::OnExit - } - )) + parse(&["ws", "--rm"]), + Ok(workspace("ws", Verb::Attach { rm: RmOnExit::Yes })) ); assert_eq!( - parse(&["ws", "--autorm", "--", "make", "test"]), - Ok(workspace("ws", run_with(&["make", "test"], Autorm::OnExit))) + parse(&["ws", "--rm", "--", "make", "test"]), + Ok(workspace("ws", run_with(&["make", "test"], RmOnExit::Yes))) ); // Position is not the grammar's business: clap strips the flag wherever it // sits, and only `--force` has a slot that means something. assert_eq!( - parse(&["--autorm", "ws"]), - Ok(workspace( - "ws", - Verb::Attach { - autorm: Autorm::OnExit - } - )) + parse(&["--rm", "ws"]), + Ok(workspace("ws", Verb::Attach { rm: RmOnExit::Yes })) ); } #[test] - fn autorm_with_no_workspace_named_picks_one_and_keeps_the_flag() { + fn rm_with_no_workspace_named_picks_one_and_keeps_the_flag() { assert_eq!( - parse(&["--autorm"]), + parse(&["--rm"]), Ok(Command::Select { - verb: Verb::Attach { - autorm: Autorm::OnExit - }, + verb: Verb::Attach { rm: RmOnExit::Yes }, devcontainer: None }) ); @@ -1674,20 +1562,15 @@ mod tests { // The empty command is not a command, and the attach it collapses to is // still the attach that was asked to clean up after itself. assert_eq!( - parse(&["ws", "--autorm", "--"]), - Ok(workspace( - "ws", - Verb::Attach { - autorm: Autorm::OnExit - } - )) + parse(&["ws", "--rm", "--"]), + Ok(workspace("ws", Verb::Attach { rm: RmOnExit::Yes })) ); } #[test] fn every_verb_word_refuses_the_flag() { // `code` is why this is a refusal and not a shrug: it returns the moment - // devpod has told VS Code where to connect, so honouring `--autorm` there + // devpod has told VS Code where to connect, so honouring `--rm` there // would delete the container out from under a window still opening. // // `restart`, `recreate` and `reset` are in this list for a different reason @@ -1700,79 +1583,83 @@ mod tests { "up", "stop", "rm", "code", "restart", "recreate", "reset", "dotfiles", ] { assert_eq!( - parse(&["ws", word, "--autorm"]), - Err(GrammarError::AutormNotAllowed { command: word }), - "dl ws {word} --autorm" + parse(&["ws", word, "--rm"]), + Err(GrammarError::RmNotAllowed { command: word }), + "dl ws {word} --rm" ); // And from the other position, since the verb wins from either. assert_eq!( - parse(&[word, "ws", "--autorm"]), - Err(GrammarError::AutormNotAllowed { command: word }), - "dl {word} ws --autorm" + parse(&[word, "ws", "--rm"]), + Err(GrammarError::RmNotAllowed { command: word }), + "dl {word} ws --rm" ); } } #[test] - fn the_flag_spelled_verbs_refuse_it_too() { - // `--rm` and `--stop` are the same two verbs and get the same answer, under - // the word rather than the flag: the sentence names the verb, not the spelling. + fn the_rm_verb_beside_the_rm_flag_is_two_requests_and_says_so() { + // docker's split, in the one line where a person can trip over it: the word + // deletes now, the flag deletes when a session ends, and `dl ws rm --rm` has + // asked for both. Refused rather than silently treated as either, and the + // sentence names the verb — which is the spelling that already does what the + // line most likely wanted. assert_eq!( - parse(&["ws", "--rm", "--autorm"]), - Err(GrammarError::AutormNotAllowed { command: "rm" }) + parse(&["ws", "rm", "--rm"]), + Err(GrammarError::RmNotAllowed { command: "rm" }) ); assert_eq!( - parse(&["ws", "--stop", "--autorm"]), - Err(GrammarError::AutormNotAllowed { command: "stop" }) + parse(&["rm", "ws", "--rm", "--force"]), + Err(GrammarError::RmForced), + "the pair is named ahead of the verb, which is the more confused half" ); } #[test] fn a_command_that_opens_no_workspace_refuses_it() { assert_eq!( - parse(&["--ls", "--autorm"]), - Err(GrammarError::AutormNotAllowed { command: "--ls" }) + parse(&["--ls", "--rm"]), + Err(GrammarError::RmNotAllowed { command: "--ls" }) ); assert_eq!( - parse(&["--purge", "--autorm"]), - Err(GrammarError::AutormNotAllowed { command: "--purge" }) + parse(&["--purge", "--rm"]), + Err(GrammarError::RmNotAllowed { command: "--purge" }) ); } #[test] - fn force_does_not_compose_with_autorm() { + fn force_does_not_compose_with_rm() { // The pair looks like it should and must not: the guard is what makes the // flag safe to leave on a recalled line, and a habitual `--force` beside it // would destroy work later, unattended, with nobody reading the sentence. assert_eq!( - parse(&["ws", "--autorm", "--force"]), - Err(GrammarError::AutormForced) + parse(&["ws", "--rm", "--force"]), + Err(GrammarError::RmForced) ); assert_eq!( - parse(&["ws", "--autorm", "--force", "--", "make"]), - Err(GrammarError::AutormForced) + parse(&["ws", "--rm", "--force", "--", "make"]), + Err(GrammarError::RmForced) ); } #[test] fn the_pair_is_refused_wherever_force_sits_in_the_line() { - // `--force`'s meaning is recovered from its position, and `--autorm` is a word + // `--force`'s meaning is recovered from its position, and `--rm` is a word // in the stream that position is counted in — so without this the same pair // gets three different answers depending on where it was typed, two of them // describing the wrong problem: `Unknown command '--force'` about a workspace - // called `--autorm`, and `Unknown workspace '--force'`. + // called `--rm`, and `Unknown workspace '--force'`. for line in [ - vec!["ws", "--autorm", "--force"], - vec!["--autorm", "--force", "ws"], - vec!["--force", "--autorm", "ws"], - vec!["--autorm", "ws", "--force"], + vec!["ws", "--rm", "--force"], + vec!["--rm", "--force", "ws"], + vec!["--force", "--rm", "ws"], + vec!["--rm", "ws", "--force"], // No workspace at all: the selector's line, refused the same way rather // than opening a picker whose choice cannot be honoured. - vec!["--force", "--autorm"], + vec!["--force", "--rm"], ] { assert_eq!( parse(&line), - Err(GrammarError::AutormForced), + Err(GrammarError::RmForced), "dl {}", line.join(" ") ); @@ -1815,10 +1702,16 @@ mod tests { use clap::CommandFactory; let help = Cli::command().render_long_help().to_string(); - assert!(help.contains("dl blooop/devlaunch --autorm"), "{help}"); + assert!(help.contains("dl blooop/devlaunch --rm"), "{help}"); + assert!(help.contains("--rm is the throwaway workspace"), "{help}"); + // And the retired spellings are named there rather than only in the refusal, + // because the help is where somebody goes to find out what replaced one. assert!( - help.contains("--autorm is the throwaway workspace"), + help.contains("--stop is retired, and --autorm is what --rm is now called"), "{help}" ); + // Hidden means hidden: neither is an option this build offers. + assert!(!help.contains(" --stop\n"), "{help}"); + assert!(!help.contains(" --autorm\n"), "{help}"); } } diff --git a/rust/dl/src/commands.rs b/rust/dl/src/commands.rs index be607614..52b07493 100644 --- a/rust/dl/src/commands.rs +++ b/rust/dl/src/commands.rs @@ -22,7 +22,7 @@ use devlaunch_core::flows::listing::{self, CommandContext, DlView, Sizes}; use devlaunch_core::flows::repo_manager::CacheNotice; use devlaunch_core::runner::{Exit, Runner}; -use crate::cli::{self, Autorm, Command, ListOutput, Verb}; +use crate::cli::{self, Command, ListOutput, RmOnExit, Verb}; use crate::cold::ColdPath; use crate::launch::{self, Family, Reached}; use crate::render; @@ -531,10 +531,7 @@ fn render_workspace<'r>( } // Launch: clone, `devpod up`, fast attach, `-- ` through // `devpod ssh --command`. - Family::Launch { - verb: launched, - autorm, - } => { + Family::Launch { verb: launched, rm } => { let ran = launch::render_launch( context, cache, @@ -544,14 +541,12 @@ fn render_workspace<'r>( &launched, devcontainer, ); - after_the_session( - runner, context, cache, refresh, &mut cold, target, autorm, ran, - ) + after_the_session(runner, context, cache, refresh, &mut cold, target, rm, ran) } } } -/// `--autorm`: the workspace, once the session it was opened for has ended. +/// `--rm`: the workspace, once the session it was opened for has ended. /// /// Three decisions live here, and each one is the answer to a question the flag /// cannot dodge. @@ -566,7 +561,7 @@ fn render_workspace<'r>( /// one an earlier draft of this got wrong by keying on the exit code. It leaves the /// container **running**, devpod's record written and the clone cut — which is why /// [`lifecycle::create_record`] exists at all — so an unattended -/// `dl owner/repo --autorm -- make test` against a broken devcontainer would leak +/// `dl owner/repo --rm -- make test` against a broken devcontainer would leak /// precisely the workspace the flag was reached for. A session devpod refused /// outright, and an OpenSSH that is not installed, leave the same thing behind and /// are collected for the same reason. @@ -580,12 +575,12 @@ fn render_workspace<'r>( /// rather than anything carried out of the launch. That is a round trip this path /// could have saved — the launch already named the workspace — and it buys the one /// thing worth more than a round trip on a path that deletes: there is exactly one -/// answer to "which workspace is ``", so `--autorm` and `rm` cannot disagree -/// about what they are removing. It is also cheap where it matters: by now the +/// answer to "which workspace is ``", so `--rm` and the `rm` verb cannot +/// disagree about what they are removing. It is also cheap where it matters: by now the /// workspace exists and its record is written, so a bare `owner/repo` reads its /// default branch off the record rather than the network. /// -/// **Whose exit code.** The launch's, always. `dl owner/repo --autorm -- make test` +/// **Whose exit code.** The launch's, always. `dl owner/repo --rm -- make test` /// is read by a script that wants the test's answer, and a cleanup that refused is /// not the test failing. The refusal is loud on stderr and the workspace is still /// there, which is the recoverable state — `dl rm --force` is one line away, @@ -598,11 +593,11 @@ fn after_the_session<'r>( refresh: &mut Refresh<'_>, cold: &mut ColdPath<'r>, target: &str, - autorm: Autorm, + rm: RmOnExit, ran: launch::Ran, ) -> Ending { let launch::Ran { ending, reached } = ran; - let Autorm::OnExit = autorm else { + let RmOnExit::Yes = rm else { return ending; }; let Reached::TheWorkspace = reached else { @@ -610,9 +605,8 @@ fn after_the_session<'r>( }; // Said before the removal rather than after it: what it names is the reason // somebody may want to hit Ctrl-C, and a notice that arrives once the container - // is already gone is a receipt rather than a warning. Same rule as the - // `--rm`/`--stop` override notice in `command_line`. - eprintln!("{}", render::autorm_removing(target)); + // is already gone is a receipt rather than a warning. + eprintln!("{}", render::rm_on_exit_removing(target)); // The launch has already spent this command's one refresh — `run_attach` forces // one the moment the session returns — and that child is describing a world this // removal is about to change. Without re-arming, the cache a user's next @@ -1106,7 +1100,7 @@ fn render_select<'r>( // Each workspace after the first is one more state change after // whatever refresh the last one spawned, so the child indexing the // old world must not be the last word — the same reasoning as - // `--autorm`'s re-arm in `after_the_session`. A no-op for the + // `--rm`'s re-arm in `after_the_session`. A no-op for the // single pick every verb used to be. if already_acted > 0 { refresh.rearm(); diff --git a/rust/dl/src/launch.rs b/rust/dl/src/launch.rs index 719343e4..43c90e23 100644 --- a/rust/dl/src/launch.rs +++ b/rust/dl/src/launch.rs @@ -46,7 +46,7 @@ use devlaunch_core::flows::provision::{ }; use devlaunch_core::runner::Runner; -use crate::cli::{Autorm, Verb}; +use crate::cli::{RmOnExit, Verb}; use crate::cold::ColdPath; use crate::commands::Ending; use crate::render; @@ -65,29 +65,29 @@ pub(crate) enum Family { /// Everything that opens a workspace, and whether the workspace is to go once /// the session does. /// - /// [`Autorm`] rides *with* the launch verb rather than beside it in the + /// [`RmOnExit`] rides *with* the launch verb rather than beside it in the /// dispatcher, because the two are read together exactly once and only ever /// together: the removal is what happens after this launch, not a second thing /// the command was asked for. - Launch { verb: LaunchVerb, autorm: Autorm }, + Launch { verb: LaunchVerb, rm: RmOnExit }, } /// What each verb asks for, from the word the grammar resolved. /// -/// The `Autorm::No` on the six word verbs is not a default this function chose: the -/// grammar refuses `--autorm` on all of them ([`cli::GrammarError::AutormNotAllowed`]), +/// The `RmOnExit::No` on the six word verbs is not a default this function chose: the +/// grammar refuses `--rm` on all of them ([`cli::GrammarError::RmNotAllowed`]), /// so no other answer can reach here — the arms say it because [`Verb`] gives them /// nothing to say it with. pub(crate) fn family(verb: &Verb) -> Family { - let (launched, autorm) = match verb { + let (launched, rm) = match verb { Verb::Stop => return Family::Stop, Verb::Remove { force } => return Family::Remove { force: *force }, - Verb::Attach { autorm } => (LaunchVerb::Attach { command: None }, *autorm), + Verb::Attach { rm } => (LaunchVerb::Attach { command: None }, *rm), // Python's `" ".join(args[2:])`: the words are rejoined with single spaces // and the result is one shell command, quoted whole into the remote // payload. A word that needed quoting to survive the *host's* shell has // already been unquoted by it, so the join is what the user typed. - Verb::Run(words, autorm) => ( + Verb::Run(words, rm) => ( LaunchVerb::Attach { command: Some( words @@ -97,19 +97,16 @@ pub(crate) fn family(verb: &Verb) -> Family { .join(" "), ), }, - *autorm, + *rm, ), - Verb::Up => (LaunchVerb::Up, Autorm::No), - Verb::Code => (LaunchVerb::Code, Autorm::No), - Verb::Recreate => (LaunchVerb::Recreate, Autorm::No), - Verb::Restart => (LaunchVerb::Restart, Autorm::No), - Verb::Reset => (LaunchVerb::Reset, Autorm::No), - Verb::Dotfiles => (LaunchVerb::Dotfiles, Autorm::No), + Verb::Up => (LaunchVerb::Up, RmOnExit::No), + Verb::Code => (LaunchVerb::Code, RmOnExit::No), + Verb::Recreate => (LaunchVerb::Recreate, RmOnExit::No), + Verb::Restart => (LaunchVerb::Restart, RmOnExit::No), + Verb::Reset => (LaunchVerb::Reset, RmOnExit::No), + Verb::Dotfiles => (LaunchVerb::Dotfiles, RmOnExit::No), }; - Family::Launch { - verb: launched, - autorm, - } + Family::Launch { verb: launched, rm } } /// Lending the host's tools into every workspace dl opens. @@ -215,7 +212,7 @@ pub(crate) fn render_launch<'r>( "{} does not name a workspace: its path has no final component to name one after.", render::python_repr(target) ); - // Nothing was opened, so there is nothing for `--autorm` to close. + // Nothing was opened, so there is nothing for `--rm` to close. return Ran { ending: Ending::Refused, reached: Reached::Nothing, @@ -251,7 +248,7 @@ pub(crate) fn render_launch<'r>( /// is both "no such workspace" and "the container came up and the session would not /// open", and those are opposite answers to "is there something to remove": the /// first created nothing, the second left a running container and the clone behind. -/// Reading the exit code to guess between them is how `--autorm` came to leak the +/// Reading the exit code to guess between them is how `--rm` came to leak the /// workspaces it exists to collect. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum Reached { @@ -336,7 +333,7 @@ fn ran(outcome: Result, cache: &Path) -> Ran { // `Reached::TheWorkspace` for both, and for `UpRefused` that is the // point: a build that failed in `postCreateCommand` leaves the // container running and the clone cut, so this is exactly the - // workspace an unattended `--autorm` line is there to collect. + // workspace an unattended `--rm` line is there to collect. LaunchRefusal::UpRefused { exit } => Ran { ending: Ending::Child(exit), reached: Reached::TheWorkspace, diff --git a/rust/dl/src/lib.rs b/rust/dl/src/lib.rs index b539e56c..3d0a8ed4 100644 --- a/rust/dl/src/lib.rs +++ b/rust/dl/src/lib.rs @@ -57,12 +57,6 @@ pub use devlaunch_core::flows::herdr; /// `dl` quotes what a tool or an environment said. pub use render::python_repr; -/// The sentence a `--rm`/`--stop` appended to a line prints about what it -/// overrode: `aid` swallows the *prompt* when the suffix wins, so it owes the same -/// notice `dl` owes for a swallowed positional word, in the same words. See -/// [`render::overridden_notice`]. -pub use render::overridden_notice; - /// The version both binaries print, single-sourced from `Cargo.toml`. pub const VERSION: &str = env!("CARGO_PKG_VERSION"); @@ -315,21 +309,13 @@ fn command_line(argv: &[String]) -> Result { return Err(usage.exit_code()); } }; - let resolved = cli::resolve(parsed, argv).map_err(|grammar| { + cli::resolve(parsed, argv).map_err(|grammar| { eprintln!("{}", grammar_refusal(&grammar)); // Python's `logging.error(...); return 1` for every shape it refused after // parsing. Deliberately not clap's 2: these are the refusals Python also // made, and they keep its code. commands::Ending::Refused.code() - })?; - // Printed before the command runs, not after: what it names is the reason a - // person may want to hit Ctrl-C, and a notice that arrives once the container - // is already gone is a receipt rather than a warning. - if let Some(overridden) = &resolved.overridden { - let words: Vec = overridden.words.iter().cloned().collect(); - eprintln!("{}", overridden_notice(overridden.flag, &words)); - } - Ok(resolved.command) + }) } /// What a command line clap accepted but `dl` could not make a command of. @@ -353,6 +339,21 @@ fn grammar_refusal(refused: &cli::GrammarError) -> String { retired.word(), retired.instead() ), + // Divergence row 32. Each names the spelling that replaced it, because both + // moved on account of `--rm` changing meaning and neither person typing one + // can be assumed to know that yet. + cli::GrammarError::RetiredFlag(cli::RetiredFlag::Stop) => { + "--stop is no longer a flag: the flag spellings now modify a session \ + (--rm deletes the workspace once one ends) rather than name a verb. Use \ + 'dl stop' to stop a workspace." + .to_owned() + } + cli::GrammarError::RetiredFlag(cli::RetiredFlag::Autorm) => { + "--autorm is now spelled --rm: 'dl --rm' opens the workspace and \ + deletes it when the session ends, the way 'docker run --rm' does. Use \ + 'dl rm' to delete one now." + .to_owned() + } cli::GrammarError::TargetNotAllowed { command } => { format!("{command} takes no workspace: it is not a workspace command.") } @@ -367,20 +368,25 @@ fn grammar_refusal(refused: &cli::GrammarError) -> String { format!("--devcontainer means nothing for {command}: it opens no workspace.") } // The two forms it *does* apply to are named, and so is what to type to - // delete a workspace now: somebody who reached for `--autorm` on another - // verb wants the workspace gone at some point, and this sentence is where - // they find out when. Deliberately no claim about *why* the verb refuses — + // delete a workspace now: somebody who reached for `--rm` on another verb + // wants the workspace gone at some point, and this sentence is where they + // find out when. Deliberately no claim about *why* the verb refuses — // `restart`, `recreate` and `reset` do hand over a session, and the reason - // they are out is that `--autorm` is defined for the throwaway workspace - // rather than as a modifier on every verb that ends in a shell. - cli::GrammarError::AutormNotAllowed { command } => format!( - "--autorm applies to 'dl ' and 'dl -- ', not to \ - {command}. Use 'dl rm' to delete a workspace now." + // they are out is that `--rm` is defined for the throwaway workspace rather + // than as a modifier on every verb that ends in a shell. + // + // It has to read correctly for `command` = `rm`, which `dl rm --rm` + // reaches, and it does: the flag is not the verb, and the verb alone is + // already the answer the sentence points at. + cli::GrammarError::RmNotAllowed { command } => format!( + "--rm deletes the workspace when a session ends, so it applies to \ + 'dl ' and 'dl -- ', not to '{command}'. Use \ + 'dl rm' to delete a workspace now." ), - cli::GrammarError::AutormForced => { - "--force does not apply to --autorm: an automatic removal always stops at work \ - that is nowhere else, which is what makes it safe to leave on a line you recall. \ - Use 'dl rm --force' to delete one despite it." + cli::GrammarError::RmForced => { + "--force does not apply to --rm: a removal that waits for the session always \ + stops at work that is nowhere else, which is what makes it safe to leave on a \ + line you recall. Use 'dl rm --force' to delete one despite it." .to_owned() } cli::GrammarError::Devcontainer { diff --git a/rust/dl/src/render.rs b/rust/dl/src/render.rs index 9ab4461e..3a408904 100644 --- a/rust/dl/src/render.rs +++ b/rust/dl/src/render.rs @@ -294,30 +294,6 @@ impl Formatter for PythonPretty { } } -// --------------------------------------------------------------------------- -// The suffix verbs' notice -// --------------------------------------------------------------------------- - -/// What `--rm`/`--stop` overrode by being appended, said out loud. -/// -/// Written here and shared with `aid` rather than duplicated on both sides, -/// because both sides can be the one that swallows the words: dl when the stale -/// instruction was a positional word it now ignores, aid when it was the prompt aid -/// would otherwise have handed an agent. One sentence, one wording. -/// -/// It is not optional, and that is the point of the suffix form being safe at all. -/// The line that removes a workspace is now allowed to carry an instruction it will -/// not carry out, so it has to say which — a deliberate `--rm` and a slip look -/// identical until then. stderr, like every other diagnostic here. -pub fn overridden_notice(flag: &str, words: &[String]) -> String { - let named: Vec = words.iter().map(|word| python_repr(word)).collect(); - let was = if named.len() == 1 { "was" } else { "were" }; - format!( - "{flag} overrode the rest of the line: {} {was} not acted on.", - named.join(", ") - ) -} - // --------------------------------------------------------------------------- // Python's repr, for the diagnostics that quote what a tool said // --------------------------------------------------------------------------- @@ -1161,15 +1137,15 @@ pub(crate) fn removal_refusal(refused: &RemovalRefused, spec: &str) -> String { } } -/// `--autorm`, at the moment the session has ended and the removal begins. +/// `--rm`, at the moment the session has ended and the removal begins. /// /// Named rather than silent, and it names the *target as typed* rather than the /// resolved workspace id for two reasons: the id has not been resolved yet when this /// is said, and the word the user recognises is the one they wrote. Whatever the /// removal then decides — the clone lines, or [`removal_refusal`]'s sentence and a /// workspace still standing — reads as an answer to this. -pub(crate) fn autorm_removing(spec: &str) -> String { - format!("--autorm: the session has ended, removing {spec}.") +pub(crate) fn rm_on_exit_removing(spec: &str) -> String { + format!("--rm: the session has ended, removing {spec}.") } /// devpod would not let go of the workspace, and the clone was kept. diff --git a/rust/dl/tests/interrupt.rs b/rust/dl/tests/interrupt.rs index de606461..897649ac 100644 --- a/rust/dl/tests/interrupt.rs +++ b/rust/dl/tests/interrupt.rs @@ -243,8 +243,8 @@ fn blocking_session() -> (tempfile::TempDir, PathBuf) { } #[test] -fn a_ctrl_c_that_reaches_dl_mid_session_leaves_an_autorm_workspace_standing() { - // The limit `--autorm` documents, measured rather than reasoned about. dl's +fn a_ctrl_c_that_reaches_dl_mid_session_leaves_a_throwaway_workspace_standing() { + // The limit `--rm` documents, measured rather than reasoned about. dl's // SIGINT disposition is a signal handler, and a handler may not run a removal — // it cannot allocate, cannot lock, and does not return — so a SIGINT delivered // *to dl* ends the process before the removal it was going to make. @@ -260,7 +260,7 @@ fn a_ctrl_c_that_reaches_dl_mid_session_leaves_an_autorm_workspace_standing() { let ssh_started = root_path.join("ssh.started"); let mut child = Command::new(env!("CARGO_BIN_EXE_dl")) - .args(["devlaunch-main-zovomobo", "--autorm"]) + .args(["devlaunch-main-zovomobo", "--rm"]) .env_clear() .keeping_coverage() .env("PATH", format!("{root}/bin:{root}/gh-bin:/usr/bin:/bin")) diff --git a/rust/dl/tests/launch.rs b/rust/dl/tests/launch.rs index 407eb3c6..278e42a1 100644 --- a/rust/dl/tests/launch.rs +++ b/rust/dl/tests/launch.rs @@ -1415,11 +1415,11 @@ fn the_prose_timing_summary_names_a_launchs_round_trips() { } // =========================================================================== -// --autorm: the workspace, once the session it was opened for has ended +// --rm: the workspace, once the session it was opened for has ended // =========================================================================== #[test] -fn autorm_removes_the_workspace_once_the_session_has_ended() { +fn rm_on_exit_removes_the_workspace_once_the_session_has_ended() { // The whole flag as one observation: the fast attach happens exactly as it does // without the flag, and *then* the removal — resolved through the same // `devpod status` a `dl rm` resolves through, so the two cannot disagree @@ -1428,7 +1428,7 @@ fn autorm_removes_the_workspace_once_the_session_has_ended() { let clone = "cache/devlaunch/repos/blooop/devlaunch/devlaunch-main-zovomobo"; assert!(world.path(clone).exists(), "the fixture's clean clone"); - let run = world.dl(&[MAIN, "--autorm"]); + let run = world.dl(&[MAIN, "--rm"]); run.exited(0); // devpod's own line, inherited: the delete is a passthrough call. assert_eq!(run.out, format!("Successfully deleted workspace {MAIN}\n")); @@ -1440,7 +1440,7 @@ fn autorm_removes_the_workspace_once_the_session_has_ended() { // Said before the removal, not after: what it names is the reason to // reach for Ctrl-C, and a notice that arrives once the container is // gone is a receipt rather than a warning. - &format!("--autorm: the session has ended, removing {MAIN}."), + &format!("--rm: the session has ended, removing {MAIN}."), "Removed workspace clone: {ROOT}/cache/devlaunch/repos/blooop/devlaunch/\ devlaunch-main-zovomobo", &format!("Removed local clone for {MAIN}"), @@ -1462,11 +1462,11 @@ fn autorm_removes_the_workspace_once_the_session_has_ended() { } #[test] -fn autorm_runs_the_command_first_and_removes_the_workspace_after_it() { - // `dl --autorm -- ` is the throwaway one-shot: the payload is the same +fn rm_on_exit_runs_the_command_first_and_removes_the_workspace_after_it() { + // `dl --rm -- ` is the throwaway one-shot: the payload is the same // quoted `bash -lc` an ordinary `-- ` sends, and the removal follows it. let world = World::with(&["--warm"]); - let run = world.dl(&[MAIN, "--autorm", "--", "echo", "hi"]); + let run = world.dl(&[MAIN, "--rm", "--", "echo", "hi"]); run.exited(0); assert_eq!( world.calls().exact(&world.root), @@ -1480,7 +1480,7 @@ fn autorm_runs_the_command_first_and_removes_the_workspace_after_it() { } #[test] -fn autorm_stops_at_work_that_is_nowhere_else_and_leaves_the_workspace_standing() { +fn rm_on_exit_stops_at_work_that_is_nowhere_else_and_leaves_the_workspace_standing() { // The guard is the whole safety story of the flag, and it is `rm`'s guard rather // than a second one: an uncommitted file in the clone refuses the removal, in // the same sentence `dl rm` refuses it with, and the workspace survives the @@ -1490,7 +1490,7 @@ fn autorm_stops_at_work_that_is_nowhere_else_and_leaves_the_workspace_standing() std::fs::write(world.path(clone).join("scratch.txt"), "unsaved\n") .expect("the clone is dirtied"); - let run = world.dl(&[MAIN, "--autorm"]); + let run = world.dl(&[MAIN, "--rm"]); // The session's ending, not the cleanup's: a script reading `$?` after this // wants the session's answer, and a removal that refused is not the session // failing. @@ -1518,12 +1518,12 @@ fn autorm_stops_at_work_that_is_nowhere_else_and_leaves_the_workspace_standing() } #[test] -fn autorm_hands_back_the_sessions_own_status_and_still_removes() { +fn rm_on_exit_hands_back_the_sessions_own_status_and_still_removes() { // The two halves are independent: `--remote-exit` ends the session with the // remote program's 130, which is what the shell reads, and the removal that // follows changes it to nothing. let world = World::with(&["--warm", "--remote-exit"]); - let run = world.dl(&[MAIN, "--autorm"]); + let run = world.dl(&[MAIN, "--rm"]); run.exited(130); assert!( world @@ -1535,12 +1535,12 @@ fn autorm_hands_back_the_sessions_own_status_and_still_removes() { } #[test] -fn autorm_removes_nothing_when_no_session_was_ever_handed_over() { +fn rm_on_exit_removes_nothing_when_no_session_was_ever_handed_over() { // A launch that refused before a session created nothing this line should now // delete, and answering one refusal with a second unrelated one is exactly what // keying the removal on `Ending::Session` avoids. let world = World::with(&["--warm"]); - let run = world.dl(&["no-such-workspace", "--autorm"]); + let run = world.dl(&["no-such-workspace", "--rm"]); run.exited(1); assert!( !world @@ -1553,7 +1553,7 @@ fn autorm_removes_nothing_when_no_session_was_ever_handed_over() { } #[test] -fn a_cold_launch_with_autorm_removes_the_clone_it_just_created() { +fn a_cold_launch_with_rm_on_exit_removes_the_clone_it_just_created() { // The one ordering this flag could plausibly get wrong. A cold launch *writes* // `metadata.json` — the record naming the clone it cut — and the removal then // reads it to find that clone. Both go through the command's one `ColdPath`, so @@ -1563,7 +1563,7 @@ fn a_cold_launch_with_autorm_removes_the_clone_it_just_created() { let world = World::with(&[]); let clone = format!("cache/devlaunch/repos/blooop/devlaunch/{COLD}"); - let run = world.dl(&["blooop/devlaunch@cold", "--autorm"]); + let run = world.dl(&["blooop/devlaunch@cold", "--rm"]); run.exited(0); assert!( run.err.contains(&format!("Removed local clone for {COLD}")), @@ -1591,15 +1591,15 @@ fn a_cold_launch_with_autorm_removes_the_clone_it_just_created() { } #[test] -fn autorm_removes_a_workspace_whose_session_devpod_never_ran() { +fn rm_on_exit_removes_a_workspace_whose_session_devpod_never_ran() { // A deliberate choice, pinned rather than left to fall out of `Ending::Session`: // `devpod ssh` refusing is still a session that was handed over and came back, // and the removal happens. The alternative — keep the workspace when the // transport failed — leaks exactly the workspaces an unattended - // `dl repo --autorm -- ` was reached for to stop leaking, and the container + // `dl repo --rm -- ` was reached for to stop leaking, and the container // is reproducible from the spec where a leaked one has to be found by hand. let world = World::with(&["--warm", "--fail-session"]); - let run = world.dl(&[MAIN, "--autorm"]); + let run = world.dl(&[MAIN, "--rm"]); // devpod's own status, unchanged by the removal that followed it. run.exited(3); assert!( @@ -1612,22 +1612,21 @@ fn autorm_removes_a_workspace_whose_session_devpod_never_ran() { } #[test] -fn autorm_collects_a_workspace_whose_up_failed() { +fn rm_on_exit_collects_a_workspace_whose_up_failed() { // The case keying the cleanup on the exit code got wrong, and the one that // matters most: a `devpod up` that dies leaves the container running, devpod's // record written and the clone cut — see `lifecycle::create_record`, which exists - // because of it. An unattended `dl owner/repo --autorm -- ` against a broken + // because of it. An unattended `dl owner/repo --rm -- ` against a broken // devcontainer would otherwise leak exactly the workspace the flag was reached // for, on every run, silently. let world = World::with(&["--stopped", "--fail-up"]); let clone = "cache/devlaunch/repos/blooop/devlaunch/devlaunch-main-zovomobo"; - let run = world.dl(&[MAIN, "--autorm"]); + let run = world.dl(&[MAIN, "--rm"]); // devpod's own status, unchanged: the removal is not the `up` failing. run.exited(7); assert!( - run.err - .contains("--autorm: the session has ended, removing"), + run.err.contains("--rm: the session has ended, removing"), "the removal never ran: {}", run.err ); @@ -1660,10 +1659,10 @@ fn a_launch_that_created_nothing_is_not_followed_by_a_removal() { // nothing to remove, and a removal attempted anyway would report a second, // unrelated failure about a workspace that never existed. let world = World::with(&["--warm"]); - let run = world.dl(&["/", "--autorm"]); + let run = world.dl(&["/", "--rm"]); run.exited(1); assert!( - !run.err.contains("--autorm"), + !run.err.contains("--rm"), "a launch that created nothing announced a removal: {}", run.err ); @@ -1678,7 +1677,7 @@ fn a_launch_that_created_nothing_is_not_followed_by_a_removal() { } #[test] -fn autorm_refreshes_the_completions_after_the_removal_and_not_only_before_it() { +fn rm_on_exit_refreshes_the_completions_after_the_removal_and_not_only_before_it() { // The launch forces a refresh the moment the session returns, which spends this // command's one detached child — and that child is indexing a world with the // workspace still in it. Without re-arming the latch, the cache a user's next @@ -1688,7 +1687,7 @@ fn autorm_refreshes_the_completions_after_the_removal_and_not_only_before_it() { // Two children is the observable: each one's first act is a `devpod list`, and // nothing else on this path lists at all. let world = World::with(&["--warm"]); - world.dl(&[MAIN, "--autorm"]).exited(0); + world.dl(&[MAIN, "--rm"]).exited(0); let deadline = Instant::now() + Duration::from_secs(20); let mut lists = 0; diff --git a/rust/dl/tests/lifecycle.rs b/rust/dl/tests/lifecycle.rs index 9609a9b2..012a3436 100644 --- a/rust/dl/tests/lifecycle.rs +++ b/rust/dl/tests/lifecycle.rs @@ -401,26 +401,55 @@ fn a_config_choice_a_stop_cannot_honour_is_said_rather_than_discarded() { } #[test] -fn the_flag_spelled_verbs_stop_and_remove_a_workspace() { - // Divergence row 15. dl.py's docstring promised `dl --stop `; its dispatch - // read `--stop` as a workspace spec, so Python answered - // `Unknown workspace '--stop'` and exited 1. There is no Python golden for this - // one — the row is the reason it works. +fn the_retired_flag_spellings_name_the_words_that_replaced_them() { + // Divergence rows 15 and 30 gave `--stop` and `--rm` a verb-first grammar and + // then made them appendable; row 32 takes both spellings back, because `--rm` + // now means "delete the workspace when the session ends" and a look-alike flag + // that cancels the line instead is the one thing that pair must not be. Refused + // at exit 1 with the word to type — never clap's exit 2, which would name the + // spelling and not the replacement. let world = World::base(); - world.dl(&["--stop", "someones-project"]).exited(0); + let stopping = world.dl(&["--stop", "someones-project"]); + stopping.exited(1); + assert!( + stopping.err.contains("dl stop"), + "{}", + stopping.err + ); + assert!(world.devpod_calls().is_empty(), "a refusal touched devpod"); + + // And the words still do the work, from either position. + let stopped = World::base(); + stopped.dl(&["stop", "someones-project"]).exited(0); assert_eq!( - world.devpod_calls().last().expect("a call"), + stopped.devpod_calls().last().expect("a call"), "devpod stop someones-project" ); let removing = World::base(); - removing.dl(&["--rm", "someones-project"]).exited(0); + removing.dl(&["someones-project", "rm"]).exited(0); assert_eq!( removing.devpod_calls().last().expect("a call"), "devpod delete someones-project" ); } +#[test] +fn autorm_is_refused_with_the_spelling_that_replaced_it() { + // The one that matters most for a line recalled from history: the behaviour is + // unchanged, only the name moved, so a silent no-op would be the worst answer + // and a refusal naming `--rm` is the whole of what is owed. + let world = World::base(); + let run = world.dl(&["someones-project", "--autorm"]); + run.exited(1); + assert!( + run.err.contains("--autorm is now spelled --rm"), + "{}", + run.err + ); + assert!(world.devpod_calls().is_empty(), "a refusal touched devpod"); +} + // =========================================================================== // dl rm // =========================================================================== diff --git a/rust/dl/tests/read_side.rs b/rust/dl/tests/read_side.rs index d0719676..8739704f 100644 --- a/rust/dl/tests/read_side.rs +++ b/rust/dl/tests/read_side.rs @@ -803,7 +803,7 @@ fn a_verb_with_no_workspace_opens_the_selector_and_no_terminal_picks_nothing() { // draw no picker at all, so nothing is picked. Python's ending for a pick that // never came is this one: the help on stdout, exit 1 — and the help is clap's // (row 3). - // The invitation names what the picker will take: `stop` and `--rm` apply per + // The invitation names what the picker will take: `stop` and `rm` apply per // workspace, so TAB may mark several; a `-- ` ends in one session, so the // line is the single pick's. On a terminal that sentence is skim's header, drawn // inside the picker where it can be read; stdout is the *fallback*, and this @@ -816,7 +816,7 @@ fn a_verb_with_no_workspace_opens_the_selector_and_no_terminal_picks_nothing() { let one = "Select workspace (type to filter):\n"; for (args, invitation) in [ (vec!["stop"], several), - (vec!["--rm"], several), + (vec!["rm"], several), (vec!["--", "make", "test"], one), ] { let run = world.dl(&args);