From ae4f8438d37acc142d6a6599a020354cf57e0e3b Mon Sep 17 00:00:00 2001 From: Jesper van den Ende Date: Tue, 14 Dec 2021 23:41:25 +0100 Subject: [PATCH 1/9] feat(watch): support watching external files --- cli/flags.rs | 48 +++++++++++++++++++++++++++++------------ cli/main.rs | 16 +++++++++----- cli/tools/standalone.rs | 2 +- 3 files changed, 46 insertions(+), 20 deletions(-) diff --git a/cli/flags.rs b/cli/flags.rs index a879be299e076d..66b561209cf17e 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -251,7 +251,7 @@ pub struct Flags { pub unsafely_ignore_certificate_errors: Option>, pub v8_flags: Vec, pub version: bool, - pub watch: bool, + pub watch: Option>, } fn join_paths(allowlist: &[PathBuf], d: &str) -> String { @@ -1643,10 +1643,16 @@ fn compat_arg<'a, 'b>() -> Arg<'a, 'b> { fn watch_arg<'a, 'b>() -> Arg<'a, 'b> { Arg::with_name("watch") .long("watch") + .value_name("FILES") + .min_values(0) + .takes_value(true) + .use_delimiter(true) + .require_equals(true) .help("UNSTABLE: Watch for file changes and restart process automatically") .long_help( "UNSTABLE: Watch for file changes and restart process automatically. -Only local files from entry point module graph are watched.", +Watches used script files by default, or listed files when specified. +Passing values only has an effect on the 'deno run' command.", ) } @@ -1743,7 +1749,7 @@ fn bundle_parse(flags: &mut Flags, matches: &clap::ArgMatches) { None }; - flags.watch = matches.is_present("watch"); + watch_arg_parse(flags, matches, false); flags.subcommand = DenoSubcommand::Bundle(BundleFlags { source_file, @@ -1874,7 +1880,7 @@ fn eval_parse(flags: &mut Flags, matches: &clap::ArgMatches) { fn fmt_parse(flags: &mut Flags, matches: &clap::ArgMatches) { config_arg_parse(flags, matches); - flags.watch = matches.is_present("watch"); + watch_arg_parse(flags, matches, false); let files = match matches.values_of("files") { Some(f) => f.map(PathBuf::from).collect(), None => vec![], @@ -1996,7 +2002,7 @@ fn lsp_parse(flags: &mut Flags, _matches: &clap::ArgMatches) { fn lint_parse(flags: &mut Flags, matches: &clap::ArgMatches) { config_arg_parse(flags, matches); - flags.watch = matches.is_present("watch"); + watch_arg_parse(flags, matches, false); let files = match matches.values_of("files") { Some(f) => f.map(PathBuf::from).collect(), None => vec![], @@ -2061,7 +2067,7 @@ fn run_parse(flags: &mut Flags, matches: &clap::ArgMatches) { flags.argv.push(v); } - flags.watch = matches.is_present("watch"); + watch_arg_parse(flags, matches, true); flags.subcommand = DenoSubcommand::Run(RunFlags { script }); } @@ -2135,7 +2141,7 @@ fn test_parse(flags: &mut Flags, matches: &clap::ArgMatches) { }; flags.coverage_dir = matches.value_of("coverage").map(String::from); - flags.watch = matches.is_present("watch"); + watch_arg_parse(flags, matches, false); flags.subcommand = DenoSubcommand::Test(TestFlags { no_run, doc, @@ -2409,6 +2415,20 @@ fn inspect_arg_validate(val: String) -> Result<(), String> { } } +fn watch_arg_parse(flags: &mut Flags, matches: &clap::ArgMatches, allow_extra: bool) { + flags.watch = match matches.values_of("watch") { + Some(f) => { + let extra_files = f.map(PathBuf::from).collect(); + if allow_extra { + Some(extra_files) + } else { + Some(vec![]) + } + }, + None => None, + }; +} + // TODO(ry) move this to utility module and add test. /// Strips fragment part of URL. Panics on bad URL. pub fn resolve_urls(urls: Vec) -> Vec { @@ -2513,7 +2533,7 @@ mod tests { subcommand: DenoSubcommand::Run(RunFlags { script: "script.ts".to_string(), }), - watch: true, + watch: Some(vec![]), ..Flags::default() } ); @@ -2746,7 +2766,7 @@ mod tests { single_quote: None, prose_wrap: None, }), - watch: true, + watch: Some(vec![]), ..Flags::default() } ); @@ -2773,7 +2793,7 @@ mod tests { single_quote: None, prose_wrap: None, }), - watch: true, + watch: Some(vec![]), ..Flags::default() } ); @@ -2821,7 +2841,7 @@ mod tests { prose_wrap: None, }), config_path: Some("deno.jsonc".to_string()), - watch: true, + watch: Some(vec![]), ..Flags::default() } ); @@ -3543,7 +3563,7 @@ mod tests { source_file: "source.ts".to_string(), out_file: None, }), - watch: true, + watch: Some(vec![]), ..Flags::default() } ) @@ -4280,7 +4300,7 @@ mod tests { ignore: vec![], concurrent_jobs: NonZeroUsize::new(1).unwrap(), }), - watch: false, + watch: None, ..Flags::default() } ); @@ -4303,7 +4323,7 @@ mod tests { ignore: vec![], concurrent_jobs: NonZeroUsize::new(1).unwrap(), }), - watch: true, + watch: Some(vec![]), ..Flags::default() } ); diff --git a/cli/main.rs b/cli/main.rs index 891aa1ac5930e5..5b5fcd3a91fbb4 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -577,7 +577,7 @@ async fn lint_command( None }; - tools::lint::lint(maybe_lint_config, lint_flags, flags.watch).await?; + tools::lint::lint(maybe_lint_config, lint_flags, flags.watch.is_some()).await?; Ok(0) } @@ -894,7 +894,7 @@ async fn bundle_command( } }; - if flags.watch { + if flags.watch.is_some() { file_watcher::watch_func(resolver, operation, "Bundle").await?; } else { let module_graph = @@ -943,7 +943,7 @@ async fn format_command( return Ok(0); } - tools::fmt::format(fmt_flags, flags.watch, maybe_fmt_config).await?; + tools::fmt::format(fmt_flags, flags.watch.is_some(), maybe_fmt_config).await?; Ok(0) } @@ -1011,6 +1011,7 @@ async fn run_with_watch(flags: Flags, script: String) -> Result { let script1 = script.clone(); let script2 = script.clone(); let flags = flags.clone(); + let watch_flag = flags.watch.clone(); async move { let main_module = resolve_url_or_path(&script1)?; let ps = ProcState::build(flags).await?; @@ -1067,6 +1068,11 @@ async fn run_with_watch(flags: Flags, script: String) -> Result { }) .collect(); + // Add the extra files listed in the watch flag + if let Some(watch_paths) = watch_flag { + paths_to_watch.extend(watch_paths); + } + if let Some(import_map) = ps.flags.import_map_path.as_ref() { paths_to_watch .push(fs_util::resolve_from_cwd(std::path::Path::new(import_map))?); @@ -1180,7 +1186,7 @@ async fn run_command( return run_from_stdin(flags).await; } - if flags.watch { + if flags.watch.is_some() { return run_with_watch(flags, run_flags.script).await; } @@ -1294,7 +1300,7 @@ async fn test_command( ); } - if flags.watch { + if flags.watch.is_some() { tools::test::run_tests_with_watch( flags, test_flags.include, diff --git a/cli/tools/standalone.rs b/cli/tools/standalone.rs index d8ec14dc605562..2277c76973fdca 100644 --- a/cli/tools/standalone.rs +++ b/cli/tools/standalone.rs @@ -239,6 +239,6 @@ pub fn compile_to_runtime_flags( unstable: flags.unstable, v8_flags: flags.v8_flags, version: false, - watch: false, + watch: None, }) } From e58cf421e9cd5fbbbfd2afe194473bb1756ec17f Mon Sep 17 00:00:00 2001 From: Jesper van den Ende Date: Wed, 15 Dec 2021 00:37:03 +0100 Subject: [PATCH 2/9] test(watch) add external watch files test --- cli/tests/integration/watcher_tests.rs | 37 ++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/cli/tests/integration/watcher_tests.rs b/cli/tests/integration/watcher_tests.rs index ddfccd8376434e..db448258d22b37 100644 --- a/cli/tests/integration/watcher_tests.rs +++ b/cli/tests/integration/watcher_tests.rs @@ -575,6 +575,43 @@ fn run_watch() { check_alive_then_kill(child); } +#[test] +fn run_watch_external_watch_files() { + let t = TempDir::new().unwrap(); + let file_to_watch = t.path().join("file_to_watch.js"); + write(&file_to_watch, "console.log('Hello world');").unwrap(); + + let external_file_to_watch = t.path().join("external_file_to_watch.txt"); + write(&external_file_to_watch, "Hello world").unwrap(); + + let mut watch_arg = "--watch=".to_owned(); + let external_file_to_watch_str = external_file_to_watch.clone().into_os_string().into_string().unwrap(); + watch_arg.push_str(&external_file_to_watch_str); + + let mut child = util::deno_cmd() + .current_dir(util::testdata_path()) + .arg("run") + .arg(watch_arg) + .arg("--unstable") + .arg(&file_to_watch) + .env("NO_COLOR", "1") + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .unwrap(); + let (mut stdout_lines, mut stderr_lines) = child_lines(&mut child); + + assert_contains!(stdout_lines.next().unwrap(), "Hello world"); + wait_for("Process finished", &mut stderr_lines); + + // Change content of the external file + write(&external_file_to_watch, "Hello world2").unwrap(); + + assert_contains!(stderr_lines.next().unwrap(), "Restarting"); + wait_for("Process finished", &mut stderr_lines); + check_alive_then_kill(child); +} + #[test] fn run_watch_load_unload_events() { let t = TempDir::new().unwrap(); From 0fc1d449ab310d55dfba1ba60bdeb7c2bf4db02e Mon Sep 17 00:00:00 2001 From: Jesper van den Ende Date: Wed, 15 Dec 2021 00:40:07 +0100 Subject: [PATCH 3/9] lint --- cli/flags.rs | 8 ++++++-- cli/main.rs | 6 ++++-- cli/tests/integration/watcher_tests.rs | 6 +++++- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/cli/flags.rs b/cli/flags.rs index 66b561209cf17e..f0738feccdf47e 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -2415,7 +2415,11 @@ fn inspect_arg_validate(val: String) -> Result<(), String> { } } -fn watch_arg_parse(flags: &mut Flags, matches: &clap::ArgMatches, allow_extra: bool) { +fn watch_arg_parse( + flags: &mut Flags, + matches: &clap::ArgMatches, + allow_extra: bool, +) { flags.watch = match matches.values_of("watch") { Some(f) => { let extra_files = f.map(PathBuf::from).collect(); @@ -2424,7 +2428,7 @@ fn watch_arg_parse(flags: &mut Flags, matches: &clap::ArgMatches, allow_extra: b } else { Some(vec![]) } - }, + } None => None, }; } diff --git a/cli/main.rs b/cli/main.rs index 5b5fcd3a91fbb4..bffe6f329a66d6 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -577,7 +577,8 @@ async fn lint_command( None }; - tools::lint::lint(maybe_lint_config, lint_flags, flags.watch.is_some()).await?; + tools::lint::lint(maybe_lint_config, lint_flags, flags.watch.is_some()) + .await?; Ok(0) } @@ -943,7 +944,8 @@ async fn format_command( return Ok(0); } - tools::fmt::format(fmt_flags, flags.watch.is_some(), maybe_fmt_config).await?; + tools::fmt::format(fmt_flags, flags.watch.is_some(), maybe_fmt_config) + .await?; Ok(0) } diff --git a/cli/tests/integration/watcher_tests.rs b/cli/tests/integration/watcher_tests.rs index db448258d22b37..02367d7808fd5e 100644 --- a/cli/tests/integration/watcher_tests.rs +++ b/cli/tests/integration/watcher_tests.rs @@ -585,7 +585,11 @@ fn run_watch_external_watch_files() { write(&external_file_to_watch, "Hello world").unwrap(); let mut watch_arg = "--watch=".to_owned(); - let external_file_to_watch_str = external_file_to_watch.clone().into_os_string().into_string().unwrap(); + let external_file_to_watch_str = external_file_to_watch + .clone() + .into_os_string() + .into_string() + .unwrap(); watch_arg.push_str(&external_file_to_watch_str); let mut child = util::deno_cmd() From b026f460783fb6a5d5ba7e22e24411e5d1be72ab Mon Sep 17 00:00:00 2001 From: Jesper van den Ende Date: Wed, 15 Dec 2021 01:27:33 +0100 Subject: [PATCH 4/9] =?UTF-8?q?Change=20=E2=80=94help=20text?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bartek Iwańczuk --- cli/flags.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cli/flags.rs b/cli/flags.rs index f0738feccdf47e..ab838bd237dd9f 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -1651,8 +1651,7 @@ fn watch_arg<'a, 'b>() -> Arg<'a, 'b> { .help("UNSTABLE: Watch for file changes and restart process automatically") .long_help( "UNSTABLE: Watch for file changes and restart process automatically. -Watches used script files by default, or listed files when specified. -Passing values only has an effect on the 'deno run' command.", +Local files from entry point module graph are watched by default. Additional paths might be watched by passing them as arguments to this flag. ) } From 316a5ea8ca87f9055235bb9542a7e29a27a9c5e5 Mon Sep 17 00:00:00 2001 From: Jesper van den Ende Date: Wed, 15 Dec 2021 01:34:49 +0100 Subject: [PATCH 5/9] Lint --- cli/flags.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cli/flags.rs b/cli/flags.rs index ab838bd237dd9f..afb03fbeee1e80 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -1651,7 +1651,8 @@ fn watch_arg<'a, 'b>() -> Arg<'a, 'b> { .help("UNSTABLE: Watch for file changes and restart process automatically") .long_help( "UNSTABLE: Watch for file changes and restart process automatically. -Local files from entry point module graph are watched by default. Additional paths might be watched by passing them as arguments to this flag. +Local files from entry point module graph are watched by default. +Additional paths might be watched by passing them as arguments to this flag.", ) } From 5146e771cb861057e05bec0dede52c998fe8d452 Mon Sep 17 00:00:00 2001 From: Jesper van den Ende Date: Wed, 15 Dec 2021 19:39:55 +0100 Subject: [PATCH 6/9] Only take values in `--watch` for `run` command --- cli/flags.rs | 39 ++++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/cli/flags.rs b/cli/flags.rs index afb03fbeee1e80..af799bbde2d5c5 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -549,7 +549,7 @@ fn bundle_subcommand<'a, 'b>() -> App<'a, 'b> { .required(true), ) .arg(Arg::with_name("out_file").takes_value(true).required(false)) - .arg(watch_arg()) + .arg(watch_arg(false)) .about("Bundle module and dependencies into single file") .long_about( "Output a single JavaScript file with all dependencies. @@ -882,7 +882,7 @@ Ignore formatting a file by adding an ignore comment at the top of the file: .multiple(true) .required(false), ) - .arg(watch_arg()) + .arg(watch_arg(false)) .arg( Arg::with_name("options-use-tabs") .long("options-use-tabs") @@ -1158,7 +1158,7 @@ Ignore linting a file by adding an ignore comment at the top of the file: .multiple(true) .required(false), ) - .arg(watch_arg()) + .arg(watch_arg(false)) } fn repl_subcommand<'a, 'b>() -> App<'a, 'b> { @@ -1177,7 +1177,7 @@ fn repl_subcommand<'a, 'b>() -> App<'a, 'b> { fn run_subcommand<'a, 'b>() -> App<'a, 'b> { runtime_args(SubCommand::with_name("run"), true, true) .arg( - watch_arg() + watch_arg(true) .conflicts_with("inspect") .conflicts_with("inspect-brk"), ) @@ -1305,7 +1305,7 @@ fn test_subcommand<'a, 'b>() -> App<'a, 'b> { .multiple(true), ) .arg( - watch_arg() + watch_arg(false) .conflicts_with("no-run") .conflicts_with("coverage"), ) @@ -1640,20 +1640,29 @@ fn compat_arg<'a, 'b>() -> Arg<'a, 'b> { .help("Node compatibility mode. Currently only enables built-in node modules like 'fs' and globals like 'process'.") } -fn watch_arg<'a, 'b>() -> Arg<'a, 'b> { - Arg::with_name("watch") +fn watch_arg<'a, 'b>(takes_files: bool) -> Arg<'a, 'b> { + let arg = Arg::with_name("watch") .long("watch") - .value_name("FILES") - .min_values(0) - .takes_value(true) - .use_delimiter(true) - .require_equals(true) - .help("UNSTABLE: Watch for file changes and restart process automatically") - .long_help( - "UNSTABLE: Watch for file changes and restart process automatically. + .help("UNSTABLE: Watch for file changes and restart process automatically"); + + if takes_files { + arg + .value_name("FILES") + .min_values(0) + .takes_value(true) + .use_delimiter(true) + .require_equals(true) + .long_help( + "UNSTABLE: Watch for file changes and restart process automatically. Local files from entry point module graph are watched by default. Additional paths might be watched by passing them as arguments to this flag.", + ) + } else { + arg.long_help( + "UNSTABLE: Watch for file changes and restart process automatically. +Only local files from entry point module graph are watched.", ) + } } fn no_check_arg<'a, 'b>() -> Arg<'a, 'b> { From 7b9d62230ee45348bf341ac09680b1d33f492278 Mon Sep 17 00:00:00 2001 From: Jesper van den Ende Date: Wed, 15 Dec 2021 20:00:26 +0100 Subject: [PATCH 7/9] Cleanup watch arg parsing --- cli/flags.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/cli/flags.rs b/cli/flags.rs index af799bbde2d5c5..e0502140d68e88 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -2429,17 +2429,13 @@ fn watch_arg_parse( matches: &clap::ArgMatches, allow_extra: bool, ) { - flags.watch = match matches.values_of("watch") { - Some(f) => { - let extra_files = f.map(PathBuf::from).collect(); - if allow_extra { - Some(extra_files) - } else { - Some(vec![]) - } + if allow_extra { + if let Some(f) = matches.values_of("watch") { + flags.watch = Some(f.map(PathBuf::from).collect()); } - None => None, - }; + } else if matches.is_present("watch") { + flags.watch = Some(vec![]); + } } // TODO(ry) move this to utility module and add test. From c1d6655edcbabad769ef09d02509aebb6547483d Mon Sep 17 00:00:00 2001 From: Jesper van den Ende Date: Wed, 15 Dec 2021 21:18:13 +0100 Subject: [PATCH 8/9] test(watch): add run_watch_with_external test --- cli/flags.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/cli/flags.rs b/cli/flags.rs index e0502140d68e88..27aa9c14fa8ff9 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -2548,6 +2548,23 @@ mod tests { ); } + + #[test] + fn run_watch_with_external() { + let r = flags_from_vec(svec!["deno", "run", "--watch=file1,file2", "script.ts"]); + let flags = r.unwrap(); + assert_eq!( + flags, + Flags { + subcommand: DenoSubcommand::Run(RunFlags { + script: "script.ts".to_string(), + }), + watch: Some(vec![PathBuf::from("file1"), PathBuf::from("file2")]), + ..Flags::default() + } + ); + } + #[test] fn run_reload_allow_write() { let r = From f1d000915ff0a727964b725a66211ebf402aa7f6 Mon Sep 17 00:00:00 2001 From: Jesper van den Ende Date: Wed, 15 Dec 2021 21:21:15 +0100 Subject: [PATCH 9/9] Format --- cli/flags.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/flags.rs b/cli/flags.rs index 27aa9c14fa8ff9..452c2fd9a51600 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -2548,10 +2548,10 @@ mod tests { ); } - #[test] fn run_watch_with_external() { - let r = flags_from_vec(svec!["deno", "run", "--watch=file1,file2", "script.ts"]); + let r = + flags_from_vec(svec!["deno", "run", "--watch=file1,file2", "script.ts"]); let flags = r.unwrap(); assert_eq!( flags,