-
Notifications
You must be signed in to change notification settings - Fork 5
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.
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
Add argument & command validation in scope of repo #234
Conversation
src/commander/commander.ts
Outdated
@@ -38,6 +39,38 @@ export function getCommanderFromConfiguration( | |||
let parsedCommand: ParseResults["parsedCommand"] = undefined; | |||
const helpStr = `Refer to [help docs](${docsPath}) and/or [source code](${config.pipelineScripts.repository}).`; | |||
|
|||
const exitOverride = (commandKey: string) => (e: CommanderError) => { | |||
if (e.code === "commander.excessArguments") { | |||
throw new Error(((e as CommanderError).message = `Unknown subcommand of "${commandKey}". ${helpStr}`)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not understanding the intentions here. Are we changing one error's message and throwing another new error?
Would it make sense to do instead:
throw new Error(`Unknown subcommand of "${commandKey}". ${helpStr}`);
or
(e as CommanderError).message = `Unknown subcommand of "${commandKey}". ${helpStr}`;
throw e;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, by the looks of it
throw new Error(((e as CommanderError).message = `Unknown subcommand of "${commandKey}". ${helpStr}`));
is equivalent to
throw new Error(`Unknown subcommand of "${commandKey}". ${helpStr}`);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
haha i just copied from the bottom one, and refactored in really ugly way not even thinking 😂😂
thanks, fixed!
Cancel cases | ||
*/ | ||
Cancel cases | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Automatic formatting with IDEA? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
most probably :)
src/commander/commander.ts
Outdated
@@ -38,6 +39,38 @@ export function getCommanderFromConfiguration( | |||
let parsedCommand: ParseResults["parsedCommand"] = undefined; | |||
const helpStr = `Refer to [help docs](${docsPath}) and/or [source code](${config.pipelineScripts.repository}).`; | |||
|
|||
const exitOverride = (commandKey: string) => (e: CommanderError) => { | |||
if (e.code === "commander.excessArguments") { | |||
throw new Error(((e as CommanderError).message = `Unknown subcommand of "${commandKey}". ${helpStr}`)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, by the looks of it
throw new Error(((e as CommanderError).message = `Unknown subcommand of "${commandKey}". ${helpStr}`));
is equivalent to
throw new Error(`Unknown subcommand of "${commandKey}". ${helpStr}`);
Co-authored-by: Yuri Volkov <[email protected]> Signed-off-by: Maksym H <[email protected]>
…and-bot into mak-fix-commands
Closes #233
Tested locally
Also added validation of commands & subcommands in scope of repo - will fail with related message