revert: undo compact single-dash flag change from #21#22
revert: undo compact single-dash flag change from #21#22root-Manas wants to merge 1 commit intomainfrom
Conversation
|
Closing per maintainer request; keeping compact flag changes from #21. PDTM PATH fix is local WSL config, not repo content. |
There was a problem hiding this comment.
Pull request overview
Reverts PR #21’s compact single-dash/3-letter CLI flag model, restoring the prior long-flag-centered behavior and simplifying argument normalization.
Changes:
- Removed compact-flag normalization and related tests.
- Switched pflag registrations back to long flags with conventional single-letter shorthands (where applicable).
- Updated help/guide text and command normalization to use long flags (e.g.,
--scan,--setup).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cmd/macaron/main.go | Removes compact-flag machinery and restores long/shorthand flag definitions + updated normalization/help text. |
| cmd/macaron/main_test.go | Updates tests to reflect restored long-flag behavior and drops compact-flag test cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if strings.HasPrefix(tok, "-") { | ||
| args = append(args, tok) | ||
| continue | ||
| } | ||
| args = append(args, "--scn", tok) | ||
| args = append(args, "--scan", tok) |
There was a problem hiding this comment.
In the "scan" branch, the normalization loop treats any token not starting with "-" as a scan target and rewrites it to "--scan ". This breaks common invocations where a flag takes a separate value (e.g. macaron scan example.com --mode fast will rewrite fast into another --scan target). Consider removing this token-rewrite approach and instead: (1) drop the "scan" command token, (2) let pflag parse the remaining flags normally, and (3) treat leftover positional args as scan targets after parsing (or only rewrite truly positional args while tracking which flags consume a following value).
| withArgs([]string{"macaron", "-setup"}, func() { | ||
| normalizeLegacyArgs() | ||
| if osArgs()[1] != "-stp" { | ||
| t.Fatalf("expected -stp, got %s", osArgs()[1]) | ||
| if osArgs()[1] != "--setup" { | ||
| t.Fatalf("expected --setup, got %s", osArgs()[1]) | ||
| } |
There was a problem hiding this comment.
normalizeLegacyArgs() now also rewrites -install-tools to --install-tools, but the tests only cover the -setup case. Please add a test case for the -install-tools legacy form so this rollback behavior stays protected.
Reverts merged PR #21 to restore previous CLI flag behavior. This rollback is requested by maintainer.