Skip to content

Commit 0ed82fb

Browse files
committed
fix: --help エラー処理のレビュー指摘対応
- operand 判定を cmd.args 先頭トークンに限定し、未知オプションの値を サブコマンド名と誤認しないようにする (--bogus json --help 等)。 副作用として --help が先行する場合 (geonic --help hello) は 従来どおりヘルプ表示に戻る - エラーメッセージはユーザーが入力したエイリアス (models, batch) を そのまま echo し、正規名に置換しない (rawArgs から復元) - unknown-command エラー出力を unknownCommandError() に統合し、 showHelp との重複を解消 - テストの spy ボイラープレートを runExpectingError/runExpectingHelp に 集約し、既存テストも移行
1 parent b14321c commit 0ed82fb

2 files changed

Lines changed: 91 additions & 53 deletions

File tree

src/commands/help.ts

Lines changed: 42 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,12 @@ export function formatCommandDetails(
220220
return lines.join("\n");
221221
}
222222

223+
function unknownCommandError(attempted: string): never {
224+
console.error(chalk.red(`Error: '${attempted}' is not a geonic command.`));
225+
console.error(`\nSee 'geonic help' for available commands.`);
226+
process.exit(1);
227+
}
228+
223229
function showHelp(program: Command, args: string[]): void {
224230
if (args.length === 0) {
225231
console.log(formatTopLevelHelp(program));
@@ -232,12 +238,7 @@ function showHelp(program: Command, args: string[]): void {
232238
for (let i = 0; i < args.length; i++) {
233239
const found = findCommand(current, args[i]);
234240
if (!found) {
235-
const attempted = args.slice(0, i + 1).join(" ");
236-
console.error(
237-
chalk.red(`Error: '${attempted}' is not a geonic command.`),
238-
);
239-
console.error(`\nSee 'geonic help' for available commands.`);
240-
process.exit(1);
241+
unknownCommandError(args.slice(0, i + 1).join(" "));
241242
}
242243
resolved.push(found);
243244
current = found;
@@ -249,13 +250,31 @@ function showHelp(program: Command, args: string[]): void {
249250
console.log(formatCommandDetails(program, target, path));
250251
}
251252

252-
function reportUnknownCommand(cmd: Command, operand: string): never {
253-
// getCommandPath() starts with the program name ("geonic"); drop it
254-
const prefix = getCommandPath(cmd).split(" ").slice(1);
255-
const attempted = [...prefix, operand].join(" ");
256-
console.error(chalk.red(`Error: '${attempted}' is not a geonic command.`));
257-
console.error(`\nSee 'geonic help' for available commands.`);
258-
process.exit(1);
253+
// Resolve the chain from the program down to cmd back to the tokens the user
254+
// actually typed (rawArgs), so aliases are echoed as typed (e.g. "models"),
255+
// not as their canonical name ("custom-data-models").
256+
function typedCommandPath(cmd: Command): string[] {
257+
const chain: Command[] = [];
258+
let current: Command | null = cmd;
259+
while (current && current.parent) {
260+
chain.unshift(current);
261+
current = current.parent;
262+
}
263+
264+
// rawArgs exists at runtime but is missing from commander's typings
265+
const root = getRootProgram(cmd) as Command & { rawArgs: string[] };
266+
const rawArgs = root.rawArgs.slice(2); // drop node + script
267+
268+
const typed: string[] = [];
269+
let i = 0;
270+
for (const c of chain) {
271+
const names = [c.name(), ...c.aliases()];
272+
while (i < rawArgs.length && !names.includes(rawArgs[i])) {
273+
i++;
274+
}
275+
typed.push(i < rawArgs.length ? rawArgs[i] : c.name());
276+
}
277+
return typed;
259278
}
260279

261280
export function enforceKnownCommandHelp(program: Command): void {
@@ -270,10 +289,16 @@ export function enforceKnownCommandHelp(program: Command): void {
270289
cmd.outputHelp = (
271290
contextOptions?: HelpContext | ((str: string) => string),
272291
): void => {
273-
// cmd.args is operands followed by unparsed options such as --help
274-
const operand = cmd.args.find((arg) => !arg.startsWith("-"));
275-
if (operand !== undefined && !findCommand(cmd, operand)) {
276-
reportUnknownCommand(cmd, operand);
292+
// cmd.args lists operands first, then unparsed options such as --help.
293+
// Only a leading non-dash token is a genuine operand — a non-dash token
294+
// found later may be the value of an unrecognized option.
295+
const operand = cmd.args[0];
296+
if (
297+
operand !== undefined &&
298+
!operand.startsWith("-") &&
299+
!findCommand(cmd, operand)
300+
) {
301+
unknownCommandError([...typedCommandPath(cmd), operand].join(" "));
277302
}
278303
originalOutputHelp(contextOptions as HelpContext | undefined);
279304
};

tests/help.test.ts

Lines changed: 49 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,27 @@ function findCommand(program: ReturnType<typeof createProgram>, ...names: string
1616
return current;
1717
}
1818

19+
// Parses argv expecting the unknown-command error path: captures stderr,
20+
// asserts process.exit(1), and returns the stripped stderr output.
21+
async function runExpectingError(argv: string[]): Promise<string> {
22+
const prog = createProgram();
23+
prog.exitOverride();
24+
const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
25+
const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => {
26+
throw new Error("process.exit");
27+
});
28+
try {
29+
await prog.parseAsync(["node", "geonic", ...argv]);
30+
} catch {
31+
// expected
32+
}
33+
const output = stripAnsi(errSpy.mock.calls.map((c) => c[0]).join("\n"));
34+
expect(exitSpy).toHaveBeenCalledWith(1);
35+
errSpy.mockRestore();
36+
exitSpy.mockRestore();
37+
return output;
38+
}
39+
1940
describe("help", () => {
2041
const program = createProgram();
2142

@@ -609,21 +630,8 @@ describe("help", () => {
609630
});
610631

611632
it("shows error for invalid command name in help", async () => {
612-
const prog = createProgram();
613-
prog.exitOverride();
614-
const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
615-
const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => {
616-
throw new Error("process.exit");
617-
});
618-
try {
619-
await prog.parseAsync(["node", "geonic", "help", "nonexistent"]);
620-
} catch {
621-
// expected
622-
}
623-
const output = errSpy.mock.calls.map((c) => c[0]).join("\n");
624-
expect(stripAnsi(output)).toContain("'nonexistent' is not a geonic command");
625-
errSpy.mockRestore();
626-
exitSpy.mockRestore();
633+
const output = await runExpectingError(["help", "nonexistent"]);
634+
expect(output).toContain("'nonexistent' is not a geonic command");
627635
});
628636

629637
it("shows top-level help when help is called with no args", async () => {
@@ -684,22 +692,22 @@ describe("help", () => {
684692
});
685693

686694
describe("unknown subcommand with --help", () => {
687-
async function runExpectingError(argv: string[]): Promise<string> {
695+
// Parses argv expecting help to be shown: captures stdout and returns it,
696+
// asserting no error was printed.
697+
async function runExpectingHelp(argv: string[]): Promise<string> {
688698
const prog = createProgram();
689699
prog.exitOverride();
700+
const writeSpy = vi.spyOn(process.stdout, "write").mockImplementation(() => true);
690701
const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
691-
const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => {
692-
throw new Error("process.exit");
693-
});
694702
try {
695703
await prog.parseAsync(["node", "geonic", ...argv]);
696704
} catch {
697-
// expected
705+
// exitOverride throws on help
698706
}
699-
const output = stripAnsi(errSpy.mock.calls.map((c) => c[0]).join("\n"));
700-
expect(exitSpy).toHaveBeenCalledWith(1);
707+
const output = stripAnsi(writeSpy.mock.calls.map((c) => c[0]).join(""));
708+
expect(errSpy).not.toHaveBeenCalled();
709+
writeSpy.mockRestore();
701710
errSpy.mockRestore();
702-
exitSpy.mockRestore();
703711
return output;
704712
}
705713

@@ -720,20 +728,25 @@ describe("help", () => {
720728
});
721729

722730
it("still shows help for a leaf command with an argument and --help", async () => {
723-
const prog = createProgram();
724-
prog.exitOverride();
725-
const writeSpy = vi.spyOn(process.stdout, "write").mockImplementation(() => true);
726-
const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
727-
try {
728-
await prog.parseAsync(["node", "geonic", "entities", "get", "urn:x", "--help"]);
729-
} catch {
730-
// exitOverride throws on help
731-
}
732-
const output = stripAnsi(writeSpy.mock.calls.map((c) => c[0]).join(""));
731+
const output = await runExpectingHelp(["entities", "get", "urn:x", "--help"]);
733732
expect(output).toContain("geonic entities get");
734-
expect(errSpy).not.toHaveBeenCalled();
735-
writeSpy.mockRestore();
736-
errSpy.mockRestore();
733+
});
734+
735+
it("echoes the alias the user typed, not the canonical command name", async () => {
736+
const output = await runExpectingError(["models", "badsub", "--help"]);
737+
expect(output).toContain("'models badsub' is not a geonic command");
738+
expect(output).not.toContain("custom-data-models");
739+
});
740+
741+
it("does not mistake an unknown option's value for a subcommand", async () => {
742+
// "--bogus json" must not be reported as `geonic entities json`
743+
const output = await runExpectingHelp(["entities", "--bogus", "json", "--help"]);
744+
expect(output).toContain("geonic entities");
745+
});
746+
747+
it("shows top-level help when --help precedes a non-command token", async () => {
748+
const output = await runExpectingHelp(["--help", "hello"]);
749+
expect(output).toContain("AVAILABLE COMMANDS");
737750
});
738751
});
739752

0 commit comments

Comments
 (0)