Add a parent command property wrapper to gain access to parent state #802
Conversation
|
@swift-ci please test |
|
I like this design, but with the generic property wrapper I'm not sure how |
The ParentCommand property wrapper constrains the Also, unlike the OptionGroup, the intent is to make it so that the options of the parent command aren't available as part of the child command's options. I believe that's the way that this patch should work. Maybe I'm missing something? |
…into parent_command_property_wrapper
|
@swift-ci please test |
|
@swift-ci please test |
…into parent_command_property_wrapper
…into parent_command_property_wrapper
|
@swift-ci please test |
…into parent_command_property_wrapper
|
@swift-ci please test |
There was a problem hiding this comment.
Thanks, @cmcgee1024! In addition to the notes here, could you include something in the tests that verifies that the parent command's contents aren't included in the child command's help screen?
| XCTAssertNotNil(toolInfo.command.arguments!.first { $0.valueName == "foo" }) | ||
| let childInfo = toolInfo.command.subcommands!.first { cmd in | ||
| cmd.commandName == "child" | ||
| } | ||
| // It's not there in the child subcommand | ||
| XCTAssertNil(childInfo!.arguments!.first { $0.valueName == "foo" }) |
There was a problem hiding this comment.
Could you leave out the force unwraps here? The output is a better when there's a test failure rather than a trap during the test.
| @propertyWrapper | ||
| public struct ParentCommand<Value: ParsableCommand>: Decodable, ParsedWrapper { | ||
| internal var _parsedValue: Parsed<Value> | ||
| internal var _visibility: ArgumentVisibility |
There was a problem hiding this comment.
This property isn't needed. The visibility is always effectively hidden for parent command references. It's removed now.
|
|
||
| /// A wrapper that adds a reference to a parent command. | ||
| /// | ||
| /// Use the parent command wrapper to gain access to a parent command's state. |
There was a problem hiding this comment.
I think it would be useful to compare/contrast with @OptionGroup here. Something like:
| /// Use the parent command wrapper to gain access to a parent command's state. | |
| /// Use the `@ParentCommand` wrapper to gain access to a parent command's state. | |
| /// The arguments, options, and flags in a `@ParentCommand` type are omitted from | |
| /// the help screen for the including child command, and only appear in the parent's | |
| /// help screen. To include the help in both screens, use the ``OptionGroup`` | |
| /// wrapper instead. |
There was a problem hiding this comment.
Thanks, I've incorporated this additional text in the latest commit.
Avoid using force unwrap in the test case Add more detail to the ParentCommand property wrapper documentation to distinguish from OptionGroup
I believe that the first section of the test case verifies the child command's help screen to assert that there aren't parent options showing up in there: https://github.com/apple/swift-argument-parser/pull/802/files#diff-4e092f6da2faedbd5525e77af586a9fce9327b926db3225813b72ea6119e06b2R143 |
Create a property wrapper called
ParentCommandthat allows a subcommand togain access to the state of one of its parent commands based on the type.
Checklist