Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

use shell-quote.quote(args) instead of args.join(" ") #4215

Merged
merged 4 commits into from
Oct 23, 2023
Merged

use shell-quote.quote(args) instead of args.join(" ") #4215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8228b0e
override shell-quote.quote to allow non-string args
RamIdeas Oct 18, 2023
49678a1
use shell-quote.quote instead of cmd.join(" ")
RamIdeas Oct 18, 2023
f6daef1
add changeset
RamIdeas Oct 18, 2023
bd08fa0
update snapshots
RamIdeas Oct 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/cold-parrots-deliver.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"create-cloudflare": patch
"wrangler": patch
---

fix various logging of shell commands to correctly quote args when needed
5 changes: 4 additions & 1 deletion packages/create-cloudflare/src/cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { detectPackageManager } from "helpers/packages";
import semver from "semver";
import { version } from "../package.json";
import { validateProjectDirectory } from "./common";
import * as shellquote from "./helpers/shell-quote";
import { templateMap } from "./templateMap";
import type { C3Args } from "types";

Expand Down Expand Up @@ -55,7 +56,9 @@ export const runLatest = async () => {
// the parsing logic of `npm create` requires `--` to be supplied
// before any flags intended for the target command.
const argString =
npm === "npm" ? `-- ${args.join(" ")}` : `${args.join(" ")}`;
npm === "npm"
? `-- ${shellquote.quote(args)}`
: `${shellquote.quote(args)}`;

await runCommand(`${npm} create cloudflare@latest ${argString}`);
};
Expand Down
6 changes: 5 additions & 1 deletion packages/create-cloudflare/src/helpers/shell-quote.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import shellquote from "shell-quote";

export const quote = shellquote.quote;
export const quote = function (args: (string | number | boolean)[]) {
dario-piotrowicz marked this conversation as resolved.
Show resolved Hide resolved
const stringArgs = args.map((arg) => String(arg));

return shellquote.quote(stringArgs);
};

export function parse(cmd: string, env?: Record<string, string>): string[] {
// This is a workaround for a bug in shell-quote on Windows
Expand Down
5 changes: 4 additions & 1 deletion packages/create-cloudflare/src/pages.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
runDeploy,
setupProjectDirectory,
} from "./common";
import * as shellquote from "./helpers/shell-quote";
import type { C3Args, PagesGeneratorContext } from "types";

/** How many times to retry the create project command before failing. */
Expand Down Expand Up @@ -162,7 +163,9 @@ const createProject = async (ctx: PagesGeneratorContext) => {
const CLOUDFLARE_ACCOUNT_ID = ctx.account.id;

try {
const compatFlags = ctx.framework?.config.compatibilityFlags?.join(" ");
const compatFlags = shellquote.quote(
ctx.framework?.config.compatibilityFlags ?? []
);
const compatFlagsArg = compatFlags
? `--compatibility-flags ${compatFlags}`
: "";
Expand Down
2 changes: 1 addition & 1 deletion packages/wrangler/src/__tests__/generate.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe("generate", () => {
`"✨ Created no-template/wrangler.toml"`
);
expect(std.warn).toMatchInlineSnapshot(`
"▲ [WARNING] The \`init\` command is no longer supported. Please use \`mockpm create cloudflare@2 no-template\` instead.
"▲ [WARNING] The \`init\` command is no longer supported. Please use \`mockpm create cloudflare/@2 no-template\` instead.
Copy link
Contributor Author

@RamIdeas RamIdeas Oct 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something weird is going on with jest snapshot encoding

  • double backslash \\ is changed into a forward slash /
  • this doesn't effect real usage: \\@ is what is actually returned from shell-quote and that works

You can confirm by running wrangler init (with this PRs build) and seeing the C3 command logged out correctly


The \`init\` command will be removed in a future version.

Expand Down
86 changes: 43 additions & 43 deletions packages/wrangler/src/__tests__/init.test.ts
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions packages/wrangler/src/init.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,8 @@ export async function initHandler(args: InitArgs) {
c3Arguments.unshift(...shellquote.parse(getC3CommandFromEnv()));

// Deprecate the `init --from-dash` command
const replacementC3Command = `\`${packageManager.type} ${c3Arguments.join(
" "
const replacementC3Command = `\`${packageManager.type} ${shellquote.quote(
c3Arguments
)}\``;

logger.warn(
Expand Down
3 changes: 2 additions & 1 deletion packages/wrangler/src/pages/dev.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import { logger } from "../logger";
import * as metrics from "../metrics";
import { getBasePath } from "../paths";
import * as shellquote from "../utils/shell-quote";
import { buildFunctions } from "./buildFunctions";
import { ROUTES_SPEC_VERSION, SECONDS_TO_WAIT_FOR_PROXY } from "./constants";
import {
Expand Down Expand Up @@ -797,7 +798,7 @@
);
}

logger.log(`Running ${command.join(" ")}...`);
logger.log(`Running ${shellquote.quote(command)}...`);

Check warning on line 801 in packages/wrangler/src/pages/dev.ts

View check run for this annotation

Codecov / codecov/patch

packages/wrangler/src/pages/dev.ts#L801 

Added line #L801 was not covered by tests
const proxy = spawn(
command[0].toString(),
command.slice(1).map((value) => value.toString()),
Expand Down
6 changes: 5 additions & 1 deletion packages/wrangler/src/utils/shell-quote.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import shellquote from "shell-quote";

export const quote = shellquote.quote;
export const quote = function (args: (string | number | boolean)[]) {
const stringArgs = args.map((arg) => String(arg));
dario-piotrowicz marked this conversation as resolved.
Show resolved Hide resolved

return shellquote.quote(stringArgs);
};

export function parse(cmd: string, env?: Record<string, string>): string[] {
// This is a workaround for a bug in shell-quote on Windows
Expand Down
Loading