Fix for @Option(transform:) with optional type
#619
Conversation
Due to the restructuring in #477, there was ambiguity between the unconstrained `@Option` initializer that uses a transform (but no initial value) and the one that is constrained to the property being optional. This marks the unconstrained version as disfavored, which allows overload resolution to select the optional version when appropriate. Fixes #618.
|
@swift-ci Please test |
|
Thank you Nate. |
|
@swift-ci Please test |
|
Question: to facilitate the understanding of the documentation, should we make such below changes too? Option.swift, Line 508, change: To: Option.swift, Line 577, change: To: |
|
@swift-ci Please test |
| /// @Option var name: String | ||
| /// ``` | ||
| /// | ||
| /// - Parameters: | ||
| /// - name: A specification for what names are allowed for this flag. | ||
| /// - parsingStrategy: The behavior to use when looking for this option's value. | ||
| /// - name: A specification for what names are allowed for this option. |
There was a problem hiding this comment.
The rename from foo to name may add to confusion, since there is also a name parameter in the initializer.
I suggested using title like in previous example.
| /// Creates a property with a default value provided by standard Swift default | ||
| /// value syntax, parsing with the given closure. | ||
| /// Creates an optional property that reads its value from a labeled option, | ||
| /// parsing with the given closure, with an explicit `nil` default. |
There was a problem hiding this comment.
In general, maybe yes. But it would also work with non-nil default, like:
var char: Character? = " "
The fact that it's a mutable optional would then allow it to eventually become nil later in time (when a program wants to free some memory for instance).
There was a problem hiding this comment.
That way of declaring (optional with a non-nil default) ends up using a different initializer that has a deprecation warning, since the behavior usually isn't what the author intends. I hadn't considered the use case you're describing here, but in general the ArgumentParser API is designed to essentially provide a way to write a function interface via the command line; the type declarations should match the CL interface.
| /// ```swift | ||
| /// @Option() | ||
| /// var foo: [String] | ||
| /// @Option var char: [Character] |
There was a problem hiding this comment.
nit, plural: char -> chars
Same thing on lines 748 and 796
|
@swift-ci Please test |
Due to the restructuring in #477, there was ambiguity between the unconstrained
@Optioninitializer that uses a transform (but no initial value) and the one that is constrained to the property being optional. This marks the unconstrained version as disfavored, which allows overload resolution to select the optional version when appropriate.Fixes #618.
Checklist