From 69e698c3be73b6e31605abd317b835f63670172c Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Mon, 30 Sep 2024 15:36:35 +0200 Subject: [PATCH] terminal: Fix blinking settings & blinking with custom shape (#18538) This is a follow-up to #18530 thanks to this comment here: https://github.com/zed-industries/zed/pull/18530#issuecomment-2382870564 In short: it fixes the `blinking` setting and the `cursor_shape` setting as it relates to blinking. Turns out our `blinking` setting was always the wrong value when using `terminal_controlled` and the terminal _would_ control the blinking. Example script to test with: ```bash echo -e "0 normal \x1b[\x30 q"; sleep 2 echo -e "1 blink block \x1b[\x31 q"; sleep 2 echo -e "2 solid block \x1b[\x32 q"; sleep 2 echo -e "3 blink under \x1b[\x33 q"; sleep 2 echo -e "4 solid under \x1b[\x34 q"; sleep 2 echo -e "5 blink vert \x1b[\x35 q"; sleep 2 echo -e "6 solid vert \x1b[\x36 q"; sleep 2 echo -e "0 normal \x1b[\x30 q"; sleep 2 echo -e "color \x1b]12;#00ff00\x1b\\"; sleep 2 echo -e "reset \x1b]112\x1b\\ \x1b[\x30 q" ``` Before the changes in here, this script would set the cursor shape and the blinking, but the blinking boolean would always be wrong. This change here makes sure that it works consistently: - `terminal.cursor_shape` only controls the *default* shape of the terminal, not the blinking. - `terminal.blinking = on` means that it's *always* blinking, regardless of what terminal programs want - `terminal.blinking = off` means that it's *never* blinking, regardless of what terminal programs want - `terminal.blinking = terminal_controlled (default)` means that it's blinking depending on what terminal programs want. when a terminal program resets the cursor to default, it sets it back to `terminal.cursor_shape` if that is set. Release Notes: - Fixed the behavior of `{"terminal": {"blinking": "[on|off|terminal_controlled]"}` to work correctly and to work correctly when custom `cursor_shape` is set. - `terminal.cursor_shape` only controls the *default* shape of the terminal, not the blinking. - `terminal.blinking = on` means that it's *always* blinking, regardless of what terminal programs want - `terminal.blinking = off` means that it's *never* blinking, regardless of what terminal programs want - `terminal.blinking = terminal_controlled (default)` means that it's blinking depending on what terminal programs want. when a terminal program resets the cursor to default, it sets it back to `terminal.cursor_shape` if that is set. Demo: https://github.com/user-attachments/assets/b3fbeafd-ad58-41c8-9c07-1f03bc31771f Co-authored-by: Bennet --- crates/project/src/terminals.rs | 1 - crates/terminal/src/terminal.rs | 21 ++++++++++----------- crates/terminal_view/src/terminal_view.rs | 19 ++++++++++++++----- 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/crates/project/src/terminals.rs b/crates/project/src/terminals.rs index ababb3261b3376413ca750211f2f993fb80d09c1..54dd48cf433ff34bcd70ed3530599fd4d838e74d 100644 --- a/crates/project/src/terminals.rs +++ b/crates/project/src/terminals.rs @@ -215,7 +215,6 @@ impl Project { spawn_task, shell, env, - Some(settings.blinking), settings.cursor_shape.unwrap_or_default(), settings.alternate_scroll, settings.max_scroll_history_lines, diff --git a/crates/terminal/src/terminal.rs b/crates/terminal/src/terminal.rs index f9767b07d12612905489b287e59d2607342094c7..b51308df37884592f9b57e2996047bbb4e1a82df 100644 --- a/crates/terminal/src/terminal.rs +++ b/crates/terminal/src/terminal.rs @@ -42,7 +42,7 @@ use serde::{Deserialize, Serialize}; use settings::Settings; use smol::channel::{Receiver, Sender}; use task::{HideStrategy, Shell, TaskId}; -use terminal_settings::{AlternateScroll, CursorShape, TerminalBlink, TerminalSettings}; +use terminal_settings::{AlternateScroll, CursorShape, TerminalSettings}; use theme::{ActiveTheme, Theme}; use util::truncate_and_trailoff; @@ -102,7 +102,7 @@ pub enum Event { CloseTerminal, Bell, Wakeup, - BlinkChanged, + BlinkChanged(bool), SelectionsChanged, NewNavigationTarget(Option), Open(MaybeNavigationTarget), @@ -315,7 +315,6 @@ impl TerminalBuilder { task: Option, shell: Shell, mut env: HashMap, - blink_settings: Option, cursor_shape: CursorShape, alternate_scroll: AlternateScroll, max_scroll_history_lines: Option, @@ -378,16 +377,11 @@ impl TerminalBuilder { let (events_tx, events_rx) = unbounded(); //Set up the terminal... let mut term = Term::new( - config, + config.clone(), &TerminalSize::default(), ZedListener(events_tx.clone()), ); - //Start off blinking if we need to - if let Some(TerminalBlink::On) = blink_settings { - term.set_private_mode(PrivateMode::Named(NamedPrivateMode::BlinkingCursor)); - } - //Alacritty defaults to alternate scrolling being on, so we just need to turn it off. if let AlternateScroll::Off = alternate_scroll { term.unset_private_mode(PrivateMode::Named(NamedPrivateMode::AlternateScroll)); @@ -437,6 +431,7 @@ impl TerminalBuilder { pty_tx: Notifier(pty_tx), completion_tx, term, + term_config: config, events: VecDeque::with_capacity(10), //Should never get this high. last_content: Default::default(), last_mouse: None, @@ -588,6 +583,7 @@ pub struct Terminal { pty_tx: Notifier, completion_tx: Sender<()>, term: Arc>>, + term_config: Config, events: VecDeque, /// This is only used for mouse mode cell change detection last_mouse: Option<(AlacPoint, AlacDirection)>, @@ -672,7 +668,9 @@ impl Terminal { self.write_to_pty(format(self.last_content.size.into())) } AlacTermEvent::CursorBlinkingChange => { - cx.emit(Event::BlinkChanged); + let terminal = self.term.lock(); + let blinking = terminal.cursor_style().blinking; + cx.emit(Event::BlinkChanged(blinking)); } AlacTermEvent::Bell => { cx.emit(Event::Bell); @@ -957,7 +955,8 @@ impl Terminal { } pub fn set_cursor_shape(&mut self, cursor_shape: CursorShape) { - self.term.lock().set_cursor_style(Some(cursor_shape.into())); + self.term_config.default_cursor_style = cursor_shape.into(); + self.term.lock().set_options(self.term_config.clone()); } pub fn total_lines(&self) -> usize { diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index f7b38e3f5ca3e435582d935dcf4ba45544111853..ce65be30c6d4f2ba8e378faf8dcca10137366757 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/crates/terminal_view/src/terminal_view.rs @@ -104,7 +104,7 @@ pub struct TerminalView { context_menu: Option<(View, gpui::Point, Subscription)>, cursor_shape: CursorShape, blink_state: bool, - blinking_on: bool, + blinking_terminal_enabled: bool, blinking_paused: bool, blink_epoch: usize, can_navigate_to_selected_word: bool, @@ -184,7 +184,7 @@ impl TerminalView { context_menu: None, cursor_shape, blink_state: true, - blinking_on: false, + blinking_terminal_enabled: false, blinking_paused: false, blink_epoch: 0, can_navigate_to_selected_word: false, @@ -434,7 +434,6 @@ impl TerminalView { pub fn should_show_cursor(&self, focused: bool, cx: &mut gpui::ViewContext) -> bool { //Don't blink the cursor when not focused, blinking is disabled, or paused if !focused - || !self.blinking_on || self.blinking_paused || self .terminal @@ -450,7 +449,10 @@ impl TerminalView { //If the user requested to never blink, don't blink it. TerminalBlink::Off => true, //If the terminal is controlling it, check terminal mode - TerminalBlink::TerminalControlled | TerminalBlink::On => self.blink_state, + TerminalBlink::TerminalControlled => { + !self.blinking_terminal_enabled || self.blink_state + } + TerminalBlink::On => self.blink_state, } } @@ -642,7 +644,14 @@ fn subscribe_for_terminal_events( cx.emit(Event::Wakeup); } - Event::BlinkChanged => this.blinking_on = !this.blinking_on, + Event::BlinkChanged(blinking) => { + if matches!( + TerminalSettings::get_global(cx).blinking, + TerminalBlink::TerminalControlled + ) { + this.blinking_terminal_enabled = *blinking; + } + } Event::TitleChanged => { cx.emit(ItemEvent::UpdateTab);