arthurpalves / badgy

A command line tool that creates variants of your icon by adding badge overlays
MIT License
174 stars 11 forks source link

Switch to ArgumentParser #10

Closed arthurpalves closed 4 years ago

arthurpalves commented 4 years ago

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.

elfenlaid commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

elfenlaid commented 4 years ago

@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