From 5fb1411e4d611c15212dd87ef5094336547ebada Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Tue, 8 Apr 2025 16:55:18 +0200 Subject: [PATCH] debugger: Add scrollbars, fix papercuts with completions menu and jumps (#28321) Closes #ISSUE Release Notes: - N/A --- crates/debugger_ui/src/session/running.rs | 8 ++- .../src/session/running/console.rs | 72 ++++++++++++++----- .../src/session/running/module_list.rs | 40 ++++++++++- .../src/session/running/stack_frame_list.rs | 42 ++++++++++- crates/debugger_ui/src/tests/module_list.rs | 3 +- crates/debugger_ui/src/tests/variable_list.rs | 29 -------- crates/editor/src/editor.rs | 17 +++-- 7 files changed, 149 insertions(+), 62 deletions(-) diff --git a/crates/debugger_ui/src/session/running.rs b/crates/debugger_ui/src/session/running.rs index 6d07e01203..9fa7ae2894 100644 --- a/crates/debugger_ui/src/session/running.rs +++ b/crates/debugger_ui/src/session/running.rs @@ -242,6 +242,7 @@ fn new_debugger_pane( }))); pane.display_nav_history_buttons(None); pane.set_custom_drop_handle(cx, custom_drop_handle); + pane.set_render_tab_bar_buttons(cx, |_, _, _| (None, None)); pane }); @@ -343,12 +344,13 @@ impl RunningState { SharedString::new_static("Modules"), cx, )), - true, + false, false, None, window, cx, ); + this.activate_item(0, false, false, window, cx); }); let rightmost_pane = new_debugger_pane(workspace.clone(), project.clone(), window, cx); rightmost_pane.update(cx, |this, cx| { @@ -446,7 +448,7 @@ impl RunningState { } #[cfg(test)] - pub(crate) fn activate_variable_list(&self, window: &mut Window, cx: &mut App) { + pub(crate) fn activate_modules_list(&self, window: &mut Window, cx: &mut App) { let (variable_list_position, pane) = self .panes .panes() @@ -454,7 +456,7 @@ impl RunningState { .find_map(|pane| { pane.read(cx) .items_of_type::() - .position(|view| view.read(cx).tab_name == *"Variables") + .position(|view| view.read(cx).tab_name == *"Modules") .map(|view| (view, pane)) }) .unwrap(); diff --git a/crates/debugger_ui/src/session/running/console.rs b/crates/debugger_ui/src/session/running/console.rs index 5f248ec3c7..b112453d71 100644 --- a/crates/debugger_ui/src/session/running/console.rs +++ b/crates/debugger_ui/src/session/running/console.rs @@ -8,7 +8,7 @@ use dap::OutputEvent; use editor::{CompletionProvider, Editor, EditorElement, EditorStyle, ExcerptId}; use fuzzy::StringMatchCandidate; use gpui::{Context, Entity, Render, Subscription, Task, TextStyle, WeakEntity}; -use language::{Buffer, CodeLabel}; +use language::{Buffer, CodeLabel, ToOffset}; use menu::Confirm; use project::{ Completion, @@ -392,25 +392,61 @@ impl ConsoleQueryBarCompletionProvider { ) }) }); - + let snapshot = buffer.read(cx).text_snapshot(); cx.background_executor().spawn(async move { + let completions = completion_task.await?; + Ok(Some( - completion_task - .await? - .iter() - .map(|completion| project::Completion { - old_range: buffer_position..buffer_position, // TODO(debugger): change this - new_text: completion.text.clone().unwrap_or(completion.label.clone()), - label: CodeLabel { - filter_range: 0..completion.label.len(), - text: completion.label.clone(), - runs: Vec::new(), - }, - icon_path: None, - documentation: None, - confirm: None, - source: project::CompletionSource::Custom, - insert_text_mode: None, + completions + .into_iter() + .map(|completion| { + let new_text = completion + .text + .as_ref() + .unwrap_or(&completion.label) + .to_owned(); + let mut word_bytes_length = 0; + for chunk in snapshot + .reversed_chunks_in_range(language::Anchor::MIN..buffer_position) + { + let mut processed_bytes = 0; + if let Some(_) = chunk.chars().rfind(|c| { + let is_whitespace = c.is_whitespace(); + if !is_whitespace { + processed_bytes += c.len_utf8(); + } + + is_whitespace + }) { + word_bytes_length += processed_bytes; + break; + } else { + word_bytes_length += chunk.len(); + } + } + + let buffer_offset = buffer_position.to_offset(&snapshot); + let start = buffer_offset - word_bytes_length; + let start = snapshot.anchor_before(start); + let old_range = start..buffer_position; + + project::Completion { + old_range, + new_text, + label: CodeLabel { + filter_range: 0..completion.label.len(), + text: completion.label, + runs: Vec::new(), + }, + icon_path: None, + documentation: None, + confirm: None, + source: project::CompletionSource::BufferWord { + word_range: buffer_position..language::Anchor::MAX, + resolved: false, + }, + insert_text_mode: None, + } }) .collect(), )) diff --git a/crates/debugger_ui/src/session/running/module_list.rs b/crates/debugger_ui/src/session/running/module_list.rs index cf6ce64f62..898f8fbafb 100644 --- a/crates/debugger_ui/src/session/running/module_list.rs +++ b/crates/debugger_ui/src/session/running/module_list.rs @@ -1,13 +1,14 @@ use anyhow::anyhow; use gpui::{ - AnyElement, Empty, Entity, FocusHandle, Focusable, ListState, Subscription, WeakEntity, list, + AnyElement, Empty, Entity, FocusHandle, Focusable, ListState, MouseButton, Stateful, + Subscription, WeakEntity, list, }; use project::{ ProjectItem as _, ProjectPath, debugger::session::{Session, SessionEvent}, }; use std::{path::Path, sync::Arc}; -use ui::prelude::*; +use ui::{Scrollbar, ScrollbarState, prelude::*}; use util::maybe; use workspace::Workspace; @@ -17,6 +18,7 @@ pub struct ModuleList { session: Entity, workspace: WeakEntity, focus_handle: FocusHandle, + scrollbar_state: ScrollbarState, _subscription: Subscription, } @@ -50,6 +52,7 @@ impl ModuleList { }); Self { + scrollbar_state: ScrollbarState::new(list.clone()), list, session, workspace, @@ -153,6 +156,38 @@ impl ModuleList { self.session .update(cx, |session, cx| session.modules(cx).to_vec()) } + fn render_vertical_scrollbar(&self, cx: &mut Context) -> Stateful
{ + div() + .occlude() + .id("module-list-vertical-scrollbar") + .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(|_, _, _, cx| { + 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())) + } } impl Focusable for ModuleList { @@ -177,5 +212,6 @@ impl Render for ModuleList { .size_full() .p_1() .child(list(self.list.clone()).size_full()) + .child(self.render_vertical_scrollbar(cx)) } } diff --git a/crates/debugger_ui/src/session/running/stack_frame_list.rs b/crates/debugger_ui/src/session/running/stack_frame_list.rs index 8cecf55594..745798170d 100644 --- a/crates/debugger_ui/src/session/running/stack_frame_list.rs +++ b/crates/debugger_ui/src/session/running/stack_frame_list.rs @@ -4,14 +4,14 @@ use std::sync::Arc; use anyhow::{Result, anyhow}; use dap::StackFrameId; use gpui::{ - AnyElement, Entity, EventEmitter, FocusHandle, Focusable, ListState, Subscription, Task, - WeakEntity, list, + AnyElement, Entity, EventEmitter, FocusHandle, Focusable, ListState, MouseButton, Stateful, + Subscription, Task, WeakEntity, list, }; use language::PointUtf16; use project::debugger::session::{Session, SessionEvent, StackFrame}; use project::{ProjectItem, ProjectPath}; -use ui::{Tooltip, prelude::*}; +use ui::{Scrollbar, ScrollbarState, Tooltip, prelude::*}; use util::ResultExt; use workspace::Workspace; @@ -32,6 +32,7 @@ pub struct StackFrameList { entries: Vec, workspace: WeakEntity, current_stack_frame_id: Option, + scrollbar_state: ScrollbarState, } #[allow(clippy::large_enum_variant)] @@ -75,6 +76,7 @@ impl StackFrameList { }); Self { + scrollbar_state: ScrollbarState::new(list.clone()), list, session, workspace, @@ -493,6 +495,39 @@ impl StackFrameList { } } } + + fn render_vertical_scrollbar(&self, cx: &mut Context) -> Stateful
{ + div() + .occlude() + .id("stack-frame-list-vertical-scrollbar") + .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(|_, _, _, cx| { + 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())) + } } impl Render for StackFrameList { @@ -507,6 +542,7 @@ impl Render for StackFrameList { .size_full() .p_1() .child(list(self.list.clone()).size_full()) + .child(self.render_vertical_scrollbar(cx)) } } diff --git a/crates/debugger_ui/src/tests/module_list.rs b/crates/debugger_ui/src/tests/module_list.rs index 8f5848864e..f408850771 100644 --- a/crates/debugger_ui/src/tests/module_list.rs +++ b/crates/debugger_ui/src/tests/module_list.rs @@ -138,7 +138,8 @@ async fn test_module_list(executor: BackgroundExecutor, cx: &mut TestAppContext) .clone() }); - running_state.update(cx, |_, cx| { + running_state.update_in(cx, |this, window, cx| { + this.activate_modules_list(window, cx); cx.refresh_windows(); }); diff --git a/crates/debugger_ui/src/tests/variable_list.rs b/crates/debugger_ui/src/tests/variable_list.rs index d74fe17955..13ffd97bc4 100644 --- a/crates/debugger_ui/src/tests/variable_list.rs +++ b/crates/debugger_ui/src/tests/variable_list.rs @@ -207,9 +207,6 @@ async fn test_basic_fetch_initial_scope_and_variables( .expect("Session should be running by this point") .clone() }); - running_state.update_in(cx, |this, window, cx| { - this.activate_variable_list(window, cx); - }); cx.run_until_parked(); running_state.update(cx, |running_state, cx| { @@ -481,9 +478,6 @@ async fn test_fetch_variables_for_multiple_scopes( .expect("Session should be running by this point") .clone() }); - running_state.update_in(cx, |this, window, cx| { - this.activate_variable_list(window, cx); - }); cx.run_until_parked(); running_state.update(cx, |running_state, cx| { @@ -800,10 +794,6 @@ async fn test_keyboard_navigation(executor: BackgroundExecutor, cx: &mut TestApp variable_list.update(cx, |_, cx| cx.focus_self(window)); running }); - running_state.update_in(cx, |this, window, cx| { - this.activate_variable_list(window, cx); - }); - cx.run_until_parked(); cx.dispatch_action(SelectFirst); cx.dispatch_action(SelectFirst); cx.run_until_parked(); @@ -1572,21 +1562,6 @@ async fn test_variable_list_only_sends_requests_when_rendering( cx.run_until_parked(); - // We shouldn't make any variable requests unless we're rendering the variable list - running_state.update_in(cx, |running_state, window, cx| { - let variable_list = running_state.variable_list().read(cx); - let empty: Vec = vec![]; - - assert_eq!(empty, variable_list.variables()); - assert!(!made_scopes_request.load(Ordering::SeqCst)); - - cx.focus_self(window); - }); - running_state.update_in(cx, |this, window, cx| { - this.activate_variable_list(window, cx); - }); - cx.run_until_parked(); - running_state.update(cx, |running_state, cx| { let (stack_frame_list, stack_frame_id) = running_state.stack_frame_list().update(cx, |list, _| { @@ -1898,10 +1873,6 @@ async fn test_it_fetches_scopes_variables_when_you_select_a_stack_frame( .expect("Session should be running by this point") .clone() }); - running_state.update_in(cx, |this, window, cx| { - this.activate_variable_list(window, cx); - }); - cx.run_until_parked(); running_state.update(cx, |running_state, cx| { let (stack_frame_list, stack_frame_id) = diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 65a62a9ba3..dfc50d3ff2 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -1384,7 +1384,9 @@ impl Editor { window, |editor, _, event, window, cx| match event { BreakpointStoreEvent::ActiveDebugLineChanged => { - editor.go_to_active_debug_line(window, cx); + if editor.go_to_active_debug_line(window, cx) { + cx.stop_propagation(); + } } _ => {} }, @@ -15910,8 +15912,9 @@ impl Editor { } } - pub fn go_to_active_debug_line(&mut self, window: &mut Window, cx: &mut Context) { - let _ = maybe!({ + // Returns true if the editor handled a go-to-line request + pub fn go_to_active_debug_line(&mut self, window: &mut Window, cx: &mut Context) -> bool { + maybe!({ let breakpoint_store = self.breakpoint_store.as_ref()?; let Some((_, _, active_position)) = @@ -15929,6 +15932,7 @@ impl Editor { .read(cx) .snapshot(); + let mut handled = false; for (id, ExcerptRange { context, .. }) in self .buffer .read(cx) @@ -15942,6 +15946,7 @@ impl Editor { let snapshot = self.buffer.read(cx).snapshot(cx); let multibuffer_anchor = snapshot.anchor_in_excerpt(id, active_position)?; + handled = true; self.clear_row_highlights::(); self.go_to_line::( multibuffer_anchor, @@ -15952,9 +15957,9 @@ impl Editor { cx.notify(); } - - Some(()) - }); + handled.then_some(()) + }) + .is_some() } pub fn copy_file_name_without_extension(