From 8c92da45a94416bfba48d6966391f77af0ec679c Mon Sep 17 00:00:00 2001 From: tims <0xtimsb@gmail.com> Date: Sat, 18 Jan 2025 22:06:41 +0530 Subject: [PATCH] terminal: Add scrollbar (#23256) Closes #4798 This PR implements a scrollbar for the terminal by turning `ScrollableHandle` into a trait, allowing us to implement a custom scroll handle, `TerminalScrollHandle`. It works by converting terminal lines into pixels that `ScrollableHandle` understands. When `ScrollableHandle` provides a changed offset (e.g., when you drag the scrollbar), we convert this pixel offset back into the number of lines to scroll and update the terminal content accordingly. While the current version works as expected, I believe the scrollbar's offset updates could potentially be turned into an event. This event could then be subscribed to in `TerminalView`, not needing to update the terminal's offset in the `render` method as it might have performance implications. Further ideas on this are welcome. Preview: https://github.com/user-attachments/assets/560f0aac-4544-4007-8f0b-8833386f608f Todo: - [x] Experiment with custom scrollbar responding to terminal mouse scroll - [x] Refactor existing scrollbar handle into a trait - [x] Update terminal to use the scrollbar trait instead of a custom scrollbar implementation - [x] Figure out how scrollbar events like mouse drag should notify the terminal to update its state - [x] Code clean up - [x] Scrollbar hide setting for terminal Release Notes: - Added scrollbar to the terminal --- assets/settings/default.json | 17 ++ crates/recent_projects/src/remote_servers.rs | 6 +- crates/terminal/src/terminal_settings.rs | 36 ++++ .../terminal_view/src/terminal_scrollbar.rs | 91 +++++++++ crates/terminal_view/src/terminal_view.rs | 193 ++++++++++++++++-- crates/ui/src/components/scrollbar.rs | 155 +++++++------- docs/src/configuring-zed.md | 5 +- 7 files changed, 412 insertions(+), 91 deletions(-) create mode 100644 crates/terminal_view/src/terminal_scrollbar.rs diff --git a/assets/settings/default.json b/assets/settings/default.json index 9967d4004b..04b9bdc29e 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -908,6 +908,23 @@ // The shell running in the terminal needs to be configured to emit the title. // Example: `echo -e "\e]2;New Title\007";` "breadcrumbs": true + }, + /// Scrollbar-related settings + "scrollbar": { + /// When to show the scrollbar in the terminal. + /// This setting can take four values: + /// + /// 1. null (default): Inherit editor settings + /// 2. Show the scrollbar if there's important information or + /// follow the system's configured behavior (default): + /// "auto" + /// 3. Match the system's configured behavior: + /// "system" + /// 4. Always show the scrollbar: + /// "always" + /// 5. Never show the scrollbar: + /// "never" + "show": null } // Set the terminal's font size. If this option is not included, // the terminal will default to matching the buffer's font size. diff --git a/crates/recent_projects/src/remote_servers.rs b/crates/recent_projects/src/remote_servers.rs index b9b0083b93..03262286fb 100644 --- a/crates/recent_projects/src/remote_servers.rs +++ b/crates/recent_projects/src/remote_servers.rs @@ -1252,7 +1252,11 @@ impl RemoteServerProjects { cx.notify(); })); - let ui::ScrollableHandle::NonUniform(scroll_handle) = scroll_state.scroll_handle() else { + let Some(scroll_handle) = scroll_state + .scroll_handle() + .as_any() + .downcast_ref::() + else { unreachable!() }; diff --git a/crates/terminal/src/terminal_settings.rs b/crates/terminal/src/terminal_settings.rs index 760eb14b21..1007f16434 100644 --- a/crates/terminal/src/terminal_settings.rs +++ b/crates/terminal/src/terminal_settings.rs @@ -47,6 +47,40 @@ pub struct TerminalSettings { pub detect_venv: VenvSettings, pub max_scroll_history_lines: Option, pub toolbar: Toolbar, + pub scrollbar: ScrollbarSettings, +} + +#[derive(Copy, Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq, Eq)] +pub struct ScrollbarSettings { + /// When to show the scrollbar in the terminal. + /// + /// Default: inherits editor scrollbar settings + pub show: Option, +} + +#[derive(Copy, Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq, Eq)] +pub struct ScrollbarSettingsContent { + /// When to show the scrollbar in the terminal. + /// + /// Default: inherits editor scrollbar settings + pub show: Option>, +} + +/// When to show the scrollbar in the terminal. +/// +/// Default: auto +#[derive(Copy, Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum ShowScrollbar { + /// Show the scrollbar if there's important information or + /// follow the system's configured behavior. + Auto, + /// Match the system's configured behavior. + System, + /// Always show the scrollbar. + Always, + /// Never show the scrollbar. + Never, } #[derive(Clone, Debug, Default, Serialize, Deserialize, JsonSchema)] @@ -187,6 +221,8 @@ pub struct TerminalSettingsContent { pub max_scroll_history_lines: Option, /// Toolbar related settings pub toolbar: Option, + /// Scrollbar-related settings + pub scrollbar: Option, } impl settings::Settings for TerminalSettings { diff --git a/crates/terminal_view/src/terminal_scrollbar.rs b/crates/terminal_view/src/terminal_scrollbar.rs new file mode 100644 index 0000000000..e86ebdf558 --- /dev/null +++ b/crates/terminal_view/src/terminal_scrollbar.rs @@ -0,0 +1,91 @@ +use std::{ + any::Any, + cell::{Cell, RefCell}, + rc::Rc, +}; + +use gpui::{size, Bounds, Point}; +use terminal::Terminal; +use ui::{px, ContentSize, Pixels, ScrollableHandle}; + +#[derive(Debug)] +struct ScrollHandleState { + line_height: Pixels, + total_lines: usize, + viewport_lines: usize, + display_offset: usize, +} + +impl ScrollHandleState { + fn new(terminal: &Terminal) -> Self { + Self { + line_height: terminal.last_content().size.line_height, + total_lines: terminal.total_lines(), + viewport_lines: terminal.viewport_lines(), + display_offset: terminal.last_content().display_offset, + } + } +} + +#[derive(Debug, Clone)] +pub struct TerminalScrollHandle { + state: Rc>, + pub future_display_offset: Rc>>, +} + +impl TerminalScrollHandle { + pub fn new(terminal: &Terminal) -> Self { + Self { + state: Rc::new(RefCell::new(ScrollHandleState::new(terminal))), + future_display_offset: Rc::new(Cell::new(None)), + } + } + + pub fn update(&self, terminal: &Terminal) { + *self.state.borrow_mut() = ScrollHandleState::new(terminal); + } +} + +impl ScrollableHandle for TerminalScrollHandle { + fn content_size(&self) -> Option { + let state = self.state.borrow(); + Some(ContentSize { + size: size(px(0.), px(state.total_lines as f32 * state.line_height.0)), + scroll_adjustment: Some(Point::new(px(0.), px(0.))), + }) + } + + fn offset(&self) -> Point { + let state = self.state.borrow(); + let scroll_offset = state.total_lines - state.viewport_lines - state.display_offset; + Point::new( + px(0.), + -px(scroll_offset as f32 * self.state.borrow().line_height.0), + ) + } + + fn set_offset(&self, point: Point) { + let state = self.state.borrow(); + let offset_delta = (point.y.0 / state.line_height.0).round() as i32; + + let max_offset = state.total_lines - state.viewport_lines; + let display_offset = ((max_offset as i32 + offset_delta) as usize).min(max_offset); + + self.future_display_offset.set(Some(display_offset)); + } + + fn viewport(&self) -> Bounds { + let state = self.state.borrow(); + Bounds::new( + Point::new(px(0.), px(0.)), + size( + px(0.), + px(state.viewport_lines as f32 * state.line_height.0), + ), + ) + } + + fn as_any(&self) -> &dyn Any { + self + } +} diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index 6d87b03d05..176cfca8f2 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/crates/terminal_view/src/terminal_view.rs @@ -1,15 +1,20 @@ mod persistence; pub mod terminal_element; pub mod terminal_panel; +pub mod terminal_scrollbar; pub mod terminal_tab_tooltip; use collections::HashSet; -use editor::{actions::SelectAll, scroll::Autoscroll, Editor}; +use editor::{ + actions::SelectAll, + scroll::{Autoscroll, ScrollbarAutoHide}, + Editor, EditorSettings, +}; use futures::{stream::FuturesUnordered, StreamExt}; use gpui::{ anchored, deferred, div, impl_actions, AnyElement, AppContext, DismissEvent, EventEmitter, FocusHandle, FocusableView, KeyContext, KeyDownEvent, Keystroke, Model, MouseButton, - MouseDownEvent, Pixels, Render, ScrollWheelEvent, Styled, Subscription, Task, View, + MouseDownEvent, Pixels, Render, ScrollWheelEvent, Stateful, Styled, Subscription, Task, View, VisualContext, WeakModel, WeakView, }; use language::Bias; @@ -21,15 +26,18 @@ use terminal::{ index::Point, term::{search::RegexSearch, TermMode}, }, - terminal_settings::{CursorShape, TerminalBlink, TerminalSettings, WorkingDirectory}, + terminal_settings::{self, CursorShape, TerminalBlink, TerminalSettings, WorkingDirectory}, Clear, Copy, Event, MaybeNavigationTarget, Paste, ScrollLineDown, ScrollLineUp, ScrollPageDown, ScrollPageUp, ScrollToBottom, ScrollToTop, ShowCharacterPalette, TaskStatus, Terminal, TerminalSize, ToggleViMode, }; use terminal_element::{is_blank, TerminalElement}; use terminal_panel::TerminalPanel; +use terminal_scrollbar::TerminalScrollHandle; use terminal_tab_tooltip::TerminalTooltip; -use ui::{h_flex, prelude::*, ContextMenu, Icon, IconName, Label, Tooltip}; +use ui::{ + h_flex, prelude::*, ContextMenu, Icon, IconName, Label, Scrollbar, ScrollbarState, Tooltip, +}; use util::{ paths::{PathWithPosition, SanitizedPath}, ResultExt, @@ -120,6 +128,10 @@ pub struct TerminalView { show_breadcrumbs: bool, block_below_cursor: Option>, scroll_top: Pixels, + scrollbar_state: ScrollbarState, + scroll_handle: TerminalScrollHandle, + show_scrollbar: bool, + hide_scrollbar_task: Option>, _subscriptions: Vec, _terminal_subscriptions: Vec, } @@ -167,6 +179,8 @@ impl TerminalView { .cursor_shape .unwrap_or_default(); + let scroll_handle = TerminalScrollHandle::new(terminal.read(cx)); + Self { terminal, workspace: workspace_handle, @@ -184,6 +198,10 @@ impl TerminalView { show_breadcrumbs: TerminalSettings::get_global(cx).toolbar.breadcrumbs, block_below_cursor: None, scroll_top: Pixels::ZERO, + scrollbar_state: ScrollbarState::new(scroll_handle.clone()), + scroll_handle, + show_scrollbar: !Self::should_autohide_scrollbar(cx), + hide_scrollbar_task: None, _subscriptions: vec![ focus_in, focus_out, @@ -619,6 +637,136 @@ impl TerminalView { subscribe_for_terminal_events(&terminal, self.workspace.clone(), cx); self.terminal = terminal; } + + // Hack: Using editor in terminal causes cyclic dependency i.e. editor -> terminal -> project -> editor. + fn map_show_scrollbar_from_editor_to_terminal( + show_scrollbar: editor::ShowScrollbar, + ) -> terminal_settings::ShowScrollbar { + match show_scrollbar { + editor::ShowScrollbar::Auto => terminal_settings::ShowScrollbar::Auto, + editor::ShowScrollbar::System => terminal_settings::ShowScrollbar::System, + editor::ShowScrollbar::Always => terminal_settings::ShowScrollbar::Always, + editor::ShowScrollbar::Never => terminal_settings::ShowScrollbar::Never, + } + } + + fn should_show_scrollbar(cx: &AppContext) -> bool { + let show = TerminalSettings::get_global(cx) + .scrollbar + .show + .unwrap_or_else(|| { + Self::map_show_scrollbar_from_editor_to_terminal( + EditorSettings::get_global(cx).scrollbar.show, + ) + }); + match show { + terminal_settings::ShowScrollbar::Auto => true, + terminal_settings::ShowScrollbar::System => true, + terminal_settings::ShowScrollbar::Always => true, + terminal_settings::ShowScrollbar::Never => false, + } + } + + fn should_autohide_scrollbar(cx: &AppContext) -> bool { + let show = TerminalSettings::get_global(cx) + .scrollbar + .show + .unwrap_or_else(|| { + Self::map_show_scrollbar_from_editor_to_terminal( + EditorSettings::get_global(cx).scrollbar.show, + ) + }); + match show { + terminal_settings::ShowScrollbar::Auto => true, + terminal_settings::ShowScrollbar::System => cx + .try_global::() + .map_or_else(|| cx.should_auto_hide_scrollbars(), |autohide| autohide.0), + terminal_settings::ShowScrollbar::Always => false, + terminal_settings::ShowScrollbar::Never => true, + } + } + + fn hide_scrollbar(&mut self, cx: &mut ViewContext) { + const SCROLLBAR_SHOW_INTERVAL: Duration = Duration::from_secs(1); + if !Self::should_autohide_scrollbar(cx) { + return; + } + self.hide_scrollbar_task = Some(cx.spawn(|panel, mut cx| async move { + cx.background_executor() + .timer(SCROLLBAR_SHOW_INTERVAL) + .await; + panel + .update(&mut cx, |panel, cx| { + panel.show_scrollbar = false; + cx.notify(); + }) + .log_err(); + })) + } + + fn render_scrollbar(&self, cx: &mut ViewContext) -> Option> { + if !Self::should_show_scrollbar(cx) + || !(self.show_scrollbar || self.scrollbar_state.is_dragging()) + { + return None; + } + + if self.terminal.read(cx).total_lines() == self.terminal.read(cx).viewport_lines() { + return None; + } + + self.scroll_handle.update(self.terminal.read(cx)); + + if let Some(new_display_offset) = self.scroll_handle.future_display_offset.take() { + self.terminal.update(cx, |term, _| { + let delta = new_display_offset as i32 - term.last_content.display_offset as i32; + match delta.cmp(&0) { + std::cmp::Ordering::Greater => term.scroll_up_by(delta as usize), + std::cmp::Ordering::Less => term.scroll_down_by(-delta as usize), + std::cmp::Ordering::Equal => {} + } + }); + } + + Some( + div() + .occlude() + .id("terminal-view-scroll") + .on_mouse_move(cx.listener(|_, _, cx| { + cx.notify(); + cx.stop_propagation() + })) + .on_hover(|_, cx| { + cx.stop_propagation(); + }) + .on_any_mouse_down(|_, cx| { + cx.stop_propagation(); + }) + .on_mouse_up( + MouseButton::Left, + cx.listener(|terminal_view, _, cx| { + if !terminal_view.scrollbar_state.is_dragging() + && !terminal_view.focus_handle.contains_focused(cx) + { + terminal_view.hide_scrollbar(cx); + cx.notify(); + } + cx.stop_propagation(); + }), + ) + .on_scroll_wheel(cx.listener(|_, _, cx| { + cx.notify(); + })) + .h_full() + .absolute() + .right_1() + .top_1() + .bottom_0() + .w(px(12.)) + .cursor_default() + .children(Scrollbar::vertical(self.scrollbar_state.clone())), + ) + } } fn subscribe_for_terminal_events( @@ -933,6 +1081,7 @@ impl TerminalView { terminal.focus_out(); terminal.set_cursor_shape(CursorShape::Hollow); }); + self.hide_scrollbar(cx); cx.notify(); } } @@ -945,6 +1094,8 @@ impl Render for TerminalView { let focused = self.focus_handle.is_focused(cx); div() + .occlude() + .id("terminal-view") .size_full() .relative() .track_focus(&self.focus_handle(cx)) @@ -973,18 +1124,32 @@ impl Render for TerminalView { } }), ) + .on_hover(cx.listener(|this, hovered, cx| { + if *hovered { + this.show_scrollbar = true; + this.hide_scrollbar_task.take(); + cx.notify(); + } else if !this.focus_handle.contains_focused(cx) { + this.hide_scrollbar(cx); + } + })) .child( // TODO: Oddly this wrapper div is needed for TerminalElement to not steal events from the context menu - div().size_full().child(TerminalElement::new( - terminal_handle, - terminal_view_handle, - self.workspace.clone(), - self.focus_handle.clone(), - focused, - self.should_show_cursor(focused, cx), - self.can_navigate_to_selected_word, - self.block_below_cursor.clone(), - )), + div() + .size_full() + .child(TerminalElement::new( + terminal_handle, + terminal_view_handle, + self.workspace.clone(), + self.focus_handle.clone(), + focused, + self.should_show_cursor(focused, cx), + self.can_navigate_to_selected_word, + self.block_below_cursor.clone(), + )) + .when_some(self.render_scrollbar(cx), |div, scrollbar| { + div.child(scrollbar) + }), ) .children(self.context_menu.as_ref().map(|(menu, position, _)| { deferred( diff --git a/crates/ui/src/components/scrollbar.rs b/crates/ui/src/components/scrollbar.rs index dea8b3a396..dda518632f 100644 --- a/crates/ui/src/components/scrollbar.rs +++ b/crates/ui/src/components/scrollbar.rs @@ -1,5 +1,5 @@ #![allow(missing_docs)] -use std::{cell::Cell, ops::Range, rc::Rc}; +use std::{any::Any, cell::Cell, fmt::Debug, ops::Range, rc::Rc, sync::Arc}; use crate::{prelude::*, px, relative, IntoElement}; use gpui::{ @@ -15,80 +15,84 @@ pub struct Scrollbar { kind: ScrollbarAxis, } -/// Wrapper around scroll handles. -#[derive(Clone, Debug)] -pub enum ScrollableHandle { - Uniform(UniformListScrollHandle), - NonUniform(ScrollHandle), +impl ScrollableHandle for UniformListScrollHandle { + fn content_size(&self) -> Option { + Some(ContentSize { + size: self.0.borrow().last_item_size.map(|size| size.contents)?, + scroll_adjustment: None, + }) + } + + fn set_offset(&self, point: Point) { + self.0.borrow().base_handle.set_offset(point); + } + + fn offset(&self) -> Point { + self.0.borrow().base_handle.offset() + } + + fn viewport(&self) -> Bounds { + self.0.borrow().base_handle.bounds() + } + + fn as_any(&self) -> &dyn Any { + self + } +} + +impl ScrollableHandle for ScrollHandle { + fn content_size(&self) -> Option { + let last_children_index = self.children_count().checked_sub(1)?; + + let mut last_item = self.bounds_for_item(last_children_index)?; + let mut scroll_adjustment = None; + + if last_children_index != 0 { + // todo: PO: this is slightly wrong for horizontal scrollbar, as the last item is not necessarily the longest one. + let first_item = self.bounds_for_item(0)?; + last_item.size.height += last_item.origin.y; + last_item.size.width += last_item.origin.x; + + scroll_adjustment = Some(first_item.origin); + last_item.size.height -= first_item.origin.y; + last_item.size.width -= first_item.origin.x; + } + + Some(ContentSize { + size: last_item.size, + scroll_adjustment, + }) + } + + fn set_offset(&self, point: Point) { + self.set_offset(point); + } + + fn offset(&self) -> Point { + self.offset() + } + + fn viewport(&self) -> Bounds { + self.bounds() + } + + fn as_any(&self) -> &dyn Any { + self + } } #[derive(Debug)] -struct ContentSize { - size: Size, - scroll_adjustment: Option>, +pub struct ContentSize { + pub size: Size, + pub scroll_adjustment: Option>, } -impl ScrollableHandle { - fn content_size(&self) -> Option { - match self { - ScrollableHandle::Uniform(handle) => Some(ContentSize { - size: handle.0.borrow().last_item_size.map(|size| size.contents)?, - scroll_adjustment: None, - }), - ScrollableHandle::NonUniform(handle) => { - let last_children_index = handle.children_count().checked_sub(1)?; - - let mut last_item = handle.bounds_for_item(last_children_index)?; - let mut scroll_adjustment = None; - if last_children_index != 0 { - // todo: PO: this is slightly wrong for horizontal scrollbar, as the last item is not necessarily the longest one. - let first_item = handle.bounds_for_item(0)?; - last_item.size.height += last_item.origin.y; - last_item.size.width += last_item.origin.x; - - scroll_adjustment = Some(first_item.origin); - last_item.size.height -= first_item.origin.y; - last_item.size.width -= first_item.origin.x; - } - Some(ContentSize { - size: last_item.size, - scroll_adjustment, - }) - } - } - } - fn set_offset(&self, point: Point) { - let base_handle = match self { - ScrollableHandle::Uniform(handle) => &handle.0.borrow().base_handle, - ScrollableHandle::NonUniform(handle) => &handle, - }; - base_handle.set_offset(point); - } - fn offset(&self) -> Point { - let base_handle = match self { - ScrollableHandle::Uniform(handle) => &handle.0.borrow().base_handle, - ScrollableHandle::NonUniform(handle) => &handle, - }; - base_handle.offset() - } - fn viewport(&self) -> Bounds { - let base_handle = match self { - ScrollableHandle::Uniform(handle) => &handle.0.borrow().base_handle, - ScrollableHandle::NonUniform(handle) => &handle, - }; - base_handle.bounds() - } -} -impl From for ScrollableHandle { - fn from(value: UniformListScrollHandle) -> Self { - Self::Uniform(value) - } -} - -impl From for ScrollableHandle { - fn from(value: ScrollHandle) -> Self { - Self::NonUniform(value) - } +pub trait ScrollableHandle: Debug + 'static { + fn content_size(&self) -> Option; + fn set_offset(&self, point: Point); + fn offset(&self) -> Point; + fn viewport(&self) -> Bounds; + fn as_any(&self) -> &dyn Any; } /// A scrollbar state that should be persisted across frames. @@ -97,15 +101,15 @@ pub struct ScrollbarState { // If Some(), there's an active drag, offset by percentage from the origin of a thumb. drag: Rc>>, parent_id: Option, - scroll_handle: ScrollableHandle, + scroll_handle: Arc, } impl ScrollbarState { - pub fn new(scroll: impl Into) -> Self { + pub fn new(scroll: impl ScrollableHandle) -> Self { Self { drag: Default::default(), parent_id: None, - scroll_handle: scroll.into(), + scroll_handle: Arc::new(scroll), } } @@ -115,8 +119,8 @@ impl ScrollbarState { self } - pub fn scroll_handle(&self) -> ScrollableHandle { - self.scroll_handle.clone() + pub fn scroll_handle(&self) -> &Arc { + &self.scroll_handle } pub fn is_dragging(&self) -> bool { @@ -171,6 +175,7 @@ impl Scrollbar { pub fn horizontal(state: ScrollbarState) -> Option { Self::new(state, ScrollbarAxis::Horizontal) } + fn new(state: ScrollbarState, kind: ScrollbarAxis) -> Option { let thumb = state.thumb_range(kind)?; Some(Self { thumb, state, kind }) diff --git a/docs/src/configuring-zed.md b/docs/src/configuring-zed.md index 88f0aeaab7..163c17b2b8 100644 --- a/docs/src/configuring-zed.md +++ b/docs/src/configuring-zed.md @@ -1764,7 +1764,10 @@ List of `integer` column numbers "toolbar": { "breadcrumbs": true }, - "working_directory": "current_project_directory" + "working_directory": "current_project_directory", + "scrollbar": { + "show": null + } } } ```