Skip to content

fix(nushell): strip surrounding quotes from --flag="value" tokens#2437

Open
worktrunk-bot wants to merge 2 commits intomainfrom
fix/nushell-strip-equals-quotes
Open

fix(nushell): strip surrounding quotes from --flag="value" tokens#2437
worktrunk-bot wants to merge 2 commits intomainfrom
fix/nushell-strip-equals-quotes

Conversation

@worktrunk-bot
Copy link
Copy Markdown
Collaborator

Summary

Fixes #2410. Nushell's def --wrapped re-tokenizes --flag "value" into two tokens but keeps --flag="value" as a single literal token (with the surrounding quote characters), so the worktrunk binary received argv like --base="releases/4.x.x" and clap parsed the quotes as part of the value. Strip matching surrounding " / ' from --flag=… / -x=… tokens before forwarding ...$args.

What changed

  • templates/nushell.nu — one-pass each over $args running str replace -r '^(-{1,2}[^=]+)=(["''])(.*)["'']$' '${1}=${3}' before the binary is invoked.
  • tests/integration_tests/shell_wrapper.rs — adds test_wrapper_nu_strips_quoted_equals_flag, an end-to-end test that creates a releases/4.x.x branch, runs wt switch -c feature --base="releases/4.x.x" through the actual nu wrapper via PTY, and asserts the worktree is created. Verified the test fails on main (without the template change) and passes with it.
  • src/shell/snapshots/worktrunk__shell__tests__init_nu.snap — refreshed for the new wrapper lines.

Forms exercised

Verified by hand against a real wt config shell init nu install on nushell 0.112.2:

form before after
--base "releases/4.x.x"
--base releases/4.x.x
--base=releases/4.x.x
--base="releases/4.x.x"
--base='releases/4.x.x'
-b="releases/4.x.x"

Test plan

  • cargo run -- hook pre-merge --yes passes locally (3377/3377 tests; lychee unavailable in this env, CI will run it)
  • New regression test fails without the template change and passes with it
  • Manual reproduction in nushell 0.112.2 with the patched wrapper

🤖 Generated with Claude Code

Nushell's `def --wrapped` re-tokenizes the space-separated form (`--flag
"value"` → ["--flag", "value"]) but keeps the equals form as a single
literal token (`--flag="value"` → ['--flag="value"']), so the worktrunk
binary received the quote characters in argv and clap parsed them as
part of the value. Fixes #2410.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@worktrunk-bot
Copy link
Copy Markdown
Collaborator Author

test (linux) and code-coverage both failed at the Install nushell step. hustcer/setup-nu@v3 returned "No release for Nushell matching version specifier * of default feature" for arch=x64, platform=linux — same error in two consecutive reruns. macOS test passed in 9m54s (the new test_wrapper_nu_strips_quoted_equals_flag runs there too, since macOS uses the same setup-nu action with the same config). The action worked on Linux in run 25004292538 at 15:32 UTC, ~1.5 hours before this PR — likely a transient upstream regression. Leaving for a maintainer to rerun once upstream recovers.

@worktrunk-bot
Copy link
Copy Markdown
Collaborator Author

CI failures (test (linux), code-coverage) are unrelated to the change — they fail on the Install nushell step before any tests run, with No release for Nushell matching version specifier * of default feature. The action is hustcer/setup-nu@v3 invoking GitHub's release API for arch = x64, platform = linux, feature = default; the same call for arch = arm64, platform = darwin ran ~12 minutes earlier and succeeded (macos log), and repos/nushell/nushell/releases returns valid data for 0.112.2 when queried directly. The same x64-Linux failure is hitting a sibling PR (pre-commit-ci-update-config) at the same wall-clock window, so this looks like a transient regression in setup-nu's Linux release-resolution logic — not caused by anything in this PR. Earlier runs from ~15:30 today (e.g. f6d2cc6e) passed. Should clear on rerun once the action's behaviour stabilises; pinning to a specific nushell version is also an option if the issue persists.

test (macos) passed (the new test_wrapper_nu_strips_quoted_equals_flag ran there, since shell-integration tests are unix-gated and macOS got a working nu). Locally with nu 0.112.2 installed manually, the new test passes and cargo run -- hook pre-merge --yes is green (3377/3377 tests).

@max-sixty
Copy link
Copy Markdown
Owner

But surely there's something we're doing wrong in our wrapper, rather than needing to regex strip string args??

We want a simple implementation that does as little as possible; can we review this behavior?

@worktrunk-bot
Copy link
Copy Markdown
Collaborator Author

Fair pushback — let me lay out what I checked, because the answer landed on "the strip is doing real work, but you may prefer to skip the fix entirely."

Why bash/zsh/fish don't need this and nushell does. In POSIX shells, the function never sees the quotes — the shell tokenises --base="releases/4.x.x" into argv element --base=releases/4.x.x before the function runs, and $argv (fish) / "$@" (bash, zsh) is just that already-clean argv. Nushell's def --wrapped is intentionally different: it preserves unknown flag tokens verbatim so the wrapper can forward them as-typed. Verified against nu 0.112.2:

def --wrapped wrap [...args] { ^python3 -c "import sys; print(sys.argv[1:])" ...$args }

wrap --base "releases/4.x.x"      # argv: ['--base', 'releases/4.x.x']   ✓ tokenised
wrap --base=releases/4.x.x        # argv: ['--base=releases/4.x.x']      ✓ tokenised
wrap --base="releases/4.x.x"      # argv: ['--base="releases/4.x.x"']    ✗ literal quotes preserved

Calling python3 directly (no def --wrapped in front) gives the clean argv in all three forms — exactly what git --message="…" is doing. The "git works in nushell" observation is "no wrapper in front of git", not "git parses better".

What I considered as alternatives, and why I landed on the strip:

  1. Declare every flag on the wrapper (def --wrapped wt [--base: string, --from: string, …]). I tested this — it does fix the equals-quote form for declared flags only. Unknown flags still pass through with literal quotes. Enumerating every wt flag and keeping it in sync with clap is more code and more drift surface than the 3-line strip.
  2. Drop the def --wrapped wrapper. The wrapper exists to consume the cd / exec directive files (so wt switch actually changes the shell's CWD) and to handle the list streaming carve-out. Without it those features break. We can't make this a pure extern (which is signature-only).
  3. Filing upstream. I haven't found an existing nushell issue for the equals-quote case in --wrapped specifically — closest is #17087, which is a different --wrapped parse quirk. Worth a separate report, but it's not load-bearing for this PR.

The "do less" path that actually exists. Drop the strip and document the limitation: tell nushell users that the supported forms are --base value, --base "value with spaces", and --base=value (no quotes). The only broken form is --base="value" — and a value without spaces never needs the quotes, while a value with spaces can use the space form. That's a real option if you'd rather keep the wrapper minimal. The cost is a sharper edge for users who reach for --flag="value" out of habit; the upside is the wrapper says exactly what it says today.

I'm happy either way — close this PR and I'll send a tiny doc/FAQ note instead, or land it as-is. (One small tightening if we keep it: the regex's closing-quote class doesn't backreference the opening quote, so --foo="bar' would also get stripped. Rust's regex crate doesn't support backrefs, so the fix is to do two passes — one for "…", one for '…'. Happy to roll that in if we keep the strip.)

@max-sixty
Copy link
Copy Markdown
Owner

so in general, CLI apps don't support nushell with --foo="bar"? including git, for example?

@worktrunk-bot
Copy link
Copy Markdown
Collaborator Author

No — git --message="hello world" works fine in nushell when called directly. Nushell strips the quotes during external-command tokenisation, same as POSIX shells.

The bug is specific to commands invoked through a def --wrapped function. --wrapped deliberately preserves unknown flag tokens verbatim (so the wrapper can forward them as-typed), and that preservation keeps the literal " characters in the equals form. Verified earlier against a one-line def --wrapped wrap [...args] { ^python3 -c "..." ...$args } — the space form and bare-equals form tokenise correctly; only --flag="value" survives with quotes intact.

We need the wrapper because of the cd/exec directive files (so wt switch actually changes the shell's CWD) and the list streaming carve-out — those features can't work from a bare extern declaration. So the wrapper has to exist, and any wrapper hits this nu quirk.

git, cargo, ls, etc. don't have wrappers in front of them, so they're unaffected. Anyone who aliased git via def --wrapped my-git [...args] { ^git ...$args } would see the same thing.

Given that, the choices are still the two from before:

  1. Keep the strip (this PR, fix per-flag-form for users) and tighten the regex to backreference the opening quote (two passes).
  2. Drop the strip and document that --flag="value" isn't supported in the nu wrapper — users should use --flag value or --flag=value.

Happy to go either way; let me know which you'd prefer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

wt switch fails to resolve base branch for worktree creation: "branch does not exist"

2 participants