From d3e1387ebb0414c9e9eac8a575293e4baf281635 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Fri, 5 Jan 2024 16:14:58 +0100 Subject: [PATCH] refactor(tools/repl): reorganize code (#21810) Some drive-by cleanup as I'm working through https://github.com/denoland/deno_core/pull/415. --- cli/cdp.rs | 18 ++++++ cli/tools/repl/mod.rs | 112 ++++++++++++++++++++++---------------- cli/tools/repl/session.rs | 25 +++++---- 3 files changed, 97 insertions(+), 58 deletions(-) diff --git a/cli/cdp.rs b/cli/cdp.rs index b79240965b94f5..c4fbd226e9d7f7 100644 --- a/cli/cdp.rs +++ b/cli/cdp.rs @@ -526,3 +526,21 @@ pub struct ExceptionThrown { pub timestamp: f64, pub exception_details: ExceptionDetails, } + +/// +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ExecutionContextCreated { + pub context: ExecutionContextDescription, +} + +/// +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ExecutionContextDescription { + pub id: ExecutionContextId, + pub origin: String, + pub name: String, + pub unique_id: String, + pub aux_data: Value, +} diff --git a/cli/tools/repl/mod.rs b/cli/tools/repl/mod.rs index ad2be7ec42b9fd..f1fef6d54ba76a 100644 --- a/cli/tools/repl/mod.rs +++ b/cli/tools/repl/mod.rs @@ -33,7 +33,57 @@ pub use session::REPL_INTERNALS_NAME; use super::test::TestEvent; use super::test::TestEventSender; -#[allow(clippy::await_holding_refcell_ref)] +struct Repl { + session: ReplSession, + editor: ReplEditor, + message_handler: RustylineSyncMessageHandler, +} + +impl Repl { + async fn run(&mut self) -> Result<(), AnyError> { + loop { + let line = read_line_and_poll( + &mut self.session, + &mut self.message_handler, + self.editor.clone(), + ) + .await; + match line { + Ok(line) => { + self.editor.set_should_exit_on_interrupt(false); + self.editor.update_history(line.clone()); + let output = self.session.evaluate_line_and_get_output(&line).await; + + // We check for close and break here instead of making it a loop condition to get + // consistent behavior in when the user evaluates a call to close(). + if self.session.closing().await? { + break; + } + + println!("{output}"); + } + Err(ReadlineError::Interrupted) => { + if self.editor.should_exit_on_interrupt() { + break; + } + self.editor.set_should_exit_on_interrupt(true); + println!("press ctrl+c again to exit"); + continue; + } + Err(ReadlineError::Eof) => { + break; + } + Err(err) => { + println!("Error: {err:?}"); + break; + } + } + } + + Ok(()) + } +} + async fn read_line_and_poll( repl_session: &mut ReplSession, message_handler: &mut RustylineSyncMessageHandler, @@ -42,7 +92,7 @@ async fn read_line_and_poll( let mut line_fut = spawn_blocking(move || editor.readline()); let mut poll_worker = true; let notifications_rc = repl_session.notifications.clone(); - let mut notifications = notifications_rc.borrow_mut(); + let mut notifications = notifications_rc.lock().await; loop { tokio::select! { @@ -131,7 +181,7 @@ pub async fn run(flags: Flags, repl_flags: ReplFlags) -> Result { .await?; worker.setup_repl().await?; let worker = worker.into_main_worker(); - let mut repl_session = ReplSession::initialize( + let session = ReplSession::initialize( cli_options, npm_resolver, resolver, @@ -141,20 +191,27 @@ pub async fn run(flags: Flags, repl_flags: ReplFlags) -> Result { test_event_receiver, ) .await?; - let mut rustyline_channel = rustyline_channel(); + let rustyline_channel = rustyline_channel(); let helper = EditorHelper { - context_id: repl_session.context_id, + context_id: session.context_id, sync_sender: rustyline_channel.0, }; let editor = ReplEditor::new(helper, history_file_path)?; + let mut repl = Repl { + session, + editor, + message_handler: rustyline_channel.1, + }; + if let Some(eval_files) = repl_flags.eval_files { for eval_file in eval_files { match read_eval_file(cli_options, file_fetcher, &eval_file).await { Ok(eval_source) => { - let output = repl_session + let output = repl + .session .evaluate_line_and_get_output(&eval_source) .await; // only output errors @@ -170,7 +227,7 @@ pub async fn run(flags: Flags, repl_flags: ReplFlags) -> Result { } if let Some(eval) = repl_flags.eval { - let output = repl_session.evaluate_line_and_get_output(&eval).await; + let output = repl.session.evaluate_line_and_get_output(&eval).await; // only output errors if let EvaluationOutput::Error(error_text) = output { println!("Error in --eval flag: {error_text}"); @@ -191,44 +248,7 @@ pub async fn run(flags: Flags, repl_flags: ReplFlags) -> Result { } } - loop { - let line = read_line_and_poll( - &mut repl_session, - &mut rustyline_channel.1, - editor.clone(), - ) - .await; - match line { - Ok(line) => { - editor.set_should_exit_on_interrupt(false); - editor.update_history(line.clone()); - let output = repl_session.evaluate_line_and_get_output(&line).await; - - // We check for close and break here instead of making it a loop condition to get - // consistent behavior in when the user evaluates a call to close(). - if repl_session.closing().await? { - break; - } - - println!("{output}"); - } - Err(ReadlineError::Interrupted) => { - if editor.should_exit_on_interrupt() { - break; - } - editor.set_should_exit_on_interrupt(true); - println!("press ctrl+c again to exit"); - continue; - } - Err(ReadlineError::Eof) => { - break; - } - Err(err) => { - println!("Error: {err:?}"); - break; - } - } - } + repl.run().await?; - Ok(repl_session.worker.exit_code()) + Ok(repl.session.worker.exit_code()) } diff --git a/cli/tools/repl/session.rs b/cli/tools/repl/session.rs index e2f4f5fd3a1aa7..83cc60dd0be24c 100644 --- a/cli/tools/repl/session.rs +++ b/cli/tools/repl/session.rs @@ -1,7 +1,5 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -use std::cell::RefCell; -use std::rc::Rc; use std::sync::Arc; use crate::args::CliOptions; @@ -49,6 +47,7 @@ use deno_semver::npm::NpmPackageReqReference; use once_cell::sync::Lazy; use regex::Match; use regex::Regex; +use tokio::sync::Mutex; fn comment_source_to_position_range( comment_start: SourcePos, @@ -177,7 +176,7 @@ pub struct ReplSession { session: LocalInspectorSession, pub context_id: u64, pub language_server: ReplLanguageServer, - pub notifications: Rc>>, + pub notifications: Arc>>, referrer: ModuleSpecifier, main_module: ModuleSpecifier, test_reporter_factory: Box Box>, @@ -218,18 +217,20 @@ impl ReplSession { loop { let notification = notification_rx.next().await.unwrap(); - let method = notification.get("method").unwrap().as_str().unwrap(); - let params = notification.get("params").unwrap(); - if method == "Runtime.executionContextCreated" { - let context = params.get("context").unwrap(); - assert!(context - .get("auxData") - .unwrap() + let notification = + serde_json::from_value::(notification)?; + if notification.method == "Runtime.executionContextCreated" { + let execution_context_created = serde_json::from_value::< + cdp::ExecutionContextCreated, + >(notification.params)?; + assert!(execution_context_created + .context + .aux_data .get("isDefault") .unwrap() .as_bool() .unwrap()); - context_id = context.get("id").unwrap().as_u64().unwrap(); + context_id = execution_context_created.context.id; break; } } @@ -247,7 +248,7 @@ impl ReplSession { context_id, language_server, referrer, - notifications: Rc::new(RefCell::new(notification_rx)), + notifications: Arc::new(Mutex::new(notification_rx)), test_reporter_factory: Box::new(|| { Box::new(PrettyTestReporter::new(false, true, false, true)) }),