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 <bennet@zed.dev>
This commit is contained in:
Thorsten Ball 2024-09-30 15:36:35 +02:00 committed by GitHub
parent 215bce1974
commit 69e698c3be
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 24 additions and 17 deletions

View file

@ -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,

View file

@ -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<MaybeNavigationTarget>),
Open(MaybeNavigationTarget),
@ -315,7 +315,6 @@ impl TerminalBuilder {
task: Option<TaskState>,
shell: Shell,
mut env: HashMap<String, String>,
blink_settings: Option<TerminalBlink>,
cursor_shape: CursorShape,
alternate_scroll: AlternateScroll,
max_scroll_history_lines: Option<usize>,
@ -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<FairMutex<Term<ZedListener>>>,
term_config: Config,
events: VecDeque<InternalEvent>,
/// 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 {

View file

@ -104,7 +104,7 @@ pub struct TerminalView {
context_menu: Option<(View<ContextMenu>, gpui::Point<Pixels>, 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<Self>) -> 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);