feat(runtime): Remove dependency on clap

[kierendavies]

This removes the shuttle_runtime dependency on clap, by explicitly implementing very simple argument parsing.

I hope you like macros 😇. If not, I'll just replace the macro with its expansion - which is exactly the code I wrote before converting it to a macro.

Closes #819

/claim #819

[jonaro00]

@kierendavies Nice! Some things I thought about:

• You want to do the same thing for runtime/next/args.rs
• If no args are given at all, it would be nice to get "Shuttle runtime started without internal args. Try using cargo shuttle run instead." in the panic message :)
• If port gets its default value 0, that should probably be handled (or just let if panic later i guess)

[oddgrd]

Thanks @kierendavies! I'll take a closer look at this, but I think @jonaro00's review should be addressed, as well as the CI errors. Whenever you have time, I need to brush up on declarative macros as well 🙈

[kierendavies]

Thanks y'all.

• You want to do the same thing for runtime/next/args.rs

I missed this because my toolchain wasn't using --all-features 🤦

• If no args are given at all, it would be nice to get "Shuttle runtime started without internal args. Try using cargo shuttle run instead." in the panic message :)

Good one, but better to keep as a separate PR.

• If port gets its default value 0, that should probably be handled (or just let if panic later i guess)

I don't see that as the responsibility of the argument parser. I'm a fan of letting the later consumers deal with it, like you said, because they can also detect ports already in use, low-numbered ports without root, etc. Alternatively, you could create a wrapper type which validates in its FromStr implementation (but there are reasons not to do that).

[chesedo]

Oh, wow 🤩 Thanks @kierendavies!

I left a few minor notes

[chesedo]

Maybe these should be in a separate macro_rule (unwrap_arg?). They feel a bit unexpected here.

[kierendavies]

I you call an exported macro from another module, then every macro it calls must also be exported. I tracked down this trick for dealing with it without polluting any namespaces. http://danielkeep.github.io/tlborm/book/pat-internal-rules.html

[chesedo]

Hmmm, calling the new macro using its full namespace will also work here 😄

[chesedo]

• If no args are given at all, it would be nice to get "Shuttle runtime started without internal args. Try using cargo shuttle run instead." in the panic message :)

Good one, but better to keep as a separate PR.

I don't think we need to handle the no args case, but maybe the error enum's Display can still have something about "Try using cargo shuttle run instead." for all the cases?

[kierendavies]

I don't think we need to handle the no args case, but maybe the error enum's Display can still have something about "Try using cargo shuttle run instead." for all the cases?

That's still new behaviour which means it should be in another PR 😉

[chesedo]

That's still new behaviour which means it should be in another PR wink

That's fair, we should at the very least open an issue then

[oddgrd]

I also think we should have the suggested next step in the error message, since a user might run cargo run and be met with this error that will cause confusion. But if you prefer to do this in a separate PR that is fine.

The current implementation will also just show the first missing arg, not all of them like before, but that's a bit of a nit and I wouldn't say it's blocking.

Otherwise looks great to me, thanks Kieren!