fix: case-insensitive arguments.#42
Conversation
- Normalize arguments to lowercase before comparison so that --codec-CPU etc. will still be accepted as valid flags instead of being silenlty ignored.
📝 WalkthroughWalkthroughCommand-line parsing in ChangesCLI option parsing
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Warning |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main.cpp`:
- Around line 158-164: Restrict the lowercase transformation in the
argument-normalization block to dash-prefixed flags longer than two characters,
preserving case for short options such as -M, -H, and -P. Keep the existing
long-option matching flow unchanged and ensure comparisons against uppercase
short flags remain reachable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
|
|
||
| // Normalize flag arguments to lowercase for case-insensitive matching | ||
| std::string arg_lower = arg; | ||
| if (!arg.empty() && arg[0] == '-') { | ||
| std::transform(arg_lower.begin(), arg_lower.end(), arg_lower.begin(), | ||
| [](unsigned char c) { return static_cast<char>(std::tolower(c)); }); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Case-insensitive short options create collisions and dead code.
Lowercasing all dash-prefixed arguments introduces critical logic errors for short options that rely on case sensitivity:
-M(Metal) is lowercased to-m, colliding with--modelat line 166.-H(Host) is lowercased to-h, colliding with--helpat line 234.-P(Port) is lowercased to-p, which skips the condition chain entirely.
Additionally, comparisons like arg_lower == "-M" and arg_lower == "-H" become dead code because arg_lower will never contain uppercase letters after the transformation.
To fix this while keeping long options case-insensitive, restrict the lowercase transformation to flags longer than 2 characters. This correctly preserves standard short flags like -M, -H, and -P.
🐛 Proposed fix
- if (!arg.empty() && arg[0] == '-') {
+ if (arg.length() > 2 && arg[0] == '-') {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Normalize flag arguments to lowercase for case-insensitive matching | |
| std::string arg_lower = arg; | |
| if (!arg.empty() && arg[0] == '-') { | |
| std::transform(arg_lower.begin(), arg_lower.end(), arg_lower.begin(), | |
| [](unsigned char c) { return static_cast<char>(std::tolower(c)); }); | |
| } | |
| // Normalize flag arguments to lowercase for case-insensitive matching | |
| std::string arg_lower = arg; | |
| if (arg.length() > 2 && arg[0] == '-') { | |
| std::transform(arg_lower.begin(), arg_lower.end(), arg_lower.begin(), | |
| [](unsigned char c) { return static_cast<char>(std::tolower(c)); }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main.cpp` around lines 158 - 164, Restrict the lowercase transformation
in the argument-normalization block to dash-prefixed flags longer than two
characters, preserving case for short options such as -M, -H, and -P. Keep the
existing long-option matching flow unchanged and ensure comparisons against
uppercase short flags remain reachable.
Summary by CodeRabbit
--Log-Levelare accepted consistently.