diff --git a/.vscodeignore b/.vscodeignore index 9837514a..5f8a2456 100644 --- a/.vscodeignore +++ b/.vscodeignore @@ -3,6 +3,7 @@ !README.md !CHANGELOG.md !LICENSE +!ThirdPartyNotices.txt !R/*.R !R/cppProperties/** !R/help/** @@ -19,3 +20,6 @@ !dist/**/*.css !dist/**/*.ejs !dist/resources/sess/** +!dist/resources/codicon.ttf +!dist/resources/LICENSE +!dist/resources/LICENSE-CODE diff --git a/ThirdPartyNotices.txt b/ThirdPartyNotices.txt new file mode 100644 index 00000000..a81ca18c --- /dev/null +++ b/ThirdPartyNotices.txt @@ -0,0 +1,32 @@ +VS Code Codicons (@vscode/codicons) +Copyright (c) Microsoft Corporation. +Source: https://github.com/microsoft/vscode-codicons + +The List Viewer uses the package's CSS and icon font. The icon artwork is +unchanged. + +The extension also includes SVG copies of the open-preview, preview, globe, +graph and help icons in images/icons/dark and images/icons/light, with colors +set for the corresponding themes. These are used for viewer tab icons, +including the Data Viewer and List Viewer. +Original integration: https://github.com/REditorSupport/vscode-R/pull/759 + +The artwork is licensed under Creative Commons Attribution 4.0 International: +https://creativecommons.org/licenses/by/4.0/ +The package's code is licensed under the MIT License. + +The bundled license texts are in dist/resources/LICENSE (CC BY 4.0) and +dist/resources/LICENSE-CODE (MIT). These files apply to Codicons. + + +R logo +Copyright (c) 2016 The R Foundation. +Source: https://www.r-project.org/logo/ + +Files: images/Rlogo.svg and images/Rlogo.png. +The SVG reproduces the official artwork unchanged. The PNG is an 800 x 700 +raster variant used as the extension icon. + +The logo is distributed under Creative Commons Attribution-ShareAlike 4.0 +International: +https://creativecommons.org/licenses/by-sa/4.0/ diff --git a/esbuild.js b/esbuild.js index 1ab93a29..cfa0d554 100644 --- a/esbuild.js +++ b/esbuild.js @@ -10,12 +10,13 @@ function copyResources() { fs.mkdirSync(destDir, { recursive: true }); const resources = [ - './node_modules/jquery/dist/jquery.min.js', - './node_modules/jquery.json-viewer/json-viewer/jquery.json-viewer.js', - './node_modules/jquery.json-viewer/json-viewer/jquery.json-viewer.css', './node_modules/ag-grid-community/dist/ag-grid-community.min.noStyle.js', './node_modules/ag-grid-community/styles/ag-grid.min.css', - './node_modules/ag-grid-community/styles/ag-theme-balham.min.css' + './node_modules/ag-grid-community/styles/ag-theme-balham.min.css', + './node_modules/@vscode/codicons/dist/codicon.css', + './node_modules/@vscode/codicons/dist/codicon.ttf', + './node_modules/@vscode/codicons/LICENSE', + './node_modules/@vscode/codicons/LICENSE-CODE' ]; for (const res of resources) { diff --git a/package.json b/package.json index 864a9a51..6c3a5a6b 100644 --- a/package.json +++ b/package.json @@ -1354,7 +1354,7 @@ { "command": "r.workspaceViewer.view", "group": "inline", - "when": "view == workspaceViewer && viewItem == rootNode" + "when": "view == workspaceViewer && (viewItem == rootNode || viewItem == viewableNode)" }, { "command": "r.workspaceViewer.remove", @@ -2561,14 +2561,13 @@ "oxlint-tsgolint": "~7.0.2003" }, "dependencies": { + "@vscode/codicons": "0.0.46-24", "ag-grid-community": "^36.2.0", "cheerio": "1.0.0-rc.12", "ejs": "^3.1.10", "fs-extra": "^10.1.0", "highlight.js": "^11.11.1", "httpgd": "0.1.6", - "jquery": "^3.7.1", - "jquery.json-viewer": "^1.5.0", "js-yaml": "^4.3.2", "node-fetch": "^2.7.0", "vscode-languageclient": "^9.0.1", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 94e2c900..5527cfef 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -11,6 +11,9 @@ importers: .: dependencies: + '@vscode/codicons': + specifier: 0.0.46-24 + version: 0.0.46-24 ag-grid-community: specifier: ^36.2.0 version: 36.2.0 @@ -29,12 +32,6 @@ importers: httpgd: specifier: 0.1.6 version: 0.1.6 - jquery: - specifier: ^3.7.1 - version: 3.7.1 - jquery.json-viewer: - specifier: ^1.5.0 - version: 1.5.0 js-yaml: specifier: ^4.3.2 version: 4.3.2 @@ -867,6 +864,9 @@ packages: resolution: {integrity: sha512-edSdeAqkdxBVzA1yL1LrLCml1YjyCVvPMtMqJpbF+6K609tHe8V6sQUzFQSGcYNhcuhOceZtjvN32+mpIth30A==} engines: {node: '>=22.0.0'} + '@vscode/codicons@0.0.46-24': + resolution: {integrity: sha512-KIDWUYxG6eh+DHRg+3G4bS487T+g1ZkLfB4HylOBbCcHsvLdfLMRM/m92DS1Ljhi8nglLr9ucwwWhHTEAC+DpA==} + '@vscode/test-cli@0.0.12': resolution: {integrity: sha512-iYN0fDg29+a2Xelle/Y56Xvv7Nc8Thzq4VwpzAF/SIE6918rDicqfsQxV6w1ttr2+SOm+10laGuY9FG2ptEKsQ==} engines: {node: '>=18'} @@ -1642,12 +1642,6 @@ packages: engines: {node: '>=10'} hasBin: true - jquery.json-viewer@1.5.0: - resolution: {integrity: sha512-M/mRFXg14V/UUAlz7TBNBIDmQdWt05BunsqC/UjEx5BoFdQpNpfkfDdVn+VtjX951n/an/T9GWB3apBp02x8Mg==} - - jquery@3.7.1: - resolution: {integrity: sha512-m4avr8yL8kmFN8psrbFFFmB/If14iN5o9nw/NgnnM+kybDJpRsAynV2BsfpTYrTRysYUdADVD7CkUUizgkpLfg==} - js-tokens@4.0.0: resolution: {integrity: sha512-RdJUflcE3cUzKiMqQgsCu06FPu9UdIJO0beYbPhHN4k6apgJtifcoCtT9bcxOpYBtpD2kCM6Sbzg4CausW/PKQ==} @@ -3013,6 +3007,8 @@ snapshots: transitivePeerDependencies: - supports-color + '@vscode/codicons@0.0.46-24': {} + '@vscode/test-cli@0.0.12': dependencies: '@types/mocha': 10.0.10 @@ -3850,10 +3846,6 @@ snapshots: filelist: 1.0.6 picocolors: 1.1.1 - jquery.json-viewer@1.5.0: {} - - jquery@3.7.1: {} - js-tokens@4.0.0: {} js-yaml@4.3.2: diff --git a/sess/DESCRIPTION b/sess/DESCRIPTION index f3bbfba8..4b99fa9d 100644 --- a/sess/DESCRIPTION +++ b/sess/DESCRIPTION @@ -1,7 +1,7 @@ Package: sess Type: Package Title: High-Performance IPC Bridge for R Sessions -Version: 3.0.9000.9000 +Version: 3.0.9000.9001 Authors@R: c( person(given = "Randy", family = "Lai", diff --git a/sess/R/handlers.R b/sess/R/handlers.R index 7a64089c..bc96216e 100644 --- a/sess/R/handlers.R +++ b/sess/R/handlers.R @@ -22,7 +22,10 @@ workspace_env_names <- function(env) { } workspace_child_count <- function(object) { - if (is.environment(object)) { + # POSIXlt is list-backed, but [[ returns another date-time, not a child list. + if (inherits(object, "POSIXlt")) { + 0L + } else if (is.environment(object)) { length(workspace_env_names(object)) } else if (isS4(object)) { length(methods::slotNames(object)) @@ -120,6 +123,7 @@ workspace_child_item <- function(object, str, selector) { class = paste(class(object), collapse = ", "), type = typeof(object), has_children = workspace_child_count(object) > 0L, + viewable = TRUE, selector = selector ) } @@ -132,72 +136,360 @@ workspace_child_label <- function(name, index) { } } -get_workspace_children <- function(name, path = list(), start = 1L) { +# Keep one viewer of each type for a workspace root, including its descendants. +workspace_show_view <- function(object, title, owner, context = NULL) { + previous <- .sess_env$view_owner + previous_context <- .sess_env$listview_context + on.exit({ + .sess_env$view_owner <- previous + .sess_env$listview_context <- previous_context + }, add = TRUE) + .sess_env$view_owner <- owner + .sess_env$listview_context <- context + utils::View(object, title = title) + TRUE +} + +handle_workspace_view <- function(name, path = list()) { + root <- listview_state(workspace_object(name), name, name) + context <- listview_context(root, path) + workspace_show_view(context$data, context$navigation$title, name, context) +} + +# Lists and vectors share one viewer; only their row presentation differs. +listview_supported <- function(object) { + !dataview_is_table(object) && + (is.list(object) || is.pairlist(object) || is.environment(object) || isS4(object) || + listview_is_vector(object)) +} + +listview_is_vector <- function(object) { + !dataview_is_table(object) && (is.atomic(object) || inherits(object, "POSIXlt")) +} + +listview_state <- function(object, title, owner) { + kind <- if (is.environment(object)) "name" else if (isS4(object)) "slot" else "index" + list( + type = "list", data = object, title = title, owner = owner, kind = kind, + names = switch(kind, + name = workspace_env_names(object), + slot = methods::slotNames(object), + index = names(object) + ), + child_names = new.env(parent = emptyenv()) + ) +} + +# Resolve workspace selectors once; navigation paths are relative to the retained root. +listview_context <- function(root, selectors, expression_env = NULL) { + location <- listview_location( + root, selectors, resolve_selectors = TRUE, expression_env = expression_env + ) + list(root = root, data = location$data, navigation = listview_navigation(location)) +} + +# Read a binding without invoking an active binding for a second time. +listview_binding <- function(name, envir) { + while (!identical(envir, emptyenv()) && !exists(name, envir = envir, inherits = FALSE)) { + envir <- parent.env(envir) + } + if (identical(envir, emptyenv()) || bindingIsActive(name, envir)) return(NULL) + get(name, envir = envir, inherits = FALSE) +} + +# Only known structural extraction can be represented by an index breadcrumb. +listview_structural_extraction <- function(object, operator, envir) { + if (operator == "@") return(isS4(object)) + if (!is.object(object)) return(TRUE) + if (isS4(object) || !identical(class(object), "data.frame")) return(FALSE) + if (operator == "$" && + !is.null(utils::getS3method("$", "data.frame", optional = TRUE, envir = envir))) { + return(FALSE) + } + # Check both the caller's extraction and the viewer's later indexed traversal. + all(vapply(list(envir, environment(listview_location)), function(method_env) { + identical(utils::getS3method("[[", "data.frame", optional = TRUE, envir = method_env), + base::`[[.data.frame`) + }, logical(1))) +} + +# Direct View(x$a) calls can also start with a complete breadcrumb path. +listview_expression_context <- function(expression, envir, owner, value) { tryCatch({ - object <- workspace_object(name, path) - child_count <- workspace_child_count(object) - if (child_count == 0L) { - return(list(children = I(list()), next_start = NULL)) + selectors <- list() + while (is.call(expression) && length(expression) == 3L && + is.symbol(expression[[1L]])) { + operator <- as.character(expression[[1L]]) + if (!operator %in% c("$", "[[", "@")) return(NULL) + if (!identical(listview_binding(operator, envir), get(operator, envir = baseenv()))) { + return(NULL) + } + selector <- expression[[3L]] + if (operator %in% c("$", "@") && is.symbol(selector)) selector <- as.character(selector) + if (length(selector) != 1L || !(is.character(selector) || is.numeric(selector))) return(NULL) + selectors <- c(list(list(kind = operator, value = selector)), selectors) + expression <- expression[[2L]] + } + if (!length(selectors) || !is.symbol(expression)) return(NULL) + name <- as.character(expression) + object <- listview_binding(name, envir) + if (is.null(object) || !listview_structural_extraction(object, selectors[[1L]]$kind, envir)) { + return(NULL) + } + root <- listview_state(object, name, owner) + context <- listview_context(root, selectors, expression_env = envir) + # Even a safe path is optional: the already-evaluated argument is authoritative. + if (!identical(context$data, value)) return(NULL) + context + }, error = function(e) NULL) +} + +listview_navigation <- function(location) { + list( + title = location$title, path = I(location$path), breadcrumbs = I(location$breadcrumbs), + vector = listview_is_vector(location$data) + ) +} + +handle_listview_navigate <- function(view_id, path = list()) { + state <- dataview_get_state(view_id) + if (!identical(state$type, "list")) stop("Not a list view") + location <- listview_location(state, path) + if (dataview_is_table(location$data)) { + return(workspace_show_view( + location$data, + location$title, + state$owner %||% state$title + )) + } + if (!listview_supported(location$data)) stop("Not a list view") + listview_navigation(location) +} + +listview_location <- function(state, path = list(), resolve_selectors = FALSE, + expression_env = NULL) { + visited <- integer() + state$breadcrumbs <- list(list(label = state$title, path = I(list()))) + for (index in path) { + if (resolve_selectors) { + if (!is.null(expression_env) && + !listview_structural_extraction(state$data, index$kind, expression_env)) { + stop("Cannot reconstruct custom extraction as an index path") + } + index <- if (index$kind %in% c("index", "[[") && is.numeric(index$value)) { + index$value + } else if (index$kind == "$" && state$kind == "index") { + pmatch(index$value, state$names, duplicates.ok = TRUE) + } else { + match(index$value, state$names) + } + } + child_count <- if (state$kind == "index") length(state$data) else length(state$names) + if (length(index) != 1L || !is.numeric(index) || is.na(index) || + index != as.integer(index) || index < 1L || index > child_count) { + stop("Invalid list item index") + } + child_name <- if (is.null(state$names)) NULL else state$names[[index]] + if (state$kind == "name" && + (!exists(child_name, envir = state$data, inherits = FALSE) || + bindingIsActive(child_name, state$data))) { + stop("List item is unavailable") + } + child <- switch(state$kind, + name = get(child_name, envir = state$data, inherits = FALSE), + slot = methods::slot(state$data, child_name), + index = state$data[[index]] + ) + state$title <- if (state$kind == "slot") { + paste0(state$title, "@", child_name) + } else if (!is.null(child_name) && !is.na(child_name) && nzchar(child_name)) { + paste0(state$title, "$", child_name) + } else { + paste0(state$title, "[[", index, "]]") + } + state$data <- child + state$kind <- if (is.environment(child)) "name" else if (isS4(child)) "slot" else "index" + visited <- c(visited, index) + state$names <- switch(state$kind, + name = { + # Reuse the binding snapshot without enumerating the environment again. + key <- paste(visited, collapse = "/") + if (is.environment(state$child_names) && + exists(key, envir = state$child_names, inherits = FALSE)) { + get(key, envir = state$child_names, inherits = FALSE) + } else { + child_names <- workspace_env_names(child) + if (is.environment(state$child_names)) assign(key, child_names, envir = state$child_names) + child_names + } + }, + slot = methods::slotNames(child), + index = names(child) + ) + label <- if (!is.null(child_name) && !is.na(child_name) && nzchar(child_name)) { + child_name + } else { + paste0("[[", index, "]]") } + state$breadcrumbs <- c(state$breadcrumbs, list(list(label = label, path = I(as.list(visited))))) + } + state$path <- as.list(visited) + state +} + +listview_format_values <- function(values) { + # Format before extracting individual elements, which may discard their class. + formatted <- dataview_format_column(values) + if (is.character(values) && !is.object(values)) { + return(encodeString(formatted, quote = "\"", na.encode = TRUE)) + } + if (is.numeric(formatted) && !is.object(formatted)) { + # Match the numeric precision sent to the data viewer over IPC. + formatted <- vapply(formatted, function(value) { + if (is.finite(value)) { + as.character(jsonlite::toJSON(value, auto_unbox = TRUE, digits = NA)) + } else { + as.character(value) + } + }, character(1)) + } else if (!is.character(formatted)) { + formatted <- format(formatted, trim = TRUE, justify = "none") + } + formatted[is.na(formatted)] <- "NA" + formatted +} + +listview_summary <- function(object) { + if (is.atomic(object) || inherits(object, "POSIXlt")) { + size <- length(object) + type <- if (is.object(object)) { + paste(class(object), collapse = "/") + } else { + switch(typeof(object), integer = "int", double = "num", complex = "cplx", + logical = "logi", character = "chr", raw = "raw") + } + if (!size) return(paste0(type, "(0)")) + shape <- if (!is.null(dim(object))) { + paste0(" [", paste0("1:", dim(object), collapse = ", "), "]") + } else if (size > 1L) { + paste0(" [1:", size, "]") + } + paste0(type, shape, " ", + listview_format_values(object[1L]), if (size > 1L) " ...") + } else { + trimws(try_capture_str(object)) + } +} +get_workspace_children <- function(name = NULL, path = list(), start = 1L, view_id = NULL) { + tryCatch({ + vector_rows <- FALSE + if (is.null(view_id)) { + object <- workspace_object(name, path) + kind <- if (is.environment(object)) "name" else if (isS4(object)) "slot" else "index" + child_names <- switch(kind, + name = workspace_env_names(object), + slot = methods::slotNames(object), + index = names(object) + ) + } else { + state <- dataview_get_state(view_id) + if (!identical(state$type, "list")) stop("Not a list view") + state <- listview_location(state, path) + object <- state$data + vector_rows <- listview_is_vector(object) + kind <- if (vector_rows) "index" else state$kind + child_names <- if (vector_rows) names(object) else state$names + } + child_count <- if (kind == "index") { + if (vector_rows) { + length(object) + } else { + workspace_child_count(object) + } + } else { + length(child_names) + } start <- max(1L, as.integer(start)) end <- min(child_count, start + workspace_child_page_size - 1L) if (start > end) { return(list(children = I(list()), next_start = NULL)) } + formatted_values <- if (vector_rows) listview_format_values(object[seq.int(start, end)]) - children <- if (is.environment(object)) { - child_names <- workspace_env_names(object)[seq.int(start, end)] - lapply(child_names, function(child_name) { - if (bindingIsActive(child_name, object)) { - list( - str = paste0("$ ", child_name, ": (active-binding)"), - class = "active_binding", - type = "active_binding", - has_children = FALSE - ) + children <- lapply(seq.int(start, end), function(index) { + child_name <- if (is.null(child_names)) NULL else child_names[[index]] + label <- if (vector_rows) { + if (!is.null(child_name) && !is.na(child_name) && nzchar(child_name)) { + child_name } else { - child <- get(child_name, envir = object, inherits = FALSE) - workspace_child_item( - child, - paste0("$ ", child_name, ": ", trimws(try_capture_str(child))), - list(kind = "name", value = child_name) - ) + paste0("[", index, "]") } - }) - } else if (isS4(object)) { - child_names <- methods::slotNames(object)[seq.int(start, end)] - lapply(child_names, function(child_name) { - child <- methods::slot(object, child_name) - workspace_child_item( - child, - paste0("@ ", child_name, ": ", trimws(try_capture_str(child))), - list(kind = "slot", value = child_name) - ) - }) - } else { - indices <- seq.int(start, end) - child_names <- names(object) - lapply(indices, function(index) { - child <- object[[index]] - child_name <- if (is.null(child_names)) NULL else child_names[[index]] - workspace_child_item( - child, - paste0( - workspace_child_label(child_name, index), - ": ", - trimws(try_capture_str(child)) - ), - list(kind = "index", value = index) + } else if (kind == "slot") { + paste0("@ ", child_name) + } else { + workspace_child_label(child_name, index) + } + unavailable <- if (kind == "name" && !exists(child_name, envir = object, inherits = FALSE)) { + "removed" + } else if (kind == "name" && bindingIsActive(child_name, object)) { + "active_binding" + } else { + NULL + } + if (!is.null(unavailable)) { + summary <- if (unavailable == "removed") "(removed)" else "(active-binding)" + if (!is.null(view_id)) { + return(list(label = label, str = summary, viewable = FALSE, index = index)) + } + return(list( + str = paste0(label, ": ", summary), class = unavailable, + type = unavailable, has_children = FALSE + )) + } + child <- if (!vector_rows) switch(kind, + name = get(child_name, envir = object, inherits = FALSE), + slot = methods::slot(object, child_name), + index = object[[index]] + ) + summary <- if (vector_rows) { + formatted_values[[index - start + 1L]] + } else { + listview_summary(child) + } + if (!is.null(view_id)) { + list( + label = label, str = summary, viewable = !vector_rows, index = index, + has_children = !vector_rows && workspace_child_count(child) > 0L ) - }) - } + } else { + selector <- if (kind == "index") { + list(kind = kind, value = index, name = child_name) + } else { + list(kind = kind, value = child_name) + } + workspace_child_item(child, paste0(label, ": ", summary), selector) + } + }) + list(children = I(children), next_start = if (end < child_count) end + 1L else NULL) + }, error = function(e) { + if (!is.null(view_id)) stop(e) + list(children = I(list()), next_start = NULL) + }) +} - list( - children = I(children), - next_start = if (end < child_count) end + 1L else NULL - ) - }, error = function(e) list(children = I(list()), next_start = NULL)) +handle_listview_view <- function(view_id, index, path = list()) { + state <- .sess_env$dataviews[[as.character(view_id)]] + if (is.null(state) || !identical(state$type, "list")) { + return(FALSE) + } + path <- c(path, list(index)) + location <- tryCatch(listview_location(state, path), error = function(e) NULL) + if (is.null(location)) return(FALSE) + if (listview_supported(location$data)) { + return(listview_navigation(location)) + } + workspace_show_view(location$data, location$title, state$owner %||% state$title) } handle_hover <- function(expr_str) { @@ -512,11 +804,16 @@ dataview_new_id <- function() { } } -dataview_register <- function(data, view_id = NULL, live = FALSE) { +dataview_set_state <- function(view_id, state) { if (is.null(.sess_env$dataviews)) { .sess_env$dataviews <- list() } + state$state_generation <- (.sess_env$dataviews[[view_id]]$state_generation %||% 0L) + 1L + .sess_env$dataviews[[view_id]] <- state + state +} +dataview_register <- function(data, view_id = NULL, live = FALSE) { if (is.null(view_id)) { view_id <- dataview_new_id() } @@ -524,9 +821,10 @@ dataview_register <- function(data, view_id = NULL, live = FALSE) { state <- dataview_to_state(data) state$live <- live state$revision <- .sess_env$dataview_revision %||% 0 - .sess_env$dataviews[[view_id]] <- state + state <- dataview_set_state(view_id, state) list( view_id = view_id, + state_generation = state$state_generation, total_rows = state$total_rows, columns = dataview_columns(state) ) @@ -547,6 +845,7 @@ dataview_get_state <- function(view_id, refresh = FALSE) { !identical(state$total_rows, current$total_rows)) { current$live <- TRUE current$revision <- revision + current$state_generation <- state$state_generation state <- current .sess_env$dataviews[[view_id]] <- state } @@ -976,7 +1275,9 @@ handle_dataview_page <- function(params) { handle_dataview_dispose <- function(params) { view_id <- as.character(params$view_id %||% "") - if (!is.null(.sess_env$dataviews) && !is.null(.sess_env$dataviews[[view_id]])) { + state <- if (is.null(.sess_env$dataviews)) NULL else .sess_env$dataviews[[view_id]] + state_generation <- suppressWarnings(as.integer(params$state_generation %||% NA_integer_)) + if (!is.null(state) && identical(state$state_generation, state_generation)) { .sess_env$dataviews[[view_id]] <- NULL } TRUE diff --git a/sess/R/hooks.R b/sess/R/hooks.R index 62e3ed90..549c59ab 100644 --- a/sess/R/hooks.R +++ b/sess/R/hooks.R @@ -103,49 +103,82 @@ runtime_start <- function(use_rstudioapi = TRUE, } show_dataview <- function(x, title = deparse(substitute(x))) { - # make sure title is computed. + # Capture the root before forcing x so View(x$a) shares the viewer for x. + original_expression <- substitute(x) + expression <- original_expression + while (is.call(expression) && is.symbol(expression[[1L]]) && + as.character(expression[[1L]]) %in% c("$", "[[", "@")) { + expression <- expression[[2L]] + } + owner <- .sess_env$view_owner + if (is.null(owner) && missing(title) && is.symbol(expression)) { + owner <- as.character(expression) + } force(title) - if (isTRUE(.sess_env$interactive_connected) && .interactive_rich_value(x)) { + if (is.null(.sess_env$view_owner) && isTRUE(.sess_env$interactive_connected) && + .interactive_rich_value(x)) { return(invisible(NULL)) } - if (dataview_is_table(x)) { - title_key <- paste(as.character(title), collapse = "\n") - dataview_registry <- .sess_env$dataview_registry - has_view_id <- nzchar(title_key) && - exists(title_key, envir = dataview_registry, inherits = FALSE) - view_id <- if (has_view_id) { - get(title_key, envir = dataview_registry, inherits = FALSE) - } else { - id <- dataview_new_id() - if (nzchar(title_key)) { - assign(title_key, id, envir = dataview_registry) - } - id + view_type <- if (dataview_is_table(x)) { + "table" + } else if (listview_supported(x)) { + "list" + } else { + "object" + } + title_key <- paste(as.character(title), collapse = "\n") + owner <- owner %||% title_key + registry_key <- paste0(view_type, ":", owner) + dataview_registry <- .sess_env$dataview_registry + view_id <- if (nzchar(title_key) && + exists(registry_key, envir = dataview_registry, inherits = FALSE)) { + get(registry_key, envir = dataview_registry, inherits = FALSE) + } else { + id <- dataview_new_id() + if (nzchar(title_key)) { + assign(registry_key, id, envir = dataview_registry) } + id + } + if (view_type == "table") { registration <- dataview_register(x, view_id = view_id) notify_client("dataview", list( title = title, source = "table", type = "json", - view_id = registration$view_id + view_id = registration$view_id, + state_generation = registration$state_generation )) - } else if (is.list(x)) { - file_path <- tempfile(tmpdir = .sess_env$tempdir, fileext = ".json") - jsonlite::write_json(x, file_path, auto_unbox = TRUE, null = "null", na = "string") + } else if (view_type == "list") { + context <- .sess_env$listview_context + if (is.null(context) && missing(title)) { + context <- listview_expression_context(original_expression, parent.frame(), owner, x) + } + root <- if (is.null(context)) listview_state(x, title_key, owner) else context$root + navigation <- if (is.null(context)) { + listview_navigation(listview_location(root)) + } else { + context$navigation + } + root <- dataview_set_state(view_id, root) notify_client("dataview", list( title = title, - file = file_path, - source = "list", - type = "json" + source = view_type, + type = "json", + view_id = view_id, + state_generation = root$state_generation, + navigation = navigation )) } else { code <- if (is.primitive(x)) utils::capture.output(print(x)) else deparse(x) - file_path <- tempfile(tmpdir = .sess_env$tempdir, fileext = ".R") + file_path <- .sess_env$dataviews[[view_id]]$file %||% + tempfile(tmpdir = .sess_env$tempdir, fileext = ".R") writeLines(code, file_path) + .sess_env$dataviews[[view_id]] <- list(type = "object", file = file_path) notify_client("dataview", list( title = title, file = file_path, diff --git a/sess/R/server.R b/sess/R/server.R index 86b5f195..1882ad2f 100644 --- a/sess/R/server.R +++ b/sess/R/server.R @@ -256,7 +256,7 @@ connect <- function(endpoint = NULL, use_rstudioapi = TRUE, use_httpgd = NULL, host <- Sys.info()[["nodename"]] if (is.null(host) || is.na(host)) host <- "" list( - protocol_version = 1L, + protocol_version = 2L, interactive_token = .sess_env$interactive_token, sess_version = as.character(utils::packageVersion("sess")), session_id = .session_id(), @@ -450,10 +450,13 @@ dispatch_message <- function(line) { TRUE }, "workspace" = function(p) get_workspace_data(), - "workspace_children" = function(p) get_workspace_children(p$name, p$path, p$start), + "workspace_children" = function(p) get_workspace_children(p$name, p$path, p$start, p$view_id), + "workspace_view" = function(p) handle_workspace_view(p$name, p$path), "hover" = function(p) handle_hover(p$expr), "completion" = function(p) handle_complete(p$expr, p$trigger), "plot_latest" = function(p) handle_plot_latest(p), + "listview_navigate" = function(p) handle_listview_navigate(p$view_id, p$path), + "listview_view" = function(p) handle_listview_view(p$view_id, p$index, p$path), "dataview_init" = function(p) handle_dataview_init(p), "dataview_page" = function(p) handle_dataview_page(p), "dataview_dispose" = function(p) handle_dataview_dispose(p) diff --git a/sess/README.md b/sess/README.md index f2b94868..c999fe1a 100644 --- a/sess/README.md +++ b/sess/README.md @@ -193,7 +193,7 @@ On connecting, `sess` sends an `attach` notification: "jsonrpc": "2.0", "method": "attach", "params": { - "protocol_version": 1, + "protocol_version": 2, "sess_version": "3.0.0", "session_id": "sess-session-...", "host": "compute42", @@ -217,6 +217,12 @@ The attached socket's lifetime controls cleanup. A replacement socket with the same identity supersedes the old one; a late close cannot remove its replacement. A fork child receives its own identity. Terminal PID association is local-only. +Protocol version 2 includes the viewer-state generation contract: dynamic table +and list `dataview` notifications provide `state_generation`, and +`dataview_dispose` requires it. Both the extension and `sess` must support version +2. The extension/webview `documentGeneration` field is internal to the client +and is not part of the `sess` IPC contract. + ### Notifications from R to client Sent with `notify_client()`. @@ -225,7 +231,7 @@ Sent with `notify_client()`. |---|---|---| | `attach` | see above | Connection is established. | | `workspace_updated` | none | A top-level command completes. | -| `dataview` | `title`, `source`, `type`, and `view_id` (tables) or `file` (other objects) | `View()` is called. | +| `dataview` | `title`, `source`, `type`; `view_id` and `state_generation` for tables/lists, plus `navigation` for lists; `file` for other objects | `View()` is called. | | `plot_updated` | none | The standard device records a new or changed plot. | | `httpgd` | `url` | An httpgd device is opened. | | `help` | `requestPath` | A help page or help search is printed. | @@ -258,7 +264,14 @@ arrives. All are used by `rstudioapi` emulation: | `plot_latest` | `width`, `height`, `format` (`svglite` or `png`), `devArgs` | `format`, `data` (base64) | | `dataview_init` | `view_id` | `columns`, `totalRows` | | `dataview_page` | `view_id`, `startRow`, `endRow`, `sortModel`, `filterModel` | `rows`, `totalRows`, `totalUnfiltered`, `lastRow` | -| `dataview_dispose` | `view_id` | `true` | +| `dataview_dispose` | `view_id`, `state_generation` | `true` | + +`state_generation` is an integer issued by `sess` when viewer data is registered. +It changes when data for an existing `view_id` is replaced. When closing a viewer, +the client sends the `state_generation` from its latest `dataview` notification. +Disposal deletes data only if the generation matches the current registration; +a missing or mismatched generation leaves the data intact and still returns +`true`. This prevents a delayed close request from deleting replacement data. Example exchange: diff --git a/sess/inst/tinytest/test-dataview.R b/sess/inst/tinytest/test-dataview.R index 07433a65..512998d2 100644 --- a/sess/inst/tinytest/test-dataview.R +++ b/sess/inst/tinytest/test-dataview.R @@ -322,3 +322,92 @@ local({ expect_identical(page$rows[["2"]], df$Ozone[c(4L, 3L), , drop = FALSE]) expect_identical(serialize(df, NULL), original) }) + +# Workspace children expose View for both structured and text-viewable objects. +local({ + selector <- list(kind = "index", value = 1L) + expect_true(sess:::workspace_child_item(data.frame(x = 1), "df", selector)$viewable) + expect_true(sess:::workspace_child_item(matrix(1:4, 2), "matrix", selector)$viewable) + expect_true(sess:::workspace_child_item(list(x = 1), "list", selector)$viewable) + expect_true(sess:::workspace_child_item(new.env(), "environment", selector)$viewable) + expect_true(sess:::workspace_child_item(pairlist(x = 1), "pairlist", selector)$viewable) + methods::setClass("list_viewer_test_slots", slots = c(child = "list")) + on.exit(methods::removeClass("list_viewer_test_slots"), add = TRUE) + object <- methods::new("list_viewer_test_slots", child = list(x = 1)) + expect_true(sess:::workspace_child_item(object, "S4", selector)$viewable) + expect_true(sess:::workspace_child_item(1:3, "vector", selector)$viewable) +}) + +# List pages inspect only the requested children, with stable indices across pages. +local({ + runtime <- sess:::.sess_env + previous <- runtime$dataviews + on.exit(runtime$dataviews <- previous, add = TRUE) + object <- new.env(parent = emptyenv()) + child_names <- paste0("item", seq_len(501L)) + for (name in child_names[1:500]) assign(name, 1L, envir = object) + delayedAssign("item501", stop("unrequested binding was evaluated"), assign.env = object) + runtime$dataviews$paging_test <- list( + type = "list", data = object, kind = "name", names = child_names, title = "object" + ) + + first <- sess:::get_workspace_children(view_id = "paging_test", start = 1L) + expect_length(first$children, 500L) + expect_equal(first$next_start, 501L) + expect_equal(vapply(first$children, `[[`, 1L, "index"), 1:500) + + # Removing a binding must not shift later indices or fail the next page. + rm("item501", envir = object) + last <- sess:::get_workspace_children(view_id = "paging_test", start = 501L) + expect_equal(last$children[[1L]]$label, "$ item501") + expect_false(last$children[[1L]]$viewable) + expect_null(last$next_start) + makeActiveBinding("item501", function() stop("active binding was evaluated"), object) + last <- sess:::get_workspace_children(view_id = "paging_test", start = 501L) + expect_equal(last$children[[1L]]$str, "(active-binding)") + expect_false(last$children[[1L]]$viewable) + expect_length(sess:::get_workspace_children(view_id = "paging_test", start = 502L)$children, 0L) +}) + + +# A delayed panel close cannot dispose state recreated under the same viewer id. +local({ + runtime <- sess:::.sess_env + previous <- runtime$dataviews + on.exit(runtime$dataviews <- previous, add = TRUE) + + cases <- list( + list = list( + first = sess:::listview_state(list(value = 1L), "x", "x"), + replacement = sess:::listview_state(list(value = 2L), "x", "x") + ), + table = list( + first = sess:::dataview_to_state(data.frame(value = 1L)), + replacement = sess:::dataview_to_state(data.frame(value = 2L)) + ) + ) + + for (name in names(cases)) { + runtime$dataviews <- list() + first <- sess:::dataview_set_state("replacement_test", cases[[name]]$first) + replacement <- sess:::dataview_set_state( + "replacement_test", cases[[name]]$replacement + ) + expect_true(replacement$state_generation > first$state_generation, info = name) + + expect_true(sess:::handle_dataview_dispose(list( + view_id = "replacement_test", state_generation = first$state_generation + )), info = name) + expect_identical( + runtime$dataviews$replacement_test$state_generation, replacement$state_generation, info = name + ) + expect_identical( + runtime$dataviews$replacement_test$data, cases[[name]]$replacement$data, info = name + ) + + expect_true(sess:::handle_dataview_dispose(list( + view_id = "replacement_test", state_generation = replacement$state_generation + )), info = name) + expect_null(runtime$dataviews$replacement_test, info = name) + } +}) diff --git a/sess/inst/tinytest/test-interactive-large-table.R b/sess/inst/tinytest/test-interactive-large-table.R index 8e1e9ff0..45799fea 100644 --- a/sess/inst/tinytest/test-interactive-large-table.R +++ b/sess/inst/tinytest/test-interactive-large-table.R @@ -22,6 +22,9 @@ local({ expect_identical(small$value$x1, 1:1000) expect_true(as.numeric(object.size(small$value)) < 200000) full <- register(large, live = TRUE)$view_id + generation <- env$dataviews[[full]]$state_generation + sess:::handle_dataview_init(list(view_id = full)) + expect_identical(env$dataviews[[full]]$state_generation, generation) expect_identical(page(full, startRow = n - 2L, endRow = n)$rows[["1"]], (n - 1L):n) expect_null(env$dataviews[[full]]$query_indices) diff --git a/sess/inst/tinytest/test-ipc.R b/sess/inst/tinytest/test-ipc.R index ef862e29..372e34f0 100644 --- a/sess/inst/tinytest/test-ipc.R +++ b/sess/inst/tinytest/test-ipc.R @@ -158,7 +158,10 @@ local({ response <- jsonlite::fromJSON(processx::conn_read_chars(pipe[[1L]])) expect_true(all(abs(response$result$rows[["1"]] - expected) <= abs(expected) * 1e-14)) - disposed <- sess:::handle_dataview_dispose(list(view_id = registration$view_id)) + disposed <- sess:::handle_dataview_dispose(list( + view_id = registration$view_id, + state_generation = registration$state_generation + )) expect_true(isTRUE(disposed)) expect_error( sess:::handle_dataview_init(list(view_id = registration$view_id)), @@ -264,10 +267,10 @@ local({ expect_false(identical(getHook("grid.newpage"), old_grid_hook)) dataview_data <- data.frame(value = 1:2) - assign("lifecycle dataview", "runtime_view_before_restart", + assign("table:lifecycle dataview", "runtime_view_before_restart", envir = .sess_env$dataview_registry) utils::View(dataview_data, title = "lifecycle dataview") - first_view_id <- get("lifecycle dataview", envir = .sess_env$dataview_registry) + first_view_id <- get("table:lifecycle dataview", envir = .sess_env$dataview_registry) expect_identical(first_view_id, "runtime_view_before_restart") expect_true(first_view_id %in% names(.sess_env$dataviews)) @@ -284,7 +287,7 @@ local({ length(grep("^sess.plot$", callbacks_after_first_start))) utils::View(dataview_data, title = "lifecycle dataview") - second_view_id <- get("lifecycle dataview", envir = .sess_env$dataview_registry) + second_view_id <- get("table:lifecycle dataview", envir = .sess_env$dataview_registry) expect_false(identical(first_view_id, second_view_id)) expect_true(second_view_id %in% names(.sess_env$dataviews)) diff --git a/sess/inst/tinytest/test-listview-format.R b/sess/inst/tinytest/test-listview-format.R new file mode 100644 index 00000000..56f257c1 --- /dev/null +++ b/sess/inst/tinytest/test-listview-format.R @@ -0,0 +1,113 @@ +# List rows use the same class formatting and numeric precision as table cells. +local({ + runtime <- sess:::.sess_env + previous <- runtime$dataviews + previous_options <- options(digits = 3L) + on.exit({ + runtime$dataviews <- previous + options(previous_options) + }, add = TRUE) + page <- function(object, path = list(), start = 1L) { + runtime$dataviews$format_test <- sess:::listview_state(object, "x", "x") + sess:::get_workspace_children(view_id = "format_test", path = path, start = start) + } + text <- function(page) vapply(page$children, `[[`, "", "str") + + numbers <- c(1.23456789012345, 1.54e-100, -6.65e-13, 1.23456789012345e14, + 0, NA_real_, NaN, Inf, -Inf) + expected <- c("1.23456789012345", "1.54e-100", "-6.65e-13", "123456789012345", + "0", "NA", "NaN", "Inf", "-Inf") + expect_equal(text(page(numbers)), expected) + expect_equal(text(page(list(nested = numbers), list(1L))), expected) + expect_equal(text(page(as.list(numbers))), paste("num", expected)) + for (index in seq_along(numbers)) { + expect_true(sess:::listview_supported(numbers[index])) + expect_true(sess:::listview_is_vector(numbers[index])) + expect_equal(text(page(numbers[index])), expected[index]) + expect_false(page(numbers[index])$children[[1L]]$has_children) + expect_false(page(numbers[index])$children[[1L]]$viewable) + } + expect_true(grepl(expected[[1L]], text(page(list(numbers)))[[1L]], fixed = TRUE)) + expect_equal(text(page(list(matrix(rep(numbers[[1L]], 4L), nrow = 2L)))), + paste0("num [1:2, 1:2] ", expected[[1L]], " ...")) + expect_identical(numbers, c( + 1.23456789012345, 1.54e-100, -6.65e-13, 1.23456789012345e14, 0, NA_real_, NaN, Inf, -Inf + )) + expect_equal(text(page(c("001", NA_character_))), c('"001"', "NA")) + + registerS3method("[", "listview_number", function(x, ...) { + structure(NextMethod(), class = class(x), unit = attr(x, "unit")) + }) + registerS3method("format", "listview_number", function(x, ...) { + paste0(attr(x, "unit"), ":", sprintf("%.9f", as.numeric(x))) + }) + registerS3method("[", "listview_label", function(x, ...) { + structure(NextMethod(), class = class(x)) + }) + registerS3method("format", "listview_label", function(x, ...) { + paste0("ID:", as.character(x)) + }) + class_env <- environment() + methods::setClass("listview_s4_number", contains = "numeric", where = class_env) + methods::setMethod("[", "listview_s4_number", function(x, i, j, ..., drop = TRUE) { + methods::new("listview_s4_number", as.numeric(x)[i]) + }, where = class_env) + registerS3method("format", "listview_s4_number", function(x, ...) { + paste0("S4:", sprintf("%.9f", as.numeric(x))) + }) + on.exit({ + methods::removeMethod("[", "listview_s4_number", where = class_env) + methods::removeClass("listview_s4_number", where = class_env) + rm(list = c("[.listview_number", "format.listview_number", "format.listview_s4_number", + "[.listview_label", "format.listview_label"), + envir = get(".__S3MethodsTable__.", asNamespace("base"))) + }, add = TRUE) + + cases <- list( + S3 = structure(c(1.234567891, NA_real_), class = "listview_number", unit = "USD"), + S4 = methods::new("listview_s4_number", c(1.234567891, NA_real_)), + difftime = as.difftime(c(60, NA), units = "secs"), + factor = factor(c("first", NA)), + date = as.Date(c("2026-01-01", NA)), + character_class = structure(c("001", NA_character_), class = "listview_label"), + complex = c(1 + 2i, NA_complex_) + ) + if (requireNamespace("bit64", quietly = TRUE)) { + cases$integer64 <- bit64::as.integer64(c("9007199254740993", NA)) + } + for (name in names(cases)) { + values <- cases[[name]] + original <- serialize(values, NULL) + table <- data.frame(id = seq_along(values)) + table$value <- values + table_text <- as.character(sess:::dataview_rows( + sess:::dataview_to_state(table), seq_along(values) + )[["2"]]) + table_text[is.na(table_text)] <- "NA" + expect_equal(text(page(values)), table_text, info = name) + expect_equal(text(page(list(value = values), list(1L))), table_text, info = name) + expect_true(grepl(table_text[[1L]], text(page(list(values[1L])))[[1L]], fixed = TRUE), + info = name) + expect_identical(serialize(values, NULL), original, info = name) + scalar <- values[1L] + expect_true(sess:::listview_supported(scalar), info = name) + expect_true(sess:::listview_is_vector(scalar), info = name) + expect_equal(text(page(scalar)), table_text[1L], info = name) + page(list(value = scalar)) + navigation <- sess:::handle_listview_view("format_test", 1L) + expect_true(navigation$vector, info = name) + expect_equal(navigation$path, list(1L), check.attributes = FALSE, info = name) + expect_false(page(scalar)$children[[1L]]$has_children, info = name) + expect_false(page(scalar)$children[[1L]]$viewable, info = name) + } + + values <- structure(rep(1.234567891, 501L), class = "listview_number", unit = "USD") + names(values) <- paste0("item", seq_along(values)) + first <- page(values) + last <- page(values, start = 501L) + expect_length(first$children, 500L) + expect_equal(first$next_start, 501L) + expect_equal(unique(text(first)), "USD:1.234567891") + expect_equal(text(last), "USD:1.234567891") + expect_equal(last$children[[1L]]$label, "item501") +}) diff --git a/sess/inst/tinytest/test-listview.R b/sess/inst/tinytest/test-listview.R new file mode 100644 index 00000000..abb07121 --- /dev/null +++ b/sess/inst/tinytest/test-listview.R @@ -0,0 +1,579 @@ +# Expanding and opening descendants keeps one list/table viewer per root object. +local({ + runtime <- sess:::.sess_env + previous_con <- runtime$con + previous_views <- runtime$dataviews + previous_registry <- runtime$dataview_registry + root <- paste0(".sess_listview_test_", Sys.getpid()) + other <- paste0(root, "_other") + table_root <- paste0(root, "_table") + pipe <- processx::conn_create_pipepair() + on.exit({ + sess:::runtime_stop() + runtime$con <- previous_con + runtime$dataviews <- previous_views + runtime$dataview_registry <- previous_registry + rm( + list = intersect(c(root, other, table_root), ls(envir = .GlobalEnv, all.names = TRUE)), + envir = .GlobalEnv + ) + lapply(pipe, close) + }, add = TRUE) + runtime$con <- pipe[[2L]] + sess:::runtime_start(use_rstudioapi = FALSE, plot_backend = "standard") + x <- list(a = list(b = list(value = 1L), df = data.frame(nested = 2L)), + df = data.frame(top = 1L)) + assign(root, x, envir = .GlobalEnv) + assign(other, x, envir = .GlobalEnv) + + # Exercise the JSON-RPC methods used by both the workspace and list webviews. + notifications <- list() + read_messages <- function() { + messages <- strsplit(processx::conn_read_chars(pipe[[1L]]), "\n", fixed = TRUE)[[1L]] + responses <- lapply(messages[nzchar(messages)], jsonlite::fromJSON, simplifyVector = FALSE) + notifications <<- Filter(function(message) identical(message$method, "dataview"), responses) + responses + } + notification <- function() { + if (length(notifications)) tail(notifications, 1L)[[1L]]$params else NULL + } + request <- function(method, params) { + sess:::dispatch_message(as.character(jsonlite::toJSON( + list(jsonrpc = "2.0", id = "list-test", method = method, params = params), + auto_unbox = TRUE + ))) + responses <- read_messages() + replies <- Filter(function(message) identical(message$id, "list-test"), responses) + expect_length(replies, 1L) + expect_null(replies[[1L]]$error) + replies[[1L]]$result + } + id <- function(type, name = root) get(paste0(type, ":", name), runtime$dataview_registry) + selector <- function(index, name) list(kind = "index", value = index, name = name) + + expect_true(request("workspace_view", list(name = root))) + list_id <- id("list") + page <- request("workspace_children", list(view_id = list_id, start = 1L)) + expect_true(page$children[[1L]]$has_children) + expect_true(page$children[[1L]]$viewable) + expect_true(page$children[[2L]]$has_children) + expect_true(page$children[[2L]]$viewable) + + nested <- request("workspace_children", list(view_id = list_id, path = list(1L), start = 1L)) + expect_equal(vapply(nested$children, `[[`, "", "label"), c("$ b", "$ df")) + expect_identical(runtime$dataviews[[list_id]]$data, x) + expect_length(runtime$dataviews, 1L) + leaf <- request("workspace_children", list(view_id = list_id, path = list(1L, 1L), start = 1L)) + expect_false(leaf$children[[1L]]$has_children) + expect_true(leaf$children[[1L]]$viewable) + + # Open x$df, then x$a$df from an expanded row: the table id is unchanged. + expect_true(request("listview_view", list(view_id = list_id, index = 2L, path = list()))) + table_id <- id("table") + expect_false(identical(table_id, list_id)) + expect_true(request("listview_view", list(view_id = list_id, index = 2L, path = list(1L)))) + expect_identical(id("table"), table_id) + columns <- sess:::handle_dataview_init(list(view_id = table_id))$columns + expect_equal(as.character(columns[[2L]]$headerName), "nested") + + # The workspace icon and the list icon share the same root's panels. + expect_true(request("workspace_view", list(name = root, path = list(selector(1L, "a"))))) + expect_identical(id("list"), list_id) + expect_identical(runtime$dataviews[[list_id]]$data, x) + navigation <- request("listview_view", list(view_id = list_id, index = 1L, path = list(1L))) + expect_equal(navigation$title, paste0(root, "$a$b")) + expect_equal(navigation$path, list(1L, 1L)) + expect_equal(vapply(navigation$breadcrumbs, `[[`, "", "label"), c(root, "a", "b")) + expect_equal(navigation$breadcrumbs[[2L]]$path, list(1L)) + expect_identical(id("list"), list_id) + expect_identical(runtime$dataviews[[list_id]]$data, x) + parent <- request("listview_navigate", list(view_id = list_id, path = list(1L))) + expect_equal(parent$title, paste0(root, "$a")) + back <- request("listview_navigate", list(view_id = list_id, path = list())) + expect_equal(back$title, root) + expect_equal(back$path, list()) + expect_length(runtime$dataviews, 2L) + expect_true(request("workspace_view", list(name = root))) + expect_identical(id("list"), list_id) + expect_identical(runtime$dataviews[[list_id]]$data, x) + + expect_true(request("workspace_view", list(name = other))) + expect_false(identical(id("list", other), list_id)) + expect_true(request("workspace_view", list(name = other, path = list(selector(2L, "df"))))) + expect_false(identical(id("table", other), table_id)) + + # A list column can use the List Viewer without changing the table root's viewer. + table_object <- data.frame(id = 1:2) + table_object$nested <- I(list(list(value = 1L), list(value = 2L))) + assign(table_root, table_object, envir = .GlobalEnv) + expect_true(request("workspace_view", list(name = table_root))) + root_table_id <- id("table", table_root) + expect_true(request("workspace_view", list( + name = table_root, + path = list(selector(1L, "id")) + ))) + vector_id <- id("list", table_root) + expect_true(notification()$navigation$vector) + expect_false(identical(vector_id, root_table_id)) + vector_page <- request("workspace_children", list( + view_id = vector_id, + path = list(1L), + start = 1L + )) + expect_equal(vapply(vector_page$children, `[[`, "", "label"), c("[1]", "[2]")) + expect_equal(vapply(vector_page$children, `[[`, "", "str"), c("1", "2")) + expect_false(any(vapply(vector_page$children, `[[`, FALSE, "viewable"))) + expect_false(any(vapply(vector_page$children, `[[`, FALSE, "has_children"))) + expect_true(request("workspace_view", list( + name = table_root, + path = list(selector(2L, "nested")) + ))) + table_list_id <- id("list", table_root) + expect_identical(table_list_id, vector_id) + expect_false(notification()$navigation$vector) + expect_false(identical(table_list_id, root_table_id)) + expect_true(request("listview_navigate", list( + view_id = table_list_id, + path = list() + ))) + expect_identical(id("table", table_root), root_table_id) + + # Direct View calls also group nested expressions by their root. + utils::View(x) + direct_id <- id("list", "x") + utils::View(x$a) + expect_identical(id("list", "x"), direct_id) + expect_identical(runtime$dataviews[[direct_id]]$data, x) + utils::View(x$a$b) + expect_identical(runtime$dataviews[[direct_id]]$data, x) + direct_context <- sess:::listview_expression_context(quote(x$a$b), environment(), "x", x$a$b) + expect_equal(direct_context$navigation$path, list(1L, 1L), check.attributes = FALSE) + expect_equal(direct_context$navigation$title, "x$a$b") + indexed_context <- sess:::listview_expression_context( + quote(x[[1L]][[1L]]), environment(), "x", x$a$b + ) + expect_identical(indexed_context$data, x$a$b) + expect_equal(indexed_context$navigation$path, list(1L, 1L), check.attributes = FALSE) + + # Active roots are evaluated once, including when inherited by the caller. + for (inherited in c(FALSE, TRUE)) { + calls <- 0L + evaluated <- NULL + binding_env <- new.env(parent = environment()) + makeActiveBinding("active_root", function() { + calls <<- calls + 1L + evaluated <<- list(nested = list(value = c(calls, calls + 10L))) + evaluated + }, binding_env) + caller <- if (inherited) new.env(parent = binding_env) else binding_env + expressions <- c("active_root", "active_root$nested", "active_root$nested$value") + for (expression in expressions) { + calls <- 0L + eval(parse(text = paste0("utils::View(", expression, ")")), caller) + read_messages() + expect_identical(calls, 1L) + expected <- switch(expression, + active_root = evaluated, + "active_root$nested" = evaluated$nested, + "active_root$nested$value" = evaluated$nested$value + ) + expect_identical(runtime$dataviews[[id("list", "active_root")]]$data, expected) + expect_equal(notification()$navigation$title, expression) + expect_length(notification()$navigation$breadcrumbs, 1L) + } + } + + # Custom $ extraction can differ from the indexed path used by breadcrumbs. + dollar_calls <- 0L + `$.listview_custom` <- function(object, name) { + dollar_calls <<- dollar_calls + 1L + custom_value + } + index_calls <- 0L + `[[.listview_custom` <- function(object, index) { + index_calls <<- index_calls + 1L + unclass(object)[[index]] + } + # Register the method so dispatch inside the package can find it too. + registerS3method("[[", "listview_custom", `[[.listview_custom`, envir = asNamespace("base")) + on.exit({ + rm("[[.listview_custom", envir = get(".__S3MethodsTable__.", asNamespace("base"))) + }, add = TRUE) + custom <- structure(list(a = c(1L, 2L)), class = "listview_custom") + outer <- list(custom = custom) + for (custom_value in list(c(90L, 91L), list(returned = c(90L, 91L)))) { + for (expression in c("custom$a", "outer$custom$a")) { + dollar_calls <- 0L + index_calls <- 0L + eval(parse(text = paste0("utils::View(", expression, ")"))) + read_messages() + expect_identical(dollar_calls, 1L) + expect_null(sess:::listview_expression_context( + parse(text = expression)[[1L]], environment(), "custom", custom_value + )) + expect_identical(index_calls, 0L) + view_id <- notification()$view_id + expect_identical(runtime$dataviews[[view_id]]$data, custom_value) + expect_equal(notification()$navigation$title, expression) + expect_length(notification()$navigation$breadcrumbs, 1L) + expect_length(notification()$navigation$path, 0L) + page <- sess:::get_workspace_children(view_id = view_id) + if (is.atomic(custom_value)) { + expect_equal(vapply(page$children, `[[`, "", "str"), c("90", "91")) + } else { + expect_equal(page$children[[1L]]$label, "$ returned") + } + } + } + + index_calls <- 0L + utils::View(custom[["a"]]) + read_messages() + expect_identical(index_calls, 1L) + expect_identical(runtime$dataviews[[notification()$view_id]]$data, c(1L, 2L)) + + # A locally rebound operator changes extraction even for an ordinary list. + for (operator in c("$", "[[", "@")) { + for (active in c(FALSE, TRUE)) { + caller <- new.env(parent = environment()) + caller$x <- list(a = c(1L, 2L)) + extraction_calls <- 0L + binding_calls <- 0L + extraction <- function(object, name) { + extraction_calls <<- extraction_calls + 1L + c(90L, 91L) + } + if (active) { + makeActiveBinding(operator, function() { + binding_calls <<- binding_calls + 1L + extraction + }, caller) + } else { + assign(operator, extraction, envir = caller) + } + expression <- switch(operator, "$" = "x$a", "[[" = "x[[1L]]", "@" = "x@a") + eval(parse(text = paste0("utils::View(", expression, ")")), caller) + read_messages() + expect_identical(extraction_calls, 1L) + expect_identical(binding_calls, if (active) 1L else 0L) + expect_identical(runtime$dataviews[[notification()$view_id]]$data, c(90L, 91L)) + expect_length(notification()$navigation$path, 0L) + } + } + + # Standard data-frame columns retain their parent navigation. + frame <- data.frame(values = 1:2) + for (expression in c("frame$values", "frame$val", "frame[[1L]]", "frame[['values']]")) { + eval(parse(text = paste0("utils::View(", expression, ")"))) + read_messages() + expect_identical(runtime$dataviews[[notification()$view_id]]$data, frame) + expect_equal(notification()$navigation$path, list(1L)) + expect_equal(vapply(notification()$navigation$breadcrumbs, `[[`, "", "label"), + c("frame", "values")) + } + + # Local S3 overrides of standard data-frame extraction must also run only once. + for (operator in c("$", "[[")) { + caller <- new.env(parent = environment()) + caller$frame <- frame + extraction_calls <- 0L + assign(paste0(operator, ".data.frame"), function(object, name) { + extraction_calls <<- extraction_calls + 1L + c(90L, 91L) + }, envir = caller) + expression <- if (operator == "$") "frame$values" else "frame[[1L]]" + eval(parse(text = paste0("utils::View(", expression, ")")), caller) + read_messages() + expect_identical(extraction_calls, 1L) + expect_identical(runtime$dataviews[[notification()$view_id]]$data, c(90L, 91L)) + expect_length(notification()$navigation$path, 0L) + } + + # A structural path must not replace an already-evaluated, different value. + expect_null(sess:::listview_expression_context( + quote(x$a$b), environment(), "x", list(other = 1L) + )) + + utils::View(x$df) + direct_table <- id("table", "x") + utils::View(x$a$df) + expect_identical(id("table", "x"), direct_table) + utils::View(seq_len(501L)) + direct_vector <- id("list", "seq_len(501L)") + first_vector_page <- sess:::get_workspace_children( + view_id = direct_vector, + start = 1L + ) + expect_length(first_vector_page$children, 500L) + expect_equal(first_vector_page$next_start, 501L) + last_vector_page <- sess:::get_workspace_children( + view_id = direct_vector, + start = 501L + ) + expect_length(last_vector_page$children, 1L) + expect_equal(last_vector_page$children[[1L]]$label, "[501]") + expect_equal(last_vector_page$children[[1L]]$str, "501") + + # Named vectors retain names, duplicates and positions across page boundaries. + named_values <- seq_len(501L) + names(named_values) <- rep("", length(named_values)) + names(named_values)[c(1L, 3L, 4L, 5L, 6L, 7L, 501L)] <- + c("first", "first", NA, "a b", "", "\u540d\u524d", "last") + named_root <- list(values = named_values) + for (expression in c("named_values", "named_root$values")) { + eval(parse(text = paste0("utils::View(", expression, ")"))) + read_messages() + params <- list(view_id = notification()$view_id, path = notification()$navigation$path) + page <- do.call(sess:::get_workspace_children, params) + expect_equal(vapply(page$children[seq_len(7L)], `[[`, "", "label"), + c("first", "[2]", "first", "[4]", "a b", "", "\u540d\u524d")) + expect_equal(vapply(page$children[seq_len(7L)], `[[`, "", "str"), as.character(seq_len(7L))) + expect_equal(vapply(page$children, `[[`, 0L, "index"), seq_len(500L)) + expect_false(any(vapply(page$children, `[[`, FALSE, "has_children"))) + expect_false(any(vapply(page$children, `[[`, FALSE, "viewable"))) + expect_equal(page$next_start, 501L) + params$start <- page$next_start + last <- request("workspace_children", params) + expect_equal(last$children[[1L]]$label, "last") + expect_equal(last$children[[1L]]$str, "501") + expect_equal(last$children[[1L]]$index, 501L) + expect_null(last$next_start) + } + + utils::View(x$a$b$value) + read_messages() + expect_equal(notification()$view_id, direct_id) + expect_true(notification()$navigation$vector) + expect_equal(notification()$navigation$path, list(1L, 1L, 1L)) + scalar_page <- request("workspace_children", list( + view_id = direct_id, path = list(1L, 1L, 1L) + )) + expect_length(scalar_page$children, 1L) + expect_equal(scalar_page$children[[1L]]$str, "1") + utils::View(x$a$df$nested) + read_messages() + expect_equal(notification()$view_id, direct_id) + expect_true(notification()$navigation$vector) + expect_equal(notification()$navigation$path, list(1L, 2L, 1L)) + navigation <- request("listview_view", list(view_id = direct_id, path = list(1L, 1L), index = 1L)) + expect_true(navigation$vector) + expect_equal(navigation$path, list(1L, 1L, 1L)) + expect_length(notifications, 0L) + + # Scalars use the vector formatting route regardless of storage type or class. + for (sample in list(as.difftime(1, units = "mins"), as.Date("2026-01-01"), + as.POSIXct("2026-01-01 12:34:56", tz = "UTC"), factor("ready"), + TRUE, "001", as.raw(255))) { + utils::View(sample, title = "scalar") + read_messages() + expect_equal(notification()$source, "list") + expect_true(notification()$navigation$vector) + scalar_page <- request("workspace_children", list(view_id = notification()$view_id)) + expect_length(scalar_page$children, 1L) + expected <- if (is.character(sample)) { + encodeString(sample, quote = "\"") + } else { + format(sample, trim = TRUE, justify = "none") + } + expect_equal(scalar_page$children[[1L]]$str, expected) + expect_false(scalar_page$children[[1L]]$has_children) + expect_false(scalar_page$children[[1L]]$viewable) + } + + for (sample in list(numeric(), character(), logical(), raw())) { + utils::View(sample, title = "empty vector") + read_messages() + expect_equal(notification()$source, "list") + expect_true(notification()$navigation$vector) + empty_page <- request("workspace_children", list(view_id = notification()$view_id)) + expect_length(empty_page$children, 0L) + } + + functions <- list(first = function() 1L, second = function() 2L) + utils::View(functions$first) + text_id <- id("object", "functions") + text_file <- runtime$dataviews[[text_id]]$file + utils::View(functions$second) + expect_identical(id("object", "functions"), text_id) + expect_identical(runtime$dataviews[[text_id]]$file, text_file) + expect_equal(readLines(text_file), deparse(functions$second)) + unlink(text_file) + + # Vectors opened from expanded list rows retain the root and every breadcrumb. + values <- list(nested = list(v = seq_len(501L))) + assign(other, values, envir = .GlobalEnv) + expect_true(request("workspace_view", list(name = other))) + values_list_id <- id("list", other) + vector_navigation <- request("listview_view", list( + view_id = values_list_id, path = list(1L), index = 1L + )) + expect_length(notifications, 0L) + expect_true(vector_navigation$vector) + expect_equal(vector_navigation$path, list(1L, 1L)) + expect_equal(vapply(vector_navigation$breadcrumbs, `[[`, "", "label"), + c(other, "nested", "v")) + expect_identical(runtime$dataviews[[values_list_id]]$data, values) + first <- sess:::get_workspace_children(view_id = values_list_id, path = list(1L, 1L)) + expect_length(first$children, 500L) + expect_equal(first$next_start, 501L) + last <- request("workspace_children", list( + view_id = values_list_id, path = list(1L, 1L), start = 501L + )) + expect_equal(vapply(last$children, `[[`, "", "str"), "501") + parent <- request("listview_navigate", list(view_id = values_list_id, path = list(1L))) + expect_length(notifications, 0L) + expect_false(parent$vector) + expect_equal(parent$path, list(1L)) + back <- request("listview_navigate", list(view_id = values_list_id, path = list(1L, 1L))) + expect_equal(back, vector_navigation) + expect_true(request("workspace_view", list( + name = other, path = list(selector(1L, "nested"), selector(1L, "v")) + ))) + expect_equal(notification()$view_id, values_list_id) + expect_equal(notification()$source, "list") + expect_equal(notification()$navigation, vector_navigation) + utils::View(values) + direct_values_id <- id("list", "values") + utils::View(values$nested$v) + read_messages() + expect_equal(notification()$view_id, direct_values_id) + expect_true(notification()$navigation$vector) + expect_equal(notification()$navigation$path, list(1L, 1L)) + expect_equal(vapply(notification()$navigation$breadcrumbs, `[[`, "", "label"), + c("values", "nested", "v")) + + # POSIXlt values are vector leaves, including scalar, missing and empty values. + dates <- as.POSIXlt(c("2026-01-01 12:34:56", "2026-01-02 01:02:03", NA), tz = "UTC") + for (sample in list(dates, dates[1L], dates[3L], dates[FALSE])) { + utils::View(sample, title = "POSIXlt values") + read_messages() + expect_equal(notification()$source, "list") + expect_true(notification()$navigation$vector) + page <- request("workspace_children", list(view_id = notification()$view_id)) + expect_length(page$children, length(sample)) + expected <- format(sample, "%Y-%m-%d %H:%M:%S") + expected[is.na(expected)] <- "NA" + expect_equal(vapply(page$children, `[[`, "", "str"), expected) + expect_false(any(vapply(page$children, `[[`, FALSE, "has_children"))) + expect_false(any(vapply(page$children, `[[`, FALSE, "viewable"))) + expect_equal(sess:::workspace_child_count(sample), 0L) + } + values <- list(dates = dates) + assign(other, values, envir = .GlobalEnv) + expect_true(request("workspace_view", list(name = other))) + page <- request("workspace_children", list(name = other)) + expect_false(page$children[[1L]]$has_children) + expect_true(page$children[[1L]]$viewable) + page <- request("workspace_children", list(view_id = values_list_id)) + expect_false(page$children[[1L]]$has_children) + expect_true(page$children[[1L]]$viewable) + navigation <- request("listview_view", list(view_id = values_list_id, index = 1L)) + expect_length(notifications, 0L) + expect_true(navigation$vector) + expect_equal(navigation$path, list(1L)) + expect_true(request("workspace_view", list(name = other, path = list(selector(1L, "dates"))))) + expect_equal(notification()$source, "list") + expect_equal(notification()$view_id, values_list_id) + expect_equal(notification()$navigation, navigation) + expect_false(any(startsWith(ls(runtime$dataview_registry), "vector:"))) + + # Invalid or unavailable descendants cannot navigate or force active bindings. + expect_false(sess:::handle_listview_view(list_id, 0L)) + expect_false(sess:::handle_listview_view(list_id, 1.5)) + expect_false(sess:::handle_listview_view(list_id, 1L, list(99L))) + expect_true(sess:::handle_dataview_dispose(list( + view_id = list_id, state_generation = runtime$dataviews[[list_id]]$state_generation + ))) + expect_null(runtime$dataviews[[list_id]]) + sess:::handle_workspace_view(root) + expect_identical(id("list"), list_id) + expect_identical(runtime$dataviews[[list_id]]$data, x) +}) + +# Nested expansion pages retain lazy loading and handle environments, pairlists and slots. +local({ + runtime <- sess:::.sess_env + previous <- runtime$dataviews + on.exit(runtime$dataviews <- previous, add = TRUE) + env <- new.env(parent = emptyenv()) + env$child <- pairlist(value = list(1L)) + makeActiveBinding("active", function() stop("active binding evaluated"), env) + methods::setClass("list_viewer_nested_slots", slots = c(child = "list")) + on.exit(methods::removeClass("list_viewer_nested_slots"), add = TRUE) + object <- list( + many = rep(list(list(value = 1L)), 501L), + env = env, + slots = methods::new("list_viewer_nested_slots", child = list(value = 1L)) + ) + runtime$dataviews$nested_test <- list( + type = "list", data = object, kind = "index", names = names(object), title = "object", + child_names = new.env(parent = emptyenv()) + ) + page <- function(path, start = 1L) { + sess:::get_workspace_children(view_id = "nested_test", path = path, start = start) + } + first <- page(list(1L)) + expect_length(first$children, 500L) + expect_equal(first$next_start, 501L) + last <- page(list(1L), 501L) + expect_length(last$children, 1L) + expect_equal(last$children[[1L]]$index, 501L) + expect_true(last$children[[1L]]$has_children) + expect_equal(page(list(1L, 501L))$children[[1L]]$label, "$ value") + env_page <- page(list(2L)) + labels <- vapply(env_page$children, `[[`, "", "label") + active_index <- match("$ active", labels) + child_index <- match("$ child", labels) + expect_false(env_page$children[[active_index]]$viewable) + expect_false(sess:::handle_listview_view("nested_test", active_index, list(2L))) + expect_true(page(list(2L, child_index))$children[[1L]]$has_children) + rm("child", envir = env) + env$replacement <- list(value = 2L) + changed <- page(list(2L)) + expect_equal(changed$children[[child_index]]$label, "$ child") + expect_false(changed$children[[child_index]]$viewable) + expect_false(sess:::handle_listview_view("nested_test", child_index, list(2L))) + expect_equal(page(list(3L))$children[[1L]]$label, "@ child") + expect_equal(page(list(3L, 1L))$children[[1L]]$label, "$ value") +}) + +# Resolve a deep selector path once rather than repeatedly extracting its prefixes. +local({ + class_name <- paste0("sess_counted_list_", Sys.getpid()) + method_name <- paste0("[[.", class_name) + extractions <- 0L + assign(method_name, function(object, index, ...) { + extractions <<- extractions + 1L + unclass(object)[[index]] + }, envir = .GlobalEnv) + on.exit(rm(list = method_name, envir = .GlobalEnv), add = TRUE) + object <- 1:3 + for (i in seq_len(6L)) object <- structure(list(child = object), class = class_name) + root <- sess:::listview_state(object, "object", "object") + selectors <- rep(list(list(kind = "index", value = 1L)), 6L) + context <- sess:::listview_context(root, selectors) + expect_equal(extractions, 6L) + expect_identical(context$data, 1:3) + expect_equal(context$navigation$path, rep(list(1L), 6L), check.attributes = FALSE) + expect_equal(context$navigation$title, paste0("object", paste(rep("$child", 6L), collapse = ""))) +}) + +# Repeated pages use cached environment names without enumerating bindings again. +local({ + scans <- 0L + location <- sess:::listview_location + environment(location) <- new.env(parent = environment(location)) + environment(location)$workspace_env_names <- function(object) { + scans <<- scans + 1L + sess:::workspace_env_names(object) + } + child <- new.env(parent = emptyenv()) + child$value <- 1:3 + root <- sess:::listview_state(list(child = child), "object", "object") + first <- location(root, list(1L)) + expect_equal(scans, 1L) + child$added <- 4:6 + again <- location(root, list(1L)) + expect_equal(scans, 1L) + expect_identical(again$names, first$names) +}) diff --git a/sess/inst/tinytest/test-session-contract.R b/sess/inst/tinytest/test-session-contract.R index d19ecee8..3518957f 100644 --- a/sess/inst/tinytest/test-session-contract.R +++ b/sess/inst/tinytest/test-session-contract.R @@ -13,7 +13,7 @@ local({ first <- sess:::.session_attach_metadata() second <- sess:::.session_attach_metadata() - expect_equal(first$protocol_version, 1L) + expect_equal(first$protocol_version, 2L) expect_true(is.character(first$session_id) && nzchar(first$session_id)) expect_equal(first$session_id, second$session_id) expect_false(identical(first$session_id, as.character(first$pid))) diff --git a/src/extension.ts b/src/extension.ts index 1cfcbe3e..b511094a 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -153,7 +153,7 @@ export async function activate(context: vscode.ExtensionContext): Promise node?.label && workspaceViewer.viewItem(node), + 'r.workspaceViewer.view': (node: workspaceViewer.GlobalEnvItem) => node && workspaceViewer.viewItem(node), 'r.workspaceViewer.remove': (node: workspaceViewer.GlobalEnvItem) => node?.label && workspaceViewer.removeItem(node), 'r.workspaceViewer.clear': workspaceViewer.clearWorkspace, 'r.workspaceViewer.load': workspaceViewer.loadWorkspace, diff --git a/src/interactive/backend.ts b/src/interactive/backend.ts index b4460e96..a4fd5740 100644 --- a/src/interactive/backend.ts +++ b/src/interactive/backend.ts @@ -9,7 +9,7 @@ export interface RuntimeMetadata { rPath?: string; libraryPaths?: string[]; } -export type InspectionMethod = 'workspace' | 'workspace_children' | 'hover' | 'completion' | +export type InspectionMethod = 'workspace' | 'workspace_children' | 'workspace_view' | 'listview_navigate' | 'listview_view' | 'hover' | 'completion' | 'dataview_init' | 'dataview_page' | 'dataview_dispose'; export interface InspectionRequest { method: InspectionMethod; params: Record; timeout?: number } export interface InputReply { value: string } @@ -56,7 +56,7 @@ export interface SessionBackend { export type BackendFactory = (settings: AgentSettings) => SessionBackend; export function inspectionMethod(value: unknown): InspectionMethod { - if (!['workspace', 'workspace_children', 'hover', 'completion', 'dataview_init', 'dataview_page', 'dataview_dispose'].includes(String(value))) { + if (!['workspace', 'workspace_children', 'workspace_view', 'listview_navigate', 'listview_view', 'hover', 'completion', 'dataview_init', 'dataview_page', 'dataview_dispose'].includes(String(value))) { throw new Error('Unsupported inspection method'); } return value as InspectionMethod; diff --git a/src/interactive/backends/sessBridge.ts b/src/interactive/backends/sessBridge.ts index dd9a635d..29c550ee 100644 --- a/src/interactive/backends/sessBridge.ts +++ b/src/interactive/backends/sessBridge.ts @@ -55,7 +55,7 @@ export class SessBridge { this.parse(socket, message => { if (message.method === 'attach') { const params = object(message.params); - if (params.protocol_version !== 1 || params.interactive_token !== this.token || + if (params.protocol_version !== 2 || params.interactive_token !== this.token || (this.sess && this.sess !== socket)) { socket.destroy(); return; } this.sess = socket; attached = true; this.emit({ type: 'metadata', metadata: { rPid: Number(params.pid), rVersion: String(params.version), diff --git a/src/interactive/manager.ts b/src/interactive/manager.ts index ee98dd8b..4612516d 100644 --- a/src/interactive/manager.ts +++ b/src/interactive/manager.ts @@ -16,6 +16,7 @@ import { DISPLAY_MIME, InteractiveSerializer } from './notebook'; import { setInteractiveExecutor } from './executionTarget'; import * as session from '../session'; import * as util from '../util'; +import type { ListViewNavigation } from '../listViewer'; import { escapeXml } from './plotSvg'; import { ensureWorkspaceViewer } from '../extension'; import { queryTablePage } from './tableQuery'; @@ -951,6 +952,26 @@ export class InteractiveManager implements vscode.Disposable, vscode.TreeDataPro void this.prompt(view, event.data); } else if (event.type === 'clientRequest' && view.client.control) { void this.clientRequest(view, event.data); + } else if (event.type === 'viewer' && event.data.method === 'dataview') { + const params = object(event.data.params ?? {}); + const title = typeof params.title === 'string' ? params.title + : Array.isArray(params.title) && params.title.every((line: unknown) => typeof line === 'string') + ? params.title.join(',') : undefined; + const viewer = util.config().get>('session.viewers.viewColumn')?.view ?? 'Two'; + if (viewer !== 'Disable' + && typeof params.source === 'string' && params.source + && typeof params.type === 'string' && params.type + && title + && (params.file === undefined || params.file === null || typeof params.file === 'string') + && (params.view_id === undefined || params.view_id === null || typeof params.view_id === 'string')) { + await session.showDataView( + params.source, params.type, title, params.file ?? '', viewer, + params.view_id || undefined, + params.navigation as ListViewNavigation | undefined, + view.target.sessionId, + typeof params.state_generation === 'number' ? params.state_generation : undefined, + ); + } } else if (event.type === 'notification') { const params = object(event.data.params ?? {}); if (event.data.method === 'help') { await session.showHelpNotification(params); } @@ -1184,7 +1205,7 @@ export class InteractiveManager implements vscode.Disposable, vscode.TreeDataPro switch (message.action) { case 'table': if (data.kind !== 'table') { return; } - await session.showDataView('table', 'json', `${view.client.manifest.label}: ${data.fullViewId ? 'full table' : 'table'}`, '', 'Beside', String(data.fullViewId ?? data.viewId), view.target); break; + await session.showDataView('table', 'json', `${view.client.manifest.label}: ${data.fullViewId ? 'full table' : 'table'}`, '', 'Beside', String(data.fullViewId ?? data.viewId), undefined, view.target?.sessionId ?? null); break; case 'page': { if (data.kind !== 'table') { return; } result = await queryTablePage(data, message, request => view.client.request('inspect', request)); break; diff --git a/src/listViewer.ts b/src/listViewer.ts new file mode 100644 index 00000000..a1aaa09e --- /dev/null +++ b/src/listViewer.ts @@ -0,0 +1,187 @@ +export interface ListViewNavigation { + title: string; + path: number[]; + breadcrumbs: { label: string; path: number[] }[]; + /** Row presentation at this path; lists and vectors share the same viewer. */ + vector?: boolean; +} + +/** Script shared by the list webview and its interaction tests. */ +export function getListViewerScript(documentGeneration: number, initial: ListViewNavigation = { + title: '', path: [], breadcrumbs: [{ label: '', path: [] }], +}): string { + return ` + const vscode = acquireVsCodeApi(); + const documentGeneration = ${documentGeneration}; + const pending = new Map(); + let nextRequestId = 0; + const list = document.getElementById('list'); + const back = document.getElementById('back'); + const breadcrumbs = document.getElementById('breadcrumbs'); + const navigationStatus = document.getElementById('navigation-status'); + const pages = new Map(); + const history = []; + let current; + let navigating = false; + + function showNavigation(navigation, goingBack = false) { + const key = JSON.stringify(navigation.path); + if (current) { + pages.get(JSON.stringify(current.path)).scrollTop = list.scrollTop; + if (goingBack) history.pop(); + else if (JSON.stringify(current.path) !== key) history.push(current); + } + current = navigation; + document.body.classList.toggle('vector', !!navigation.vector); + let page = pages.get(key); + if (!page) { + const element = document.createElement('div'); + page = { element, scrollTop: 0, loader: createPage(element, navigation.path, navigation.vector) }; + pages.set(key, page); + } + list.replaceChildren(page.element); + list.scrollTop = page.scrollTop; + back.disabled = history.length === 0; + breadcrumbs.replaceChildren(); + navigation.breadcrumbs.forEach((crumb, index) => { + if (index) { + const separator = document.createElement('span'); + separator.className = 'codicon codicon-chevron-right'; + separator.setAttribute('aria-hidden', 'true'); + breadcrumbs.appendChild(separator); + } + const isCurrent = index === navigation.breadcrumbs.length - 1; + const item = document.createElement(isCurrent ? 'span' : 'button'); + item.className = 'breadcrumb'; + item.textContent = crumb.label; + item.title = crumb.label; + if (isCurrent) item.setAttribute('aria-current', 'page'); + else item.addEventListener('click', () => navigate('listview/navigate', { path: crumb.path })); + breadcrumbs.appendChild(item); + }); + page.loader.loadOnce(); + } + + function navigate(message, params, goingBack = false) { + if (navigating) return; + navigating = true; + navigationStatus.textContent = ''; + const requestId = ++nextRequestId; + pending.set(requestId, (response) => { + navigating = false; + if (response.error) { + navigationStatus.textContent = response.error; + } else if (response.navigation) { + showNavigation(response.navigation, goingBack); + } + }); + vscode.postMessage({ message, documentGeneration, requestId, ...params }); + } + + back.addEventListener('click', () => { + if (history.length) navigate('listview/navigate', { path: history[history.length - 1].path }, true); + }); + + function createPage(container, path, vector = false) { + const rows = document.createElement('div'); + const more = document.createElement('button'); + more.className = 'load-more'; + more.textContent = 'Load more'; + const status = document.createElement('div'); + status.setAttribute('role', 'status'); + container.appendChild(rows); + container.appendChild(more); + container.appendChild(status); + let nextStart = 1; + let loading = false; + let loaded = false; + + function loadPage() { + if (loading || nextStart === null) return; + loading = true; + more.disabled = true; + more.textContent = 'Loading…'; + status.textContent = ''; + const requestId = ++nextRequestId; + pending.set(requestId, (message) => { + loading = false; + more.disabled = false; + if (message.error) { + status.textContent = message.error; + more.textContent = 'Retry'; + return; + } + loaded = true; + for (const item of message.children) { + const expandable = item.has_children; + const entry = document.createElement(expandable ? 'details' : 'div'); + const row = document.createElement(expandable ? 'summary' : 'div'); + row.className = 'item'; + if (!vector) { + const arrow = document.createElement('span'); + arrow.className = expandable ? 'arrow codicon codicon-chevron-right' : 'arrow'; + arrow.setAttribute('aria-hidden', 'true'); + row.appendChild(arrow); + } + const label = document.createElement('span'); + label.className = 'label'; + label.textContent = item.label; + row.appendChild(label); + const str = document.createElement('span'); + str.className = 'str'; + str.textContent = item.str; + row.appendChild(str); + + if (item.viewable) { + const button = document.createElement('button'); + button.title = 'View'; + button.setAttribute('aria-label', 'View ' + item.label); + const icon = document.createElement('span'); + icon.className = 'codicon codicon-open-preview'; + icon.setAttribute('aria-hidden', 'true'); + button.appendChild(icon); + button.addEventListener('click', (event) => { + event.preventDefault(); + event.stopPropagation(); + navigate('listview/view', { path, index: item.index }); + }); + row.appendChild(button); + } + entry.appendChild(row); + if (expandable) { + const children = document.createElement('div'); + children.className = 'children'; + entry.appendChild(children); + let page; + entry.addEventListener('toggle', () => { + if (entry.open) { + page ??= createPage(children, [...path, item.index]); + page.loadOnce(); + } + }); + } + rows.appendChild(entry); + } + nextStart = message.next_start ?? null; + more.hidden = nextStart === null; + more.textContent = 'Load more'; + status.textContent = rows.childElementCount ? '' : 'No items'; + }); + vscode.postMessage({ message: 'listview/page', documentGeneration, requestId, path, start: nextStart }); + } + more.addEventListener('click', loadPage); + return { loadOnce: () => { if (!loaded) loadPage(); } }; + } + + window.addEventListener('message', (event) => { + const message = event.data; + if (!['listview/page', 'listview/navigation'].includes(message.message) || message.documentGeneration !== documentGeneration) return; + const receive = pending.get(message.requestId); + if (receive) { + pending.delete(message.requestId); + receive(message); + } + }); + showNavigation(${JSON.stringify(initial).replace(/(); const documentSessions = new Map(); @@ -259,6 +260,7 @@ interface DataViewRequestMessage { message: 'dataview/request'; action: 'init' | 'page'; requestId: number; + documentGeneration: number; startRow?: number; endRow?: number; sortModel?: unknown[]; @@ -266,7 +268,9 @@ interface DataViewRequestMessage { } const dynamicDataViewPanels = new Map(); -let dynamicDataViewReloadRevision = 0; +const dynamicDataViewStateGenerations = new WeakMap(); +const documentGenerations = new WeakMap(); +let documentGenerationRevision = 0; function escapeHtml(text: string): string { const map: Record = { @@ -279,11 +283,44 @@ function escapeHtml(text: string): string { return text.replace(/[&<>"']/g, c => map[c]); } -function attachDynamicDataViewBridge(panel: vscode.WebviewPanel, viewId: string, baseTitle: string, owner: Session | undefined): void { - const panelKey = `${owner?.sessionId ?? ''}:${viewId}`; - const postResponse = (requestId: number, ok: boolean, result?: unknown, error?: string) => { - void panel.webview.postMessage({ +function registerDataViewPanel( + panel: vscode.WebviewPanel, key: string, viewId: string, sessionId: string | null, + stateGeneration?: number, +): void { + // The panel's webview getter throws once onDidDispose fires. + const webview = panel.webview; + dynamicDataViewPanels.set(key, panel); + if (stateGeneration !== undefined) { + dynamicDataViewStateGenerations.set(panel, stateGeneration); + } + panel.onDidDispose(() => { + documentGenerations.delete(webview); + const currentStateGeneration = dynamicDataViewStateGenerations.get(panel); + dynamicDataViewStateGenerations.delete(panel); + if (dynamicDataViewPanels.get(key) !== panel) { + return; + } + dynamicDataViewPanels.delete(key); + // Interactive transcripts retain this handle after the expanded viewer closes. + if (currentStateGeneration === undefined && sessions.get(sessionId ?? '')?.requester) { return; } + void sessionRequest({ + method: 'dataview_dispose', + params: { view_id: viewId, state_generation: currentStateGeneration }, + }, sessionId); + }); +} + +function attachDynamicDataViewBridge(panel: vscode.WebviewPanel, viewId: string, sessionId: string | null): void { + const webview = panel.webview; + const postResponse = ( + documentGeneration: number, requestId: number, ok: boolean, result?: unknown, error?: string + ) => { + if (documentGeneration !== documentGenerations.get(webview)) { + return; + } + void webview.postMessage({ message: 'dataview/response', + documentGeneration, requestId, ok, result, @@ -291,9 +328,11 @@ function attachDynamicDataViewBridge(panel: vscode.WebviewPanel, viewId: string, }); }; - panel.webview.onDidReceiveMessage(async (raw: unknown) => { + webview.onDidReceiveMessage(async (raw: unknown) => { const msg = raw as Partial; - if (msg.message !== 'dataview/request' || typeof msg.requestId !== 'number') { + if (msg.message !== 'dataview/request' || typeof msg.requestId !== 'number' || + typeof msg.documentGeneration !== 'number' || + msg.documentGeneration !== documentGenerations.get(webview)) { return; } @@ -302,12 +341,11 @@ function attachDynamicDataViewBridge(panel: vscode.WebviewPanel, viewId: string, const result = await sessionRequest({ method: 'dataview_init', params: { view_id: viewId }, - }, owner) as DataViewInitResult | undefined; + }, sessionId) as DataViewInitResult | undefined; if (!result || !Array.isArray(result.columns) || typeof result.totalRows !== 'number') { throw new Error('Invalid dataview_init response'); } - panel.title = baseTitle; - postResponse(msg.requestId, true, result); + postResponse(msg.documentGeneration, msg.requestId, true, result); return; } @@ -321,35 +359,21 @@ function attachDynamicDataViewBridge(panel: vscode.WebviewPanel, viewId: string, sortModel: Array.isArray(msg.sortModel) ? msg.sortModel : [], filterModel: msg.filterModel ?? {}, }, - }, owner) as DataViewPageResult | undefined; + }, sessionId) as DataViewPageResult | undefined; if (!result || !Array.isArray(result.rows) || typeof result.totalRows !== 'number' || typeof result.totalUnfiltered !== 'number') { throw new Error('Invalid dataview_page response'); } - panel.title = baseTitle; - postResponse(msg.requestId, true, result); + postResponse(msg.documentGeneration, msg.requestId, true, result); return; } - postResponse(msg.requestId, false, undefined, `Unsupported dataview action: ${String(msg.action)}`); + postResponse(msg.documentGeneration, msg.requestId, false, undefined, `Unsupported dataview action: ${String(msg.action)}`); } catch (e) { - postResponse(msg.requestId, false, undefined, e instanceof Error ? e.message : String(e)); + postResponse(msg.documentGeneration, msg.requestId, false, undefined, e instanceof Error ? e.message : String(e)); } }); - - panel.onDidDispose(() => { - if (dynamicDataViewPanels.get(panelKey) !== panel) { - return; - } - dynamicDataViewPanels.delete(panelKey); - // Interactive transcripts retain this handle after the expanded viewer closes. - if (owner?.requester) { return; } - void sessionRequest({ - method: 'dataview_dispose', - params: { view_id: viewId }, - }, owner); - }); } export function deploySessionWatcher(extensionPath: string): void { @@ -1059,18 +1083,26 @@ export function openExternalBrowser(): void { } } -export async function showDataView(source: string, type: string, title: string, file: string, viewer: string, viewId?: string, owner = activeSession): Promise { +export async function showDataView( + source: string, type: string, title: string, file: string, viewer: string, + viewId?: string, navigation?: ListViewNavigation, + sessionId: string | null = activeSession?.sessionId ?? null, + stateGeneration?: number, +): Promise { resDir ??= path.join(extensionContext.extensionPath, 'dist', 'resources'); console.info(`[showDataView] source: ${source}, type: ${type}, title: ${title}, file: ${file}, viewer: ${viewer}, viewId: ${String(viewId ?? '')}`); + const panelKey = JSON.stringify([sessionId, viewId]); if (source === 'table') { if (viewId) { - const existing = dynamicDataViewPanels.get(`${owner?.sessionId ?? ''}:${viewId}`); + const existing = dynamicDataViewPanels.get(panelKey); if (existing) { + if (stateGeneration !== undefined) { + dynamicDataViewStateGenerations.set(existing, stateGeneration); + } existing.title = title; - existing.reveal(ViewColumn[viewer as keyof typeof ViewColumn], true); - const content = await getTableHtml(existing.webview, undefined, title); - existing.webview.html = `${content}\n`; + existing.reveal(existing.viewColumn, true); + existing.webview.html = await getTableHtml(existing.webview, undefined, title); return; } } @@ -1088,12 +1120,27 @@ export async function showDataView(source: string, type: string, title: string, }); panel.iconPath = new UriIcon('open-preview'); if (viewId) { - dynamicDataViewPanels.set(`${owner?.sessionId ?? ''}:${viewId}`, panel); - attachDynamicDataViewBridge(panel, viewId, title, owner); + registerDataViewPanel(panel, panelKey, viewId, sessionId, stateGeneration); + attachDynamicDataViewBridge(panel, viewId, sessionId); } const content = await getTableHtml(panel.webview, file || undefined, title); panel.webview.html = content; } else if (source === 'list') { + if (viewId) { + const existing = dynamicDataViewPanels.get(panelKey); + if (existing) { + if (stateGeneration !== undefined) { + dynamicDataViewStateGenerations.set(existing, stateGeneration); + } + existing.title = title; + existing.reveal(existing.viewColumn, true); + existing.webview.html = getListHtml( + existing.webview, title, navigation + ); + return; + } + } + const panel = window.createWebviewPanel('dataview', title, { preserveFocus: true, @@ -1105,9 +1152,63 @@ export async function showDataView(source: string, type: string, title: string, retainContextWhenHidden: true, localResourceRoots: [Uri.file(resDir)], }); - const content = await getListHtml(panel.webview, file, title); - panel.iconPath = new UriIcon('open-preview'); - panel.webview.html = content; + panel.iconPath = new UriIcon('preview'); + if (viewId) { + registerDataViewPanel(panel, panelKey, viewId, sessionId, stateGeneration); + const webview = panel.webview; + webview.onDidReceiveMessage(async (message: { + message?: string; index?: number; start?: number; requestId?: number; + path?: number[]; documentGeneration?: number; + }) => { + if (message.documentGeneration !== documentGenerations.get(webview)) { + return; + } + if (!Array.isArray(message.path) || !message.path.every(index => Number.isSafeInteger(index) && index > 0)) { + return; + } + if (message.message === 'listview/navigate' || + (message.message === 'listview/view' && Number.isSafeInteger(message.index))) { + const result = await sessionRequest({ + method: message.message === 'listview/navigate' ? 'listview_navigate' : 'listview_view', + params: { view_id: viewId, index: message.index, path: message.path }, + }, sessionId) as ListViewNavigation | boolean | undefined; + if (message.documentGeneration !== documentGenerations.get(webview)) { + return; + } + const navigation = result && typeof result === 'object' && Array.isArray(result.breadcrumbs) + ? result : undefined; + if (navigation) { + panel.title = navigation.title; + } + void webview.postMessage({ + message: 'listview/navigation', + documentGeneration: message.documentGeneration, + requestId: message.requestId, + navigation, + error: result ? undefined : 'Unable to open this item. Check the R session and try again.', + }); + } else if (message.message === 'listview/page' && typeof message.start === 'number' && + Number.isInteger(message.start) && typeof message.requestId === 'number') { + const page = await sessionRequest({ + method: 'workspace_children', + params: { view_id: viewId, start: message.start, path: message.path }, + }, sessionId) as { children?: unknown; next_start?: number | null } | undefined; + if (message.documentGeneration !== documentGenerations.get(webview)) { + return; + } + void webview.postMessage({ + message: 'listview/page', + documentGeneration: message.documentGeneration, + requestId: message.requestId, + ...page, + error: Array.isArray(page?.children) ? undefined : 'Unable to load items. Check the R session and try again.', + }); + } + }); + } + panel.webview.html = getListHtml( + panel.webview, title, navigation + ); } else { await commands.executeCommand('vscode.open', Uri.file(file), { preserveFocus: true, @@ -1121,6 +1222,8 @@ export async function showDataView(source: string, type: string, title: string, export async function getTableHtml(webview: Webview, file: string | undefined, title: string): Promise { const pageSize = config().get('session.data.pageSize', 500); if (!file) { + const documentGeneration = ++documentGenerationRevision; + documentGenerations.set(webview, documentGeneration); return ` @@ -1298,6 +1401,7 @@ export async function getTableHtml(webview: Webview, file: string | undefined, t - - - - -

+    
+    
+    
+ `; @@ -2074,6 +2236,10 @@ async function handleNotification(message: Record, socket: IpcS terminalSessionAttached.fire(terminalPid); } + if (terminalPid) { + terminalSessionAttached.fire(terminalPid); + } + // Reload does not trigger a terminal-selection event after every attach. // Prefer its connected session when a terminal reconnects in the background. const selectedSession = terminalPid && selectedTerminal === window.activeTerminal && selectedTerminalPid @@ -2157,6 +2323,9 @@ async function handleNotification(message: Record, socket: IpcS params.file ?? '', viewer, params.view_id || undefined, + params.navigation as ListViewNavigation | undefined, + socket._sessionId ?? null, + typeof params.state_generation === 'number' ? params.state_generation : undefined, ); } } @@ -2300,10 +2469,14 @@ export async function cleanupSession(sessionId: string, closingSocket?: IpcSocke } } -export async function sessionRequest(data: Record, target = activeSession): Promise { +export async function sessionRequest( + data: Record, target: Session | string | null | undefined = activeSession, +): Promise { try { - if (target?.requester) { return await target.requester(data); } - const socket = target?.socket ?? pipeClient; + const owner = typeof target === 'string' ? sessions.get(target) : target; + if (owner?.requester) { return await owner.requester(data); } + // An explicitly bound viewer must never fall back to the active session. + const socket = owner?.socket ?? (target === undefined ? pipeClient : undefined); if (!socket || socket.destroyed) { throw new Error('IPC socket is not connected'); } diff --git a/src/test/suite/dataViewerSessions.test.ts b/src/test/suite/dataViewerSessions.test.ts new file mode 100644 index 00000000..495676bd --- /dev/null +++ b/src/test/suite/dataViewerSessions.test.ts @@ -0,0 +1,252 @@ +import * as assert from 'assert'; +import * as net from 'net'; +import * as path from 'path'; +import * as sinon from 'sinon'; +import * as vscode from 'vscode'; +import * as extension from '../../extension'; +import * as session from '../../session'; +import * as util from '../../util'; +import { mockExtensionContext } from '../common/mockvscode'; + +interface Request { + id: number; + method: string; + params?: { view_id?: string; state_generation?: number }; +} + +interface Client { + id: string; + socket: net.Socket; + requests: Request[]; +} + +interface Panel { + panel: vscode.WebviewPanel; + receive: (message: unknown) => Promise; + replies: Array<{ ok?: boolean; error?: string }>; +} + +async function waitFor(condition: () => boolean): Promise { + const deadline = Date.now() + 5000; + while (!condition()) { + if (Date.now() > deadline) { + throw new Error('Timed out waiting for viewer IPC'); + } + await new Promise(resolve => setTimeout(resolve, 10)); + } +} + +suite('Viewer session ownership', () => { + let sandbox: sinon.SinonSandbox; + const clients: Client[] = []; + const panels: Panel[] = []; + + setup(() => { + sandbox = sinon.createSandbox(); + const root = path.join(__dirname, '..', '..', '..'); + mockExtensionContext(root, sandbox); + sandbox.stub(extension, 'enableSessionWatcher').value(true); + sandbox.stub(util, 'config').returns({ + get: (_key: string, defaultValue: unknown) => defaultValue, + } as vscode.WorkspaceConfiguration); + session.deploySessionWatcher(root); + sandbox.stub(vscode.window, 'createWebviewPanel').callsFake((_type, title) => { + const disposed = new vscode.EventEmitter(); + let closed = false; + const item: Panel = { + panel: undefined as unknown as vscode.WebviewPanel, + receive: () => Promise.resolve(), replies: [], + }; + item.panel = { + title, viewColumn: vscode.ViewColumn.Two, reveal: sandbox.stub(), + webview: { + html: '', asWebviewUri: (uri: vscode.Uri) => uri, + onDidReceiveMessage: (listener: Panel['receive']) => { item.receive = listener; }, + postMessage: (reply: Panel['replies'][number]) => { + item.replies.push(reply); + return Promise.resolve(true); + }, + }, + onDidDispose: disposed.event, + dispose: () => { + if (!closed) { + closed = true; + disposed.fire(); + disposed.dispose(); + } + }, + } as unknown as vscode.WebviewPanel; + panels.push(item); + return item.panel; + }); + }); + + teardown(async () => { + panels.splice(0).forEach(item => { item.panel.dispose(); }); + for (const client of clients.splice(0)) { + await session.cleanupSession(client.id); + client.socket.destroy(); + } + await session.shutdownSessionWatcher(); + sandbox.restore(); + }); + + function notify(client: Client, method: string, params: Record): void { + client.socket.write(JSON.stringify({ jsonrpc: '2.0', method, params }) + '\n'); + } + + async function attach(id: string): Promise { + const previousSocket = session.activeSession?.socket; + const socket = net.createConnection(await session.getGlobalPipePath()); + const client = { id, socket, requests: [] as Request[] }; + clients.push(client); + let buffer = ''; + socket.on('data', (data: Buffer) => { + buffer += data.toString(); + let newline: number; + while ((newline = buffer.indexOf('\n')) >= 0) { + const request = JSON.parse(buffer.slice(0, newline)) as Request; + buffer = buffer.slice(newline + 1); + client.requests.push(request); + let result: unknown = true; + switch (request.method) { + case 'workspace': result = { globalenv: {}, search: [], loaded_namespaces: [] }; break; + case 'dataview_init': result = { columns: [], totalRows: 1 }; break; + case 'dataview_page': result = { rows: [{ owner: id }], totalRows: 1, totalUnfiltered: 1 }; break; + case 'workspace_children': result = { children: [], next_start: null }; break; + case 'listview_navigate': + case 'listview_view': + result = { title: 'x$child', path: [1], breadcrumbs: [{ label: 'x', path: [] }] }; + break; + } + socket.write(JSON.stringify({ jsonrpc: '2.0', id: request.id, result }) + '\n'); + } + }); + await new Promise((resolve, reject) => { + socket.once('connect', resolve); + socket.once('error', reject); + }); + notify(client, 'attach', { + protocol_version: 2, session_id: id, host: 'viewer-test-host', pid: id, + version: '4.6.0', tempdir: '/tmp', wd: '/tmp', + }); + await waitFor(() => session.activeSession?.sessionId === id && + session.activeSession.socket !== previousSocket); + // A round trip ensures the new connection has completed its handshake. + await session.sessionRequest({ method: 'workspace' }, id); + return client; + } + + async function open( + client: Client, source: 'list' | 'table', existing?: Panel, stateGeneration = 1, + ): Promise { + const count = panels.length; + const html = existing?.panel.webview.html; + notify(client, 'dataview', { + source, type: 'json', title: 'x', view_id: `same-${source}-id`, state_generation: stateGeneration, + navigation: { title: 'x', path: [], breadcrumbs: [{ label: 'x', path: [] }] }, + }); + await waitFor(() => existing ? existing.panel.webview.html !== html : panels.length > count); + if (existing) { + assert.strictEqual(panels.length, count); + } + return existing ?? panels[count]; + } + + async function send(panel: Panel, message: Record): Promise { + const documentGeneration = Number(/const documentGeneration = (\d+)/.exec(panel.panel.webview.html)?.[1]); + await panel.receive({ documentGeneration, requestId: 1, path: [], start: 1, ...message }); + } + + const viewerRequests = (client: Client) => client.requests.filter(request => request.method !== 'workspace'); + + test('identical viewer ids stay separate and background views keep their originating session', async () => { + const a = await attach('viewer-session-a'); + const b = await attach('viewer-session-b'); + const aList = await open(a, 'list'); + const aTable = await open(a, 'table'); + const bList = await open(b, 'list'); + const bTable = await open(b, 'table'); + assert.strictEqual(panels.length, 4); + await open(a, 'list', aList, 2); + await open(a, 'table', aTable, 2); + assert.strictEqual(session.activeSession?.sessionId, b.id); + + const exercise = async (list: Panel, table: Panel) => { + await send(table, { message: 'dataview/request', action: 'init' }); + await send(table, { message: 'dataview/request', action: 'page', startRow: 0, endRow: 1 }); + await send(list, { message: 'listview/page' }); + await send(list, { message: 'listview/navigate' }); + await send(list, { message: 'listview/view', index: 1 }); + }; + const expected = ['dataview_init', 'dataview_page', 'workspace_children', 'listview_navigate', 'listview_view']; + await exercise(aList, aTable); + assert.deepStrictEqual(viewerRequests(a).map(request => request.method), expected); + assert.deepStrictEqual(viewerRequests(b), []); + assert.strictEqual(await session.activateSessionById(a.id), true); + await exercise(bList, bTable); + assert.deepStrictEqual(viewerRequests(b).map(request => request.method), expected); + + await open(b, 'list', bList, 2); + await open(b, 'table', bTable, 2); + bList.panel.dispose(); + bTable.panel.dispose(); + await waitFor(() => viewerRequests(b).length === 7); + assert.deepStrictEqual(viewerRequests(b).slice(-2).map(request => request.params?.view_id), + ['same-list-id', 'same-table-id']); + assert.deepStrictEqual(viewerRequests(b).slice(-2).map(request => request.params?.state_generation), + [2, 2]); + assert.strictEqual(viewerRequests(a).length, 5); + await open(a, 'list', aList); + await open(a, 'table', aTable); + }); + + test('Interactive viewers retain transcript tables and dispose standalone list state', async () => { + const request = sandbox.stub().resolves({ columns: [], totalRows: 1 }); + const owner = session.registerSessionTransport('interactive-viewer', 'host', '/tmp', request); + try { + const other = await attach('other-viewer-session'); + await session.showDataView('table', 'json', 'table', '', 'Two', 'transcript-table', undefined, owner.sessionId); + await send(panels[0], { message: 'dataview/request', action: 'init' }); + sinon.assert.calledWithExactly(request, { method: 'dataview_init', params: { view_id: 'transcript-table' } }); + request.resetHistory(); + panels[0].panel.dispose(); + sinon.assert.notCalled(request); + await session.showDataView('list', 'json', 'list', '', 'Two', 'standalone-list', undefined, owner.sessionId, 3); + panels[1].panel.dispose(); + sinon.assert.calledOnceWithExactly(request, { + method: 'dataview_dispose', params: { view_id: 'standalone-list', state_generation: 3 }, + }); + assert.deepStrictEqual(viewerRequests(other), []); + } finally { + session.unregisterSessionTransport(owner); + } + }); + + test('viewers follow the same session on reconnect and never fall back after disconnect', async () => { + const a = await attach('viewer-reconnect-a'); + const list = await open(a, 'list'); + const table = await open(a, 'table'); + const b = await attach('viewer-reconnect-b'); + const replacement = await attach(a.id); + assert.strictEqual(await session.activateSessionById(b.id), true); + await open(replacement, 'list', list); + await open(replacement, 'table', table); + await send(list, { message: 'listview/page' }); + await send(table, { message: 'dataview/request', action: 'init' }); + assert.deepStrictEqual(viewerRequests(replacement).map(request => request.method), + ['workspace_children', 'dataview_init']); + assert.deepStrictEqual(viewerRequests(a), []); + assert.deepStrictEqual(viewerRequests(b), []); + + await session.cleanupSession(a.id); + await send(list, { message: 'listview/page' }); + await send(table, { message: 'dataview/request', action: 'page' }); + assert.ok(list.replies.at(-1)?.error); + assert.strictEqual(table.replies.at(-1)?.ok, false); + list.panel.dispose(); + table.panel.dispose(); + assert.deepStrictEqual(viewerRequests(b), []); + assert.strictEqual(session.activeSession?.sessionId, b.id); + }); +}); diff --git a/src/test/suite/interactiveEditor.test.ts b/src/test/suite/interactiveEditor.test.ts index b182397d..99de72a3 100644 --- a/src/test/suite/interactiveEditor.test.ts +++ b/src/test/suite/interactiveEditor.test.ts @@ -1183,6 +1183,34 @@ cat("\n")`; sinon.assert.notCalled(errors); } finally { errors.restore(); saved.restore(); format.restore(); } }); + test('Interactive list viewers open nested tables in their original session', async () => { + await vscode.commands.executeCommand('r.interactive.open', manifests[0]); + const original = vscode.window.createWebviewPanel; + const panels: vscode.WebviewPanel[] = []; + const receivers: sinon.SinonSpy[] = []; + const create = sinon.stub(vscode.window, 'createWebviewPanel').callsFake((type, title, column, options) => { + const panel = original(type, title, column, options); + panels.push(panel); + receivers.push(sinon.spy(panel.webview, 'onDidReceiveMessage')); + return panel; + }); + try { + await vscode.commands.executeCommand('r.runSelection', 'rebase_view <- list(table=data.frame(value=1:2)); View(rebase_view)'); + await until(() => panels.length === 1 && receivers[0].calledOnce); + assert.strictEqual(panels[0].title, 'rebase_view'); + await vscode.commands.executeCommand('r.interactive.open', manifests[1]); + const receive = receivers[0].firstCall.args[0] as (message: unknown) => Promise; + const documentGeneration = Number(/const documentGeneration = (\d+)/.exec(panels[0].webview.html)?.[1]); + await receive({ message: 'listview/view', documentGeneration, requestId: 1, path: [], index: 1 }); + await until(() => panels.length === 2); + assert.strictEqual(panels[1].title, 'rebase_view$table'); + } finally { + panels.forEach(panel => { panel.dispose(); }); + receivers.forEach(receiver => { receiver.restore(); }); + create.restore(); + } + }); + test('large native cells export their snapshot scope and open the full data viewer', async () => { await vscode.commands.executeCommand('r.interactive.open', manifests[0]); const notebook = vscode.workspace.notebookDocuments.find(doc => doc.metadata.rSessionId === manifests[0].id); assert.ok(notebook); diff --git a/src/test/suite/listViewer.test.ts b/src/test/suite/listViewer.test.ts new file mode 100644 index 00000000..7d4f01f5 --- /dev/null +++ b/src/test/suite/listViewer.test.ts @@ -0,0 +1,294 @@ +import * as assert from 'assert'; +import * as vm from 'vm'; +import { getListViewerScript, ListViewNavigation } from '../../listViewer'; + +// Minimal DOM surface for exercising the actual webview script and its RPCs. +class Element { + children: Element[] = []; + listeners = new Map void>(); + textContent = ''; + innerHTML = ''; + className = ''; + hidden = false; + disabled = false; + open = false; + scrollTop = 0; + constructor(readonly tag: string) { } + get childElementCount(): number { return this.children.length; } + appendChild(child: Element): void { this.children.push(child); } + replaceChildren(...children: Element[]): void { this.children = children; } + setAttribute(): void { /* Attributes do not affect these interaction tests. */ } + addEventListener(type: string, listener: (event: unknown) => void): void { + this.listeners.set(type, listener); + } + fire(type: string, event: unknown = {}): void { this.listeners.get(type)?.(event); } +} + +interface Request { + message: string; + documentGeneration: number; + requestId: number; + path: number[]; + index?: number; + start?: number; +} + +function createViewer(initial: ListViewNavigation = { + title: 'x', path: [], breadcrumbs: [{ label: 'x', path: [] }], +}) { + const root = new Element('div'); + const back = new Element('button'); + const breadcrumbs = new Element('nav'); + const status = new Element('div'); + const bodyClasses = new Set(); + const elements: Record = { list: root, back, breadcrumbs, 'navigation-status': status }; + const messages: Request[] = []; + let receive: (event: unknown) => void = () => undefined; + vm.runInNewContext(getListViewerScript(7, initial), { + acquireVsCodeApi: () => ({ postMessage: (message: Request) => { + messages.push(JSON.parse(JSON.stringify(message)) as Request); + } }), + document: { + body: { classList: { toggle: (name: string, enabled: boolean) => { + if (enabled) { + bodyClasses.add(name); + } else { + bodyClasses.delete(name); + } + } } }, + createElement: (tag: string) => new Element(tag), + getElementById: (id: string) => elements[id] ?? new Element('template'), + }, + window: { addEventListener: (_type: string, listener: typeof receive) => { receive = listener; } }, + }); + const reply = (request: Request, result: Record) => receive({ + data: { ...request, children: [], next_start: null, ...result }, + }); + return { + get root() { return root.children[0]; }, + viewport: root, back, breadcrumbs, status, messages, reply, bodyClasses, + }; +} + +suite('List viewer', () => { + test('Back restores expanded rows, loaded pages and scroll without fetching them again', () => { + const viewer = createViewer(); + const { messages, reply, back, viewport } = viewer; + assert.strictEqual(back.disabled, true); + reply(messages[0], { children: [{ label: '$ a', index: 1, viewable: true, has_children: true }] }); + const original = viewer.root; + const entry = original.children[0].children[0]; + entry.open = true; + entry.fire('toggle'); + reply(messages[1], { children: [{ label: '$ b', index: 1, viewable: true, has_children: true }] }); + viewport.scrollTop = 120; + const openButton = entry.children[0].children.at(-1); + assert.ok(openButton); + openButton.fire('click', { preventDefault: () => undefined, stopPropagation: () => undefined }); + reply(messages[2], { + message: 'listview/navigation', navigation: { + title: 'x$a', path: [1], breadcrumbs: [{ label: 'x', path: [] }, { label: 'a', path: [1] }], + }, + }); + assert.strictEqual(back.disabled, false); + assert.deepStrictEqual(messages[3].path, [1]); + reply(messages[3], { children: [] }); + back.fire('click'); + assert.strictEqual(messages[4].message, 'listview/navigate'); + assert.deepStrictEqual(messages[4].path, []); + reply(messages[4], { + message: 'listview/navigation', navigation: { title: 'x', path: [], breadcrumbs: [{ label: 'x', path: [] }] }, + }); + assert.strictEqual(viewer.root, original); + assert.strictEqual(entry.open, true); + assert.strictEqual(viewport.scrollTop, 120); + assert.strictEqual(messages.length, 5); + assert.strictEqual(back.disabled, true); + }); + + test('breadcrumbs work when opened directly at a deep item and Back follows page history', () => { + const initial = { + title: 'x$a$b', path: [1, 2], breadcrumbs: [ + { label: 'x', path: [] }, { label: 'a', path: [1] }, { label: 'b', path: [1, 2] }, + ], + }; + const viewer = createViewer(initial); + assert.deepStrictEqual(viewer.messages[0].path, [1, 2]); + viewer.reply(viewer.messages[0], { children: [] }); + const original = viewer.root; + assert.deepStrictEqual(viewer.breadcrumbs.children.map(item => item.className), + ['breadcrumb', 'codicon codicon-chevron-right', 'breadcrumb', 'codicon codicon-chevron-right', 'breadcrumb']); + viewer.breadcrumbs.children[0].fire('click'); + assert.deepStrictEqual(viewer.messages[1].path, []); + viewer.reply(viewer.messages[1], { + message: 'listview/navigation', navigation: { title: 'x', path: [], breadcrumbs: [{ label: 'x', path: [] }] }, + }); + viewer.reply(viewer.messages[2], { children: [] }); + viewer.back.fire('click'); + assert.deepStrictEqual(viewer.messages[3].path, [1, 2]); + viewer.reply(viewer.messages[3], { message: 'listview/navigation', navigation: initial }); + assert.strictEqual(viewer.root, original); + }); + + test('opening a table and failed navigation leave the list and its history intact', () => { + const viewer = createViewer(); + viewer.reply(viewer.messages[0], { children: [{ label: '$ df', index: 1, viewable: true }] }); + const original = viewer.root; + const button = original.children[0].children[0].children[0].children.at(-1); + assert.ok(button); + const click = { preventDefault: () => undefined, stopPropagation: () => undefined }; + button.fire('click', click); + viewer.reply(viewer.messages[1], { message: 'listview/navigation' }); + assert.strictEqual(viewer.root, original); + assert.strictEqual(viewer.back.disabled, true); + button.fire('click', click); + viewer.reply(viewer.messages[2], { message: 'listview/navigation', error: 'Item removed' }); + assert.strictEqual(viewer.status.textContent, 'Item removed'); + assert.strictEqual(viewer.root, original); + assert.strictEqual(viewer.back.disabled, true); + }); + + test('expands lazily, caches collapsed children, and opens descendants separately', () => { + const { root, messages, reply } = createViewer(); + assert.deepStrictEqual(messages[0].path, []); + reply(messages[0], { children: [{ label: '$ a', str: 'List of 1', index: 1, viewable: true, has_children: true }] }); + const entry = root.children[0].children[0]; + assert.strictEqual(entry.tag, 'details'); + assert.strictEqual(entry.children[1].childElementCount, 0); + assert.strictEqual(messages.length, 1); + entry.open = true; + entry.fire('toggle'); + assert.strictEqual(entry.children[1].childElementCount, 3); + assert.deepStrictEqual(messages[1].path, [1]); + reply(messages[1], { children: [{ label: '$ b', str: 'List of 1', index: 2, viewable: true, has_children: true }] }); + entry.open = false; + entry.fire('toggle'); + entry.open = true; + entry.fire('toggle'); + assert.strictEqual(messages.length, 2); + + const nested = entry.children[1].children[0].children[0]; + nested.open = true; + nested.fire('toggle'); + assert.deepStrictEqual(messages[2].path, [1, 2]); + let prevented = false; + let stopped = false; + const openButton = nested.children[0].children.at(-1); + assert.ok(openButton); + openButton.fire('click', { + preventDefault: () => { prevented = true; }, + stopPropagation: () => { stopped = true; }, + }); + assert.ok(prevented && stopped); + assert.strictEqual(messages[3].message, 'listview/view'); + assert.deepStrictEqual(messages[3].path, [1]); + assert.strictEqual(messages[3].index, 2); + }); + + test('routes concurrent expansion pages, retries failures, and paginates each branch', () => { + const { root, messages, reply } = createViewer(); + reply(messages[0], { children: [1, 2].map(index => ({ index, label: String(index), has_children: true })) }); + const [first, second] = root.children[0].children; + first.open = second.open = true; + first.fire('toggle'); + second.fire('toggle'); + reply(messages[2], { children: [{ label: 'second', index: 1 }], next_start: 501 }); + reply(messages[1], { error: 'Temporarily unavailable' }); + const firstPage = first.children[1]; + const secondPage = second.children[1]; + assert.strictEqual(firstPage.children[2].textContent, 'Temporarily unavailable'); + assert.strictEqual(secondPage.children[0].childElementCount, 1); + firstPage.children[1].fire('click'); + assert.deepStrictEqual(messages[3].path, [1]); + reply(messages[3], { children: [] }); + assert.strictEqual(firstPage.children[2].textContent, 'No items'); + secondPage.children[1].fire('click'); + assert.deepStrictEqual(messages[4].path, [2]); + assert.strictEqual(messages[4].start, 501); + reply(messages[4], { children: [{ label: 'last', index: 501 }] }); + assert.strictEqual(secondPage.children[0].childElementCount, 2); + assert.strictEqual(secondPage.children[1].hidden, true); + }); + + test('renders named and unnamed vector rows as plain text', () => { + const viewer = createViewer({ + title: 'x', path: [], breadcrumbs: [{ label: 'x', path: [] }], vector: true, + }); + const labels = ['first', '[2]', 'first', '', 'a b']; + viewer.reply(viewer.messages[0], { + children: labels.map((label, index) => ({ + label, str: '12.4', index: index + 1, + viewable: false, has_children: false, + })), + }); + labels.forEach((label, index) => { + const row = viewer.root.children[0].children[index].children[0]; + assert.deepStrictEqual(row.children.map(child => child.className), ['label', 'str']); + assert.strictEqual(row.children[0].textContent, label); + assert.strictEqual(row.children[0].innerHTML, ''); + assert.strictEqual(row.children[1].textContent, '12.4'); + assert.ok(!row.children.some(child => child.tag === 'button')); + }); + }); + + test('vectors share list navigation, Back and cached pages, including late page responses', () => { + const rootNavigation = { title: 'x', path: [], breadcrumbs: [{ label: 'x', path: [] }] }; + const vectorNavigation = { + title: 'x$v', path: [1], vector: true, + breadcrumbs: [{ label: 'x', path: [] }, { label: 'v', path: [1] }], + }; + const viewer = createViewer(rootNavigation); + const { messages, reply, back, bodyClasses } = viewer; + reply(messages[0], { children: [{ label: '$ v', index: 1, viewable: true }] }); + const original = viewer.root; + viewer.viewport.scrollTop = 120; + const button = original.children[0].children[0].children[0].children.at(-1); + assert.ok(button); + const click = { preventDefault: () => undefined, stopPropagation: () => undefined }; + button.fire('click', click); + reply(messages[1], { message: 'listview/navigation', navigation: vectorNavigation }); + const vectorPage = viewer.root; + assert.deepStrictEqual(messages[2].path, [1]); + assert.ok(bodyClasses.has('vector')); + assert.strictEqual(back.disabled, false); + + back.fire('click'); + reply(messages[3], { message: 'listview/navigation', navigation: rootNavigation }); + assert.strictEqual(viewer.root, original); + assert.strictEqual(viewer.viewport.scrollTop, 120); + assert.ok(!bodyClasses.has('vector')); + reply(messages[2], { + children: [{ label: '[1]', str: '42', index: 1, viewable: false, has_children: false }], + }); + + button.fire('click', click); + reply(messages[4], { message: 'listview/navigation', navigation: vectorNavigation }); + assert.strictEqual(viewer.root, vectorPage); + assert.strictEqual(messages.length, 5); + const row = vectorPage.children[0].children[0].children[0]; + assert.deepStrictEqual(row.children.map(child => child.className), ['label', 'str']); + assert.ok(bodyClasses.has('vector')); + viewer.breadcrumbs.children[0].fire('click'); + reply(messages[5], { message: 'listview/navigation', navigation: rootNavigation }); + assert.strictEqual(viewer.root, original); + assert.ok(!bodyClasses.has('vector')); + back.fire('click'); + reply(messages[6], { message: 'listview/navigation', navigation: vectorNavigation }); + assert.strictEqual(viewer.root, vectorPage); + assert.ok(bodyClasses.has('vector')); + }); + + test('ignores old viewer responses and hides open buttons for unavailable items', () => { + const { root, messages, reply } = createViewer(); + reply(messages[0], { documentGeneration: 6, children: [{ label: 'stale' }] }); + assert.strictEqual(root.children[0].childElementCount, 0); + reply(messages[0], { children: [ + { label: 'removed', viewable: false }, + { label: 'scalar', index: 2, viewable: true, has_children: false }, + ] }); + const [removed, scalar] = root.children[0].children; + assert.ok(!removed.children[0].children.some(child => child.tag === 'button')); + assert.ok(scalar.children[0].children.some(child => child.tag === 'button')); + assert.strictEqual(scalar.tag, 'div'); + }); +}); diff --git a/src/test/suite/listViewerPanels.test.ts b/src/test/suite/listViewerPanels.test.ts new file mode 100644 index 00000000..23f04f74 --- /dev/null +++ b/src/test/suite/listViewerPanels.test.ts @@ -0,0 +1,218 @@ +import * as assert from 'assert'; +import * as path from 'path'; +import * as sinon from 'sinon'; +import * as vscode from 'vscode'; +import { mockExtensionContext } from '../common/mockvscode'; +import * as session from '../../session'; +import { GlobalEnvItem } from '../../workspaceViewer'; + +suite('List viewer panels', () => { + let sandbox: sinon.SinonSandbox; + const panels: vscode.WebviewPanel[] = []; + + setup(() => { + sandbox = sinon.createSandbox(); + const root = path.join(__dirname, '..', '..', '..'); + mockExtensionContext(root, sandbox); + session.deploySessionWatcher(root); + }); + + teardown(() => { + panels.splice(0).forEach(panel => { panel.dispose(); }); + sandbox.restore(); + }); + + for (const source of ['list', 'table']) { + test(`reopens a disposed ${source} viewer using real VS Code panels`, async () => { + const create = sandbox.spy(vscode.window, 'createWebviewPanel'); + const viewId = `test-real-${source}`; + const open = async () => { + try { + await session.showDataView(source, 'json', 'x', '', 'Two', viewId); + } finally { + for (const panel of create.returnValues) { + if (!panels.includes(panel)) { + panels.push(panel); + } + } + } + }; + + await open(); + await open(); + sinon.assert.calledOnce(create); + create.firstCall.returnValue.dispose(); + await open(); + sinon.assert.calledTwice(create); + assert.notStrictEqual(create.firstCall.returnValue, create.secondCall.returnValue); + create.secondCall.returnValue.dispose(); + await open(); + sinon.assert.calledThrice(create); + }); + + test(`handles pending ${source} requests when a real VS Code panel closes`, async () => { + const panel = vscode.window.createWebviewPanel('dataview', 'x', vscode.ViewColumn.Two, {}); + panels.push(panel); + const receive = sandbox.spy(panel.webview, 'onDidReceiveMessage'); + sandbox.stub(vscode.window, 'createWebviewPanel').returns(panel); + await session.showDataView(source, 'json', 'x', '', 'Two', `test-pending-${source}`); + const documentGeneration = Number(/const documentGeneration = (\d+)/.exec(panel.webview.html)?.[1]); + const listener = receive.firstCall.args[0] as (message: unknown) => Promise; + // With no R session, requests settle asynchronously with an unavailable response. + const pending = source === 'table' + ? [listener({ + message: 'dataview/request', action: 'page', documentGeneration, requestId: 1, + })] + : ['listview/page', 'listview/navigate'].map(message => listener({ + message, documentGeneration, requestId: 1, path: [], start: 1, + })); + panel.dispose(); + await Promise.all(pending); + }); + } + + test('reuses separate list and table panels, updates titles, and reopens closed viewers', async () => { + const reveals: sinon.SinonStub[] = []; + const create = sandbox.stub(vscode.window, 'createWebviewPanel').callsFake((_type, title) => { + const disposed = new vscode.EventEmitter(); + const reveal = sandbox.stub(); + reveals.push(reveal); + const panel = { + title, viewColumn: vscode.ViewColumn.Three, + reveal, + webview: { + html: '', asWebviewUri: (uri: vscode.Uri) => uri, + onDidReceiveMessage: sandbox.stub(), + }, + onDidDispose: disposed.event, + dispose: () => { disposed.fire(); disposed.dispose(); }, + } as unknown as vscode.WebviewPanel; + panels.push(panel); + return panel; + }); + + await session.showDataView('list', 'json', 'x', '', 'Two', 'test-list-x'); + await session.showDataView('list', 'json', 'x$a', '', 'Two', 'test-list-x'); + await session.showDataView('list', 'json', 'x$a$b', '', 'Two', 'test-list-x'); + sinon.assert.calledOnce(create); + assert.strictEqual(panels[0].title, 'x$a$b'); + const listIcon = panels[0].iconPath as { dark: vscode.Uri; light: vscode.Uri }; + assert.strictEqual(path.basename(listIcon.dark.fsPath), 'preview.svg'); + assert.strictEqual(path.basename(listIcon.light.fsPath), 'preview.svg'); + sinon.assert.alwaysCalledWithExactly(reveals[0], vscode.ViewColumn.Three, true); + + await session.showDataView('table', 'json', 'x$df', '', 'Two', 'test-table-x'); + await session.showDataView('table', 'json', 'x$a$df', '', 'Two', 'test-table-x'); + assert.strictEqual(create.callCount, 2); + assert.strictEqual(panels[1].title, 'x$a$df'); + const tableIcon = panels[1].iconPath as { dark: vscode.Uri; light: vscode.Uri }; + assert.strictEqual(path.basename(tableIcon.dark.fsPath), 'open-preview.svg'); + assert.strictEqual(path.basename(tableIcon.light.fsPath), 'open-preview.svg'); + assert.strictEqual(panels[0].title, 'x$a$b'); + await session.showDataView('list', 'json', 'y', '', 'Two', 'test-list-y'); + assert.strictEqual(create.callCount, 3); + await session.showDataView('list', 'json', 'x$id', '', 'Two', 'test-list-x', { + title: 'x$id', path: [1], vector: true, + breadcrumbs: [{ label: 'x', path: [] }, { label: 'id', path: [1] }], + }); + assert.strictEqual(create.callCount, 3); + assert.strictEqual(panels[0].title, 'x$id'); + await session.showDataView('list', 'json', 'x', '', 'Two', 'test-list-x'); + assert.strictEqual(create.callCount, 3); + assert.strictEqual(panels[0].title, 'x'); + const first = panels.shift(); + assert.ok(first); + first.dispose(); + await session.showDataView('list', 'json', 'x', '', 'Two', 'test-list-x'); + assert.strictEqual(create.callCount, 4); + }); + + test('ignores requests and replies from a previous list page after panel reuse', async () => { + let receive: (message: unknown) => Promise = () => Promise.resolve(); + const postMessage = sandbox.stub().resolves(true); + const disposed = new vscode.EventEmitter(); + const panel = { + title: '', viewColumn: vscode.ViewColumn.Two, reveal: sandbox.stub(), + webview: { + html: '', asWebviewUri: (uri: vscode.Uri) => uri, postMessage, + onDidReceiveMessage: (listener: typeof receive) => { receive = listener; }, + }, + onDidDispose: disposed.event, + dispose: () => { disposed.fire(); disposed.dispose(); }, + } as unknown as vscode.WebviewPanel; + panels.push(panel); + sandbox.stub(vscode.window, 'createWebviewPanel').returns(panel); + const documentGeneration = () => Number(/const documentGeneration = (\d+)/.exec(panel.webview.html)?.[1]); + await session.showDataView('list', 'json', 'x', '', 'Two', 'test-list-document-generation'); + + for (const message of ['listview/navigate', 'listview/page']) { + const request = { message, documentGeneration: documentGeneration(), requestId: 1, path: [], start: 1 }; + // sessionRequest settles asynchronously even without an attached R session. + const pending = receive(request); + await session.showDataView('list', 'json', 'x$updated', '', 'Two', 'test-list-document-generation'); + await pending; + sinon.assert.notCalled(postMessage); + await receive(request); + sinon.assert.notCalled(postMessage); + assert.strictEqual(panel.title, 'x$updated'); + } + await receive({ message: 'listview/page', documentGeneration: documentGeneration(), requestId: 2, path: [], start: 1 }); + sinon.assert.calledOnce(postMessage); + const response = postMessage.firstCall.args[0] as { documentGeneration: number }; + assert.strictEqual(response.documentGeneration, documentGeneration()); + }); + + test('ignores requests and replies from a previous data viewer document after panel reuse', async () => { + let receive: (message: unknown) => Promise = () => Promise.resolve(); + const postMessage = sandbox.stub().resolves(true); + const disposed = new vscode.EventEmitter(); + const panel = { + title: '', viewColumn: vscode.ViewColumn.Two, reveal: sandbox.stub(), + webview: { + html: '', asWebviewUri: (uri: vscode.Uri) => uri, postMessage, + onDidReceiveMessage: (listener: typeof receive) => { receive = listener; }, + }, + onDidDispose: disposed.event, + dispose: () => { disposed.fire(); disposed.dispose(); }, + } as unknown as vscode.WebviewPanel; + panels.push(panel); + sandbox.stub(vscode.window, 'createWebviewPanel').returns(panel); + const documentGeneration = () => + Number(/const documentGeneration = (\d+)/.exec(panel.webview.html)?.[1]); + await session.showDataView('table', 'json', 'x', '', 'Two', 'test-dataview-document-generation'); + + const request = { + message: 'dataview/request', action: 'page', + documentGeneration: documentGeneration(), requestId: 1, + }; + const pending = receive(request); + await session.showDataView( + 'table', 'json', 'x$updated', '', 'Two', 'test-dataview-document-generation' + ); + await pending; + sinon.assert.notCalled(postMessage); + await receive(request); + sinon.assert.notCalled(postMessage); + + await receive({ + ...request, documentGeneration: documentGeneration(), requestId: 2, + }); + sinon.assert.calledOnce(postMessage); + const response = postMessage.firstCall.args[0] as { documentGeneration: number }; + assert.strictEqual(response.documentGeneration, documentGeneration()); + }); + + test('supported workspace children retain open actions alongside expansion', () => { + for (const type of ['list', 'environment', 'pairlist', 'S4', 'double', 'closure']) { + const structured = ['list', 'environment', 'pairlist', 'S4'].includes(type); + const node = new GlobalEnvItem('', type, '$ item', type, 1, undefined, structured, + 'x', [{ kind: 'index', value: 1 }], undefined, true); + assert.strictEqual(node.contextValue, 'viewableNode'); + assert.strictEqual(node.collapsibleState, structured + ? vscode.TreeItemCollapsibleState.Collapsed : vscode.TreeItemCollapsibleState.None); + } + const unavailable = new GlobalEnvItem('', 'active_binding', '', 'active_binding', 1, + undefined, false, 'x', [], undefined, false); + assert.notStrictEqual(unavailable.contextValue, 'viewableNode'); + }); +}); diff --git a/src/test/suite/session.test.ts b/src/test/suite/session.test.ts index a1d29c9b..396f3b4f 100644 --- a/src/test/suite/session.test.ts +++ b/src/test/suite/session.test.ts @@ -289,7 +289,7 @@ suite('Session Communication', () => { const connection = await api.getConnectionInfo(); assert.ok(connection); - assert.strictEqual(connection.protocolVersion, 1); + assert.strictEqual(connection.protocolVersion, 2); assert.strictEqual(connection.endpoint, session.globalPipePath); assert.strictEqual(connection.plotBackend, 'standard'); assert.ok(!('socket' in connection), 'connection info should contain plain contract data only'); @@ -328,7 +328,7 @@ suite('Session Communication', () => { jsonrpc: '2.0', method: 'attach', params: { - protocol_version: 1, + protocol_version: 2, session_id: id, host: 'test-remote-host', version: '4.4.0', @@ -594,9 +594,8 @@ suite('Session Communication', () => { term.sendText('View(list(value = 1L), title = c("List title", "second line"))\n'); await waitFor(() => createWebviewPanelSpy.calledWith('dataview', 'List title,second line'), 10000, 200); - const executeCommandSpy = sandbox.spy(vscode.commands, 'executeCommand'); term.sendText('View(1:3, title = c("Object title", "second line"))\n'); - await waitFor(() => executeCommandSpy.calledWith('vscode.open'), 10000, 200); + await waitFor(() => createWebviewPanelSpy.calledWith('dataview', 'Object title,second line'), 10000, 200); // Objects and arrays containing non-string values must still be rejected. const dataViewCount = createWebviewPanelSpy.withArgs('dataview').callCount; @@ -704,7 +703,7 @@ suite('Session Communication', () => { sockets.push(socket); client.write(`${JSON.stringify({ jsonrpc: '2.0', method: 'attach', params: { - protocol_version: 1, session_id: `readiness-${terminalPid}`, host: os.hostname(), + protocol_version: 2, session_id: `readiness-${terminalPid}`, host: os.hostname(), pid: terminalPid, version: '4.4.0', tempdir: '/tmp', wd: '/tmp', }, })}\n`); @@ -790,7 +789,7 @@ suite('Session Communication', () => { const id = `reconnected-${terminalPid}`; client.write(`${JSON.stringify({ jsonrpc: '2.0', method: 'attach', params: { - protocol_version: 1, session_id: id, host: os.hostname(), + protocol_version: 2, session_id: id, host: os.hostname(), pid: terminalPid, version: '4.4.0', tempdir: '/tmp', wd: '/tmp', }, })}\n`); @@ -848,7 +847,7 @@ suite('Session Communication', () => { }); client.write(`${JSON.stringify({ jsonrpc: '2.0', method: 'attach', params: { - protocol_version: 1, session_id: 'manual-recovery-first', + protocol_version: 2, session_id: 'manual-recovery-first', host: os.hostname(), pid: 45240, version: '4.5.0', tempdir: os.tmpdir(), wd: os.tmpdir(), info: { version: 'R 4.5.0', command: 'R', start_time: '' }, @@ -1005,7 +1004,7 @@ suite('Session Communication', () => { jsonrpc: '2.0', method: 'attach', params: { - protocol_version: 1, + protocol_version: 2, session_id: 'stable-session-id', host: 'remote-compute-node', sess_version: '3.0.0', @@ -1053,7 +1052,7 @@ suite('Session Communication', () => { jsonrpc: '2.0', method: 'attach', params: { - protocol_version: 1, + protocol_version: 2, session_id: 'other-session-id', host: 'other-compute-node', sess_version: '3.0.0', @@ -1088,7 +1087,7 @@ suite('Session Communication', () => { jsonrpc: '2.0', method: 'attach', params: { - protocol_version: 1, + protocol_version: 2, session_id: sessionId, host: 'remote-compute-node', sess_version: '3.0.0', @@ -1136,11 +1135,11 @@ suite('Session Communication', () => { client.write(`${JSON.stringify({ jsonrpc: '2.0', method: 'attach', - params: { protocol_version: 2, session_id: 'future-session' } + params: { protocol_version: 1, session_id: 'legacy-session' } })}\n`); await waitFor(() => showError.called); - assert.match(String(showError.firstCall.args[0]), /unsupported sess protocol version 2/); - assert.notStrictEqual(session.activeSession?.sessionId, 'future-session'); + assert.match(String(showError.firstCall.args[0]), /unsupported sess protocol version 1; this extension requires protocol version 2/); + assert.notStrictEqual(session.activeSession?.sessionId, 'legacy-session'); } finally { client.destroy(); } @@ -1159,7 +1158,7 @@ suite('Session Communication', () => { jsonrpc: '2.0', method: 'attach', params: { - protocol_version: 1, + protocol_version: 2, session_id: 'malformed-session', tempdir: { path: os.tmpdir() }, wd: os.tmpdir() diff --git a/src/test/suite/workspaceViewer.test.ts b/src/test/suite/workspaceViewer.test.ts index 8bec4cfc..ab0c4e13 100644 --- a/src/test/suite/workspaceViewer.test.ts +++ b/src/test/suite/workspaceViewer.test.ts @@ -113,6 +113,25 @@ suite('Workspace Viewer', () => { sinon.assert.notCalled(second.execute as sinon.SinonStub); }); + test('nested View keeps the originating session after focus changes', async () => { + session.updateSessionWorkspace(first, data('nested')); + const request = sandbox.stub().resolves({ children: [{ + str: '$ table', class: 'data.frame', type: 'list', has_children: true, + viewable: true, selector: { kind: 'index', value: 1 }, + }] }); + first.requester = request; + const children = await provider.getChildren((await envNodes())[0]); + const child = children[0] as workspace.GlobalEnvItem; + assert.strictEqual(child.contextValue, 'viewableNode'); + await session.activateSession(second); + request.resetHistory(); + await workspace.viewItem(child); + sinon.assert.calledOnceWithExactly(request, { + method: 'workspace_view', params: { name: 'nested', path: [{ kind: 'index', value: 1 }] }, + }); + sinon.assert.notCalled(second.execute as sinon.SinonStub); + }); + test('clear confirmation captures the displayed session before switching', async () => { const prompt = sandbox.stub(vscode.window, 'showInformationMessage').callsFake(async () => { await session.activateSession(second); diff --git a/src/workspaceViewer.ts b/src/workspaceViewer.ts index 0c08f5df..78cae3a7 100644 --- a/src/workspaceViewer.ts +++ b/src/workspaceViewer.ts @@ -21,6 +21,7 @@ interface WorkspaceChild { class: string; type: string; has_children: boolean; + viewable?: boolean; selector?: WorkspaceSelector; } @@ -161,7 +162,8 @@ export class WorkspaceDataProvider implements TreeDataProvider { child.has_children, element.rootName, child.selector ? [...element.objectPath, child.selector] : element.objectPath, - element.owner + element.owner, + child.viewable ) ); if (page.nextStart !== undefined) { @@ -369,6 +371,7 @@ export class GlobalEnvItem extends TreeItem { rootName?: string, objectPath?: WorkspaceSelector[], public readonly owner?: Session, + viewable?: boolean, ) { super( label, @@ -387,7 +390,7 @@ export class GlobalEnvItem extends TreeItem { ); this.tooltip = this.getTooltip(label, rClass, treeLevel); this.iconPath = this.getIcon(type, dim); - this.contextValue = treeLevel === 0 ? 'rootNode' : `childNode${this.treeLevel}`; + this.contextValue = treeLevel === 0 ? 'rootNode' : viewable ? 'viewableNode' : `childNode${this.treeLevel}`; } private getDescription(dim: number[] | undefined, str: string, rClass: string, type: string): string { @@ -492,8 +495,15 @@ export async function loadWorkspace(): Promise { } export async function viewItem(node: GlobalEnvItem): Promise { - if (node.owner && node.label) { - await runWorkspaceCode(`View(get(${JSON.stringify(node.label)}, envir = .GlobalEnv, inherits = FALSE), title = ${JSON.stringify(node.label)})`, node.owner); + if (node.owner && !node.owner.workspaceUnavailable && node.rootName) { + if (!node.objectPath.length) { + await runWorkspaceCode(`View(get(${JSON.stringify(node.rootName)}, envir = .GlobalEnv, inherits = FALSE), title = ${JSON.stringify(node.rootName)})`, node.owner); + return; + } + await sessionRequest({ + method: 'workspace_view', + params: { name: node.rootName, path: node.objectPath }, + }, node.owner); } }