[WIP] Switch to getopts for flag parsing

[kevinkassimo]

Using getopts crate. Also use help message generated by getopts directly.

[kevinkassimo]

Hmm it seems fetching changes for third party failed...

Okay, I need to create a PR in deno_third_party for that...

[ry]

Great! This looks good.

I have a problem tho:

Error 1 ~/src/deno> ./out/debug/deno  tests/http_bench.ts 4501 --allow-net
PANIC file 'libcore/result.rs' line 945
Abort trap: 6
Error 134 ~/src/deno> ./out/debug/deno --allow-net  tests/http_bench.ts 4501
PANIC file 'libcore/result.rs' line 945
Abort trap: 6

I'm not able to pass a port to tests/http_bench.ts. Ideally both of the above versions would work.

[kitsonk]

You would think that there would be some way to enumerate over the values, change the opts string from foo-bar to foo_bar and automatically set the flags. It would have to be one less place to maintain when adding a flag then.

[kevinkassimo]

Hmm I could look into that, though I don't think getopts crate itself offers some easy way to do this...

[kitsonk]

I know you are already down this path a ways, but docopt seems like it would be a more direct way of getting a set of flags out. See: https://docs.rs/docopt/1.0.1/docopt/#type-based-decoding

[kevinkassimo]

Its ArgvMap seems nice, would look into it.
Just now I played around and made an ugly hack... (Probably not worth the reduced lines of code...):

pub fn set_flags(
  args: Vec<String>,
) -> Result<(DenoFlags, Vec<String>, String), String> {
  let args = v8_set_flags(args);

  let mut flags = DenoFlags::default();
  let mut opts = Options::new();
  let matches: Option<Matches>;
  {
    let mut flags_tuples = vec![
      ("", "allow-write", "Allow file system write access.", Some(&mut flags.allow_write)),
      ("", "allow-net", "Allow network access.", Some(&mut flags.allow_net)),
      ("", "allow-env", "Allow environment access.", Some(&mut flags.allow_env)),
      ("", "recompile", "Force recompilation of TypeScript code.", Some(&mut flags.recompile)),
      ("h", "help", "Print this message.", Some(&mut flags.help)),
      ("D", "log-debug", "Log debug output.", Some(&mut flags.log_debug)),
      ("v", "version", "Print the version.", Some(&mut flags.version)),
      ("r", "reload", "Reload cached remote resources.", Some(&mut flags.reload)),
      ("", "v8-options", "Print V8 command line options.", None), // kept for generating usage string
      ("", "deps", "Print module dependencies.", Some(&mut flags.deps_flag)),
      ("", "types", "Print runtime TypeScript declarations.", Some(&mut flags.types_flag))
    ];
    // TODO(kevinkassimo): v8_set_flags intercepts '-help' with single '-'
    // Resolve that and then uncomment line below (enabling Go style -long-flag)
    // opts.long_only(true);
    opts.parsing_style(ParsingStyle::FloatingFrees);
    for (short_flag, long_flag, flag_desc, _) in flags_tuples.iter() {
      opts.optflag(&short_flag, &long_flag, &flag_desc);
    }
  
    let matches_ = match opts.parse(&args) {
      Ok(m) => m,
      Err(f) => {
        return Err(f.to_string());
      }
    };

    for (_, long_flag, _, maybe_flag_ref) in flags_tuples.iter_mut() {
      if matches_.opt_present(long_flag) {
        if let Some(flag_ref) = maybe_flag_ref {
          **flag_ref = true;
        }
      }
    }

    matches = Some(matches_);
  }
  let rest: Vec<_> = matches.unwrap().free.iter().map(|s| s.clone()).collect();
  return Ok((flags, rest, get_usage(&opts)));
}

[kevinkassimo]

@ry I checked backtrace (temporarily commenting out panic::set_hook) and it seems to be unrelated with getopts:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: AddrParseError(())', libcore/result.rs:945:5
...
   8: core::result::unwrap_failed
   9: <core::result::Result<T, E>>::unwrap
  10: deno_bin::ops::op_listen::{{closure}}

It is complaining that

deno/tests/http_bench.ts

Line 6 in a4fb517

const addr = deno.args[1] || "127.0.0.1:4500";

would pass "4501" to

deno/src/ops.rs

Line 1123 in a4fb517

let addr = SocketAddr::from_str(address).unwrap();

which is not a valid address, and the parsing would fail.

If you try ./out/debug/deno -D --allow-net tests/http_bench.ts 127.0.0.1:4501 it would work...

(We really need to have nice panic messages back...)

[ry]

@kevinkassimo My mistake - when I pass "127.0.0.1:4501" as the argument

 ./out/debug/deno --allow-net tests/http_bench.ts 127.0.0.1:4501

it's fine. This is due to bad Addr parsing on my part.

@kitsonk I like getopt - it's simpler.

[ry]

LGTM - Thank you!

(Maybe I'll give up on my crusade to have single dash options.)

[kitsonk]

(Maybe I'll give up on my crusade to have single dash options.)

Don't give up 😭 I am with you.

[kevinkassimo]

@ry not necessary. getopts has long_only that allows treating -- and - in the same way. The only thing need to resolve is the collision with v8's -help flag

[ry]

@kevinkassimo Dope. Can you comment what's need in #1014... which I will now reopen.