Conversation
peter-jerry-ye
force-pushed
the
zihang/argparse
branch
2 times, most recently
from
6280975 to
2ede161
Compare
Pull Request Test Coverage Report for Build 2640Details
💛 - Coveralls |
peter-jerry-ye
force-pushed
the
zihang/argparse
branch
3 times, most recently
from
b62c1c9 to
b286791
Compare
peter-jerry-ye
force-pushed
the
zihang/argparse
branch
from
7e43c91 to
b59017e
Compare
![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: b59017e8e6
ℹ️ 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".
| let parent_count = parent.counts.get(name).unwrap_or(0) | ||
| let child_count = child.counts.get(name).unwrap_or(0) | ||
| let total = parent_count + child_count | ||
| parent.counts[name] = total | ||
| parent.flags[name] = total > 0 |
There was a problem hiding this comment.
When a global Count flag is set before a subcommand and negated after it (for example --verbose run --no-verbose), the child parse records a reset (count=0), but this merge branch always adds parent and child counts. That turns an explicit later negation into a non-zero total and reports the flag as enabled, which is the opposite of in-command --no-... behavior; the child argv reset needs to override additive merging.
Useful? React with 👍 / 👎.
| let choose_child = has_child && | ||
| (!has_parent || prefer_child_source(parent_source, child_source)) | ||
| if choose_child { |
There was a problem hiding this comment.
This path lets a child argv value replace a parent argv value for non-multi global options, so Set options passed both before and after a subcommand (e.g. --mode a run --mode b) are accepted instead of raising the duplicate-use error enforced elsewhere. That makes duplicate validation depend on token placement rather than argument definition and bypasses check_duplicate_set_occurrence for global options.
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
3 participants
This PR adds an arg parser that is inspired by clap but with reduced API because we want to focus on the clarity of the behavior. A lot of extra checks were imposed.
The API of this PR is roughly determined, but the internal implementation can still be improved drastically.
@FlyCloudC Do you think you would like to give it a review in terms of the API? (I know the internal implementation is a bit messy).