criterion.rs icon indicating copy to clipboard operation
criterion.rs copied to clipboard

Published 20 hours ago • verified publisher icon bheisler

Switch CLI arg parsing from `clap` to `lexopt`

Open CosmicHorrorDev opened this issue 1 year ago • 4 comments

Resolves #596

(pinging @lemmih I'm targeting version-0.4 in case you want to include this in version 0.4)

This swaps the CLI arg parsing from clap to pico-args. Doing so drops the number of dependencies by 6 (and all of pico-args itself is just over 800 lines comments and all)

I think the organization speaks for itself. I was going to add more tests along with doing some more manual tests, but I may have sprained my wrist today, so I'm going to go easy on the typing :sweat_smile:

CosmicHorrorDev avatar Jul 17 '22 01:07 CosmicHorrorDev

Consider https://github.com/blyxxyz/lexopt as well it is "more correct" across some dimensions, while being similarly lean.

matklad avatar Jul 19 '22 12:07 matklad

You had me at more correct!

Thanks for the heads up! The error handling was a little more fiddly, but overall it was pretty easy to switch over to since I could keep a lot of the existing work (help generation, tests, etc)

CosmicHorrorDev avatar Jul 20 '22 00:07 CosmicHorrorDev

Note that #602 is going to require additional support.

sunshowers avatar Jul 20 '22 01:07 sunshowers

Thanks for pointing that out!

It looks like porting the cli work for that should be pretty trivial too, so I'm fine with having either PR merged first

CosmicHorrorDev avatar Jul 20 '22 01:07 CosmicHorrorDev

anything I can do to help get this one merged?

marcograss avatar Nov 20 '22 02:11 marcograss

Thanks for asking @marcograss

I've mostly fixed merge conflicts locally. It just needs a little more work. I've got the next week off work to visit family, so I can probably find some time to finish up my changes

CosmicHorrorDev avatar Nov 20 '22 03:11 CosmicHorrorDev

Is this ready or is there more work that needs to be done here?

Manishearth avatar Feb 26 '23 02:02 Manishearth

Unless I'm forgetting something this should be ready for review whenever that winds up happening

CosmicHorrorDev avatar Feb 26 '23 02:02 CosmicHorrorDev

This is quite the large change. Is anyone up for reviewing it?

lemmih avatar Apr 02 '23 09:04 lemmih

I'm really busy with other work atm, and don't feel up to going back to this PR to try and get it merged (fun fact: since I started on this PR I both broke my wrist, and had it fully heal as of many months ago)

Someone else is free to pick up the torch if they want. Sorry to all the people who left feedback in various ways

CosmicHorrorDev avatar Jun 05 '23 14:06 CosmicHorrorDev