From fde9cb86f915582f4005972b5483a05eed7528cb Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Wed, 16 Jul 2025 14:25:12 -0700 Subject: [PATCH 1/5] WIP poking around --- crates/ark/src/browser.rs | 2 ++ crates/ark/src/ui/events.rs | 1 + 2 files changed, 3 insertions(+) diff --git a/crates/ark/src/browser.rs b/crates/ark/src/browser.rs index 37cd7f521..e23d37665 100644 --- a/crates/ark/src/browser.rs +++ b/crates/ark/src/browser.rs @@ -53,6 +53,8 @@ unsafe fn ps_browse_url_impl(url: SEXP) -> anyhow::Result { // For all other URLs, create a ShowUrl event and send it to the main // thread; Positron will handle it. + + // @jennybc: This looks an awful lot like ps_ui_show_url(). let params = ShowUrlParams { url }; let event = UiFrontendEvent::ShowUrl(params); diff --git a/crates/ark/src/ui/events.rs b/crates/ark/src/ui/events.rs index ee67f1bc3..4363abb57 100644 --- a/crates/ark/src/ui/events.rs +++ b/crates/ark/src/ui/events.rs @@ -97,6 +97,7 @@ pub unsafe extern "C-unwind" fn ps_ui_set_selection_ranges(ranges: SEXP) -> anyh } #[harp::register] +// we KNOW this is a real URL pub unsafe extern "C-unwind" fn ps_ui_show_url(url: SEXP) -> anyhow::Result { let params = ShowUrlParams { url: RObject::view(url).try_into()?, From 79f2a2599be61e84e0694ac8f5d3d810dfdf2ef9 Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Fri, 18 Jul 2025 13:30:04 -0700 Subject: [PATCH 2/5] Rough in over here in ark --- crates/ark/src/browser.rs | 46 +++++++++++++++++-------------------- crates/ark/src/ui/events.rs | 30 +++++++++++++++++++----- 2 files changed, 45 insertions(+), 31 deletions(-) diff --git a/crates/ark/src/browser.rs b/crates/ark/src/browser.rs index e23d37665..5aadcb395 100644 --- a/crates/ark/src/browser.rs +++ b/crates/ark/src/browser.rs @@ -5,8 +5,6 @@ // // -use amalthea::comm::ui_comm::ShowUrlParams; -use amalthea::comm::ui_comm::UiFrontendEvent; use harp::object::RObject; use libr::Rf_ScalarLogical; use libr::SEXP; @@ -14,6 +12,8 @@ use libr::SEXP; use crate::help::message::HelpEvent; use crate::help::message::ShowHelpUrlParams; use crate::interface::RMain; +use crate::ui::events::send_open_with_system_default_event; +use crate::ui::events::send_show_url_event; #[harp::register] pub unsafe extern "C-unwind" fn ps_browse_url(url: SEXP) -> anyhow::Result { @@ -35,35 +35,31 @@ fn handle_help_url(url: String) -> anyhow::Result<()> { } unsafe fn ps_browse_url_impl(url: SEXP) -> anyhow::Result { - // Extract URL. - let url = RObject::view(url).to::()?; - let _span = tracing::trace_span!("browseURL", url = %url).entered(); + // Extract URL string for analysis + let url_string = RObject::view(url).to::()?; + let _span = tracing::trace_span!("browseURL", url = %url_string).entered(); // Handle help server requests. - if is_help_url(&url) { + if is_help_url(&url_string) { log::trace!("Help is handling URL"); - handle_help_url(url)?; + handle_help_url(url_string)?; return Ok(Rf_ScalarLogical(1)); - } else { - log::trace!("Help is not handling URL"); } - // TODO: What is the right thing to do outside of Positron when - // `options(browser =)` is called? Right now we error. - - // For all other URLs, create a ShowUrl event and send it to the main - // thread; Positron will handle it. - - // @jennybc: This looks an awful lot like ps_ui_show_url(). - let params = ShowUrlParams { url }; - let event = UiFrontendEvent::ShowUrl(params); - - let main = RMain::get(); - let ui_comm_tx = main - .get_ui_comm_tx() - .ok_or_else(|| anyhow::anyhow!("UI comm not connected."))?; - - ui_comm_tx.send_event(event); + // Handle web URLs + if is_web_url(&url_string) { + log::trace!("Handling web URL"); + send_show_url_event(&url_string)?; + return Ok(Rf_ScalarLogical(1)); + } + // This is probably a file path? Send to the front end and ask for system + // default opener. + log::trace!("Treating as file path"); + send_open_with_system_default_event(&url_string)?; Ok(Rf_ScalarLogical(1)) } + +fn is_web_url(url: &str) -> bool { + url.starts_with("http://") || url.starts_with("https://") +} diff --git a/crates/ark/src/ui/events.rs b/crates/ark/src/ui/events.rs index 4363abb57..a31e1c6e8 100644 --- a/crates/ark/src/ui/events.rs +++ b/crates/ark/src/ui/events.rs @@ -96,24 +96,42 @@ pub unsafe extern "C-unwind" fn ps_ui_set_selection_ranges(ranges: SEXP) -> anyh Ok(R_NilValue) } -#[harp::register] -// we KNOW this is a real URL -pub unsafe extern "C-unwind" fn ps_ui_show_url(url: SEXP) -> anyhow::Result { +pub fn send_show_url_event(url: &str) -> anyhow::Result<()> { let params = ShowUrlParams { - url: RObject::view(url).try_into()?, + url: url.to_string(), }; - let event = UiFrontendEvent::ShowUrl(params); let main = RMain::get(); let ui_comm_tx = main .get_ui_comm_tx() - .ok_or_else(|| ui_comm_not_connected("ui_show_url"))?; + .ok_or_else(|| ui_comm_not_connected("show_url"))?; ui_comm_tx.send_event(event); + Ok(()) +} + +#[harp::register] +pub unsafe extern "C-unwind" fn ps_ui_show_url(url: SEXP) -> anyhow::Result { + let url_string = RObject::view(url).to::()?; + send_show_url_event(&url_string)?; Ok(R_NilValue) } +pub fn send_open_with_system_default_event(path: &str) -> anyhow::Result<()> { + // For now, we'll use a placeholder event type + // This will be replaced when ui_comm.rs is regenerated + let _params = serde_json::json!({ + "path": path + }); + + // TODO: Replace with proper OpenWithSystemDefault event after regeneration + log::info!("Would send OpenWithSystemDefault event for path: {}", path); + + // Temporary: just return Ok for now until the event type exists + Ok(()) +} + pub fn ps_ui_robj_as_ranges(ranges: SEXP) -> anyhow::Result> { let ranges_as_r_objects: Vec = RObject::view(ranges).try_into()?; let ranges_as_result: Result>, _> = ranges_as_r_objects From 1041ba00260526e6bee2d0d0bcfda3526326a145 Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Fri, 18 Jul 2025 18:24:25 -0700 Subject: [PATCH 3/5] Regenerate UI comm; actually implement the event --- crates/amalthea/src/comm/ui_comm.rs | 11 +++++++++++ crates/ark/src/ui/events.rs | 18 ++++++++++-------- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/crates/amalthea/src/comm/ui_comm.rs b/crates/amalthea/src/comm/ui_comm.rs index 0aa195569..01a1256b9 100644 --- a/crates/amalthea/src/comm/ui_comm.rs +++ b/crates/amalthea/src/comm/ui_comm.rs @@ -292,6 +292,13 @@ pub struct ShowHtmlFileParams { pub height: i64, } +/// Parameters for the OpenWithSystem method. +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] +pub struct OpenWithSystemParams { + /// The file path to open with the system default application + pub path: String, +} + /** * Backend RPC request types for the ui comm */ @@ -502,6 +509,10 @@ pub enum UiFrontendEvent { #[serde(rename = "show_html_file")] ShowHtmlFile(ShowHtmlFileParams), + /// Open a file or folder with the system default application + #[serde(rename = "open_with_system")] + OpenWithSystem(OpenWithSystemParams), + /// This event is used to signal that the stored messages the front-end /// replays when constructing multi-output plots should be reset. This /// happens for things like a holoviews extension being changed. diff --git a/crates/ark/src/ui/events.rs b/crates/ark/src/ui/events.rs index a31e1c6e8..220c4385b 100644 --- a/crates/ark/src/ui/events.rs +++ b/crates/ark/src/ui/events.rs @@ -6,6 +6,7 @@ // use amalthea::comm::ui_comm::OpenEditorParams; +use amalthea::comm::ui_comm::OpenWithSystemParams; use amalthea::comm::ui_comm::OpenWorkspaceParams; use amalthea::comm::ui_comm::Position; use amalthea::comm::ui_comm::Range; @@ -119,16 +120,17 @@ pub unsafe extern "C-unwind" fn ps_ui_show_url(url: SEXP) -> anyhow::Result anyhow::Result<()> { - // For now, we'll use a placeholder event type - // This will be replaced when ui_comm.rs is regenerated - let _params = serde_json::json!({ - "path": path - }); + let params = OpenWithSystemParams { + path: path.to_string(), + }; + let event = UiFrontendEvent::OpenWithSystem(params); - // TODO: Replace with proper OpenWithSystemDefault event after regeneration - log::info!("Would send OpenWithSystemDefault event for path: {}", path); + let main = RMain::get(); + let ui_comm_tx = main + .get_ui_comm_tx() + .ok_or_else(|| ui_comm_not_connected("open_with_system"))?; + ui_comm_tx.send_event(event); - // Temporary: just return Ok for now until the event type exists Ok(()) } From 6747682135d453ed76a37b5b1c0c2ef7496dbfd1 Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Fri, 18 Jul 2025 19:44:43 -0700 Subject: [PATCH 4/5] Normalize on this side In particular, I'm thinking of expanding `~`, i.e. the home directory --- crates/ark/src/browser.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/ark/src/browser.rs b/crates/ark/src/browser.rs index 5aadcb395..9aefc7705 100644 --- a/crates/ark/src/browser.rs +++ b/crates/ark/src/browser.rs @@ -6,6 +6,7 @@ // use harp::object::RObject; +use harp::utils::r_normalize_path; use libr::Rf_ScalarLogical; use libr::SEXP; @@ -55,8 +56,9 @@ unsafe fn ps_browse_url_impl(url: SEXP) -> anyhow::Result { // This is probably a file path? Send to the front end and ask for system // default opener. - log::trace!("Treating as file path"); - send_open_with_system_default_event(&url_string)?; + log::trace!("Treating as file path and asking system to open"); + let path = r_normalize_path(url.into())?; + send_open_with_system_default_event(&path)?; Ok(Rf_ScalarLogical(1)) } From d9ec6038dd0d354e50250339cac2e65e7f1e09eb Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Mon, 21 Jul 2025 14:27:40 -0700 Subject: [PATCH 5/5] Align function name with event name --- crates/ark/src/browser.rs | 4 ++-- crates/ark/src/ui/events.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/ark/src/browser.rs b/crates/ark/src/browser.rs index 9aefc7705..f9d6a630f 100644 --- a/crates/ark/src/browser.rs +++ b/crates/ark/src/browser.rs @@ -13,7 +13,7 @@ use libr::SEXP; use crate::help::message::HelpEvent; use crate::help::message::ShowHelpUrlParams; use crate::interface::RMain; -use crate::ui::events::send_open_with_system_default_event; +use crate::ui::events::send_open_with_system_event; use crate::ui::events::send_show_url_event; #[harp::register] @@ -58,7 +58,7 @@ unsafe fn ps_browse_url_impl(url: SEXP) -> anyhow::Result { // default opener. log::trace!("Treating as file path and asking system to open"); let path = r_normalize_path(url.into())?; - send_open_with_system_default_event(&path)?; + send_open_with_system_event(&path)?; Ok(Rf_ScalarLogical(1)) } diff --git a/crates/ark/src/ui/events.rs b/crates/ark/src/ui/events.rs index 220c4385b..97efee245 100644 --- a/crates/ark/src/ui/events.rs +++ b/crates/ark/src/ui/events.rs @@ -119,7 +119,7 @@ pub unsafe extern "C-unwind" fn ps_ui_show_url(url: SEXP) -> anyhow::Result anyhow::Result<()> { +pub fn send_open_with_system_event(path: &str) -> anyhow::Result<()> { let params = OpenWithSystemParams { path: path.to_string(), };