From ccea797f801ecf9243b18fe8a31a97b78fc0212e Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Mon, 19 Jan 2026 21:08:46 -0300 Subject: [PATCH 1/9] Fix S4 class inheritance in ark method dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For S4 objects, `class(object)` only returns the immediate class name, not the full inheritance hierarchy like S3 objects do. This meant that methods registered for a parent S4 class would not be found when dispatching on a subclass. Now `call_ark_method` uses `methods::extends()` for S4 objects to get the complete class hierarchy including superclasses. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- crates/ark/src/modules/positron/methods.R | 28 ++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/crates/ark/src/modules/positron/methods.R b/crates/ark/src/modules/positron/methods.R index 5c9a4337d..ea74a80d5 100644 --- a/crates/ark/src/modules/positron/methods.R +++ b/crates/ark/src/modules/positron/methods.R @@ -92,6 +92,25 @@ ark_methods_table$ark_positron_variable_get_children <- new.env( ark_methods_table$ark_positron_help_get_handler <- new.env( parent = emptyenv() ) + +#' Check if object is a connection that can be viewed in the Connections Pane +#' +#' @param x Object to check +#' @param ... Additional arguments (unused) +#' @return Logical value: TRUE if the object is a viewable connection, FALSE otherwise +ark_methods_table$ark_positron_variable_is_connection <- new.env( + parent = emptyenv() +) + +#' View a connection object in the Connections Pane +#' +#' @param x Connection object to view +#' @param ... Additional arguments (unused) +#' @return NULL, called for side effects +ark_methods_table$ark_positron_variable_view_connection <- new.env( + parent = emptyenv() +) + lockEnvironment(ark_methods_table, TRUE) ark_methods_allowed_packages <- c("torch", "reticulate", "duckplyr") @@ -169,7 +188,14 @@ call_ark_method <- function(generic, object, ...) { return(NULL) } - for (cls in class(object)) { + # Get all classes to check, including S4 superclasses + classes <- class(object) + if (isS4(object)) { + # For S4 objects, get the full inheritance hierarchy + classes <- methods::extends(class(object)) + } + + for (cls in classes) { if (!is.null(method <- get0(cls, envir = methods_table))) { return(eval( as.call(list(method, object, ...)), From ec695891faaa8aab10b9d76d33e8d90a5fabaec2 Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Mon, 19 Jan 2026 21:09:09 -0300 Subject: [PATCH 2/9] Add viewer support for database connections in Variables Pane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the ability to view database connections (starting with ODBC) from the Variables Pane. When a user clicks to view a connection variable, it opens in the Connections Pane. Changes: - Add `VariableIsConnection` and `VariableViewConnection` to ArkGenerics - Add `is_connection()` and `view_connection()` functions in variable.rs - Modify `has_viewer()` to return true for connection objects - Modify `view()` in r_variables.rs to handle connections - Add method table entries for the new generics in methods.R - Implement ODBC connection support using `odbc:::on_connection_opened()` with reconstructed connection code from the connection info The ODBC methods are registered via setHook when the odbc package loads. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- crates/ark/src/methods.rs | 6 ++ crates/ark/src/modules/positron/connection.R | 68 +++++++++++++++++++- crates/ark/src/variables/r_variables.rs | 8 +++ crates/ark/src/variables/variable.rs | 37 +++++++++++ 4 files changed, 118 insertions(+), 1 deletion(-) diff --git a/crates/ark/src/methods.rs b/crates/ark/src/methods.rs index 8f9f22f8b..bc6e0c0be 100644 --- a/crates/ark/src/methods.rs +++ b/crates/ark/src/methods.rs @@ -45,6 +45,12 @@ pub enum ArkGenerics { #[strum(serialize = "ark_positron_help_get_handler")] HelpGetHandler, + + #[strum(serialize = "ark_positron_variable_is_connection")] + VariableIsConnection, + + #[strum(serialize = "ark_positron_variable_view_connection")] + VariableViewConnection, } impl ArkGenerics { diff --git a/crates/ark/src/modules/positron/connection.R b/crates/ark/src/modules/positron/connection.R index 601481cdf..396dd0fa4 100644 --- a/crates/ark/src/modules/positron/connection.R +++ b/crates/ark/src/modules/positron/connection.R @@ -283,6 +283,72 @@ connection_flatten_object_types <- function(object_tree) { actions = NULL ) - if (is.null(id)) return("hello") + if (is.null(id)) { + return("hello") + } id } + +# ODBC Connection Support +# These methods enable viewing ODBC connections in the Connections Pane +# from the Variables Pane + +# Register ODBC connection methods when the odbc package is loaded +setHook( + packageEvent("odbc", "onLoad"), + function(...) { + # Check if an object is an ODBC connection + .ark.register_method( + "ark_positron_variable_is_connection", + "OdbcConnection", + function(x) { + inherits(x, "OdbcConnection") && odbc::dbIsValid(x) + } + ) + + # View an ODBC connection in the Connections Pane + .ark.register_method( + "ark_positron_variable_view_connection", + "OdbcConnection", + function(x) { + # Reconstruct the connection code from the connection info + info <- x@info + code <- .ps.odbc_connection_code(info) + + # Use odbc's built-in connection observer integration + odbc:::on_connection_opened(x, code = code) + invisible(NULL) + } + ) + } +) + +# Helper to reconstruct ODBC connection code from connection info +.ps.odbc_connection_code <- function(info) { + # Try to reconstruct a reasonable connection string + # Priority: DSN > connection string parameters + if (!is.null(info$sourcename) && nzchar(info$sourcename)) { + # DSN-based connection + sprintf('odbc::dbConnect(odbc::odbc(), dsn = "%s")', info$sourcename) + } else { + # Build connection string from available parameters + params <- character() + + if (!is.null(info$servername) && nzchar(info$servername)) { + params <- c(params, sprintf('server = "%s"', info$servername)) + } + if (!is.null(info$dbname) && nzchar(info$dbname)) { + params <- c(params, sprintf('database = "%s"', info$dbname)) + } + + if (length(params) > 0) { + sprintf( + "odbc::dbConnect(odbc::odbc(), %s)", + paste(params, collapse = ", ") + ) + } else { + # Fallback if we can't determine connection parameters + "# Connection opened from Variables Pane" + } + } +} diff --git a/crates/ark/src/variables/r_variables.rs b/crates/ark/src/variables/r_variables.rs index 7db8fd0c7..c96d50adc 100644 --- a/crates/ark/src/variables/r_variables.rs +++ b/crates/ark/src/variables/r_variables.rs @@ -45,6 +45,8 @@ use crate::data_explorer::summary_stats::summary_stats; use crate::lsp::events::EVENTS; use crate::r_task; use crate::thread::RThreadSafe; +use crate::variables::variable::is_connection; +use crate::variables::variable::view_connection; use crate::variables::variable::PositronVariable; use crate::view::view; @@ -381,6 +383,12 @@ impl RVariables { let env = self.env.get().clone(); let obj = PositronVariable::resolve_data_object(env.clone(), &path)?; + // Check if this is a connection that should be viewed in the Connections Pane + if is_connection(obj.sexp) { + view_connection(obj.sexp).map_err(harp::Error::Anyhow)?; + return Ok(None); + } + if r_is_function(obj.sexp) { harp::as_result(view(&obj, &path, &env))?; return Ok(None); diff --git a/crates/ark/src/variables/variable.rs b/crates/ark/src/variables/variable.rs index 6aa8c9ceb..3a0ecbbc9 100644 --- a/crates/ark/src/variables/variable.rs +++ b/crates/ark/src/variables/variable.rs @@ -594,6 +594,11 @@ fn has_viewer(value: SEXP) -> bool { return true; } + // Check if this is a connection that can be viewed + if is_connection(value) { + return true; + } + if !(r_is_data_frame(value) || r_is_matrix(value)) { return false; } @@ -615,6 +620,38 @@ fn has_viewer(value: SEXP) -> bool { } } +/// Check if the value is a connection that can be viewed in the Connections Pane. +/// This dispatches to the `ark_positron_variable_is_connection` method. +pub fn is_connection(value: SEXP) -> bool { + match ArkGenerics::VariableIsConnection.try_dispatch::(value, vec![]) { + Err(err) => { + log::error!( + "Error from '{}' method: {err}", + ArkGenerics::VariableIsConnection.to_string() + ); + false + }, + // No method found, not a connection + Ok(None) => false, + // Method found, use its result + Ok(Some(val)) => val, + } +} + +/// View a connection object in the Connections Pane. +/// This dispatches to the `ark_positron_variable_view_connection` method. +pub fn view_connection(value: SEXP) -> anyhow::Result<()> { + match ArkGenerics::VariableViewConnection.try_dispatch::(value, vec![]) { + Err(err) => { + return Err(anyhow!("Error viewing connection: {err}")); + }, + Ok(None) => { + return Err(anyhow!("No view_connection method found for this object")); + }, + Ok(Some(_)) => Ok(()), + } +} + enum EnvironmentVariableNode { Concrete { object: RObject }, R6Node { object: RObject, name: String }, From c07b3efc407ce0fd2bacebeee29184d40e8f2509 Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Tue, 20 Jan 2026 09:22:57 -0300 Subject: [PATCH 3/9] Return success status from view_connection methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `ark_positron_variable_view_connection` method now returns TRUE on success and FALSE on failure, allowing callers to detect and report errors. Also removes redundant `inherits()` check in OdbcConnection handler since method dispatch already ensures the correct class. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- crates/ark/src/modules/positron/connection.R | 4 ++-- crates/ark/src/modules/positron/methods.R | 2 +- crates/ark/src/variables/variable.rs | 8 ++++++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/crates/ark/src/modules/positron/connection.R b/crates/ark/src/modules/positron/connection.R index 396dd0fa4..2e9824721 100644 --- a/crates/ark/src/modules/positron/connection.R +++ b/crates/ark/src/modules/positron/connection.R @@ -302,7 +302,7 @@ setHook( "ark_positron_variable_is_connection", "OdbcConnection", function(x) { - inherits(x, "OdbcConnection") && odbc::dbIsValid(x) + odbc::dbIsValid(x) } ) @@ -317,7 +317,7 @@ setHook( # Use odbc's built-in connection observer integration odbc:::on_connection_opened(x, code = code) - invisible(NULL) + invisible(TRUE) } ) } diff --git a/crates/ark/src/modules/positron/methods.R b/crates/ark/src/modules/positron/methods.R index ea74a80d5..9d65803e6 100644 --- a/crates/ark/src/modules/positron/methods.R +++ b/crates/ark/src/modules/positron/methods.R @@ -106,7 +106,7 @@ ark_methods_table$ark_positron_variable_is_connection <- new.env( #' #' @param x Connection object to view #' @param ... Additional arguments (unused) -#' @return NULL, called for side effects +#' @return Logical value: TRUE on success, FALSE otherwise ark_methods_table$ark_positron_variable_view_connection <- new.env( parent = emptyenv() ) diff --git a/crates/ark/src/variables/variable.rs b/crates/ark/src/variables/variable.rs index 3a0ecbbc9..43deb8743 100644 --- a/crates/ark/src/variables/variable.rs +++ b/crates/ark/src/variables/variable.rs @@ -640,15 +640,19 @@ pub fn is_connection(value: SEXP) -> bool { /// View a connection object in the Connections Pane. /// This dispatches to the `ark_positron_variable_view_connection` method. +/// The method should return TRUE on success, FALSE otherwise. pub fn view_connection(value: SEXP) -> anyhow::Result<()> { - match ArkGenerics::VariableViewConnection.try_dispatch::(value, vec![]) { + match ArkGenerics::VariableViewConnection.try_dispatch::(value, vec![]) { Err(err) => { return Err(anyhow!("Error viewing connection: {err}")); }, Ok(None) => { return Err(anyhow!("No view_connection method found for this object")); }, - Ok(Some(_)) => Ok(()), + Ok(Some(false)) => { + return Err(anyhow!("Failed to view connection")); + }, + Ok(Some(true)) => Ok(()), } } From 6afb3b109ddcd463aa9d7d54d448abbc33c70170 Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Tue, 20 Jan 2026 09:34:39 -0300 Subject: [PATCH 4/9] Send focus event when a connection is opened MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `ps_connection_focus` to send a focus event to the frontend, allowing the Connections Pane to focus on a connection. The focus event is now sent automatically whenever `connectionOpened` is called, whether for a new connection or an existing one. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- crates/ark/src/connections/r_connection.rs | 15 +++++++++++++++ crates/ark/src/modules/positron/connection.R | 7 +++++++ 2 files changed, 22 insertions(+) diff --git a/crates/ark/src/connections/r_connection.rs b/crates/ark/src/connections/r_connection.rs index bb30f50ec..370e93f91 100644 --- a/crates/ark/src/connections/r_connection.rs +++ b/crates/ark/src/connections/r_connection.rs @@ -358,3 +358,18 @@ pub unsafe extern "C-unwind" fn ps_connection_updated(id: SEXP) -> Result Result { + let main = RMain::get(); + let comm_id: String = RObject::view(id).to::()?; + + let event = ConnectionsFrontendEvent::Focus; + + main.get_comm_manager_tx().send(CommManagerEvent::Message( + comm_id, + CommMsg::Data(serde_json::to_value(event)?), + ))?; + + Ok(R_NilValue) +} diff --git a/crates/ark/src/modules/positron/connection.R b/crates/ark/src/modules/positron/connection.R index 2e9824721..c8dc708a5 100644 --- a/crates/ark/src/modules/positron/connection.R +++ b/crates/ark/src/modules/positron/connection.R @@ -21,6 +21,11 @@ .ps.Call("ps_connection_updated", id) } +#' @export +.ps.connection_focus <- function(id) { + .ps.Call("ps_connection_focus", id) +} + #' @export .ps.connection_observer <- function() { connections <- new.env(parent = emptyenv()) @@ -45,6 +50,7 @@ for (id in ls(envir = connections)) { con <- get(id, envir = connections) if (identical(con$host, host) && identical(con$type, type)) { + .ps.connection_focus(id) return(invisible(id)) } } @@ -67,6 +73,7 @@ # until the end of the connection. objectTypes = connection_flatten_object_types(listObjectTypes()) ) + .ps.connection_focus(id) invisible(id) } From b0009d5f52dbb6e051567c36492c0704763207fc Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Tue, 20 Jan 2026 09:50:35 -0300 Subject: [PATCH 5/9] relocate changes --- crates/ark/src/modules/positron/connection.R | 60 ++++++++++---------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/crates/ark/src/modules/positron/connection.R b/crates/ark/src/modules/positron/connection.R index c8dc708a5..ed907240a 100644 --- a/crates/ark/src/modules/positron/connection.R +++ b/crates/ark/src/modules/positron/connection.R @@ -177,6 +177,36 @@ connection_flatten_object_types <- function(object_tree) { return(TRUE) } +# Helper to reconstruct ODBC connection code from connection info +.ps.odbc_connection_code <- function(info) { + # Try to reconstruct a reasonable connection string + # Priority: DSN > connection string parameters + if (!is.null(info$sourcename) && nzchar(info$sourcename)) { + # DSN-based connection + sprintf('odbc::dbConnect(odbc::odbc(), dsn = "%s")', info$sourcename) + } else { + # Build connection string from available parameters + params <- character() + + if (!is.null(info$servername) && nzchar(info$servername)) { + params <- c(params, sprintf('server = "%s"', info$servername)) + } + if (!is.null(info$dbname) && nzchar(info$dbname)) { + params <- c(params, sprintf('database = "%s"', info$dbname)) + } + + if (length(params) > 0) { + sprintf( + "odbc::dbConnect(odbc::odbc(), %s)", + paste(params, collapse = ", ") + ) + } else { + # Fallback if we can't determine connection parameters + "# Connection opened from Variables Pane" + } + } +} + #' @export .ps.connection_icon <- function(id, ...) { con <- get(id, getOption("connectionObserver")$.connections) @@ -329,33 +359,3 @@ setHook( ) } ) - -# Helper to reconstruct ODBC connection code from connection info -.ps.odbc_connection_code <- function(info) { - # Try to reconstruct a reasonable connection string - # Priority: DSN > connection string parameters - if (!is.null(info$sourcename) && nzchar(info$sourcename)) { - # DSN-based connection - sprintf('odbc::dbConnect(odbc::odbc(), dsn = "%s")', info$sourcename) - } else { - # Build connection string from available parameters - params <- character() - - if (!is.null(info$servername) && nzchar(info$servername)) { - params <- c(params, sprintf('server = "%s"', info$servername)) - } - if (!is.null(info$dbname) && nzchar(info$dbname)) { - params <- c(params, sprintf('database = "%s"', info$dbname)) - } - - if (length(params) > 0) { - sprintf( - "odbc::dbConnect(odbc::odbc(), %s)", - paste(params, collapse = ", ") - ) - } else { - # Fallback if we can't determine connection parameters - "# Connection opened from Variables Pane" - } - } -} From c0899acf41f9628e97b1a8c48a9c6f5b8655195e Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Tue, 20 Jan 2026 10:42:00 -0300 Subject: [PATCH 6/9] Skip focus event in tests when RMain is not initialized MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prevents panic in connection tests by checking if RMain is initialized before attempting to send the focus event. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- crates/ark/src/connections/r_connection.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/ark/src/connections/r_connection.rs b/crates/ark/src/connections/r_connection.rs index 370e93f91..a88d5ff58 100644 --- a/crates/ark/src/connections/r_connection.rs +++ b/crates/ark/src/connections/r_connection.rs @@ -361,6 +361,10 @@ pub unsafe extern "C-unwind" fn ps_connection_updated(id: SEXP) -> Result Result { + if !RMain::is_initialized() { + return Ok(R_NilValue); + } + let main = RMain::get(); let comm_id: String = RObject::view(id).to::()?; From 530a6b7654c2d7a7fcc7e661ae1e0eaf5389e61f Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Tue, 20 Jan 2026 11:50:12 -0300 Subject: [PATCH 7/9] Add BigQuery connection support for Variables Pane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Registers methods for `BigQueryConnection` class when the bigrquery package is loaded, enabling users to view BigQuery connections in the Connections Pane from the Variables Pane. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- crates/ark/src/modules/positron/connection.R | 51 ++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/crates/ark/src/modules/positron/connection.R b/crates/ark/src/modules/positron/connection.R index ed907240a..e5a3ed0d5 100644 --- a/crates/ark/src/modules/positron/connection.R +++ b/crates/ark/src/modules/positron/connection.R @@ -359,3 +359,54 @@ setHook( ) } ) + +# BigQuery Connection Support +# These methods enable viewing BigQuery connections in the Connections Pane +# from the Variables Pane + +# Register BigQuery connection methods when the bigrquery package is loaded +setHook( + packageEvent("bigrquery", "onLoad"), + function(...) { + # Check if an object is a BigQuery connection + .ark.register_method( + "ark_positron_variable_is_connection", + "BigQueryConnection", + function(x) { + DBI::dbIsValid(x) + } + ) + + # View a BigQuery connection in the Connections Pane + .ark.register_method( + "ark_positron_variable_view_connection", + "BigQueryConnection", + function(x) { + # Reconstruct the connection code + code <- .ps.bigrquery_connection_code(x) + + # Use bigrquery's built-in connection observer integration + bigrquery:::on_connection_opened(x, code = code) + invisible(TRUE) + } + ) + } +) + +# Helper to reconstruct BigQuery connection code +.ps.bigrquery_connection_code <- function(con) { + project <- con@project + dataset <- con@dataset + billing <- con@billing + + params <- c(sprintf('project = "%s"', project)) + + if (!is.null(dataset) && nzchar(dataset)) { + params <- c(params, sprintf('dataset = "%s"', dataset)) + } + if (!is.null(billing) && nzchar(billing) && billing != project) { + params <- c(params, sprintf('billing = "%s"', billing)) + } + + sprintf("DBI::dbConnect(bigrquery::bigquery(), %s)", paste(params, collapse = ", ")) +} From 8c01f0fd9acc1e3388af9ff4474191be1c83232b Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Tue, 20 Jan 2026 12:00:19 -0300 Subject: [PATCH 8/9] Make variable viewer system generic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace connection-specific methods with generic viewer methods: - Remove `ark_positron_variable_is_connection` and `view_connection` - Add generic `ark_positron_variable_view` for custom view actions - Extend `has_viewer` to dispatch for all objects, not just data frames Connection packages now register: - `kind` returning "connection" for proper icon display - `has_viewer` returning validity check - `view` for the actual view action This allows any package to implement custom viewers for their objects. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- crates/ark/src/methods.rs | 7 +-- crates/ark/src/modules/positron/connection.R | 26 +++++++-- crates/ark/src/modules/positron/methods.R | 15 +---- crates/ark/src/variables/r_variables.rs | 8 +-- crates/ark/src/variables/variable.rs | 58 ++++++-------------- 5 files changed, 44 insertions(+), 70 deletions(-) diff --git a/crates/ark/src/methods.rs b/crates/ark/src/methods.rs index bc6e0c0be..61b8dca9e 100644 --- a/crates/ark/src/methods.rs +++ b/crates/ark/src/methods.rs @@ -46,11 +46,8 @@ pub enum ArkGenerics { #[strum(serialize = "ark_positron_help_get_handler")] HelpGetHandler, - #[strum(serialize = "ark_positron_variable_is_connection")] - VariableIsConnection, - - #[strum(serialize = "ark_positron_variable_view_connection")] - VariableViewConnection, + #[strum(serialize = "ark_positron_variable_view")] + VariableView, } impl ArkGenerics { diff --git a/crates/ark/src/modules/positron/connection.R b/crates/ark/src/modules/positron/connection.R index e5a3ed0d5..66baa5854 100644 --- a/crates/ark/src/modules/positron/connection.R +++ b/crates/ark/src/modules/positron/connection.R @@ -334,9 +334,16 @@ connection_flatten_object_types <- function(object_tree) { setHook( packageEvent("odbc", "onLoad"), function(...) { - # Check if an object is an ODBC connection + # Mark ODBC connections as "connection" kind .ark.register_method( - "ark_positron_variable_is_connection", + "ark_positron_variable_kind", + "OdbcConnection", + function(x) "connection" + ) + + # Enable viewer for valid ODBC connections + .ark.register_method( + "ark_positron_variable_has_viewer", "OdbcConnection", function(x) { odbc::dbIsValid(x) @@ -345,7 +352,7 @@ setHook( # View an ODBC connection in the Connections Pane .ark.register_method( - "ark_positron_variable_view_connection", + "ark_positron_variable_view", "OdbcConnection", function(x) { # Reconstruct the connection code from the connection info @@ -368,9 +375,16 @@ setHook( setHook( packageEvent("bigrquery", "onLoad"), function(...) { - # Check if an object is a BigQuery connection + # Mark BigQuery connections as "connection" kind + .ark.register_method( + "ark_positron_variable_kind", + "BigQueryConnection", + function(x) "connection" + ) + + # Enable viewer for valid BigQuery connections .ark.register_method( - "ark_positron_variable_is_connection", + "ark_positron_variable_has_viewer", "BigQueryConnection", function(x) { DBI::dbIsValid(x) @@ -379,7 +393,7 @@ setHook( # View a BigQuery connection in the Connections Pane .ark.register_method( - "ark_positron_variable_view_connection", + "ark_positron_variable_view", "BigQueryConnection", function(x) { # Reconstruct the connection code diff --git a/crates/ark/src/modules/positron/methods.R b/crates/ark/src/modules/positron/methods.R index 9d65803e6..3b81cc791 100644 --- a/crates/ark/src/modules/positron/methods.R +++ b/crates/ark/src/modules/positron/methods.R @@ -93,21 +93,12 @@ ark_methods_table$ark_positron_help_get_handler <- new.env( parent = emptyenv() ) -#' Check if object is a connection that can be viewed in the Connections Pane +#' Custom view action for objects in Variables Pane #' -#' @param x Object to check -#' @param ... Additional arguments (unused) -#' @return Logical value: TRUE if the object is a viewable connection, FALSE otherwise -ark_methods_table$ark_positron_variable_is_connection <- new.env( - parent = emptyenv() -) - -#' View a connection object in the Connections Pane -#' -#' @param x Connection object to view +#' @param x Object to view #' @param ... Additional arguments (unused) #' @return Logical value: TRUE on success, FALSE otherwise -ark_methods_table$ark_positron_variable_view_connection <- new.env( +ark_methods_table$ark_positron_variable_view <- new.env( parent = emptyenv() ) diff --git a/crates/ark/src/variables/r_variables.rs b/crates/ark/src/variables/r_variables.rs index c96d50adc..1db5f9665 100644 --- a/crates/ark/src/variables/r_variables.rs +++ b/crates/ark/src/variables/r_variables.rs @@ -45,8 +45,7 @@ use crate::data_explorer::summary_stats::summary_stats; use crate::lsp::events::EVENTS; use crate::r_task; use crate::thread::RThreadSafe; -use crate::variables::variable::is_connection; -use crate::variables::variable::view_connection; +use crate::variables::variable::try_custom_view; use crate::variables::variable::PositronVariable; use crate::view::view; @@ -383,9 +382,8 @@ impl RVariables { let env = self.env.get().clone(); let obj = PositronVariable::resolve_data_object(env.clone(), &path)?; - // Check if this is a connection that should be viewed in the Connections Pane - if is_connection(obj.sexp) { - view_connection(obj.sexp).map_err(harp::Error::Anyhow)?; + // Try custom view method first (e.g., for connections) + if try_custom_view(obj.sexp).map_err(harp::Error::Anyhow)? { return Ok(None); } diff --git a/crates/ark/src/variables/variable.rs b/crates/ark/src/variables/variable.rs index 43deb8743..52dd325d2 100644 --- a/crates/ark/src/variables/variable.rs +++ b/crates/ark/src/variables/variable.rs @@ -594,16 +594,7 @@ fn has_viewer(value: SEXP) -> bool { return true; } - // Check if this is a connection that can be viewed - if is_connection(value) { - return true; - } - - if !(r_is_data_frame(value) || r_is_matrix(value)) { - return false; - } - - // We have a data.frame or matrix. Dispatch to the has_viewer method + // Dispatch to the has_viewer method for any object match ArkGenerics::VariableHasViewer.try_dispatch::(value, vec![]) { Err(err) => { log::error!( @@ -613,46 +604,30 @@ fn has_viewer(value: SEXP) -> bool { // The viewer method exists, but failed false }, - // A matching viewer method was not found - Ok(None) => true, // The viewer method was found, use its result Ok(Some(val)) => val, + // No method found, fall back to default logic for data frames/matrices + Ok(None) => r_is_data_frame(value) || r_is_matrix(value), } } -/// Check if the value is a connection that can be viewed in the Connections Pane. -/// This dispatches to the `ark_positron_variable_is_connection` method. -pub fn is_connection(value: SEXP) -> bool { - match ArkGenerics::VariableIsConnection.try_dispatch::(value, vec![]) { - Err(err) => { - log::error!( - "Error from '{}' method: {err}", - ArkGenerics::VariableIsConnection.to_string() - ); - false - }, - // No method found, not a connection - Ok(None) => false, - // Method found, use its result - Ok(Some(val)) => val, - } -} - -/// View a connection object in the Connections Pane. -/// This dispatches to the `ark_positron_variable_view_connection` method. -/// The method should return TRUE on success, FALSE otherwise. -pub fn view_connection(value: SEXP) -> anyhow::Result<()> { - match ArkGenerics::VariableViewConnection.try_dispatch::(value, vec![]) { +/// Try to view an object using a custom view method. +/// This dispatches to the `ark_positron_variable_view` method. +/// Returns Ok(true) if a custom view was handled, Ok(false) if no method found, +/// or Err if the method failed. +pub fn try_custom_view(value: SEXP) -> anyhow::Result { + match ArkGenerics::VariableView.try_dispatch::(value, vec![]) { Err(err) => { - return Err(anyhow!("Error viewing connection: {err}")); + return Err(anyhow!("Error in custom view: {err}")); }, Ok(None) => { - return Err(anyhow!("No view_connection method found for this object")); + // No custom view method found + Ok(false) }, Ok(Some(false)) => { - return Err(anyhow!("Failed to view connection")); + return Err(anyhow!("Custom view method failed")); }, - Ok(Some(true)) => Ok(()), + Ok(Some(true)) => Ok(true), } } @@ -1821,9 +1796,8 @@ mod tests { assert_eq!(variable.kind, VariableKind::Other); - // Even though the viewer method returns TRUE, the object is not a data.frame - // or matrix, so it doesn't have a viewer. - assert_eq!(variable.has_viewer, false); + // The has_viewer method returns TRUE, so the object has a viewer + assert_eq!(variable.has_viewer, true); // Now inspect `x` let path = vec![String::from("x")]; From bc2f6c60522af3ada8999f2811e38824984e1cfd Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Mon, 26 Jan 2026 13:43:31 -0300 Subject: [PATCH 9/9] Address PR review comments on naming conventions - Remove `.ps.` prefix from internal helper functions that are not exported (`odbc_connection_code`, `bigrquery_connection_code`, `connection_focus`) - Rename `try_custom_view` to `try_dispatch_view` for consistency with the `try_dispatch()` pattern Co-Authored-By: Claude Opus 4.5 --- crates/ark/src/modules/positron/connection.R | 15 +++++++-------- crates/ark/src/variables/r_variables.rs | 4 ++-- crates/ark/src/variables/variable.rs | 2 +- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/crates/ark/src/modules/positron/connection.R b/crates/ark/src/modules/positron/connection.R index 66baa5854..abea1beeb 100644 --- a/crates/ark/src/modules/positron/connection.R +++ b/crates/ark/src/modules/positron/connection.R @@ -21,8 +21,7 @@ .ps.Call("ps_connection_updated", id) } -#' @export -.ps.connection_focus <- function(id) { +connection_focus <- function(id) { .ps.Call("ps_connection_focus", id) } @@ -50,7 +49,7 @@ for (id in ls(envir = connections)) { con <- get(id, envir = connections) if (identical(con$host, host) && identical(con$type, type)) { - .ps.connection_focus(id) + connection_focus(id) return(invisible(id)) } } @@ -73,7 +72,7 @@ # until the end of the connection. objectTypes = connection_flatten_object_types(listObjectTypes()) ) - .ps.connection_focus(id) + connection_focus(id) invisible(id) } @@ -178,7 +177,7 @@ connection_flatten_object_types <- function(object_tree) { } # Helper to reconstruct ODBC connection code from connection info -.ps.odbc_connection_code <- function(info) { +odbc_connection_code <- function(info) { # Try to reconstruct a reasonable connection string # Priority: DSN > connection string parameters if (!is.null(info$sourcename) && nzchar(info$sourcename)) { @@ -357,7 +356,7 @@ setHook( function(x) { # Reconstruct the connection code from the connection info info <- x@info - code <- .ps.odbc_connection_code(info) + code <- odbc_connection_code(info) # Use odbc's built-in connection observer integration odbc:::on_connection_opened(x, code = code) @@ -397,7 +396,7 @@ setHook( "BigQueryConnection", function(x) { # Reconstruct the connection code - code <- .ps.bigrquery_connection_code(x) + code <- bigrquery_connection_code(x) # Use bigrquery's built-in connection observer integration bigrquery:::on_connection_opened(x, code = code) @@ -408,7 +407,7 @@ setHook( ) # Helper to reconstruct BigQuery connection code -.ps.bigrquery_connection_code <- function(con) { +bigrquery_connection_code <- function(con) { project <- con@project dataset <- con@dataset billing <- con@billing diff --git a/crates/ark/src/variables/r_variables.rs b/crates/ark/src/variables/r_variables.rs index 1db5f9665..793ba040f 100644 --- a/crates/ark/src/variables/r_variables.rs +++ b/crates/ark/src/variables/r_variables.rs @@ -45,7 +45,7 @@ use crate::data_explorer::summary_stats::summary_stats; use crate::lsp::events::EVENTS; use crate::r_task; use crate::thread::RThreadSafe; -use crate::variables::variable::try_custom_view; +use crate::variables::variable::try_dispatch_view; use crate::variables::variable::PositronVariable; use crate::view::view; @@ -383,7 +383,7 @@ impl RVariables { let obj = PositronVariable::resolve_data_object(env.clone(), &path)?; // Try custom view method first (e.g., for connections) - if try_custom_view(obj.sexp).map_err(harp::Error::Anyhow)? { + if try_dispatch_view(obj.sexp).map_err(harp::Error::Anyhow)? { return Ok(None); } diff --git a/crates/ark/src/variables/variable.rs b/crates/ark/src/variables/variable.rs index 52dd325d2..56edd0ce4 100644 --- a/crates/ark/src/variables/variable.rs +++ b/crates/ark/src/variables/variable.rs @@ -615,7 +615,7 @@ fn has_viewer(value: SEXP) -> bool { /// This dispatches to the `ark_positron_variable_view` method. /// Returns Ok(true) if a custom view was handled, Ok(false) if no method found, /// or Err if the method failed. -pub fn try_custom_view(value: SEXP) -> anyhow::Result { +pub fn try_dispatch_view(value: SEXP) -> anyhow::Result { match ArkGenerics::VariableView.try_dispatch::(value, vec![]) { Err(err) => { return Err(anyhow!("Error in custom view: {err}"));