fix CLI arg for v8 flags

[bartlomieju]

This PR resolves problem from #2112

CC @kevinkassimo you can apply this fix now, I'll figure out a way to test it properly in the morning

[kevinkassimo]

@bartlomieju Great, thank you

[bartlomieju]

@ry please don't merge yet

[ry]

@bartlomieju Will you add a test for this?

[bartlomieju]

@ry yes, that's why I didn't want it merged. I was caught up in work this week, but should be able to finish it today/tomorrow.

[bartlomieju]

@ry I've added integration test, is it sufficient?

[ry]

@bartlomieju I’d prefer a unit test in cli/flags.rs (easier to reason about)

[bartlomieju]

@ry that makes sense. I'll try to do that - my first thought was to mock out v8_set_flags but it looks like it's not so straight-forward in Rust. If you had something concrete in mind any tip would be much appreciated 😅

[ry]

@bartlomieju So I think there should be a test like this:

#[test]
fn test_set_flags_v8_flags() {
  let (flags, rest) = set_flags(svec!["deno",  "--v8-flags=--enable-gc,--gc-stats=1"]).unwrap();
  assert_eq!(rest, svec!["deno"]);
  assert_eq!(
    flags,
    DenoFlags {
      version: true,
      v8_flags: vec!["--enable-gc".to_string(), "--gc-stats=1".to_string()],
      ..DenoFlags::default()
    }
  );
}

The set_flags() function seems a bit messy in that it's partially parsing (cli_app.get_matches_from(args)) and partially handling flags (calling v8_set_flags).

I think this is a good time to clean this up, since this patch is relatively simple. I think the main thing is that v8_set_flags() should be called in a separate place (maybe even in main()?) from where the flag parsing happens. Consider combining DenoFlags::<ArgMatches>::from() with set_flags into a new function called parse_flags(args: Vec<String>) -> (DenoFlags, Vec<String>) (AFAICT the Result return value in set_flags() is unused - there are no places where an error is returned.)

(Also I think this --v8-flags and --v8-options stuff is quite messy and not user friendly. But we can fix that later.)

[bartlomieju]

@ry thanks for reply, regarding cleanup, please see #2157 - I did some heavy refactors regarding flag parsing there.

[ry]

@bartlomieju cool - let's just move the discuss there and I'll close this.