Skip to content

Conversation

@Gozala
Copy link

@Gozala Gozala commented Apr 1, 2020

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.

OnlyAnother 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 Gozala changed the title Update dependencies Add support for non () return types Apr 1, 2020
@Gozala
Copy link
Author

Gozala commented Apr 1, 2020

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
Copy link
Author

Gozala commented Apr 1, 2020

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
Copy link
Author

Gozala commented Apr 1, 2020

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

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

Successfully merging this pull request may close these issues.

Support for functions that return anything other than ()

1 participant