From 148547ecd1e3cbaf0c8de235c5f205eddfc35617 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 11 Feb 2025 15:14:12 -0800 Subject: [PATCH 1/4] Rework edit prediction preview mode (#24700) Don't animate the cursor when previewing jumps. Instead, display the jump popover with a line that resembles a cursor, indicating the jump destination. If the jump destination is outside of the view port, there is an extra step in which `tab` scrolls the viewport to reveal the jump destination. Release Notes: - N/A --------- Co-authored-by: danilo-leal Co-authored-by: agu-z --- crates/editor/src/editor.rs | 441 +++++-------------------- crates/editor/src/element.rs | 284 ++++++++-------- crates/editor/src/scroll.rs | 10 + crates/editor/src/scroll/autoscroll.rs | 23 -- crates/ui/src/traits/styled_ext.rs | 4 +- 5 files changed, 244 insertions(+), 518 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index e356cf8f762e98d5aeff1a62b16059fa9d27be0d..74309e3b74e5e101a23ffb92659966478973cf66 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -520,296 +520,15 @@ pub enum MenuInlineCompletionsPolicy { ByProvider, } -// TODO az do we need this? -#[derive(Clone)] pub enum EditPredictionPreview { /// Modifier is not pressed Inactive, - /// Modifier pressed, animating to active - MovingTo { - animation: Range, - scroll_position_at_start: Option>, - target_point: DisplayPoint, - }, - Arrived { - scroll_position_at_start: Option>, - scroll_position_at_arrival: Option>, - target_point: Option, - }, - /// Modifier released, animating from active - MovingFrom { - animation: Range, - target_point: DisplayPoint, + /// Modifier pressed + Active { + previous_scroll_position: Option, }, } -impl EditPredictionPreview { - fn start( - &mut self, - completion: &InlineCompletion, - snapshot: &EditorSnapshot, - cursor: DisplayPoint, - ) -> bool { - if matches!(self, Self::MovingTo { .. } | Self::Arrived { .. }) { - return false; - } - (*self, _) = Self::start_now(completion, snapshot, cursor); - true - } - - fn restart( - &mut self, - completion: &InlineCompletion, - snapshot: &EditorSnapshot, - cursor: DisplayPoint, - ) -> bool { - match self { - Self::Inactive => false, - Self::MovingTo { target_point, .. } - | Self::Arrived { - target_point: Some(target_point), - .. - } => { - let (new_preview, new_target_point) = Self::start_now(completion, snapshot, cursor); - - if new_target_point != Some(*target_point) { - *self = new_preview; - return true; - } - - false - } - Self::Arrived { - target_point: None, .. - } => { - let (new_preview, _) = Self::start_now(completion, snapshot, cursor); - - *self = new_preview; - true - } - Self::MovingFrom { .. } => false, - } - } - - fn start_now( - completion: &InlineCompletion, - snapshot: &EditorSnapshot, - cursor: DisplayPoint, - ) -> (Self, Option) { - let now = Instant::now(); - match completion { - InlineCompletion::Edit { .. } => ( - Self::Arrived { - target_point: None, - scroll_position_at_start: None, - scroll_position_at_arrival: None, - }, - None, - ), - InlineCompletion::Move { target, .. } => { - let target_point = target.to_display_point(&snapshot.display_snapshot); - let duration = Self::animation_duration(cursor, target_point); - - ( - Self::MovingTo { - animation: now..now + duration, - scroll_position_at_start: Some(snapshot.scroll_position()), - target_point, - }, - Some(target_point), - ) - } - } - } - - fn animation_duration(a: DisplayPoint, b: DisplayPoint) -> Duration { - const SPEED: f32 = 8.0; - - let row_diff = b.row().0.abs_diff(a.row().0); - let column_diff = b.column().abs_diff(a.column()); - let distance = ((row_diff.pow(2) + column_diff.pow(2)) as f32).sqrt(); - Duration::from_millis((distance * SPEED) as u64) - } - - fn end( - &mut self, - cursor: DisplayPoint, - scroll_pixel_position: gpui::Point, - window: &mut Window, - cx: &mut Context, - ) -> bool { - let (scroll_position, target_point) = match self { - Self::MovingTo { - scroll_position_at_start, - target_point, - .. - } - | Self::Arrived { - scroll_position_at_start, - scroll_position_at_arrival: None, - target_point: Some(target_point), - .. - } => (*scroll_position_at_start, target_point), - Self::Arrived { - scroll_position_at_start, - scroll_position_at_arrival: Some(scroll_at_arrival), - target_point: Some(target_point), - } => { - const TOLERANCE: f32 = 4.0; - - let diff = *scroll_at_arrival - scroll_pixel_position.map(|p| p.0); - - if diff.x.abs() < TOLERANCE && diff.y.abs() < TOLERANCE { - (*scroll_position_at_start, target_point) - } else { - (None, target_point) - } - } - Self::Arrived { - target_point: None, .. - } => { - *self = Self::Inactive; - return true; - } - Self::MovingFrom { .. } | Self::Inactive => return false, - }; - - let now = Instant::now(); - let duration = Self::animation_duration(cursor, *target_point); - let target_point = *target_point; - - *self = Self::MovingFrom { - animation: now..now + duration, - target_point, - }; - - if let Some(scroll_position) = scroll_position { - cx.spawn_in(window, |editor, mut cx| async move { - smol::Timer::after(duration).await; - editor - .update_in(&mut cx, |editor, window, cx| { - if let Self::MovingFrom { .. } | Self::Inactive = - editor.edit_prediction_preview - { - editor.set_scroll_position(scroll_position, window, cx) - } - }) - .log_err(); - }) - .detach(); - } - - true - } - - /// Whether the preview is active or we are animating to or from it. - fn is_active(&self) -> bool { - matches!( - self, - Self::MovingTo { .. } | Self::Arrived { .. } | Self::MovingFrom { .. } - ) - } - - /// Returns true if the preview is active, not cancelled, and the animation is settled. - fn is_active_settled(&self) -> bool { - matches!(self, Self::Arrived { .. }) - } - - #[allow(clippy::too_many_arguments)] - fn move_state( - &mut self, - snapshot: &EditorSnapshot, - visible_row_range: Range, - line_layouts: &[LineWithInvisibles], - scroll_pixel_position: gpui::Point, - line_height: Pixels, - target: Anchor, - cursor: Option, - ) -> Option { - let delta = match self { - Self::Inactive => return None, - Self::Arrived { .. } => 1., - Self::MovingTo { - animation, - scroll_position_at_start: original_scroll_position, - target_point, - } => { - let now = Instant::now(); - if animation.end < now { - *self = Self::Arrived { - scroll_position_at_start: *original_scroll_position, - scroll_position_at_arrival: Some(scroll_pixel_position.map(|p| p.0)), - target_point: Some(*target_point), - }; - 1.0 - } else { - (now - animation.start).as_secs_f32() - / (animation.end - animation.start).as_secs_f32() - } - } - Self::MovingFrom { animation, .. } => { - let now = Instant::now(); - if animation.end < now { - *self = Self::Inactive; - return None; - } else { - let delta = (now - animation.start).as_secs_f32() - / (animation.end - animation.start).as_secs_f32(); - 1.0 - delta - } - } - }; - - let cursor = cursor?; - - if !visible_row_range.contains(&cursor.row()) { - return None; - } - - let target_position = target.to_display_point(&snapshot.display_snapshot); - - if !visible_row_range.contains(&target_position.row()) { - return None; - } - - let target_row_layout = - &line_layouts[target_position.row().minus(visible_row_range.start) as usize]; - let target_column = target_position.column() as usize; - - let target_character_x = target_row_layout.x_for_index(target_column); - - let target_x = target_character_x - scroll_pixel_position.x; - let target_y = - (target_position.row().as_f32() - scroll_pixel_position.y / line_height) * line_height; - - let origin_x = line_layouts[cursor.row().minus(visible_row_range.start) as usize] - .x_for_index(cursor.column() as usize); - let origin_y = - (cursor.row().as_f32() - scroll_pixel_position.y / line_height) * line_height; - - let delta = 1.0 - (-10.0 * delta).exp2(); - - let x = origin_x + (target_x - origin_x) * delta; - let y = origin_y + (target_y - origin_y) * delta; - - Some(EditPredictionMoveState { - delta, - position: point(x, y), - }) - } -} - -pub(crate) struct EditPredictionMoveState { - delta: f32, - position: gpui::Point, -} - -impl EditPredictionMoveState { - pub fn is_animation_completed(&self) -> bool { - self.delta >= 1. - } -} - #[derive(Copy, Clone, Eq, PartialEq, PartialOrd, Ord, Debug, Default)] struct EditorActionId(usize); @@ -1786,6 +1505,15 @@ impl Editor { } fn key_context(&self, window: &Window, cx: &App) -> KeyContext { + self.key_context_internal(self.has_active_inline_completion(), window, cx) + } + + fn key_context_internal( + &self, + has_active_edit_prediction: bool, + window: &Window, + cx: &App, + ) -> KeyContext { let mut key_context = KeyContext::new_with_defaults(); key_context.add("Editor"); let mode = match self.mode { @@ -1836,10 +1564,9 @@ impl Editor { key_context.set("extension", extension.to_string()); } - if self.has_active_inline_completion() { + if has_active_edit_prediction { key_context.add("copilot_suggestion"); key_context.add(EDIT_PREDICTION_KEY_CONTEXT); - if showing_completions || self.edit_prediction_requires_modifier() { key_context.add(EDIT_PREDICTION_REQUIRES_MODIFIER_KEY_CONTEXT); } @@ -1857,12 +1584,10 @@ impl Editor { window: &Window, cx: &App, ) -> AcceptEditPredictionBinding { - let mut context = self.key_context(window, cx); - context.add(EDIT_PREDICTION_KEY_CONTEXT); - + let key_context = self.key_context_internal(true, window, cx); AcceptEditPredictionBinding( window - .bindings_for_action_in_context(&AcceptEditPrediction, context) + .bindings_for_action_in_context(&AcceptEditPrediction, key_context) .into_iter() .rev() .next(), @@ -5067,6 +4792,15 @@ impl Editor { self.snippet_stack.is_empty() && self.edit_predictions_enabled() } + pub fn edit_prediction_preview_is_active(&self) -> bool { + match self.edit_prediction_preview { + EditPredictionPreview::Inactive => false, + EditPredictionPreview::Active { .. } => { + self.edit_prediction_requires_modifier() || self.has_visible_completions_menu() + } + } + } + pub fn inline_completions_enabled(&self, cx: &App) -> bool { let cursor = self.selections.newest_anchor().head(); if let Some((buffer, cursor_position)) = @@ -5232,10 +4966,40 @@ impl Editor { match &active_inline_completion.completion { InlineCompletion::Move { target, .. } => { let target = *target; - // Note that this is also done in vim's handler of the Tab action. - self.change_selections(Some(Autoscroll::newest()), window, cx, |selections| { - selections.select_anchor_ranges([target..target]); - }); + + if let Some(position_map) = &self.last_position_map { + if position_map + .visible_row_range + .contains(&target.to_display_point(&position_map.snapshot).row()) + || !self.edit_prediction_preview_is_active() + { + // Note that this is also done in vim's handler of the Tab action. + self.change_selections( + Some(Autoscroll::newest()), + window, + cx, + |selections| { + selections.select_anchor_ranges([target..target]); + }, + ); + self.clear_row_highlights::(); + + self.edit_prediction_preview = EditPredictionPreview::Active { + previous_scroll_position: None, + }; + } else { + self.edit_prediction_preview = EditPredictionPreview::Active { + previous_scroll_position: Some(position_map.snapshot.scroll_anchor), + }; + self.highlight_rows::( + target..target, + cx.theme().colors().editor_highlighted_line_background, + true, + cx, + ); + self.request_autoscroll(Autoscroll::fit(), cx); + } + } } InlineCompletion::Edit { edits, .. } => { if let Some(provider) = self.edit_prediction_provider() { @@ -5403,7 +5167,7 @@ impl Editor { /// like we are not previewing and the LSP autocomplete menu is visible /// or we are in `when_holding_modifier` mode. pub fn edit_prediction_visible_in_cursor_popover(&self, has_completion: bool) -> bool { - if self.edit_prediction_preview.is_active() + if self.edit_prediction_preview_is_active() || !self.show_edit_predictions_in_menu() || !self.edit_predictions_enabled() { @@ -5425,7 +5189,7 @@ impl Editor { cx: &mut Context, ) { if self.show_edit_predictions_in_menu() { - self.update_edit_prediction_preview(&modifiers, position_map, window, cx); + self.update_edit_prediction_preview(&modifiers, window, cx); } let mouse_position = window.mouse_position(); @@ -5445,7 +5209,6 @@ impl Editor { fn update_edit_prediction_preview( &mut self, modifiers: &Modifiers, - position_map: &PositionMap, window: &mut Window, cx: &mut Context, ) { @@ -5455,37 +5218,31 @@ impl Editor { }; if &accept_keystroke.modifiers == modifiers { - let Some(completion) = self.active_inline_completion.as_ref() else { - return; - }; - - if !self.edit_prediction_requires_modifier() && !self.has_visible_completions_menu() { - return; - } - - let transitioned = self.edit_prediction_preview.start( - &completion.completion, - &position_map.snapshot, - self.selections - .newest_anchor() - .head() - .to_display_point(&position_map.snapshot), - ); + if !self.edit_prediction_preview_is_active() { + self.edit_prediction_preview = EditPredictionPreview::Active { + previous_scroll_position: None, + }; - if transitioned { - self.request_autoscroll(Autoscroll::fit(), cx); self.update_visible_inline_completion(window, cx); cx.notify(); } - } else if self.edit_prediction_preview.end( - self.selections - .newest_anchor() - .head() - .to_display_point(&position_map.snapshot), - position_map.scroll_pixel_position, - window, - cx, - ) { + } else if let EditPredictionPreview::Active { + previous_scroll_position, + } = self.edit_prediction_preview + { + if let (Some(previous_scroll_position), Some(position_map)) = + (previous_scroll_position, self.last_position_map.as_ref()) + { + self.set_scroll_position( + previous_scroll_position + .scroll_position(&position_map.snapshot.display_snapshot), + window, + cx, + ); + } + + self.edit_prediction_preview = EditPredictionPreview::Inactive; + self.clear_row_highlights::(); self.update_visible_inline_completion(window, cx); cx.notify(); } @@ -5493,7 +5250,7 @@ impl Editor { fn update_visible_inline_completion( &mut self, - window: &mut Window, + _window: &mut Window, cx: &mut Context, ) -> Option<()> { let selection = self.selections.newest_anchor(); @@ -5643,15 +5400,6 @@ impl Editor { )); self.stale_inline_completion_in_menu = None; - let editor_snapshot = self.snapshot(window, cx); - if self.edit_prediction_preview.restart( - &completion, - &editor_snapshot, - cursor.to_display_point(&editor_snapshot), - ) { - self.request_autoscroll(Autoscroll::fit(), cx); - } - self.active_inline_completion = Some(InlineCompletionState { inlay_ids, completion, @@ -5879,7 +5627,7 @@ impl Editor { } pub fn context_menu_visible(&self) -> bool { - !self.edit_prediction_preview.is_active() + !self.edit_prediction_preview_is_active() && self .context_menu .borrow() @@ -5990,12 +5738,12 @@ impl Editor { Icon::new(IconName::ZedPredictUp) }, ) - .child(Label::new("Hold")) + .child(Label::new("Hold").size(LabelSize::Small)) .children(ui::render_modifiers( &accept_keystroke.modifiers, PlatformStyle::platform(), Some(Color::Default), - None, + Some(IconSize::Small.rems().into()), true, )) .into_any(), @@ -14056,23 +13804,6 @@ impl Editor { } } - pub fn previewing_edit_prediction_move( - &mut self, - ) -> Option<(Anchor, &mut EditPredictionPreview)> { - if !self.edit_prediction_preview.is_active() { - return None; - }; - - self.active_inline_completion - .as_ref() - .and_then(|completion| match completion.completion { - InlineCompletion::Move { target, .. } => { - Some((target, &mut self.edit_prediction_preview)) - } - _ => None, - }) - } - pub fn show_local_cursors(&self, window: &mut Window, cx: &mut App) -> bool { (self.read_only(cx) || self.blink_manager.read(cx).visible()) && self.focus_handle.is_focused(window) @@ -14905,7 +14636,7 @@ impl Editor { } pub fn has_visible_completions_menu(&self) -> bool { - !self.edit_prediction_preview.is_active() + !self.edit_prediction_preview_is_active() && self.context_menu.borrow().as_ref().map_or(false, |menu| { menu.visible() && matches!(menu, CodeContextMenu::Completions(_)) }) diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 5695d8c0d38f3e4ae4decf3d250094a41e54a92e..c0006797ce8929b43e6100f7e0ad987d4beacfc0 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -16,12 +16,12 @@ use crate::{ mouse_context_menu::{self, MenuPosition, MouseContextMenu}, scroll::{axis_pair, scroll_amount::ScrollAmount, AxisPair}, AcceptEditPrediction, BlockId, ChunkReplacement, CursorShape, CustomBlockId, DisplayPoint, - DisplayRow, DocumentHighlightRead, DocumentHighlightWrite, EditDisplayMode, - EditPredictionPreview, Editor, EditorMode, EditorSettings, EditorSnapshot, EditorStyle, - ExpandExcerpts, FocusedBlock, GoToHunk, GoToPrevHunk, GutterDimensions, HalfPageDown, - HalfPageUp, HandleInput, HoveredCursor, InlineCompletion, JumpData, LineDown, LineUp, - OpenExcerpts, PageDown, PageUp, Point, RevertSelectedHunks, RowExt, RowRangeExt, SelectPhase, - Selection, SoftWrap, StickyHeaderExcerpt, ToPoint, ToggleFold, CURSORS_VISIBLE_FOR, + DisplayRow, DocumentHighlightRead, DocumentHighlightWrite, EditDisplayMode, Editor, EditorMode, + EditorSettings, EditorSnapshot, EditorStyle, ExpandExcerpts, FocusedBlock, GoToHunk, + GoToPrevHunk, GutterDimensions, HalfPageDown, HalfPageUp, HandleInput, HoveredCursor, + InlineCompletion, JumpData, LineDown, LineUp, OpenExcerpts, PageDown, PageUp, Point, + RevertSelectedHunks, RowExt, RowRangeExt, SelectPhase, Selection, SoftWrap, + StickyHeaderExcerpt, ToPoint, ToggleFold, CURSORS_VISIBLE_FOR, EDIT_PREDICTION_REQUIRES_MODIFIER_KEY_CONTEXT, FILE_HEADER_HEIGHT, GIT_BLAME_MAX_AUTHOR_CHARS_DISPLAYED, MAX_LINE_LEN, MULTI_BUFFER_EXCERPT_HEADER_HEIGHT, }; @@ -1116,7 +1116,6 @@ impl EditorElement { em_width: Pixels, em_advance: Pixels, autoscroll_containing_element: bool, - newest_selection_head: Option, window: &mut Window, cx: &mut App, ) -> Vec { @@ -1124,29 +1123,7 @@ impl EditorElement { let cursor_layouts = self.editor.update(cx, |editor, cx| { let mut cursors = Vec::new(); - let previewing_move = - if let Some((target, preview)) = editor.previewing_edit_prediction_move() { - cursors.extend(self.layout_edit_prediction_preview_cursor( - snapshot, - visible_display_row_range.clone(), - line_layouts, - content_origin, - scroll_pixel_position, - line_height, - em_advance, - preview, - target, - newest_selection_head, - window, - cx, - )); - - true - } else { - false - }; - - let show_local_cursors = !previewing_move && editor.show_local_cursors(window, cx); + let show_local_cursors = editor.show_local_cursors(window, cx); for (player_color, selections) in selections { for selection in selections { @@ -1288,50 +1265,6 @@ impl EditorElement { cursor_layouts } - #[allow(clippy::too_many_arguments)] - fn layout_edit_prediction_preview_cursor( - &self, - snapshot: &EditorSnapshot, - visible_row_range: Range, - line_layouts: &[LineWithInvisibles], - content_origin: gpui::Point, - scroll_pixel_position: gpui::Point, - line_height: Pixels, - em_advance: Pixels, - preview: &mut EditPredictionPreview, - target: Anchor, - cursor: Option, - window: &mut Window, - cx: &mut App, - ) -> Option { - let state = preview.move_state( - snapshot, - visible_row_range, - line_layouts, - scroll_pixel_position, - line_height, - target, - cursor, - )?; - - if !state.is_animation_completed() { - window.request_animation_frame(); - } - - let mut cursor = CursorLayout { - color: self.style.local_player.cursor, - block_width: em_advance, - origin: state.position, - line_height, - shape: CursorShape::Bar, - block_text: None, - cursor_name: None, - }; - - cursor.layout(content_origin, None, window, cx); - Some(cursor) - } - fn layout_scrollbars( &self, snapshot: &EditorSnapshot, @@ -3607,6 +3540,7 @@ impl EditorElement { fn layout_edit_prediction_popover( &self, text_bounds: &Bounds, + content_origin: gpui::Point, editor_snapshot: &EditorSnapshot, visible_row_range: Range, scroll_top: f32, @@ -3631,61 +3565,118 @@ impl EditorElement { } // Adjust text origin for horizontal scrolling (in some cases here) - let start_point = - text_bounds.origin - gpui::Point::new(scroll_pixel_position.x, Pixels(0.0)); + let start_point = content_origin - gpui::Point::new(scroll_pixel_position.x, Pixels(0.0)); // Clamp left offset after extreme scrollings let clamp_start = |point: gpui::Point| gpui::Point { - x: point.x.max(text_bounds.origin.x), + x: point.x.max(content_origin.x), y: point.y, }; match &active_inline_completion.completion { InlineCompletion::Move { target, .. } => { - if editor.edit_prediction_requires_modifier() { - let cursor_position = - target.to_display_point(&editor_snapshot.display_snapshot); + let target_display_point = target.to_display_point(editor_snapshot); - if !editor.edit_prediction_preview.is_active_settled() - || !visible_row_range.contains(&cursor_position.row()) - { + if editor.edit_prediction_requires_modifier() { + if !editor.edit_prediction_preview_is_active() { return None; } - let accept_keybind = editor.accept_edit_prediction_keybind(window, cx); - let accept_keystroke = accept_keybind.keystroke()?; + if target_display_point.row() < visible_row_range.start { + let mut element = inline_completion_accept_indicator( + "Scroll", + Some(IconName::ZedPredictUp), + editor, + window, + cx, + )? + .into_any(); - let mut element = div() - .px_2() - .py_1() - .elevation_2(cx) - .border_color(cx.theme().colors().border) - .rounded_br(px(0.)) - .child(Label::new(accept_keystroke.key.clone()).buffer_font(cx)) + element.layout_as_root(AvailableSpace::min_size(), window, cx); + + let cursor = newest_selection_head?; + let cursor_row_layout = line_layouts + .get(cursor.row().minus(visible_row_range.start) as usize)?; + let cursor_column = cursor.column() as usize; + + let cursor_character_x = cursor_row_layout.x_for_index(cursor_column); + + const PADDING_Y: Pixels = px(24.); + + let origin = start_point + point(cursor_character_x, PADDING_Y); + + element.prepaint_at(origin, window, cx); + return Some(element); + } else if target_display_point.row() >= visible_row_range.end { + let mut element = inline_completion_accept_indicator( + "Scroll", + Some(IconName::ZedPredictDown), + editor, + window, + cx, + )? .into_any(); - let size = element.layout_as_root(AvailableSpace::min_size(), window, cx); + let size = element.layout_as_root(AvailableSpace::min_size(), window, cx); - let cursor_row_layout = &line_layouts - [cursor_position.row().minus(visible_row_range.start) as usize]; - let cursor_column = cursor_position.column() as usize; + let cursor = newest_selection_head?; + let cursor_row_layout = line_layouts + .get(cursor.row().minus(visible_row_range.start) as usize)?; + let cursor_column = cursor.column() as usize; - let cursor_character_x = cursor_row_layout.x_for_index(cursor_column); - let target_y = (cursor_position.row().as_f32() - - scroll_pixel_position.y / line_height) - * line_height; + let cursor_character_x = cursor_row_layout.x_for_index(cursor_column); + const PADDING_Y: Pixels = px(24.); - let offset = point( - cursor_character_x - size.width, - target_y - size.height - PADDING_Y, - ); + let origin = start_point + + point( + cursor_character_x, + text_bounds.size.height - size.height - PADDING_Y, + ); - element.prepaint_at(text_bounds.origin + offset, window, cx); + element.prepaint_at(origin, window, cx); + return Some(element); + } else { + const POLE_WIDTH: Pixels = px(2.); + + let mut element = v_flex() + .child( + inline_completion_accept_indicator( + "Jump", None, editor, window, cx, + )? + .rounded_br(px(0.)), + ) + .child( + div() + .w(POLE_WIDTH) + .bg(cx.theme().colors().border) + .h(line_height), + ) + .items_end() + .into_any(); + + let size = element.layout_as_root(AvailableSpace::min_size(), window, cx); + + let line_layout = + line_layouts + .get(target_display_point.row().minus(visible_row_range.start) + as usize)?; + let target_column = target_display_point.column() as usize; + + let target_x = line_layout.x_for_index(target_column); + let target_y = (target_display_point.row().as_f32() * line_height) + - scroll_pixel_position.y; + + let origin = clamp_start( + start_point + point(target_x, target_y) + - point(size.width - POLE_WIDTH, size.height - line_height), + ); - return Some(element); + element.prepaint_at(origin, window, cx); + + return Some(element); + } } - let target_display_point = target.to_display_point(editor_snapshot); if target_display_point.row().as_f32() < scroll_top { let mut element = inline_completion_accept_indicator( "Jump to Edit", @@ -3693,7 +3684,8 @@ impl EditorElement { editor, window, cx, - )?; + )? + .into_any(); let size = element.layout_as_root(AvailableSpace::min_size(), window, cx); let offset = point((text_bounds.size.width - size.width) / 2., PADDING_Y); @@ -3706,7 +3698,8 @@ impl EditorElement { editor, window, cx, - )?; + )? + .into_any(); let size = element.layout_as_root(AvailableSpace::min_size(), window, cx); let offset = point( (text_bounds.size.width - size.width) / 2., @@ -3722,7 +3715,8 @@ impl EditorElement { editor, window, cx, - )?; + )? + .into_any(); let target_line_end = DisplayPoint::new( target_display_point.row(), editor_snapshot.line_len(target_display_point.row()), @@ -3782,7 +3776,8 @@ impl EditorElement { Some(( inline_completion_accept_indicator( "Accept", None, editor, window, cx, - )?, + )? + .into_any(), editor.display_to_pixel_point( target_line_end, editor_snapshot, @@ -5805,7 +5800,7 @@ fn inline_completion_accept_indicator( editor: &Editor, window: &mut Window, cx: &App, -) -> Option { +) -> Option
{ let accept_binding = editor.accept_edit_prediction_keybind(window, cx); let accept_keystroke = accept_binding.keystroke()?; @@ -5815,7 +5810,7 @@ fn inline_completion_accept_indicator( .text_size(TextSize::XSmall.rems(cx)) .text_color(cx.theme().colors().text) .gap_1() - .when(!editor.edit_prediction_preview.is_active(), |parent| { + .when(!editor.edit_prediction_preview_is_active(), |parent| { parent.children(ui::render_modifiers( &accept_keystroke.modifiers, PlatformStyle::platform(), @@ -5826,33 +5821,44 @@ fn inline_completion_accept_indicator( }) .child(accept_keystroke.key.clone()); - let padding_right = if icon.is_some() { px(4.) } else { px(8.) }; - let accent_color = cx.theme().colors().text_accent; - let editor_bg_color = cx.theme().colors().editor_background; - let bg_color = editor_bg_color.blend(accent_color.opacity(0.2)); + let result = h_flex() + .gap_1() + .border_1() + .rounded_md() + .shadow_sm() + .child(accept_key) + .child(Label::new(label).size(LabelSize::Small)) + .when_some(icon, |element, icon| { + element.child( + div() + .mt(px(1.5)) + .child(Icon::new(icon).size(IconSize::Small)), + ) + }); + + let colors = cx.theme().colors(); - Some( - h_flex() + let result = if editor.edit_prediction_requires_modifier() { + result + .py_1() + .px_2() + .elevation_2(cx) + .border_color(colors.border) + } else { + let accent_color = colors.text_accent; + let editor_bg_color = colors.editor_background; + let bg_color = editor_bg_color.blend(accent_color.opacity(0.2)); + let padding_right = if icon.is_some() { px(4.) } else { px(8.) }; + + result + .bg(bg_color) + .border_color(colors.text_accent.opacity(0.8)) .py_0p5() .pl_1() .pr(padding_right) - .gap_1() - .bg(bg_color) - .border_1() - .border_color(cx.theme().colors().text_accent.opacity(0.8)) - .rounded_md() - .shadow_sm() - .child(accept_key) - .child(Label::new(label).size(LabelSize::Small)) - .when_some(icon, |element, icon| { - element.child( - div() - .mt(px(1.5)) - .child(Icon::new(icon).size(IconSize::Small)), - ) - }) - .into_any(), - ) + }; + + Some(result) } pub struct AcceptEditPredictionBinding(pub(crate) Option); @@ -7357,7 +7363,7 @@ impl Element for EditorElement { let visible_row_range = start_row..end_row; let non_visible_cursors = cursors .iter() - .any(move |c| !visible_row_range.contains(&c.0.row())); + .any(|c| !visible_row_range.contains(&c.0.row())); let visible_cursors = self.layout_visible_cursors( &snapshot, @@ -7373,7 +7379,6 @@ impl Element for EditorElement { em_width, em_advance, autoscroll_containing_element, - newest_selection_head, window, cx, ); @@ -7527,6 +7532,7 @@ impl Element for EditorElement { let inline_completion_popover = self.layout_edit_prediction_popover( &text_hitbox.bounds, + content_origin, &snapshot, start_row..end_row, scroll_position.y, @@ -7598,6 +7604,7 @@ impl Element for EditorElement { let position_map = Rc::new(PositionMap { size: bounds.size, + visible_row_range, scroll_pixel_position, scroll_max, line_layouts, @@ -7991,6 +7998,7 @@ pub(crate) struct PositionMap { pub scroll_max: gpui::Point, pub em_width: Pixels, pub em_advance: Pixels, + pub visible_row_range: Range, pub line_layouts: Vec, pub snapshot: EditorSnapshot, pub text_hitbox: Hitbox, diff --git a/crates/editor/src/scroll.rs b/crates/editor/src/scroll.rs index 0b2c5819267d6e4ac103a1d603dbf121e71c73d0..637e7de466da9ee704cc9df20421462ee6f241d3 100644 --- a/crates/editor/src/scroll.rs +++ b/crates/editor/src/scroll.rs @@ -3,6 +3,7 @@ pub(crate) mod autoscroll; pub(crate) mod scroll_amount; use crate::editor_settings::{ScrollBeyondLastLine, ScrollbarAxes}; +use crate::EditPredictionPreview; use crate::{ display_map::{DisplaySnapshot, ToDisplayPoint}, hover_popover::hide_hover, @@ -495,6 +496,15 @@ impl Editor { hide_hover(self, cx); let workspace_id = self.workspace.as_ref().and_then(|workspace| workspace.1); + if let EditPredictionPreview::Active { + previous_scroll_position, + } = &mut self.edit_prediction_preview + { + if !autoscroll { + previous_scroll_position.take(); + } + } + self.scroll_manager.set_scroll_position( scroll_position, &display_map, diff --git a/crates/editor/src/scroll/autoscroll.rs b/crates/editor/src/scroll/autoscroll.rs index d4c57fd11394b9b9a5fd1f8eb31e2246266b2dc6..bb42cec57f8b73901c51baa70da838851a29457f 100644 --- a/crates/editor/src/scroll/autoscroll.rs +++ b/crates/editor/src/scroll/autoscroll.rs @@ -145,29 +145,6 @@ impl Editor { target_top = newest_selection_top; target_bottom = newest_selection_top + 1.; } - - if self.edit_prediction_preview.is_active() { - if let Some(completion) = self.active_inline_completion.as_ref() { - match completion.completion { - crate::InlineCompletion::Edit { .. } => {} - crate::InlineCompletion::Move { target, .. } => { - let target_row = target.to_display_point(&display_map).row().as_f32(); - - if target_row < target_top { - target_top = target_row; - } else if target_row >= target_bottom { - target_bottom = target_row + 1.; - } - - let selections_fit = target_bottom - target_top <= visible_lines; - if !selections_fit { - target_top = target_row; - target_bottom = target_row + 1.; - } - } - } - } - } } let margin = if matches!(self.mode, EditorMode::AutoHeight { .. }) { diff --git a/crates/ui/src/traits/styled_ext.rs b/crates/ui/src/traits/styled_ext.rs index d2aee0770b906018f58a87cec4c4051df195ee8b..48a515afd79343f3105ea408848c5bbda49e7de8 100644 --- a/crates/ui/src/traits/styled_ext.rs +++ b/crates/ui/src/traits/styled_ext.rs @@ -3,7 +3,7 @@ use gpui::{hsla, App, Styled}; use crate::prelude::*; use crate::ElevationIndex; -fn elevated(this: E, cx: &mut App, index: ElevationIndex) -> E { +fn elevated(this: E, cx: &App, index: ElevationIndex) -> E { this.bg(cx.theme().colors().elevated_surface_background) .rounded_lg() .border_1() @@ -54,7 +54,7 @@ pub trait StyledExt: Styled + Sized { /// Sets `bg()`, `rounded_lg()`, `border()`, `border_color()`, `shadow()` /// /// Examples: Notifications, Palettes, Detached/Floating Windows, Detached/Floating Panels - fn elevation_2(self, cx: &mut App) -> Self { + fn elevation_2(self, cx: &App) -> Self { elevated(self, cx, ElevationIndex::ElevatedSurface) } From cc931a8fcc2f8e26cf7a382838dd474e747d66b1 Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Tue, 11 Feb 2025 18:45:37 -0500 Subject: [PATCH 2/4] theme: Add support for setting light/dark icon themes (#24702) This PR adds support for configuring both a light and dark icon theme in `settings.json`. In addition to accepting just an icon theme name, the `icon_theme` field now also accepts an object in the following form: ```jsonc { "icon_theme": { "mode": "system", "light": "Zed (Default)", "dark": "Zed (Default)" } } ``` Both `light` and `dark` are required, and indicate which icon theme should be used when the system is in light mode and dark mode, respectively. The `mode` field is optional and indicates which icon theme should be used: - `"system"` - Use the icon theme that corresponds to the system's appearance. - `"light"` - Use the icon theme indicated by the `light` field. - `"dark"` - Use the icon theme indicated by the `dark` field. Closes https://github.com/zed-industries/zed/issues/24695. Release Notes: - Added support for configuring both a light and dark icon theme and switching between them based on system preference. --- crates/theme/src/settings.rs | 155 +++++++++++++++--- .../theme_selector/src/icon_theme_selector.rs | 8 +- crates/workspace/src/workspace.rs | 1 + 3 files changed, 142 insertions(+), 22 deletions(-) diff --git a/crates/theme/src/settings.rs b/crates/theme/src/settings.rs index f44e45d549dd4b0c7692c3c027047109884a9781..dfce40f4411db770066c2d9e0c90714deccc9549 100644 --- a/crates/theme/src/settings.rs +++ b/crates/theme/src/settings.rs @@ -112,7 +112,6 @@ pub struct ThemeSettings { /// The terminal font family can be overridden using it's own setting. pub buffer_line_height: BufferLineHeight, /// The current theme selection. - /// TODO: Document this further pub theme_selection: Option, /// The active theme. pub active_theme: Arc, @@ -120,6 +119,8 @@ pub struct ThemeSettings { /// /// Note: This setting is still experimental. See [this tracking issue](https://github.com/zed-industries/zed/issues/18078) pub theme_overrides: Option, + /// The current icon theme selection. + pub icon_theme_selection: Option, /// The active icon theme. pub active_icon_theme: Arc, /// The density of the UI. @@ -167,25 +168,28 @@ impl ThemeSettings { /// Reloads the current icon theme. /// - /// Reads the [`ThemeSettings`] to know which icon theme should be loaded. + /// Reads the [`ThemeSettings`] to know which icon theme should be loaded, + /// taking into account the current [`SystemAppearance`]. pub fn reload_current_icon_theme(cx: &mut App) { let mut theme_settings = ThemeSettings::get_global(cx).clone(); + let system_appearance = SystemAppearance::global(cx); - let active_theme = theme_settings.active_icon_theme.clone(); - let mut icon_theme_name = active_theme.name.as_ref(); + if let Some(icon_theme_selection) = theme_settings.icon_theme_selection.clone() { + let mut icon_theme_name = icon_theme_selection.icon_theme(*system_appearance); - // If the selected theme doesn't exist, fall back to the default theme. - let theme_registry = ThemeRegistry::global(cx); - if theme_registry - .get_icon_theme(icon_theme_name) - .ok() - .is_none() - { - icon_theme_name = DEFAULT_ICON_THEME_NAME; - }; + // If the selected icon theme doesn't exist, fall back to the default theme. + let theme_registry = ThemeRegistry::global(cx); + if theme_registry + .get_icon_theme(icon_theme_name) + .ok() + .is_none() + { + icon_theme_name = DEFAULT_ICON_THEME_NAME; + }; - if let Some(_theme) = theme_settings.switch_icon_theme(icon_theme_name, cx) { - ThemeSettings::override_global(theme_settings, cx); + if let Some(_theme) = theme_settings.switch_icon_theme(icon_theme_name, cx) { + ThemeSettings::override_global(theme_settings, cx); + } } } } @@ -299,6 +303,55 @@ impl ThemeSelection { } } +fn icon_theme_name_ref(_: &mut SchemaGenerator) -> Schema { + Schema::new_ref("#/definitions/IconThemeName".into()) +} + +/// Represents the selection of an icon theme, which can be either static or dynamic. +#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq, Eq)] +#[serde(untagged)] +pub enum IconThemeSelection { + /// A static icon theme selection, represented by a single icon theme name. + Static(#[schemars(schema_with = "icon_theme_name_ref")] String), + /// A dynamic icon theme selection, which can change based on the [`ThemeMode`]. + Dynamic { + /// The mode used to determine which theme to use. + #[serde(default)] + mode: ThemeMode, + /// The icon theme to use for light mode. + #[schemars(schema_with = "icon_theme_name_ref")] + light: String, + /// The icon theme to use for dark mode. + #[schemars(schema_with = "icon_theme_name_ref")] + dark: String, + }, +} + +impl IconThemeSelection { + /// Returns the icon theme name based on the given [`Appearance`]. + pub fn icon_theme(&self, system_appearance: Appearance) -> &str { + match self { + Self::Static(theme) => theme, + Self::Dynamic { mode, light, dark } => match mode { + ThemeMode::Light => light, + ThemeMode::Dark => dark, + ThemeMode::System => match system_appearance { + Appearance::Light => light, + Appearance::Dark => dark, + }, + }, + } + } + + /// Returns the [`ThemeMode`] for the [`IconThemeSelection`]. + pub fn mode(&self) -> Option { + match self { + IconThemeSelection::Static(_) => None, + IconThemeSelection::Dynamic { mode, .. } => Some(*mode), + } + } +} + /// Settings for rendering text in UI and text buffers. #[derive(Clone, Debug, Default, Serialize, Deserialize, JsonSchema)] pub struct ThemeSettingsContent { @@ -344,7 +397,7 @@ pub struct ThemeSettingsContent { pub theme: Option, /// The name of the icon theme to use. #[serde(default)] - pub icon_theme: Option, + pub icon_theme: Option, /// UNSTABLE: Expect many elements to be broken. /// @@ -393,6 +446,27 @@ impl ThemeSettingsContent { } } + /// Sets the icon theme for the given appearance to the icon theme with the specified name. + pub fn set_icon_theme(&mut self, icon_theme_name: String, appearance: Appearance) { + if let Some(selection) = self.icon_theme.as_mut() { + let icon_theme_to_update = match selection { + IconThemeSelection::Static(theme) => theme, + IconThemeSelection::Dynamic { mode, light, dark } => match mode { + ThemeMode::Light => light, + ThemeMode::Dark => dark, + ThemeMode::System => match appearance { + Appearance::Light => light, + Appearance::Dark => dark, + }, + }, + }; + + *icon_theme_to_update = icon_theme_name.to_string(); + } else { + self.theme = Some(ThemeSelection::Static(icon_theme_name.to_string())); + } + } + /// Sets the mode for the theme. pub fn set_mode(&mut self, mode: ThemeMode) { if let Some(selection) = self.theme.as_mut() { @@ -419,6 +493,27 @@ impl ThemeSettingsContent { dark: ThemeSettings::DEFAULT_DARK_THEME.into(), }); } + + if let Some(selection) = self.icon_theme.as_mut() { + match selection { + IconThemeSelection::Static(icon_theme) => { + // If the icon theme was previously set to a single static + // theme, we don't know whether it was a light or dark + // theme, so we just use it for both. + self.icon_theme = Some(IconThemeSelection::Dynamic { + mode, + light: icon_theme.clone(), + dark: icon_theme.clone(), + }); + } + IconThemeSelection::Dynamic { + mode: mode_to_update, + .. + } => *mode_to_update = mode, + } + } else { + self.icon_theme = Some(IconThemeSelection::Static(DEFAULT_ICON_THEME_NAME.into())); + } } } @@ -588,10 +683,15 @@ impl settings::Settings for ThemeSettings { .or(themes.get(&zed_default_dark().name)) .unwrap(), theme_overrides: None, + icon_theme_selection: defaults.icon_theme.clone(), active_icon_theme: defaults .icon_theme .as_ref() - .and_then(|name| themes.get_icon_theme(name).ok()) + .and_then(|selection| { + themes + .get_icon_theme(selection.icon_theme(*system_appearance)) + .ok() + }) .unwrap_or_else(|| themes.get_icon_theme(DEFAULT_ICON_THEME_NAME).unwrap()), ui_density: defaults.ui_density.unwrap_or(UiDensity::Default), unnecessary_code_fade: defaults.unnecessary_code_fade.unwrap_or(0.0), @@ -647,8 +747,12 @@ impl settings::Settings for ThemeSettings { this.apply_theme_overrides(); if let Some(value) = &value.icon_theme { - if let Some(icon_theme) = themes.get_icon_theme(value).log_err() { - this.active_icon_theme = icon_theme.clone(); + this.icon_theme_selection = Some(value.clone()); + + let icon_theme_name = value.icon_theme(*system_appearance); + + if let Some(icon_theme) = themes.get_icon_theme(icon_theme_name).log_err() { + this.active_icon_theme = icon_theme; } } @@ -689,8 +793,21 @@ impl settings::Settings for ThemeSettings { ..Default::default() }; + let icon_theme_names = ThemeRegistry::global(cx) + .list_icon_themes() + .into_iter() + .map(|icon_theme| Value::String(icon_theme.name.to_string())) + .collect(); + + let icon_theme_name_schema = SchemaObject { + instance_type: Some(InstanceType::String.into()), + enum_values: Some(icon_theme_names), + ..Default::default() + }; + root_schema.definitions.extend([ ("ThemeName".into(), theme_name_schema.into()), + ("IconThemeName".into(), icon_theme_name_schema.into()), ("FontFamilies".into(), params.font_family_schema()), ("FontFallbacks".into(), params.font_fallback_schema()), ]); diff --git a/crates/theme_selector/src/icon_theme_selector.rs b/crates/theme_selector/src/icon_theme_selector.rs index 28af76ad7159042bb78e1c571bdd7cdd6c6b83a0..36429fc86a7638a9e51dfff819c859e02770d314 100644 --- a/crates/theme_selector/src/icon_theme_selector.rs +++ b/crates/theme_selector/src/icon_theme_selector.rs @@ -7,7 +7,7 @@ use gpui::{ use picker::{Picker, PickerDelegate}; use settings::{update_settings_file, Settings as _, SettingsStore}; use std::sync::Arc; -use theme::{IconTheme, ThemeMeta, ThemeRegistry, ThemeSettings}; +use theme::{Appearance, IconTheme, ThemeMeta, ThemeRegistry, ThemeSettings}; use ui::{prelude::*, v_flex, ListItem, ListItemSpacing}; use util::ResultExt; use workspace::{ui::HighlightedLabel, ModalView}; @@ -151,7 +151,7 @@ impl PickerDelegate for IconThemeSelectorDelegate { fn confirm( &mut self, _: bool, - _window: &mut Window, + window: &mut Window, cx: &mut Context>, ) { self.selection_completed = true; @@ -165,8 +165,10 @@ impl PickerDelegate for IconThemeSelectorDelegate { value = theme_name ); + let appearance = Appearance::from(window.appearance()); + update_settings_file::(self.fs.clone(), cx, move |settings, _| { - settings.icon_theme = Some(theme_name.to_string()); + settings.set_icon_theme(theme_name.to_string(), appearance); }); self.selector diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index e4e3a7c783ec71aa29ce8518b85d122899aecf80..2ad9cf1c4d8408161d627174d3e287132ce319d4 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -1082,6 +1082,7 @@ impl Workspace { *SystemAppearance::global_mut(cx) = SystemAppearance(window_appearance.into()); ThemeSettings::reload_current_theme(cx); + ThemeSettings::reload_current_icon_theme(cx); }), cx.on_release(move |this, cx| { this.app_state.workspace_store.update(cx, move |store, _| { From 9a9fdce2530943c090e6fc9531e660c096a10388 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 11 Feb 2025 16:32:15 -0800 Subject: [PATCH 3/4] Fixes for accept edit popovers (#24703) Follow-up to #24700 Release Notes: - N/A --------- Co-authored-by: danilo-leal Co-authored-by: agu-z --- crates/editor/src/editor.rs | 25 ++++++++++--------- crates/editor/src/element.rs | 48 +++++++++++++++--------------------- 2 files changed, 34 insertions(+), 39 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 74309e3b74e5e101a23ffb92659966478973cf66..e8fc0593e7b8cd05020e6ac482f6920837d790f7 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -1567,6 +1567,7 @@ impl Editor { if has_active_edit_prediction { key_context.add("copilot_suggestion"); key_context.add(EDIT_PREDICTION_KEY_CONTEXT); + if showing_completions || self.edit_prediction_requires_modifier() { key_context.add(EDIT_PREDICTION_REQUIRES_MODIFIER_KEY_CONTEXT); } @@ -4793,12 +4794,10 @@ impl Editor { } pub fn edit_prediction_preview_is_active(&self) -> bool { - match self.edit_prediction_preview { - EditPredictionPreview::Inactive => false, - EditPredictionPreview::Active { .. } => { - self.edit_prediction_requires_modifier() || self.has_visible_completions_menu() - } - } + matches!( + self.edit_prediction_preview, + EditPredictionPreview::Active { .. } + ) } pub fn inline_completions_enabled(&self, cx: &App) -> bool { @@ -4971,7 +4970,7 @@ impl Editor { if position_map .visible_row_range .contains(&target.to_display_point(&position_map.snapshot).row()) - || !self.edit_prediction_preview_is_active() + || !self.edit_prediction_requires_modifier() { // Note that this is also done in vim's handler of the Tab action. self.change_selections( @@ -5218,7 +5217,10 @@ impl Editor { }; if &accept_keystroke.modifiers == modifiers { - if !self.edit_prediction_preview_is_active() { + if matches!( + self.edit_prediction_preview, + EditPredictionPreview::Inactive + ) { self.edit_prediction_preview = EditPredictionPreview::Active { previous_scroll_position: None, }; @@ -5795,15 +5797,16 @@ impl Editor { .max_w(max_width) .flex_1() .px_2() - .py_1() .elevation_2(cx) .border_color(cx.theme().colors().border) - .child(completion) - .child(ui::Divider::vertical()) + .child(div().py_1().overflow_hidden().child(completion)) .child( h_flex() .h_full() + .border_l_1() + .border_color(cx.theme().colors().border) .gap_1() + .py_1() .pl_2() .child( h_flex() diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index c0006797ce8929b43e6100f7e0ad987d4beacfc0..78f07c60fabfeff7e42c76b3ee1716f1975d4909 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -3585,7 +3585,7 @@ impl EditorElement { if target_display_point.row() < visible_row_range.start { let mut element = inline_completion_accept_indicator( "Scroll", - Some(IconName::ZedPredictUp), + Some(IconName::ArrowUp), editor, window, cx, @@ -3601,7 +3601,7 @@ impl EditorElement { let cursor_character_x = cursor_row_layout.x_for_index(cursor_column); - const PADDING_Y: Pixels = px(24.); + const PADDING_Y: Pixels = px(12.); let origin = start_point + point(cursor_character_x, PADDING_Y); @@ -3610,7 +3610,7 @@ impl EditorElement { } else if target_display_point.row() >= visible_row_range.end { let mut element = inline_completion_accept_indicator( "Scroll", - Some(IconName::ZedPredictDown), + Some(IconName::ArrowDown), editor, window, cx, @@ -3625,7 +3625,7 @@ impl EditorElement { let cursor_column = cursor.column() as usize; let cursor_character_x = cursor_row_layout.x_for_index(cursor_column); - const PADDING_Y: Pixels = px(24.); + const PADDING_Y: Pixels = px(12.); let origin = start_point + point( @@ -3643,12 +3643,14 @@ impl EditorElement { inline_completion_accept_indicator( "Jump", None, editor, window, cx, )? - .rounded_br(px(0.)), + .rounded_br(px(0.)) + .rounded_tr(px(0.)) + .border_r_2(), ) .child( div() .w(POLE_WIDTH) - .bg(cx.theme().colors().border) + .bg(cx.theme().colors().text_accent.opacity(0.8)) .h(line_height), ) .items_end() @@ -5821,11 +5823,23 @@ fn inline_completion_accept_indicator( }) .child(accept_keystroke.key.clone()); + let colors = cx.theme().colors(); + + let accent_color = colors.text_accent; + let editor_bg_color = colors.editor_background; + let bg_color = editor_bg_color.blend(accent_color.opacity(0.2)); + let padding_right = if icon.is_some() { px(4.) } else { px(8.) }; + let result = h_flex() .gap_1() .border_1() .rounded_md() .shadow_sm() + .bg(bg_color) + .border_color(colors.text_accent.opacity(0.8)) + .py_0p5() + .pl_1() + .pr(padding_right) .child(accept_key) .child(Label::new(label).size(LabelSize::Small)) .when_some(icon, |element, icon| { @@ -5836,28 +5850,6 @@ fn inline_completion_accept_indicator( ) }); - let colors = cx.theme().colors(); - - let result = if editor.edit_prediction_requires_modifier() { - result - .py_1() - .px_2() - .elevation_2(cx) - .border_color(colors.border) - } else { - let accent_color = colors.text_accent; - let editor_bg_color = colors.editor_background; - let bg_color = editor_bg_color.blend(accent_color.opacity(0.2)); - let padding_right = if icon.is_some() { px(4.) } else { px(8.) }; - - result - .bg(bg_color) - .border_color(colors.text_accent.opacity(0.8)) - .py_0p5() - .pl_1() - .pr(padding_right) - }; - Some(result) } From f5fd3d98ad1eac632e6a4d653f9640dc86ec474e Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 11 Feb 2025 17:40:40 -0700 Subject: [PATCH 4/4] Fix project diff focus (#24691) Release Notes: - N/A --- crates/git_ui/src/project_diff.rs | 53 ++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 19 deletions(-) diff --git a/crates/git_ui/src/project_diff.rs b/crates/git_ui/src/project_diff.rs index 485750b9b28c053f4111d93c394fbbc888501b7a..c48934850e4f8337debc19c1e28553af8450614a 100644 --- a/crates/git_ui/src/project_diff.rs +++ b/crates/git_ui/src/project_diff.rs @@ -335,6 +335,22 @@ impl ProjectDiff { cx, ); }); + if self.multibuffer.read(cx).is_empty() + && self + .editor + .read(cx) + .focus_handle(cx) + .contains_focused(window, cx) + { + self.focus_handle.focus(window); + } else if self.focus_handle.contains_focused(window, cx) + && !self.multibuffer.read(cx).is_empty() + { + self.editor.update(cx, |editor, cx| { + editor.focus_handle(cx).focus(window); + editor.move_to_beginning(&Default::default(), window, cx); + }); + } if self.pending_scroll.as_ref() == Some(&path_key) { self.scroll_to_path(path_key, window, cx); } @@ -365,8 +381,12 @@ impl ProjectDiff { impl EventEmitter for ProjectDiff {} impl Focusable for ProjectDiff { - fn focus_handle(&self, _: &App) -> FocusHandle { - self.focus_handle.clone() + fn focus_handle(&self, cx: &App) -> FocusHandle { + if self.multibuffer.read(cx).is_empty() { + self.focus_handle.clone() + } else { + self.editor.focus_handle(cx) + } } } @@ -537,22 +557,17 @@ impl Item for ProjectDiff { impl Render for ProjectDiff { fn render(&mut self, _: &mut Window, cx: &mut Context) -> impl IntoElement { let is_empty = self.multibuffer.read(cx).is_empty(); - if is_empty { - div() - .bg(cx.theme().colors().editor_background) - .flex() - .items_center() - .justify_center() - .size_full() - .child(Label::new("No uncommitted changes")) - } else { - div() - .bg(cx.theme().colors().editor_background) - .flex() - .items_center() - .justify_center() - .size_full() - .child(self.editor.clone()) - } + + div() + .track_focus(&self.focus_handle) + .bg(cx.theme().colors().editor_background) + .flex() + .items_center() + .justify_center() + .size_full() + .when(is_empty, |el| { + el.child(Label::new("No uncommitted changes")) + }) + .when(!is_empty, |el| el.child(self.editor.clone())) } }