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

Optional arrays #317

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Optional arrays #317

wants to merge 11 commits into from

Conversation

swiftyfinch
Copy link

@swiftyfinch swiftyfinch commented May 20, 2021

It's the solution for issue #307
The author closed the issue, but I still need this one.

Checklist

  • I've added at least one test that validates that my change is working, if appropriate
  • I've followed the code style of the rest of the project
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary

Didn't find any documentation about using arrays in Option properties.

@swiftyfinch
Copy link
Author

@natecook1000 Hey, could you validate this draft?
It suits my command-line tool and now I can use it like this:

struct Rugby: ParsableCommand {
    @Option(parsing: .upToNextOption) var focus: [String]?
    
    func run() throws { print(focus) }
}
Command                         | Output
---------------------------------------------------------------------------------
rugby				| nil
rugby --focus			| []
rugby --focus Alamofire SnapKit | ["Alamofire", "SnapKit"]

@swiftyfinch
Copy link
Author

@swift-ci please test

@natecook1000
Copy link
Member

Thanks for this, @swiftyfinch — I'm not totally convinced that this is the right design to solve this issue, but it does seem sensible. Can you make sure the behavior makes sense / add tests for using this with the other parsing strategies? We'll also want an initializer that works with non-ExpressibleByArgument types.

@swiftyfinch
Copy link
Author

@natecook1000 Okey, I will try) How can I run checks with @swift-ci?

@natecook1000
Copy link
Member

@swiftyfinch I think only committers can kick off CI… Just let me know when you're ready for review!

@swiftyfinch
Copy link
Author

  • Finalize handling all parsing strategies
  • Add non-ExpressibleByArgument init
  • Add more tests for the most part of cases

@natecook1000 Ready for review

@natecook1000
Copy link
Member

@swift-ci Please test

@swiftyfinch
Copy link
Author

@natecook1000 I fixed compile error. Please, run tests again.

@kylemacomber
Copy link
Contributor

@swift-ci please test

@swiftyfinch
Copy link
Author

@natecook1000 @kylemacomber Hey, any updates here?

@swiftyfinch
Copy link
Author

Any updates?

@swiftyfinch
Copy link
Author

@natecook1000 @kylemacomber Hey, do I need to resolve conflicts or this solution doesn't have a chance to be merged?
Just let me know if you are waiting for me. There weren't any updates last three months.

@mwielgoszewski
Copy link

Bumping this, as this feature would be nice.

@vlm
Copy link
Contributor

vlm commented Sep 3, 2023

Bumping this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants