Replace structopt by clap (#155)

[theredfish]

This pull request is a draft, waiting for the official release of clap v3. However, if needed, It can already be reviewed as the api shouldn't drastically change.

The doc comments on top of structs deriving clap::Args or clap::Parser is still an issue with Clap v3. So I let the workaround in place.

Closes #155

[tyranron]

@theredfish from clap-rs/clap#2869 it seems they'll wait a month before making release candidate stable.

I'd opt to finish this PR and merge it now. Either way we'll introduce a bc break switching to stable 3.0 release. But, at least, it would be smaller to go from rc to stable, rather than from structopt to clap.

Also, way we may first release an rc version too.

[theredfish]

Ok perfect @tyranron , I have some failing jobs that are connected to clap itself. I will check the error and see if I can do anything.

[theredfish]

@tyranron what is exactly the need of building with minimal versions for transitive dependencies there? I'm not used to this so any details could help me.
I tried locally cargo update -Z minimal-versions and termcolor is downgraded : Updating termcolor v1.1.2 -> v1.1.0.

This dependency of clap doesn't contain the function set_dimmed in the minimal version.

[ilslv]

@theredfish your PR is fine, but there is a problem with clap, where minimal version of the termcolor crate is 1.1.0, but set_dimmed doesn't appear until 1.1.1.

UPD: should be fixed by clap-rs/clap#3172 which is waiting for crate-ci/escargot#53

[tyranron]

@theredfish

what is exactly the need of building with minimal versions for transitive dependencies there?

To ensure that minimal versions are specified correctly, so the code will actually compile for the range versions declared in the manifest. Due to the ecosystem mostly not doing this check from the start (so their manifests are oftenly broken), sometimes we have such inconveniences with transitive dependencies. So we're trying to fix them where we can. Usually, these are non-blockers, as you always can temporary pin-point the version of bad transitive dependency in your crate's manifest.

[theredfish]

Ok great ! Thank you for your help and the explanations. I will wait for the merge and the new RC version of clap 👌.

[ilslv]

@theredfish fix for minimal-version has been released as part of clap 3.0.0-rc.7

[theredfish]

    Compiling gherkin v0.11.1
    Checking cucumber v0.11.0-dev (/home/runner/work/cucumber/cucumber)
error[E0405]: cannot find trait `StructOpt` in this scope
   --> src/cli.rs:296:18
    |
296 |     L: Colored + StructOpt,
    |                  ^^^^^^^^^ not found in this scope
    |
help: consider importing this trait
    |
30  | use clap::StructOpt;
    |

I suppose that I'm hitting a cache, because it doesn't make sense and I don't have the same number of lines. Seems to correspond to a line in main here. I will also rebase.

Edit : or maybe this is a merge pipeline so it's running my code with main, it would make sense. It should be fixed by rebasing.

[tyranron]

@theredfish thanks! 🍻

[tyranron]

@theredfish would you be so kind to make this PR editable for maintainers, so I can push some changes before merging it?

[theredfish]

@theredfish would you be so kind to make this PR editable for maintainers, so I can push some changes before merging it?

Done! Btw I don't really know if we should or not activate this by default. I thought it was some kind of protection for the commits, to avoid for example the history to be rewritten. But I don't really know if it's for this or not.
Do you use it / is there any recommendation with this option when submitting a PR?

[tyranron]

@theredfish thanks! I usually do use it al the time for maintainers being able to pick up and adjust the stuff. I cannot recall any situations where I've wanted to forbid maintainers to edit my PRs. I don't know about recommendations, though.

[theredfish]

Thank you for your time. I took a look to your corrections, it's instructive 👍.