-
-
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 all commits
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,3 +1,4 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import difflib | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import json | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import sys | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from argparse import ArgumentParser | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -518,7 +519,32 @@ 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]}'?", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| file=sys.stderr, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print(f"Error: Unknown argument '{arg}'", file=sys.stderr) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print(f"Error: Unexpected argument '{arg}'", file=sys.stderr) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+536
to
+543
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. Localize the new CLI error messages. These strings are hardcoded in English, so unknown-argument errors bypass the existing message catalog even though the CLI supports 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+532
to
+544
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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.
That's true but not for this PR.