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

Unshackle Spin from Clap 3 #2885

Open
rylev opened this issue Oct 14, 2024 · 3 comments
Open

Unshackle Spin from Clap 3 #2885

rylev opened this issue Oct 14, 2024 · 3 comments

Comments

@rylev
Copy link
Collaborator

rylev commented Oct 14, 2024

We've had two failed attempts at updating our dependency on Clap from version 3 to version 4:

The issue is a breaking change from clap 3 to 4 which does not have a known work around. Clap 4 doesn't lazily evaluate args like Clap 3 did, and once it discovers an arg that can only go into the trigger_args bucket of otherwise unknown args, it starts slurping up everything else and putting it into trigger_args.

This means the following used to work but no longer does:

spin up --some-arg-meant-for-the-trigger --env "foo=bar"

This does not work because as soon as --some-arg-meant-for-the-trigger is seen, everything gets lumped into trigger_args. If --env is put first, than things work as expected.

@itowlson
Copy link
Contributor

More related links:

@lann
Copy link
Collaborator

lann commented Oct 15, 2024

I was having a think about this and wondered to myself "how did this ever work in Clap 3?" Well reader, it doesn't:

$ spin up --listen --direct-mounts localhost:9876
...
Serving http://127.0.0.1:9876

@lann
Copy link
Collaborator

lann commented Oct 15, 2024

Some options:

  • Implement our own "argument pre-processor" that takes all args after up and extracts all known flags (and their values!) before passing on to the clap subcommand parser. This would be annoying and possibly difficult, but could probably approximately preserve the existing behavior.
  • Require that trigger args come after up args. I don't think we necessarily need to require the -- separator here, but we still might want a validator that looks for known up args in the trigger args list in order to give users a more useful error than the "unknown arg" that the trigger args parser would give.

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

No branches or pull requests

3 participants