From 239ffa49e1ba81f3864c02cebde94a2590362f4c Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Tue, 27 May 2025 09:50:41 -0400 Subject: [PATCH] debugger: Improve keyboard navigability of variable list (#31462) This PR adds actions for copying variable names and values and editing variable values from the variable list. Previously these were only accessible using the mouse. It also fills in keybindings for expanding and collapsing entries on Linux that we already had on macOS. Release Notes: - Debugger Beta: Added the `variable_list::EditVariable`, `variable_list::CopyVariableName`, and `variable_list::CopyVariableValue` actions and default keybindings. --- assets/keymaps/default-linux.json | 10 ++ assets/keymaps/default-macos.json | 5 +- crates/debugger_ui/src/session/running.rs | 15 +- .../src/session/running/variable_list.rs | 157 +++++++++++------- 4 files changed, 120 insertions(+), 67 deletions(-) diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index 5dad3caf4c44f96e1dfe1844e720f7d464797dd4..eab1f72ff1ccb757cff2497d3089f1bb9eacb109 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -873,6 +873,16 @@ "ctrl-i": "debugger::ToggleSessionPicker" } }, + { + "context": "VariableList", + "bindings": { + "left": "variable_list::CollapseSelectedEntry", + "right": "variable_list::ExpandSelectedEntry", + "enter": "variable_list::EditVariable", + "ctrl-c": "variable_list::CopyVariableValue", + "ctrl-alt-c": "variable_list::CopyVariableName" + } + }, { "context": "BreakpointList", "bindings": { diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index d3502baead4b1a1d002eb438b0ecee0507f23b7f..570be05a313206d0223e2f1f973f60f0a8477034 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -841,7 +841,10 @@ "use_key_equivalents": true, "bindings": { "left": "variable_list::CollapseSelectedEntry", - "right": "variable_list::ExpandSelectedEntry" + "right": "variable_list::ExpandSelectedEntry", + "enter": "variable_list::EditVariable", + "cmd-c": "variable_list::CopyVariableValue", + "cmd-alt-c": "variable_list::CopyVariableName" } }, { diff --git a/crates/debugger_ui/src/session/running.rs b/crates/debugger_ui/src/session/running.rs index bb081940ee49a04c4b1a180ed727bcb8aa14982c..8d0d6c617cf7b52d7c9eab7b8af37bd1702c4adc 100644 --- a/crates/debugger_ui/src/session/running.rs +++ b/crates/debugger_ui/src/session/running.rs @@ -496,13 +496,22 @@ pub(crate) fn new_debugger_pane( pub struct DebugTerminal { pub terminal: Option>, focus_handle: FocusHandle, + _subscriptions: [Subscription; 1], } impl DebugTerminal { - fn empty(cx: &mut Context) -> Self { + fn empty(window: &mut Window, cx: &mut Context) -> Self { + let focus_handle = cx.focus_handle(); + let focus_subscription = cx.on_focus(&focus_handle, window, |this, window, cx| { + if let Some(terminal) = this.terminal.as_ref() { + terminal.focus_handle(cx).focus(window); + } + }); + Self { terminal: None, - focus_handle: cx.focus_handle(), + focus_handle, + _subscriptions: [focus_subscription], } } } @@ -588,7 +597,7 @@ impl RunningState { StackFrameList::new(workspace.clone(), session.clone(), weak_state, window, cx) }); - let debug_terminal = cx.new(DebugTerminal::empty); + let debug_terminal = cx.new(|cx| DebugTerminal::empty(window, cx)); let variable_list = cx.new(|cx| VariableList::new(session.clone(), stack_frame_list.clone(), window, cx)); diff --git a/crates/debugger_ui/src/session/running/variable_list.rs b/crates/debugger_ui/src/session/running/variable_list.rs index 4eb8575e0076c3536f5d75ef883fa079dc11cbf5..46c6edeefc6e9551a51c125d165df90559fda3f2 100644 --- a/crates/debugger_ui/src/session/running/variable_list.rs +++ b/crates/debugger_ui/src/session/running/variable_list.rs @@ -2,17 +2,26 @@ use super::stack_frame_list::{StackFrameList, StackFrameListEvent}; use dap::{ScopePresentationHint, StackFrameId, VariablePresentationHintKind, VariableReference}; use editor::Editor; use gpui::{ - AnyElement, ClickEvent, ClipboardItem, Context, DismissEvent, Entity, FocusHandle, Focusable, - Hsla, MouseButton, MouseDownEvent, Point, Stateful, Subscription, TextStyleRefinement, - UniformListScrollHandle, actions, anchored, deferred, uniform_list, + Action, AnyElement, ClickEvent, ClipboardItem, Context, DismissEvent, Entity, FocusHandle, + Focusable, Hsla, MouseButton, MouseDownEvent, Point, Stateful, Subscription, + TextStyleRefinement, UniformListScrollHandle, actions, anchored, deferred, uniform_list, }; use menu::{SelectFirst, SelectLast, SelectNext, SelectPrevious}; use project::debugger::session::{Session, SessionEvent}; use std::{collections::HashMap, ops::Range, sync::Arc}; use ui::{ContextMenu, ListItem, Scrollbar, ScrollbarState, prelude::*}; -use util::{debug_panic, maybe}; - -actions!(variable_list, [ExpandSelectedEntry, CollapseSelectedEntry]); +use util::debug_panic; + +actions!( + variable_list, + [ + ExpandSelectedEntry, + CollapseSelectedEntry, + CopyVariableName, + CopyVariableValue, + EditVariable + ] +); #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub(crate) struct EntryState { @@ -351,7 +360,7 @@ impl VariableList { } fn select_first(&mut self, _: &SelectFirst, window: &mut Window, cx: &mut Context) { - self.cancel_variable_edit(&Default::default(), window, cx); + self.cancel(&Default::default(), window, cx); if let Some(variable) = self.entries.first() { self.selection = Some(variable.path.clone()); self.build_entries(cx); @@ -359,7 +368,7 @@ impl VariableList { } fn select_last(&mut self, _: &SelectLast, window: &mut Window, cx: &mut Context) { - self.cancel_variable_edit(&Default::default(), window, cx); + self.cancel(&Default::default(), window, cx); if let Some(variable) = self.entries.last() { self.selection = Some(variable.path.clone()); self.build_entries(cx); @@ -367,7 +376,7 @@ impl VariableList { } fn select_prev(&mut self, _: &SelectPrevious, window: &mut Window, cx: &mut Context) { - self.cancel_variable_edit(&Default::default(), window, cx); + self.cancel(&Default::default(), window, cx); if let Some(selection) = &self.selection { let index = self.entries.iter().enumerate().find_map(|(ix, var)| { if &var.path == selection && ix > 0 { @@ -391,7 +400,7 @@ impl VariableList { } fn select_next(&mut self, _: &SelectNext, window: &mut Window, cx: &mut Context) { - self.cancel_variable_edit(&Default::default(), window, cx); + self.cancel(&Default::default(), window, cx); if let Some(selection) = &self.selection { let index = self.entries.iter().enumerate().find_map(|(ix, var)| { if &var.path == selection { @@ -414,40 +423,26 @@ impl VariableList { } } - fn cancel_variable_edit( - &mut self, - _: &menu::Cancel, - window: &mut Window, - cx: &mut Context, - ) { + fn cancel(&mut self, _: &menu::Cancel, window: &mut Window, cx: &mut Context) { self.edited_path.take(); self.focus_handle.focus(window); cx.notify(); } - fn confirm_variable_edit( - &mut self, - _: &menu::Confirm, - _window: &mut Window, - cx: &mut Context, - ) { - let res = maybe!({ - let (var_path, editor) = self.edited_path.take()?; - let state = self.entry_states.get(&var_path)?; + fn confirm(&mut self, _: &menu::Confirm, _window: &mut Window, cx: &mut Context) { + if let Some((var_path, editor)) = self.edited_path.take() { + let Some(state) = self.entry_states.get(&var_path) else { + return; + }; let variables_reference = state.parent_reference; - let name = var_path.leaf_name?; + let Some(name) = var_path.leaf_name else { + return; + }; let value = editor.read(cx).text(cx); self.session.update(cx, |session, cx| { session.set_variable_value(variables_reference, name.into(), value, cx) }); - Some(()) - }); - - if res.is_none() { - log::error!( - "Couldn't confirm variable edit because variable doesn't have a leaf name or a parent reference id" - ); } } @@ -495,38 +490,16 @@ impl VariableList { fn deploy_variable_context_menu( &mut self, - variable: ListEntry, + _variable: ListEntry, position: Point, window: &mut Window, cx: &mut Context, ) { - let Some(dap_var) = variable.as_variable() else { - debug_panic!("Trying to open variable context menu on a scope"); - return; - }; - - let variable_value = dap_var.value.clone(); - let variable_name = dap_var.name.clone(); - let this = cx.entity().clone(); - let context_menu = ContextMenu::build(window, cx, |menu, _, _| { - menu.entry("Copy name", None, move |_, cx| { - cx.write_to_clipboard(ClipboardItem::new_string(variable_name.clone())) - }) - .entry("Copy value", None, { - let variable_value = variable_value.clone(); - move |_, cx| { - cx.write_to_clipboard(ClipboardItem::new_string(variable_value.clone())) - } - }) - .entry("Set value", None, move |window, cx| { - this.update(cx, |variable_list, cx| { - let editor = Self::create_variable_editor(&variable_value, window, cx); - variable_list.edited_path = Some((variable.path.clone(), editor)); - - cx.notify(); - }); - }) + menu.action("Copy Name", CopyVariableName.boxed_clone()) + .action("Copy Value", CopyVariableValue.boxed_clone()) + .action("Edit Value", EditVariable.boxed_clone()) + .context(self.focus_handle.clone()) }); cx.focus_view(&context_menu, window); @@ -547,6 +520,59 @@ impl VariableList { self.open_context_menu = Some((context_menu, position, subscription)); } + fn copy_variable_name( + &mut self, + _: &CopyVariableName, + _window: &mut Window, + cx: &mut Context, + ) { + let Some(selection) = self.selection.as_ref() else { + return; + }; + let Some(entry) = self.entries.iter().find(|entry| &entry.path == selection) else { + return; + }; + let Some(variable) = entry.as_variable() else { + return; + }; + cx.write_to_clipboard(ClipboardItem::new_string(variable.name.clone())); + } + + fn copy_variable_value( + &mut self, + _: &CopyVariableValue, + _window: &mut Window, + cx: &mut Context, + ) { + let Some(selection) = self.selection.as_ref() else { + return; + }; + let Some(entry) = self.entries.iter().find(|entry| &entry.path == selection) else { + return; + }; + let Some(variable) = entry.as_variable() else { + return; + }; + cx.write_to_clipboard(ClipboardItem::new_string(variable.value.clone())); + } + + fn edit_variable(&mut self, _: &EditVariable, window: &mut Window, cx: &mut Context) { + let Some(selection) = self.selection.as_ref() else { + return; + }; + let Some(entry) = self.entries.iter().find(|entry| &entry.path == selection) else { + return; + }; + let Some(variable) = entry.as_variable() else { + return; + }; + + let editor = Self::create_variable_editor(&variable.value, window, cx); + self.edited_path = Some((entry.path.clone(), editor)); + + cx.notify(); + } + #[track_caller] #[cfg(test)] pub(crate) fn assert_visual_entries(&self, expected: Vec<&str>) { @@ -815,12 +841,14 @@ impl VariableList { .on_secondary_mouse_down(cx.listener({ let variable = variable.clone(); move |this, event: &MouseDownEvent, window, cx| { + this.selection = Some(variable.path.clone()); this.deploy_variable_context_menu( variable.clone(), event.position, window, cx, - ) + ); + cx.stop_propagation(); } })) .child( @@ -943,10 +971,13 @@ impl Render for VariableList { .on_action(cx.listener(Self::select_last)) .on_action(cx.listener(Self::select_prev)) .on_action(cx.listener(Self::select_next)) + .on_action(cx.listener(Self::cancel)) + .on_action(cx.listener(Self::confirm)) .on_action(cx.listener(Self::expand_selected_entry)) .on_action(cx.listener(Self::collapse_selected_entry)) - .on_action(cx.listener(Self::cancel_variable_edit)) - .on_action(cx.listener(Self::confirm_variable_edit)) + .on_action(cx.listener(Self::copy_variable_name)) + .on_action(cx.listener(Self::copy_variable_value)) + .on_action(cx.listener(Self::edit_variable)) .child( uniform_list( cx.entity().clone(),