From 47bbc4234c3051d13740bd7264b38c3cf8bb97a2 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Thu, 27 Jul 2023 09:21:26 -0400 Subject: [PATCH] Some fixes to make `edit()` and `file.edit()` more robust (#68) * Make R side of editor handler more robust * Make Rust side of editor handler more robust --- crates/ark/src/lsp/editor.rs | 17 ++++++- crates/ark/src/modules/private/options.R | 35 ++++++++++++++ crates/ark/src/modules/private/tools.R | 58 +++++++++++++++++++++++- crates/ark/src/modules/public/options.R | 12 +---- 4 files changed, 109 insertions(+), 13 deletions(-) create mode 100644 crates/ark/src/modules/private/options.R diff --git a/crates/ark/src/lsp/editor.rs b/crates/ark/src/lsp/editor.rs index e068672d4..e023d22fb 100644 --- a/crates/ark/src/lsp/editor.rs +++ b/crates/ark/src/lsp/editor.rs @@ -8,6 +8,7 @@ use harp::vector::CharacterVector; use harp::vector::Vector; use libR_sys::*; +use stdext::unwrap; use tokio::runtime::Runtime; use tower_lsp::lsp_types::ShowDocumentParams; use tower_lsp::lsp_types::Url; @@ -19,13 +20,25 @@ unsafe extern "C" fn ps_editor(file: SEXP, _title: SEXP) -> SEXP { let rt = Runtime::new().unwrap(); let globals = R_CALLBACK_GLOBALS.as_ref().unwrap(); let files = CharacterVector::new_unchecked(file); + for file in files.iter() { if let Some(file) = file { rt.block_on(async move { + 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); + return; + }); + globals .lsp_client .show_document(ShowDocumentParams { - uri: Url::from_file_path(file).unwrap(), + uri, external: Some(false), take_focus: Some(true), selection: None, @@ -36,5 +49,5 @@ unsafe extern "C" fn ps_editor(file: SEXP, _title: SEXP) -> SEXP { } } - file + R_NilValue } diff --git a/crates/ark/src/modules/private/options.R b/crates/ark/src/modules/private/options.R new file mode 100644 index 000000000..a94998a6f --- /dev/null +++ b/crates/ark/src/modules/private/options.R @@ -0,0 +1,35 @@ +# +# options.R +# +# Copyright (C) 2023 Posit Software, PBC. All rights reserved. +# +# + +handler_editor <- function(file, title, ..., name = NULL) { + # `file.edit()` calls this as `editor(file = file, title = title)` + # `edit()` calls this as `editor(name = name, file = file, title = title)` + + if (!is.null(name)) { + stop("Editing objects is not currently supported.", call. = FALSE) + } + + if (identical(file, "")) { + # i.e. `edit()` with no arguments. Also `file.edit("")`. + # Opens a temporary file for editing. + file <- tempfile(fileext = ".txt") + } + + file <- as.character(file) + title <- as.character(title) + + # Get absolute path to file + file <- normalizePath(file, mustWork = FALSE) + + # Make sure the requested files exist, creating them if they don't + ensure_file(file) + + # Edit those files. + .ps.Call("ps_editor", file, title) + + invisible() +} diff --git a/crates/ark/src/modules/private/tools.R b/crates/ark/src/modules/private/tools.R index 1ca191717..8a22ec997 100644 --- a/crates/ark/src/modules/private/tools.R +++ b/crates/ark/src/modules/private/tools.R @@ -14,13 +14,69 @@ } ensure_directory <- function(path) { - dir.create(path, showWarnings = FALSE, recursive = TRUE) + exists <- dir.exists(path) + + if (all(exists)) { + # Nothing to do if they all exist + return(invisible()) + } + + path <- path[!exists] + + # Try to create missing ones (`dir.create()` isn't vectorized) + for (elt in path) { + dir.create(elt, showWarnings = FALSE, recursive = TRUE) + } + + exists <- dir.exists(path) + + if (all(exists)) { + # Nothing left to do if the missing ones were successfully created + return(invisible()) + } + + path <- path[!exists] + path <- encodeString(path, quote = "\"") + path <- paste0(path, collapse = ", ") + + stop("Can't create the directory at: ", path, call. = FALSE) } ensure_parent_directory <- function(path) { ensure_directory(dirname(path)) } +ensure_file <- function(path) { + exists <- file.exists(path) + + if (all(exists)) { + # All files exist already, nothing to do + return(invisible()) + } + + path <- path[!exists] + + # Create parent directories as needed + ensure_parent_directory(path) + + # Try to create the missing files + file.create(path, showWarnings = FALSE) + + exists <- file.exists(path) + + if (all(exists)) { + # We successfully created the new files and can detect + # their existance + return(invisible()) + } + + path <- path[!exists] + path <- encodeString(path, quote = "\"") + path <- paste0(path, collapse = ", ") + + stop("Can't create the files at: ", path, call. = FALSE) +} + # Checks if a package is installed without loading it. # Could be slow on network drives. is_installed <- function(pkg, minimum_version = NULL) { diff --git a/crates/ark/src/modules/public/options.R b/crates/ark/src/modules/public/options.R index db28e94a8..a3a4b4d2b 100644 --- a/crates/ark/src/modules/public/options.R +++ b/crates/ark/src/modules/public/options.R @@ -12,16 +12,8 @@ options(max.print = 1000) options(help_type = "html") # Use internal editor -options(editor = function(file, title, ...) { - - # Make sure the requested files exist. - file <- as.character(path.expand(file)) - ensure_parent_directory(file) - file.create(file[!file.exists(file)]) - - # Edit those files. - .ps.Call("ps_editor", as.character(file), as.character(title)) - +options(editor = function(file, title, ..., name = NULL) { + handler_editor(file = file, title = title, ..., name = name) }) # Use custom browser implementation