Migrate to clap 4.x (WIP)

[schneiderfelipe]

This PR migrates to clap 4.3. This in particular requires/benefits from some changes to/on how some errors work, including depending on thiserror; I believe those are good changes overall.

I plan on doing the following in this PR:

• migrate to clap 4.x
• migrating errors to use thiserror
• some glitter (use of Self and of format! only)
• removing tests that check some CLI validation errors; they currently reflect the state of the PR though, so you can see how CLI error messages differ those changes have been isolated in the last commit

Please let me know what you think @noeddl, in particular on the last point.

[noeddl]

Thanks a lot for the PR @schneiderfelipe! It looks good to me on first glance but I will take a closer look later when I have some more time, especially on the failing checks. I will probably have to update the minimum supported Rust version to allow for clap 4.

As for testing CLI validation errors: Did clap 4 improve anything in this regard or would you consider these tests superfluous no matter which clap version?

[schneiderfelipe]

Thanks a lot for the PR @schneiderfelipe! It looks good to me on first glance but I will take a closer look later when I have some more time, especially on the failing checks.

Some of the checks are failing due to -D warnings. In fact, some of those fail on unmodified code, maybe due to new clippy warnings having been added in the last couple years? I have some changes on my side addressing those (and other) clippy warnings, but I believe that would be outside the scope of this PR (I'm happy to contribute some of those soon 🤗)?

I will probably have to update the minimum supported Rust version to allow for clap 4.

I think the MSRV for clap 4.3 (what this PR is asking for) is 1.64.0. For comparison, clap 4.0 seems to ask for 1.60.0, so that's the interval we could choose from? cargo msrv gave me 1.64.0 if I remember correctly.

As for testing CLI validation errors: Did clap 4 improve anything in this regard or would you consider these tests superfluous no matter which clap version?

That's because some of those validations used to happen here, but since they are being proposed to happen on clap side (e.g. see here), I suggest these tests could be removed.

[noeddl]

I suggest that we first update the MSRV to 1.64.0 and make sure all the checks are passing (i.e. by addressing the clippy warnings) in a separate PR. Then we can rebase this PR on the new master after the other one has been merged. What do you think? If you want to you could do that using the changes that you already have in place regarding the clippy warnings.

That's because some of those validations used to happen here, but since they are being proposed to happen on clap side (e.g. see here), I suggest these tests could be removed.

I see, thanks for the clarification.

[schneiderfelipe]

Good call, I can do that just fine. In the meanwhile, can I ask you to rerun the CI workflow on the master branch, so that we can have a baseline?

[noeddl]

I had to update the CI to allow for manually triggering it in the future but now the checks have in any case run on master. 🙂 Looks like they fail at the same places as when running on this PR except for the 1.56.0 job - which is what was to be expected.

[noeddl]

Hi @schneiderfelipe, are you still interested in doing the preparations that we discussed above (updating the MSRV and taking care of the clippy warnings)? Otherwise I would like to do them myself so that I can merge this PR and allow for any further development.

[schneiderfelipe]

Hi Anett, my time got short lately. Feel free to do it if you want. I'll probably catch pace at a later time! Cheers! Em ter, 7 de mai de 2024 06:53, Anett Seeker ***@***.***> escreveu:

…

Hi @schneiderfelipe <https://github.com/schneiderfelipe>, are you still interested in doing the preparations that we discussed above (updating the MSRV and taking care of the clippy warnings)? Otherwise I would like to do them myself so that I can merge this PR and allow for any further development. — Reply to this email directly, view it on GitHub <#123 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAJCBP33GTEBHIFD4LJ2GLZBCQB5AVCNFSM6AAAAABFPSZTEWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJXHEYDMNBSGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[noeddl]

@schneiderfelipe Thanks again for the PR and thanks for bringing back some life to this project. 🙂