-
Couldn't load subscription status.
- Fork 32
Streamline argument delegation between parsers #100
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
Conversation
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.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| if scan.Subcommand != nil && token.Index == scan.Subcommand.Token.Index { | ||
| debugPrint("Descending into positional subcommand", scan.Subcommand.Command.Name, "at relativeDepth", scan.Subcommand.RelativeDepth) | ||
| childArgs := append([]string{}, scan.ForwardArgs...) | ||
| childArgs = append(childArgs, args[token.Index+1:]...) | ||
| return scan.Subcommand.Command.parse(p, childArgs) | ||
| } |
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.
[P1] Handle -h before subcommand without treating it as unknown
The new parser descends into a matching subcommand before checking the HelpRequested flag, so an invocation like ParseArgs([]string{"-h", "child"}) now returns an "Unknown arguments supplied: h" error instead of honoring the help request. Because the early return at this block hands control to the child parser, the later help check around lines 305‑307 is never reached and the -h token is not recorded as consumed, triggering the unknown‑argument fallback. This is a regression from the previous implementation which exited cleanly with help output.
Useful? React with 👍 / 👎.
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.
@codex Add a test for proper -h flag handling at both the root parser and within subcommands parsers. Ensure also that the -h and -help and --h and --help flags all work in all situations. Verify that all 4 help flags are not parsed when passed as part of trailing arguments.
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.
Summary
- Added regression coverage ensuring
-h,-help,--h, and--helpwork at the root parser, inside subcommands, and when placed after--so trailing arguments remain intact. parser_refactor_regression_test.goL243-L315 - Prevented subcommand delegation whenever a help flag is detected so the current parser shows help before descending, while still forwarding remaining arguments when no help was requested. subCommand.goL238-L311
Testing
- ✅
go test ./...
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.
@codex Please review this code for potential breaking issues as part of the pill request review process.
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.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
Summary
parseAllFlagsFromArgswith a documented struct that tracks positionals, forwarded tokens, and subcommand matchesTesting
https://chatgpt.com/codex/tasks/task_e_68cc2dd50c7c8323b2c63827a48fc9fc