-
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) |
|
|
||
| // If -h is followed by a number, keep it as-is (header count) | ||
| // Otherwise, convert to -? (show help) | ||
| if !nextIsNumber { | ||
| result = append(result, "-?") | ||
| } else { | ||
| result = append(result, arg) | ||
| } | ||
| continue | ||
| } | ||
|
|
Copilot
AI
Feb 1, 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.
When -h is followed by a non-numeric argument (e.g., sqlcmd -h abc), the conversion to -? preserves the non-numeric argument, which will later cause an "Unknown command" error (line 242-244) instead of showing help. Consider consuming/ignoring the non-numeric argument when converting -h to help mode, so that sqlcmd -h abc shows help rather than an error about "abc".
| // If -h is followed by a number, keep it as-is (header count) | |
| // Otherwise, convert to -? (show help) | |
| if !nextIsNumber { | |
| result = append(result, "-?") | |
| } else { | |
| result = append(result, arg) | |
| } | |
| continue | |
| } | |
| // If -h is followed by a number, keep it as-is (header count) | |
| // Otherwise, convert to -? (show help) and consume any non-numeric argument | |
| if !nextIsNumber { | |
| result = append(result, "-?") | |
| // If there was a following non-numeric argument, consume it | |
| if hasNextArg { | |
| i++ | |
| } | |
| } else { | |
| result = append(result, arg) | |
| } | |
| continue | |
| } |
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 flagTest coverage in
cmd/sqlcmd/sqlcmd_test.go:Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
learn.microsoft.com/home/REDACTED/work/_temp/ghcca-node/node/bin/node /home/REDACTED/work/_temp/ghcca-node/node/bin/node --enable-source-maps /home/REDACTED/work/_temp/copilot-developer-action-main/dist/index.js(dns block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.