Skip to content

Commit 615e01c

Browse files
dfalbelclaude
andauthored
Add viewer support for database connections in Variables Pane (#1004)
* Fix S4 class inheritance in ark method dispatch 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 <[email protected]> * Add viewer support for database connections in Variables Pane 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 <[email protected]> * Return success status from view_connection methods 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 <[email protected]> * Send focus event when a connection is opened 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 <[email protected]> * relocate changes * Skip focus event in tests when RMain is not initialized 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 <[email protected]> * Add BigQuery connection support for Variables Pane 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 <[email protected]> * Make variable viewer system generic 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 <[email protected]> * 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 <[email protected]> --------- Co-authored-by: Claude Opus 4.5 <[email protected]>
1 parent da035bc commit 615e01c

File tree

6 files changed

+209
-12
lines changed

6 files changed

+209
-12
lines changed

crates/ark/src/connections/r_connection.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,3 +358,22 @@ pub unsafe extern "C-unwind" fn ps_connection_updated(id: SEXP) -> Result<SEXP,
358358

359359
Ok(R_NilValue)
360360
}
361+
362+
#[harp::register]
363+
pub unsafe extern "C-unwind" fn ps_connection_focus(id: SEXP) -> Result<SEXP, anyhow::Error> {
364+
if !RMain::is_initialized() {
365+
return Ok(R_NilValue);
366+
}
367+
368+
let main = RMain::get();
369+
let comm_id: String = RObject::view(id).to::<String>()?;
370+
371+
let event = ConnectionsFrontendEvent::Focus;
372+
373+
main.get_comm_manager_tx().send(CommManagerEvent::Message(
374+
comm_id,
375+
CommMsg::Data(serde_json::to_value(event)?),
376+
))?;
377+
378+
Ok(R_NilValue)
379+
}

crates/ark/src/methods.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ pub enum ArkGenerics {
4545

4646
#[strum(serialize = "ark_positron_help_get_handler")]
4747
HelpGetHandler,
48+
49+
#[strum(serialize = "ark_positron_variable_view")]
50+
VariableView,
4851
}
4952

5053
impl ArkGenerics {

crates/ark/src/modules/positron/connection.R

Lines changed: 138 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@
2121
.ps.Call("ps_connection_updated", id)
2222
}
2323

24+
connection_focus <- function(id) {
25+
.ps.Call("ps_connection_focus", id)
26+
}
27+
2428
#' @export
2529
.ps.connection_observer <- function() {
2630
connections <- new.env(parent = emptyenv())
@@ -45,6 +49,7 @@
4549
for (id in ls(envir = connections)) {
4650
con <- get(id, envir = connections)
4751
if (identical(con$host, host) && identical(con$type, type)) {
52+
connection_focus(id)
4853
return(invisible(id))
4954
}
5055
}
@@ -67,6 +72,7 @@
6772
# until the end of the connection.
6873
objectTypes = connection_flatten_object_types(listObjectTypes())
6974
)
75+
connection_focus(id)
7076
invisible(id)
7177
}
7278

@@ -170,6 +176,36 @@ connection_flatten_object_types <- function(object_tree) {
170176
return(TRUE)
171177
}
172178

179+
# Helper to reconstruct ODBC connection code from connection info
180+
odbc_connection_code <- function(info) {
181+
# Try to reconstruct a reasonable connection string
182+
# Priority: DSN > connection string parameters
183+
if (!is.null(info$sourcename) && nzchar(info$sourcename)) {
184+
# DSN-based connection
185+
sprintf('odbc::dbConnect(odbc::odbc(), dsn = "%s")', info$sourcename)
186+
} else {
187+
# Build connection string from available parameters
188+
params <- character()
189+
190+
if (!is.null(info$servername) && nzchar(info$servername)) {
191+
params <- c(params, sprintf('server = "%s"', info$servername))
192+
}
193+
if (!is.null(info$dbname) && nzchar(info$dbname)) {
194+
params <- c(params, sprintf('database = "%s"', info$dbname))
195+
}
196+
197+
if (length(params) > 0) {
198+
sprintf(
199+
"odbc::dbConnect(odbc::odbc(), %s)",
200+
paste(params, collapse = ", ")
201+
)
202+
} else {
203+
# Fallback if we can't determine connection parameters
204+
"# Connection opened from Variables Pane"
205+
}
206+
}
207+
}
208+
173209
#' @export
174210
.ps.connection_icon <- function(id, ...) {
175211
con <- get(id, getOption("connectionObserver")$.connections)
@@ -283,6 +319,107 @@ connection_flatten_object_types <- function(object_tree) {
283319
actions = NULL
284320
)
285321

286-
if (is.null(id)) return("hello")
322+
if (is.null(id)) {
323+
return("hello")
324+
}
287325
id
288326
}
327+
328+
# ODBC Connection Support
329+
# These methods enable viewing ODBC connections in the Connections Pane
330+
# from the Variables Pane
331+
332+
# Register ODBC connection methods when the odbc package is loaded
333+
setHook(
334+
packageEvent("odbc", "onLoad"),
335+
function(...) {
336+
# Mark ODBC connections as "connection" kind
337+
.ark.register_method(
338+
"ark_positron_variable_kind",
339+
"OdbcConnection",
340+
function(x) "connection"
341+
)
342+
343+
# Enable viewer for valid ODBC connections
344+
.ark.register_method(
345+
"ark_positron_variable_has_viewer",
346+
"OdbcConnection",
347+
function(x) {
348+
odbc::dbIsValid(x)
349+
}
350+
)
351+
352+
# View an ODBC connection in the Connections Pane
353+
.ark.register_method(
354+
"ark_positron_variable_view",
355+
"OdbcConnection",
356+
function(x) {
357+
# Reconstruct the connection code from the connection info
358+
info <- x@info
359+
code <- odbc_connection_code(info)
360+
361+
# Use odbc's built-in connection observer integration
362+
odbc:::on_connection_opened(x, code = code)
363+
invisible(TRUE)
364+
}
365+
)
366+
}
367+
)
368+
369+
# BigQuery Connection Support
370+
# These methods enable viewing BigQuery connections in the Connections Pane
371+
# from the Variables Pane
372+
373+
# Register BigQuery connection methods when the bigrquery package is loaded
374+
setHook(
375+
packageEvent("bigrquery", "onLoad"),
376+
function(...) {
377+
# Mark BigQuery connections as "connection" kind
378+
.ark.register_method(
379+
"ark_positron_variable_kind",
380+
"BigQueryConnection",
381+
function(x) "connection"
382+
)
383+
384+
# Enable viewer for valid BigQuery connections
385+
.ark.register_method(
386+
"ark_positron_variable_has_viewer",
387+
"BigQueryConnection",
388+
function(x) {
389+
DBI::dbIsValid(x)
390+
}
391+
)
392+
393+
# View a BigQuery connection in the Connections Pane
394+
.ark.register_method(
395+
"ark_positron_variable_view",
396+
"BigQueryConnection",
397+
function(x) {
398+
# Reconstruct the connection code
399+
code <- bigrquery_connection_code(x)
400+
401+
# Use bigrquery's built-in connection observer integration
402+
bigrquery:::on_connection_opened(x, code = code)
403+
invisible(TRUE)
404+
}
405+
)
406+
}
407+
)
408+
409+
# Helper to reconstruct BigQuery connection code
410+
bigrquery_connection_code <- function(con) {
411+
project <- con@project
412+
dataset <- con@dataset
413+
billing <- con@billing
414+
415+
params <- c(sprintf('project = "%s"', project))
416+
417+
if (!is.null(dataset) && nzchar(dataset)) {
418+
params <- c(params, sprintf('dataset = "%s"', dataset))
419+
}
420+
if (!is.null(billing) && nzchar(billing) && billing != project) {
421+
params <- c(params, sprintf('billing = "%s"', billing))
422+
}
423+
424+
sprintf("DBI::dbConnect(bigrquery::bigquery(), %s)", paste(params, collapse = ", "))
425+
}

crates/ark/src/modules/positron/methods.R

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,16 @@ ark_methods_table$ark_positron_variable_get_children <- new.env(
9292
ark_methods_table$ark_positron_help_get_handler <- new.env(
9393
parent = emptyenv()
9494
)
95+
96+
#' Custom view action for objects in Variables Pane
97+
#'
98+
#' @param x Object to view
99+
#' @param ... Additional arguments (unused)
100+
#' @return Logical value: TRUE on success, FALSE otherwise
101+
ark_methods_table$ark_positron_variable_view <- new.env(
102+
parent = emptyenv()
103+
)
104+
95105
lockEnvironment(ark_methods_table, TRUE)
96106

97107
ark_methods_allowed_packages <- c("torch", "reticulate", "duckplyr")
@@ -169,7 +179,14 @@ call_ark_method <- function(generic, object, ...) {
169179
return(NULL)
170180
}
171181

172-
for (cls in class(object)) {
182+
# Get all classes to check, including S4 superclasses
183+
classes <- class(object)
184+
if (isS4(object)) {
185+
# For S4 objects, get the full inheritance hierarchy
186+
classes <- methods::extends(class(object))
187+
}
188+
189+
for (cls in classes) {
173190
if (!is.null(method <- get0(cls, envir = methods_table))) {
174191
return(eval(
175192
as.call(list(method, object, ...)),

crates/ark/src/variables/r_variables.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ use crate::data_explorer::summary_stats::summary_stats;
4545
use crate::lsp::events::EVENTS;
4646
use crate::r_task;
4747
use crate::thread::RThreadSafe;
48+
use crate::variables::variable::try_dispatch_view;
4849
use crate::variables::variable::PositronVariable;
4950
use crate::view::view;
5051

@@ -381,6 +382,11 @@ impl RVariables {
381382
let env = self.env.get().clone();
382383
let obj = PositronVariable::resolve_data_object(env.clone(), &path)?;
383384

385+
// Try custom view method first (e.g., for connections)
386+
if try_dispatch_view(obj.sexp).map_err(harp::Error::Anyhow)? {
387+
return Ok(None);
388+
}
389+
384390
if r_is_function(obj.sexp) {
385391
harp::as_result(view(&obj, &path, &env))?;
386392
return Ok(None);

crates/ark/src/variables/variable.rs

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -594,11 +594,7 @@ fn has_viewer(value: SEXP) -> bool {
594594
return true;
595595
}
596596

597-
if !(r_is_data_frame(value) || r_is_matrix(value)) {
598-
return false;
599-
}
600-
601-
// We have a data.frame or matrix. Dispatch to the has_viewer method
597+
// Dispatch to the has_viewer method for any object
602598
match ArkGenerics::VariableHasViewer.try_dispatch::<bool>(value, vec![]) {
603599
Err(err) => {
604600
log::error!(
@@ -608,10 +604,30 @@ fn has_viewer(value: SEXP) -> bool {
608604
// The viewer method exists, but failed
609605
false
610606
},
611-
// A matching viewer method was not found
612-
Ok(None) => true,
613607
// The viewer method was found, use its result
614608
Ok(Some(val)) => val,
609+
// No method found, fall back to default logic for data frames/matrices
610+
Ok(None) => r_is_data_frame(value) || r_is_matrix(value),
611+
}
612+
}
613+
614+
/// Try to view an object using a custom view method.
615+
/// This dispatches to the `ark_positron_variable_view` method.
616+
/// Returns Ok(true) if a custom view was handled, Ok(false) if no method found,
617+
/// or Err if the method failed.
618+
pub fn try_dispatch_view(value: SEXP) -> anyhow::Result<bool> {
619+
match ArkGenerics::VariableView.try_dispatch::<bool>(value, vec![]) {
620+
Err(err) => {
621+
return Err(anyhow!("Error in custom view: {err}"));
622+
},
623+
Ok(None) => {
624+
// No custom view method found
625+
Ok(false)
626+
},
627+
Ok(Some(false)) => {
628+
return Err(anyhow!("Custom view method failed"));
629+
},
630+
Ok(Some(true)) => Ok(true),
615631
}
616632
}
617633

@@ -1780,9 +1796,8 @@ mod tests {
17801796

17811797
assert_eq!(variable.kind, VariableKind::Other);
17821798

1783-
// Even though the viewer method returns TRUE, the object is not a data.frame
1784-
// or matrix, so it doesn't have a viewer.
1785-
assert_eq!(variable.has_viewer, false);
1799+
// The has_viewer method returns TRUE, so the object has a viewer
1800+
assert_eq!(variable.has_viewer, true);
17861801

17871802
// Now inspect `x`
17881803
let path = vec![String::from("x")];

0 commit comments

Comments
 (0)