From be53ecbbaad523056d772c3325cbea60cb54fa8e Mon Sep 17 00:00:00 2001 From: Steven Xu Date: Sun, 11 Jun 2023 08:38:11 +1000 Subject: [PATCH] refactor: merge `repl_buffer_state`, `repl_cursor_pos` into one mutex (#9031) # Description Merge `repl_buffer_state`, `repl_cursor_pos` into one mutex. # User-Facing Changes # Tests + Formatting # After Submitting --- crates/nu-cli/src/commands/commandline.rs | 44 +++++++----------- crates/nu-cli/src/repl.rs | 45 ++++++------------- crates/nu-protocol/src/engine/engine_state.rs | 16 ++++--- src/test_bins.rs | 7 +-- 4 files changed, 44 insertions(+), 68 deletions(-) diff --git a/crates/nu-cli/src/commands/commandline.rs b/crates/nu-cli/src/commands/commandline.rs index 48a4d88bf3..67c33b65ae 100644 --- a/crates/nu-cli/src/commands/commandline.rs +++ b/crates/nu-cli/src/commands/commandline.rs @@ -61,27 +61,20 @@ impl Command for Commandline { _input: PipelineData, ) -> Result { if let Some(cmd) = call.opt::(engine_state, stack, 0)? { - let mut buffer = engine_state - .repl_buffer_state - .lock() - .expect("repl buffer state mutex"); - let mut cursor_pos = engine_state - .repl_cursor_pos - .lock() - .expect("repl cursor pos mutex"); + let mut repl = engine_state.repl_state.lock().expect("repl state mutex"); if call.has_flag("cursor") { let cmd_str = cmd.as_string()?; match cmd_str.parse::() { Ok(n) => { - *cursor_pos = if n <= 0 { + repl.cursor_pos = if n <= 0 { 0usize } else { - buffer + repl.buffer .grapheme_indices(true) .map(|(i, _c)| i) .nth(n as usize) - .unwrap_or(buffer.len()) + .unwrap_or(repl.buffer.len()) } } Err(_) => { @@ -96,30 +89,25 @@ impl Command for Commandline { } } } else if call.has_flag("append") { - buffer.push_str(&cmd.as_string()?); + repl.buffer.push_str(&cmd.as_string()?); } else if call.has_flag("insert") { let cmd_str = cmd.as_string()?; - buffer.insert_str(*cursor_pos, &cmd_str); - *cursor_pos += cmd_str.len(); + let cursor_pos = repl.cursor_pos; + repl.buffer.insert_str(cursor_pos, &cmd_str); + repl.cursor_pos += cmd_str.len(); } else { - *buffer = cmd.as_string()?; - *cursor_pos = buffer.len(); + repl.buffer = cmd.as_string()?; + repl.cursor_pos = repl.buffer.len(); } Ok(Value::Nothing { span: call.head }.into_pipeline_data()) } else { - let buffer = engine_state - .repl_buffer_state - .lock() - .expect("repl buffer state mutex"); + let repl = engine_state.repl_state.lock().expect("repl state mutex"); if call.has_flag("cursor") { - let cursor_pos = engine_state - .repl_cursor_pos - .lock() - .expect("repl cursor pos mutex"); - let char_pos = buffer + let char_pos = repl + .buffer .grapheme_indices(true) - .chain(std::iter::once((buffer.len(), ""))) - .position(|(i, _c)| i == *cursor_pos) + .chain(std::iter::once((repl.buffer.len(), ""))) + .position(|(i, _c)| i == repl.cursor_pos) .expect("Cursor position isn't on a grapheme boundary"); Ok(Value::String { val: char_pos.to_string(), @@ -128,7 +116,7 @@ impl Command for Commandline { .into_pipeline_data()) } else { Ok(Value::String { - val: buffer.to_string(), + val: repl.buffer.to_string(), span: call.head, } .into_pipeline_data()) diff --git a/crates/nu-cli/src/repl.rs b/crates/nu-cli/src/repl.rs index 4a816557b9..b12e89b480 100644 --- a/crates/nu-cli/src/repl.rs +++ b/crates/nu-cli/src/repl.rs @@ -475,30 +475,19 @@ pub fn evaluate_repl( // hook if let Some(hook) = config.hooks.pre_execution.clone() { // Set the REPL buffer to the current command for the "pre_execution" hook - let mut repl_buffer = engine_state - .repl_buffer_state - .lock() - .expect("repl buffer state mutex"); - *repl_buffer = s.to_string(); - drop(repl_buffer); + let mut repl = engine_state.repl_state.lock().expect("repl state mutex"); + repl.buffer = s.to_string(); + drop(repl); if let Err(err) = eval_hook(engine_state, stack, None, vec![], &hook) { report_error_new(engine_state, &err); } } - let mut repl_cursor = engine_state - .repl_cursor_pos - .lock() - .expect("repl cursor pos mutex"); - *repl_cursor = line_editor.current_insertion_point(); - drop(repl_cursor); - let mut repl_buffer = engine_state - .repl_buffer_state - .lock() - .expect("repl buffer state mutex"); - *repl_buffer = line_editor.current_buffer_contents().to_string(); - drop(repl_buffer); + let mut repl = engine_state.repl_state.lock().expect("repl state mutex"); + repl.cursor_pos = line_editor.current_insertion_point(); + repl.buffer = line_editor.current_buffer_contents().to_string(); + drop(repl); if shell_integration { run_ansi_sequence(PRE_EXECUTE_MARKER)?; @@ -685,23 +674,15 @@ pub fn evaluate_repl( run_ansi_sequence(RESET_APPLICATION_MODE)?; } - let mut repl_buffer = engine_state - .repl_buffer_state - .lock() - .expect("repl buffer state mutex"); - let mut repl_cursor_pos = engine_state - .repl_cursor_pos - .lock() - .expect("repl cursor pos mutex"); + let mut repl = engine_state.repl_state.lock().expect("repl state mutex"); line_editor.run_edit_commands(&[ EditCommand::Clear, - EditCommand::InsertString(repl_buffer.to_string()), - EditCommand::MoveToPosition(*repl_cursor_pos), + EditCommand::InsertString(repl.buffer.to_string()), + EditCommand::MoveToPosition(repl.cursor_pos), ]); - *repl_buffer = "".to_string(); - drop(repl_buffer); - *repl_cursor_pos = 0; - drop(repl_cursor_pos); + repl.buffer = "".to_string(); + repl.cursor_pos = 0; + drop(repl); } Ok(Signal::CtrlC) => { // `Reedline` clears the line content. New prompt is shown diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index 464b4bf3c6..a8c463bf96 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/crates/nu-protocol/src/engine/engine_state.rs @@ -60,6 +60,12 @@ pub enum VirtualPath { Dir(Vec), } +pub struct ReplState { + pub buffer: String, + // A byte position, as `EditCommand::MoveToPosition` is also a byte position + pub cursor_pos: usize, +} + /// The core global engine state. This includes all global definitions as well as any global state that /// will persist for the whole session. /// @@ -118,10 +124,8 @@ pub struct EngineState { pub previous_env_vars: HashMap, pub config: Config, pub pipeline_externals_state: Arc<(AtomicU32, AtomicU32)>, - pub repl_buffer_state: Arc>, + pub repl_state: Arc>, pub table_decl_id: Option, - // A byte position, as `EditCommand::MoveToPosition` is also a byte position - pub repl_cursor_pos: Arc>, #[cfg(feature = "plugin")] pub plugin_signatures: Option, #[cfg(not(windows))] @@ -174,8 +178,10 @@ impl EngineState { previous_env_vars: HashMap::new(), config: Config::default(), pipeline_externals_state: Arc::new((AtomicU32::new(0), AtomicU32::new(0))), - repl_buffer_state: Arc::new(Mutex::new("".to_string())), - repl_cursor_pos: Arc::new(Mutex::new(0)), + repl_state: Arc::new(Mutex::new(ReplState { + buffer: "".to_string(), + cursor_pos: 0, + })), table_decl_id: None, #[cfg(feature = "plugin")] plugin_signatures: None, diff --git a/src/test_bins.rs b/src/test_bins.rs index 3f14519ca2..497f44f26b 100644 --- a/src/test_bins.rs +++ b/src/test_bins.rs @@ -213,10 +213,11 @@ pub fn nu_repl() { // Check for pre_execution hook let config = engine_state.get_config(); - *engine_state - .repl_buffer_state + engine_state + .repl_state .lock() - .expect("repl buffer state mutex") = line.to_string(); + .expect("repl state mutex") + .buffer = line.to_string(); if let Some(hook) = config.hooks.pre_execution.clone() { if let Err(err) = eval_hook(&mut engine_state, &mut stack, None, vec![], &hook) {