Skip to content

Conversation

@alphatroya
Copy link
Contributor

@alphatroya alphatroya commented Oct 2, 2020

I have courage to suggest you to replace Commander library by apple argument parser library. This library from maintainers of the Swift have better support and stability.

Also it uses new Swift 5.1 features (property wrappers) and code looks very nice (IMHO of course).

Another benefit - build in generation of the shell completions.

In this PR I tried to keep the same options and flags names to provide compatibility with your documentation.

New interface:
Снимок экрана 2020-10-02 в 11 03 56

The old one:
Снимок экрана 2020-10-02 в 11 04 05

try ImageExtractor.extract(input: (image, platforms), output: (iconName, outputPath))
} catch {
print("Image Extraction Error has occured 😱")
throw ExitCode(1)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also think in that and L48 cases program flow should be aborted to prevent wrong success info printing

@Nonchalant Nonchalant self-requested a review October 2, 2020 10:44
Copy link
Owner

@Nonchalant Nonchalant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the great suggestions!
I've been wanting to adopt swift-argument-parser myself!
I just supported Xcode12, so I'm just asking for your help in resolving conflicts here!
I'm so glad!

@alphatroya alphatroya requested a review from Nonchalant October 2, 2020 11:20
Copy link
Owner

@Nonchalant Nonchalant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much 🍀

@Nonchalant Nonchalant merged commit a020560 into Nonchalant:master Oct 2, 2020
@alphatroya alphatroya deleted the argument-parser branch October 2, 2020 18:26
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.

2 participants