fix: preserve dash-prefixed tokens in DefaultCommand fallback#31
Conversation
When the default-command fallback re-parsed [DefaultCommand, ..args], dash-prefixed tokens that weren't declared options still failed with 'Unknown option' — e.g. 'app --literal' or 'app Alice --suffix' — contradicting the DefaultCommand contract that unrecognized options are retried as args. Add ArgParser.Parse(args, command, unknownOptionsAsArguments) which passes unknown dash-prefixed tokens through verbatim as positional arguments while recognized framework/global/command options still parse normally. The fallback path in CliHost.RunWithDefaultCommandAsync opts in; the default parse behavior is unchanged. Commands that reject positional args still produce the usual usage error. Test-first: added failing integration tests (greet host: '--literal', 'Alice --suffix', and 'Alice --formal' for the recognized-option case) and ArgParser unit tests covering the new mode, default-mode rejection, and the no-positionals guard, then implemented the fix. Spec updated per constitution C2. Fixes #30 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2bcafeb8a2
ℹ️ 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
- 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 address that feedback".
| /// through verbatim as positional arguments instead of failing the parse. Used by the | ||
| /// default-command fallback so original tokens reach the default command (issue #30). | ||
| /// </param> | ||
| public ParseResult Parse (string[] args, ICliCommand? command = null, bool unknownOptionsAsArguments = false) |
There was a problem hiding this comment.
Because optional parameters are baked in at compile time, changing the existing public ArgParser.Parse(string[], ICliCommand?) method to a three-argument signature removes the old CLR method. Any already-compiled consumer that calls Parse(args, command) against a newer Terminal.Gui.Cli assembly will hit MissingMethodException unless it is recompiled; keeping the old overload and delegating to the new implementation preserves compatibility while still enabling the fallback behavior.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 27d38ab. Restored the original Parse(string[], ICliCommand?) overload delegating to the new three-argument implementation, so already-compiled consumers keep binding to the old CLR method.
Generated by Claude Code
…bility Optional parameters bind at compile time, so widening the existing public Parse(string[], ICliCommand?) to three parameters removed the old CLR method and would throw MissingMethodException for already-compiled consumers. Keep the original overload delegating to the new three-argument implementation. Addresses Codex P2 review feedback on #31. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Summary
Fixes #30 (Codex P2 from #28's review): when the
DefaultCommandfallback re-parsed[DefaultCommand, ..args], dash-prefixed tokens that weren't declared options still failed withUnknown option— soapp --literalorapp Alice --suffixreturned a usage error even though theDefaultCommandcontract says unrecognized options are retried as args to the default command.Approach (test-first)
["--literal"]and["Alice", "--suffix"](both failed with exit 64 /Unknown option), plus["Alice", "--formal"]to pin the recognized-option behavior. AddedArgParserunit tests for the new mode, default-mode rejection, and the no-positionals guard.ArgParser.Parsegains an optionalunknownOptionsAsArgumentsparameter: dash-prefixed tokens matching no framework, global, or command option pass through verbatim as positional arguments. OnlyCliHost.RunWithDefaultCommandAsyncopts in — default parsing is unchanged, and recognized options still parse as options in the fallback.Commands that don't accept positional args still get the usual
does not accept positional argumentsusage error, so nothing is silently swallowed.Spec (constitution C2)
specs/library-spec.mddefault-command-dispatch section updated to document the pass-through semantics.Verification
jb cleanupcode+dotnet format→ clean diff🤖 Generated with Claude Code