Scroll to follow expanding part of editor::SelectLargerSyntaxNode (#27295)

When the selection grows both ways, the new code prioritizes the top part instead of bottom one, this is usually more helpful considering that most programming language grammars tend to define tokens right before large delimited blocks, and rarely after (because humans and parsers read from top to bottom). Also, revert selection when convenient, so you have more control over what you're selecting, looking at the selection `head` is commonly more convenient than at the `tail`. Release Notes: - Improve scrolling of `editor::SelectLargerSyntaxNode` for better visibility.
2025-03-22 06:06:13 -03:00 · 2025-03-22 06:06:13 -03:00 · 9918b6cade
commit 9918b6cade
parent fa677bdc38
5 changed files with 194 additions and 114 deletions
--- a/crates/editor/src/editor.rs
+++ b/crates/editor/src/editor.rs
@ -188,16 +188,12 @@ use ui::{
 use util::{maybe, post_inc, RangeExt, ResultExt, TryFutureExt};
 use workspace::{
    item::{ItemHandle, PreviewTabsSettings},
-    ItemId, RestoreOnStartupBehavior, SERIALIZATION_THROTTLE_TIME,
-};
-use workspace::{
    notifications::{DetachAndPromptErr, NotificationId, NotifyTaskExt},
-    WorkspaceSettings,
+    searchable::SearchEvent,
+    Item as WorkspaceItem, ItemId, ItemNavHistory, OpenInTerminal, OpenTerminal,
+    RestoreOnStartupBehavior, SplitDirection, TabBarSettings, Toast, ViewId, Workspace,
+    WorkspaceId, WorkspaceSettings, SERIALIZATION_THROTTLE_TIME,
 };
-use workspace::{
-    searchable::SearchEvent, ItemNavHistory, SplitDirection, ViewId, Workspace, WorkspaceId,
-};
-use workspace::{Item as WorkspaceItem, OpenInTerminal, OpenTerminal, TabBarSettings, Toast};

 use crate::hover_links::{find_url, find_url_from_range};
 use crate::signature_help::{SignatureHelpHiddenBy, SignatureHelpState};
@ -641,7 +637,7 @@ pub struct Editor {
    selection_history: SelectionHistory,
    autoclose_regions: Vec<AutocloseRegion>,
    snippet_stack: InvalidationStack<SnippetState>,
-    select_larger_syntax_node_stack: Vec<Box<[Selection<usize>]>>,
+    select_syntax_node_history: SelectSyntaxNodeHistory,
    ime_transaction: Option<TransactionId>,
    active_diagnostics: Option<ActiveDiagnosticGroup>,
    show_inline_diagnostics: bool,
@ -1032,6 +1028,42 @@ pub struct ClipboardSelection {
    pub first_line_indent: u32,
 }

+// selections, scroll behavior, was newest selection reversed
+type SelectSyntaxNodeHistoryState = (
+    Box<[Selection<usize>]>,
+    SelectSyntaxNodeScrollBehavior,
+    bool,
+);
+
+#[derive(Default)]
+struct SelectSyntaxNodeHistory {
+    stack: Vec<SelectSyntaxNodeHistoryState>,
+    // disable temporarily to allow changing selections without losing the stack
+    pub disable_clearing: bool,
+}
+
+impl SelectSyntaxNodeHistory {
+    pub fn try_clear(&mut self) {
+        if !self.disable_clearing {
+            self.stack.clear();
+        }
+    }
+
+    pub fn push(&mut self, selection: SelectSyntaxNodeHistoryState) {
+        self.stack.push(selection);
+    }
+
+    pub fn pop(&mut self) -> Option<SelectSyntaxNodeHistoryState> {
+        self.stack.pop()
+    }
+}
+
+enum SelectSyntaxNodeScrollBehavior {
+    CursorTop,
+    CenterSelection,
+    CursorBottom,
+}
+
 #[derive(Debug)]
 pub(crate) struct NavigationData {
    cursor_anchor: Anchor,
@ -1374,7 +1406,7 @@ impl Editor {
            selection_history: Default::default(),
            autoclose_regions: Default::default(),
            snippet_stack: Default::default(),
-            select_larger_syntax_node_stack: Vec::new(),
+            select_syntax_node_history: SelectSyntaxNodeHistory::default(),
            ime_transaction: Default::default(),
            active_diagnostics: None,
            show_inline_diagnostics: ProjectSettings::get_global(cx).diagnostics.inline.enabled,
@ -2136,7 +2168,7 @@ impl Editor {
        self.add_selections_state = None;
        self.select_next_state = None;
        self.select_prev_state = None;
-        self.select_larger_syntax_node_stack.clear();
+        self.select_syntax_node_history.try_clear();
        self.invalidate_autoclose_regions(&self.selections.disjoint_anchors(), buffer);
        self.snippet_stack
            .invalidate(&self.selections.disjoint_anchors(), buffer);
@ -11819,13 +11851,19 @@ impl Editor {
        window: &mut Window,
        cx: &mut Context<Self>,
    ) {
+        let Some(visible_row_count) = self.visible_row_count() else {
+            return;
+        };
+        let old_selections: Box<[_]> = self.selections.all::<usize>(cx).into();
+        if old_selections.is_empty() {
+            return;
+        }
+
        let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx));
        let buffer = self.buffer.read(cx).snapshot(cx);
-        let old_selections = self.selections.all::<usize>(cx).into_boxed_slice();

-        let mut stack = mem::take(&mut self.select_larger_syntax_node_stack);
        let mut selected_larger_node = false;
-        let new_selections = old_selections
+        let mut new_selections = old_selections
            .iter()
            .map(|selection| {
                let old_range = selection.start..selection.end;
@ -11867,13 +11905,56 @@ impl Editor {
            })
            .collect::<Vec<_>>();

-        if selected_larger_node {
-            stack.push(old_selections);
-            self.change_selections(Some(Autoscroll::fit()), window, cx, |s| {
-                s.select(new_selections);
-            });
+        if !selected_larger_node {
+            return; // don't put this call in the history
        }
-        self.select_larger_syntax_node_stack = stack;
+
+        // scroll based on transformation done to the last selection created by the user
+        let (last_old, last_new) = old_selections
+            .last()
+            .zip(new_selections.last().cloned())
+            .expect("old_selections isn't empty");
+
+        // revert selection
+        let is_selection_reversed = {
+            let should_newest_selection_be_reversed = last_old.start != last_new.start;
+            new_selections.last_mut().expect("checked above").reversed =
+                should_newest_selection_be_reversed;
+            should_newest_selection_be_reversed
+        };
+
+        if selected_larger_node {
+            self.select_syntax_node_history.disable_clearing = true;
+            self.change_selections(None, window, cx, |s| {
+                s.select(new_selections.clone());
+            });
+            self.select_syntax_node_history.disable_clearing = false;
+        }
+
+        let start_row = last_new.start.to_display_point(&display_map).row().0;
+        let end_row = last_new.end.to_display_point(&display_map).row().0;
+        let selection_height = end_row - start_row + 1;
+        let scroll_margin_rows = self.vertical_scroll_margin() as u32;
+
+        // if fits on screen (considering margin), keep it in the middle, else, scroll to selection head
+        let scroll_behavior = if visible_row_count >= selection_height + scroll_margin_rows * 2 {
+            let middle_row = (end_row + start_row) / 2;
+            let selection_center = middle_row.saturating_sub(visible_row_count / 2);
+            self.set_scroll_top_row(DisplayRow(selection_center), window, cx);
+            SelectSyntaxNodeScrollBehavior::CenterSelection
+        } else if is_selection_reversed {
+            self.scroll_cursor_top(&Default::default(), window, cx);
+            SelectSyntaxNodeScrollBehavior::CursorTop
+        } else {
+            self.scroll_cursor_bottom(&Default::default(), window, cx);
+            SelectSyntaxNodeScrollBehavior::CursorBottom
+        };
+
+        self.select_syntax_node_history.push((
+            old_selections,
+            scroll_behavior,
+            is_selection_reversed,
+        ));
    }

    pub fn select_smaller_syntax_node(
@ -11882,13 +11963,44 @@ impl Editor {
        window: &mut Window,
        cx: &mut Context<Self>,
    ) {
-        let mut stack = mem::take(&mut self.select_larger_syntax_node_stack);
-        if let Some(selections) = stack.pop() {
-            self.change_selections(Some(Autoscroll::fit()), window, cx, |s| {
+        let Some(visible_row_count) = self.visible_row_count() else {
+            return;
+        };
+
+        if let Some((mut selections, scroll_behavior, is_selection_reversed)) =
+            self.select_syntax_node_history.pop()
+        {
+            let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx));
+
+            if let Some(selection) = selections.last_mut() {
+                selection.reversed = is_selection_reversed;
+            }
+
+            self.select_syntax_node_history.disable_clearing = true;
+            self.change_selections(None, window, cx, |s| {
                s.select(selections.to_vec());
            });
+            self.select_syntax_node_history.disable_clearing = false;
+
+            let newest = self.selections.newest::<usize>(cx);
+            let start_row = newest.start.to_display_point(&display_map).row().0;
+            let end_row = newest.end.to_display_point(&display_map).row().0;
+
+            match scroll_behavior {
+                SelectSyntaxNodeScrollBehavior::CursorTop => {
+                    self.scroll_cursor_top(&Default::default(), window, cx);
+                }
+                SelectSyntaxNodeScrollBehavior::CenterSelection => {
+                    let middle_row = (end_row + start_row) / 2;
+                    let selection_center = middle_row.saturating_sub(visible_row_count / 2);
+                    // centralize the selection, not the cursor
+                    self.set_scroll_top_row(DisplayRow(selection_center), window, cx);
+                }
+                SelectSyntaxNodeScrollBehavior::CursorBottom => {
+                    self.scroll_cursor_bottom(&Default::default(), window, cx);
+                }
+            }
        }
-        self.select_larger_syntax_node_stack = stack;
    }

    fn refresh_runnables(&mut self, window: &mut Window, cx: &mut Context<Self>) -> Task<()> {
--- a/crates/editor/src/editor_tests.rs
+++ b/crates/editor/src/editor_tests.rs
@ -6033,7 +6033,7 @@ async fn test_select_larger_smaller_syntax_node(cx: &mut TestAppContext) {
                use mod1::mod2::{mod3, «mod4ˇ»};

                fn fn_1«ˇ(param1: bool, param2: &str)» {
-                    let var1 = "«textˇ»";
+                    let var1 = "«ˇtext»";
                }
            "#},
            cx,
@ -6101,7 +6101,7 @@ async fn test_select_larger_smaller_syntax_node(cx: &mut TestAppContext) {
                use mod1::mod2::{mod3, «mod4ˇ»};

                fn fn_1«ˇ(param1: bool, param2: &str)» {
-                    let var1 = "«textˇ»";
+                    let var1 = "«ˇtext»";
                }
            "#},
            cx,
@ -6170,7 +6170,7 @@ async fn test_select_larger_smaller_syntax_node(cx: &mut TestAppContext) {
                use mod1::mod2::«{mod3, mod4}ˇ»;

                fn fn_1«ˇ(param1: bool, param2: &str)» {
-                    «let var1 = "text";ˇ»
+                    «ˇlet var1 = "text";»
                }
            "#},
            cx,
--- a/crates/editor/src/scroll.rs
+++ b/crates/editor/src/scroll.rs
@ -462,6 +462,28 @@ impl Editor {
        self.set_scroll_position_internal(scroll_position, true, false, window, cx);
    }

+    /// Scrolls so that `row` is at the top of the editor view.
+    pub fn set_scroll_top_row(
+        &mut self,
+        row: DisplayRow,
+        window: &mut Window,
+        cx: &mut Context<Editor>,
+    ) {
+        let snapshot = self.snapshot(window, cx).display_snapshot;
+        let new_screen_top = DisplayPoint::new(row, 0);
+        let new_screen_top = new_screen_top.to_offset(&snapshot, Bias::Left);
+        let new_anchor = snapshot.buffer_snapshot.anchor_before(new_screen_top);
+
+        self.set_scroll_anchor(
+            ScrollAnchor {
+                anchor: new_anchor,
+                offset: Default::default(),
+            },
+            window,
+            cx,
+        );
+    }
+
    pub(crate) fn set_scroll_position_internal(
        &mut self,
        scroll_position: gpui::Point<f32>,
--- a/crates/editor/src/scroll/actions.rs
+++ b/crates/editor/src/scroll/actions.rs
@ -1,8 +1,8 @@
 use super::Axis;
 use crate::{
-    Autoscroll, Bias, Editor, EditorMode, NextScreen, NextScrollCursorCenterTopBottom,
-    ScrollAnchor, ScrollCursorBottom, ScrollCursorCenter, ScrollCursorCenterTopBottom,
-    ScrollCursorTop, SCROLL_CENTER_TOP_BOTTOM_DEBOUNCE_TIMEOUT,
+    display_map::DisplayRow, Autoscroll, Editor, EditorMode, NextScreen,
+    NextScrollCursorCenterTopBottom, ScrollCursorBottom, ScrollCursorCenter,
+    ScrollCursorCenterTopBottom, ScrollCursorTop, SCROLL_CENTER_TOP_BOTTOM_DEBOUNCE_TIMEOUT,
 };
 use gpui::{Context, Point, Window};

@ -40,41 +40,17 @@ impl Editor {
        window: &mut Window,
        cx: &mut Context<Self>,
    ) {
-        let snapshot = self.snapshot(window, cx).display_snapshot;
-        let visible_rows = if let Some(visible_rows) = self.visible_line_count() {
-            visible_rows as u32
-        } else {
-            return;
-        };
-
-        let scroll_margin_rows = self.vertical_scroll_margin() as u32;
-        let mut new_screen_top = self.selections.newest_display(cx).head();
-        *new_screen_top.column_mut() = 0;
        match self.next_scroll_position {
            NextScrollCursorCenterTopBottom::Center => {
-                *new_screen_top.row_mut() = new_screen_top.row().0.saturating_sub(visible_rows / 2);
+                self.scroll_cursor_center(&Default::default(), window, cx);
            }
            NextScrollCursorCenterTopBottom::Top => {
-                *new_screen_top.row_mut() =
-                    new_screen_top.row().0.saturating_sub(scroll_margin_rows);
+                self.scroll_cursor_top(&Default::default(), window, cx);
            }
            NextScrollCursorCenterTopBottom::Bottom => {
-                *new_screen_top.row_mut() = new_screen_top
-                    .row()
-                    .0
-                    .saturating_sub(visible_rows.saturating_sub(scroll_margin_rows));
+                self.scroll_cursor_bottom(&Default::default(), window, cx);
            }
        }
-        self.set_scroll_anchor(
-            ScrollAnchor {
-                anchor: snapshot
-                    .buffer_snapshot
-                    .anchor_before(new_screen_top.to_offset(&snapshot, Bias::Left)),
-                offset: Default::default(),
-            },
-            window,
-            cx,
-        );

        self.next_scroll_position = self.next_scroll_position.next();
        self._scroll_cursor_center_top_bottom_task = cx.spawn(async move |editor, cx| {
@ -95,23 +71,10 @@ impl Editor {
        window: &mut Window,
        cx: &mut Context<Editor>,
    ) {
-        let snapshot = self.snapshot(window, cx).display_snapshot;
        let scroll_margin_rows = self.vertical_scroll_margin() as u32;
-
-        let mut new_screen_top = self.selections.newest_display(cx).head();
-        *new_screen_top.row_mut() = new_screen_top.row().0.saturating_sub(scroll_margin_rows);
-        *new_screen_top.column_mut() = 0;
-        let new_screen_top = new_screen_top.to_offset(&snapshot, Bias::Left);
-        let new_anchor = snapshot.buffer_snapshot.anchor_before(new_screen_top);
-
-        self.set_scroll_anchor(
-            ScrollAnchor {
-                anchor: new_anchor,
-                offset: Default::default(),
-            },
-            window,
-            cx,
-        )
+        let new_screen_top = self.selections.newest_display(cx).head().row().0;
+        let new_screen_top = new_screen_top.saturating_sub(scroll_margin_rows);
+        self.set_scroll_top_row(DisplayRow(new_screen_top), window, cx);
    }

    pub fn scroll_cursor_center(
@ -120,27 +83,12 @@ impl Editor {
        window: &mut Window,
        cx: &mut Context<Editor>,
    ) {
-        let snapshot = self.snapshot(window, cx).display_snapshot;
-        let visible_rows = if let Some(visible_rows) = self.visible_line_count() {
-            visible_rows as u32
-        } else {
+        let Some(visible_rows) = self.visible_line_count().map(|count| count as u32) else {
            return;
        };
-
-        let mut new_screen_top = self.selections.newest_display(cx).head();
-        *new_screen_top.row_mut() = new_screen_top.row().0.saturating_sub(visible_rows / 2);
-        *new_screen_top.column_mut() = 0;
-        let new_screen_top = new_screen_top.to_offset(&snapshot, Bias::Left);
-        let new_anchor = snapshot.buffer_snapshot.anchor_before(new_screen_top);
-
-        self.set_scroll_anchor(
-            ScrollAnchor {
-                anchor: new_anchor,
-                offset: Default::default(),
-            },
-            window,
-            cx,
-        )
+        let new_screen_top = self.selections.newest_display(cx).head().row().0;
+        let new_screen_top = new_screen_top.saturating_sub(visible_rows / 2);
+        self.set_scroll_top_row(DisplayRow(new_screen_top), window, cx);
    }

    pub fn scroll_cursor_bottom(
@ -149,30 +97,13 @@ impl Editor {
        window: &mut Window,
        cx: &mut Context<Editor>,
    ) {
-        let snapshot = self.snapshot(window, cx).display_snapshot;
        let scroll_margin_rows = self.vertical_scroll_margin() as u32;
-        let visible_rows = if let Some(visible_rows) = self.visible_line_count() {
-            visible_rows as u32
-        } else {
+        let Some(visible_rows) = self.visible_line_count().map(|count| count as u32) else {
            return;
        };
-
-        let mut new_screen_top = self.selections.newest_display(cx).head();
-        *new_screen_top.row_mut() = new_screen_top
-            .row()
-            .0
-            .saturating_sub(visible_rows.saturating_sub(scroll_margin_rows));
-        *new_screen_top.column_mut() = 0;
-        let new_screen_top = new_screen_top.to_offset(&snapshot, Bias::Left);
-        let new_anchor = snapshot.buffer_snapshot.anchor_before(new_screen_top);
-
-        self.set_scroll_anchor(
-            ScrollAnchor {
-                anchor: new_anchor,
-                offset: Default::default(),
-            },
-            window,
-            cx,
-        )
+        let new_screen_top = self.selections.newest_display(cx).head().row().0;
+        let new_screen_top =
+            new_screen_top.saturating_sub(visible_rows.saturating_sub(scroll_margin_rows));
+        self.set_scroll_top_row(DisplayRow(new_screen_top), window, cx);
    }
 }
--- a/crates/editor/src/scroll/autoscroll.rs
+++ b/crates/editor/src/scroll/autoscroll.rs
@ -38,6 +38,16 @@ impl Autoscroll {
        Self::Strategy(AutoscrollStrategy::TopRelative(n))
    }

+    /// Scrolls so that the newest cursor is at the top.
+    pub fn top() -> Self {
+        Self::Strategy(AutoscrollStrategy::Top)
+    }
+
+    /// Scrolls so that the newest cursor is roughly an n-th line from the bottom.
+    pub fn bottom_relative(n: usize) -> Self {
+        Self::Strategy(AutoscrollStrategy::BottomRelative(n))
+    }
+
    /// Scrolls so that the newest cursor is at the bottom.
    pub fn bottom() -> Self {
        Self::Strategy(AutoscrollStrategy::Bottom)
@ -54,6 +64,7 @@ pub enum AutoscrollStrategy {
    Top,
    Bottom,
    TopRelative(usize),
+    BottomRelative(usize),
 }

 impl AutoscrollStrategy {
@ -213,6 +224,10 @@ impl Editor {
                scroll_position.y = target_top - lines as f32;
                self.set_scroll_position_internal(scroll_position, local, true, window, cx);
            }
+            AutoscrollStrategy::BottomRelative(lines) => {
+                scroll_position.y = target_bottom + lines as f32;
+                self.set_scroll_position_internal(scroll_position, local, true, window, cx);
+            }
        }

        self.scroll_manager.last_autoscroll = Some((