From 44a67411ab8b638c7a75785a7bbb43c4d6ae2122 Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Tue, 29 Apr 2025 10:49:36 -0300 Subject: [PATCH 01/13] Include the extract operator within help topic node --- crates/ark/src/lsp/help_topic.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/ark/src/lsp/help_topic.rs b/crates/ark/src/lsp/help_topic.rs index 978a6f30d..2dd670ec0 100644 --- a/crates/ark/src/lsp/help_topic.rs +++ b/crates/ark/src/lsp/help_topic.rs @@ -89,6 +89,7 @@ fn locate_help_node(tree: &Tree, point: Point) -> Option> { // Even if they are at `p<>kg::fun`, we assume they really want docs for `fun`. let node = match node.parent() { Some(parent) if matches!(parent.node_type(), NodeType::NamespaceOperator(_)) => parent, + Some(parent) if matches!(parent.node_type(), NodeType::ExtractOperator(_)) => parent, Some(_) => node, None => node, }; @@ -138,5 +139,12 @@ mod tests { let node = locate_help_node(&tree, point).unwrap(); let text = node.utf8_text(text.as_bytes()).unwrap(); assert_eq!(text, "dplyr:::across"); + + // R6 methods, or reticulate accessors + let (text, point) = point_from_cursor("tf$a@bs(x)"); + let tree = parser.parse(text.as_str(), None).unwrap(); + let node = locate_help_node(&tree, point).unwrap(); + let text = node.utf8_text(text.as_bytes()).unwrap(); + assert_eq!(text, "tf$abs"); } } From aad273154f4fef823091a5f474e92b734d9c5172 Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Tue, 29 Apr 2025 10:51:03 -0300 Subject: [PATCH 02/13] Allow extensions to register custom help handlers --- crates/ark/src/modules/positron/help.R | 36 +++++++++++++++++++++++ crates/ark/src/modules/positron/methods.R | 3 ++ 2 files changed, 39 insertions(+) diff --git a/crates/ark/src/modules/positron/help.R b/crates/ark/src/modules/positron/help.R index 531c95422..169cd7b7b 100644 --- a/crates/ark/src/modules/positron/help.R +++ b/crates/ark/src/modules/positron/help.R @@ -44,6 +44,37 @@ help <- function(topic, package = NULL) { # found. #' @export .ps.help.showHelpTopic <- function(topic) { + help_handler <- tryCatch( + { + # Before we do anything to find the help page, we evaluate the topic expression + # to see if the object can be found in the current environment and if it has a + # custom help handler (eg. reticulate objects). + object <- eval( + parse(text = topic), + envir = new.env(parent = globalenv()) + ) + # call_ark_method() returns NULL if no method is found for the object + # ark_positron_help_get_handler() must return a function that's called for + # its side effects (potentially showing documentation) and returning `TRUE` + # if it could handle the request. We could also make it + # actually show help imediatelly, but that makes it hard to separate + # non-existant methods, from methods that return `NULL` and actual errors. + # This also allows methods to skip matching objects for which they don't want + # to support, by simply returning a `NULL` handler. + call_ark_method( + "ark_positron_help_get_handler", + object + ) + }, + error = function(e) { + NULL + } + ) + + if (!is.null(help_handler)) { + return(help_handler(topic)) + } + info <- split_topic(topic) topic <- info$topic package <- info$package @@ -230,6 +261,11 @@ getHtmlHelpContentsDevImpl <- function(x) { .ps.Call("ps_browse_url", as.character(url)) } +#' @export +.ps.help.browse_external_url <- function(url) { + .ps.Call("ps_help_browse_external_url", as.character(url)) +} + # @param rd_file Path to an `.Rd` file. # @returns The result of converting that `.Rd` to HTML and concatenating to a # string. diff --git a/crates/ark/src/modules/positron/methods.R b/crates/ark/src/modules/positron/methods.R index f9bd220d2..93288c69d 100644 --- a/crates/ark/src/modules/positron/methods.R +++ b/crates/ark/src/modules/positron/methods.R @@ -25,6 +25,9 @@ ark_methods_table$ark_positron_variable_get_children <- new.env( ark_methods_table$ark_positron_variable_has_viewer <- new.env( parent = emptyenv() ) +ark_methods_table$ark_positron_help_get_handler <- new.env( + parent = emptyenv() +) lockEnvironment(ark_methods_table, TRUE) ark_methods_allowed_packages <- c("torch", "reticulate", "duckplyr") From 9d61aa2c4188e1caccf9c7705ac09eff1bf545b9 Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Tue, 29 Apr 2025 10:51:37 -0300 Subject: [PATCH 03/13] Add entrypoint to allow serving web-pages in the help pane --- crates/ark/src/browser.rs | 6 +++- crates/ark/src/help/message.rs | 7 +++++ crates/ark/src/help/r_help.rs | 53 +++++++++++++++++++++++++--------- 3 files changed, 52 insertions(+), 14 deletions(-) diff --git a/crates/ark/src/browser.rs b/crates/ark/src/browser.rs index f9d6a630f..4de512063 100644 --- a/crates/ark/src/browser.rs +++ b/crates/ark/src/browser.rs @@ -11,6 +11,7 @@ use libr::Rf_ScalarLogical; use libr::SEXP; use crate::help::message::HelpEvent; +use crate::help::message::ShowHelpUrlKind; use crate::help::message::ShowHelpUrlParams; use crate::interface::RMain; use crate::ui::events::send_open_with_system_event; @@ -30,7 +31,10 @@ fn is_help_url(url: &str) -> bool { fn handle_help_url(url: String) -> anyhow::Result<()> { RMain::with(|main| { - let event = HelpEvent::ShowHelpUrl(ShowHelpUrlParams { url }); + let event = HelpEvent::ShowHelpUrl(ShowHelpUrlParams { + url, + kind: ShowHelpUrlKind::HelpProxy, + }); main.send_help_event(event) }) } diff --git a/crates/ark/src/help/message.rs b/crates/ark/src/help/message.rs index fad7d8390..f75cbf14b 100644 --- a/crates/ark/src/help/message.rs +++ b/crates/ark/src/help/message.rs @@ -15,10 +15,17 @@ pub enum HelpEvent { ShowHelpUrl(ShowHelpUrlParams), } +#[derive(Debug)] +pub enum ShowHelpUrlKind { + HelpProxy, + External, +} + #[derive(Debug)] pub struct ShowHelpUrlParams { /// Url to attempt to show. pub url: String, + pub kind: ShowHelpUrlKind, } impl std::fmt::Display for HelpEvent { diff --git a/crates/ark/src/help/r_help.rs b/crates/ark/src/help/r_help.rs index f4bb0ccd4..959578ceb 100644 --- a/crates/ark/src/help/r_help.rs +++ b/crates/ark/src/help/r_help.rs @@ -18,13 +18,18 @@ use crossbeam::channel::Sender; use crossbeam::select; use harp::exec::RFunction; use harp::exec::RFunctionExt; +use harp::RObject; +use libr::R_NilValue; +use libr::SEXP; use log::info; use log::trace; use log::warn; use stdext::spawn; use crate::help::message::HelpEvent; +use crate::help::message::ShowHelpUrlKind; use crate::help::message::ShowHelpUrlParams; +use crate::interface::RMain; use crate::r_task; /** @@ -182,27 +187,37 @@ impl RHelp { /// coming through here has already been verified to look like a help URL with /// `is_help_url()`, so if we get an unexpected prefix, that's an error. fn handle_show_help_url(&self, params: ShowHelpUrlParams) -> anyhow::Result<()> { - let url = params.url; + let url = params.url.clone(); - if !Self::is_help_url(url.as_str(), self.r_port) { - let prefix = Self::help_url_prefix(self.r_port); - return Err(anyhow!( - "Help URL '{url}' doesn't have expected prefix '{prefix}'." - )); - } + let url = match params.kind { + ShowHelpUrlKind::HelpProxy => { + if !Self::is_help_url(url.as_str(), self.r_port) { + let prefix = Self::help_url_prefix(self.r_port); + return Err(anyhow!( + "Help URL '{url}' doesn't have expected prefix '{prefix}'." + )); + } - // Re-direct the help event to our help proxy server. - let r_prefix = Self::help_url_prefix(self.r_port); - let proxy_prefix = Self::help_url_prefix(self.proxy_port); + // Re-direct the help event to our help proxy server. + let r_prefix = Self::help_url_prefix(self.r_port); + let proxy_prefix = Self::help_url_prefix(self.proxy_port); - let proxy_url = url.replace(r_prefix.as_str(), proxy_prefix.as_str()); + url.replace(r_prefix.as_str(), proxy_prefix.as_str()) + }, + ShowHelpUrlKind::External => { + // The URL is not a help URL; just use it as-is. + url + }, + }; log::trace!( - "Sending frontend event `ShowHelp` with R url '{url}' and proxy url '{proxy_url}'" + "Sending frontend event `ShowHelp` with R url '{}' and proxy url '{}'", + params.url, + url ); let msg = HelpFrontendEvent::ShowHelp(ShowHelpParams { - content: proxy_url, + content: url, kind: ShowHelpKind::Url, focus: true, }); @@ -232,3 +247,15 @@ impl RHelp { .and_then(|x| x.try_into()) } } + +#[harp::register] +pub unsafe extern "C-unwind" fn ps_help_browse_external_url( + url: SEXP, +) -> Result { + RMain::get().send_help_event(HelpEvent::ShowHelpUrl(ShowHelpUrlParams { + url: RObject::view(url).to::()?, + kind: ShowHelpUrlKind::External, + }))?; + + Ok(R_NilValue) +} From a92d71fec28518751789093e4ab7b5d3b876f9f3 Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Thu, 28 Aug 2025 09:29:41 -0300 Subject: [PATCH 04/13] move implementation to rust and use debug_env() --- crates/ark/src/help/r_help.rs | 39 ++++++++++++++++++++++++++ crates/ark/src/methods.rs | 3 ++ crates/ark/src/modules/positron/help.R | 31 -------------------- 3 files changed, 42 insertions(+), 31 deletions(-) diff --git a/crates/ark/src/help/r_help.rs b/crates/ark/src/help/r_help.rs index 959578ceb..0bf69f357 100644 --- a/crates/ark/src/help/r_help.rs +++ b/crates/ark/src/help/r_help.rs @@ -19,6 +19,7 @@ use crossbeam::select; use harp::exec::RFunction; use harp::exec::RFunctionExt; use harp::RObject; +use libr::R_GlobalEnv; use libr::R_NilValue; use libr::SEXP; use log::info; @@ -30,6 +31,7 @@ use crate::help::message::HelpEvent; use crate::help::message::ShowHelpUrlKind; use crate::help::message::ShowHelpUrlParams; use crate::interface::RMain; +use crate::methods::ArkGenerics; use crate::r_task; /** @@ -231,6 +233,10 @@ impl RHelp { #[tracing::instrument(level = "trace", skip(self))] fn show_help_topic(&self, topic: String) -> anyhow::Result { let found = r_task(|| unsafe { + if let Ok(Some(result)) = Self::r_help_handler(topic.clone()) { + return Ok(result); + } + RFunction::from(".ps.help.showHelpTopic") .add(topic) .call()? @@ -239,6 +245,39 @@ impl RHelp { Ok(found) } + fn r_help_handler(_topic: String) -> anyhow::Result> { + unsafe { + let mut env = R_GlobalEnv; + + #[cfg(not(test))] + { + if let Some(debug_env) = &RMain::get().debug_env() { + // Mem-Safety: Object protected by `RMain` for the duration of the `r_task()` + env = debug_env.sexp; + } + } + + let obj = harp::parse_eval0(_topic.as_str(), env)?; + let handler: Option = + ArkGenerics::HelpGetHandler.try_dispatch(obj.sexp, vec![])?; + + if let Some(handler) = handler { + let mut fun = RFunction::new_inlined(handler); + match fun.call_in(env) { + Err(err) => { + log::error!("Error calling help handler: {:?}", err); + return Err(anyhow!("Error calling help handler: {:?}", err)); + }, + Ok(result) => { + return Ok(Some(result.try_into()?)); + }, + } + } + } + + Ok(None) + } + pub fn r_start_or_reconnect_to_help_server() -> harp::Result { // Start the R help server. // If it is already started, it just returns the preexisting port number. diff --git a/crates/ark/src/methods.rs b/crates/ark/src/methods.rs index f4d859700..8f9f22f8b 100644 --- a/crates/ark/src/methods.rs +++ b/crates/ark/src/methods.rs @@ -42,6 +42,9 @@ pub enum ArkGenerics { #[strum(serialize = "ark_positron_variable_has_viewer")] VariableHasViewer, + + #[strum(serialize = "ark_positron_help_get_handler")] + HelpGetHandler, } impl ArkGenerics { diff --git a/crates/ark/src/modules/positron/help.R b/crates/ark/src/modules/positron/help.R index 169cd7b7b..c0fd46ff5 100644 --- a/crates/ark/src/modules/positron/help.R +++ b/crates/ark/src/modules/positron/help.R @@ -44,37 +44,6 @@ help <- function(topic, package = NULL) { # found. #' @export .ps.help.showHelpTopic <- function(topic) { - help_handler <- tryCatch( - { - # Before we do anything to find the help page, we evaluate the topic expression - # to see if the object can be found in the current environment and if it has a - # custom help handler (eg. reticulate objects). - object <- eval( - parse(text = topic), - envir = new.env(parent = globalenv()) - ) - # call_ark_method() returns NULL if no method is found for the object - # ark_positron_help_get_handler() must return a function that's called for - # its side effects (potentially showing documentation) and returning `TRUE` - # if it could handle the request. We could also make it - # actually show help imediatelly, but that makes it hard to separate - # non-existant methods, from methods that return `NULL` and actual errors. - # This also allows methods to skip matching objects for which they don't want - # to support, by simply returning a `NULL` handler. - call_ark_method( - "ark_positron_help_get_handler", - object - ) - }, - error = function(e) { - NULL - } - ) - - if (!is.null(help_handler)) { - return(help_handler(topic)) - } - info <- split_topic(topic) topic <- info$topic package <- info$package From 34f9cc9eabf8ccfb4dd29fe93a3caab479471627 Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Thu, 28 Aug 2025 10:07:40 -0300 Subject: [PATCH 05/13] add documentation for ark methods --- crates/ark/src/modules/positron/methods.R | 68 ++++++++++++++++++++++- 1 file changed, 66 insertions(+), 2 deletions(-) diff --git a/crates/ark/src/modules/positron/methods.R b/crates/ark/src/modules/positron/methods.R index 93288c69d..5c9a4337d 100644 --- a/crates/ark/src/modules/positron/methods.R +++ b/crates/ark/src/modules/positron/methods.R @@ -6,25 +6,89 @@ # ark_methods_table <- new.env(parent = emptyenv()) + +#' Customize display value for objects in Variables Pane +#' +#' @param x Object to get the display value for +#' @param ... Additional arguments (unused) +#' @param width Maximum expected width. This is just a suggestion, the UI +#' can still truncate the string to different widths. +#' @return A length 1 character vector containing the display value ark_methods_table$ark_positron_variable_display_value <- new.env( parent = emptyenv() ) + +#' Customize display type for objects in Variables Pane +#' +#' @param x Object to get the display type for +#' @param ... Additional arguments (unused) +#' @param include_length Boolean indicating whether to include object length +#' @return A length 1 character vector describing the object type ark_methods_table$ark_positron_variable_display_type <- new.env( parent = emptyenv() ) + +#' Check if object has inspectable children in Variables Pane +#' +#' @param x Object to check for children +#' @param ... Additional arguments (unused) +#' @return Logical value: TRUE if the object can be inspected, FALSE otherwise ark_methods_table$ark_positron_variable_has_children <- new.env( parent = emptyenv() ) + +#' Specify variable kind for Variables Pane organization +#' +#' @param x Object to get the variable kind for +#' @param ... Additional arguments (unused) +#' @return Length 1 character vector specifying the kind of variable (e.g., "table", "other") +#' See the `pub enum VariableKind` for all accepted types. ark_methods_table$ark_positron_variable_kind <- new.env(parent = emptyenv()) + +#' Get specific child element from object for Variables Pane inspection +#' +#' @param x Object to get child from +#' @param ... Additional arguments (unused) +#' @param index Integer > 1, representing the index position of the child +#' @param name Character string or NULL, the name of the child +#' @return The child object at the specified index/name ark_methods_table$ark_positron_variable_get_child_at <- new.env( parent = emptyenv() ) -ark_methods_table$ark_positron_variable_get_children <- new.env( + +#' Control viewer availability for objects in Variables Pane +#' +#' @param x Object to check for viewer support +#' @param ... Additional arguments (unused) +#' @return Logical value: TRUE if viewer should be enabled, FALSE to disable +ark_methods_table$ark_positron_variable_has_viewer <- new.env( parent = emptyenv() ) -ark_methods_table$ark_positron_variable_has_viewer <- new.env( + +#' Get child objects for Variables Pane inspection +#' +#' @param x Object to get children from +#' @param ... Additional arguments (unused) +#' @return Named list of child objects to be displayed. +#' The above methods are called in the elements of this list to make the display +#' of child objects consistent. +ark_methods_table$ark_positron_variable_get_children <- new.env( parent = emptyenv() ) + +#' Get the help handler for an R object +#' +#' @param obj An R object to obtain the help handler for. +#' +#' @returns Returns a help handler or `NULL` if +#' the object can't be handled. +#' +#' The returned help handler is a function with no arguments that is expected to +#' show the help documentation for the object as a side effect and return +#' `TRUE` if it was able to do so, or `FALSE` otherwise. +#' +#' It may use e.g `.ps.help.browse_external_url` to display a URL +#' in the help pane. ark_methods_table$ark_positron_help_get_handler <- new.env( parent = emptyenv() ) From f7145582b11acd7ba79f86d0a1317658c3a6d829 Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Thu, 28 Aug 2025 10:12:49 -0300 Subject: [PATCH 06/13] make env immutable --- crates/ark/src/help/r_help.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/crates/ark/src/help/r_help.rs b/crates/ark/src/help/r_help.rs index 0bf69f357..24bf59f94 100644 --- a/crates/ark/src/help/r_help.rs +++ b/crates/ark/src/help/r_help.rs @@ -245,17 +245,19 @@ impl RHelp { Ok(found) } + // Must be called in a `r_task` context. fn r_help_handler(_topic: String) -> anyhow::Result> { unsafe { - let mut env = R_GlobalEnv; - - #[cfg(not(test))] - { - if let Some(debug_env) = &RMain::get().debug_env() { - // Mem-Safety: Object protected by `RMain` for the duration of the `r_task()` - env = debug_env.sexp; + let env = (|| { + #[cfg(not(test))] + { + if let Some(debug_env) = &RMain::get().debug_env() { + // Mem-Safety: Object protected by `RMain` for the duration of the `r_task()` + return debug_env.sexp; + } } - } + R_GlobalEnv + })(); let obj = harp::parse_eval0(_topic.as_str(), env)?; let handler: Option = From 3df31f5cd5e8e15493432ee43bb546037fb9a579 Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Thu, 28 Aug 2025 11:11:46 -0300 Subject: [PATCH 07/13] Can simplify slightly --- crates/ark/src/help/r_help.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/crates/ark/src/help/r_help.rs b/crates/ark/src/help/r_help.rs index 24bf59f94..e8cbc3982 100644 --- a/crates/ark/src/help/r_help.rs +++ b/crates/ark/src/help/r_help.rs @@ -250,11 +250,9 @@ impl RHelp { unsafe { let env = (|| { #[cfg(not(test))] - { - if let Some(debug_env) = &RMain::get().debug_env() { - // Mem-Safety: Object protected by `RMain` for the duration of the `r_task()` - return debug_env.sexp; - } + if let Some(debug_env) = &RMain::get().debug_env() { + // Mem-Safety: Object protected by `RMain` for the duration of the `r_task()` + return debug_env.sexp; } R_GlobalEnv })(); From de0959ccbbd02a1710172f6ce8ce8a726e03bc99 Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Thu, 28 Aug 2025 12:09:15 -0300 Subject: [PATCH 08/13] check RMain initialized --- crates/ark/src/help/r_help.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/ark/src/help/r_help.rs b/crates/ark/src/help/r_help.rs index e8cbc3982..49044140d 100644 --- a/crates/ark/src/help/r_help.rs +++ b/crates/ark/src/help/r_help.rs @@ -250,10 +250,13 @@ impl RHelp { unsafe { let env = (|| { #[cfg(not(test))] - if let Some(debug_env) = &RMain::get().debug_env() { - // Mem-Safety: Object protected by `RMain` for the duration of the `r_task()` - return debug_env.sexp; + if RMain::is_initialized() { + if let Some(debug_env) = &RMain::get().debug_env() { + // Mem-Safety: Object protected by `RMain` for the duration of the `r_task()` + return debug_env.sexp; + } } + R_GlobalEnv })(); From 31515dddfe33ec19d346e63fbb6bdf18b7e7217c Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Tue, 11 Nov 2025 13:48:28 -0300 Subject: [PATCH 09/13] Rename topic --- crates/ark/src/help/r_help.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ark/src/help/r_help.rs b/crates/ark/src/help/r_help.rs index 49044140d..52b1f40aa 100644 --- a/crates/ark/src/help/r_help.rs +++ b/crates/ark/src/help/r_help.rs @@ -246,7 +246,7 @@ impl RHelp { } // Must be called in a `r_task` context. - fn r_help_handler(_topic: String) -> anyhow::Result> { + fn r_help_handler(topic: String) -> anyhow::Result> { unsafe { let env = (|| { #[cfg(not(test))] @@ -260,7 +260,7 @@ impl RHelp { R_GlobalEnv })(); - let obj = harp::parse_eval0(_topic.as_str(), env)?; + let obj = harp::parse_eval0(topic.as_str(), env)?; let handler: Option = ArkGenerics::HelpGetHandler.try_dispatch(obj.sexp, vec![])?; From 03039fec854f77e6ef1a688b5cee20ab36d376f6 Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Wed, 12 Nov 2025 14:38:17 -0300 Subject: [PATCH 10/13] don't fallback when the help topic is clearly an expression --- crates/ark/src/help/r_help.rs | 55 +++++++++++++++++++++++++++++------ 1 file changed, 46 insertions(+), 9 deletions(-) diff --git a/crates/ark/src/help/r_help.rs b/crates/ark/src/help/r_help.rs index 52b1f40aa..002910792 100644 --- a/crates/ark/src/help/r_help.rs +++ b/crates/ark/src/help/r_help.rs @@ -232,16 +232,35 @@ impl RHelp { #[tracing::instrument(level = "trace", skip(self))] fn show_help_topic(&self, topic: String) -> anyhow::Result { - let found = r_task(|| unsafe { - if let Ok(Some(result)) = Self::r_help_handler(topic.clone()) { - return Ok(result); - } + let topic = HelpTopic::parse(topic); + + let found = match topic { + HelpTopic::Simple(symbol) => r_task(|| unsafe { + // Try evaluating the help handler first and then fall back to + // the default help topic display function. + + if let Ok(Some(result)) = Self::r_help_handler(symbol.clone()) { + return Ok(result); + } + + RFunction::from(".ps.help.showHelpTopic") + .add(symbol) + .call()? + .to::() + }), + HelpTopic::Expression(expression) => { + // For expressions, we have to use the help handler + // If that fails there's no fallback. + r_task(|| match Self::r_help_handler(expression) { + Ok(Some(result)) => Ok(result), + // No method found + Ok(None) => Ok(false), + // Error during evaluation + Err(err) => Err(harp::Error::Anyhow(err)), + }) + }, + }?; - RFunction::from(".ps.help.showHelpTopic") - .add(topic) - .call()? - .to::() - })?; Ok(found) } @@ -290,6 +309,24 @@ impl RHelp { } } +enum HelpTopic { + // no obvious expression syntax — e.g. "abs", "base::abs" + Simple(String), + // contains expression syntax — e.g. "tensorflow::tf$abs", "model@coef" + // such that there will never exist a help topic with that name + Expression(String), +} + +impl HelpTopic { + pub fn parse(topic: String) -> Self { + if topic.contains('$') || topic.contains('@') { + Self::Expression(topic) + } else { + Self::Simple(topic) + } + } +} + #[harp::register] pub unsafe extern "C-unwind" fn ps_help_browse_external_url( url: SEXP, From 695498ee4c9ea3f401931caba78e43e14786c12c Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Wed, 12 Nov 2025 14:45:27 -0300 Subject: [PATCH 11/13] rename making it clear this is not the default approach --- crates/ark/src/help/r_help.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/ark/src/help/r_help.rs b/crates/ark/src/help/r_help.rs index 002910792..3bff54d39 100644 --- a/crates/ark/src/help/r_help.rs +++ b/crates/ark/src/help/r_help.rs @@ -239,7 +239,7 @@ impl RHelp { // Try evaluating the help handler first and then fall back to // the default help topic display function. - if let Ok(Some(result)) = Self::r_help_handler(symbol.clone()) { + if let Ok(Some(result)) = Self::r_custom_help_handler(symbol.clone()) { return Ok(result); } @@ -251,7 +251,7 @@ impl RHelp { HelpTopic::Expression(expression) => { // For expressions, we have to use the help handler // If that fails there's no fallback. - r_task(|| match Self::r_help_handler(expression) { + r_task(|| match Self::r_custom_help_handler(expression) { Ok(Some(result)) => Ok(result), // No method found Ok(None) => Ok(false), @@ -265,7 +265,8 @@ impl RHelp { } // Must be called in a `r_task` context. - fn r_help_handler(topic: String) -> anyhow::Result> { + // Tries calling a custom help handler defined as an ark method. + fn r_custom_help_handler(topic: String) -> anyhow::Result> { unsafe { let env = (|| { #[cfg(not(test))] From 9ab8106f665e71dc8a3395da46e033b6bef3434b Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Wed, 12 Nov 2025 14:57:15 -0300 Subject: [PATCH 12/13] Add one test case and simplify --- crates/ark/src/lsp/help_topic.rs | 60 +++++++++++++------------------- 1 file changed, 25 insertions(+), 35 deletions(-) diff --git a/crates/ark/src/lsp/help_topic.rs b/crates/ark/src/lsp/help_topic.rs index 2dd670ec0..024091257 100644 --- a/crates/ark/src/lsp/help_topic.rs +++ b/crates/ark/src/lsp/help_topic.rs @@ -111,40 +111,30 @@ mod tests { .set_language(&tree_sitter_r::LANGUAGE.into()) .expect("failed to create parser"); - // On the RHS - let (text, point) = point_from_cursor("dplyr::ac@ross(x:y, sum)"); - let tree = parser.parse(text.as_str(), None).unwrap(); - let node = locate_help_node(&tree, point).unwrap(); - let text = node.utf8_text(text.as_bytes()).unwrap(); - assert_eq!(text, "dplyr::across"); - - // On the LHS (Returns function help for `across()`, not package help for `dplyr`, - // as we assume that is more useful for the user). - let (text, point) = point_from_cursor("dpl@yr::across(x:y, sum)"); - let tree = parser.parse(text.as_str(), None).unwrap(); - let node = locate_help_node(&tree, point).unwrap(); - let text = node.utf8_text(text.as_bytes()).unwrap(); - assert_eq!(text, "dplyr::across"); - - // In the operator - let (text, point) = point_from_cursor("dplyr:@:across(x:y, sum)"); - let tree = parser.parse(text.as_str(), None).unwrap(); - let node = locate_help_node(&tree, point).unwrap(); - let text = node.utf8_text(text.as_bytes()).unwrap(); - assert_eq!(text, "dplyr::across"); - - // Internal `:::` - let (text, point) = point_from_cursor("dplyr:::ac@ross(x:y, sum)"); - let tree = parser.parse(text.as_str(), None).unwrap(); - let node = locate_help_node(&tree, point).unwrap(); - let text = node.utf8_text(text.as_bytes()).unwrap(); - assert_eq!(text, "dplyr:::across"); - - // R6 methods, or reticulate accessors - let (text, point) = point_from_cursor("tf$a@bs(x)"); - let tree = parser.parse(text.as_str(), None).unwrap(); - let node = locate_help_node(&tree, point).unwrap(); - let text = node.utf8_text(text.as_bytes()).unwrap(); - assert_eq!(text, "tf$abs"); + // (text cursor, expected help topic) + let cases = vec![ + // On the RHS + ("dplyr::ac@ross(x:y, sum)", "dplyr::across"), + // On the LHS (Returns function help for `across()`, not package help for `dplyr`, + // as we assume that is more useful for the user). + ("dpl@yr::across(x:y, sum)", "dplyr::across"), + // In the operator + ("dplyr:@:across(x:y, sum)", "dplyr::across"), + // Internal `:::` + ("dplyr:::ac@ross(x:y, sum)", "dplyr:::across"), + // R6 methods, or reticulate accessors + ("tf$a@bs(x)", "tf$abs"), + ("t@f$abs(x)", "tf$abs"), + // With the package namespace + ("tensorflow::tf$ab@s(x)", "tensorflow::tf$abs"), + ]; + + for (code, expected) in cases { + let (text, point) = point_from_cursor(code); + let tree = parser.parse(text.as_str(), None).unwrap(); + let node = locate_help_node(&tree, point).unwrap(); + let text = node.utf8_text(text.as_bytes()).unwrap(); + assert_eq!(text, expected); + } } } From 559ffb1a73e809dd024b5f303f516691322d49cb Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Wed, 12 Nov 2025 17:16:18 -0300 Subject: [PATCH 13/13] refactor tests + add test for custom handlers --- crates/ark/tests/help.rs | 99 ++++++++++++++++++++++++++++------------ 1 file changed, 71 insertions(+), 28 deletions(-) diff --git a/crates/ark/tests/help.rs b/crates/ark/tests/help.rs index 8f4dc41b9..e295e2b54 100644 --- a/crates/ark/tests/help.rs +++ b/crates/ark/tests/help.rs @@ -13,49 +13,53 @@ use amalthea::comm::help_comm::HelpBackendRequest; use amalthea::comm::help_comm::ShowHelpTopicParams; use amalthea::socket::comm::CommInitiator; use amalthea::socket::comm::CommSocket; +use ark::help::message::HelpEvent; use ark::help::r_help::RHelp; use ark::help_proxy; use ark::r_task::r_task; +use crossbeam::channel::Sender; use harp::exec::RFunction; -/** - * Basic test for the R help comm; requests help for a topic and ensures that we - * get a reply. - */ -#[test] -fn test_help_comm() { - // Create the comm socket for the Help comm - let comm = CommSocket::new( - CommInitiator::FrontEnd, - String::from("test-help-comm-id"), - String::from("positron.help"), - ); +struct TestRHelp { + comm: CommSocket, + _help_event_tx: Sender, +} - let incoming_tx = comm.incoming_tx.clone(); - let outgoing_rx = comm.outgoing_rx.clone(); +impl TestRHelp { + fn new(comm_id: String) -> Self { + let comm = CommSocket::new( + CommInitiator::FrontEnd, + comm_id, + String::from("positron.help"), + ); + // Start the help comm. It's important to save the help event sender so + // that the help comm doesn't exit before we're done with it; allowing the + // sender to be dropped signals the help comm to exit. + let r_port = r_task(|| RHelp::r_start_or_reconnect_to_help_server().unwrap()); + let proxy_port = help_proxy::start(r_port).unwrap(); + let _help_event_tx = RHelp::start(comm.clone(), r_port, proxy_port).unwrap(); - // Start the help comm. It's important to save the help event sender so - // that the help comm doesn't exit before we're done with it; allowing the - // sender to be dropped signals the help comm to exit. - let r_port = r_task(|| RHelp::r_start_or_reconnect_to_help_server().unwrap()); - let proxy_port = help_proxy::start(r_port).unwrap(); - let _help_event_tx = RHelp::start(comm, r_port, proxy_port).unwrap(); + Self { + comm, + _help_event_tx, + } + } - // Utility function for testing `ShowHelpTopic` requests - let test_topic = |topic: &str, id: &str| { + fn test_topic(&self, topic: &str, id: &str) { // Send a request for the help topic let request = HelpBackendRequest::ShowHelpTopic(ShowHelpTopicParams { topic: String::from(topic), }); let data = serde_json::to_value(request).unwrap(); let request_id = String::from(id); - incoming_tx + self.comm + .incoming_tx .send(CommMsg::Rpc(request_id.clone(), data)) .unwrap(); // Wait for the response (up to 1 second; this should be fast!) let duration = std::time::Duration::from_secs(1); - let response = outgoing_rx.recv_timeout(duration).unwrap(); + let response = self.comm.outgoing_rx.recv_timeout(duration).unwrap(); match response { CommMsg::Rpc(id, val) => { let response = serde_json::from_value::(val).unwrap(); @@ -71,13 +75,22 @@ fn test_help_comm() { panic!("Unexpected response from help comm: {:?}", response); }, } - }; + } +} - test_topic("library", "help-test-id-1"); - test_topic("utils::find", "help-test-id-2"); +/** + * Basic test for the R help comm; requests help for a topic and ensures that we + * get a reply. + */ +#[test] +fn test_help_comm() { + let r_help = TestRHelp::new(String::from("test-help-comm-id")); + + r_help.test_topic("library", "help-test-id-1"); + r_help.test_topic("utils::find", "help-test-id-2"); // Can come through this way if users request help while their cursor is on // an internal function - test_topic("utils:::find", "help-test-id-3"); + r_help.test_topic("utils:::find", "help-test-id-3"); // Figure out which port the R help server is running on (or would run on) let r_help_port = r_task(|| unsafe { @@ -98,3 +111,33 @@ fn test_help_comm() { ); assert!(RHelp::is_help_url(url.as_str(), r_help_port)); } + +#[test] +fn test_custom_help_handlers() { + let r_help = TestRHelp::new(String::from("test-help-comm-id-2")); + + // Add a test help handler for an object + r_task(|| { + harp::parse_eval_global( + r#" + + called <- FALSE + .ark.register_method("ark_positron_help_get_handler", "foo", function(x) { + function() { + called <<- TRUE + } + }) + + obj <- new.env() + obj$hello <- structure(list(), class = "foo") + "#, + ) + .unwrap(); + }); + + r_help.test_topic("obj$hello", "help-test-id-4"); + assert_eq!( + r_task(|| unsafe { harp::parse_eval_global("called").unwrap().to::() }).unwrap(), + true, + ); +}