Skip to content

Commit ceead15

Browse files
committed
Keep track of verbatim URL for outward DAP messages
1 parent e51205f commit ceead15

6 files changed

Lines changed: 216 additions & 75 deletions

File tree

crates/ark/src/console/console_annotate.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -940,9 +940,10 @@ pub unsafe extern "C-unwind" fn ps_annotate_source(
940940
// If there are no breakpoints for this file, return NULL to signal no
941941
// annotation needed. Scope the mutable borrow so we can re-borrow after.
942942
let annotated = {
943-
let Some((_, breakpoints)) = dap_guard.breakpoints.get_mut(&uri_id) else {
943+
let Some(entry) = dap_guard.breakpoints.get_mut(&uri_id) else {
944944
return Ok(harp::r_null());
945945
};
946+
let breakpoints = &mut entry.breakpoints;
946947
if breakpoints.is_empty() {
947948
return Ok(harp::r_null());
948949
}
@@ -960,8 +961,10 @@ pub unsafe extern "C-unwind" fn ps_annotate_source(
960961
// Remove disabled breakpoints. Their verification state is now stale since
961962
// they weren't injected during this annotation. If the user re-enables
962963
// them, they'll be treated as new unverified breakpoints.
963-
if let Some((_, breakpoints)) = dap_guard.breakpoints.get_mut(&uri_id) {
964-
breakpoints.retain(|bp| !matches!(bp.state, BreakpointState::Disabled));
964+
if let Some(entry) = dap_guard.breakpoints.get_mut(&uri_id) {
965+
entry
966+
.breakpoints
967+
.retain(|bp| !matches!(bp.state, BreakpointState::Disabled));
965968
}
966969

967970
Ok(RObject::from(annotated).sexp)

crates/ark/src/console/console_repl.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1534,7 +1534,7 @@ impl Console {
15341534
let breakpoints = uri_id
15351535
.as_ref()
15361536
.and_then(|uri_id| dap_guard.breakpoints.get_mut(uri_id))
1537-
.map(|(_, v)| v.as_mut_slice());
1537+
.map(|entry| entry.breakpoints.as_mut_slice());
15381538

15391539
match PendingInputs::read(&code, loc, breakpoints) {
15401540
Ok(ParseResult::Success(inputs)) => {

crates/ark/src/dap/dap_jupyter_handler.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -186,12 +186,13 @@ impl DapJupyterHandler {
186186
let breakpoints: Vec<serde_json::Value> = state
187187
.breakpoints
188188
.iter()
189-
.map(|(uri, (_, bps))| {
190-
let source = uri
191-
.as_url()
192-
.to_file_path()
193-
.map_or_else(|_| uri.to_string(), |p| p.to_string_lossy().into_owned());
194-
let source_breakpoints: Vec<serde_json::Value> = bps
189+
.map(|(_, entry)| {
190+
// Echo the verbatim path the frontend sent rather than the
191+
// normalized key, so it sees back its own bytes (e.g. its
192+
// Windows drive-letter casing).
193+
let source = entry.verbatim_path.clone();
194+
let source_breakpoints: Vec<serde_json::Value> = entry
195+
.breakpoints
195196
.iter()
196197
.filter(|bp| !matches!(bp.state, BreakpointState::Disabled))
197198
.map(|bp| {

crates/ark/src/dap/dap_server.rs

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ use stdext::result::ResultExt;
3636
use stdext::spawn;
3737

3838
use super::dap_state::Breakpoint;
39+
use super::dap_state::BreakpointEntry;
3940
use super::dap_state::BreakpointState;
4041
use super::dap_state::Dap;
4142
use super::dap_state::DapBackendEvent;
@@ -293,7 +294,7 @@ impl DapHandler {
293294
// changed after a reconnection, the breakpoints are no longer valid.
294295
let doc_hash = blake3::hash(doc_content.as_bytes());
295296
let doc_changed = match &old_breakpoints {
296-
Some((existing_hash, _)) => existing_hash != &doc_hash,
297+
Some(entry) => entry.hash != doc_hash,
297298
None => true,
298299
};
299300

@@ -322,7 +323,7 @@ impl DapHandler {
322323
log::trace!("DAP: Document unchanged for {uri}, preserving breakpoint states");
323324

324325
// Unwrap Safety: `doc_changed` is false, so `old_breakpoints` is Some
325-
let (_, old_breakpoints) = old_breakpoints.unwrap();
326+
let old_breakpoints = old_breakpoints.unwrap().breakpoints;
326327
// Use original_line for lookup since that's what the frontend sends back
327328
let mut old_by_line: HashMap<u32, Breakpoint> = old_breakpoints
328329
.into_iter()
@@ -426,7 +427,11 @@ impl DapHandler {
426427
})
427428
.collect();
428429

429-
state.breakpoints.insert(uri, (doc_hash, new_breakpoints));
430+
state.breakpoints.insert(uri, BreakpointEntry {
431+
verbatim_path: path.clone(),
432+
hash: doc_hash,
433+
breakpoints: new_breakpoints,
434+
});
430435

431436
drop(state);
432437

@@ -1009,6 +1014,14 @@ fn into_dap_frame(frame: &FrameInfo, fallback_sources: &HashMap<String, String>)
10091014
// Retrieve either `path` or `source_reference` depending on the `source` type.
10101015
// In the `Text` case, a `source_reference` should always exist because we loaded
10111016
// the map with all possible text values in `start_debug()`.
1017+
//
1018+
// We report R's view of the path (the srcref filename), not the verbatim
1019+
// path the frontend sent for a breakpoint. A breakpoint's `verbatim_path` can
1020+
// differ (e.g. a symlinked path that R resolved through `normalizePath()`),
1021+
// and we could match a frame back to it through `BreakpointMap`'s canonical
1022+
// index. But that would only reach files that happen to have a breakpoint,
1023+
// so a frame's path would flip form depending on whether a breakpoint is
1024+
// set. Using R's path keeps every frame consistent regardless.
10121025
let path = match source {
10131026
FrameSource::File(path) => Some(path),
10141027
FrameSource::Text(source) => fallback_sources.get(&source).cloned().or_else(|| {

0 commit comments

Comments
 (0)