fix: windows dev script quoting and set trailing space#893
Open
kylegrahammatzen wants to merge 1 commit intouseautumn:devfrom
Open
fix: windows dev script quoting and set trailing space#893kylegrahammatzen wants to merge 1 commit intouseautumn:devfrom
kylegrahammatzen wants to merge 1 commit intouseautumn:devfrom
Conversation
|
@kylegrahammatzen is attempting to deploy a commit to the Autumn Team on Vercel. A member of the Team first needs to authorize it. |
| ]; | ||
| } | ||
| const serverCmd = isWindows | ||
| ? `cd server && set SERVER_PORT=${SERVER_PORT}&& bun start` |
Contributor
There was a problem hiding this comment.
Intentional && placement — worth a comment
The absence of a space before && is intentional and correct: in Windows cmd.exe, the set command captures every character after = up to the next unescaped &, so set SERVER_PORT=8080 && ... stores "8080 " (trailing space) while set SERVER_PORT=8080&& ... stores "8080". This is easy to misread as a typo and silently revert. A short inline comment would help future maintainers:
Suggested change
| ? `cd server && set SERVER_PORT=${SERVER_PORT}&& bun start` | |
| ? `cd server && set SERVER_PORT=${SERVER_PORT}&& bun start` // no space before && (avoids trailing space in value) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/dev.ts
Line: 99
Comment:
**Intentional `&&` placement — worth a comment**
The absence of a space before `&&` is intentional and correct: in Windows `cmd.exe`, the `set` command captures every character after `=` up to the next unescaped `&`, so `set SERVER_PORT=8080 && ...` stores `"8080 "` (trailing space) while `set SERVER_PORT=8080&& ...` stores `"8080"`. This is easy to misread as a typo and silently revert. A short inline comment would help future maintainers:
```suggestion
? `cd server && set SERVER_PORT=${SERVER_PORT}&& bun start` // no space before && (avoids trailing space in value)
```
How can I resolve this? If you propose a fix, please make it concise.70d829b to
5558002
Compare
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
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
Fix Windows dev script quoting by passing commands as array args to concurrently instead of through cmd /c
Windows didn't like cmd /c mangling inner double quotes, causing
bun devto fail when setting up the repository.Related Issues
Follow-up to #397
Type of Change
Checklist
Additional Context
Tested on Windows 11 and WSL Ubuntu 24.04 where WSL had bunx not found from a fresh install so verifying on macOS is needed.
Summary by cubic
Fix Windows dev script quoting by invoking bun x concurrently directly with array args instead of cmd/sh wrappers. Prevents bun dev, workers, Vite, and checkout dev servers from failing on Windows.
Written for commit 5558002. Summary will update on new commits.
Greptile Summary
This PR fixes two Windows-specific issues in the
bun devscript: shell quote mangling caused by wrapping theconcurrentlyinvocation incmd /c, and a trailing-space bug in Windowssetvariable assignments. The fix unifies both platforms by spawningbunx concurrentlydirectly as an array argument toBun.spawn, with each sub-command passed as its own element rather than as a single shell-escaped string.Key changes:
Bun.spawn(["cmd", "/c",bunx concurrently ... "${cmd1}" "${cmd2}"])withBun.spawn(["bunx", "concurrently", ..., cmd1, cmd2])on both Windows and Unix, eliminatingcmd /cdouble-quote mangling that brokebun devon Windows.set SERVER_PORT=${SERVER_PORT} && bun dev→set SERVER_PORT=${SERVER_PORT}&& bun dev(removing the space before&&). In Windowscmd.exethesetcommand captures all characters up to the next unescaped&, so the old form stored a trailing space in the variable value; the new form does not.workers:devpush (isWindows ? "..." : "..."where both branches were identical) by using a single platform-agnostic string.Confidence Score: 4/5
cmd /c/sh -cwrappers in favour of directBun.spawnwith array args) is correct and the Windowssettrailing-space fix is valid cmd.exe syntax. The only gap is that the author explicitly noted macOS verification is still needed — the Unix path is simpler than before so failure is unlikely, but it has not been confirmed yet.sh -cto directbunxinvocation.Important Files Changed
Bun.spawncall to invokebunx concurrentlydirectly with commands as separate array arguments, eliminating thecmd /c/sh -cwrapper that caused Windows quote mangling; also removes trailing spaces fromset VAR=value &&→set VAR=value&&on Windows.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[bun dev / scripts/dev.ts] --> B{serverOnly?} B -- yes --> C{isWindows?} C -- yes --> D["serverCmd = 'cd server && set SERVER_PORT=PORT&& bun start'"] C -- no --> E["serverCmd = 'cd server && SERVER_PORT=PORT bun start'"] D & E --> F["Bun.spawn(['bunx','concurrently','-n','server,workers','-c','green,yellow', serverCmd, workersCmd])"] B -- no --> G{isWindows?} G -- yes --> H["cmds = ['cd server && set SERVER_PORT=PORT&& bun dev', ...]"] G -- no --> I["cmds = ['cd server && SERVER_PORT=PORT bun dev', ...]"] H & I --> J["Bun.spawn(['bunx','concurrently','-n',names,'-c',colors,...cmds])"] F --> K[concurrently spawns each cmd via system shell] J --> KLast reviewed commit: 70d829b