Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor CLI entry point #2157

Merged
merged 14 commits into from
Apr 21, 2019
Next Next commit
refactor CLI entry point
  • Loading branch information
bartlomieju committed Apr 20, 2019
commit bcd31fae9d943dc287c935a6d984e58ce90444d9
290 changes: 125 additions & 165 deletions cli/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ static ENV_VARIABLES_HELP: &str = "ENVIRONMENT VARIABLES:
DENO_DIR Set deno's base directory
NO_COLOR Set to disable color";

fn create_cli_app<'a, 'b>() -> App<'a, 'b> {
pub fn create_cli_app<'a, 'b>() -> App<'a, 'b> {
App::new("deno")
.bin_name("deno")
.global_settings(&[AppSettings::ColorNever])
Expand Down Expand Up @@ -198,46 +198,7 @@ fn create_cli_app<'a, 'b>() -> App<'a, 'b> {
}

#[cfg_attr(feature = "cargo-clippy", allow(stutter))]
pub fn set_flags(
args: Vec<String>,
) -> Result<(DenoFlags, Vec<String>), String> {
let mut rest_argv: Vec<String> = vec!["deno".to_string()];
let cli_app = create_cli_app();
let matches = cli_app.get_matches_from(args);

match matches.subcommand() {
("eval", Some(info_match)) => {
let code: &str = info_match.value_of("code").unwrap();
rest_argv.extend(vec![code.to_string()]);
}
("info", Some(info_match)) => {
let file: &str = info_match.value_of("file").unwrap();
rest_argv.extend(vec![file.to_string()]);
}
("fmt", Some(fmt_match)) => {
let files: Vec<String> = fmt_match
.values_of("files")
.unwrap()
.map(String::from)
.collect();
rest_argv.extend(files);
}
(script, Some(script_match)) => {
rest_argv.extend(vec![script.to_string()]);
// check if there are any extra arguments that should
// be passed to script
if script_match.is_present("") {
let script_args: Vec<String> = script_match
.values_of("")
.unwrap()
.map(String::from)
.collect();
rest_argv.extend(script_args);
}
}
_ => {}
}

pub fn set_flags(matches: ArgMatches) -> Result<DenoFlags, String> {
bartlomieju marked this conversation as resolved.
Show resolved Hide resolved
if matches.is_present("v8-options") {
// display v8 help and exit
// TODO(bartlomieju): this relies on `v8_set_flags` to swap `--v8-options` to help
Expand All @@ -256,138 +217,137 @@ pub fn set_flags(
}
bartlomieju marked this conversation as resolved.
Show resolved Hide resolved

let flags = DenoFlags::from(matches);
Ok((flags, rest_argv))
Ok(flags)
}

#[test]
fn test_set_flags_1() {
let (flags, rest) = set_flags(svec!["deno", "--version"]).unwrap();
assert_eq!(rest, svec!["deno"]);
assert_eq!(
flags,
DenoFlags {
version: true,
..DenoFlags::default()
}
);
}
#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_set_flags_2() {
let (flags, rest) =
set_flags(svec!["deno", "-r", "-D", "script.ts"]).unwrap();
assert_eq!(rest, svec!["deno", "script.ts"]);
assert_eq!(
flags,
DenoFlags {
log_debug: true,
reload: true,
..DenoFlags::default()
}
);
}
fn flags_from_vec(args: Vec<String>) -> DenoFlags {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this function should be the high level public “parse_flags” while the other function which is currently called “parse_flags” should be either combined into this or called something else (and be internal to this module)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I had similar feeling about it, but that means we no longer have matches var available in main(), that gives nice switch on matches.subcommand().

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively parse_flags may return DenoFlags, ArgMatches to handle both cases

let cli_app = create_cli_app();
let matches = cli_app.get_matches_from(args);
set_flags(matches).unwrap()
}

#[test]
fn test_set_flags_3() {
let (flags, rest) =
set_flags(svec!["deno", "-r", "--allow-write", "script.ts"]).unwrap();
assert_eq!(rest, svec!["deno", "script.ts"]);
assert_eq!(
flags,
DenoFlags {
reload: true,
allow_write: true,
..DenoFlags::default()
}
);
}
#[test]
fn test_set_flags_1() {
let flags = flags_from_vec(svec!["deno", "--version"]);
assert_eq!(
flags,
DenoFlags {
version: true,
..DenoFlags::default()
}
);
}

#[test]
fn test_set_flags_4() {
let (flags, rest) =
set_flags(svec!["deno", "-Dr", "--allow-write", "script.ts"]).unwrap();
assert_eq!(rest, svec!["deno", "script.ts"]);
assert_eq!(
flags,
DenoFlags {
log_debug: true,
reload: true,
allow_write: true,
..DenoFlags::default()
}
);
}
#[test]
fn test_set_flags_2() {
let flags = flags_from_vec(svec!["deno", "-r", "-D", "script.ts"]);
assert_eq!(
flags,
DenoFlags {
log_debug: true,
reload: true,
..DenoFlags::default()
}
);
}

#[test]
fn test_set_flags_5() {
let (flags, rest) = set_flags(svec!["deno", "--types"]).unwrap();
assert_eq!(rest, svec!["deno"]);
assert_eq!(
flags,
DenoFlags {
types: true,
..DenoFlags::default()
}
)
}
#[test]
fn test_set_flags_3() {
let flags =
flags_from_vec(svec!["deno", "-r", "--allow-write", "script.ts"]);
assert_eq!(
flags,
DenoFlags {
reload: true,
allow_write: true,
..DenoFlags::default()
}
);
}

#[test]
fn test_set_flags_6() {
let (flags, rest) =
set_flags(svec!["deno", "--allow-net", "gist.ts", "--title", "X"]).unwrap();
assert_eq!(rest, svec!["deno", "gist.ts", "--title", "X"]);
assert_eq!(
flags,
DenoFlags {
allow_net: true,
..DenoFlags::default()
}
)
}
#[test]
fn test_set_flags_4() {
let flags =
flags_from_vec(svec!["deno", "-Dr", "--allow-write", "script.ts"]);
assert_eq!(
flags,
DenoFlags {
log_debug: true,
reload: true,
allow_write: true,
..DenoFlags::default()
}
);
}

#[test]
fn test_set_flags_7() {
let (flags, rest) =
set_flags(svec!["deno", "--allow-all", "gist.ts"]).unwrap();
assert_eq!(rest, svec!["deno", "gist.ts"]);
assert_eq!(
flags,
DenoFlags {
allow_net: true,
allow_env: true,
allow_run: true,
allow_read: true,
allow_write: true,
allow_high_precision: true,
..DenoFlags::default()
}
)
}
#[test]
fn test_set_flags_5() {
let flags = flags_from_vec(svec!["deno", "--types"]);
assert_eq!(
flags,
DenoFlags {
types: true,
..DenoFlags::default()
}
)
}

#[test]
fn test_set_flags_8() {
let (flags, rest) =
set_flags(svec!["deno", "--allow-read", "gist.ts"]).unwrap();
assert_eq!(rest, svec!["deno", "gist.ts"]);
assert_eq!(
flags,
DenoFlags {
allow_read: true,
..DenoFlags::default()
}
)
}
#[test]
fn test_set_flags_6() {
let flags =
flags_from_vec(svec!["deno", "--allow-net", "gist.ts", "--title", "X"]);
assert_eq!(
flags,
DenoFlags {
allow_net: true,
..DenoFlags::default()
}
)
}

#[test]
fn test_set_flags_9() {
let (flags, rest) =
set_flags(svec!["deno", "--allow-high-precision", "script.ts"]).unwrap();
assert_eq!(rest, svec!["deno", "script.ts"]);
assert_eq!(
flags,
DenoFlags {
allow_high_precision: true,
..DenoFlags::default()
}
)
#[test]
fn test_set_flags_7() {
let flags = flags_from_vec(svec!["deno", "--allow-all", "gist.ts"]);
assert_eq!(
flags,
DenoFlags {
allow_net: true,
allow_env: true,
allow_run: true,
allow_read: true,
allow_write: true,
allow_high_precision: true,
..DenoFlags::default()
}
)
}

#[test]
fn test_set_flags_8() {
let flags = flags_from_vec(svec!["deno", "--allow-read", "gist.ts"]);
assert_eq!(
flags,
DenoFlags {
allow_read: true,
..DenoFlags::default()
}
)
}

#[test]
fn test_set_flags_9() {
let flags =
flags_from_vec(svec!["deno", "--allow-high-precision", "script.ts"]);
assert_eq!(
flags,
DenoFlags {
allow_high_precision: true,
..DenoFlags::default()
}
)
}
}
Loading