From 5490cfed2000a063ef0baec500ab7d539203067c Mon Sep 17 00:00:00 2001 From: Zheyu Zhang Date: Tue, 1 Feb 2022 00:39:39 +0800 Subject: [PATCH] feat(cli): add "--no-clear-screen" flag (#13454) This commit adds "--no-clear-screen" flag which can be used with "--watch" flag to disable clearing of terminal screen on each file change. --- cli/file_watcher.rs | 44 ++++-- cli/flags.rs | 178 +++++++++++++++++++++++++ cli/main.rs | 33 +++-- cli/tests/integration/watcher_tests.rs | 48 +++++++ cli/tools/fmt.rs | 16 ++- cli/tools/lint.rs | 28 ++-- cli/tools/standalone.rs | 1 + cli/tools/test.rs | 10 +- 8 files changed, 321 insertions(+), 37 deletions(-) diff --git a/cli/file_watcher.rs b/cli/file_watcher.rs index 87531bb77b..fd7d4e1a1c 100644 --- a/cli/file_watcher.rs +++ b/cli/file_watcher.rs @@ -91,18 +91,19 @@ where paths_to_watch, result, } => { - // Clear screen first - eprint!("{}", CLEAR_SCREEN); - info!( - "{} File change detected! Restarting!", - colors::intense_blue("Watcher"), - ); return (paths_to_watch, result); } } } } +pub struct PrintConfig { + /// printing watcher status to terminal. + pub job_name: String, + /// determine whether to clear the terminal screen + pub clear_screen: bool, +} + /// Creates a file watcher, which will call `resolver` with every file change. /// /// - `resolver` is used for resolving file paths to be watched at every restarting @@ -113,12 +114,10 @@ where /// - `operation` is the actual operation we want to run every time the watcher detects file /// changes. For example, in the case where we would like to bundle, then `operation` would /// have the logic for it like bundling the code. -/// -/// - `job_name` is just used for printing watcher status to terminal. pub async fn watch_func( mut resolver: R, mut operation: O, - job_name: &str, + print_config: PrintConfig, ) -> Result<(), AnyError> where R: FnMut(Option>) -> F1, @@ -128,11 +127,26 @@ where { let (sender, mut receiver) = DebouncedReceiver::new_with_sender(); + let PrintConfig { + job_name, + clear_screen, + } = print_config; + // Store previous data. If module resolution fails at some point, the watcher will try to // continue watching files using these data. let mut paths_to_watch; let mut resolution_result; + let print_after_restart = || { + if clear_screen { + eprint!("{}", CLEAR_SCREEN); + } + info!( + "{} File change detected! Restarting!", + colors::intense_blue("Watcher"), + ); + }; + match resolver(None).await { ResolutionResult::Ignore => { // The only situation where it makes sense to ignore the initial 'change' @@ -149,6 +163,8 @@ where let (paths, result) = next_restart(&mut resolver, &mut receiver).await; paths_to_watch = paths; resolution_result = result; + + print_after_restart(); } ResolutionResult::Restart { paths_to_watch: paths, @@ -159,8 +175,10 @@ where } }; - // Clear screen first - eprint!("{}", CLEAR_SCREEN); + if clear_screen { + eprint!("{}", CLEAR_SCREEN); + } + info!("{} {} started.", colors::intense_blue("Watcher"), job_name,); loop { @@ -175,6 +193,8 @@ where paths_to_watch = paths; } resolution_result = result; + + print_after_restart(); continue; }, _ = fut => {}, @@ -202,6 +222,8 @@ where } resolution_result = result; + print_after_restart(); + drop(watcher); } } diff --git a/cli/flags.rs b/cli/flags.rs index 8b308658da..d4ba685fb8 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -254,6 +254,7 @@ pub struct Flags { pub v8_flags: Vec, pub version: bool, pub watch: Option>, + pub no_clear_screen: bool, } fn join_paths(allowlist: &[PathBuf], d: &str) -> String { @@ -561,6 +562,7 @@ fn bundle_subcommand<'a>() -> App<'a> { .arg(Arg::new("source_file").takes_value(true).required(true)) .arg(Arg::new("out_file").takes_value(true).required(false)) .arg(watch_arg(false)) + .arg(no_clear_screen_arg()) .about("Bundle module and dependencies into single file") .long_about( "Output a single JavaScript file with all dependencies. @@ -900,6 +902,7 @@ Ignore formatting a file by adding an ignore comment at the top of the file: .required(false), ) .arg(watch_arg(false)) + .arg(no_clear_screen_arg()) .arg( Arg::new("options-use-tabs") .long("options-use-tabs") @@ -1169,6 +1172,7 @@ Ignore linting a file by adding an ignore comment at the top of the file: .required(false), ) .arg(watch_arg(false)) + .arg(no_clear_screen_arg()) } fn repl_subcommand<'a>() -> App<'a> { @@ -1191,6 +1195,7 @@ fn run_subcommand<'a>() -> App<'a> { .conflicts_with("inspect") .conflicts_with("inspect-brk"), ) + .arg(no_clear_screen_arg()) .setting(AppSettings::TrailingVarArg) .arg(script_arg().required(true)) .about("Run a JavaScript or TypeScript program") @@ -1320,6 +1325,7 @@ fn test_subcommand<'a>() -> App<'a> { .conflicts_with("no-run") .conflicts_with("coverage"), ) + .arg(no_clear_screen_arg()) .arg(script_arg().last(true)) .about("Run tests") .long_about( @@ -1672,6 +1678,13 @@ Only local files from entry point module graph are watched.", } } +fn no_clear_screen_arg<'a>() -> Arg<'a> { + Arg::new("no-clear-screen") + .requires("watch") + .long("no-clear-screen") + .help("Do not clear terminal screen when under watch mode") +} + fn no_check_arg<'a>() -> Arg<'a> { Arg::new("no-check") .takes_value(true) @@ -1910,6 +1923,7 @@ fn eval_parse(flags: &mut Flags, matches: &clap::ArgMatches) { fn fmt_parse(flags: &mut Flags, matches: &clap::ArgMatches) { config_arg_parse(flags, matches); watch_arg_parse(flags, matches, false); + let files = match matches.values_of("files") { Some(f) => f.map(PathBuf::from).collect(), None => vec![], @@ -2451,6 +2465,10 @@ fn watch_arg_parse( } else if matches.is_present("watch") { flags.watch = Some(vec![]); } + + if matches.is_present("no-clear-screen") { + flags.no_clear_screen = true; + } } // TODO(ry) move this to utility module and add test. @@ -2580,6 +2598,30 @@ mod tests { ); } + #[test] + fn run_watch_with_no_clear_screen() { + let r = flags_from_vec(svec![ + "deno", + "run", + "--watch", + "--no-clear-screen", + "script.ts" + ]); + + let flags = r.unwrap(); + assert_eq!( + flags, + Flags { + subcommand: DenoSubcommand::Run(RunFlags { + script: "script.ts".to_string(), + }), + watch: Some(vec![]), + no_clear_screen: true, + ..Flags::default() + } + ); + } + #[test] fn run_reload_allow_write() { let r = @@ -2813,6 +2855,28 @@ mod tests { } ); + let r = + flags_from_vec(svec!["deno", "fmt", "--watch", "--no-clear-screen"]); + assert_eq!( + r.unwrap(), + Flags { + subcommand: DenoSubcommand::Fmt(FmtFlags { + ignore: vec![], + check: false, + files: vec![], + ext: "ts".to_string(), + use_tabs: None, + line_width: None, + indent_width: None, + single_quote: None, + prose_wrap: None, + }), + watch: Some(vec![]), + no_clear_screen: true, + ..Flags::default() + } + ); + let r = flags_from_vec(svec![ "deno", "fmt", @@ -2941,6 +3005,62 @@ mod tests { } ); + let r = flags_from_vec(svec![ + "deno", + "lint", + "--watch", + "script_1.ts", + "script_2.ts" + ]); + assert_eq!( + r.unwrap(), + Flags { + subcommand: DenoSubcommand::Lint(LintFlags { + files: vec![ + PathBuf::from("script_1.ts"), + PathBuf::from("script_2.ts") + ], + rules: false, + maybe_rules_tags: None, + maybe_rules_include: None, + maybe_rules_exclude: None, + json: false, + ignore: vec![], + }), + watch: Some(vec![]), + ..Flags::default() + } + ); + + let r = flags_from_vec(svec![ + "deno", + "lint", + "--watch", + "--no-clear-screen", + "script_1.ts", + "script_2.ts" + ]); + assert_eq!( + r.unwrap(), + Flags { + subcommand: DenoSubcommand::Lint(LintFlags { + files: vec![ + PathBuf::from("script_1.ts"), + PathBuf::from("script_2.ts") + ], + rules: false, + maybe_rules_tags: None, + maybe_rules_include: None, + maybe_rules_exclude: None, + json: false, + ignore: vec![], + }), + watch: Some(vec![]), + no_clear_screen: true, + ..Flags::default() + } + ); + let r = flags_from_vec(svec!["deno", "lint", "--ignore=script_1.ts,script_2.ts"]); assert_eq!( @@ -3611,6 +3731,29 @@ mod tests { ) } + #[test] + fn bundle_watch_with_no_clear_screen() { + let r = flags_from_vec(svec![ + "deno", + "bundle", + "--watch", + "--no-clear-screen", + "source.ts" + ]); + assert_eq!( + r.unwrap(), + Flags { + subcommand: DenoSubcommand::Bundle(BundleFlags { + source_file: "source.ts".to_string(), + out_file: None, + }), + watch: Some(vec![]), + no_clear_screen: true, + ..Flags::default() + } + ) + } + #[test] fn run_import_map() { let r = flags_from_vec(svec![ @@ -4371,6 +4514,31 @@ mod tests { ); } + #[test] + fn test_watch_with_no_clear_screen() { + let r = + flags_from_vec(svec!["deno", "test", "--watch", "--no-clear-screen"]); + assert_eq!( + r.unwrap(), + Flags { + subcommand: DenoSubcommand::Test(TestFlags { + no_run: false, + doc: false, + fail_fast: None, + filter: None, + allow_none: false, + shuffle: None, + include: None, + ignore: vec![], + concurrent_jobs: NonZeroUsize::new(1).unwrap(), + }), + watch: Some(vec![]), + no_clear_screen: true, + ..Flags::default() + } + ); + } + #[test] fn bundle_with_cafile() { let r = flags_from_vec(svec![ @@ -4676,4 +4844,14 @@ mod tests { vec![PathBuf::from("dir/a.js"), PathBuf::from("dir/b.js")] ); } + + #[test] + fn test_no_clear_watch_flag_without_watch_flag() { + let r = flags_from_vec(svec!["deno", "run", "--no-clear-screen", "foo.js"]); + assert!(r.is_err()); + let error_message = r.unwrap_err().to_string(); + assert!(&error_message + .contains("error: The following required arguments were not provided:")); + assert!(&error_message.contains("--watch=...")); + } } diff --git a/cli/main.rs b/cli/main.rs index c996425c72..b6bc0c7ad6 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -529,15 +529,7 @@ async fn lint_command( return Ok(0); } - let ps = ProcState::build(flags.clone()).await?; - let maybe_lint_config = if let Some(config_file) = &ps.maybe_config_file { - config_file.to_lint_config()? - } else { - None - }; - - tools::lint::lint(maybe_lint_config, lint_flags, flags.watch.is_some()) - .await?; + tools::lint::lint(flags, lint_flags).await?; Ok(0) } @@ -853,7 +845,15 @@ async fn bundle_command( }; if flags.watch.is_some() { - file_watcher::watch_func(resolver, operation, "Bundle").await?; + file_watcher::watch_func( + resolver, + operation, + file_watcher::PrintConfig { + job_name: "Bundle".to_string(), + clear_screen: !flags.no_clear_screen, + }, + ) + .await?; } else { let module_graph = if let ResolutionResult::Restart { result, .. } = resolver(None).await { @@ -894,8 +894,7 @@ async fn format_command( return Ok(0); } - tools::fmt::format(fmt_flags, flags.watch.is_some(), maybe_fmt_config) - .await?; + tools::fmt::format(flags, fmt_flags, maybe_fmt_config).await?; Ok(0) } @@ -1118,7 +1117,15 @@ async fn run_with_watch(flags: Flags, script: String) -> Result { } }; - file_watcher::watch_func(resolver, operation, "Process").await?; + file_watcher::watch_func( + resolver, + operation, + file_watcher::PrintConfig { + job_name: "Process".to_string(), + clear_screen: !flags.no_clear_screen, + }, + ) + .await?; Ok(0) } diff --git a/cli/tests/integration/watcher_tests.rs b/cli/tests/integration/watcher_tests.rs index 89d4bf3f75..cdc7a2c98a 100644 --- a/cli/tests/integration/watcher_tests.rs +++ b/cli/tests/integration/watcher_tests.rs @@ -997,3 +997,51 @@ fn test_watch_module_graph_error_referrer() { wait_for("Process failed", &mut stderr_lines); check_alive_then_kill(child); } + +#[test] +fn watch_with_no_clear_screen_flag() { + let t = TempDir::new().unwrap(); + let file_to_watch = t.path().join("file_to_watch.js"); + write(&file_to_watch, "export const foo = 0;").unwrap(); + + // choose deno run subcommand to test --no-clear-screen flag + let mut child = util::deno_cmd() + .current_dir(util::testdata_path()) + .arg("run") + .arg("--watch") + .arg("--no-clear-screen") + .arg("--unstable") + .arg(&file_to_watch) + .env("NO_COLOR", "1") + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .unwrap(); + let (_, mut stderr_lines) = child_lines(&mut child); + + let next_line = stderr_lines.next().unwrap(); + + // no clear screen + assert!(!&next_line.contains(CLEAR_SCREEN)); + assert_contains!(&next_line, "Process started"); + assert_contains!( + stderr_lines.next().unwrap(), + "Process finished. Restarting on file change..." + ); + + // Change content of the file + write(&file_to_watch, "export const bar = 0;").unwrap(); + + let next_line = stderr_lines.next().unwrap(); + + // no clear screen + assert!(!&next_line.contains(CLEAR_SCREEN)); + + assert_contains!(&next_line, "Watcher File change detected! Restarting!"); + assert_contains!( + stderr_lines.next().unwrap(), + "Process finished. Restarting on file change..." + ); + + check_alive_then_kill(child); +} diff --git a/cli/tools/fmt.rs b/cli/tools/fmt.rs index 3044f13d5b..a0ab3ea568 100644 --- a/cli/tools/fmt.rs +++ b/cli/tools/fmt.rs @@ -14,7 +14,7 @@ use crate::config_file::ProseWrap; use crate::diff::diff; use crate::file_watcher; use crate::file_watcher::ResolutionResult; -use crate::flags::FmtFlags; +use crate::flags::{Flags, FmtFlags}; use crate::fs_util::specifier_to_file_path; use crate::fs_util::{collect_files, get_extension}; use crate::text_encoding; @@ -39,8 +39,8 @@ use std::sync::{Arc, Mutex}; /// Format JavaScript/TypeScript files. pub async fn format( + flags: Flags, fmt_flags: FmtFlags, - watch: bool, maybe_fmt_config: Option, ) -> Result<(), AnyError> { let FmtFlags { @@ -136,8 +136,16 @@ pub async fn format( Ok(()) }; - if watch { - file_watcher::watch_func(resolver, operation, "Fmt").await?; + if flags.watch.is_some() { + file_watcher::watch_func( + resolver, + operation, + file_watcher::PrintConfig { + job_name: "Fmt".to_string(), + clear_screen: !flags.no_clear_screen, + }, + ) + .await?; } else { let files = collect_files(&include_files, &exclude_files, is_supported_ext_fmt) diff --git a/cli/tools/lint.rs b/cli/tools/lint.rs index b103550e25..74c5540fcd 100644 --- a/cli/tools/lint.rs +++ b/cli/tools/lint.rs @@ -8,9 +8,10 @@ //! the same functions as ops available in JS runtime. use crate::config_file::LintConfig; use crate::file_watcher::ResolutionResult; -use crate::flags::LintFlags; +use crate::flags::{Flags, LintFlags}; use crate::fmt_errors; use crate::fs_util::{collect_files, is_supported_ext, specifier_to_file_path}; +use crate::proc_state::ProcState; use crate::tools::fmt::run_parallelized; use crate::{colors, file_watcher}; use deno_ast::MediaType; @@ -48,11 +49,7 @@ fn create_reporter(kind: LintReporterKind) -> Box { } } -pub async fn lint( - maybe_lint_config: Option, - lint_flags: LintFlags, - watch: bool, -) -> Result<(), AnyError> { +pub async fn lint(flags: Flags, lint_flags: LintFlags) -> Result<(), AnyError> { let LintFlags { maybe_rules_tags, maybe_rules_include, @@ -69,6 +66,13 @@ pub async fn lint( let mut include_files = args.clone(); let mut exclude_files = ignore.clone(); + let ps = ProcState::build(flags.clone()).await?; + let maybe_lint_config = if let Some(config_file) = &ps.maybe_config_file { + config_file.to_lint_config()? + } else { + None + }; + if let Some(lint_config) = maybe_lint_config.as_ref() { if include_files.is_empty() { include_files = lint_config @@ -166,13 +170,21 @@ pub async fn lint( Ok(()) }; - if watch { + if flags.watch.is_some() { if args.len() == 1 && args[0].to_string_lossy() == "-" { return Err(generic_error( "Lint watch on standard input is not supported.", )); } - file_watcher::watch_func(resolver, operation, "Lint").await?; + file_watcher::watch_func( + resolver, + operation, + file_watcher::PrintConfig { + job_name: "Lint".to_string(), + clear_screen: !flags.no_clear_screen, + }, + ) + .await?; } else { if args.len() == 1 && args[0].to_string_lossy() == "-" { let reporter_lock = diff --git a/cli/tools/standalone.rs b/cli/tools/standalone.rs index a0960d0516..ec957e195e 100644 --- a/cli/tools/standalone.rs +++ b/cli/tools/standalone.rs @@ -248,5 +248,6 @@ pub fn compile_to_runtime_flags( v8_flags: flags.v8_flags, version: false, watch: None, + no_clear_screen: false, }) } diff --git a/cli/tools/test.rs b/cli/tools/test.rs index 5e2d74ecf1..daae61d7f6 100644 --- a/cli/tools/test.rs +++ b/cli/tools/test.rs @@ -1279,7 +1279,15 @@ pub async fn run_tests_with_watch( } }; - file_watcher::watch_func(resolver, operation, "Test").await?; + file_watcher::watch_func( + resolver, + operation, + file_watcher::PrintConfig { + job_name: "Test".to_string(), + clear_screen: !flags.no_clear_screen, + }, + ) + .await?; Ok(()) }