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

chore: update clap dependency to 4.1 #1198

Closed
wants to merge 1 commit into from
Closed

Conversation

cardoso
Copy link
Contributor

@cardoso cardoso commented Feb 24, 2023

I kept changes to a minimum this time, so there's redundant parameters or with better alternatives in v4, but those should be easy to improve as needed. The display order in help was also kept the same by overriding the new default in each Subcommand.

@vdice
Copy link
Member

vdice commented Feb 24, 2023

Thanks @cardoso, appreciate how this brings the clap update but in perhaps a more staged approach so that usage can be further updated (and more recent functionality/features utilized) in follow-ups.

Glancing through the changes, all looks well; however, the e2e tests are catching similar breaking CLI behavior as seen in #1053 (review). Do you think it should be fairly straightforward to address these (eg retain current CLI behavior) and still land the dependency updates?

@cardoso
Copy link
Contributor Author

cardoso commented Feb 27, 2023

It's not possible with clap 4. It really wants unknown args to be either after -- or next to each outer, so it assumes anything after an unknown arg is also unknown. I'm pushing a sample workaround to see what the tests say...

@cardoso cardoso force-pushed the clap-4 branch 2 times, most recently from ce822aa to e922a1a Compare February 27, 2023 08:20
@cardoso
Copy link
Contributor Author

cardoso commented Feb 27, 2023

REF: clap-rs/clap#1404 (comment)

@cardoso
Copy link
Contributor Author

cardoso commented Mar 1, 2023

I will close this one as blocked by clap-rs/clap#1404 or alternatively, once the breaking change can be made to the to the more conventional spin up [args] -- [trigger args].

I think I reached a solution at some point, but it involved refactoring a lot of code that's receiving constant changes. Suffice to say...

image

@itowlson
Copy link
Contributor

itowlson commented Mar 1, 2023

Thank you @cardoso for all your effort on this - I'm sorry it didn't work out. This definitely seems to be a regression between clap 3 and clap 4, but looking at that issue it seems to be 'by design.' Frustrating!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants