From f7e77123cc89ffa65947f2dff2521d3b617f7173 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Wed, 7 May 2025 14:04:11 +0300 Subject: [PATCH] Do not flicker when switching cmd-hovered words in terminal (#30098) Closes https://github.com/zed-industries/zed/issues/25110 https://github.com/user-attachments/assets/4624c256-8dfb-48eb-a726-6cf130d946da Terminal may update its hovered word way before reporting it to the terminal view, and that processing the file check later. Hence, store the terminal hover data in the terminal view and avoid highlights when it's different from what the terminal has (as the source of truth here). In addition, now only does hover refreshes when the terminal hover actually changes, not on every event report. Release Notes: - Fixed underline flicker when switching cmd-hovered words in terminal --- crates/terminal/src/terminal.rs | 2 +- crates/terminal_view/src/terminal_element.rs | 45 ++++--- crates/terminal_view/src/terminal_view.rs | 117 ++++++++++++------- 3 files changed, 102 insertions(+), 62 deletions(-) diff --git a/crates/terminal/src/terminal.rs b/crates/terminal/src/terminal.rs index 9cd11311f2b137f5b99f3164eebe895fd39ae236..56f269b00868f007e22d873fb0502c96a9828ff2 100644 --- a/crates/terminal/src/terminal.rs +++ b/crates/terminal/src/terminal.rs @@ -605,7 +605,7 @@ pub struct TerminalContent { pub scrolled_to_bottom: bool, } -#[derive(Clone)] +#[derive(Debug, Clone, Eq, PartialEq)] pub struct HoveredWord { pub word: String, pub word_match: RangeInclusive, diff --git a/crates/terminal_view/src/terminal_element.rs b/crates/terminal_view/src/terminal_element.rs index 7b1304159f71903b6387bdb2dbb71b7bb5297758..93402e33a9894a969436d224064a725e1f21cdcc 100644 --- a/crates/terminal_view/src/terminal_element.rs +++ b/crates/terminal_view/src/terminal_element.rs @@ -730,29 +730,38 @@ impl Element for TerminalElement { let background_color = theme.colors().terminal_background; - let (last_hovered_word, hover_target) = self.terminal.update(cx, |terminal, cx| { - terminal.set_size(dimensions); - terminal.sync(window, cx); - - if window.modifiers().secondary() - && bounds.contains(&window.mouse_position()) - && self.terminal_view.read(cx).hover_target_tooltip.is_some() - { - let hover_target = self.terminal_view.read(cx).hover_target_tooltip.clone(); - let last_hovered_word = terminal.last_content.last_hovered_word.clone(); - (last_hovered_word, hover_target) - } else { - (None, None) - } - }); + let (last_hovered_word, hover_tooltip) = + self.terminal.update(cx, |terminal, cx| { + terminal.set_size(dimensions); + terminal.sync(window, cx); + + if window.modifiers().secondary() + && bounds.contains(&window.mouse_position()) + && self.terminal_view.read(cx).hover.is_some() + { + let registered_hover = self.terminal_view.read(cx).hover.as_ref(); + if terminal.last_content.last_hovered_word.as_ref() + == registered_hover.map(|hover| &hover.hovered_word) + { + ( + terminal.last_content.last_hovered_word.clone(), + registered_hover.map(|hover| hover.tooltip.clone()), + ) + } else { + (None, None) + } + } else { + (None, None) + } + }); let scroll_top = self.terminal_view.read(cx).scroll_top; - let hyperlink_tooltip = hover_target.as_ref().map(|hover_target| { + let hyperlink_tooltip = hover_tooltip.map(|hover_tooltip| { let offset = bounds.origin + point(gutter, px(0.)) - point(px(0.), scroll_top); let mut element = div() .size_full() .id("terminal-element") - .tooltip(Tooltip::text(hover_target.clone())) + .tooltip(Tooltip::text(hover_tooltip)) .into_any_element(); element.prepaint_as_root(offset, bounds.size.into(), window, cx); element @@ -922,7 +931,7 @@ impl Element for TerminalElement { self.register_mouse_listeners(layout.mode, &layout.hitbox, window); if window.modifiers().secondary() && bounds.contains(&window.mouse_position()) - && self.terminal_view.read(cx).hover_target_tooltip.is_some() + && self.terminal_view.read(cx).hover.is_some() { window.set_cursor_style(gpui::CursorStyle::PointingHand, Some(&layout.hitbox)); } else { diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index 7b5d7176434e31280b17882ae00bc41aef4a7331..949b601796630893e0ebe4e08ddcf661c2c20435 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/crates/terminal_view/src/terminal_view.rs @@ -15,9 +15,9 @@ use persistence::TERMINAL_DB; use project::{Entry, Metadata, Project, search::SearchQuery, terminals::TerminalKind}; use schemars::JsonSchema; use terminal::{ - Clear, Copy, Event, MaybeNavigationTarget, Paste, ScrollLineDown, ScrollLineUp, ScrollPageDown, - ScrollPageUp, ScrollToBottom, ScrollToTop, ShowCharacterPalette, TaskState, TaskStatus, - Terminal, TerminalBounds, ToggleViMode, + Clear, Copy, Event, HoveredWord, MaybeNavigationTarget, Paste, ScrollLineDown, ScrollLineUp, + ScrollPageDown, ScrollPageUp, ScrollToBottom, ScrollToTop, ShowCharacterPalette, TaskState, + TaskStatus, Terminal, TerminalBounds, ToggleViMode, alacritty_terminal::{ index::Point, term::{TermMode, search::RegexSearch}, @@ -112,7 +112,7 @@ pub struct TerminalView { cwd_serialized: bool, blinking_paused: bool, blink_epoch: usize, - hover_target_tooltip: Option, + hover: Option, hover_tooltip_update: Task<()>, workspace_id: Option, show_breadcrumbs: bool, @@ -126,6 +126,12 @@ pub struct TerminalView { _terminal_subscriptions: Vec, } +#[derive(Debug)] +struct HoverTarget { + tooltip: String, + hovered_word: HoveredWord, +} + impl EventEmitter for TerminalView {} impl EventEmitter for TerminalView {} impl EventEmitter for TerminalView {} @@ -196,7 +202,7 @@ impl TerminalView { blinking_terminal_enabled: false, blinking_paused: false, blink_epoch: 0, - hover_target_tooltip: None, + hover: None, hover_tooltip_update: Task::ready(()), embedded, workspace_id, @@ -881,54 +887,79 @@ fn subscribe_for_terminal_events( } Event::NewNavigationTarget(maybe_navigation_target) => { - match maybe_navigation_target.as_ref() { - None => { - terminal_view.hover_target_tooltip = None; - terminal_view.hover_tooltip_update = Task::ready(()); + match maybe_navigation_target + .as_ref() + .zip(terminal.read(cx).last_content.last_hovered_word.as_ref()) + { + Some((MaybeNavigationTarget::Url(url), hovered_word)) => { + if Some(hovered_word) + != terminal_view + .hover + .as_ref() + .map(|hover| &hover.hovered_word) + { + terminal_view.hover = Some(HoverTarget { + tooltip: url.clone(), + hovered_word: hovered_word.clone(), + }); + terminal_view.hover_tooltip_update = Task::ready(()); + cx.notify(); + } } - Some(MaybeNavigationTarget::Url(url)) => { - terminal_view.hover_target_tooltip = Some(url.clone()); - terminal_view.hover_tooltip_update = Task::ready(()); + Some((MaybeNavigationTarget::PathLike(path_like_target), hovered_word)) => { + if Some(hovered_word) + != terminal_view + .hover + .as_ref() + .map(|hover| &hover.hovered_word) + { + let valid_files_to_open_task = possible_open_target( + &workspace, + &path_like_target.terminal_dir, + &path_like_target.maybe_path, + cx, + ); + let hovered_word = hovered_word.clone(); + + terminal_view.hover = None; + terminal_view.hover_tooltip_update = + cx.spawn(async move |terminal_view, cx| { + let file_to_open = valid_files_to_open_task.await; + terminal_view + .update(cx, |terminal_view, _| match file_to_open { + Some( + OpenTarget::File(path, _) + | OpenTarget::Worktree(path, _), + ) => { + terminal_view.hover = Some(HoverTarget { + tooltip: path.to_string(|path| { + path.to_string_lossy().to_string() + }), + hovered_word, + }); + } + None => { + terminal_view.hover = None; + } + }) + .ok(); + }); + cx.notify(); + } } - Some(MaybeNavigationTarget::PathLike(path_like_target)) => { - let valid_files_to_open_task = possible_open_target( - &workspace, - &path_like_target.terminal_dir, - &path_like_target.maybe_path, - cx, - ); - - terminal_view.hover_tooltip_update = - cx.spawn(async move |terminal_view, cx| { - let file_to_open = valid_files_to_open_task.await; - terminal_view - .update(cx, |terminal_view, _| match file_to_open { - Some( - OpenTarget::File(path, _) - | OpenTarget::Worktree(path, _), - ) => { - terminal_view.hover_target_tooltip = - Some(path.to_string(|path| { - path.to_string_lossy().to_string() - })); - } - None => { - terminal_view.hover_target_tooltip = None; - } - }) - .ok(); - }); + None => { + terminal_view.hover = None; + terminal_view.hover_tooltip_update = Task::ready(()); + cx.notify(); } } - - cx.notify() } Event::Open(maybe_navigation_target) => match maybe_navigation_target { MaybeNavigationTarget::Url(url) => cx.open_url(url), MaybeNavigationTarget::PathLike(path_like_target) => { - if terminal_view.hover_target_tooltip.is_none() { + if terminal_view.hover.is_none() { return; } let task_workspace = workspace.clone();