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.

Sign up for GitHub

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

Jump to bottom

Provide Optional initial value for input property wrappers #556

Open
tarbaiev-smg opened this issue Feb 26, 2023 · 3 comments · May be fixed by #557
Open

Provide Optional initial value for input property wrappers #556

tarbaiev-smg opened this issue Feb 26, 2023 · 3 comments · May be fixed by #557

Comments

@tarbaiev-smg
Copy link

tarbaiev-smg commented Feb 26, 2023 •
edited
Loading

Currently it's only possible to use initial value if the property type is non-Optional.
I'd like to use environment variable as a fallback value for a command line argument (similar how it's done in Fastlane), but keep the argument required. To allow this I'd like to propose adding the initial argument to initializers of non-Optional type property wrappers which do not contain the wrappedValue argument.

For example adjusting the Option initializer:

  public init(
    name: NameSpecification = .long,
    parsing parsingStrategy: SingleValueParsingStrategy = .next,
    help: ArgumentHelp? = nil,
    completion: CompletionKind? = nil,
    initial: Value? = nil // <-- Using default value preserves backward compatibility
  ) {
    self.init(_parsedValue: .init { key in
      let arg = ArgumentDefinition(
        container: Bare<Value>.self,
        key: key,
        kind: .name(key: key, specification: name),
        help: help,
        parsingStrategy: parsingStrategy.base,
        initial: initial, // <-- injecting the initial value
        completion: completion)

      return ArgumentSet(arg)
    })

would allow us to use it as follows:

@Option(initial: ProcessInfo.processInfo["REQUIRED_OPTION"])
var requiredOption: String
@tarbaiev-smg tarbaiev-smg linked a pull request Feb 27, 2023 that will close this issue
Open
4 tasks
@natecook1000
Copy link
Member

Hi @tarbaiev-smg — thanks for the issue!

It seems like this overlaps a bit with #4 and #41, such that I don't think I want to land this fix quite yet. In particular, instead of just specifying a key for the environment, users may need to transform the value, which should be handled by the type's ExpressibleByArgument conformance or by the supplied transform closure. Adding this parameter would also make it hard to have multiple layers of fallback values; the command-line arg should take precedence, then anything in a config file, the environment, and finally we can prompt for more values using an upcoming interactive mode.

Would love to hear your thoughts about how this could fit in with those other requests!

@tarbaiev-smg
Copy link
Author

@natecook1000 Thanks for your notes. I had some doubts about the design. However also tried to keep the changes minimal.
I always appreciate flexibility and customization options in API's. And this solution allows anybody to adjust it for their needs, introducing as many fallback layers as they need. There the command-line arg does have precedence over the initial value.

As an example I was able to solve my needs by writing a simple extension to the Option and use it as follows:

    @Option(environmentKey: .exportPath)
    var exportPath: URL

where exportPath is an enum case with an environment variable name.

Likewise some out-of-the-box solutions (like the ones you mentioned) take a lot of effort and time to implement and in the end might still be too limited to fit the majority's needs, while the users might already benefit from a more low-level API.

So maybe the #4 might be built on top of this solution (especially if nobody has started working on it yet)?

I also wasn't sure about the transformation logic, and decided to match the behavior of wrappedValue, which has to be of the same type as the property, hence already transformed. but it wouldn't be hard to implement another initializer transforming the initial value, if needed.

@tarbaiev-smg
Copy link
Author

Hi @natecook1000,
It's been a year since I proposed this feature and not sure how to proceed with it.
The #4 and #41 you mentioned before have had no significant traction either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@natecook1000 @tarbaiev-smg