-
Notifications
You must be signed in to change notification settings - Fork 82
Fix -h and -help flags to show help instead of requiring arguments #683
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: main
Are you sure you want to change the base?
Conversation
Co-authored-by: dlevy-msft-sql <194277063+dlevy-msft-sql@users.noreply.github.com>
- Convert -h without an argument to -? to show help - Keep -h with a numeric argument for setting header count (e.g., -h 10, -h -1) - Convert -help to --help to show help - Add comprehensive tests for the new behavior Co-authored-by: dlevy-msft-sql <194277063+dlevy-msft-sql@users.noreply.github.com>
- Remove trailing whitespace from preprocessHelpFlags function - Restore NOTICE.md that was accidentally modified by build process Co-authored-by: dlevy-msft-sql <194277063+dlevy-msft-sql@users.noreply.github.com>
- Remove unnecessary blank lines in convertOsArgs and preprocessHelpFlags - Add test case for -h followed by non-numeric value Co-authored-by: dlevy-msft-sql <194277063+dlevy-msft-sql@users.noreply.github.com>
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.
Pull request overview
This PR fixes the behavior of the -h and -help flags to show help instead of requiring arguments, addressing issue #604 where sqlcmd -h returned a "Missing argument" error and sqlcmd -? requires quoting in zsh due to glob expansion.
Changes:
- Added
preprocessHelpFlags()function that converts-hto-?when not followed by a number, preserving backward compatibility with-h <number>for setting header count - Converts
-helpto--helpto avoid shell parsing issues (since-helpwould be parsed as-h elp) - Added comprehensive test coverage for the new conversion logic and end-to-end parsing behavior
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| cmd/sqlcmd/sqlcmd.go | Implements preprocessHelpFlags() function with logic to detect numeric vs non-numeric arguments following -h, and integrates it into the argument preprocessing pipeline |
| cmd/sqlcmd/sqlcmd_test.go | Adds 10 new test cases covering conversion logic (7 tests in TestConvertOsArgs) and end-to-end parsing behavior (3 tests in TestValidCommandLineToArgsConversion) |
|
@copilot fix the review items |
When -h is followed by a non-numeric argument that is not a flag (e.g., 'abc'), consume it to prevent "Unknown command" errors. Flags are preserved for normal processing (e.g., -h -E converts to -? -E, not just -?). This addresses PR review feedback to improve user experience. Co-authored-by: dlevy-msft-sql <194277063+dlevy-msft-sql@users.noreply.github.com>
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.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Fix -h and -help flags to show help instead of requiring arguments
sqlcmd -hreturns "Missing argument" error because-hexpects an integer for header count. Additionally,-?requires quoting in zsh due to glob expansion, making help discovery difficult.Changes
Argument preprocessing in
cmd/sqlcmd/sqlcmd.go:preprocessHelpFlags()that converts-hto-?when not followed by a number-helpto--help(avoids shell parsing issues)-h <number>for backward compatibility with header count flag-hto prevent "Unknown command" errorsTest coverage in
cmd/sqlcmd/sqlcmd_test.go:Verification
✅ All tests pass
✅
sqlcmd -h abcnow shows help (instead of "Unknown command" error)✅
sqlcmd -h -Eshows help and preserves-Eflag✅ Backward compatibility maintained:
sqlcmd -h 10still sets header countOriginal prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.