cloudflare / pingora

A library for building fast, reliable and evolvable network services.
Apache License 2.0
20.25k stars 1.1k forks source link

Replace structopt by clap #235

Closed palant closed 1 month ago

palant commented 1 month ago

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 {}
johnhurt commented 1 month ago

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

palant commented 1 month ago

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.

johnhurt commented 1 month ago

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 💪

palant commented 1 month ago

@johnhurt Ok, then there is #239 now.