Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace structopt by clap #235

Closed
palant opened this issue May 8, 2024 · 4 comments
Closed

Replace structopt by clap #235

palant opened this issue May 8, 2024 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@palant
Copy link
Contributor

palant commented May 8, 2024

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 (TeXitoi/structopt#539, clap-rs/clap#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 {}
@andrewhavck andrewhavck added the enhancement New feature or request label May 9, 2024
@johnhurt johnhurt self-assigned this May 10, 2024
@johnhurt
Copy link
Contributor

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

@palant
Copy link
Contributor Author

palant commented May 10, 2024

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
Copy link
Contributor

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
Copy link
Contributor Author

palant commented May 10, 2024

@johnhurt Ok, then there is #239 now.

drcaramelsyrup pushed a commit that referenced this issue May 24, 2024
---
Fixed formatting

Includes-commit: 05f9754
Includes-commit: 29286c7
Replicated-from: #239
escoffier pushed a commit to escoffier/pingora that referenced this issue Sep 6, 2024
---
Fixed formatting

Includes-commit: 05f9754
Includes-commit: 29286c7
Replicated-from: cloudflare#239
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants