feat: add multicall support for the CLI #24
Conversation
| .map(|executable| { | ||
| executable | ||
| .file_name() | ||
| .expect("ERROR: failed to obtain the executable's file name") |
There was a problem hiding this comment.
I went for an expect here since this function only returns None if the file ends in .., which should be possible in this case.
Source: https://doc.rust-lang.org/std/path/struct.Path.html#method.file_name
There was a problem hiding this comment.
You mean it should be possible only if someone manually renames the executable, right?
| Defaulting to miden-client." | ||
| ); | ||
| }) | ||
| .unwrap_or(OsString::from("miden-client")) |
There was a problem hiding this comment.
If, for whatever reason, current_exe fails to return the executable's name, I went with returning a default. Mainly because the error case scenario is a bit far fetched, for instance, deleting the executable file during execution (for more details, see: https://doc.rust-lang.org/std/env/fn.current_exe.html#errors).
Worst case scenario, the displayed message is invalid, but we do get a diagnostic thanks to inspect_err.
| #[command(multicall(true))] | ||
| pub struct MidenClientCLI { | ||
| #[command(subcommand)] | ||
| action: Command, | ||
| behavior: Behavior, | ||
| } | ||
|
|
There was a problem hiding this comment.
We require a "wrapper" struct when using multicall because argv[0] gets stripped (see docs, and the executable name gets treated directly as the argument.
There was a problem hiding this comment.
We should document the MidenClientCLI a bit more to reflect this IMO
lima-limon-inc
changed the title
lima-limon-inc
force-pushed
the
fabrizioorsi-multicall-cli
branch
from
c3af25b to
f644498
Compare
lima-limon-inc
changed the title
multicall suuport for the CLI
| let cli = Cli::parse(); | ||
| let input = <MidenClientCLI as clap::CommandFactory>::command(); | ||
| let matches = input.get_matches(); | ||
| let parsed = MidenClientCLI::from_arg_matches(&matches).map_err(|err| err.exit()).unwrap(); |
There was a problem hiding this comment.
This unwrap looks like it could be removed
let parsed = MidenClientCLI::from_arg_matches(&matches)
.unwrap_or_else(|err| err.exit());
| )] | ||
| pub struct Cli { | ||
| #[command(multicall(true))] | ||
| pub struct MidenClientCLI { |
There was a problem hiding this comment.
| pub struct MidenClientCLI { | |
| pub struct MidenClientCli { |
lima-limon-inc
force-pushed
the
fabrizioorsi-multicall-cli
branch
from
45d073c to
3f35b2f
Compare
|
Heads up! I'm squashing and rebasing this and opening a PR upstream. |
lima-limon-inc
force-pushed
the
fabrizioorsi-multicall-cli
branch
3 times, most recently
from
9e0c870 to
53f9fe3
Compare
Signed-off-by: Tomas Fabrizio Orsi <tomas.orsi@lambdaclass.com>
Suggested-by: Francisco Krause Arnim <francisco.krause@lambdaclass.com> Signed-off-by: Tomas Fabrizio Orsi <tomas.orsi@lambdaclass.com>
lima-limon-inc
force-pushed
the
fabrizioorsi-multicall-cli
branch
from
53f9fe3 to
f49b5c4
Compare
Signed-off-by: Tomas Fabrizio Orsi <tomas.orsi@lambdaclass.com>
Signed-off-by: Tomas Fabrizio Orsi <tomas.orsi@lambdaclass.com>
Signed-off-by: Tomas Fabrizio Orsi <tomas.orsi@lambdaclass.com>
Signed-off-by: Tomas Fabrizio Orsi <tomas.orsi@lambdaclass.com>
juan518munoz
changed the title
multicall suuport for the CLI multicall support for the CLI
2 participants
This PR makes the client's CLI multicall compatible. With this, the client's CLI is aware of whether it is being called under the
miden-clientname or a different one, like it is the case withmidenupinvocation of the client under themiden clientname.This value is stored in the
Clistruct, although it is not being currently used.Besides, the
CLIENT_BINARY_NAMEconstant was replaced in favor of function that returns the name by which the client's executable was called.