Improve shell quote handling with proper nested quote support#217
Merged
branchseer merged 5 commits intomainfrom Mar 10, 2026
Merged
Improve shell quote handling with proper nested quote support#217branchseer merged 5 commits intomainfrom
branchseer merged 5 commits intomainfrom
Conversation
The previous `unquote_str` function naively stripped all quote characters
regardless of context, causing single quotes inside double-quoted strings
to be removed. This corrupted arguments like:
node -e "require('child_process').execSync('vp config')"
where 'child_process' would lose its quotes, breaking the JavaScript.
Replace with `brush_parser::word::parse` which properly parses word
pieces with quoting context, then flatten the pieces to extract literal
text. This correctly preserves single quotes inside double quotes and
vice versa.
Fixes running `prepare` script from packages like node-modules/urllib.
https://claude.ai/code/session_01SoJXo78ET9sowKTSdAuFAg
Tests DoubleQuotedSequence recursion with escape sequences, literal single quotes, and bail-out on parameter expansion / command substitution. https://claude.ai/code/session_01SoJXo78ET9sowKTSdAuFAg
fengmk2
approved these changes
Mar 10, 2026
The test helper `parse_and_flatten` necessarily uses `String` because the underlying `flatten_pieces` function takes `&mut String`. Added `#[expect]` attributes matching the pattern already used in production code. https://claude.ai/code/session_01SoJXo78ET9sowKTSdAuFAg
Replace disallowed String type with vite_str::Str in unquote() and flatten_pieces(), removing the need for #[expect(clippy::disallowed_types)] attributes and redundant .into() conversions at call sites. https://claude.ai/code/session_01SoJXo78ET9sowKTSdAuFAg
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replace
unquote_strwithbrush_parser::word::parsefor context-aware shell unquoting. The old function stripped all quote characters uniformly, breaking commands with nested quotes.Example
A
preparescript like:{ "prepare": "node -e \"const v = parseInt(process.versions.node, 10); if (v >= 20) require('child_process').execSync('vp config', {stdio: 'inherit'});\"" }Was parsed incorrectly because
unquote_strstripped the single quotes inside the double-quoted string:Test plan
cargo test -p vite_shell— all 7 tests passtest_unquote_preserves_nested_quotes— single quotes in double quotes, double quotes in single quotes, escape sequencestest_flatten_pieces_recursion— recursive flattening throughword::parse, bail on$VARand$(cmd)test_parse_urllib_prepare— real-world prepare script with nested quoteshttps://claude.ai/code/session_01SoJXo78ET9sowKTSdAuFAg