Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Apply default options for a CLI parent command when a sub command is parsed #4593

Merged
merged 2 commits into from
Jan 20, 2025

Conversation

chalcolith
Copy link
Member

@chalcolith chalcolith commented Jan 20, 2025

Prior to this fix, parsing a sub-command proceeded before assigning defaults to the parent's options. This fix factors out the default processing so it can occur before parsing a sub command.

Fixes #4580

@chalcolith chalcolith added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label Jan 20, 2025
@chalcolith chalcolith requested a review from a team January 20, 2025 19:52
@ponylang-main
Copy link
Contributor

Hi @chalcolith,

The changelog - fixed label was added to this pull request; all PRs with a changelog label need to have release notes included as part of the PR. If you haven't added release notes already, please do.

Release notes are added by creating a uniquely named file in the .release-notes directory. We suggest you call the file 4593.md to match the number of this pull request.

The basic format of the release notes (using markdown) should be:

## Title

End user description of changes, why it's important,
problems it solves etc.

If a breaking change, make sure to include 1 or more
examples what code would look like prior to this change
and how to update it to work after this change.

Thanks.

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Jan 20, 2025
@SeanTAllen
Copy link
Member

@chalcolith this would close #4580, yes?

Copy link
Member

@SeanTAllen SeanTAllen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am far from an expert on this code, but if CI passes, I am good with saying "I approve".

@SeanTAllen SeanTAllen changed the title Correctly apply default options to cli parent commands when a sub-command is present Apply default options for a CLI parent command when a sub command is parsed Jan 20, 2025
@chalcolith
Copy link
Member Author

@chalcolith this would close #4580, yes?

Yes; I've updated the description.

@SeanTAllen SeanTAllen merged commit 8693c6d into main Jan 20, 2025
28 checks passed
@SeanTAllen SeanTAllen deleted the fix_4580_cli_defaults branch January 20, 2025 20:43
@ponylang-main ponylang-main removed the discuss during sync Should be discussed during an upcoming sync label Jan 20, 2025
github-actions bot pushed a commit that referenced this pull request Jan 20, 2025
github-actions bot pushed a commit that referenced this pull request Jan 20, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CommandParser ignores parent default options when parsing leaf command
3 participants