debugger: Add scrollbars, fix papercuts with completions menu and jumps (#28321)

Piotr Osiewicz created

Closes #ISSUE

Release Notes:

- N/A

Change summary

crates/debugger_ui/src/session/running.rs                  |  8 
crates/debugger_ui/src/session/running/console.rs          | 72 ++++++--
crates/debugger_ui/src/session/running/module_list.rs      | 40 ++++
crates/debugger_ui/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(-)

Detailed changes

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::<SubView>()
-                    .position(|view| view.read(cx).tab_name == *"Variables")
+                    .position(|view| view.read(cx).tab_name == *"Modules")
                     .map(|view| (view, pane))
             })
             .unwrap();

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(),
             ))

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<Session>,
     workspace: WeakEntity<Workspace>,
     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<Self>) -> Stateful<Div> {
+        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))
     }
 }

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<StackFrameEntry>,
     workspace: WeakEntity<Workspace>,
     current_stack_frame_id: Option<StackFrameId>,
+    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<Self>) -> Stateful<Div> {
+        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))
     }
 }
 

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();
     });
 

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<dap::Variable> = 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) =

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<Self>) {
-        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<Self>) -> 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::<DebugCurrentRowHighlight>();
                 self.go_to_line::<DebugCurrentRowHighlight>(
                     multibuffer_anchor,
@@ -15952,9 +15957,9 @@ impl Editor {
 
                 cx.notify();
             }
-
-            Some(())
-        });
+            handled.then_some(())
+        })
+        .is_some()
     }
 
     pub fn copy_file_name_without_extension(