Repository navigation
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Placeholder fallback loses the required equals syntax for dash-prefixed names, leaving recovery commands unusable after substitution.
1 open issue
What changed in this PR
Improves cross-shell recovery commands for icp canister create --with-icp and introduces a reusable flag builder for future follow-up commands.
Changes:
- Adds shared quoting and selection-flag formatting.
- Adds placeholder notes and regression tests for recovery commands.
- Documents the behavior in the changelog.
| File | Description |
|---|---|
| crates/icp-cli/src/commands/canister/create.rs | Uses shared flags and adds recovery notes and tests. |
| crates/icp-cli/src/commands/args.rs | Adds flag building, shell quoting, placeholders, and tests. |
| CHANGELOG.md | Documents cross-shell recovery improvements. |
Review effort: Balanced
| match word { | ||
| Some(word) => flags.text.push_str(&format!(" {word}")), | ||
| None => { | ||
| flags.text.push_str(&format!(" {short} {blank}")); |
58e021f to
68c3333
Compare
When `icp canister create --with-icp` stops before the cycles minting canister confirms the creation, it prints the command that finishes it: the ICP is already spent, so that command must paste as printed. Its flags single-quoted every value POSIX-style, which PowerShell reads differently (`'o'\''neil'`), and a name starting with `-` broke it in every shell: `--identity '-dash'` reaches the CLI as two flags. The flags now come from one builder, `commands::args::selection_flags`, which the shortfall hint will share: - short flags where there are some (`-n`, `-k`, `-e`), and `--identity`; - `--flag=value` for a value starting with `-` (`--identity=-dash`); - a value bare when nothing in it is special to any shell icp-cli completes for (the POSIX shells, fish, elvish and PowerShell), else in single quotes, else, for a name with `'`, in double quotes when nothing else in it is special there (`"o'neil"`); - anything else (a control character, a curly quote, a backslash, or a `'` with `$`, a backtick, `"` or `!`) left as a placeholder such as `<your identity>`, and the recovery message then ends with a line asking to fill it in. Unit tests pin every quoted word and flag string, and the recovery command for `-dash`, `o'neil` and a name no quoting carries. Each quoted form was run through the released 1.7.0 in zsh, bash 3.2 and 5, dash, ksh, fish, elvish and PowerShell. (cherry picked from commit 11a85d4) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Twxau844tMTwyHpQJEbmBo
68c3333 to
27d76b6
Compare
…how to look in on it
When the wait ended with the order unsettled (Ctrl-C, the hour, the
gateway out of reach) the command said one line ("Order co-1 stays
payable until ...", "Order co-1 is paid, delivering.") and a fixed hint
naming `icp cycles balance` without the flags the run used, then ended
with a short error of its own; --no-wait said the same hint. An order
an operator handles (a refund owed, or one needing review) said only
"Keep the order id".
Now the command says once, as the error it ends with, why the wait
ended (stopped; an hour passed; the gateway stopped answering), whether
the order stays payable until its deadline, is paid and being delivered,
or is in some other state, where its cycles land, and the
`icp cycles balance` command that shows them, on a line of its own with
the identity (the default named, since only the identity that made an
order can read it), network and environment the run used. --no-wait
says the same. A refund owed and an order needing review come with the
`icp canister call <gateway> order_status --query ... '("<id>")'`
command that reads the order back. Paragraphs are set apart by a blank
line, commands are indented and never wrapped, and a value no quoting
carries in every shell is left as <your identity> with a note. -q keeps
the status alone, as before.
The flags come from a builder this commit adds,
commands::args::selection_flags (PasteFlags, shell_word): the short
flag where there is one, `--flag=value` for a value starting with `-`,
which would otherwise read as a flag, and a value bare when nothing in
it is special to any of the POSIX shells, fish, elvish or PowerShell,
else single-quoted, else (for a name with `'`) double-quoted when
nothing else in it is special there; anything else is a placeholder,
`--flag=<placeholder>` when the value starts with `-`. Unit tests pin
every quoted word and every flag form. The builder was the first commit
of #823 and then #829, where it rewrote the recovery command
`icp canister create --with-icp` prints; that path is rare enough to be
left as it is.
The stand-in gateway's tests buy as a named identity, `buyer`, so the
printed flags can be asserted.
Extracted from PR #824.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015aem4tPs4TYVZb8k4LMbJJ
|
Closing without merging. The |

Cherry-pick of 11a85d4, the first commit of the closed #823, by Josh. The rest of #823 (the deploy-time shortfall hint) is not wanted; this part is, because the next PRs in the stack print pasteable follow-up commands and need the flag builder.
When
icp canister create --with-icpstops before the cycles minting canister confirms the creation, it prints the command that finishes it; the ICP is already spent, so that command must paste as printed. Its flags single-quoted every value POSIX-style, which PowerShell reads differently, and a name starting with-broke it in every shell. The flags now come from one builder,commands::args::selection_flags(PasteFlags,shell_word): short flags where there are some,--flag=valuefor a value starting with-, bare / single-quoted / double-quoted values as each shell icp-cli completes for needs, and a placeholder plus a note for anything no quoting carries.Unit tests pin every quoted word and the recovery command for
-dash,o'neiland a name no quoting carries.Part of restacking #824 into small PRs, one fix each, stacked on
mainin this order:cycles-buy/01-json-implies-yes→02-arg-group→03-gateway→04-qr-colours→05-paste-flags→ then the wait, its follow-up text and the--jsonfields. Each PR targets the one below it; after the one below merges, retarget tomain.🤖 Generated with Claude Code
https://claude.ai/code/session_01Twxau844tMTwyHpQJEbmBo