Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

debugger: Fix bug where active debug line highlights weren't cleared (#29562)

Browse source 
## Context
The bug occurred because we stopped propagating the
`BreakpointStoreEvent::SetDebugLine` whenever a new debug line highlight
had been set. This was done to prevent multiple panes from having
editors focus on the debug line. However, it stopped the event from
propagating to editors that needed to clear their debug line highlights.

I fixed this by introducing two phases
1. Clear all debug line highlights
2. Set active debug line highlight in singular editor 

I also added a test to prevent regressions from occurring

Release Notes:

- N/A
This commit is contained in:
Anthony Eid 2025-04-29 11:15:45 -04:00 • committed by GitHub
parent c168fc335c
commit 6386336eee
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 292 additions and 15 deletions
Download patch file Download diff file Expand all files Collapse all files

260
crates/debugger_ui/src/tests/debugger_panel.rs
View file

@ -14,7 +14,7 @@ use dap::{
},
};
use editor::{
Editor, EditorMode, MultiBuffer,
ActiveDebugLine, Editor, EditorMode, MultiBuffer,
actions::{self},
};
use gpui::{BackgroundExecutor, TestAppContext, VisualTestContext};
@ -1460,3 +1460,261 @@ async fn test_we_send_arguments_from_user_config(
"Launch request handler was not called"
);
}
#[gpui::test]
async fn test_active_debug_line_setting(executor: BackgroundExecutor, cx: &mut TestAppContext) {
init_test(cx);
let fs = FakeFs::new(executor.clone());
fs.insert_tree(
path!("/project"),
json!({
"main.rs": "First line\nSecond line\nThird line\nFourth line",
"second.rs": "First line\nSecond line\nThird line\nFourth line",
}),
)
.await;
let project = Project::test(fs, [path!("/project").as_ref()], cx).await;
let workspace = init_test_workspace(&project, cx).await;
let cx = &mut VisualTestContext::from_window(*workspace, cx);
let project_path = Path::new(path!("/project"));
let worktree = project
.update(cx, |project, cx| project.find_worktree(project_path, cx))
.expect("This worktree should exist in project")
.0;
let worktree_id = workspace
.update(cx, |_, _, cx| worktree.read(cx).id())
.unwrap();
let main_buffer = project
.update(cx, |project, cx| {
project.open_buffer((worktree_id, "main.rs"), cx)
})
.await
.unwrap();
let second_buffer = project
.update(cx, |project, cx| {
project.open_buffer((worktree_id, "second.rs"), cx)
})
.await
.unwrap();
let (main_editor, cx) = cx.add_window_view(|window, cx| {
Editor::new(
EditorMode::full(),
MultiBuffer::build_from_buffer(main_buffer, cx),
Some(project.clone()),
window,
cx,
)
});
let (second_editor, cx) = cx.add_window_view(|window, cx| {
Editor::new(
EditorMode::full(),
MultiBuffer::build_from_buffer(second_buffer, cx),
Some(project.clone()),
window,
cx,
)
});
let session = start_debug_session(&workspace, cx, |_| {}).unwrap();
let client = session.update(cx, |session, _| session.adapter_client().unwrap());
client.on_request::<dap::requests::Threads, _>(move |_, _| {
Ok(dap::ThreadsResponse {
threads: vec![dap::Thread {
id: 1,
name: "Thread 1".into(),
}],
})
});
client.on_request::<dap::requests::Scopes, _>(move |_, _| {
Ok(dap::ScopesResponse {
scopes: Vec::default(),
})
});
client.on_request::<StackTrace, _>(move |_, args| {
assert_eq!(args.thread_id, 1);
Ok(dap::StackTraceResponse {
stack_frames: vec![dap::StackFrame {
id: 1,
name: "frame 1".into(),
source: Some(dap::Source {
name: Some("main.rs".into()),
path: Some(path!("/project/main.rs").into()),
source_reference: None,
presentation_hint: None,
origin: None,
sources: None,
adapter_data: None,
checksums: None,
}),
line: 2,
column: 0,
end_line: None,
end_column: None,
can_restart: None,
instruction_pointer_reference: None,
module_id: None,
presentation_hint: None,
}],
total_frames: None,
})
});
client
.fake_event(dap::messages::Events::Stopped(dap::StoppedEvent {
reason: dap::StoppedEventReason::Breakpoint,
description: None,
thread_id: Some(1),
preserve_focus_hint: None,
text: None,
all_threads_stopped: None,
hit_breakpoint_ids: None,
}))
.await;
cx.run_until_parked();
main_editor.update_in(cx, |editor, window, cx| {
let active_debug_lines: Vec<_> = editor.highlighted_rows::<ActiveDebugLine>().collect();
assert_eq!(
active_debug_lines.len(),
1,
"There should be only one active debug line"
);
let point = editor
.snapshot(window, cx)
.buffer_snapshot
.summary_for_anchor::<language::Point>(&active_debug_lines.first().unwrap().0.start);
assert_eq!(point.row, 1);
});
second_editor.update(cx, |editor, _| {
let active_debug_lines: Vec<_> = editor.highlighted_rows::<ActiveDebugLine>().collect();
assert!(
active_debug_lines.is_empty(),
"There shouldn't be any active debug lines"
);
});
let handled_second_stacktrace = Arc::new(AtomicBool::new(false));
client.on_request::<StackTrace, _>({
let handled_second_stacktrace = handled_second_stacktrace.clone();
move |_, args| {
handled_second_stacktrace.store(true, Ordering::SeqCst);
assert_eq!(args.thread_id, 1);
Ok(dap::StackTraceResponse {
stack_frames: vec![dap::StackFrame {
id: 2,
name: "frame 2".into(),
source: Some(dap::Source {
name: Some("second.rs".into()),
path: Some(path!("/project/second.rs").into()),
source_reference: None,
presentation_hint: None,
origin: None,
sources: None,
adapter_data: None,
checksums: None,
}),
line: 3,
column: 0,
end_line: None,
end_column: None,
can_restart: None,
instruction_pointer_reference: None,
module_id: None,
presentation_hint: None,
}],
total_frames: None,
})
}
});
client
.fake_event(dap::messages::Events::Stopped(dap::StoppedEvent {
reason: dap::StoppedEventReason::Breakpoint,
description: None,
thread_id: Some(1),
preserve_focus_hint: None,
text: None,
all_threads_stopped: None,
hit_breakpoint_ids: None,
}))
.await;
cx.run_until_parked();
second_editor.update_in(cx, |editor, window, cx| {
let active_debug_lines: Vec<_> = editor.highlighted_rows::<ActiveDebugLine>().collect();
assert_eq!(
active_debug_lines.len(),
1,
"There should be only one active debug line"
);
let point = editor
.snapshot(window, cx)
.buffer_snapshot
.summary_for_anchor::<language::Point>(&active_debug_lines.first().unwrap().0.start);
assert_eq!(point.row, 2);
});
main_editor.update(cx, |editor, _| {
let active_debug_lines: Vec<_> = editor.highlighted_rows::<ActiveDebugLine>().collect();
assert!(
active_debug_lines.is_empty(),
"There shouldn't be any active debug lines"
);
});
assert!(
handled_second_stacktrace.load(Ordering::SeqCst),
"Second stacktrace request handler was not called"
);
// Clean up
let shutdown_session = project.update(cx, |project, cx| {
project.dap_store().update(cx, |dap_store, cx| {
dap_store.shutdown_session(session.read(cx).session_id(), cx)
})
});
shutdown_session.await.unwrap();
main_editor.update(cx, |editor, _| {
let active_debug_lines: Vec<_> = editor.highlighted_rows::<ActiveDebugLine>().collect();
assert!(
active_debug_lines.is_empty(),
"There shouldn't be any active debug lines after session shutdown"
);
});
second_editor.update(cx, |editor, _| {
let active_debug_lines: Vec<_> = editor.highlighted_rows::<ActiveDebugLine>().collect();
assert!(
active_debug_lines.is_empty(),
"There shouldn't be any active debug lines after session shutdown"
);
});
}

4
crates/debugger_ui/src/tests/stack_frame_list.rs
View file

@ -362,7 +362,7 @@ async fn test_select_stack_frame(executor: BackgroundExecutor, cx: &mut TestAppC
let snapshot = editor.snapshot(window, cx);
editor
.highlighted_rows::<editor::DebugCurrentRowHighlight>()
.highlighted_rows::<editor::ActiveDebugLine>()
.map(|(range, _)| {
let start = range.start.to_point(&snapshot.buffer_snapshot);
let end = range.end.to_point(&snapshot.buffer_snapshot);
@ -432,7 +432,7 @@ async fn test_select_stack_frame(executor: BackgroundExecutor, cx: &mut TestAppC
let snapshot = editor.snapshot(window, cx);
editor
.highlighted_rows::<editor::DebugCurrentRowHighlight>()
.highlighted_rows::<editor::ActiveDebugLine>()
.map(|(range, _)| {
let start = range.start.to_point(&snapshot.buffer_snapshot);
let end = range.end.to_point(&snapshot.buffer_snapshot);

16
crates/editor/src/editor.rs
View file

@ -286,7 +286,7 @@ impl InlayId {
}
}
pub enum DebugCurrentRowHighlight {}
pub enum ActiveDebugLine {}
enum DocumentHighlightRead {}
enum DocumentHighlightWrite {}
enum InputComposition {}
@ -1581,7 +1581,11 @@ impl Editor {
&project.read(cx).breakpoint_store(),
window,
|editor, _, event, window, cx| match event {
BreakpointStoreEvent::ActiveDebugLineChanged => {
BreakpointStoreEvent::ClearDebugLines => {
editor.clear_row_highlights::<ActiveDebugLine>();
editor.refresh_inline_values(cx);
}
BreakpointStoreEvent::SetDebugLine => {
if editor.go_to_active_debug_line(window, cx) {
cx.stop_propagation();
}
@ -16635,7 +16639,7 @@ impl Editor {
let Some(active_stack_frame) = breakpoint_store.read(cx).active_position().cloned()
else {
self.clear_row_highlights::<DebugCurrentRowHighlight>();
self.clear_row_highlights::<ActiveDebugLine>();
return None;
};
@ -16662,8 +16666,8 @@ impl Editor {
let multibuffer_anchor = snapshot.anchor_in_excerpt(id, position)?;
handled = true;
self.clear_row_highlights::<DebugCurrentRowHighlight>();
self.go_to_line::<DebugCurrentRowHighlight>(
self.clear_row_highlights::<ActiveDebugLine>();
self.go_to_line::<ActiveDebugLine>(
multibuffer_anchor,
Some(cx.theme().colors().editor_debugger_active_line_background),
window,
@ -17688,7 +17692,7 @@ impl Editor {
let current_execution_position = self
.highlighted_rows
.get(&TypeId::of::<DebugCurrentRowHighlight>())
.get(&TypeId::of::<ActiveDebugLine>())
.and_then(|lines| lines.last().map(|line| line.range.start));
self.inline_value_cache.refresh_task = cx.spawn(async move |editor, cx| {

22
crates/project/src/debugger/breakpoint_store.rs
View file

@ -111,7 +111,7 @@ enum BreakpointStoreMode {
Remote(RemoteBreakpointStore),
}
#[derive(Clone)]
#[derive(Clone, PartialEq)]
pub struct ActiveStackFrame {
pub session_id: SessionId,
pub thread_id: ThreadId,
@ -521,13 +521,26 @@ impl BreakpointStore {
self.active_stack_frame.take();
}
cx.emit(BreakpointStoreEvent::ActiveDebugLineChanged);
cx.emit(BreakpointStoreEvent::ClearDebugLines);
cx.notify();
}
pub fn set_active_position(&mut self, position: ActiveStackFrame, cx: &mut Context<Self>) {
if self
.active_stack_frame
.as_ref()
.is_some_and(|active_position| active_position == &position)
{
return;
}
if self.active_stack_frame.is_some() {
cx.emit(BreakpointStoreEvent::ClearDebugLines);
}
self.active_stack_frame = Some(position);
cx.emit(BreakpointStoreEvent::ActiveDebugLineChanged);
cx.emit(BreakpointStoreEvent::SetDebugLine);
cx.notify();
}
@ -693,7 +706,8 @@ pub enum BreakpointUpdatedReason {
}
pub enum BreakpointStoreEvent {
ActiveDebugLineChanged,
SetDebugLine,
ClearDebugLines,
BreakpointsUpdated(Arc<Path>, BreakpointUpdatedReason),
BreakpointsCleared(Vec<Arc<Path>>),
}

3
crates/project/src/debugger/session.rs
View file

@ -652,7 +652,7 @@ impl Session {
local.unset_breakpoints_from_paths(paths, cx).detach();
}
}
BreakpointStoreEvent::ActiveDebugLineChanged => {}
BreakpointStoreEvent::SetDebugLine | BreakpointStoreEvent::ClearDebugLines => {}
})
.detach();
cx.on_app_quit(Self::on_app_quit).detach();
@ -1809,6 +1809,7 @@ impl Session {
.insert(variables_reference, variables.clone());
cx.emit(SessionEvent::Variables);
cx.emit(SessionEvent::InvalidateInlineValue);
Some(variables)
},
cx,

2
crates/workspace/src/workspace.rs
View file

@ -998,7 +998,7 @@ impl Workspace {
| BreakpointStoreEvent::BreakpointsCleared(_) => {
workspace.serialize_workspace(window, cx);
}
BreakpointStoreEvent::ActiveDebugLineChanged => {}
BreakpointStoreEvent::SetDebugLine | BreakpointStoreEvent::ClearDebugLines => {}
},
)
.detach();