From bbbd1e9902e1a341ca5583324994b6fdaefb679b Mon Sep 17 00:00:00 2001 From: Michael Sloan Date: Fri, 10 Jan 2025 21:48:44 -0700 Subject: [PATCH 01/10] LSP debug logs: Default to soft wrap + fold long lines + autoscroll (#22996) Closes #18737 Release notes: - Improved LSP debug logs by defaulting to soft wrap and folding a suffix of long lines. Also adds autoscroll, so if the cursor is on the last line of the logs they will scroll like `tail`. --- crates/editor/src/editor.rs | 14 ++++++++++++++ crates/language_tools/src/lsp_log.rs | 15 +++++++++++++-- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 6d87575e96ecc3..4334a34bf088f0 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -10901,6 +10901,20 @@ impl Editor { self.fold_creases(ranges, true, cx); } + pub fn fold_ranges( + &mut self, + ranges: Vec>, + auto_scroll: bool, + cx: &mut ViewContext, + ) { + let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx)); + let ranges = ranges + .into_iter() + .map(|r| Crease::simple(r, display_map.fold_placeholder.clone())) + .collect::>(); + self.fold_creases(ranges, auto_scroll, cx); + } + pub fn fold_creases( &mut self, creases: Vec>, diff --git a/crates/language_tools/src/lsp_log.rs b/crates/language_tools/src/lsp_log.rs index b4883943335e77..66c6fb14d475e4 100644 --- a/crates/language_tools/src/lsp_log.rs +++ b/crates/language_tools/src/lsp_log.rs @@ -1,13 +1,13 @@ use collections::{HashMap, VecDeque}; use copilot::Copilot; -use editor::{actions::MoveToEnd, Editor, EditorEvent}; +use editor::{actions::MoveToEnd, scroll::Autoscroll, Editor, EditorEvent}; use futures::{channel::mpsc, StreamExt}; use gpui::{ actions, div, AppContext, Context, Corner, EventEmitter, FocusHandle, FocusableView, IntoElement, Model, ModelContext, ParentElement, Render, Styled, Subscription, View, ViewContext, VisualContext, WeakModel, WindowContext, }; -use language::LanguageServerId; +use language::{language_settings::SoftWrap, LanguageServerId}; use lsp::{ notification::SetTrace, IoKind, LanguageServer, LanguageServerName, MessageType, SetTraceParams, TraceValue, @@ -649,6 +649,15 @@ impl LspLogView { ], cx, ); + let entry_length = entry.len(); + if entry_length > 1024 { + editor.fold_ranges( + vec![last_point + 1024..last_point + entry_length], + false, + cx, + ); + } + editor.request_autoscroll(Autoscroll::fit(), cx); editor.set_read_only(true); }); } @@ -691,6 +700,7 @@ impl LspLogView { editor.move_to_end(&MoveToEnd, cx); editor.set_read_only(true); editor.set_show_inline_completions(Some(false), cx); + editor.set_soft_wrap_mode(SoftWrap::EditorWidth, cx); editor }); let editor_subscription = cx.subscribe( @@ -728,6 +738,7 @@ impl LspLogView { editor.set_text(server_info, cx); editor.set_read_only(true); editor.set_show_inline_completions(Some(false), cx); + editor.set_soft_wrap_mode(SoftWrap::EditorWidth, cx); editor }); let editor_subscription = cx.subscribe( From b65dc8c5666f563057164aaf01f796eaaa43ef3e Mon Sep 17 00:00:00 2001 From: Michael Sloan Date: Fri, 10 Jan 2025 22:59:21 -0700 Subject: [PATCH 02/10] Fix jank in LSP debug log autoscroll (#22998) Not sure why scroll was janky with `Autoscroll::newest()`, but this appears to fix it. Probably better to conditionally do the autoscroll requests anyway. Release Notes: - N/A --- crates/editor/src/scroll/autoscroll.rs | 6 ++++++ crates/language_tools/src/lsp_log.rs | 7 ++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/crates/editor/src/scroll/autoscroll.rs b/crates/editor/src/scroll/autoscroll.rs index a69b4a11b74539..8ae5dea7205c2a 100644 --- a/crates/editor/src/scroll/autoscroll.rs +++ b/crates/editor/src/scroll/autoscroll.rs @@ -32,10 +32,16 @@ impl Autoscroll { pub fn focused() -> Self { Self::Strategy(AutoscrollStrategy::Focused) } + /// Scrolls so that the newest cursor is roughly an n-th line from the top. pub fn top_relative(n: usize) -> Self { Self::Strategy(AutoscrollStrategy::TopRelative(n)) } + + /// Scrolls so that the newest cursor is at the bottom. + pub fn bottom() -> Self { + Self::Strategy(AutoscrollStrategy::Bottom) + } } #[derive(PartialEq, Eq, Default, Clone, Copy)] diff --git a/crates/language_tools/src/lsp_log.rs b/crates/language_tools/src/lsp_log.rs index 66c6fb14d475e4..01ef202227d63c 100644 --- a/crates/language_tools/src/lsp_log.rs +++ b/crates/language_tools/src/lsp_log.rs @@ -642,6 +642,8 @@ impl LspLogView { log_view.editor.update(cx, |editor, cx| { editor.set_read_only(false); let last_point = editor.buffer().read(cx).len(cx); + let newest_cursor_is_at_end = + editor.selections.newest::(cx).start >= last_point; editor.edit( vec![ (last_point..last_point, entry.trim()), @@ -657,7 +659,10 @@ impl LspLogView { cx, ); } - editor.request_autoscroll(Autoscroll::fit(), cx); + + if newest_cursor_is_at_end { + editor.request_autoscroll(Autoscroll::bottom(), cx); + } editor.set_read_only(true); }); } From 14a2f7526b1dafa295204f899a2f629a49d5fad7 Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Sat, 11 Jan 2025 15:47:41 +0100 Subject: [PATCH 03/10] Variable list keyboard navigation (#84) * WIP * Add assert helper for variable list visual entries * Wip rework toggle entry (scope/variable) code * Remove commented code * Move colors to struct * Add entry to selection if you click on them * Add selected option to visual entries assert * Use pretty assertions for visual entries assert helper * Use focus handle method to give focus handle * Register select first and last actions * Correctly format selected entry * Add tests helper to get active debug panel item * Add tests for keyboard navigation * Remove not needed comment * Move other tests to test helper This also removes a test that is duplicated with the keyboard navigation tests, it covers the same * Update console test to use new test helper * Fix failing test I forgot to update the test, because we now always send a body back in a error case. * Fix clippyyyy --- Cargo.lock | 1 + assets/keymaps/default-macos.json | 8 + crates/debugger_ui/Cargo.toml | 3 +- crates/debugger_ui/src/debugger_panel_item.rs | 2 +- crates/debugger_ui/src/tests.rs | 18 +- crates/debugger_ui/src/tests/console.rs | 284 +++-- .../debugger_ui/src/tests/debugger_panel.rs | 2 +- crates/debugger_ui/src/tests/variable_list.rs | 971 ++++++++---------- crates/debugger_ui/src/variable_list.rs | 492 +++++++-- crates/proto/proto/zed.proto | 5 +- 10 files changed, 989 insertions(+), 797 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 62819fea3e7e83..763e9d0d157054 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3651,6 +3651,7 @@ dependencies = [ "language", "menu", "picker", + "pretty_assertions", "project", "rpc", "serde", diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index ac4b27f2b02b8a..655063e5504653 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -682,6 +682,14 @@ "space": "project_panel::Open" } }, + { + "context": "VariableList", + "use_key_equivalents": true, + "bindings": { + "left": "variable_list::CollapseSelectedEntry", + "right": "variable_list::ExpandSelectedEntry" + } + }, { "context": "CollabPanel && not_editing", "use_key_equivalents": true, diff --git a/crates/debugger_ui/Cargo.toml b/crates/debugger_ui/Cargo.toml index 5634651b602f39..d3ad8d5827fbd2 100644 --- a/crates/debugger_ui/Cargo.toml +++ b/crates/debugger_ui/Cargo.toml @@ -31,6 +31,7 @@ gpui.workspace = true language.workspace = true menu.workspace = true picker.workspace = true +pretty_assertions.workspace = true project.workspace = true rpc.workspace = true serde.workspace = true @@ -52,6 +53,6 @@ editor = { workspace = true, features = ["test-support"] } env_logger.workspace = true gpui = { workspace = true, features = ["test-support"] } project = { workspace = true, features = ["test-support"] } -util = { workspace = true, features = ["test-support"] } unindent.workspace = true +util = { workspace = true, features = ["test-support"] } workspace = { workspace = true, features = ["test-support"] } diff --git a/crates/debugger_ui/src/debugger_panel_item.rs b/crates/debugger_ui/src/debugger_panel_item.rs index 16d9b186036257..a6b63b58f24c55 100644 --- a/crates/debugger_ui/src/debugger_panel_item.rs +++ b/crates/debugger_ui/src/debugger_panel_item.rs @@ -863,7 +863,7 @@ impl Render for DebugPanelItem { h_flex() .key_context("DebugPanelItem") - .track_focus(&self.focus_handle) + .track_focus(&self.focus_handle(cx)) .size_full() .items_start() .child( diff --git a/crates/debugger_ui/src/tests.rs b/crates/debugger_ui/src/tests.rs index 2561fbf0b484bb..2ab4e9740da399 100644 --- a/crates/debugger_ui/src/tests.rs +++ b/crates/debugger_ui/src/tests.rs @@ -1,10 +1,10 @@ -use gpui::{Model, TestAppContext, WindowHandle}; +use gpui::{Model, TestAppContext, View, WindowHandle}; use project::Project; use settings::SettingsStore; use terminal_view::terminal_panel::TerminalPanel; use workspace::Workspace; -use crate::debugger_panel::DebugPanel; +use crate::{debugger_panel::DebugPanel, debugger_panel_item::DebugPanelItem}; mod attach_modal; mod console; @@ -57,3 +57,17 @@ pub async fn init_test_workspace( .unwrap(); window } + +pub fn active_debug_panel_item( + workspace: WindowHandle, + cx: &mut TestAppContext, +) -> View { + workspace + .update(cx, |workspace, cx| { + let debug_panel = workspace.panel::(cx).unwrap(); + debug_panel + .update(cx, |this, cx| this.active_debug_panel_item(cx)) + .unwrap() + }) + .unwrap() +} diff --git a/crates/debugger_ui/src/tests/console.rs b/crates/debugger_ui/src/tests/console.rs index 510b531234a068..0dd5a00c272f30 100644 --- a/crates/debugger_ui/src/tests/console.rs +++ b/crates/debugger_ui/src/tests/console.rs @@ -10,7 +10,7 @@ use std::sync::{ atomic::{AtomicBool, Ordering}, Arc, Mutex, }; -use tests::{init_test, init_test_workspace}; +use tests::{active_debug_panel_item, init_test, init_test_workspace}; use unindent::Unindent as _; use variable_list::{VariableContainer, VariableListEntry}; @@ -281,172 +281,140 @@ async fn test_evaluate_expression(executor: BackgroundExecutor, cx: &mut TestApp cx.run_until_parked(); // toggle nested variables for scope 1 - workspace - .update(cx, |workspace, cx| { - let debug_panel = workspace.panel::(cx).unwrap(); - let active_debug_panel_item = debug_panel - .update(cx, |this, cx| this.active_debug_panel_item(cx)) - .unwrap(); - - active_debug_panel_item.update(cx, |debug_panel_item, cx| { - let scope1_variables = scope1_variables.lock().unwrap().clone(); - - debug_panel_item - .variable_list() - .update(cx, |variable_list, cx| { - variable_list.on_toggle_variable( - scopes[0].variables_reference, // scope id - &crate::variable_list::OpenEntry::Variable { - name: scope1_variables[0].name.clone(), - depth: 1, - }, - scope1_variables[0].variables_reference, - 1, // depth - Some(false), - cx, - ); - }); + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { + let scope1_variables = scope1_variables.lock().unwrap().clone(); + + debug_panel_item + .variable_list() + .update(cx, |variable_list, cx| { + variable_list.toggle_entry( + &variable_list::OpenEntry::Variable { + scope_id: scopes[0].variables_reference, + name: scope1_variables[0].name.clone(), + depth: 1, + }, + cx, + ); }); - }) - .unwrap(); + }); cx.run_until_parked(); - workspace - .update(cx, |workspace, cx| { - let debug_panel = workspace.panel::(cx).unwrap(); - let active_debug_panel_item = debug_panel - .update(cx, |this, cx| this.active_debug_panel_item(cx)) - .unwrap(); - - active_debug_panel_item.update(cx, |item, cx| { - item.console().update(cx, |console, cx| { - console.query_bar().update(cx, |query_bar, cx| { - query_bar.set_text(format!("$variable1 = {}", NEW_VALUE), cx); - }); - - console.evaluate(&menu::Confirm, cx); - }); + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { + debug_panel_item.console().update(cx, |console, cx| { + console.query_bar().update(cx, |query_bar, cx| { + query_bar.set_text(format!("$variable1 = {}", NEW_VALUE), cx); }); - }) - .unwrap(); + + console.evaluate(&menu::Confirm, cx); + }); + }); cx.run_until_parked(); - workspace - .update(cx, |workspace, cx| { - let debug_panel = workspace.panel::(cx).unwrap(); - let active_debug_panel_item = debug_panel - .update(cx, |this, cx| this.active_debug_panel_item(cx)) - .unwrap(); - - assert_eq!( - "", - active_debug_panel_item - .read(cx) - .console() - .read(cx) - .query_bar() - .read(cx) - .text(cx) - .as_str() - ); - - assert_eq!( - format!("{}\n", NEW_VALUE), - active_debug_panel_item - .read(cx) - .console() - .read(cx) - .editor() - .read(cx) - .text(cx) - .as_str() - ); - - active_debug_panel_item.update(cx, |debug_panel_item, cx| { - debug_panel_item - .variable_list() - .update(cx, |variable_list, _| { - let scope1_variables = scope1_variables.lock().unwrap().clone(); - - // scope 1 - assert_eq!( - vec![ - VariableContainer { - container_reference: scopes[0].variables_reference, - variable: scope1_variables[0].clone(), - depth: 1, - }, - VariableContainer { - container_reference: scope1_variables[0].variables_reference, - variable: nested_variables[0].clone(), - depth: 2, - }, - VariableContainer { - container_reference: scope1_variables[0].variables_reference, - variable: nested_variables[1].clone(), - depth: 2, - }, - VariableContainer { - container_reference: scopes[0].variables_reference, - variable: scope1_variables[1].clone(), - depth: 1, - }, - ], - variable_list.variables_by_scope(1, 2).unwrap().variables() - ); - - // scope 2 - assert_eq!( - vec![VariableContainer { - container_reference: scopes[1].variables_reference, - variable: scope2_variables[0].clone(), - depth: 1, - }], - variable_list.variables_by_scope(1, 4).unwrap().variables() - ); - - // assert visual entries - assert_eq!( - vec![ - VariableListEntry::Scope(scopes[0].clone()), - VariableListEntry::Variable { - depth: 1, - scope: Arc::new(scopes[0].clone()), - has_children: true, - variable: Arc::new(scope1_variables[0].clone()), - container_reference: scopes[0].variables_reference, - }, - VariableListEntry::Variable { - depth: 2, - scope: Arc::new(scopes[0].clone()), - has_children: false, - variable: Arc::new(nested_variables[0].clone()), - container_reference: scope1_variables[0].variables_reference, - }, - VariableListEntry::Variable { - depth: 2, - scope: Arc::new(scopes[0].clone()), - has_children: false, - variable: Arc::new(nested_variables[1].clone()), - container_reference: scope1_variables[0].variables_reference, - }, - VariableListEntry::Variable { - depth: 1, - scope: Arc::new(scopes[0].clone()), - has_children: false, - variable: Arc::new(scope1_variables[1].clone()), - container_reference: scopes[0].variables_reference, - }, - VariableListEntry::Scope(scopes[1].clone()), - ], - variable_list.entries().get(&1).unwrap().clone() - ); - }); + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { + assert_eq!( + "", + debug_panel_item + .console() + .read(cx) + .query_bar() + .read(cx) + .text(cx) + .as_str() + ); + + assert_eq!( + format!("{}\n", NEW_VALUE), + debug_panel_item + .console() + .read(cx) + .editor() + .read(cx) + .text(cx) + .as_str() + ); + + debug_panel_item + .variable_list() + .update(cx, |variable_list, _| { + let scope1_variables = scope1_variables.lock().unwrap().clone(); + + // scope 1 + assert_eq!( + vec![ + VariableContainer { + container_reference: scopes[0].variables_reference, + variable: scope1_variables[0].clone(), + depth: 1, + }, + VariableContainer { + container_reference: scope1_variables[0].variables_reference, + variable: nested_variables[0].clone(), + depth: 2, + }, + VariableContainer { + container_reference: scope1_variables[0].variables_reference, + variable: nested_variables[1].clone(), + depth: 2, + }, + VariableContainer { + container_reference: scopes[0].variables_reference, + variable: scope1_variables[1].clone(), + depth: 1, + }, + ], + variable_list.variables_by_scope(1, 2).unwrap().variables() + ); + + // scope 2 + assert_eq!( + vec![VariableContainer { + container_reference: scopes[1].variables_reference, + variable: scope2_variables[0].clone(), + depth: 1, + }], + variable_list.variables_by_scope(1, 4).unwrap().variables() + ); + + // assert visual entries + assert_eq!( + vec![ + VariableListEntry::Scope(scopes[0].clone()), + VariableListEntry::Variable { + depth: 1, + scope: Arc::new(scopes[0].clone()), + has_children: true, + variable: Arc::new(scope1_variables[0].clone()), + container_reference: scopes[0].variables_reference, + }, + VariableListEntry::Variable { + depth: 2, + scope: Arc::new(scopes[0].clone()), + has_children: false, + variable: Arc::new(nested_variables[0].clone()), + container_reference: scope1_variables[0].variables_reference, + }, + VariableListEntry::Variable { + depth: 2, + scope: Arc::new(scopes[0].clone()), + has_children: false, + variable: Arc::new(nested_variables[1].clone()), + container_reference: scope1_variables[0].variables_reference, + }, + VariableListEntry::Variable { + depth: 1, + scope: Arc::new(scopes[0].clone()), + has_children: false, + variable: Arc::new(scope1_variables[1].clone()), + container_reference: scopes[0].variables_reference, + }, + VariableListEntry::Scope(scopes[1].clone()), + ], + variable_list.entries().get(&1).unwrap().clone() + ); }); - }) - .unwrap(); + }); assert!( called_evaluate.load(std::sync::atomic::Ordering::SeqCst), diff --git a/crates/debugger_ui/src/tests/debugger_panel.rs b/crates/debugger_ui/src/tests/debugger_panel.rs index fd5ca5b740e3d4..1b3e27c3e3e8a4 100644 --- a/crates/debugger_ui/src/tests/debugger_panel.rs +++ b/crates/debugger_ui/src/tests/debugger_panel.rs @@ -744,7 +744,7 @@ async fn test_handle_error_run_in_terminal_reverse_request( send_response.store(true, Ordering::SeqCst); assert!(!response.success); - assert!(response.body.is_none()); + assert!(response.body.is_some()); } }) .await; diff --git a/crates/debugger_ui/src/tests/variable_list.rs b/crates/debugger_ui/src/tests/variable_list.rs index f4bc90244dc1f5..6c75bd76eae20d 100644 --- a/crates/debugger_ui/src/tests/variable_list.rs +++ b/crates/debugger_ui/src/tests/variable_list.rs @@ -1,9 +1,8 @@ use std::sync::Arc; use crate::{ - debugger_panel::DebugPanel, - tests::{init_test, init_test_workspace}, - variable_list::{VariableContainer, VariableListEntry}, + tests::{active_debug_panel_item, init_test, init_test_workspace}, + variable_list::{CollapseSelectedEntry, ExpandSelectedEntry, VariableContainer}, }; use collections::HashMap; use dap::{ @@ -11,6 +10,7 @@ use dap::{ Scope, StackFrame, Variable, }; use gpui::{BackgroundExecutor, TestAppContext, VisualTestContext}; +use menu::{SelectFirst, SelectNext}; use project::{FakeFs, Project}; use serde_json::json; use unindent::Unindent as _; @@ -192,65 +192,39 @@ async fn test_basic_fetch_initial_scope_and_variables( cx.run_until_parked(); - workspace - .update(cx, |workspace, cx| { - let debug_panel = workspace.panel::(cx).unwrap(); - let active_debug_panel_item = debug_panel - .update(cx, |this, cx| this.active_debug_panel_item(cx)) - .unwrap(); - - active_debug_panel_item.update(cx, |debug_panel_item, cx| { - let stack_frame_list = debug_panel_item.stack_frame_list().read(cx); - - assert_eq!(1, stack_frame_list.current_stack_frame_id()); - assert_eq!(stack_frames, stack_frame_list.stack_frames().clone()); - - debug_panel_item - .variable_list() - .update(cx, |variable_list, cx| { - assert_eq!(1, variable_list.scopes().len()); - assert_eq!(scopes, variable_list.scopes().get(&1).unwrap().clone()); - assert_eq!( - vec![ - VariableContainer { - container_reference: scopes[0].variables_reference, - variable: variables[0].clone(), - depth: 1, - }, - VariableContainer { - container_reference: scopes[0].variables_reference, - variable: variables[1].clone(), - depth: 1, - }, - ], - variable_list.variables(cx) - ); - - // assert visual entries - assert_eq!( - vec![ - VariableListEntry::Scope(scopes[0].clone()), - VariableListEntry::Variable { - depth: 1, - scope: Arc::new(scopes[0].clone()), - has_children: false, - variable: Arc::new(variables[0].clone()), - container_reference: scopes[0].variables_reference, - }, - VariableListEntry::Variable { - depth: 1, - scope: Arc::new(scopes[0].clone()), - has_children: false, - variable: Arc::new(variables[1].clone()), - container_reference: scopes[0].variables_reference, - }, - ], - variable_list.entries().get(&1).unwrap().clone() - ); - }); + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { + let stack_frame_list = debug_panel_item.stack_frame_list().read(cx); + + assert_eq!(1, stack_frame_list.current_stack_frame_id()); + assert_eq!(stack_frames, stack_frame_list.stack_frames().clone()); + + debug_panel_item + .variable_list() + .update(cx, |variable_list, cx| { + assert_eq!(1, variable_list.scopes().len()); + assert_eq!(scopes, variable_list.scopes().get(&1).unwrap().clone()); + assert_eq!( + vec![ + VariableContainer { + container_reference: scopes[0].variables_reference, + variable: variables[0].clone(), + depth: 1, + }, + VariableContainer { + container_reference: scopes[0].variables_reference, + variable: variables[1].clone(), + depth: 1, + }, + ], + variable_list.variables(cx) + ); + + variable_list.assert_visual_entries( + vec!["v Scope 1", " > variable1", " > variable2"], + cx, + ); }); - }) - .unwrap(); + }); let shutdown_session = project.update(cx, |project, cx| { project.dap_store().update(cx, |dap_store, cx| { @@ -473,84 +447,56 @@ async fn test_fetch_variables_for_multiple_scopes( cx.run_until_parked(); - workspace - .update(cx, |workspace, cx| { - let debug_panel = workspace.panel::(cx).unwrap(); - let active_debug_panel_item = debug_panel - .update(cx, |this, cx| this.active_debug_panel_item(cx)) - .unwrap(); - - active_debug_panel_item.update(cx, |debug_panel_item, cx| { - let stack_frame_list = debug_panel_item.stack_frame_list().read(cx); - - assert_eq!(1, stack_frame_list.current_stack_frame_id()); - assert_eq!(stack_frames, stack_frame_list.stack_frames().clone()); - - debug_panel_item - .variable_list() - .update(cx, |variable_list, _| { - assert_eq!(1, variable_list.scopes().len()); - assert_eq!(scopes, variable_list.scopes().get(&1).unwrap().clone()); - - // scope 1 - assert_eq!( - vec![ - VariableContainer { - container_reference: scopes[0].variables_reference, - variable: variables.get(&2).unwrap()[0].clone(), - depth: 1, - }, - VariableContainer { - container_reference: scopes[0].variables_reference, - variable: variables.get(&2).unwrap()[1].clone(), - depth: 1, - }, - ], - variable_list.variables_by_scope(1, 2).unwrap().variables() - ); - - // scope 2 - assert_eq!( - vec![VariableContainer { - container_reference: scopes[1].variables_reference, - variable: variables.get(&3).unwrap()[0].clone(), - depth: 1, - }], - variable_list.variables_by_scope(1, 3).unwrap().variables() - ); - - // assert visual entries - assert_eq!( - vec![ - VariableListEntry::Scope(scopes[0].clone()), - VariableListEntry::Variable { - depth: 1, - scope: Arc::new(scopes[0].clone()), - has_children: false, - variable: Arc::new( - variables.get(&scopes[0].variables_reference).unwrap()[0] - .clone() - ), - container_reference: scopes[0].variables_reference, - }, - VariableListEntry::Variable { - depth: 1, - scope: Arc::new(scopes[0].clone()), - has_children: false, - variable: Arc::new( - variables.get(&scopes[0].variables_reference).unwrap()[1] - .clone() - ), - container_reference: scopes[0].variables_reference, - }, - VariableListEntry::Scope(scopes[1].clone()), - ], - variable_list.entries().get(&1).unwrap().clone() - ); - }); + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { + let stack_frame_list = debug_panel_item.stack_frame_list().read(cx); + + assert_eq!(1, stack_frame_list.current_stack_frame_id()); + assert_eq!(stack_frames, stack_frame_list.stack_frames().clone()); + + debug_panel_item + .variable_list() + .update(cx, |variable_list, cx| { + assert_eq!(1, variable_list.scopes().len()); + assert_eq!(scopes, variable_list.scopes().get(&1).unwrap().clone()); + + // scope 1 + assert_eq!( + vec![ + VariableContainer { + container_reference: scopes[0].variables_reference, + variable: variables.get(&2).unwrap()[0].clone(), + depth: 1, + }, + VariableContainer { + container_reference: scopes[0].variables_reference, + variable: variables.get(&2).unwrap()[1].clone(), + depth: 1, + }, + ], + variable_list.variables_by_scope(1, 2).unwrap().variables() + ); + + // scope 2 + assert_eq!( + vec![VariableContainer { + container_reference: scopes[1].variables_reference, + variable: variables.get(&3).unwrap()[0].clone(), + depth: 1, + }], + variable_list.variables_by_scope(1, 3).unwrap().variables() + ); + + variable_list.assert_visual_entries( + vec![ + "v Scope 1", + " > variable1", + " > variable2", + "> Scope 2", + ], + cx, + ); }); - }) - .unwrap(); + }); let shutdown_session = project.update(cx, |project, cx| { project.dap_store().update(cx, |dap_store, cx| { @@ -563,7 +509,7 @@ async fn test_fetch_variables_for_multiple_scopes( // tests that toggling a variable will fetch its children and shows it #[gpui::test] -async fn test_toggle_scope_and_variable(executor: BackgroundExecutor, cx: &mut TestAppContext) { +async fn test_keyboard_navigation(executor: BackgroundExecutor, cx: &mut TestAppContext) { init_test(cx); let fs = FakeFs::new(executor.clone()); @@ -798,418 +744,373 @@ async fn test_toggle_scope_and_variable(executor: BackgroundExecutor, cx: &mut T cx.run_until_parked(); - workspace - .update(cx, |workspace, cx| { - let debug_panel = workspace.panel::(cx).unwrap(); - let active_debug_panel_item = debug_panel - .update(cx, |this, cx| this.active_debug_panel_item(cx)) - .unwrap(); - - active_debug_panel_item.update(cx, |debug_panel_item, cx| { - debug_panel_item - .variable_list() - .update(cx, |variable_list, _| { - // scope 1 - assert_eq!( - vec![ - VariableContainer { - container_reference: scopes[0].variables_reference, - variable: scope1_variables[0].clone(), - depth: 1, - }, - VariableContainer { - container_reference: scopes[0].variables_reference, - variable: scope1_variables[1].clone(), - depth: 1, - }, - ], - variable_list.variables_by_scope(1, 2).unwrap().variables() - ); - - // scope 2 - assert_eq!( - vec![VariableContainer { - container_reference: scopes[1].variables_reference, - variable: scope2_variables[0].clone(), - depth: 1, - }], - variable_list.variables_by_scope(1, 4).unwrap().variables() - ); - - // assert visual entries - assert_eq!( - vec![ - VariableListEntry::Scope(scopes[0].clone()), - VariableListEntry::Variable { - depth: 1, - scope: Arc::new(scopes[0].clone()), - has_children: true, - variable: Arc::new(scope1_variables[0].clone()), - container_reference: scopes[0].variables_reference, - }, - VariableListEntry::Variable { - depth: 1, - scope: Arc::new(scopes[0].clone()), - has_children: false, - variable: Arc::new(scope1_variables[1].clone()), - container_reference: scopes[0].variables_reference, - }, - VariableListEntry::Scope(scopes[1].clone()), - ], - variable_list.entries().get(&1).unwrap().clone() - ); - }); + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { + debug_panel_item.variable_list().focus_handle(cx).focus(cx); + }); + + cx.dispatch_action(SelectFirst); + cx.run_until_parked(); + + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { + debug_panel_item + .variable_list() + .update(cx, |variable_list, cx| { + variable_list.assert_visual_entries( + vec![ + "v Scope 1 <=== selected", + " > variable1", + " > variable2", + "> Scope 2", + ], + cx, + ); }); - }) - .unwrap(); - - // toggle nested variables for scope 1 - workspace - .update(cx, |workspace, cx| { - let debug_panel = workspace.panel::(cx).unwrap(); - let active_debug_panel_item = debug_panel - .update(cx, |this, cx| this.active_debug_panel_item(cx)) - .unwrap(); - - active_debug_panel_item.update(cx, |debug_panel_item, cx| { - debug_panel_item - .variable_list() - .update(cx, |variable_list, cx| { - variable_list.on_toggle_variable( - scopes[0].variables_reference, // scope id - &crate::variable_list::OpenEntry::Variable { - name: scope1_variables[0].name.clone(), - depth: 1, - }, - scope1_variables[0].variables_reference, - 1, // depth - Some(false), - cx, - ); - }); + }); + + // select the first variable of scope 1 + cx.dispatch_action(SelectNext); + cx.run_until_parked(); + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { + debug_panel_item + .variable_list() + .update(cx, |variable_list, cx| { + variable_list.assert_visual_entries( + vec![ + "v Scope 1", + " > variable1 <=== selected", + " > variable2", + "> Scope 2", + ], + cx, + ); }); - }) - .unwrap(); + }); + // expand the nested variables of variable 1 + cx.dispatch_action(ExpandSelectedEntry); cx.run_until_parked(); + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { + debug_panel_item + .variable_list() + .update(cx, |variable_list, cx| { + variable_list.assert_visual_entries( + vec![ + "v Scope 1", + " v variable1 <=== selected", + " > nested1", + " > nested2", + " > variable2", + "> Scope 2", + ], + cx, + ); + }); + }); - workspace - .update(cx, |workspace, cx| { - let debug_panel = workspace.panel::(cx).unwrap(); - let active_debug_panel_item = debug_panel - .update(cx, |this, cx| this.active_debug_panel_item(cx)) - .unwrap(); - - active_debug_panel_item.update(cx, |debug_panel_item, cx| { - debug_panel_item - .variable_list() - .update(cx, |variable_list, _| { - // scope 1 - assert_eq!( - vec![ - VariableContainer { - container_reference: scopes[0].variables_reference, - variable: scope1_variables[0].clone(), - depth: 1, - }, - VariableContainer { - container_reference: scope1_variables[0].variables_reference, - variable: nested_variables[0].clone(), - depth: 2, - }, - VariableContainer { - container_reference: scope1_variables[0].variables_reference, - variable: nested_variables[1].clone(), - depth: 2, - }, - VariableContainer { - container_reference: scopes[0].variables_reference, - variable: scope1_variables[1].clone(), - depth: 1, - }, - ], - variable_list.variables_by_scope(1, 2).unwrap().variables() - ); - - // scope 2 - assert_eq!( - vec![VariableContainer { - container_reference: scopes[1].variables_reference, - variable: scope2_variables[0].clone(), - depth: 1, - }], - variable_list.variables_by_scope(1, 4).unwrap().variables() - ); - - // assert visual entries - assert_eq!( - vec![ - VariableListEntry::Scope(scopes[0].clone()), - VariableListEntry::Variable { - depth: 1, - scope: Arc::new(scopes[0].clone()), - has_children: true, - variable: Arc::new(scope1_variables[0].clone()), - container_reference: scopes[0].variables_reference, - }, - VariableListEntry::Variable { - depth: 2, - scope: Arc::new(scopes[0].clone()), - has_children: false, - variable: Arc::new(nested_variables[0].clone()), - container_reference: scope1_variables[0].variables_reference, - }, - VariableListEntry::Variable { - depth: 2, - scope: Arc::new(scopes[0].clone()), - has_children: false, - variable: Arc::new(nested_variables[1].clone()), - container_reference: scope1_variables[0].variables_reference, - }, - VariableListEntry::Variable { - depth: 1, - scope: Arc::new(scopes[0].clone()), - has_children: false, - variable: Arc::new(scope1_variables[1].clone()), - container_reference: scopes[0].variables_reference, - }, - VariableListEntry::Scope(scopes[1].clone()), - ], - variable_list.entries().get(&1).unwrap().clone() - ); - }); + // select the first nested variable of variable 1 + cx.dispatch_action(SelectNext); + cx.run_until_parked(); + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { + debug_panel_item + .variable_list() + .update(cx, |variable_list, cx| { + variable_list.assert_visual_entries( + vec![ + "v Scope 1", + " v variable1", + " > nested1 <=== selected", + " > nested2", + " > variable2", + "> Scope 2", + ], + cx, + ); }); - }) - .unwrap(); - - // toggle scope 2 to show variables - workspace - .update(cx, |workspace, cx| { - let debug_panel = workspace.panel::(cx).unwrap(); - let active_debug_panel_item = debug_panel - .update(cx, |this, cx| this.active_debug_panel_item(cx)) - .unwrap(); - - active_debug_panel_item.update(cx, |debug_panel_item, cx| { - debug_panel_item - .variable_list() - .update(cx, |variable_list, cx| { - variable_list.toggle_entry( - &crate::variable_list::OpenEntry::Scope { - name: scopes[1].name.clone(), - }, - cx, - ); - }); + }); + + // select the second nested variable of variable 1 + cx.dispatch_action(SelectNext); + cx.run_until_parked(); + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { + debug_panel_item + .variable_list() + .update(cx, |variable_list, cx| { + variable_list.assert_visual_entries( + vec![ + "v Scope 1", + " v variable1", + " > nested1", + " > nested2 <=== selected", + " > variable2", + "> Scope 2", + ], + cx, + ); }); - }) - .unwrap(); + }); + // select variable 2 of scope 1 + cx.dispatch_action(SelectNext); cx.run_until_parked(); + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { + debug_panel_item + .variable_list() + .update(cx, |variable_list, cx| { + variable_list.assert_visual_entries( + vec![ + "v Scope 1", + " v variable1", + " > nested1", + " > nested2", + " > variable2 <=== selected", + "> Scope 2", + ], + cx, + ); + }); + }); - workspace - .update(cx, |workspace, cx| { - let debug_panel = workspace.panel::(cx).unwrap(); - let active_debug_panel_item = debug_panel - .update(cx, |this, cx| this.active_debug_panel_item(cx)) - .unwrap(); - - active_debug_panel_item.update(cx, |debug_panel_item, cx| { - debug_panel_item - .variable_list() - .update(cx, |variable_list, _| { - // scope 1 - assert_eq!( - vec![ - VariableContainer { - container_reference: scopes[0].variables_reference, - variable: scope1_variables[0].clone(), - depth: 1, - }, - VariableContainer { - container_reference: scope1_variables[0].variables_reference, - variable: nested_variables[0].clone(), - depth: 2, - }, - VariableContainer { - container_reference: scope1_variables[0].variables_reference, - variable: nested_variables[1].clone(), - depth: 2, - }, - VariableContainer { - container_reference: scopes[0].variables_reference, - variable: scope1_variables[1].clone(), - depth: 1, - }, - ], - variable_list.variables_by_scope(1, 2).unwrap().variables() - ); - - // scope 2 - assert_eq!( - vec![VariableContainer { - container_reference: scopes[1].variables_reference, - variable: scope2_variables[0].clone(), - depth: 1, - }], - variable_list.variables_by_scope(1, 4).unwrap().variables() - ); - - // assert visual entries - assert_eq!( - vec![ - VariableListEntry::Scope(scopes[0].clone()), - VariableListEntry::Variable { - depth: 1, - scope: Arc::new(scopes[0].clone()), - has_children: true, - variable: Arc::new(scope1_variables[0].clone()), - container_reference: scopes[0].variables_reference, - }, - VariableListEntry::Variable { - depth: 2, - scope: Arc::new(scopes[0].clone()), - has_children: false, - variable: Arc::new(nested_variables[0].clone()), - container_reference: scope1_variables[0].variables_reference, - }, - VariableListEntry::Variable { - depth: 2, - scope: Arc::new(scopes[0].clone()), - has_children: false, - variable: Arc::new(nested_variables[1].clone()), - container_reference: scope1_variables[0].variables_reference, - }, - VariableListEntry::Variable { - depth: 1, - scope: Arc::new(scopes[0].clone()), - has_children: false, - variable: Arc::new(scope1_variables[1].clone()), - container_reference: scopes[0].variables_reference, - }, - VariableListEntry::Scope(scopes[1].clone()), - VariableListEntry::Variable { - depth: 1, - scope: Arc::new(scopes[1].clone()), - has_children: false, - variable: Arc::new(scope2_variables[0].clone()), - container_reference: scopes[1].variables_reference, - }, - ], - variable_list.entries().get(&1).unwrap().clone() - ); - }); + // select scope 2 + cx.dispatch_action(SelectNext); + cx.run_until_parked(); + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { + debug_panel_item + .variable_list() + .update(cx, |variable_list, cx| { + variable_list.assert_visual_entries( + vec![ + "v Scope 1", + " v variable1", + " > nested1", + " > nested2", + " > variable2", + "> Scope 2 <=== selected", + ], + cx, + ); }); - }) - .unwrap(); - - // toggle variable that has child variables to hide variables - workspace - .update(cx, |workspace, cx| { - let debug_panel = workspace.panel::(cx).unwrap(); - let active_debug_panel_item = debug_panel - .update(cx, |this, cx| this.active_debug_panel_item(cx)) - .unwrap(); - - active_debug_panel_item.update(cx, |debug_panel_item, cx| { - debug_panel_item - .variable_list() - .update(cx, |variable_list, cx| { - variable_list.toggle_entry( - &crate::variable_list::OpenEntry::Variable { - name: scope1_variables[0].name.clone(), - depth: 1, - }, - cx, - ); - }); + }); + + // expand the nested variables of scope 2 + cx.dispatch_action(ExpandSelectedEntry); + cx.run_until_parked(); + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { + debug_panel_item + .variable_list() + .update(cx, |variable_list, cx| { + variable_list.assert_visual_entries( + vec![ + "v Scope 1", + " v variable1", + " > nested1", + " > nested2", + " > variable2", + "v Scope 2 <=== selected", + " > variable3", + ], + cx, + ); }); - }) - .unwrap(); + }); + // select variable 3 of scope 2 + cx.dispatch_action(SelectNext); cx.run_until_parked(); + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { + debug_panel_item + .variable_list() + .update(cx, |variable_list, cx| { + variable_list.assert_visual_entries( + vec![ + "v Scope 1", + " v variable1", + " > nested1", + " > nested2", + " > variable2", + "v Scope 2", + " > variable3 <=== selected", + ], + cx, + ); + }); + }); - workspace - .update(cx, |workspace, cx| { - let debug_panel = workspace.panel::(cx).unwrap(); - let active_debug_panel_item = debug_panel - .update(cx, |this, cx| this.active_debug_panel_item(cx)) - .unwrap(); - - active_debug_panel_item.update(cx, |debug_panel_item, cx| { - debug_panel_item - .variable_list() - .update(cx, |variable_list, _| { - // scope 1 - assert_eq!( - vec![ - VariableContainer { - container_reference: scopes[0].variables_reference, - variable: scope1_variables[0].clone(), - depth: 1, - }, - VariableContainer { - container_reference: scope1_variables[0].variables_reference, - variable: nested_variables[0].clone(), - depth: 2, - }, - VariableContainer { - container_reference: scope1_variables[0].variables_reference, - variable: nested_variables[1].clone(), - depth: 2, - }, - VariableContainer { - container_reference: scopes[0].variables_reference, - variable: scope1_variables[1].clone(), - depth: 1, - }, - ], - variable_list.variables_by_scope(1, 2).unwrap().variables() - ); - - // scope 2 - assert_eq!( - vec![VariableContainer { - container_reference: scopes[1].variables_reference, - variable: scope2_variables[0].clone(), - depth: 1, - }], - variable_list.variables_by_scope(1, 4).unwrap().variables() - ); - - // assert visual entries - assert_eq!( - vec![ - VariableListEntry::Scope(scopes[0].clone()), - VariableListEntry::Variable { - depth: 1, - scope: Arc::new(scopes[0].clone()), - has_children: true, - variable: Arc::new(scope1_variables[0].clone()), - container_reference: scopes[0].variables_reference, - }, - VariableListEntry::Variable { - depth: 1, - scope: Arc::new(scopes[0].clone()), - has_children: false, - variable: Arc::new(scope1_variables[1].clone()), - container_reference: scopes[0].variables_reference, - }, - VariableListEntry::Scope(scopes[1].clone()), - VariableListEntry::Variable { - depth: 1, - scope: Arc::new(scopes[1].clone()), - has_children: false, - variable: Arc::new(scope2_variables[0].clone()), - container_reference: scopes[1].variables_reference, - }, - ], - variable_list.entries().get(&1).unwrap().clone() - ); - }); + // select scope 2 + cx.dispatch_action(CollapseSelectedEntry); + cx.run_until_parked(); + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { + debug_panel_item + .variable_list() + .update(cx, |variable_list, cx| { + variable_list.assert_visual_entries( + vec![ + "v Scope 1", + " v variable1", + " > nested1", + " > nested2", + " > variable2", + "v Scope 2 <=== selected", + " > variable3", + ], + cx, + ); }); - }) - .unwrap(); + }); + + // collapse variables of scope 2 + cx.dispatch_action(CollapseSelectedEntry); + cx.run_until_parked(); + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { + debug_panel_item + .variable_list() + .update(cx, |variable_list, cx| { + variable_list.assert_visual_entries( + vec![ + "v Scope 1", + " v variable1", + " > nested1", + " > nested2", + " > variable2", + "> Scope 2 <=== selected", + ], + cx, + ); + }); + }); + + // select variable 2 of scope 1 + cx.dispatch_action(CollapseSelectedEntry); + cx.run_until_parked(); + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { + debug_panel_item + .variable_list() + .update(cx, |variable_list, cx| { + variable_list.assert_visual_entries( + vec![ + "v Scope 1", + " v variable1", + " > nested1", + " > nested2", + " > variable2 <=== selected", + "> Scope 2", + ], + cx, + ); + }); + }); + + // select nested2 of variable 1 + cx.dispatch_action(CollapseSelectedEntry); + cx.run_until_parked(); + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { + debug_panel_item + .variable_list() + .update(cx, |variable_list, cx| { + variable_list.assert_visual_entries( + vec![ + "v Scope 1", + " v variable1", + " > nested1", + " > nested2 <=== selected", + " > variable2", + "> Scope 2", + ], + cx, + ); + }); + }); + + // select nested1 of variable 1 + cx.dispatch_action(CollapseSelectedEntry); + cx.run_until_parked(); + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { + debug_panel_item + .variable_list() + .update(cx, |variable_list, cx| { + variable_list.assert_visual_entries( + vec![ + "v Scope 1", + " v variable1", + " > nested1 <=== selected", + " > nested2", + " > variable2", + "> Scope 2", + ], + cx, + ); + }); + }); + + // select variable 1 of scope 1 + cx.dispatch_action(CollapseSelectedEntry); + cx.run_until_parked(); + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { + debug_panel_item + .variable_list() + .update(cx, |variable_list, cx| { + variable_list.assert_visual_entries( + vec![ + "v Scope 1", + " v variable1 <=== selected", + " > nested1", + " > nested2", + " > variable2", + "> Scope 2", + ], + cx, + ); + }); + }); + + // collapse variables of variable 1 + cx.dispatch_action(CollapseSelectedEntry); + cx.run_until_parked(); + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { + debug_panel_item + .variable_list() + .update(cx, |variable_list, cx| { + variable_list.assert_visual_entries( + vec![ + "v Scope 1", + " > variable1 <=== selected", + " > variable2", + "> Scope 2", + ], + cx, + ); + }); + }); + + // select scope 1 + cx.dispatch_action(CollapseSelectedEntry); + cx.run_until_parked(); + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { + debug_panel_item + .variable_list() + .update(cx, |variable_list, cx| { + variable_list.assert_visual_entries( + vec![ + "v Scope 1 <=== selected", + " > variable1", + " > variable2", + "> Scope 2", + ], + cx, + ); + }); + }); + + // collapse variables of scope 1 + cx.dispatch_action(CollapseSelectedEntry); + cx.run_until_parked(); + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { + debug_panel_item + .variable_list() + .update(cx, |variable_list, cx| { + variable_list + .assert_visual_entries(vec!["> Scope 1 <=== selected", "> Scope 2"], cx); + }); + }); let shutdown_session = project.update(cx, |project, cx| { project.dap_store().update(cx, |dap_store, cx| { diff --git a/crates/debugger_ui/src/variable_list.rs b/crates/debugger_ui/src/variable_list.rs index 6444938169fc4b..4f69894262a0e2 100644 --- a/crates/debugger_ui/src/variable_list.rs +++ b/crates/debugger_ui/src/variable_list.rs @@ -4,15 +4,13 @@ use dap::{ client::DebugAdapterClientId, proto_conversions::ProtoConversion, session::DebugSessionId, Scope, ScopePresentationHint, Variable, }; -use editor::{ - actions::{self, SelectAll}, - Editor, EditorEvent, -}; +use editor::{actions::SelectAll, Editor, EditorEvent}; use gpui::{ - anchored, deferred, list, AnyElement, ClipboardItem, DismissEvent, FocusHandle, FocusableView, - ListOffset, ListState, Model, MouseDownEvent, Point, Subscription, Task, View, + actions, anchored, deferred, list, AnyElement, ClipboardItem, DismissEvent, FocusHandle, + FocusableView, Hsla, ListOffset, ListState, Model, MouseDownEvent, Point, Subscription, Task, + View, }; -use menu::Confirm; +use menu::{Confirm, SelectFirst, SelectLast, SelectNext, SelectPrev}; use project::dap_store::DapStore; use proto::debugger_variable_list_entry::Entry; use rpc::proto::{ @@ -27,6 +25,8 @@ use sum_tree::{Dimension, Item, SumTree, Summary}; use ui::{prelude::*, ContextMenu, ListItem}; use util::ResultExt; +actions!(variable_list, [ExpandSelectedEntry, CollapseSelectedEntry]); + #[derive(Debug, Clone, PartialEq, Eq)] pub struct VariableContainer { pub container_reference: u64, @@ -129,8 +129,14 @@ impl SetVariableState { #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] pub enum OpenEntry { - Scope { name: String }, - Variable { name: String, depth: usize }, + Scope { + name: String, + }, + Variable { + scope_id: u64, + name: String, + depth: usize, + }, } impl OpenEntry { @@ -142,6 +148,7 @@ impl OpenEntry { proto::variable_list_open_entry::Entry::Variable(state) => Some(Self::Variable { name: state.name.clone(), depth: state.depth as usize, + scope_id: state.scope_id, }), } } @@ -153,10 +160,15 @@ impl OpenEntry { name: name.clone(), }) } - OpenEntry::Variable { name, depth } => { + OpenEntry::Variable { + name, + depth, + scope_id, + } => { proto::variable_list_open_entry::Entry::Variable(proto::DebuggerOpenEntryVariable { name: name.clone(), depth: *depth as u64, + scope_id: *scope_id, }) } }; @@ -364,16 +376,17 @@ pub struct VariableList { list: ListState, focus_handle: FocusHandle, dap_store: Model, + session_id: DebugSessionId, open_entries: Vec, client_id: DebugAdapterClientId, - session_id: DebugSessionId, - scopes: HashMap>, set_variable_editor: View, _subscriptions: Vec, + selection: Option, stack_frame_list: View, + scopes: HashMap>, set_variable_state: Option, - entries: HashMap>, fetch_variables_task: Option>>, + entries: HashMap>, variables: BTreeMap<(StackFrameId, ScopeId), ScopeVariableIndex>, open_context_menu: Option<(View, Point, Subscription)>, } @@ -416,6 +429,7 @@ impl VariableList { dap_store, focus_handle, _subscriptions, + selection: None, set_variable_editor, client_id: *client_id, session_id: *session_id, @@ -593,8 +607,11 @@ impl VariableList { return div().into_any_element(); }; - match &entries[ix] { - VariableListEntry::Scope(scope) => self.render_scope(scope, cx), + let entry = &entries[ix]; + match entry { + VariableListEntry::Scope(scope) => { + self.render_scope(scope, Some(entry) == self.selection.as_ref(), cx) + } VariableListEntry::SetVariableEditor { depth, state } => { self.render_set_variable_editor(*depth, state, cx) } @@ -610,11 +627,72 @@ impl VariableList { scope, *depth, *has_children, + Some(entry) == self.selection.as_ref(), cx, ), } } + fn toggle_variable( + &mut self, + scope_id: u64, + variable: &Variable, + depth: usize, + cx: &mut ViewContext, + ) { + let stack_frame_id = self.stack_frame_list.read(cx).current_stack_frame_id(); + + let Some(variable_index) = self.variables_by_scope(stack_frame_id, scope_id) else { + return; + }; + + let entry_id = OpenEntry::Variable { + depth, + scope_id, + name: variable.name.clone(), + }; + + let has_children = variable.variables_reference > 0; + let disclosed = has_children.then(|| self.open_entries.binary_search(&entry_id).is_ok()); + + // if we already opened the variable/we already fetched it + // we can just toggle it because we already have the nested variable + if disclosed.unwrap_or(true) || variable_index.fetched(&variable.variables_reference) { + return self.toggle_entry(&entry_id, cx); + } + + let fetch_variables_task = self.dap_store.update(cx, |store, cx| { + store.variables(&self.client_id, variable.variables_reference, cx) + }); + + let container_reference = variable.variables_reference; + let entry_id = entry_id.clone(); + + self.fetch_variables_task = Some(cx.spawn(|this, mut cx| async move { + let new_variables = fetch_variables_task.await?; + + this.update(&mut cx, |this, cx| { + let Some(index) = this.variables.get_mut(&(stack_frame_id, scope_id)) else { + return; + }; + + index.add_variables( + container_reference, + new_variables + .into_iter() + .map(|variable| VariableContainer { + variable, + depth: depth + 1, + container_reference, + }) + .collect::>(), + ); + + this.toggle_entry(&entry_id, cx); + }) + })) + } + pub fn toggle_entry(&mut self, entry_id: &OpenEntry, cx: &mut ViewContext) { match self.open_entries.binary_search(&entry_id) { Ok(ix) => { @@ -698,8 +776,9 @@ impl VariableList { if self .open_entries .binary_search(&OpenEntry::Variable { - name: variable.name.clone(), depth, + name: variable.name.clone(), + scope_id: scope.variables_reference, }) .is_err() { @@ -792,33 +871,32 @@ impl VariableList { fn fetch_nested_variables( &self, - variables_reference: u64, + container_reference: u64, depth: usize, open_entries: &Vec, cx: &mut ViewContext, ) -> Task>> { + let variables_task = self.dap_store.update(cx, |store, cx| { + store.variables(&self.client_id, container_reference, cx) + }); + cx.spawn({ let open_entries = open_entries.clone(); |this, mut cx| async move { - let variables_task = this.update(&mut cx, |this, cx| { - this.dap_store.update(cx, |store, cx| { - store.variables(&this.client_id, variables_reference, cx) - }) - })?; - let mut variables = Vec::new(); for variable in variables_task.await? { variables.push(VariableContainer { - variable: variable.clone(), - container_reference: variables_reference, depth, + container_reference, + variable: variable.clone(), }); if open_entries - .binary_search(&&OpenEntry::Variable { - name: variable.name.clone(), + .binary_search(&OpenEntry::Variable { depth, + scope_id: container_reference, + name: variable.name.clone(), }) .is_ok() { @@ -1081,6 +1159,134 @@ impl VariableList { }); } + fn select_first(&mut self, _: &SelectFirst, cx: &mut ViewContext) { + let stack_frame_id = self.stack_frame_list.read(cx).current_stack_frame_id(); + if let Some(entries) = self.entries.get(&stack_frame_id) { + self.selection = entries.first().cloned(); + cx.notify(); + }; + } + + fn select_last(&mut self, _: &SelectLast, cx: &mut ViewContext) { + let stack_frame_id = self.stack_frame_list.read(cx).current_stack_frame_id(); + if let Some(entries) = self.entries.get(&stack_frame_id) { + self.selection = entries.last().cloned(); + cx.notify(); + }; + } + + fn select_prev(&mut self, _: &SelectPrev, cx: &mut ViewContext) { + if let Some(selection) = &self.selection { + let stack_frame_id = self.stack_frame_list.read(cx).current_stack_frame_id(); + if let Some(entries) = self.entries.get(&stack_frame_id) { + if let Some(ix) = entries.iter().position(|entry| entry == selection) { + self.selection = entries.get(ix.saturating_sub(1)).cloned(); + cx.notify(); + } + } + } else { + self.select_first(&SelectFirst, cx); + } + } + + fn select_next(&mut self, _: &SelectNext, cx: &mut ViewContext) { + if let Some(selection) = &self.selection { + let stack_frame_id = self.stack_frame_list.read(cx).current_stack_frame_id(); + if let Some(entries) = self.entries.get(&stack_frame_id) { + if let Some(ix) = entries.iter().position(|entry| entry == selection) { + self.selection = entries.get(ix + 1).cloned(); + cx.notify(); + } + } + } else { + self.select_first(&SelectFirst, cx); + } + } + + fn collapse_selected_entry(&mut self, _: &CollapseSelectedEntry, cx: &mut ViewContext) { + if let Some(selection) = &self.selection { + match selection { + VariableListEntry::Scope(scope) => { + let entry_id = &OpenEntry::Scope { + name: scope.name.clone(), + }; + + if self.open_entries.binary_search(entry_id).is_err() { + self.select_prev(&SelectPrev, cx); + } else { + self.toggle_entry(entry_id, cx); + } + } + VariableListEntry::Variable { + depth, + variable, + scope, + .. + } => { + let entry_id = &OpenEntry::Variable { + depth: *depth, + name: variable.name.clone(), + scope_id: scope.variables_reference, + }; + + if self.open_entries.binary_search(entry_id).is_err() { + self.select_prev(&SelectPrev, cx); + } else { + self.toggle_variable( + scope.variables_reference, + &variable.clone(), + *depth, + cx, + ); + } + } + VariableListEntry::SetVariableEditor { .. } => {} + } + } + } + + fn expand_selected_entry(&mut self, _: &ExpandSelectedEntry, cx: &mut ViewContext) { + if let Some(selection) = &self.selection { + match selection { + VariableListEntry::Scope(scope) => { + let entry_id = &OpenEntry::Scope { + name: scope.name.clone(), + }; + + if self.open_entries.binary_search(entry_id).is_ok() { + self.select_next(&SelectNext, cx); + } else { + self.toggle_entry(entry_id, cx); + } + } + VariableListEntry::Variable { + depth, + variable, + scope, + .. + } => { + let entry_id = &OpenEntry::Variable { + depth: *depth, + name: variable.name.clone(), + scope_id: scope.variables_reference, + }; + + if self.open_entries.binary_search(entry_id).is_ok() { + self.select_next(&SelectNext, cx); + } else { + self.toggle_variable( + scope.variables_reference, + &variable.clone(), + *depth, + cx, + ); + } + } + VariableListEntry::SetVariableEditor { .. } => {} + } + } + } + fn render_set_variable_editor( &self, depth: usize, @@ -1100,105 +1306,142 @@ impl VariableList { .into_any_element() } - #[allow(clippy::too_many_arguments)] - pub fn on_toggle_variable( - &mut self, - scope_id: u64, - entry_id: &OpenEntry, - variable_reference: u64, - depth: usize, - disclosed: Option, - cx: &mut ViewContext, - ) { - let stack_frame_id = self.stack_frame_list.read(cx).current_stack_frame_id(); - - let Some(index) = self.variables_by_scope(stack_frame_id, scope_id) else { - return; - }; - - // if we already opened the variable/we already fetched it - // we can just toggle it because we already have the nested variable - if disclosed.unwrap_or(true) || index.fetched(&variable_reference) { - return self.toggle_entry(&entry_id, cx); - } - - let fetch_variables_task = self.dap_store.update(cx, |store, cx| { - store.variables(&self.client_id, variable_reference, cx) - }); - - let entry_id = entry_id.clone(); - cx.spawn(|this, mut cx| async move { - let new_variables = fetch_variables_task.await?; - - this.update(&mut cx, |this, cx| { - let Some(index) = this.variables.get_mut(&(stack_frame_id, scope_id)) else { - return; - }; + #[track_caller] + #[cfg(any(test, feature = "test-support"))] + pub fn assert_visual_entries(&self, expected: Vec<&str>, cx: &ViewContext) { + const INDENT: &'static str = " "; - index.add_variables( - variable_reference, - new_variables - .into_iter() - .map(|variable| VariableContainer { - container_reference: variable_reference, - variable, - depth: depth + 1, + let stack_frame_id = self.stack_frame_list.read(cx).current_stack_frame_id(); + let entries = self.entries.get(&stack_frame_id).unwrap(); + + let mut visual_entries = Vec::with_capacity(entries.len()); + for entry in entries { + let is_selected = Some(entry) == self.selection.as_ref(); + + match entry { + VariableListEntry::Scope(scope) => { + let is_expanded = self + .open_entries + .binary_search(&OpenEntry::Scope { + name: scope.name.clone(), }) - .collect::>(), - ); + .is_ok(); + + visual_entries.push(format!( + "{} {}{}", + if is_expanded { "v" } else { ">" }, + scope.name, + if is_selected { " <=== selected" } else { "" } + )); + } + VariableListEntry::SetVariableEditor { depth, state } => { + visual_entries.push(format!( + "{} [EDITOR: {}]{}", + INDENT.repeat(*depth), + state.name, + if is_selected { " <=== selected" } else { "" } + )); + } + VariableListEntry::Variable { + depth, + variable, + scope, + .. + } => { + let is_expanded = self + .open_entries + .binary_search(&OpenEntry::Variable { + depth: *depth, + name: variable.name.clone(), + scope_id: scope.variables_reference, + }) + .is_ok(); + + visual_entries.push(format!( + "{}{} {}{}", + INDENT.repeat(*depth), + if is_expanded { "v" } else { ">" }, + variable.name, + if is_selected { " <=== selected" } else { "" } + )); + } + }; + } - this.toggle_entry(&entry_id, cx); - }) - }) - .detach_and_log_err(cx); + pretty_assertions::assert_eq!(expected, visual_entries); } #[allow(clippy::too_many_arguments)] fn render_variable( &self, - parent_variables_reference: u64, - variable: &Variable, - scope: &Scope, + container_reference: u64, + variable: &Arc, + scope: &Arc, depth: usize, has_children: bool, + is_selected: bool, cx: &mut ViewContext, ) -> AnyElement { let scope_id = scope.variables_reference; - let variable_reference = variable.variables_reference; - let entry_id = OpenEntry::Variable { - name: variable.name.clone(), depth, + scope_id, + name: variable.name.clone(), }; let disclosed = has_children.then(|| self.open_entries.binary_search(&entry_id).is_ok()); + let colors = get_entry_color(cx); + let bg_hover_color = if !is_selected { + colors.hover + } else { + colors.default + }; + let border_color = if is_selected { + colors.marked_active + } else { + colors.default + }; + div() .id(SharedString::from(format!( "variable-{}-{}-{}", scope.variables_reference, variable.name, depth ))) - .group("") + .group("variable_list_entry") + .border_1() + .border_r_2() + .border_color(border_color) .h_4() .size_full() + .hover(|style| style.bg(bg_hover_color)) + .on_click(cx.listener({ + let scope = scope.clone(); + let variable = variable.clone(); + move |this, _, cx| { + this.selection = Some(VariableListEntry::Variable { + depth, + has_children, + container_reference, + scope: scope.clone(), + variable: variable.clone(), + }); + cx.notify(); + } + })) .child( ListItem::new(SharedString::from(format!( "variable-item-{}-{}-{}", scope.variables_reference, variable.name, depth ))) + .selectable(false) .indent_level(depth + 1) .indent_step_size(px(20.)) .always_show_disclosure_icon(true) .toggle(disclosed) .when(has_children, |list_item| { - list_item.on_toggle(cx.listener(move |this, _, cx| { - this.on_toggle_variable( - scope_id, - &entry_id, - variable_reference, - depth, - disclosed, - cx, - ) + list_item.on_toggle(cx.listener({ + let variable = variable.clone(); + move |this, _, cx| this.toggle_variable(scope_id, &variable, depth, cx) })) }) .on_secondary_mouse_down(cx.listener({ @@ -1206,7 +1449,7 @@ impl VariableList { let variable = variable.clone(); move |this, event: &MouseDownEvent, cx| { this.deploy_variable_context_menu( - parent_variables_reference, + container_reference, &scope, &variable, event.position, @@ -1230,7 +1473,12 @@ impl VariableList { .into_any() } - fn render_scope(&self, scope: &Scope, cx: &mut ViewContext) -> AnyElement { + fn render_scope( + &self, + scope: &Scope, + is_selected: bool, + cx: &mut ViewContext, + ) -> AnyElement { let element_id = scope.variables_reference; let entry_id = OpenEntry::Scope { @@ -1238,17 +1486,41 @@ impl VariableList { }; let disclosed = self.open_entries.binary_search(&entry_id).is_ok(); + let colors = get_entry_color(cx); + let bg_hover_color = if !is_selected { + colors.hover + } else { + colors.default + }; + let border_color = if is_selected { + colors.marked_active + } else { + colors.default + }; + div() .id(element_id as usize) - .group("") + .group("variable_list_entry") + .border_1() + .border_r_2() + .border_color(border_color) .flex() .w_full() .h_full() + .hover(|style| style.bg(bg_hover_color)) + .on_click(cx.listener({ + let scope = scope.clone(); + move |this, _, cx| { + this.selection = Some(VariableListEntry::Scope(scope.clone())); + cx.notify(); + } + })) .child( ListItem::new(SharedString::from(format!( "scope-{}", scope.variables_reference ))) + .selectable(false) .indent_level(1) .indent_step_size(px(20.)) .always_show_disclosure_icon(true) @@ -1269,10 +1541,20 @@ impl FocusableView for VariableList { impl Render for VariableList { fn render(&mut self, cx: &mut ViewContext) -> impl IntoElement { div() + .key_context("VariableList") + .id("variable-list") + .group("variable-list") .size_full() - .on_action( - cx.listener(|this, _: &actions::Cancel, cx| this.cancel_set_variable_value(cx)), - ) + .track_focus(&self.focus_handle(cx)) + .on_action(cx.listener(Self::select_first)) + .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::expand_selected_entry)) + .on_action(cx.listener(Self::collapse_selected_entry)) + .on_action(cx.listener(|this, _: &editor::actions::Cancel, cx| { + this.cancel_set_variable_value(cx) + })) .child(list(self.list.clone()).gap_1_5().size_full()) .children(self.open_context_menu.as_ref().map(|(menu, position, _)| { deferred( @@ -1286,6 +1568,22 @@ impl Render for VariableList { } } +struct EntryColors { + default: Hsla, + hover: Hsla, + marked_active: Hsla, +} + +fn get_entry_color(cx: &ViewContext) -> EntryColors { + let colors = cx.theme().colors(); + + EntryColors { + default: colors.panel_background, + hover: colors.ghost_element_hover, + marked_active: colors.ghost_element_selected, + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/proto/proto/zed.proto b/crates/proto/proto/zed.proto index d845dc242e774f..d876cb062b7c52 100644 --- a/crates/proto/proto/zed.proto +++ b/crates/proto/proto/zed.proto @@ -2510,8 +2510,9 @@ message DebuggerOpenEntryScope { } message DebuggerOpenEntryVariable { - string name = 1; - uint64 depth = 2; + uint64 scope_id = 1; + string name = 2; + uint64 depth = 3; } message DebuggerVariableListEntry { From c91f51dd6865b2f4864dc09e597b15a3f795b504 Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Sat, 11 Jan 2025 17:20:07 +0100 Subject: [PATCH 04/10] Log errors ins stepping tasks, and update tests I updated the tests so we don't reuse the debug panel item for each operation, instead we request a new instance each time so we can ensure the status actually changed. --- crates/debugger_ui/src/debugger_panel_item.rs | 69 +++++----- .../debugger_ui/src/tests/debugger_panel.rs | 125 ++++++++++++------ 2 files changed, 120 insertions(+), 74 deletions(-) diff --git a/crates/debugger_ui/src/debugger_panel_item.rs b/crates/debugger_ui/src/debugger_panel_item.rs index a6b63b58f24c55..88424074b0b0c0 100644 --- a/crates/debugger_ui/src/debugger_panel_item.rs +++ b/crates/debugger_ui/src/debugger_panel_item.rs @@ -527,8 +527,8 @@ impl DebugPanelItem { } #[cfg(any(test, feature = "test-support"))] - pub fn thread_status(&self, cx: &ViewContext) -> ThreadStatus { - self.thread_state.read(cx).status + pub fn thread_state(&self) -> &Model { + &self.thread_state } pub fn capabilities(&self, cx: &mut ViewContext) -> Capabilities { @@ -612,16 +612,15 @@ impl DebugPanelItem { store.continue_thread(&self.client_id, self.thread_id, cx) }); - let task = cx.spawn(|weak, mut cx| async move { - if let Err(_) = task.await { - weak.update(&mut cx, |this, cx| { - this.update_thread_state_status(ThreadStatus::Stopped, cx); + cx.spawn(|this, mut cx| async move { + if task.await.log_err().is_none() { + this.update(&mut cx, |debug_panel_item, cx| { + debug_panel_item.update_thread_state_status(ThreadStatus::Stopped, cx); }) .log_err(); } - }); - - cx.background_executor().spawn(task).detach(); + }) + .detach(); } pub fn step_over(&mut self, cx: &mut ViewContext) { @@ -632,16 +631,15 @@ impl DebugPanelItem { store.step_over(&self.client_id, self.thread_id, granularity, cx) }); - let task = cx.spawn(|weak, mut cx| async move { - if let Err(_) = task.await { - weak.update(&mut cx, |this, cx| { - this.update_thread_state_status(ThreadStatus::Stopped, cx); + cx.spawn(|this, mut cx| async move { + if task.await.log_err().is_none() { + this.update(&mut cx, |debug_panel_item, cx| { + debug_panel_item.update_thread_state_status(ThreadStatus::Stopped, cx); }) .log_err(); } - }); - - cx.background_executor().spawn(task).detach(); + }) + .detach(); } pub fn step_in(&mut self, cx: &mut ViewContext) { @@ -652,16 +650,15 @@ impl DebugPanelItem { store.step_in(&self.client_id, self.thread_id, granularity, cx) }); - let task = cx.spawn(|weak, mut cx| async move { - if let Err(_) = task.await { - weak.update(&mut cx, |this, cx| { - this.update_thread_state_status(ThreadStatus::Stopped, cx); + cx.spawn(|this, mut cx| async move { + if task.await.log_err().is_none() { + this.update(&mut cx, |debug_panel_item, cx| { + debug_panel_item.update_thread_state_status(ThreadStatus::Stopped, cx); }) .log_err(); } - }); - - cx.background_executor().spawn(task).detach(); + }) + .detach(); } pub fn step_out(&mut self, cx: &mut ViewContext) { @@ -672,16 +669,15 @@ impl DebugPanelItem { store.step_out(&self.client_id, self.thread_id, granularity, cx) }); - let task = cx.spawn(|weak, mut cx| async move { - if let Err(_) = task.await { - weak.update(&mut cx, |this, cx| { - this.update_thread_state_status(ThreadStatus::Stopped, cx); + cx.spawn(|this, mut cx| async move { + if task.await.log_err().is_none() { + this.update(&mut cx, |debug_panel_item, cx| { + debug_panel_item.update_thread_state_status(ThreadStatus::Stopped, cx); }) .log_err(); } - }); - - cx.background_executor().spawn(task).detach(); + }) + .detach(); } pub fn step_back(&mut self, cx: &mut ViewContext) { @@ -692,16 +688,15 @@ impl DebugPanelItem { store.step_back(&self.client_id, self.thread_id, granularity, cx) }); - let task = cx.spawn(|weak, mut cx| async move { - if let Err(_) = task.await { - weak.update(&mut cx, |this, cx| { - this.update_thread_state_status(ThreadStatus::Stopped, cx); + cx.spawn(|this, mut cx| async move { + if task.await.log_err().is_none() { + this.update(&mut cx, |debug_panel_item, cx| { + debug_panel_item.update_thread_state_status(ThreadStatus::Stopped, cx); }) .log_err(); } - }); - - cx.background_executor().spawn(task).detach(); + }) + .detach(); } pub fn restart_client(&self, cx: &mut ViewContext) { diff --git a/crates/debugger_ui/src/tests/debugger_panel.rs b/crates/debugger_ui/src/tests/debugger_panel.rs index 1b3e27c3e3e8a4..db29446e5e9348 100644 --- a/crates/debugger_ui/src/tests/debugger_panel.rs +++ b/crates/debugger_ui/src/tests/debugger_panel.rs @@ -8,6 +8,7 @@ use dap::{ ErrorResponse, RunInTerminalRequestArguments, StartDebuggingRequestArguments, StartDebuggingRequestArgumentsRequest, }; +use debugger_panel::ThreadStatus; use gpui::{BackgroundExecutor, TestAppContext, VisualTestContext}; use project::{FakeFs, Project}; use serde_json::json; @@ -16,7 +17,7 @@ use std::sync::{ Arc, }; use terminal_view::{terminal_panel::TerminalPanel, TerminalView}; -use tests::{init_test, init_test_workspace}; +use tests::{active_debug_panel_item, init_test, init_test_workspace}; use workspace::dock::Panel; #[gpui::test] @@ -969,23 +970,83 @@ async fn test_debug_panel_item_thread_status_reset_on_failure( .await; client - .on_request::(move |_, _| Err(ErrorResponse { error: None })) + .on_request::(move |_, _| { + Err(ErrorResponse { + error: Some(dap::Message { + id: 1, + format: "error".into(), + variables: None, + send_telemetry: None, + show_user: None, + url: None, + url_label: None, + }), + }) + }) .await; client - .on_request::(move |_, _| Err(ErrorResponse { error: None })) + .on_request::(move |_, _| { + Err(ErrorResponse { + error: Some(dap::Message { + id: 1, + format: "error".into(), + variables: None, + send_telemetry: None, + show_user: None, + url: None, + url_label: None, + }), + }) + }) .await; client - .on_request::(move |_, _| Err(ErrorResponse { error: None })) + .on_request::(move |_, _| { + Err(ErrorResponse { + error: Some(dap::Message { + id: 1, + format: "error".into(), + variables: None, + send_telemetry: None, + show_user: None, + url: None, + url_label: None, + }), + }) + }) .await; client - .on_request::(move |_, _| Err(ErrorResponse { error: None })) + .on_request::(move |_, _| { + Err(ErrorResponse { + error: Some(dap::Message { + id: 1, + format: "error".into(), + variables: None, + send_telemetry: None, + show_user: None, + url: None, + url_label: None, + }), + }) + }) .await; client - .on_request::(move |_, _| Err(ErrorResponse { error: None })) + .on_request::(move |_, _| { + Err(ErrorResponse { + error: Some(dap::Message { + id: 1, + format: "error".into(), + variables: None, + send_telemetry: None, + show_user: None, + url: None, + url_label: None, + }), + }) + }) .await; client @@ -1004,16 +1065,6 @@ async fn test_debug_panel_item_thread_status_reset_on_failure( cx.run_until_parked(); - let debug_panel_item = workspace - .update(cx, |workspace, cx| { - workspace - .panel::(cx) - .unwrap() - .update(cx, |panel, cx| panel.active_debug_panel_item(cx)) - .unwrap() - }) - .unwrap(); - for operation in &[ "step_over", "continue_thread", @@ -1021,24 +1072,35 @@ async fn test_debug_panel_item_thread_status_reset_on_failure( "step_in", "step_out", ] { - debug_panel_item.update(cx, |item, cx| match *operation { - "step_over" => item.step_over(cx), - "continue_thread" => item.continue_thread(cx), - "step_back" => item.step_back(cx), - "step_in" => item.step_in(cx), - "step_out" => item.step_out(cx), - _ => unreachable!(), - }); + active_debug_panel_item(workspace, cx).update( + cx, + |debug_panel_item, cx| match *operation { + "step_over" => debug_panel_item.step_over(cx), + "continue_thread" => debug_panel_item.continue_thread(cx), + "step_back" => debug_panel_item.step_back(cx), + "step_in" => debug_panel_item.step_in(cx), + "step_out" => debug_panel_item.step_out(cx), + _ => unreachable!(), + }, + ); cx.run_until_parked(); - debug_panel_item.update(cx, |item, cx| { + active_debug_panel_item(workspace, cx).update(cx, |debug_panel_item, cx| { assert_eq!( - item.thread_status(cx), + debug_panel_item.thread_state().read(cx).status, debugger_panel::ThreadStatus::Stopped, "Thread status not reset to Stopped after failed {}", operation ); + + // update state to running, so we can test it actually changes the status back to stopped + debug_panel_item + .thread_state() + .update(cx, |thread_state, cx| { + thread_state.status = ThreadStatus::Running; + cx.notify(); + }); }); } @@ -1049,15 +1111,4 @@ async fn test_debug_panel_item_thread_status_reset_on_failure( }); shutdown_session.await.unwrap(); - - workspace - .update(cx, |workspace, cx| { - let debug_panel = workspace.panel::(cx).unwrap(); - - debug_panel.update(cx, |this, cx| { - assert!(this.active_debug_panel_item(cx).is_none()); - assert_eq!(0, this.pane().unwrap().read(cx).items_len()); - }); - }) - .unwrap(); } From 887e2a65e129ba186f2b6f2e0a69caf907ebbddb Mon Sep 17 00:00:00 2001 From: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com> Date: Sat, 11 Jan 2025 11:26:49 -0500 Subject: [PATCH 05/10] Use Toast Notification for Debug Session Warning (#83) * Switch debug session exited without hitting breakpoint to toast notification * Move notify below the thread event, so we register the thread is added --------- Co-authored-by: Remco Smits --- crates/debugger_ui/src/debugger_panel.rs | 57 +++--------------------- 1 file changed, 7 insertions(+), 50 deletions(-) diff --git a/crates/debugger_ui/src/debugger_panel.rs b/crates/debugger_ui/src/debugger_panel.rs index 9ed11f8d2fc1ed..66a553d181f741 100644 --- a/crates/debugger_ui/src/debugger_panel.rs +++ b/crates/debugger_ui/src/debugger_panel.rs @@ -15,7 +15,7 @@ use dap::{ }; use gpui::{ actions, Action, AppContext, AsyncWindowContext, EventEmitter, FocusHandle, FocusableView, - FontWeight, Model, Subscription, Task, View, ViewContext, WeakView, + Model, Subscription, Task, View, ViewContext, WeakView, }; use project::{ dap_store::{DapStore, DapStoreEvent}, @@ -117,7 +117,6 @@ pub struct DebugPanel { focus_handle: FocusHandle, dap_store: Model, workspace: WeakView, - show_did_not_stop_warning: bool, _subscriptions: Vec, message_queue: HashMap>, thread_states: BTreeMap<(DebugAdapterClientId, u64), Model>, @@ -209,7 +208,6 @@ impl DebugPanel { size: px(300.), _subscriptions, focus_handle: cx.focus_handle(), - show_did_not_stop_warning: false, thread_states: Default::default(), message_queue: Default::default(), workspace: workspace.weak_handle(), @@ -810,8 +808,11 @@ impl DebugPanel { if let Some(thread_state) = self.thread_states.get(&(*client_id, thread_id)) { if !thread_state.read(cx).stopped && event.reason == ThreadEventReason::Exited { - self.show_did_not_stop_warning = true; - cx.notify(); + const MESSAGE: &'static str = "Debug session exited without hitting breakpoints\n\nTry adding a breakpoint, or define the correct path mapping for your debugger."; + + self.dap_store.update(cx, |_, cx| { + cx.emit(DapStoreEvent::Notification(MESSAGE.into())); + }); }; } @@ -823,6 +824,7 @@ impl DebugPanel { } cx.emit(DebugPanelEvent::Thread((*client_id, event.clone()))); + cx.notify(); } fn handle_exited_event( @@ -1034,48 +1036,6 @@ impl DebugPanel { cx.emit(DebugPanelEvent::CapabilitiesChanged(*client_id)); } - - fn render_did_not_stop_warning(&self, cx: &mut ViewContext) -> impl IntoElement { - const TITLE: &'static str = "Debug session exited without hitting any breakpoints"; - const DESCRIPTION: &'static str = - "Try adding a breakpoint, or define the correct path mapping for your debugger."; - - div() - .absolute() - .right_3() - .bottom_12() - .max_w_96() - .py_2() - .px_3() - .elevation_2(cx) - .occlude() - .child( - v_flex() - .gap_0p5() - .child( - h_flex() - .gap_1p5() - .items_center() - .child(Icon::new(IconName::Warning).color(Color::Conflict)) - .child(Label::new(TITLE).weight(FontWeight::MEDIUM)), - ) - .child( - Label::new(DESCRIPTION) - .size(LabelSize::Small) - .color(Color::Muted), - ) - .child( - h_flex().justify_end().mt_1().child( - Button::new("dismiss", "Dismiss") - .color(Color::Muted) - .on_click(cx.listener(|this, _, cx| { - this.show_did_not_stop_warning = false; - cx.notify(); - })), - ), - ), - ) - } } impl EventEmitter for DebugPanel {} @@ -1146,9 +1106,6 @@ impl Render for DebugPanel { .key_context("DebugPanel") .track_focus(&self.focus_handle) .size_full() - .when(self.show_did_not_stop_warning, |this| { - this.child(self.render_did_not_stop_warning(cx)) - }) .map(|this| { if self.pane.read(cx).items_len() == 0 { this.child( From 943609f1ac93b6b29e86b396a8eb4c3abf4d12dc Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Sat, 11 Jan 2025 18:46:00 +0100 Subject: [PATCH 06/10] Fix failing editor test --- crates/editor/src/editor_tests.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 568ed027996d51..e4e16f8c4bdfeb 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -10253,6 +10253,7 @@ async fn test_move_to_enclosing_bracket(cx: &mut gpui::TestAppContext) { cx.update_editor(|editor, cx| { editor.move_to_enclosing_bracket(&MoveToEnclosingBracket, cx) }); + cx.run_until_parked(); cx.assert_editor_state(after); }; From 4e13a00c44a4742d0d0546d545fe5ff55a69cd0d Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Sat, 11 Jan 2025 19:02:32 +0100 Subject: [PATCH 07/10] Add missing action handler for OpenDebugTasks --- crates/zed/src/zed.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index ae19ca63efba63..359e63abbc0133 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -602,6 +602,15 @@ fn register_actions( ); }, ) + .register_action( + move |_: &mut Workspace, _: &OpenDebugTasks, cx: &mut ViewContext| { + open_settings_file( + paths::debug_tasks_file(), + || settings::initial_debug_tasks_content().as_ref().into(), + cx, + ); + }, + ) .register_action(open_project_settings_file) .register_action(open_project_tasks_file) .register_action(open_project_debug_tasks_file) From 4baa7f77422c720e096affa346362023d8085d7a Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Sun, 12 Jan 2025 15:47:43 +0100 Subject: [PATCH 08/10] Don't show Telemetry data in output console --- crates/debugger_ui/src/debugger_panel_item.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/debugger_ui/src/debugger_panel_item.rs b/crates/debugger_ui/src/debugger_panel_item.rs index 88424074b0b0c0..63f1b524237fdf 100644 --- a/crates/debugger_ui/src/debugger_panel_item.rs +++ b/crates/debugger_ui/src/debugger_panel_item.rs @@ -346,13 +346,16 @@ impl DebugPanelItem { return; } - // The default value of an event category is console - // so we assume that is the output type if it doesn't exist let output_category = event .category .as_ref() .unwrap_or(&OutputEventCategory::Console); + // skip telementry output as it pollutes the users output view + if output_category == &OutputEventCategory::Telemetry { + return; + } + match output_category { OutputEventCategory::Console => { self.console.update(cx, |console, cx| { From 5aa816e85b3990ee33302dbffccba1930f029b30 Mon Sep 17 00:00:00 2001 From: Anthony Eid Date: Sun, 12 Jan 2025 15:26:25 -0500 Subject: [PATCH 09/10] Fix telemetry spelling error --- crates/debugger_ui/src/debugger_panel_item.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/debugger_ui/src/debugger_panel_item.rs b/crates/debugger_ui/src/debugger_panel_item.rs index 63f1b524237fdf..a81b0f5ddba508 100644 --- a/crates/debugger_ui/src/debugger_panel_item.rs +++ b/crates/debugger_ui/src/debugger_panel_item.rs @@ -351,7 +351,7 @@ impl DebugPanelItem { .as_ref() .unwrap_or(&OutputEventCategory::Console); - // skip telementry output as it pollutes the users output view + // skip telemetry output as it pollutes the users output view if output_category == &OutputEventCategory::Telemetry { return; } From 2736b2f477276756f40af1b495eb683143aa4c97 Mon Sep 17 00:00:00 2001 From: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com> Date: Sun, 12 Jan 2025 23:42:51 -0500 Subject: [PATCH 10/10] Update rust-embed version to fix cicd build error (#87) --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 1a3b8e788810f7..b05e181e3d0ce1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -450,7 +450,7 @@ runtimelib = { version = "0.25.0", default-features = false, features = [ "async-dispatcher-runtime", ] } rustc-demangle = "0.1.23" -rust-embed = { version = "8.4", features = ["include-exclude"] } +rust-embed = { version = "8.5", features = ["include-exclude"] } rustc-hash = "2.1.0" rustls = "0.21.12" rustls-native-certs = "0.8.0"