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

Switch to ArgumentParser #10

Closed
arthurpalves opened this issue Jun 7, 2020 · 8 comments
Closed

Switch to ArgumentParser #10

arthurpalves opened this issue Jun 7, 2020 · 8 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@arthurpalves
Copy link
Owner

ArgumentParser is Swift’s open source library to parse command-line arguments.

Consider the switch from SwiftCLI to ArgumentParser as an improvement, for something that has more contribution, stability and support from the community.

@arthurpalves arthurpalves added enhancement New feature or request help wanted Extra attention is needed labels Jun 7, 2020
@arthurpalves arthurpalves changed the title Switch to ArgumentParser for the commands Switch to ArgumentParser Jun 7, 2020
@elfenlaid
Copy link
Contributor

Oh, that would be nice.

Also, while working on the #1 I've noticed a somewhat fragile copy-pasting in validation logic. It might affect the future work on arguments handling.

I'll have a look on it, if you don't mind :)

@arthurpalves
Copy link
Owner Author

Please go ahead 👍

Before migrating to ArgumentParser I thought about building a common Badge command, which shares all validations, the others would be a subclass of it. I'm not sure if that is applicable if we switch.

@elfenlaid
Copy link
Contributor

Please go ahead 👍

thanks :)

Also a validation logic is a natural candidate for automated testing. Though they might be a burden in the short run, tests should pay off in the long run.

Unfortunately, a small package overhaul is needed to support a test target. We need to extract Badgy's logic to a library test target, as tests fail to run against an executable target.

On the other hand, the required changes are backward compatible with the previous Badgy versions.

@arthurpalves
Copy link
Owner Author

No worries, no need to convince me of the benefits, just didn't have time to get that done for such a small little project :)

Glad to have you here thinking on how to make this tool better @elfenlaid! 🚀

Please don't feel the burden of carrying these ideas by yourself, I'd suggest we create clear issues for them so that we can easily collaborate and keep track.

@elfenlaid
Copy link
Contributor

I've got a look at ArgumentParser and what an awesome piece of tech :)

Back to the migration, it seems we may split the adoption into two or three phases

  1. Mirroring the same CLI, give or take logs and error reporting
  2. Extracting the logic to base both CLIs on
    • Somewhere here a workaround for verbose flag is needed. From the first glance, it will require an actual mutable variable from Logger instead of deriving verbosity style from a flag.
  3. Switching to ArgumentParser

@arthurpalves
Copy link
Owner Author

It's looking good! 👏

Have the same being done to a private tool (soon to be open source)
I've used a similar Logger in both, so those might have to be completely changed, perhaps adopting a separate SPM for logs or create one SwiftCLI independent will probably be the way to go.

I've created a base branch for this - rc-argument-parser - so that we can slowly merge your changes

@arthurpalves
Copy link
Owner Author

@elfenlaid I believe we can close this issue and open a more descriptive one to "Remove dependency on SwiftCLI.Task"?

@elfenlaid
Copy link
Contributor

elfenlaid commented Aug 3, 2020

@elfenlaid I believe we can close this issue and open a more descriptive one to "Remove dependency on SwiftCLI.Task"?

Oh, sure 👍

Drafted the problem here #25

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants