tools: accept "help" top-level command, show usage on errors

[iximeow]

helios-build has supported --help since forever, but as @rmustacc found in #12 and i rediscovered yesterday, not -h nor a top-level "help" command. seems like it'd be good to accept all of --help, -h, helios-build help, or the bare ./helios-build to get usage information, so this change does that.

also, because it's in baseopts, this means -h is now a flag for all the subcommands too.

[jclulow]

I don't necessarily want to throw the h short option away for --help. I think it's far less consistently available than --help, and sometimes it makes sense to use it to align with use of -h for "human readable" or "host name" or whatever else.

I agree that a top-level help command is valuable, though!

[jclulow]

I have pushed some changes to the branch:

• add a handler entry for help, even though the handler function will never be called, so that it shows up in the usage information; e.g.,

  $ /ws/helios/helios-build
  Usage: helios [OPTIONS] COMMAND [OPTIONS] [ARGS...]
  
      setup            clone required repositories and run setup tasks
  
      genenv           generate environment file for illumos build
  
      bldenv           enter a bldenv shell for illumos so you can run dmake
      onu              install your non-DEBUG build of illumos on this system
      build-illumos    run a full nightly(1) and produce packages
      merge-illumos    merge DEBUG and non-DEBUG packages into one repository
      experiment-image experimental image construction for Gimlets
  
      help             display usage information
  
  
  Options:
      --help              display usage information
  
  Error: choose a command
  

• make use of help exit zero, rather than with a usage error
• we should be parsing the OsStr arguments, rather than the version that panics if not everything is UTF-8, which is cleanup I try do as I find cases where I did the wrong thing initially!

[iximeow]

ah! good changes.

you prompted me to check in a few places about -h and i realized that beadm, dladm, and generally most tools i've "successfully" used with -h have actually been interpreting -h as a subcommand (or argument) which is not known, then calling usage() to tell the user what they can do. for helios-build, we'd do similar with a change like:

         println!("{}", opts.usage(&out));
     };
 
-    let res = opts.parse(std::env::args_os().skip(1))?;
+    let res = match opts.parse(std::env::args_os().skip(1)) {
+      Ok(res) => res,
+      Err(e) => {
+        eprintln!("Error: {}", e);
+        usage();
+
+        // If we bail!() out here, we'll get another `Error: ...` printed to stderr
+        // by Rust. We've done all the reporting we want though, so just exit(1)
+        // now.
+        std::process::exit(1);
+      }
+    };
+
     if res.opt_present("help") {
         usage();
         return Ok(());

that should guide users a bit better than the status quo of

./helios-build -h
Error: Unrecognized option: 'h'

while not necessarily eating -h as a flag here or later subcommands. what do you think?

[jclulow]

I think that's reasonable, but I would print the usage first and then just bail in the usual way with the error. Sometimes the usage information can be more than a an entire screen vertically, and it's super irritating to be hit in the face with it unexpectedly and then to have to go and hunt for what you did wrong! If we put the error last, at least it's right there where your eyes are with the next shell prompt.

 $ ./helios-build -h
Usage: helios [OPTIONS] COMMAND [OPTIONS] [ARGS...]

    setup            clone required repositories and run setup tasks

    genenv           generate environment file for illumos build

    bldenv           enter a bldenv shell for illumos so you can run dmake
    onu              install your non-DEBUG build of illumos on this system
    build-illumos    run a full nightly(1) and produce packages
    merge-illumos    merge DEBUG and non-DEBUG packages into one repository
    experiment-image experimental image construction for Gimlets

    help             display usage information


Options:
    --help              display usage information

Error: Unrecognized option: 'h'

I've pushed a change to make it look like the above, and to ensure that the usage text ends up on stderr in a failure case, or stdout (for grep, etc) if you explicitly requested it.

[iximeow]

nice, thanks! works for me. i was only leaning towards "error first, then usage" because that's what other tools i was comparing against also do. error at the end seems pretty reasonable.

i'd approve, as this is now ~100% @jclulow code, but github doesn't let me self-approve. does need an approver before squash and merge, so feel free to do the honours 🫡