Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Remove ExpressibleByArgument conformance from Optional #173

Merged
merged 6 commits into from
Jun 2, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Remove ExpressibleByArgument conformance for Optional
It turns out that the conditional conformance for Optional was a bad idea, and
it should be handled more like Array, with specific initializers for the Optional
case. Primarily, this is because providing a default value for an optional property
doesn't make sense -- the default is already nil, and a non-nil default means that
the property will never be nil and therefore shouldn't be optional.
  • Loading branch information
natecook1000 committed May 30, 2020
commit a933d934e6f483f9445077db3a4d5929c7e60a9f
43 changes: 43 additions & 0 deletions Sources/ArgumentParser/Parsable Properties/Argument.swift
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,49 @@ public enum ArgumentArrayParsingStrategy {
}

extension Argument {
/// Creates an optional property that reads its value from an argument.
///
/// The argument is optional for the caller of the command and defaults to `nil`.
///
/// - Parameters:
/// - initial: A default value to use for this property.
/// - help: Information about how to use this argument.
public init<T: ExpressibleByArgument>(
help: ArgumentHelp? = nil
) where Value == T? {
self.init(_parsedValue: .init { key in
ArgumentSet(
key: key,
kind: .positional,
parsingStrategy: .nextAsValue,
parseType: T.self,
name: .long,
default: nil,
help: help)
})
}

@available(*, deprecated, message: """
Default values don't make sense for optional properties.
Remove the 'default' parameter if its value is nil,
or make your property non-optional if it's non-nil.
""")
public init<T: ExpressibleByArgument>(
default initial: T?,
help: ArgumentHelp? = nil
) where Value == T? {
self.init(_parsedValue: .init { key in
ArgumentSet(
key: key,
kind: .positional,
parsingStrategy: .nextAsValue,
parseType: T.self,
name: .long,
default: initial,
help: help)
})
}

/// Creates a property that reads its value from an argument, parsing with
/// the given closure.
///
Expand Down
50 changes: 50 additions & 0 deletions Sources/ArgumentParser/Parsable Properties/Option.swift
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,56 @@ public enum ArrayParsingStrategy {
}

extension Option {
/// Creates a property that reads its value from a labeled option.
///
/// If the property has an `Optional` type, or you provide a non-`nil`
/// value for the `initial` parameter, specifying this option is not
/// required.
///
/// - Parameters:
/// - name: A specification for what names are allowed for this flag.
/// - initial: A default value to use for this property.
/// - help: Information about how to use this option.
public init<T: ExpressibleByArgument>(
name: NameSpecification = .long,
parsing parsingStrategy: SingleValueParsingStrategy = .next,
help: ArgumentHelp? = nil
) where Value == T? {
self.init(_parsedValue: .init { key in
var arg = ArgumentDefinition(
key: key,
kind: .name(key: key, specification: name),
parsingStrategy: ArgumentDefinition.ParsingStrategy(parsingStrategy),
parser: T.init(argument:),
default: nil)
arg.help.help = help
return ArgumentSet(arg.optional)
})
}

@available(*, deprecated, message: """
Default values don't make sense for optional properties.
Remove the 'default' parameter if its value is nil,
or make your property non-optional if it's non-nil.
""")
public init<T: ExpressibleByArgument>(
name: NameSpecification = .long,
default initial: T?,
parsing parsingStrategy: SingleValueParsingStrategy = .next,
help: ArgumentHelp? = nil
) where Value == T? {
self.init(_parsedValue: .init { key in
ArgumentSet.init(
key: key,
kind: .name(key: key, specification: name),
parsingStrategy: ArgumentDefinition.ParsingStrategy(parsingStrategy),
parseType: T.self,
name: name,
default: initial,
help: help)
})
}

/// Creates a property that reads its value from a labeled option, parsing
/// with the given closure.
///
Expand Down
17 changes: 0 additions & 17 deletions Sources/ArgumentParser/Parsable Types/ExpressibleByArgument.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,23 +33,6 @@ extension String: ExpressibleByArgument {
}
}

extension Optional: ExpressibleByArgument where Wrapped: ExpressibleByArgument {
public init?(argument: String) {
if let value = Wrapped(argument: argument) {
self = value
} else {
return nil
}
}

public var defaultValueDescription: String {
guard let value = self else {
return "none"
}
return "\(value)"
}
}

extension RawRepresentable where Self: ExpressibleByArgument, RawValue: ExpressibleByArgument {
public init?(argument: String) {
if let value = RawValue(argument: argument) {
Expand Down
1 change: 1 addition & 0 deletions Sources/ArgumentParser/Parsing/ArgumentDefinition.swift
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ extension ArgumentDefinition: CustomDebugStringConvertible {
extension ArgumentDefinition {
var optional: ArgumentDefinition {
var result = self

result.help.options.insert(.isOptional)
return result
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/ArgumentParser/Parsing/ArgumentSet.swift
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ extension ArgumentSet {

extension ArgumentDefinition {
/// Create a unary / argument that parses using the given closure.
fileprivate init<A>(key: InputKey, kind: ArgumentDefinition.Kind, parsingStrategy: ParsingStrategy = .nextAsValue, parser: @escaping (String) -> A?, parseType type: A.Type = A.self, default initial: A?) {
init<A>(key: InputKey, kind: ArgumentDefinition.Kind, parsingStrategy: ParsingStrategy = .nextAsValue, parser: @escaping (String) -> A?, parseType type: A.Type = A.self, default initial: A?) {
let initialValueCreator: (InputOrigin, inout ParsedValues) throws -> Void
if let initialValue = initial {
initialValueCreator = { origin, values in
Expand Down