Canonicalise URL IDs on all platforms

crates/ark/src/console.rs

@@ -94,7 +94,6 @@ use serde_json::json;
 use stdext::result::ResultExt;
 use stdext::*;
 use tokio::sync::mpsc::UnboundedReceiver as AsyncUnboundedReceiver;
-use url::Url;
 use uuid::Uuid;
 
 use crate::console_annotate::annotate_input;
@@ -136,7 +135,7 @@ use crate::startup;
 use crate::sys::console::console_to_utf8;
 use crate::ui::UiCommMessage;
 use crate::ui::UiCommSender;
-use crate::url::ExtUrl;
+use crate::url::UrlId;
 
 static RE_DEBUG_PROMPT: Lazy<Regex> = Lazy::new(|| Regex::new(r"Browse\[\d+\]").unwrap());
 
@@ -181,7 +180,7 @@ pub enum DebugStoppedReason {
 #[derive(Debug)]
 pub enum ConsoleNotification {
     /// Notification that a document has changed, requiring breakpoint invalidation.
-    DidChangeDocument(Url),
+    DidChangeDocument(UrlId),
 }
 
 // --- Globals ---
@@ -1072,12 +1071,7 @@ impl Console {
             }
         }
 
-        let loc = req.code_location().log_err().flatten().map(|mut loc| {
-            // Normalize URI for Windows compatibility. Positron sends URIs like
-            // `file:///c%3A/...` which do not match DAP's breakpoint path keys.
-            loc.uri = ExtUrl::normalize(loc.uri);
-            loc
-        });
+        let loc = req.code_location().log_err().flatten();
 
         // Return the code to the R console to be evaluated and the corresponding exec count
         (
@@ -1562,11 +1556,7 @@ impl Console {
                 });
 
                 // Push execution context to graphics device for plot attribution.
-                // Extract code_location from the execute request for source file origin.
-                let code_location = exec_req.code_location().log_err().flatten().map(|mut loc| {
-                    loc.uri = ExtUrl::normalize(loc.uri);
-                    loc
-                });
+                let code_location = exec_req.code_location().log_err().flatten();
                 graphics_device::on_execute_request(
                     originator.header.msg_id.clone(),
                     exec_req.code.clone(),
@@ -1596,10 +1586,11 @@ impl Console {
 
                 // Keep the DAP lock while we are updating breakpoints
                 let mut dap_guard = self.debug_dap.lock().unwrap();
-                let uri = loc.as_ref().map(|l| l.uri.clone());
-                let breakpoints = uri
+
+                let uri_id = loc.as_ref().map(UrlId::from_code_location);
+                let breakpoints = uri_id
                     .as_ref()
-                    .and_then(|uri| dap_guard.breakpoints.get_mut(uri))
+                    .and_then(|uri_id| dap_guard.breakpoints.get_mut(uri_id))
                     .map(|(_, v)| v.as_mut_slice());
 
                 match PendingInputs::read(&code, loc, breakpoints) {
@@ -1618,9 +1609,9 @@ impl Console {
 
                 // Notify frontend about any breakpoints marked invalid during annotation.
                 // Remove disabled breakpoints.
-                if let Some(uri) = &uri {
-                    dap_guard.notify_invalid_breakpoints(uri);
-                    dap_guard.remove_disabled_breakpoints(uri);
+                if let Some(uri_id) = &uri_id {
+                    dap_guard.notify_invalid_breakpoints(uri_id);
+                    dap_guard.remove_disabled_breakpoints(uri_id);
                 }
 
                 drop(dap_guard);

crates/ark/src/console_annotate.rs

@@ -26,6 +26,7 @@ use crate::console::Console;
 use crate::dap::dap::Breakpoint;
 use crate::dap::dap::BreakpointState;
 use crate::dap::dap::InvalidReason;
+use crate::url::UrlId;
 
 /// Function name used for auto-stepping over injected calls such as breakpoints
 const AUTO_STEP_FUNCTION: &str = ".ark_auto_step";
@@ -928,14 +929,19 @@ pub unsafe extern "C-unwind" fn ps_annotate_source(
     let code: String = RObject::view(code).try_into()?;
     let with_visible: bool = RObject::view(with_visible).try_into()?;
 
+    // Parse the raw URI as-is for embedding in R code (`#line` directives,
+    // breakpoint calls). We must not use the canonical form here because
+    // R source references may flow back to the frontend, which expects its
+    // own URI representation.
     let uri = Url::parse(&uri)?;
+    let uri_id = UrlId::from_url(uri.clone());
 
     let mut dap_guard = Console::get().debug_dap.lock().unwrap();
 
     // If there are no breakpoints for this file, return NULL to signal no
     // annotation needed. Scope the mutable borrow so we can re-borrow after.
     let annotated = {
-        let Some((_, breakpoints)) = dap_guard.breakpoints.get_mut(&uri) else {
+        let Some((_, breakpoints)) = dap_guard.breakpoints.get_mut(&uri_id) else {
             return Ok(harp::r_null());
         };
         if breakpoints.is_empty() {
@@ -950,12 +956,12 @@ pub unsafe extern "C-unwind" fn ps_annotate_source(
     };
 
     // Notify frontend about any breakpoints marked invalid during annotation
-    dap_guard.notify_invalid_breakpoints(&uri);
+    dap_guard.notify_invalid_breakpoints(&uri_id);
 
     // Remove disabled breakpoints. Their verification state is now stale since
     // they weren't injected during this annotation. If the user re-enables
     // them, they'll be treated as new unverified breakpoints.
-    if let Some((_, breakpoints)) = dap_guard.breakpoints.get_mut(&uri) {
+    if let Some((_, breakpoints)) = dap_guard.breakpoints.get_mut(&uri_id) {
         breakpoints.retain(|bp| !matches!(bp.state, BreakpointState::Disabled));
     }
 

crates/ark/src/console_debug.rs

@@ -18,14 +18,14 @@ use harp::utils::r_is_null;
 use libr::SEXP;
 use regex::Regex;
 use stdext::result::ResultExt;
-use url::Url;
 
 use crate::console::Console;
 use crate::console::DebugCallText;
 use crate::console::DebugStoppedReason;
 use crate::modules::ARK_ENVS;
 use crate::srcref::ark_uri;
 use crate::thread::RThreadSafe;
+use crate::url::UrlId;
 
 #[derive(Debug)]
 pub struct FrameInfo {
@@ -305,7 +305,7 @@ impl Console {
             return;
         }
 
-        let Some(uri) = Url::parse(&filename).warn_on_err() else {
+        let Some(uri) = UrlId::parse(&filename).warn_on_err() else {
             return;
         };
 
@@ -521,7 +521,7 @@ pub unsafe extern "C-unwind" fn ps_is_breakpoint_enabled(
     id: SEXP,
 ) -> anyhow::Result<SEXP> {
     let uri: String = RObject::view(uri).try_into()?;
-    let uri = Url::parse(&uri)?;
+    let uri = UrlId::parse(&uri)?;
 
     let id: String = RObject::view(id).try_into()?;
 
@@ -539,7 +539,7 @@ pub unsafe extern "C-unwind" fn ps_verify_breakpoint(uri: SEXP, id: SEXP) -> any
     let uri: String = RObject::view(uri).try_into()?;
     let id: String = RObject::view(id).try_into()?;
 
-    let Some(uri) = Url::parse(&uri).log_err() else {
+    let Some(uri) = UrlId::parse(&uri).log_err() else {
         return Ok(libr::R_NilValue);
     };
 
@@ -569,7 +569,7 @@ pub unsafe extern "C-unwind" fn ps_verify_breakpoints_range(
     let start_line: i32 = RObject::view(start_line).try_into()?;
     let end_line: i32 = RObject::view(end_line).try_into()?;
 
-    let Some(uri) = Url::parse(&uri).log_err() else {
+    let Some(uri) = UrlId::parse(&uri).log_err() else {
         return Ok(libr::R_NilValue);
     };
 

crates/ark/src/dap/dap.rs

@@ -22,7 +22,6 @@ use harp::environment::R_ENVS;
 use harp::object::RObject;
 use stdext::result::ResultExt;
 use stdext::spawn;
-use url::Url;
 
 use crate::console::ConsoleOutputCapture;
 use crate::console::DebugStoppedReason;
@@ -32,6 +31,7 @@ use crate::dap::dap_variables::object_variable;
 use crate::dap::dap_variables::RVariable;
 use crate::request::RRequest;
 use crate::thread::RThreadSafe;
+use crate::url::UrlId;
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum BreakpointState {
@@ -146,8 +146,8 @@ pub struct Dap {
     /// Current call stack
     pub stack: Option<Vec<FrameInfo>>,
 
-    /// Known breakpoints keyed by URI, with document hash
-    pub breakpoints: HashMap<Url, (blake3::Hash, Vec<Breakpoint>)>,
+    /// Known breakpoints keyed by canonical URI, with document hash
+    pub breakpoints: HashMap<UrlId, (blake3::Hash, Vec<Breakpoint>)>,
 
     /// Filters for enabled condition breakpoints
     pub exception_breakpoint_filters: Vec<String>,
@@ -482,7 +482,7 @@ impl Dap {
     /// Loops over all breakpoints for the URI and verifies any unverified
     /// breakpoints that fall within the range [start_line, end_line).
     /// Sends a `BreakpointVerified` event for each newly verified breakpoint.
-    pub fn verify_breakpoints(&mut self, uri: &Url, start_line: u32, end_line: u32) {
+    pub fn verify_breakpoints(&mut self, uri: &UrlId, start_line: u32, end_line: u32) {
         let Some((_, bp_list)) = self.breakpoints.get_mut(uri) else {
             return;
         };
@@ -522,7 +522,7 @@ impl Dap {
     ///
     /// Finds the breakpoint with the given ID for the URI and marks it as verified
     /// if it was previously unverified. Sends a `BreakpointVerified` event.
-    pub fn verify_breakpoint(&mut self, uri: &Url, id: &str) {
+    pub fn verify_breakpoint(&mut self, uri: &UrlId, id: &str) {
         let Some((_, bp_list)) = self.breakpoints.get_mut(uri) else {
             return;
         };
@@ -550,10 +550,14 @@ impl Dap {
 
     /// Called when a document changes. Removes all breakpoints for the URI
     /// and sends unverified events for each one.
-    pub fn did_change_document(&mut self, uri: &Url) {
+    pub fn did_change_document(&mut self, uri: &UrlId) {
+        log::trace!("DAP: did_change_document for {uri}");
+
         let Some((_, breakpoints)) = self.breakpoints.remove(uri) else {
             return;
         };
+
+        log::trace!("DAP: Removing {} breakpoints for {uri}", breakpoints.len());
         let Some(tx) = &self.backend_events_tx else {
             return;
         };
@@ -571,7 +575,7 @@ impl Dap {
 
     /// Notify the frontend about breakpoints that were marked invalid during annotation.
     /// Sends a `BreakpointState` event with verified=false and a message for each.
-    pub fn notify_invalid_breakpoints(&self, uri: &Url) {
+    pub fn notify_invalid_breakpoints(&self, uri: &UrlId) {
         let Some(tx) = &self.backend_events_tx else {
             return;
         };
@@ -594,14 +598,14 @@ impl Dap {
     }
 
     /// Remove disabled breakpoints for a given URI.
-    pub fn remove_disabled_breakpoints(&mut self, uri: &Url) {
+    pub fn remove_disabled_breakpoints(&mut self, uri: &UrlId) {
         let Some((_, bps)) = self.breakpoints.get_mut(uri) else {
             return;
         };
         bps.retain(|bp| !matches!(bp.state, BreakpointState::Disabled));
     }
 
-    pub(crate) fn is_breakpoint_enabled(&self, uri: &Url, id: String) -> bool {
+    pub(crate) fn is_breakpoint_enabled(&self, uri: &UrlId, id: String) -> bool {
         let Some((_, breakpoints)) = self.breakpoints.get(uri) else {
             return false;
         };
@@ -689,9 +693,14 @@ impl ServerHandler for Dap {
 #[cfg(test)]
 mod tests {
     use crossbeam::channel::unbounded;
+    use url::Url;
 
     use super::*;
 
+    fn url_id(s: &str) -> UrlId {
+        UrlId::from_url(Url::parse(s).unwrap())
+    }
+
     fn create_test_dap() -> (Dap, crossbeam::channel::Receiver<DapBackendEvent>) {
         let (backend_events_tx, backend_events_rx) = unbounded();
         let (r_request_tx, _r_request_rx) = unbounded();
@@ -720,7 +729,7 @@ mod tests {
     fn test_did_change_document_removes_breakpoints() {
         let (mut dap, rx) = create_test_dap();
 
-        let uri = Url::parse("file:///test.R").unwrap();
+        let uri = url_id("file:///test.R");
         let hash = blake3::hash(b"test content");
 
         dap.breakpoints.insert(
@@ -756,7 +765,7 @@ mod tests {
     fn test_did_change_document_no_breakpoints_is_noop() {
         let (mut dap, rx) = create_test_dap();
 
-        let uri = Url::parse("file:///test.R").unwrap();
+        let uri = url_id("file:///test.R");
 
         dap.did_change_document(&uri);
 
@@ -767,8 +776,8 @@ mod tests {
     fn test_did_change_document_only_affects_target_uri() {
         let (mut dap, rx) = create_test_dap();
 
-        let uri1 = Url::parse("file:///test1.R").unwrap();
-        let uri2 = Url::parse("file:///test2.R").unwrap();
+        let uri1 = url_id("file:///test1.R");
+        let uri2 = url_id("file:///test2.R");
         let hash1 = blake3::hash(b"content 1");
         let hash2 = blake3::hash(b"content 2");
 
@@ -824,7 +833,7 @@ mod tests {
             shared_self: None,
         };
 
-        let uri = Url::parse("file:///test.R").unwrap();
+        let uri = url_id("file:///test.R");
         let hash = blake3::hash(b"test content");
 
         dap.breakpoints.insert(

crates/ark/src/dap/dap_server.rs

@@ -49,7 +49,7 @@ use crate::r_task::spawn_idle_any_prompt;
 use crate::request::debug_request_command;
 use crate::request::DebugRequest;
 use crate::request::RRequest;
-use crate::url::ExtUrl;
+use crate::url::UrlId;
 
 const THREAD_ID: i64 = -1;
 
@@ -441,8 +441,7 @@ impl<R: Read, W: Write> DapServer<R, W> {
         // We currently only support "path" URIs as Positron never sends URIs.
         // In principle the DAP frontend can negotiate whether it sends URIs or
         // file paths via the `pathFormat` field of the `Initialize` request.
-        // `ExtUrl::from_file_path` canonicalizes the path to resolve symlinks.
-        let uri = match ExtUrl::from_file_path(path) {
+        let uri = match UrlId::from_file_path(path) {
             Ok(uri) => uri,
             Err(()) => {
                 log::warn!("Can't set breakpoints for non-file path: '{path}'");

crates/ark/src/lsp/state_handlers.rs

@@ -58,7 +58,7 @@ use crate::lsp::main_loop::DidOpenVirtualDocumentParams;
 use crate::lsp::main_loop::LspState;
 use crate::lsp::state::workspace_uris;
 use crate::lsp::state::WorldState;
-use crate::url::ExtUrl;
+use crate::url::UrlId;
 
 // Handlers that mutate the world state
 
@@ -257,10 +257,9 @@ pub(crate) fn did_change(
     lsp::main_loop::index_update(vec![uri.clone()], state.clone());
 
     // Notify console about document change to invalidate breakpoints.
-    // Normalize URI to avoid Windows issues with `file:///c%3A` paths.
     lsp_state
         .console_notification_tx
-        .send(ConsoleNotification::DidChangeDocument(ExtUrl::normalize(
+        .send(ConsoleNotification::DidChangeDocument(UrlId::from_url(
             uri.clone(),
         )))
         .log_err();