Skip to content

Commit 564a6d7

Browse files
[dev-tool] Handle passthrough of -- args (Azure#19451)
Merely adding `--` args wasn't enough. Dev-tool's argument parsing is recursive, and it achieves this by passing the remainder of a branching-command's `args` back through the arg parser until a leaf-command is reached. Unfortunately, that means that if `--` args are parsed at the first step of the process, they aren't passed back into further subcommands' arguments. This meant that `--` had to be used by the first command. This change updates the argument passing behavior of `subCommand` to make it so that branching commands pass `--` arguments to their children. I had previously tested to make sure that argument parsing worked as expected, but now I've also added a test to validate that `--` options are received by child commands. @HarshaNalluru
1 parent 2d1b829 commit 564a6d7

File tree

3 files changed

+40
-10
lines changed

3 files changed

+40
-10
lines changed

common/tools/dev-tool/src/commands/about.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { createPrinter } from "../util/printer";
99
import { leafCommand, makeCommandInfo } from "../framework/command";
1010
import { printCommandUsage } from "../framework/printCommandUsage";
1111

12-
const log = createPrinter("help");
12+
const log = createPrinter("about");
1313

1414
const banner = `\
1515
_______ __ __ __
@@ -38,7 +38,7 @@ export default leafCommand(commandInfo, async (options) => {
3838

3939
if (options.args.length || options["--"]?.length) {
4040
console.log();
41-
log.warn("Warning, unused options:", JSON.stringify(options));
41+
log.warn("Warning, unused options:", JSON.stringify(options, null, 2));
4242
}
4343

4444
await printCommandUsage(baseCommandInfo, baseCommands);

common/tools/dev-tool/src/framework/command.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ export function makeCommandInfo<Opts extends CommandOptions>(
5959
return {
6060
name,
6161
description,
62-
options: options as StrictAllowMultiple<Opts>
62+
options: options as StrictAllowMultiple<Opts>,
6363
};
6464
}
6565

@@ -96,12 +96,19 @@ export function subCommand<Info extends CommandInfo<CommandOptions>>(
9696
process.exit(1);
9797
}
9898

99-
log.debug(`$ ${commandName} ${commandArgs?.join(" ") ?? ""}`);
99+
const extraArgs = options["--"];
100+
101+
const fullArgs =
102+
extraArgs !== undefined && extraArgs.length > 0
103+
? [...commandArgs, "--", ...extraArgs]
104+
: commandArgs;
105+
106+
log.debug(`$ ${commandName} ${fullArgs.join(" ") ?? ""}`);
100107

101108
if (Object.prototype.hasOwnProperty.call(commands, commandName)) {
102109
const commandModule = await commands[commandName]();
103110

104-
return await commandModule.default(...commandArgs);
111+
return await commandModule.default(...fullArgs);
105112
} else {
106113
log.error("No such sub-command:", commandName);
107114
await printCommandUsage(info, commands, console.error);

common/tools/dev-tool/test/framework.spec.ts

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ const simpleCommandInfo = makeCommandInfo("simple", "a simple command", {
1313
kind: "string",
1414
description: "a simple argument",
1515
allowMultiple: false,
16-
default: "foo"
17-
}
16+
default: "foo",
17+
},
1818
});
1919

2020
interface SimpleExpectedOptionsType {
@@ -33,15 +33,15 @@ describe("Command Framework", () => {
3333
{
3434
sub: async () => ({
3535
commandInfo: { name: "sub", description: "a leaf command" },
36-
default: leafCommand({ name: "sub", description: "a leaf command" }, async () => true)
36+
default: leafCommand({ name: "sub", description: "a leaf command" }, async () => true),
3737
}),
3838
fail: async () => ({
3939
commandInfo: { name: "fail", description: "a command that fails" },
4040
default: leafCommand(
4141
{ name: "fail", description: "a command that fails" },
4242
async () => false
43-
)
44-
})
43+
),
44+
}),
4545
}
4646
);
4747

@@ -73,5 +73,28 @@ describe("Command Framework", () => {
7373
);
7474
assert.equal(opts.simpleArg, "test");
7575
});
76+
77+
it("nested", async () => {
78+
const nestedOptions = {
79+
name: "nested",
80+
description: "",
81+
options: { internal: { kind: "boolean", description: "" } },
82+
} as const;
83+
const command = subCommand(
84+
{ name: "top-level", description: "" },
85+
{
86+
nested: () =>
87+
Promise.resolve({
88+
default: leafCommand(nestedOptions, (options) => {
89+
assert.deepStrictEqual(options["--"], ["test1", "test2"]);
90+
return Promise.resolve(true);
91+
}),
92+
commandInfo: nestedOptions,
93+
}),
94+
}
95+
);
96+
97+
assert.isTrue(await command("nested", "--internal", "--", "test1", "test2"));
98+
});
7699
});
77100
});

0 commit comments

Comments
 (0)