-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Enhance CLI error handling with argument suggestions #1445
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
2a25a98
ad5461b
249b8fc
eea1fbe
634964e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,7 +1,7 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import json | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import sys | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from argparse import ArgumentParser | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import difflib | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import yaml | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from nettacker import all_module_severity_and_desc | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -383,7 +383,7 @@ def add_arguments(self): | |||||||||||||||||||||||||||||||||||||||||||||||||||||
| method_options.add_argument( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "--set-hardware-usage", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| action="store", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dest="set_hardware_usage", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| deimport difflibst="set_hardware_usage", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| default=Config.settings.set_hardware_usage, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| help=_("set_hardware_usage"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -518,7 +518,27 @@ def parse_arguments(self): | |||||||||||||||||||||||||||||||||||||||||||||||||||||
| all ARGS with applied rules | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Checking Requirements | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| options = self.api_arguments or self.parse_args() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if self.api_arguments: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| options = self.api_arguments | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| known_args, unknown_args = self.parse_known_args() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if unknown_args: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| valid_flags = [] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for action in self._actions: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| valid_flags.extend(action.option_strings) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for arg in unknown_args: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if arg.startswith("-") and len(arg) > 1: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| suggestion = difflib.get_close_matches(arg, valid_flags, n=1) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if suggestion: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print(f"Error: Unknown argument '{arg}'. Did you mean '{suggestion[0]}'?") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print(f"Error: Unknown argument '{arg}'. Did you mean '{suggestion[0]}'?") | |
| print(f"Error: Unknown argument '{arg}'. Did you mean '{suggestion[0]}'?") |
Copilot
AI
Mar 23, 2026
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.
If an unknown argument is positional (doesn’t start with -), the code still exits with status 1 but prints no error message. Ensure all entries in unknown_args are surfaced to the user (positional and flags), ideally in a single consolidated message.
| for arg in unknown_args: | |
| if arg.startswith("-") and len(arg) > 1: | |
| suggestion = difflib.get_close_matches(arg, valid_flags, n=1) | |
| if suggestion: | |
| print(f"Error: Unknown argument '{arg}'. Did you mean '{suggestion[0]}'?") | |
| else: | |
| print(f"Error: Unknown argument '{arg}'") | |
| error_messages = [] | |
| for arg in unknown_args: | |
| # Flag-like unknown argument (starts with "-" and not just "-") | |
| if arg.startswith("-") and len(arg) > 1: | |
| suggestion = difflib.get_close_matches(arg, valid_flags, n=1) | |
| if suggestion: | |
| error_messages.append( | |
| f" {arg} (did you mean '{suggestion[0]}'?)" | |
| ) | |
| else: | |
| error_messages.append(f" {arg}") | |
| # Positional unknown argument | |
| else: | |
| error_messages.append(f" {arg} (positional argument)") | |
| if error_messages: | |
| print( | |
| "Error: Unknown arguments:\n" + "\n".join(error_messages) | |
| ) |
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Mar 23, 2026
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.
This path uses print(...) + sys.exit(1) for error handling, which bypasses the project’s existing die_failure() behavior (logging + color reset) and prints to stdout instead of stderr. Consider using die_failure(...) (or ArgumentParser.error(...) if you want argparse-style usage output) and include the suggestion text in that message.
| for arg in unknown_args: | |
| if arg.startswith("-") and len(arg) > 1: | |
| suggestion = difflib.get_close_matches(arg, valid_flags, n=1) | |
| if suggestion: | |
| print(f"Error: Unknown argument '{arg}'. Did you mean '{suggestion[0]}'?") | |
| else: | |
| print(f"Error: Unknown argument '{arg}'") | |
| sys.exit(1) | |
| error_messages = [] | |
| for arg in unknown_args: | |
| if arg.startswith("-") and len(arg) > 1: | |
| suggestion = difflib.get_close_matches(arg, valid_flags, n=1) | |
| if suggestion: | |
| error_messages.append( | |
| f"Error: Unknown argument '{arg}'. Did you mean '{suggestion[0]}'?" | |
| ) | |
| else: | |
| error_messages.append(f"Error: Unknown argument '{arg}'") | |
| if error_messages: | |
| die_failure("\n".join(error_messages)) |
Copilot
AI
Mar 23, 2026
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.
New behavior around unknown-argument detection/suggestion is introduced here, but there are no existing tests covering ArgParser behavior. Please add pytest coverage for cases like an unknown flag with a close match (suggestion emitted) and an unknown positional argument (error message emitted).
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.
add_argumentfor--set-hardware-usagehas a typo in the keyword argument name (deimport difflibst) which will raise aTypeErrorat runtime and prevents the option from being parsed. This should bedest="set_hardware_usage".