Skip to content

Add an EnumerableFlag protocol#65

Merged
natecook1000 merged 9 commits into
masterfrom
nate/caseiterable_namespecifications
May 10, 2020
Merged

Add an EnumerableFlag protocol#65
natecook1000 merged 9 commits into
masterfrom
nate/caseiterable_namespecifications

Conversation

@natecook1000
Copy link
Copy Markdown
Member

@natecook1000 natecook1000 commented Mar 5, 2020

Description

This addresses the need for providing name specifications for enum flags, since property wrappers can't be used for enumeration cases.

Detailed Design

This includes a new EnumerableFlag protocol:

/// A type that represents the different possible flags to be used by a
/// `@Flag` property.
public protocol EnumerableFlag: CaseIterable {
  /// Returns the name specification to use for the given flag.
  static func name(for value: Self) -> NameSpecification

  /// Returns the help information to show for the given flag.
  static func help(for value: Self) -> ArgumentHelp?
}

As well as new EnumerableFlag-constrained @Flag initializers that replace the current ones that are constrained to String/CaseIterable.

Documentation Plan

Wrote type- and symbol-level documentation for EnumerableFlag, revised docs for the new initializers, and updated the "Argument, Options, and Flags" guide.

Test Plan

Modified unit tests to use the new protocol and added tests that use the original, deprecated versions.

Source Impact

This deprecates the @Flag initializers that are constrained to CaseIterable/RawRepresentable and RawValue == String. These initializers will continue to work, and can be removed in a future version.

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

Comment thread Sources/ArgumentParser/Parsable Properties/Flag.swift
This addresses the need for providing name specifications for enum
flags, since property wrappers can't be used for enum cases.
@natecook1000 natecook1000 force-pushed the nate/caseiterable_namespecifications branch from b73d66d to 6576bbc Compare April 9, 2020 23:14
@natecook1000 natecook1000 marked this pull request as ready for review May 4, 2020 22:53
@natecook1000
Copy link
Copy Markdown
Member Author

@swift-ci Please test

Comment thread Documentation/02 Arguments, Options, and Flags.md Outdated
Co-authored-by: Kyle Macomber <kmacomber@apple.com>
@kylemacomber
Copy link
Copy Markdown

At first I thought the Array case was counter intuitive:

enum Color: EnumerableFlag {
    case pink, purple, silver
}

struct Example: ParsableCommand {
   @Flag() var colors: [Color]
   
   func run() throws {
       print(colors)
   }
}
% example --pink --silver
[.pink, .silver]

... and was going to suggest it be broken out into a separate property wrapper, but I see the synergy with the Int use case.

@kylemacomber
Copy link
Copy Markdown

Seems like we could accept any RangeReplaceableCollection. And also maybe SetAlgebra? It's a little weird to conform an OptionSet to CaseIterable, but that's a more efficient storage representation for your Color example.

@natecook1000
Copy link
Copy Markdown
Member Author

Seems like we could accept any RangeReplaceableCollection. And also maybe SetAlgebra? It's a little weird to conform an OptionSet to CaseIterable, but that's a more efficient storage representation for your Color example.

We could definitely make these generic over RangeReplaceableCollection, though we'd want to do that across the board (i.e. for @Option and @Argument types, too). SetAlgebra would require another set of overloads, since we don't have an abstraction that ties the two "create an empty one and then add things to it" protocols.

@natecook1000
Copy link
Copy Markdown
Member Author

@swift-ci Please test

Copy link
Copy Markdown

@kylemacomber kylemacomber left a comment

Choose a reason for hiding this comment

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

LGTM!

@natecook1000 natecook1000 merged commit e870503 into master May 10, 2020
@natecook1000 natecook1000 deleted the nate/caseiterable_namespecifications branch May 10, 2020 22:08
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.

3 participants