pingora icon indicating copy to clipboard operation
pingora copied to clipboard

Replace structopt by clap

Open palant opened this issue 1 year ago • 4 comments

What is the problem your feature solves, or the need it fulfills?

structopt crate is unmaintained and superseded by clap v3 (Maintenance note). It has bugs that clap fixed three years ago before even releasing them (https://github.com/TeXitoi/structopt/issues/539, https://github.com/clap-rs/clap/issues/2527). This creates issues when expanding Pingora’s Opt structure.

Describe the solution you'd like

Pingora should use a current clap version.

Describe alternatives you've considered

I implemented the following hack – adding a dummy field to the app’s options only to overwrite application description that has been overwritten by Opt above:

/// My description
#[derive(Debug, StructOpt)]
struct MyOpt {
    …

    #[structopt(flatten)]
    server: Opt,

    #[allow(dead_code)]
    #[structopt(flatten)]
    dummy: StructOptDummy,
}

#[derive(Debug, StructOpt)]
#[structopt(about = "My description", long_about = "My description")]
struct StructOptDummy {}

palant avatar May 08 '24 09:05 palant

No promises, but I'll try to make this happen

johnhurt avatar May 10 '24 16:05 johnhurt

No promises, but I'll try to make this happen

It isn’t complicated, structopt to clap is almost search&replace – I could do it if I knew that the pull request will be accepted rather than stuck because nobody has time to review it.

palant avatar May 10 '24 16:05 palant

Yeah, the only hurdle is to get buy in from the team since it's a change a pretty central place. If you're willing to put in the work/pr. That will make it easier for me to champion 💪

johnhurt avatar May 10 '24 17:05 johnhurt

@johnhurt Ok, then there is #239 now.

palant avatar May 10 '24 18:05 palant