-
Notifications
You must be signed in to change notification settings - Fork 355
Augment Option to support default as flag option #830
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
base: main
Are you sure you want to change the base?
Conversation
d08351c to
61a6602
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- xctest instead of swift-testing to support swift 5.7
- synopsis:
[--log-level [log-level]]or[--log-level [<log-level>]] - dump-help snapshot tests
- completion snapshot tests
- manpage support
- manpage snapshot tests
- documentation
|
Would it make more sense instead to provide this functionality via @Option(parsing: .scanningForValue(default: "text"))
var showBinPath: String?You'd add/modify the following (I haven't checked the code for proper generics/types/compilation/keywords/etc.; it's just pseudo-code to illustrate the concept. I also haven't checked to see what problems adding the new public struct SingleValueParsingStrategy: Hashable {
…
// This is an addition
public static func scanningForValue<Value>(default: Value) -> SingleValueParsingStrategy {
self.init(base: .scanningForValue(default: default))
}
…
}
struct ArgumentDefinition {
…
enum ParsingStrategy {
…
// This is a modification
case scanningForValue(default: Any? = nil)
…
}
…
}One possible issue: must ensure that the option terminator cmd --show-bin-path -- json |
|
Thanks for the feedback @rgoldberg . What you proposed makes more sense than what I implemented. |
61a6602 to
b319a92
Compare
b319a92 to
2316d12
Compare
|
After discussing with @rauhul and @natecook1000 , we came up with the following API New initializers API usage looks as follows And the help dump looks like: |
2316d12 to
d3593e3
Compare
d3593e3 to
7f148ea
Compare
In some use cases, there is a need to have an option argument behave
like a flag.
This change introduced 4 new intialiazers to `Option` that accept a
`defaultAsFlag` value.
With the following usage:
```swift
struct Example: ParsableCommand {
@option(defaultAsFlag: "default", help: "Set output format.")
var format: String?
func run() {
print("Format: \(format ?? "none")")
}
}
```
`
The `defaultAsFlag` parameter creates a hybrid that supports both patterns:
- **Flag behavior**: `--format` (sets format to "default")
- **Option behavior**: `--format json` (sets format to "json")
- **No usage**: format remains `nil`
As a user of the command line tool, the `--help` output clearly distinguishes
between the the hybrid and regular usages.
```
OPTIONS:
--format [<format>] Set output format. (default as flag: json)
````
Note the `(default as flag: ...)` text instead of regular `(default: ...)`,
and the optional value syntax `[<value>]` instead of required `<value>`.
Fixes: #829
7f148ea to
b84708e
Compare
The changes requested have been taken into account in the most up to date change.
In some use cases, there is a need to have an option argument behave like a flag.
This change introduced 4 new intialiazers to
Optionthat accept adefaultAsFlagvalue.With the following usage:
`
The
defaultAsFlagparameter creates a hybrid that supports both patterns:--format(sets format to "default")--format json(sets format to "json")nilAs a user of the command line tool, the
--helpoutput clearly distinguishes between the the hybrid and regular usages.Note the
(default as flag: ...)text instead of regular(default: ...), and the optional value syntax[<value>]instead of required<value>.Fixes: #829
Checklist