Skip to content

Conversation

@glessard
Copy link
Contributor

@glessard glessard commented Mar 5, 2020

Description

Constructors for Bool Flags with inverted pairs do not have a FlagExclusivity parameter (#60). This PR adds a FlagExclusivity parameter to two of the Flag constructors.

Detailed Design

On Flag,

public init(name: NameSpecification = .long, default initial: Bool? = false,
            inversion: FlagInversion, help: ArgumentHelp? = nil)

becomes

public init(name: NameSpecification = .long, default initial: Bool? = false,
            inversion: FlagInversion, exclusivity: FlagExclusivity = .chooseLast,
            help: ArgumentHelp? = nil)

In the process, the logic to handle flag exclusivity collision was refactored to a new static function on ArgumentSet, and is used for Bool flags with inversions as well as flags built from CaseIterable enums.

Documentation Plan

The new parameter has been documented in the doc comments.

Test Plan

New tests are added to FlagsEndToEndTests and to ErrorMessageTests

Source Impact

The parameter is additive with a default value, and therefore is source compatible.

Checklist

  • I've added at least one test that validates that my change is working, if appropriate
  • I've followed the code style of the rest of the project
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary

This resolves #60 and #61

@glessard glessard force-pushed the inversion-exclusivity-control branch from 01d0dbb to 7df1481 Compare March 5, 2020 22:11
@glessard
Copy link
Contributor Author

glessard commented Mar 5, 2020

There are 3 initializers that construct sets of flags that have a FlagExclusivity parameter. They now use all the same logic to handle collisions. This adds a resolution for #61.

@glessard glessard force-pushed the inversion-exclusivity-control branch 3 times, most recently from 7c627d8 to 403ebdf Compare March 5, 2020 23:57
@natecook1000
Copy link
Member

Thanks for implementing this, @glessard! 👏

Could you pick up the changes from #54, and adopt the template here for the PR? https://raw.githubusercontent.com/apple/swift-argument-parser/729232d90c77d2a84a229c50512afe823b0472d0/.github/PULL_REQUEST_TEMPLATE/NEW.md

@glessard glessard force-pushed the inversion-exclusivity-control branch from 403ebdf to c616875 Compare March 6, 2020 16:21
@glessard
Copy link
Contributor Author

glessard commented Mar 6, 2020

I added the parameter to @dduan's initializer from #54, and squashed the commits.

@natecook1000
Copy link
Member

LGTM! 🎉

@natecook1000 natecook1000 merged commit 3af071f into apple:master Mar 6, 2020
@glessard glessard deleted the inversion-exclusivity-control branch March 6, 2020 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Exclusivity isn't adjustable for Boolean flags with flag inversion settings

2 participants