Skip to content

Conversation

jessebraham
Copy link
Member

I've been wanting to do this for awhile and finally got around to it. While part of the reason was purely personal preference, this also (IMO) makes it easier for us to add options, performs better validation of command-line arguments, and doesn't require manual updates of the argument parsing function and help messages.

As a note, I did remove the --chip option as it was not actually being used (we perform auto-detection). I am fine with adding it back for compatibility reasons, but I personally don't see the need.

@N3xed
Copy link

N3xed commented Sep 3, 2021

Why not structopt, I think that would make it more readable. But that's just my opinion. It's basically like serde's derive macros for CLIs. Look for example at the cargo-pio CLI tool where we use it too (https://github.com/ivmarkov/embuild/blob/master/cargo-pio/src/main.rs).

Copy link
Member

@MabezDev MabezDev left a comment

Choose a reason for hiding this comment

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

I agree that structopt is nice! However either is a big improvement over picoargs, so either works for me :)

@jessebraham
Copy link
Member Author

I would rather focus my efforts on other functionality at this point. Anybody is of course welcome to make further changes on top of mine, I just don't feel strongly enough either way to justify spending any more time on this.

@N3xed
Copy link

N3xed commented Sep 4, 2021

I would rather focus my efforts on other functionality at this point. Anybody is of course welcome to make further changes on top of mine, I just don't feel strongly enough either way to justify spending any more time on this.

All good, was just a suggestion (I just like structopt :D).

@icewind1991 icewind1991 merged commit 27b2f32 into esp-rs:master Sep 4, 2021
@jessebraham jessebraham deleted the clap branch September 4, 2021 15:02
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.

4 participants