From 5466da3b52352ba67d98504cfc186db42eea4543 Mon Sep 17 00:00:00 2001 From: Darren Schroeder <343840+fdncred@users.noreply.github.com> Date: Wed, 8 May 2024 14:34:04 -0400 Subject: [PATCH] cleanup osc calls for shell_integration (#12810) # Description This PR is a continuation of #12629 and meant to address [Reilly's stated issue](https://github.com/nushell/nushell/pull/12629#issuecomment-2099660609). With this PR, nushell should work more consistently with WezTerm on Windows. However, that means continued scrolling with typing if osc133 is enabled. If it's possible to run WezTerm inside of vscode, then having osc633 enabled will also cause the display to scroll with every character typed. I think the cause of this is that reedline paints the entire prompt on each character typed. We need to figure out how to fix that, but that's in reedline. For my purposes, I keep osc133 and osc633 set to true and don't use WezTerm on Windows. Thanks @rgwood for reporting the issue. I found several logic errors. It's often good to come back to PRs and look at them with fresh eyes. I think this is pretty close to logically correct now. However, I'm approaching burn out on ansi escape codes so i could've missed something. Kudos to [escape-artist](https://github.com/rgwood/escape-artist) for helping me debug an ansi escape codes that are actually being sent to the terminal. It was an invaluable tool. # User-Facing Changes # Tests + Formatting # After Submitting --- crates/nu-cli/src/prompt.rs | 6 ++- crates/nu-cli/src/prompt_update.rs | 56 ++++++++-------------- crates/nu-cli/src/repl.rs | 76 +++++++++++++++++++++--------- 3 files changed, 79 insertions(+), 59 deletions(-) diff --git a/crates/nu-cli/src/prompt.rs b/crates/nu-cli/src/prompt.rs index a2045a201c3..744640b76f7 100644 --- a/crates/nu-cli/src/prompt.rs +++ b/crates/nu-cli/src/prompt.rs @@ -129,9 +129,11 @@ impl Prompt for NushellPrompt { { // We're in vscode and we have osc633 enabled format!("{VSCODE_PRE_PROMPT_MARKER}{prompt}{VSCODE_POST_PROMPT_MARKER}").into() - } else { - // If we're in VSCode but we don't find the env var, just return the regular markers + } else if self.shell_integration_osc133 { + // If we're in VSCode but we don't find the env var, but we have osc133 set, then use it format!("{PRE_PROMPT_MARKER}{prompt}{POST_PROMPT_MARKER}").into() + } else { + prompt.into() } } else if self.shell_integration_osc133 { format!("{PRE_PROMPT_MARKER}{prompt}{POST_PROMPT_MARKER}").into() diff --git a/crates/nu-cli/src/prompt_update.rs b/crates/nu-cli/src/prompt_update.rs index 0c5641378b1..5fe2485ca85 100644 --- a/crates/nu-cli/src/prompt_update.rs +++ b/crates/nu-cli/src/prompt_update.rs @@ -108,50 +108,34 @@ pub(crate) fn update_prompt( stack: &mut Stack, nu_prompt: &mut NushellPrompt, ) { - let left_prompt_string = get_prompt_string(PROMPT_COMMAND, config, engine_state, stack); + let configured_left_prompt_string = + match get_prompt_string(PROMPT_COMMAND, config, engine_state, stack) { + Some(s) => s, + None => "".to_string(), + }; // Now that we have the prompt string lets ansify it. // <133 A><133 B><133 C> - let left_prompt_string_133 = if config.shell_integration_osc133 { - if let Some(prompt_string) = left_prompt_string.clone() { + let left_prompt_string = if config.shell_integration_osc633 { + if stack.get_env_var(engine_state, "TERM_PROGRAM") == Some(Value::test_string("vscode")) { + // We're in vscode and we have osc633 enabled Some(format!( - "{PRE_PROMPT_MARKER}{prompt_string}{POST_PROMPT_MARKER}" + "{VSCODE_PRE_PROMPT_MARKER}{configured_left_prompt_string}{VSCODE_POST_PROMPT_MARKER}" + )) + } else if config.shell_integration_osc133 { + // If we're in VSCode but we don't find the env var, but we have osc133 set, then use it + Some(format!( + "{PRE_PROMPT_MARKER}{configured_left_prompt_string}{POST_PROMPT_MARKER}" )) } else { - left_prompt_string.clone() + configured_left_prompt_string.into() } + } else if config.shell_integration_osc133 { + Some(format!( + "{PRE_PROMPT_MARKER}{configured_left_prompt_string}{POST_PROMPT_MARKER}" + )) } else { - left_prompt_string.clone() - }; - - let left_prompt_string_633 = if config.shell_integration_osc633 { - if let Some(prompt_string) = left_prompt_string.clone() { - if stack.get_env_var(engine_state, "TERM_PROGRAM") == Some(Value::test_string("vscode")) - { - // If the user enabled osc633 and we're in vscode, use the vscode markers - Some(format!( - "{VSCODE_PRE_PROMPT_MARKER}{prompt_string}{VSCODE_POST_PROMPT_MARKER}" - )) - } else { - // otherwise, use the regular osc133 markers - Some(format!( - "{PRE_PROMPT_MARKER}{prompt_string}{POST_PROMPT_MARKER}" - )) - } - } else { - left_prompt_string.clone() - } - } else { - left_prompt_string.clone() - }; - - let left_prompt_string = match (left_prompt_string_133, left_prompt_string_633) { - (None, None) => left_prompt_string, - (None, Some(l633)) => Some(l633), - (Some(l133), None) => Some(l133), - // If both are set, it means we're in vscode, so use the vscode markers - // and even if we're not actually in vscode atm, the regular 133 markers are used - (Some(_l133), Some(l633)) => Some(l633), + configured_left_prompt_string.into() }; let right_prompt_string = get_prompt_string(PROMPT_COMMAND_RIGHT, config, engine_state, stack); diff --git a/crates/nu-cli/src/repl.rs b/crates/nu-cli/src/repl.rs index 30cccebbde2..338d924a69f 100644 --- a/crates/nu-cli/src/repl.rs +++ b/crates/nu-cli/src/repl.rs @@ -620,7 +620,7 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Stack, Reedline) { column!(), use_color, ); - } else { + } else if shell_integration_osc133 { start_time = Instant::now(); run_ansi_sequence(PRE_EXECUTION_MARKER); @@ -660,9 +660,9 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Stack, Reedline) { run_finaliziation_ansi_sequence( &stack, engine_state, + use_color, shell_integration_osc633, shell_integration_osc133, - use_color, ); } ReplOperation::RunCommand(cmd) => { @@ -679,9 +679,9 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Stack, Reedline) { run_finaliziation_ansi_sequence( &stack, engine_state, + use_color, shell_integration_osc633, shell_integration_osc133, - use_color, ); } // as the name implies, we do nothing in this case @@ -731,9 +731,9 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Stack, Reedline) { run_finaliziation_ansi_sequence( &stack, engine_state, + use_color, shell_integration_osc633, shell_integration_osc133, - use_color, ); } Ok(Signal::CtrlD) => { @@ -742,9 +742,9 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Stack, Reedline) { run_finaliziation_ansi_sequence( &stack, engine_state, + use_color, shell_integration_osc633, shell_integration_osc133, - use_color, ); println!(); @@ -763,9 +763,9 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Stack, Reedline) { run_finaliziation_ansi_sequence( &stack, engine_state, + use_color, shell_integration_osc633, shell_integration_osc133, - use_color, ); } } @@ -1298,27 +1298,46 @@ fn map_nucursorshape_to_cursorshape(shape: NuCursorShape) -> Option String { +fn get_command_finished_marker( + stack: &Stack, + engine_state: &EngineState, + shell_integration_osc633: bool, + shell_integration_osc133: bool, +) -> String { let exit_code = stack .get_env_var(engine_state, "LAST_EXIT_CODE") .and_then(|e| e.as_i64().ok()); - if vscode { - // format!("\x1b]633;D;{}\x1b\\", exit_code.unwrap_or(0)) - format!( - "{}{}{}", - VSCODE_POST_EXECUTION_MARKER_PREFIX, - exit_code.unwrap_or(0), - VSCODE_POST_EXECUTION_MARKER_SUFFIX - ) - } else { - // format!("\x1b]133;D;{}\x1b\\", exit_code.unwrap_or(0)) + if shell_integration_osc633 { + if stack.get_env_var(engine_state, "TERM_PROGRAM") == Some(Value::test_string("vscode")) { + // We're in vscode and we have osc633 enabled + format!( + "{}{}{}", + VSCODE_POST_EXECUTION_MARKER_PREFIX, + exit_code.unwrap_or(0), + VSCODE_POST_EXECUTION_MARKER_SUFFIX + ) + } else if shell_integration_osc133 { + // If we're in VSCode but we don't find the env var, just return the regular markers + format!( + "{}{}{}", + POST_EXECUTION_MARKER_PREFIX, + exit_code.unwrap_or(0), + POST_EXECUTION_MARKER_SUFFIX + ) + } else { + // We're not in vscode, so we don't need to do anything special + "\x1b[0m".to_string() + } + } else if shell_integration_osc133 { format!( "{}{}{}", POST_EXECUTION_MARKER_PREFIX, exit_code.unwrap_or(0), POST_EXECUTION_MARKER_SUFFIX ) + } else { + "\x1b[0m".to_string() } } @@ -1342,7 +1361,12 @@ fn run_finaliziation_ansi_sequence( if stack.get_env_var(engine_state, "TERM_PROGRAM") == Some(Value::test_string("vscode")) { let start_time = Instant::now(); - run_ansi_sequence(&get_command_finished_marker(stack, engine_state, true)); + run_ansi_sequence(&get_command_finished_marker( + stack, + engine_state, + shell_integration_osc633, + shell_integration_osc133, + )); perf( "post_execute_marker (633;D) ansi escape sequences", @@ -1352,10 +1376,15 @@ fn run_finaliziation_ansi_sequence( column!(), use_color, ); - } else { + } else if shell_integration_osc133 { let start_time = Instant::now(); - run_ansi_sequence(&get_command_finished_marker(stack, engine_state, false)); + run_ansi_sequence(&get_command_finished_marker( + stack, + engine_state, + shell_integration_osc633, + shell_integration_osc133, + )); perf( "post_execute_marker (133;D) ansi escape sequences", @@ -1369,7 +1398,12 @@ fn run_finaliziation_ansi_sequence( } else if shell_integration_osc133 { let start_time = Instant::now(); - run_ansi_sequence(&get_command_finished_marker(stack, engine_state, false)); + run_ansi_sequence(&get_command_finished_marker( + stack, + engine_state, + shell_integration_osc633, + shell_integration_osc133, + )); perf( "post_execute_marker (133;D) ansi escape sequences",