Update artichoke CLI to clap v4
#2196
Conversation
lopopolo
added
A-deps
https://epage.github.io/blog/2022/09/clap4/ `--help` output is changed, so update the ui-test snapshots.
lopopolo
force-pushed
the
lopopolo/artichoke-cli-clap-v4
branch
from
07e4a0e
to
3eca7b9
Compare
src/ruby/cli.rs
Outdated
| // (This is the point of this helper function. clap's functionality for | ||
| // doing this will panic on a broken pipe error.) |
There was a problem hiding this comment.
I thought the broken pipe issue has been fixed for a while...
There was a problem hiding this comment.
I think it has been, but this code has worked the whole time and doesn't have that much maintenance cost for me to carry. I also prefer having visibility into all calls to process::exit instead of having them happen in a library if I can help it.
There was a problem hiding this comment.
But I can definitely clean up this comment since the clap side of things has been fixed.
There was a problem hiding this comment.
I just pushed a commit that removes this parenthetical comment. Thanks for catching that!
There was a problem hiding this comment.
You can also use err.print() so you can control the exiting but get clap to do the printing, including colored output. If you want more control over that and are fine with ANSI escape codes, you can now also call err.render().ansi().
There was a problem hiding this comment.
Oh that's awesome!
Before
After
There was a problem hiding this comment.
Huh, noticed a couple issues in that. Addressed in clap-rs/clap#4287 which should be released soon
This was fixed in 3.x. Still keep the helper function, since it gives us control over the call to process::exit and the exit code.
|
Thanks for the code review @epage 🎉 ⚙️ |
https://epage.github.io/blog/2022/09/clap4/
--helpoutput is changed, so update the ui-test snapshots.One of the neat new things in clap v4 is the ability to add a test to assert that the
clap::Commandis configured correctly. When a package has both alib.rsand a binary target, I think#[test]s don't run in themain.rs, so this PR also takes the chance to migrate all of theartichokeclap CLI setup to a newartichoke::ruby::climodule.