RFC: feat: add support for -- to separate cabin args from program args#1204
RFC: feat: add support for -- to separate cabin args from program args#1204jimmyhmiller wants to merge 4 commits intocabinpkg:mainfrom
Conversation
|
Thanks, can you please write integration tests for this? |
|
Written. Let me know if you want something else. The run test was pretty sparse. |
|
Thanks, will take a look when I get a chance. |
ken-matsui
left a comment
ken-matsui
left a comment
There was a problem hiding this comment.
I didn't remove the old functionality because I wasn't sure if you'd want that breaking change.
Does Cargo only have the behavior you added?
| .addOpt(OPT_JOBS) | ||
| .setArg(Arg{ "args" } | ||
| .setDesc("Arguments passed to the program") | ||
| .setArg(Arg{ "[-- args]" } |
There was a problem hiding this comment.
Curious, is this the same as what Cargo prints? (I'm AFK)
There was a problem hiding this comment.
It isn't what they print, but it is how they document it. https://doc.rust-lang.org/cargo/commands/cargo-run.html If you do cargo help run, you will see the same. But the actually cargo run --help is less clear.
There was a problem hiding this comment.
I looked at the help and totally agree. Thanks!
There was a problem hiding this comment.
Did you check what the help looks like? Wouldn't it be like cabin run [OPTIONS] [[-- args]]...? I think we want something like cabin run [OPTIONS] [-- args...].
There was a problem hiding this comment.
| .setArg(Arg{ "[-- args]" } | |
| .setArg(Arg{ "-- args..." } |
IIRC, you can change .setVariadic(true) to false and add a TODO for me.
So cargo doesn't do what you are doing here with unknown args. I'd imagine, because that can lead to any new argument you add to run to be a breaking change. So you must be explicit and do the -- for program args. I'd personally recommend that. I'm happy to make the change and remove the test, but didn't want to make a breaking change (technically this is one but fairly small) without you being for it. |
|
I believe everything has been addressed |
Yeah, I think we have a good reason to have the breaking change. Can you remove the original behavior? Thanks! |
ken-matsui
force-pushed
the
main
branch
8 times, most recently
from
0779bf5 to
837e3d4
Compare
2 participants
When I wrote a program using --help, I found that cabin would always intercept it when doing
cabin run --help. So following cargo's lead, I added--to separate them. So now you can runcabin run -- --helpand it passes help to your program. I didn't remove the old functionality because I wasn't sure if you'd want that breaking change.I figured the code here is easier to start the discussion hence the rfc pr.