fix git mv commands when migrating files with $ characters
#6129
Conversation
🦋 Changeset detectedLatest commit: 4fe6202 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Rich-Harris
changed the title
git mv commands when migrating files with $ characters
| @@ -46,7 +47,7 @@ export function error(description, comment_id) { | |||
| */ | |||
| export function move_file(file, renamed, content, use_git) { | |||
| if (use_git) { | |||
| execSync(`git mv ${JSON.stringify(file)} ${JSON.stringify(renamed)}`); | |||
| execSync(`git mv ${sq.quote([file, renamed])}`); | |||
There was a problem hiding this comment.
Could we use https://nodejs.org/dist/latest/docs/api/child_process.html#child_processexecfilefile-args-options-callback instead so we can pass in an array of arguments directly and not have to worry about the shell escaping?
There was a problem hiding this comment.
Untested, but I think it'd be execFileSync('git', ['mv', file, renamed]).
There was a problem hiding this comment.
ohh, thats nice.
Seems to work alright for \n and $
execFileSync:
node -e 'console.log(require("child_process").execFileSync("echo",["bla\\n$1"]).toString())'
bla\n$1
execSync:
node -e 'console.log(require("child_process").execSync("echo bla\\n$1").toString())'
blan
There was a problem hiding this comment.
ooh yeah that's way better
This should fix #6113 — tested with the case in that issue, works fine, though I'd appreciate a sanity check from others
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. All changesets should bepatchuntil SvelteKit 1.0