diff --git a/crates/ark/src/interface.rs b/crates/ark/src/interface.rs index cba36da06..278d4d158 100644 --- a/crates/ark/src/interface.rs +++ b/crates/ark/src/interface.rs @@ -266,7 +266,6 @@ pub struct RMain { // LSP tokio runtime used to spawn LSP tasks on the executor and the // corresponding backend used to send LSP requests to the frontend. - // Used by R callbacks, like `ps_editor()` for `utils::file.edit()`. // The backend is initialized on LSP start up, and is refreshed after a // frontend reconnect. lsp_runtime: Arc, diff --git a/crates/ark/src/lsp/editor.rs b/crates/ark/src/lsp/editor.rs deleted file mode 100644 index 99c7d3a45..000000000 --- a/crates/ark/src/lsp/editor.rs +++ /dev/null @@ -1,84 +0,0 @@ -// -// editor.rs -// -// Copyright (C) 2022-2024 Posit Software, PBC. All rights reserved. -// -// - -use harp::vector::CharacterVector; -use harp::vector::Vector; -use libr::R_NilValue; -use libr::SEXP; -use stdext::unwrap; -use tower_lsp::lsp_types::ShowDocumentParams; -use tower_lsp::lsp_types::Url; - -use crate::interface::RMain; - -#[harp::register] -unsafe extern "C" fn ps_editor(file: SEXP, _title: SEXP) -> anyhow::Result { - let main = RMain::get(); - let runtime = main.get_lsp_runtime(); - - let backend = unwrap!(main.get_lsp_backend(), None => { - log::error!("Failed to open file. LSP backend has not been initialized."); - return Ok(R_NilValue); - }); - - let files = CharacterVector::new_unchecked(file); - - let mut uris = Vec::new(); - - for file in files.iter() { - let Some(file) = file else { - // `NA_character_` slipped through - continue; - }; - - let uri = Url::from_file_path(&file); - - let uri = unwrap!(uri, Err(_) => { - // The R side of this handles most issues, but we don't want to panic - // if some unknown file path slips through. - // `from_file_path()` doesn't return `Display`able errors, so we - // can't necessarily give a good reason. - log::error!("Can't open file at '{}'.", file); - continue; - }); - - uris.push(uri); - } - - // Spawn a task to open the files in the editor. We use `spawn()` rather - // than `block_on()` because we need `ps_editor()` to return immediately - // to unblock the main R thread. Otherwise, at the `await` points in - // `show_document()`, it is possible for the runtime to execute a different - // async task that requires R (like diagnostics in `enqueue_diagnostics()`), - // which would cause a deadlock if R was being blocked here. - // Using `spawn()` means that `file.edit()` and the `editor` R option hook - // don't actually wait for the file to be fully opened, but that is - // generally ok and is also what RStudio does. - // https://github.com/posit-dev/positron/issues/1885 - for uri in uris.into_iter() { - runtime.spawn(async move { - let result = backend - .client - .show_document(ShowDocumentParams { - uri: uri.clone(), - external: Some(false), - take_focus: Some(true), - selection: None, - }) - .await; - - if let Err(err) = result { - // In the unlikely event that the LSP `client` hasn't been - // initialized yet, or has shut down, we probably don't want - // to crash ark. - log::error!("Failed to open '{uri}' due to {err:?}"); - } - }); - } - - Ok(R_NilValue) -} diff --git a/crates/ark/src/lsp/mod.rs b/crates/ark/src/lsp/mod.rs index 5da28dc4a..4d1f4a8b6 100644 --- a/crates/ark/src/lsp/mod.rs +++ b/crates/ark/src/lsp/mod.rs @@ -12,7 +12,6 @@ pub mod definitions; pub mod diagnostics; pub mod document_context; pub mod documents; -pub mod editor; pub mod encoding; pub mod events; pub mod handler; diff --git a/crates/ark/src/modules/positron/editor.R b/crates/ark/src/modules/positron/editor.R index 543b4893c..12d09515d 100644 --- a/crates/ark/src/modules/positron/editor.R +++ b/crates/ark/src/modules/positron/editor.R @@ -22,7 +22,11 @@ handler_editor <- function(file, title, ..., name = NULL) { ensure_file(file) # Edit those files. - .ps.Call("ps_editor", file, title) + for (f in file) { + # This blocks until a response from the frontend, unlike RStudio which + # uses a fire-and-forget event. This shouldn't cause any issues. + .ps.ui.navigateToFile(f) + } invisible() } diff --git a/crates/ark/src/modules/positron/frontend-methods.R b/crates/ark/src/modules/positron/frontend-methods.R index a38bddcee..a73f32eb7 100644 --- a/crates/ark/src/modules/positron/frontend-methods.R +++ b/crates/ark/src/modules/positron/frontend-methods.R @@ -5,6 +5,8 @@ # # +# TODO: Unexport these methods + #' @export .ps.ui.LastActiveEditorContext <- function() { .ps.Call("ps_ui_last_active_editor_context") @@ -31,7 +33,8 @@ } #' @export -.ps.ui.navigateToFile <- function(file, line, column) { +.ps.ui.navigateToFile <- function(file = character(0), line = -1L, column = -1L) { + file <- normalizePath(file) .ps.Call("ps_ui_navigate_to_file", file, line, column) } diff --git a/crates/ark/src/modules/rstudio/stubs.R b/crates/ark/src/modules/rstudio/stubs.R index d323dad25..552efd79f 100644 --- a/crates/ark/src/modules/rstudio/stubs.R +++ b/crates/ark/src/modules/rstudio/stubs.R @@ -11,7 +11,6 @@ # TODO: support moveCursor argument stopifnot(moveCursor) - file <- normalizePath(file) invisible(.ps.ui.navigateToFile(file, line, column)) }