From b2a557d4ed3409d27189d45faff7824dbb8fbe75 Mon Sep 17 00:00:00 2001 From: Steven Xu Date: Fri, 17 Mar 2023 09:45:35 +1100 Subject: [PATCH] fix: fix `commandline` when called with no arguments (#8207) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description This fixes the `commandline` command when it's run with no arguments, so it outputs the command being run. New line characters are included. This allows for: - [A way to get current command inside pre_execution hook ยท Issue #6264 ยท nushell/nushell](https://github.com/nushell/nushell/issues/6264) - The possibility of *Atuin* to work work *Nushell*. *Atuin* hooks need to know the current repl input before it is run. # User-Facing Changes # Tests + Formatting Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass # After Submitting If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. --- Cargo.lock | 1 + crates/nu-cli/src/repl.rs | 58 ++++++---- crates/nu-cmd-lang/Cargo.toml | 1 + .../src/core_commands/commandline.rs | 92 +++++++++++---- crates/nu-protocol/src/engine/engine_state.rs | 20 +--- src/test_bins.rs | 6 + src/tests.rs | 1 + src/tests/test_commandline.rs | 109 ++++++++++++++++++ tests/hooks/mod.rs | 13 +++ 9 files changed, 244 insertions(+), 57 deletions(-) create mode 100644 src/tests/test_commandline.rs diff --git a/Cargo.lock b/Cargo.lock index fb5e32575d..5771d3b701 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2746,6 +2746,7 @@ dependencies = [ "nu-test-support", "nu-utils", "shadow-rs", + "unicode-segmentation", ] [[package]] diff --git a/crates/nu-cli/src/repl.rs b/crates/nu-cli/src/repl.rs index ef907c9357..0b234e633c 100644 --- a/crates/nu-cli/src/repl.rs +++ b/crates/nu-cli/src/repl.rs @@ -14,7 +14,7 @@ use nu_parser::{lex, parse, trim_quotes_str}; use nu_protocol::{ ast::PathMember, config::NuCursorShape, - engine::{EngineState, ReplOperation, Stack, StateWorkingSet}, + engine::{EngineState, Stack, StateWorkingSet}, format_duration, BlockId, HistoryFileFormat, PipelineData, PositionalArg, ShellError, Span, Spanned, Type, Value, VarId, }; @@ -459,18 +459,30 @@ pub fn evaluate_repl( .into_diagnostic()?; // todo: don't stop repl if error here? } - engine_state - .repl_buffer_state - .lock() - .expect("repl buffer state mutex") - .replace(line_editor.current_buffer_contents().to_string()); - // Right before we start running the code the user gave us, // fire the "pre_execution" 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"); + let next_repl_buffer = repl_buffer.to_string(); + *repl_buffer = s.to_string(); + drop(repl_buffer); + if let Err(err) = eval_hook(engine_state, stack, None, vec![], &hook) { report_error_new(engine_state, &err); } + + // Restore the REPL buffer state for the next command. It could've been edited + // by `commandline`. + let mut repl_buffer = engine_state + .repl_buffer_state + .lock() + .expect("repl buffer state mutex"); + *repl_buffer = next_repl_buffer; + drop(repl_buffer); } if shell_integration { @@ -628,23 +640,23 @@ pub fn evaluate_repl( run_ansi_sequence(RESET_APPLICATION_MODE)?; } - let mut ops = engine_state - .repl_operation_queue + let mut repl_buffer = engine_state + .repl_buffer_state .lock() - .expect("repl op queue mutex"); - while let Some(op) = ops.pop_front() { - match op { - ReplOperation::Append(s) => line_editor.run_edit_commands(&[ - EditCommand::MoveToEnd, - EditCommand::InsertString(s), - ]), - ReplOperation::Insert(s) => { - line_editor.run_edit_commands(&[EditCommand::InsertString(s)]) - } - ReplOperation::Replace(s) => line_editor - .run_edit_commands(&[EditCommand::Clear, EditCommand::InsertString(s)]), - } - } + .expect("repl buffer state mutex"); + let mut repl_cursor_pos = engine_state + .repl_cursor_pos + .lock() + .expect("repl cursor pos mutex"); + line_editor.run_edit_commands(&[ + EditCommand::Clear, + 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); } Ok(Signal::CtrlC) => { // `Reedline` clears the line content. New prompt is shown diff --git a/crates/nu-cmd-lang/Cargo.toml b/crates/nu-cmd-lang/Cargo.toml index b1e902e924..c56d2d1e31 100644 --- a/crates/nu-cmd-lang/Cargo.toml +++ b/crates/nu-cmd-lang/Cargo.toml @@ -24,6 +24,7 @@ fancy-regex = "0.11.0" itertools = "0.10.0" log = "0.4.14" shadow-rs = { version = "0.21.0", default-features = false } +unicode-segmentation = "1.10.0" [build-dependencies] shadow-rs = { version = "0.21.0", default-features = false } diff --git a/crates/nu-cmd-lang/src/core_commands/commandline.rs b/crates/nu-cmd-lang/src/core_commands/commandline.rs index 988f112a14..597491917d 100644 --- a/crates/nu-cmd-lang/src/core_commands/commandline.rs +++ b/crates/nu-cmd-lang/src/core_commands/commandline.rs @@ -1,10 +1,10 @@ use nu_engine::CallExt; use nu_protocol::ast::Call; -use nu_protocol::engine::ReplOperation; use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::Category; use nu_protocol::IntoPipelineData; use nu_protocol::{PipelineData, ShellError, Signature, SyntaxShape, Type, Value}; +use unicode_segmentation::UnicodeSegmentation; #[derive(Clone)] pub struct Commandline; @@ -17,6 +17,11 @@ impl Command for Commandline { fn signature(&self) -> Signature { Signature::build("commandline") .input_output_types(vec![(Type::Nothing, Type::Nothing)]) + .switch( + "cursor", + "Set or get the current cursor position", + Some('c'), + ) .switch( "append", "appends the string to the end of the buffer", @@ -56,30 +61,77 @@ impl Command for Commandline { _input: PipelineData, ) -> Result { if let Some(cmd) = call.opt::(engine_state, stack, 0)? { - let mut ops = engine_state - .repl_operation_queue + let mut buffer = engine_state + .repl_buffer_state .lock() - .expect("repl op queue mutex"); - ops.push_back(if call.has_flag("append") { - ReplOperation::Append(cmd.as_string()?) + .expect("repl buffer state mutex"); + let mut cursor_pos = engine_state + .repl_cursor_pos + .lock() + .expect("repl cursor pos mutex"); + + if call.has_flag("cursor") { + let cmd_str = cmd.as_string()?; + match cmd_str.parse::() { + Ok(n) => { + *cursor_pos = if n <= 0 { + 0usize + } else { + buffer + .grapheme_indices(true) + .map(|(i, _c)| i) + .nth(n as usize) + .unwrap_or(buffer.len()) + } + } + Err(_) => { + return Err(ShellError::CantConvert { + to_type: "int".to_string(), + from_type: "string".to_string(), + span: cmd.span()?, + help: Some(format!( + r#"string "{cmd_str}" does not represent a valid integer"# + )), + }) + } + } + } else if call.has_flag("append") { + buffer.push_str(&cmd.as_string()?); } else if call.has_flag("insert") { - ReplOperation::Insert(cmd.as_string()?) + let cmd_str = cmd.as_string()?; + buffer.insert_str(*cursor_pos, &cmd_str); + *cursor_pos += cmd_str.len(); } else { - ReplOperation::Replace(cmd.as_string()?) - }); - Ok(Value::Nothing { span: call.head }.into_pipeline_data()) - } else if let Some(ref cmd) = *engine_state - .repl_buffer_state - .lock() - .expect("repl buffer state mutex") - { - Ok(Value::String { - val: cmd.clone(), - span: call.head, + *buffer = cmd.as_string()?; + *cursor_pos = buffer.len(); } - .into_pipeline_data()) - } else { Ok(Value::Nothing { span: call.head }.into_pipeline_data()) + } else { + let buffer = engine_state + .repl_buffer_state + .lock() + .expect("repl buffer 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 + .grapheme_indices(true) + .position(|(i, _c)| i == *cursor_pos) + .unwrap_or(buffer.len()); + Ok(Value::String { + val: char_pos.to_string(), + span: call.head, + } + .into_pipeline_data()) + } else { + Ok(Value::String { + val: buffer.to_string(), + span: call.head, + } + .into_pipeline_data()) + } } } } diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index 4b65113541..2765deef6b 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/crates/nu-protocol/src/engine/engine_state.rs @@ -13,7 +13,7 @@ use std::num::NonZeroUsize; use std::path::Path; use std::path::PathBuf; use std::{ - collections::{HashMap, HashSet, VecDeque}, + collections::{HashMap, HashSet}, sync::{ atomic::{AtomicBool, AtomicU32}, Arc, Mutex, @@ -22,15 +22,6 @@ use std::{ static PWD_ENV: &str = "PWD"; -// TODO: move to different file? where? -/// An operation to be performed with the current buffer of the interactive shell. -#[derive(Debug, Clone)] -pub enum ReplOperation { - Append(String), - Insert(String), - Replace(String), -} - /// Organizes usage messages for various primitives #[derive(Debug, Clone)] pub struct Usage { @@ -134,8 +125,9 @@ 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_operation_queue: Arc>>, + pub repl_buffer_state: Arc>, + // 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))] @@ -186,8 +178,8 @@ 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(None)), - repl_operation_queue: Arc::new(Mutex::new(VecDeque::new())), + repl_buffer_state: Arc::new(Mutex::new("".to_string())), + repl_cursor_pos: Arc::new(Mutex::new(0)), #[cfg(feature = "plugin")] plugin_signatures: None, #[cfg(not(windows))] diff --git a/src/test_bins.rs b/src/test_bins.rs index 0a0ded7db9..37ce213c8d 100644 --- a/src/test_bins.rs +++ b/src/test_bins.rs @@ -209,6 +209,12 @@ pub fn nu_repl() { // Check for pre_execution hook let config = engine_state.get_config(); + + *engine_state + .repl_buffer_state + .lock() + .expect("repl buffer state mutex") = 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) { outcome_err(&engine_state, &err); diff --git a/src/tests.rs b/src/tests.rs index 2d14157085..8598bea906 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -1,5 +1,6 @@ mod test_bits; mod test_cell_path; +mod test_commandline; mod test_conditionals; mod test_config_path; mod test_converters; diff --git a/src/tests/test_commandline.rs b/src/tests/test_commandline.rs new file mode 100644 index 0000000000..d2019e752b --- /dev/null +++ b/src/tests/test_commandline.rs @@ -0,0 +1,109 @@ +use crate::tests::{fail_test, run_test, TestResult}; + +#[test] +fn commandline_test_get_empty() -> TestResult { + run_test("commandline", "") +} + +#[test] +fn commandline_test_append() -> TestResult { + run_test( + "commandline --replace '0๐Ÿ‘ฉโ€โค๏ธโ€๐Ÿ‘ฉ2'\n\ + commandline --cursor '2'\n\ + commandline --append 'ab'\n\ + commandline\n\ + commandline --cursor", + "0๐Ÿ‘ฉโ€โค๏ธโ€๐Ÿ‘ฉ2ab\n\ + 2", + ) +} + +#[test] +fn commandline_test_insert() -> TestResult { + run_test( + "commandline --replace '0๐Ÿ‘ฉโ€โค๏ธโ€๐Ÿ‘ฉ2'\n\ + commandline --cursor '2'\n\ + commandline --insert 'ab'\n\ + commandline\n\ + commandline --cursor", + "0๐Ÿ‘ฉโ€โค๏ธโ€๐Ÿ‘ฉab2\n\ + 4", + ) +} + +#[test] +fn commandline_test_replace() -> TestResult { + run_test( + "commandline --replace '0๐Ÿ‘ฉโ€โค๏ธโ€๐Ÿ‘ฉ2'\n\ + commandline --replace 'ab'\n\ + commandline\n\ + commandline --cursor", + "ab\n\ + 2", + ) +} + +#[test] +fn commandline_test_cursor() -> TestResult { + run_test( + "commandline --replace '0๐Ÿ‘ฉโ€โค๏ธโ€๐Ÿ‘ฉ2'\n\ + commandline --cursor '1' \n\ + commandline --insert 'x'\n\ + commandline", + "0x๐Ÿ‘ฉโ€โค๏ธโ€๐Ÿ‘ฉ2", + )?; + run_test( + "commandline --replace '0๐Ÿ‘ฉโ€โค๏ธโ€๐Ÿ‘ฉ2'\n\ + commandline --cursor '2' \n\ + commandline --insert 'x'\n\ + commandline", + "0๐Ÿ‘ฉโ€โค๏ธโ€๐Ÿ‘ฉx2", + ) +} + +#[test] +fn commandline_test_cursor_show_pos() -> TestResult { + run_test( + "commandline --replace '0๐Ÿ‘ฉโ€โค๏ธโ€๐Ÿ‘ฉ2'\n\ + commandline --cursor '1' \n\ + commandline --cursor", + "1", + )?; + run_test( + "commandline --replace '0๐Ÿ‘ฉโ€โค๏ธโ€๐Ÿ‘ฉ2'\n\ + commandline --cursor '2' \n\ + commandline --cursor", + "2", + ) +} + +#[test] +fn commandline_test_cursor_too_small() -> TestResult { + run_test( + "commandline --replace '123456'\n\ + commandline --cursor '-1' \n\ + commandline --insert '0'\n\ + commandline", + "0123456", + ) +} + +#[test] +fn commandline_test_cursor_too_large() -> TestResult { + run_test( + "commandline --replace '123456'\n\ + commandline --cursor '10' \n\ + commandline --insert '0'\n\ + commandline", + "1234560", + ) +} + +#[test] +fn commandline_test_cursor_invalid() -> TestResult { + fail_test( + "commandline --replace '123456'\n\ + commandline --cursor 'abc'", + r#"string "abc" does not represent a valid integer"#, + ) +} diff --git a/tests/hooks/mod.rs b/tests/hooks/mod.rs index d8c77fe5a3..1a17b2187e 100644 --- a/tests/hooks/mod.rs +++ b/tests/hooks/mod.rs @@ -325,6 +325,19 @@ fn pre_execution_block_preserve_env_var() { assert_eq!(actual_repl.out, "spam"); } +#[test] +fn pre_execution_commandline() { + let inp = &[ + &pre_execution_hook_code(r#"{ let-env repl_commandline = (commandline) }"#), + "echo foo!; $env.repl_commandline", + ]; + + let actual_repl = nu!(cwd: "tests/hooks", nu_repl_code(inp)); + + assert_eq!(actual_repl.err, ""); + assert_eq!(actual_repl.out, "foo!echo foo!; $env.repl_commandline"); +} + #[test] fn env_change_shadow_command() { let inp = &[