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

Default values #178

Closed
raysarebest opened this issue Feb 18, 2023 · 6 comments
Closed

Default values #178

raysarebest opened this issue Feb 18, 2023 · 6 comments
Labels
duplicate This issue or pull request already exists enhancement New feature or request

Comments

@raysarebest
Copy link

Is your feature request related to a problem? Please describe.

Parsing property wrappers like Argument, Option, and Flag don't have the ability to set a default value in the event the user doesn't pass a value for it at the command line. Currently, Argument will fatalError (effectively making all arguments required), Option will default to nil, and Flag will default to false. Allowing for a default value would enable cleaner code with less duplication since there would be less need for checking $isPresent and handling nil unnecessarily

For example, let's say I've defined an enumeration for handling conflicts between 2 values:

enum ConflictResolutionStrategy: String, LosslessStringConvertible {
    case error
    case keepExisting = "keep-existing"
    case overwrite

    init?(_ description: String) {
        self.init(rawValue: description)
    }

    var description: String {
        rawValue
    }
}

I'd like the user to be able to choose their preferred strategy, but I'd also like to have a sensible default in case they don't. Currently, my only option for using ConsoleKit to achieve something like this is to declare my property type as an Option, like this:

@Option(name: "conflict-strategy", help: "The method by which conflicts should be resolved")
var conflictStrategy: ConflictResolutionStrategy?

However, this forces me to declare my property as optional and handle the case where the user doesn't choose a strategy by checking for nil. This means that everywhere that I want to apply special handling based on the current conflictStrategy, I not only need to unwrap the optional, but I also need to choose a strategy to use every time in the event that the user didn't. This can quickly lead to fragmentation and inconsistency across the codebase around how conflicts should be handled by default, which could easily lead to different default behaviors in different places and a confusing user experience

Describe the solution you'd like

I'd like to assign a default value at the site where the property is defined with Swift's standard inline property initialization syntax, such as what's familiar from ArgumentParser's corresponding types or SwiftUI's wrappers like @State or @Published. For example:

@Option(name: "conflict-strategy", help: "The method by which conflicts should be resolved")
var conflictStrategy: ConflictResolutionStrategy = .error

This would allow the property type to be non-optional and have a default value chosen once and respected everywhere. This is possible by defining an initializer for the relevant property wrappers that accept an argument wrappedValue: WrappedValue as their first argument

Describe alternatives you've considered

Alternatively, the relevant property wrappers could take an extra parameter to their initializers, so a default value could be passed there. For example:

@Option(name: "conflict-strategy", help: "The method by which conflicts should be resolved", default: .error)
var conflictStrategy: ConflictResolutionStrategy

However, this is less obvious and discoverable in my opinion, since there's already well-established language syntax for handling this case

Additional context

N/A

@raysarebest raysarebest added the enhancement New feature or request label Feb 18, 2023
@0xTim
Copy link
Member

0xTim commented Feb 23, 2023

@raysarebest I agree passing some default values via the property wrapper in some format would be a really nice feature. For now, did you know you can do something like https://github.com/vapor/vapor/blob/main/Sources/Vapor/Commands/ServeCommand.swift#L53-L72

@raysarebest
Copy link
Author

raysarebest commented Feb 23, 2023

I did, and that's really clean for cases where you wanna do something based on the combined status of many different optionals/properties, but you're still choosing default handling at the call site instead of once when the user input is parsed. If you agree the feature's a good idea, I might open a PR for it this weekend sometime. I'm hoping it'll be relatively straightforward to implement

@0xTim
Copy link
Member

0xTim commented Feb 23, 2023

@raysarebest yep I think it's a good feature. Concur @gwynne ?

@gwynne
Copy link
Member

gwynne commented Feb 23, 2023

In theory, yes. In practice I would prefer to encourage adopting ArgumentParser itself, the results of which can then be shunted into ConsoleKit (until I or someone else gets around to formally supporting it and deprecating those parts of ConsoleKit outright).

@raysarebest
Copy link
Author

Alright, so you (@gwynne) would prefer if I didn't attempt a PR for this?

@gwynne
Copy link
Member

gwynne commented Aug 19, 2023

At this point I would tend to consider this superseded by (and thus technically a duplicate of) #180

@gwynne gwynne closed this as not planned Won't fix, can't repro, duplicate, stale Aug 19, 2023
@gwynne gwynne added the duplicate This issue or pull request already exists label Aug 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants