Skip to content

fix: bind inherited branch options for derived command settings#66

Open
bryansray wants to merge 1 commit intospectreconsole:mainfrom
bryansray:fix/issue-29-inherited-branch-options
Open

fix: bind inherited branch options for derived command settings#66
bryansray wants to merge 1 commit intospectreconsole:mainfrom
bryansray:fix/issue-29-inherited-branch-options

Conversation

@bryansray
Copy link
Copy Markdown

Fixes #29

  • I have read the Contribution Guidelines
  • I have checked that there isn't already another pull request that solves the above issue
  • All newly added code is adequately covered by tests
  • All existing tests are still running without errors

AI Disclosure

I used OpenAI Codex (GPT-5 based coding agent) to help implement this PR.
AI assistance was used to:

  • inspect the issue discussion and locate relevant code paths,
  • propose and apply code/test changes,
  • run and interpret test results.

I reviewed the changes and validated the final solution by running the full test suite.

Changes

This PR fixes inherited command options not being recognized for derived command settings under branches (issue #29), especially when the option is provided after the leaf command token.

Implementation

  • Updated option resolution in parser lookup to search the current command and its ancestors, instead of only the current command:
    • src/Spectre.Console.Cli/Internal/Parsing/CommandTreeExtensions.cs
  • This allows options defined on branch/base settings to be correctly matched and bound for derived leaf command settings.

Tests

  • Added a dedicated test fixture for branch inheritance settings:
    • src/Spectre.Console.Cli.Tests/Data/Settings/BranchInheritanceSettings.cs
  • Added regression tests in:
    • src/Spectre.Console.Cli.Tests/CommandAppTests.Branches.cs
  • New tests cover:
    1. Binding inherited branch option on derived command settings (--my-value).
    2. ValidateExamples() behavior for the same inherited option scenario.

Verification

  • Ran targeted branches tests (all TFMs): pass
  • Ran full test suite (all TFMs): pass (476/476 on net8.0, net9.0, net10.0)

@bryansray
Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

@vibingarthur
Copy link
Copy Markdown

Code Review

PR: #66 - fix: bind inherited branch options for derived command settings

Changes

  • Added test cases for option binding from branch settings
  • Fixed FindOption to traverse parent nodes when searching for options

Review

Correctness: The fix properly traverses the command tree parent chain to find inherited options
Tests: New test cases cover the scenario
Style: Clean implementation

No issues found. Good to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Command line options inherited from branch settings are not being bound.

3 participants