Use new UI event to send a putative path to the system for opening #877
Conversation
In particular, I'm thinking of expanding `~`, i.e. the home directory
| // TODO: What is the right thing to do outside of Positron when | ||
| // `options(browser =)` is called? Right now we error. |
There was a problem hiding this comment.
Does anyone want me to hang onto this comment?
There was a problem hiding this comment.
I think so, we should only inject this option when launched from Positron. Probably an easy fix if you want to do it here?
There was a problem hiding this comment.
I don't really follow how ark would hit ps_browse_url_impl() outside of Positron. Because I assume it works like this:
In Positron, ark takes charge of the "browser" option and that eventually routes through here.
But outside of Positron, ark would presumably have the default implementation of "browser", yeah?
Is the claim that ark would error outside of Positron actually true or true anymore? 🤔
There was a problem hiding this comment.
Apparently I am wrong. Why do we set an option like "browser" in ark, but outside of Positron? This feels like it might be out of scope for this PR (a bigger problem). In which case I should just retain the comment and move on.
~/tmp % jupyter console --kernel=ark
Jupyter console 6.6.3
R version 4.4.2 (2024-10-31) -- "Pile of Leaves"
Copyright (C) 2024 The R Foundation for Statistical Computing
Platform: aarch64-apple-darwin20
R is free software and comes with ABSOLUTELY NO WARRANTY.
You are welcome to redistribute it under certain conditions.
Type 'license()' or 'licence()' for distribution details.
Natural language support but running in an English locale
R is a collaborative project with many contributors.
Type 'contributors()' for more information and
'citation()' on how to cite R or R packages in publications.
Type 'demo()' for some demos, 'help()' for on-line help, or
'help.start()' for an HTML browser interface to help.
Type 'q()' to quit R.
In [1]: browseURL("~/tmp")
2025-07-21T21:35:02.025887Z ERROR No help port is available to check if '~/tmp' is a help url. Is the help comm open?
at crates/ark/src/interface.rs:2032
2025-07-21T21:35:02.026935Z ERROR Failed to browse url due to: UI comm not connected, can't run `open_with_system`.
at crates/ark/src/browser.rs:22
In [2]: getOption("browser")
Out[2]:
function(url) {
.ps.Call("ps_browse_url", as.character(url))
}
<environment: 0x11fd5bc70>
There was a problem hiding this comment.
We are already tracking this problem here: #588.
So I'm going to merge this.
| // For all other URLs, create a ShowUrl event and send it to the main | ||
| // thread; Positron will handle it. | ||
| 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."))?; |
There was a problem hiding this comment.
This code was basically duplicated in ps_ui_show_url() so now they share logic via the send_show_url_event() helper.
| // This is probably a file path? Send to the front end and ask for system | ||
| // default opener. | ||
| log::trace!("Treating as file path and asking system to open"); | ||
| let path = r_normalize_path(url.into())?; |
There was a problem hiding this comment.
The opener service on the Positron side expects a resolved path, so it's important to normalize on the backend.
| // TODO: What is the right thing to do outside of Positron when | ||
| // `options(browser =)` is called? Right now we error. |
There was a problem hiding this comment.
I think so, we should only inject this option when launched from Positron. Probably an easy fix if you want to do it here?
crates/ark/src/ui/events.rs
Outdated
| Ok(R_NilValue) | ||
| } | ||
|
|
||
| pub fn send_open_with_system_default_event(path: &str) -> anyhow::Result<()> { |
There was a problem hiding this comment.
Align with event name?
| pub fn send_open_with_system_default_event(path: &str) -> anyhow::Result<()> { | |
| pub fn send_open_with_system_event(path: &str) -> anyhow::Result<()> { |
There was a problem hiding this comment.
I like it. I'm implementing in a local commit, because I need to use "Rename Symbol" to also hit the call sites.
Addresses posit-dev/positron#5486
Companion to posit-dev/positron#8588
Surfaced by r-lib/usethis#2126