commander-rust icon indicating copy to clipboard operation
commander-rust copied to clipboard

Add support for non () return types

Open Gozala opened this issue 5 years ago • 3 comments

Fixes #10.

These changes enforce that all commands have same return type as main, without assuming it to be (). ~~I have also changed run()! so that main is able to return that type in case --help or something that does not match program runs e.g.~~

#[entry]
fn main() -> std::io::Result<()> {
  run!();

  Ok(())
}

~~Unfortunately it also implies that run()! will return in case matching command is found, which can be confusing.~~

Application had being made polymorphic Application<Out> and out: Option<Out> field can was added to hold return value of the matched command, in case no command was run it will be None.

~~Only~~Another significant change had being removal of mod _commander_rust_Inner. That is because return type of the main (#ret) may resolve to something else given the new module scope. For example following code without it will not work because Result<()> would resolve to default std::result::Result in new module scope instead of std::io::Result which it should instead.

use std::io::Result;

#[entry]
fn main() -> Result<()> {
  run!();

  Ok(())
}

Gozala avatar Apr 01 '20 05:04 Gozala

I am now realizing that I have overlooked / misunderstood what run does, specifically it returns Application while also executing matching command, which here proposed changes do break.

I'll try to figure out the way to avoid such a breaking change. Any help would be welcome.

Gozala avatar Apr 01 '20 19:04 Gozala

I have local version that causes run()! to produce (app, Option<return_value>) pair instead that way both return value and app are passed back. That is not ideal however, I'll try to make return_value part of app so that API stays backwards compatible.

Gozala avatar Apr 01 '20 20:04 Gozala

I have updated implementation to make Application polymorphic so that return value can be contained by it and there for avoid breaking API. However I am still not very happy with a result as API still clunky as main needs to check if app.out isn't none to know if command was handled or not, here is example

#[wait]
#[command(scan <path>, "Scans directory and submits all findings to knowledge-server")]
async fn scan(path: String, _cli: Cli) -> Result<()> {
  println!("Scanning resources {:?}", path);
  scanner::activate().await
}

#[wait]
#[direct()]
#[option(-p, --port, "Port to be used by the knowledge-server (Default 8080)")]
async fn base(cli: Cli) -> Result<()> {
  println!("Default action {:?}", cli);
  Ok(())
}

#[wait]
#[entry]
async fn main() -> Result<()> {
  let app = run!();

  if let Some(out) = app.out {
    out
  } else {
    serve(app)
//        ^^^ expected struct `commander_core::Cli`, found struct `commander_core::Application`
  }
}

Is it can be seen it is not even possible to delegate to default command. I think adding app.cli might help but even then I think there are two things one might want to do:

  1. Do something when no command was matched.
  2. Do something before / after the command is run.

I think current API is tailored towards 2. I think it would make sense to tailor it for 1. instead while still allow 2. if desired. I have proposed something towards that end in #19

Gozala avatar Apr 01 '20 21:04 Gozala