From 780e72ab6a092a6a7174c30bf4163857770d2ad0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sun, 7 Apr 2019 00:13:06 +0200 Subject: [PATCH] Refactor CLI flag parsing (#2025) --- Cargo.lock | 10 - cli/BUILD.gn | 1 - cli/Cargo.toml | 1 - cli/flags.rs | 346 +++++++++++++--------- cli/isolate.rs | 4 +- cli/isolate_state.rs | 2 +- cli/main.rs | 11 +- tests/001_hello.test | 2 +- tests/002_hello.test | 2 +- tests/003_relative_import.test | 2 +- tests/004_set_timeout.test | 2 +- tests/005_more_imports.test | 2 +- tests/006_url_imports.test | 2 +- tests/010_set_interval.test | 2 +- tests/012_async.test | 2 +- tests/016_double_await.test | 2 +- tests/017_import_redirect.test | 2 +- tests/018_async_catch.test | 2 +- tests/019_media_types.test | 2 +- tests/020_json_modules.test | 2 +- tests/022_info_flag.test | 2 +- tests/023_no_ext_with_headers.test | 2 +- tests/024_import_no_ext_with_headers.test | 2 +- tests/025_reload_js_type_error.test | 2 +- tests/026_redirect_javascript.js.test | 2 +- tests/026_workers.test | 4 +- tests/027_redirect_typescript.ts.test | 4 +- tests/028_args.test | 2 + tests/028_args.ts | 3 + tests/028_args.ts.out | 7 + tests/async_error.test | 2 +- tests/error_001.test | 2 +- tests/error_002.test | 2 +- tests/error_003_typescript.test | 2 +- tests/error_007_any.test | 2 +- tests/error_008_checkjs.test | 2 +- tests/error_syntax.test | 2 +- tests/exit_error42.test | 2 +- tests/https_import.test | 2 +- tests/if_main.test | 2 +- tests/import_meta.test | 2 +- tests/unbuffered_stderr.test | 2 +- tests/unbuffered_stdout.test | 2 +- tools/benchmark.py | 14 +- tools/fmt_test.py | 2 +- tools/permission_prompt_test.py | 31 +- tools/throughput_benchmark.py | 2 +- tools/unit_tests.py | 2 +- website/manual.md | 61 ++-- 49 files changed, 326 insertions(+), 243 deletions(-) create mode 100644 tests/028_args.test create mode 100644 tests/028_args.ts create mode 100644 tests/028_args.ts.out diff --git a/Cargo.lock b/Cargo.lock index 5b478ca8f5..312915d70b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -215,7 +215,6 @@ dependencies = [ "dirs 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)", "flatbuffers 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", "futures 0.1.25 (registry+https://github.com/rust-lang/crates.io-index)", - "getopts 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)", "http 0.1.16 (registry+https://github.com/rust-lang/crates.io-index)", "hyper 0.12.25 (registry+https://github.com/rust-lang/crates.io-index)", "hyper-rustls 0.16.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -317,14 +316,6 @@ dependencies = [ "num_cpus 1.10.0 (registry+https://github.com/rust-lang/crates.io-index)", ] -[[package]] -name = "getopts" -version = "0.2.18" -source = "registry+https://github.com/rust-lang/crates.io-index" -dependencies = [ - "unicode-width 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)", -] - [[package]] name = "h2" version = "0.1.17" @@ -1440,7 +1431,6 @@ dependencies = [ "checksum fuchsia-zircon-sys 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)" = "3dcaa9ae7725d12cdb85b3ad99a434db70b468c09ded17e012d86b5c1010f7a7" "checksum futures 0.1.25 (registry+https://github.com/rust-lang/crates.io-index)" = "49e7653e374fe0d0c12de4250f0bdb60680b8c80eed558c5c7538eec9c89e21b" "checksum futures-cpupool 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)" = "ab90cde24b3319636588d0c35fe03b1333857621051837ed769faefb4c2162e4" -"checksum getopts 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)" = "0a7292d30132fb5424b354f5dc02512a86e4c516fe544bb7a25e7f266951b797" "checksum h2 0.1.17 (registry+https://github.com/rust-lang/crates.io-index)" = "910a5e7be6283a9c91b3982fa5188368c8719cce2a3cf3b86048673bf9d9c36b" "checksum http 0.1.16 (registry+https://github.com/rust-lang/crates.io-index)" = "fe67e3678f2827030e89cc4b9e7ecd16d52f132c0b940ab5005f88e821500f6a" "checksum httparse 1.3.3 (registry+https://github.com/rust-lang/crates.io-index)" = "e8734b0cfd3bc3e101ec59100e101c2eecd19282202e87808b3037b442777a83" diff --git a/cli/BUILD.gn b/cli/BUILD.gn index ffc13891c9..ee032a757e 100644 --- a/cli/BUILD.gn +++ b/cli/BUILD.gn @@ -16,7 +16,6 @@ main_extern = [ "$rust_build:dirs", "$rust_build:flatbuffers", "$rust_build:futures", - "$rust_build:getopts", "$rust_build:http", "$rust_build:hyper", "$rust_build:hyper_rustls", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index f1b70b7a6a..9bebe84be8 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -24,7 +24,6 @@ clap = "2.32.0" dirs = "1.0.5" flatbuffers = "0.5.0" futures = "0.1.25" -getopts = "0.2.18" http = "0.1.16" hyper = "0.12.25" hyper-rustls = "0.16.1" diff --git a/cli/flags.rs b/cli/flags.rs index 9d187ecac8..cf1bb72bca 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -1,7 +1,6 @@ // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. +use clap::{App, AppSettings, Arg, ArgMatches, SubCommand}; use deno::v8_set_flags; -use getopts; -use getopts::Options; // Creates vector of strings, Vec #[cfg(test)] @@ -12,7 +11,6 @@ macro_rules! svec { #[cfg_attr(feature = "cargo-clippy", allow(stutter))] #[derive(Clone, Debug, PartialEq, Default)] pub struct DenoFlags { - pub help: bool, pub log_debug: bool, pub version: bool, pub reload: bool, @@ -28,16 +26,6 @@ pub struct DenoFlags { pub fmt: bool, } -pub fn get_usage(opts: &Options) -> String { - format!( - "Usage: deno script.ts {} -Environment variables: - DENO_DIR Set deno's base directory - NO_COLOR Set to disable color", - opts.usage("") - ) -} - /// Checks provided arguments for known options and sets appropriate Deno flags /// for them. Unknown options are returned for further use. /// Note: @@ -51,129 +39,217 @@ Environment variables: /// not cause an error. I also think this is ok because missing any of the /// privileged flags is not destructive. Userland flag parsing would catch these /// errors. -fn set_recognized_flags( - opts: &Options, - flags: &mut DenoFlags, - args: Vec, -) -> Result, getopts::Fail> { - let mut rest = Vec::::new(); - // getopts doesn't allow parsing unknown options so we check them - // one-by-one and handle unrecognized ones manually - // better solution welcome! - for arg in args { - let fake_args = vec![arg]; - match opts.parse(&fake_args) { - Err(getopts::Fail::UnrecognizedOption(_)) => { - rest.extend(fake_args); - } - Err(e) => { - return Err(e); - } - Ok(matches) => { - if matches.opt_present("help") { - flags.help = true; - } - if matches.opt_present("log-debug") { - flags.log_debug = true; - } - if matches.opt_present("version") { - flags.version = true; - } - if matches.opt_present("reload") { - flags.reload = true; - } - if matches.opt_present("allow-read") { - flags.allow_read = true; - } - if matches.opt_present("allow-write") { - flags.allow_write = true; - } - if matches.opt_present("allow-net") { - flags.allow_net = true; - } - if matches.opt_present("allow-env") { - flags.allow_env = true; - } - if matches.opt_present("allow-run") { - flags.allow_run = true; - } - if matches.opt_present("allow-all") { - flags.allow_read = true; - flags.allow_env = true; - flags.allow_net = true; - flags.allow_run = true; - flags.allow_read = true; - flags.allow_write = true; - } - if matches.opt_present("no-prompt") { - flags.no_prompts = true; - } - if matches.opt_present("types") { - flags.types = true; - } - if matches.opt_present("prefetch") { - flags.prefetch = true; - } - if matches.opt_present("info") { - flags.info = true; - } - if matches.opt_present("fmt") { - flags.fmt = true; - } - - if !matches.free.is_empty() { - rest.extend(matches.free); - } - } - } +fn set_recognized_flags(matches: ArgMatches, flags: &mut DenoFlags) { + if matches.is_present("log-debug") { + flags.log_debug = true; + } + if matches.is_present("version") { + flags.version = true; + } + if matches.is_present("reload") { + flags.reload = true; + } + if matches.is_present("allow-read") { + flags.allow_read = true; + } + if matches.is_present("allow-write") { + flags.allow_write = true; + } + if matches.is_present("allow-net") { + flags.allow_net = true; + } + if matches.is_present("allow-env") { + flags.allow_env = true; + } + if matches.is_present("allow-run") { + flags.allow_run = true; + } + if matches.is_present("allow-all") { + flags.allow_read = true; + flags.allow_env = true; + flags.allow_net = true; + flags.allow_run = true; + flags.allow_read = true; + flags.allow_write = true; + } + if matches.is_present("no-prompt") { + flags.no_prompts = true; + } + if matches.is_present("types") { + flags.types = true; + } + if matches.is_present("prefetch") { + flags.prefetch = true; + } + if matches.is_present("info") { + flags.info = true; + } + if matches.is_present("fmt") { + flags.fmt = true; } - Ok(rest) } #[cfg_attr(feature = "cargo-clippy", allow(stutter))] pub fn set_flags( args: Vec, -) -> Result<(DenoFlags, Vec, String), String> { - // TODO: all flags passed after "--" are swallowed by v8_set_flags - // eg. deno --allow-net ./test.ts -- --title foobar - // args === ["deno", "--allow-net" "./test.ts"] - let args = v8_set_flags(args); +) -> Result<(DenoFlags, Vec), String> { + let app_settings: Vec = vec![ + AppSettings::AllowExternalSubcommands, + AppSettings::DisableHelpSubcommand, + ]; - let mut opts = Options::new(); - // 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.optflag("", "allow-read", "Allow file system read access"); - opts.optflag("", "allow-write", "Allow file system write access"); - opts.optflag("", "allow-net", "Allow network access"); - opts.optflag("", "allow-env", "Allow environment access"); - opts.optflag("", "allow-run", "Allow running subprocesses"); - opts.optflag("A", "allow-all", "Allow all permissions"); - opts.optflag("", "no-prompt", "Do not use prompts"); - opts.optflag("h", "help", "Print this message"); - opts.optflag("D", "log-debug", "Log debug output"); - opts.optflag("v", "version", "Print the version"); - opts.optflag( - "r", - "reload", - "Reload source code cache (recompile TypeScript)", - ); - opts.optflag("", "v8-options", "Print V8 command line options"); - opts.optflag("", "types", "Print runtime TypeScript declarations"); - opts.optflag("", "prefetch", "Prefetch the dependencies"); - opts.optflag("", "info", "Show source file related info"); - opts.optflag("", "fmt", "Format code"); + let env_variables_help = "ENVIRONMENT VARIABLES: + DENO_DIR Set deno's base directory + NO_COLOR Set to disable color"; + + let clap_app = App::new("deno") + .global_settings(&vec![AppSettings::ColorNever]) + .settings(&app_settings[..]) + .after_help(env_variables_help) + .arg( + Arg::with_name("version") + .short("v") + .long("version") + .help("Print the version"), + ).arg( + Arg::with_name("allow-read") + .long("allow-read") + .help("Allow file system read access"), + ).arg( + Arg::with_name("allow-write") + .long("allow-write") + .help("Allow file system write access"), + ).arg( + Arg::with_name("allow-net") + .long("allow-net") + .help("Allow network access"), + ).arg( + Arg::with_name("allow-env") + .long("allow-env") + .help("Allow environment access"), + ).arg( + Arg::with_name("allow-run") + .long("allow-run") + .help("Allow running subprocesses"), + ).arg( + Arg::with_name("allow-all") + .short("A") + .long("allow-all") + .help("Allow all permissions"), + ).arg( + Arg::with_name("no-prompt") + .long("no-prompt") + .help("Do not use prompts"), + ).arg( + Arg::with_name("log-debug") + .short("D") + .long("log-debug") + .help("Log debug output"), + ).arg( + Arg::with_name("reload") + .short("r") + .long("reload") + .help("Reload source code cache (recompile TypeScript)"), + ).arg( + Arg::with_name("v8-options") + .long("v8-options") + .help("Print V8 command line options"), + ).arg( + Arg::with_name("v8-flags") + .long("v8-flags") + .takes_value(true) + .require_equals(true) + .help("Set V8 command line options"), + ).arg( + Arg::with_name("types") + .long("types") + .help("Print runtime TypeScript declarations"), + ).arg( + Arg::with_name("prefetch") + .long("prefetch") + .help("Prefetch the dependencies"), + ).subcommand( + // TODO(bartlomieju): version is not handled properly + SubCommand::with_name("info") + .about("Show source file related info") + .arg(Arg::with_name("file").takes_value(true).required(true)), + ).subcommand( + // TODO(bartlomieju): version is not handled properly + SubCommand::with_name("fmt").about("Format files").arg( + Arg::with_name("files") + .takes_value(true) + .multiple(true) + .required(true), + ), + ).subcommand( + // this is a fake subcommand - it's used in conjunction with + // AppSettings:AllowExternalSubcommand to treat it as an + // entry point script + SubCommand::with_name("