From 5bdb1ba3d336bc848de1986b2ad62f8ca90f7006 Mon Sep 17 00:00:00 2001 From: "4111978+Fred-Wu@users.noreply.github.com" <4111978+Fred-Wu@users.noreply.github.com> Date: Sun, 20 Sep 2026 16:38:54 +1000 Subject: [PATCH 01/30] Enable viewing nested workspace lists and tables --- package.json | 2 +- sess/R/handlers.R | 13 ++++++++++++- sess/R/server.R | 1 + sess/inst/tinytest/test-dataview.R | 8 ++++++++ src/extension.ts | 2 +- src/workspaceViewer.ts | 14 ++++++++++---- 6 files changed, 33 insertions(+), 7 deletions(-) diff --git a/package.json b/package.json index 1d4d195cd..b247feced 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", diff --git a/sess/R/handlers.R b/sess/R/handlers.R index 7a64089c9..7c266241c 100644 --- a/sess/R/handlers.R +++ b/sess/R/handlers.R @@ -120,6 +120,7 @@ workspace_child_item <- function(object, str, selector) { class = paste(class(object), collapse = ", "), type = typeof(object), has_children = workspace_child_count(object) > 0L, + viewable = is.list(object) || dataview_is_table(object), selector = selector ) } @@ -132,6 +133,16 @@ workspace_child_label <- function(name, index) { } } +handle_workspace_view <- function(name, path = list()) { + title <- paste0(c(name, vapply(path, function(selector) switch(selector$kind, + index = if (!is.null(selector$name) && !is.na(selector$name) && nzchar(selector$name)) paste0("$", selector$name) else paste0("[[", selector$value, "]]"), + name = paste0("$", selector$value), + slot = paste0("@", selector$value) + ), "")), collapse = "") + utils::View(workspace_object(name, path), title = title) + TRUE +} + get_workspace_children <- function(name, path = list(), start = 1L) { tryCatch({ object <- workspace_object(name, path) @@ -188,7 +199,7 @@ get_workspace_children <- function(name, path = list(), start = 1L) { ": ", trimws(try_capture_str(child)) ), - list(kind = "index", value = index) + list(kind = "index", value = index, name = child_name) ) }) } diff --git a/sess/R/server.R b/sess/R/server.R index 86b5f1952..754737f0b 100644 --- a/sess/R/server.R +++ b/sess/R/server.R @@ -451,6 +451,7 @@ dispatch_message <- function(line) { }, "workspace" = function(p) get_workspace_data(), "workspace_children" = function(p) get_workspace_children(p$name, p$path, p$start), + "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), diff --git a/sess/inst/tinytest/test-dataview.R b/sess/inst/tinytest/test-dataview.R index 07433a658..a8a21d4c9 100644 --- a/sess/inst/tinytest/test-dataview.R +++ b/sess/inst/tinytest/test-dataview.R @@ -322,3 +322,11 @@ local({ expect_identical(page$rows[["2"]], df$Ozone[c(4L, 3L), , drop = FALSE]) expect_identical(serialize(df, NULL), original) }) + +# Workspace children expose the View action only for supported table 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) +}) diff --git a/src/extension.ts b/src/extension.ts index 9c67765b9..a2fce9415 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/workspaceViewer.ts b/src/workspaceViewer.ts index 0c08f5df7..a1583253a 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,11 @@ 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) { + await sessionRequest({ + method: 'workspace_view', + params: { name: node.rootName, path: node.objectPath }, + }, node.owner); } } From 54c166d5d79a7314265a3ba506852d4bc66541ea Mon Sep 17 00:00:00 2001 From: "4111978+Fred-Wu@users.noreply.github.com" <4111978+Fred-Wu@users.noreply.github.com> Date: Sun, 20 Sep 2026 16:39:00 +1000 Subject: [PATCH 02/30] Show shallow list summaries with child navigation --- sess/R/handlers.R | 18 +++++++ sess/R/hooks.R | 21 +++++++- sess/R/server.R | 1 + src/session.ts | 124 +++++++++++++++++++++++++++++----------------- 4 files changed, 116 insertions(+), 48 deletions(-) diff --git a/sess/R/handlers.R b/sess/R/handlers.R index 7c266241c..7ddf74966 100644 --- a/sess/R/handlers.R +++ b/sess/R/handlers.R @@ -211,6 +211,24 @@ get_workspace_children <- function(name, path = list(), start = 1L) { }, error = function(e) list(children = I(list()), next_start = NULL)) } +handle_listview_view <- function(view_id, index) { + state <- .sess_env$dataviews[[as.character(view_id)]] + index <- as.integer(index) + if (is.null(state) || !identical(state$type, "list") || + is.na(index) || index < 1L || index > length(state$data)) { + return(FALSE) + } + child_names <- names(state$data) + child_name <- if (is.null(child_names)) NULL else child_names[[index]] + title <- if (!is.null(child_name) && !is.na(child_name) && nzchar(child_name)) { + paste0(state$title, "$", child_name) + } else { + paste0(state$title, "[[", index, "]]") + } + utils::View(state$data[[index]], title = title) + TRUE +} + handle_hover <- function(expr_str) { tryCatch( { diff --git a/sess/R/hooks.R b/sess/R/hooks.R index 62e3ed902..4577b70b0 100644 --- a/sess/R/hooks.R +++ b/sess/R/hooks.R @@ -134,13 +134,30 @@ runtime_start <- function(use_rstudioapi = TRUE, view_id = registration$view_id )) } else if (is.list(x)) { + view_id <- dataview_new_id() + .sess_env$dataviews[[view_id]] <- list( + type = "list", + data = x, + title = paste(as.character(title), collapse = "\n") + ) + x_names <- names(x) + children <- lapply(seq_along(x), function(index) { + child <- x[[index]] + list( + label = workspace_child_label(if (is.null(x_names)) NULL else x_names[[index]], index), + str = trimws(try_capture_str(child)), + viewable = is.list(child) || dataview_is_table(child), + index = index + ) + }) file_path <- tempfile(tmpdir = .sess_env$tempdir, fileext = ".json") - jsonlite::write_json(x, file_path, auto_unbox = TRUE, null = "null", na = "string") + jsonlite::write_json(list(children = I(children)), file_path, auto_unbox = TRUE) notify_client("dataview", list( title = title, file = file_path, source = "list", - type = "json" + type = "json", + view_id = view_id )) } else { code <- if (is.primitive(x)) utils::capture.output(print(x)) else deparse(x) diff --git a/sess/R/server.R b/sess/R/server.R index 754737f0b..353859879 100644 --- a/sess/R/server.R +++ b/sess/R/server.R @@ -455,6 +455,7 @@ dispatch_message <- function(line) { "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_view" = function(p) handle_listview_view(p$view_id, p$index), "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/src/session.ts b/src/session.ts index 14ef5becc..280b726d1 100644 --- a/src/session.ts +++ b/src/session.ts @@ -1060,6 +1060,22 @@ export async function showDataView(source: string, type: string, title: string, }); const content = await getListHtml(panel.webview, file, title); panel.iconPath = new UriIcon('open-preview'); + if (viewId) { + panel.webview.onDidReceiveMessage((message: { message?: string; index?: number }) => { + if (message.message === 'listview/view' && typeof message.index === 'number' && Number.isInteger(message.index)) { + void sessionRequest({ + method: 'listview_view', + params: { view_id: viewId, index: message.index }, + }); + } + }); + panel.onDidDispose(() => { + void sessionRequest({ + method: 'dataview_dispose', + params: { view_id: viewId }, + }); + }); + } panel.webview.html = content; } else { await commands.executeCommand('vscode.open', Uri.file(file), { @@ -1764,7 +1780,7 @@ export async function getTableHtml(webview: Webview, file: string | undefined, t } export async function getListHtml(webview: Webview, file: string, title: string): Promise { - const content = await readContent(file, 'utf8'); + const content = (await readContent(file, 'utf8')).replace(/ @@ -1773,64 +1789,80 @@ export async function getListHtml(webview: Webview, file: string, title: string) ${escapeHtml(title)} - - - - - -

+    
+ `; From 0588cde74354411e439d25b30cc2551776ac8824 Mon Sep 17 00:00:00 2001 From: "4111978+Fred-Wu@users.noreply.github.com" <4111978+Fred-Wu@users.noreply.github.com> Date: Sun, 20 Sep 2026 16:39:07 +1000 Subject: [PATCH 03/30] Match list viewer actions to workspace View icons --- images/icons/dark/open-preview-codicon.svg | 1 + images/icons/light/open-preview-codicon.svg | 1 + src/session.ts | 32 +++++++++++++++++---- 3 files changed, 28 insertions(+), 6 deletions(-) create mode 100644 images/icons/dark/open-preview-codicon.svg create mode 100644 images/icons/light/open-preview-codicon.svg diff --git a/images/icons/dark/open-preview-codicon.svg b/images/icons/dark/open-preview-codicon.svg new file mode 100644 index 000000000..5dc0bba6b --- /dev/null +++ b/images/icons/dark/open-preview-codicon.svg @@ -0,0 +1 @@ + diff --git a/images/icons/light/open-preview-codicon.svg b/images/icons/light/open-preview-codicon.svg new file mode 100644 index 000000000..9e363217a --- /dev/null +++ b/images/icons/light/open-preview-codicon.svg @@ -0,0 +1 @@ + diff --git a/src/session.ts b/src/session.ts index 280b726d1..f108ec79e 100644 --- a/src/session.ts +++ b/src/session.ts @@ -1056,7 +1056,7 @@ export async function showDataView(source: string, type: string, title: string, enableScripts: true, enableFindWidget: true, retainContextWhenHidden: true, - localResourceRoots: [Uri.file(resDir)], + localResourceRoots: [Uri.file(resDir), Uri.file(extensionContext.asAbsolutePath('images/icons'))], }); const content = await getListHtml(panel.webview, file, title); panel.iconPath = new UriIcon('open-preview'); @@ -1781,6 +1781,9 @@ export async function getTableHtml(webview: Webview, file: string | undefined, t export async function getListHtml(webview: Webview, file: string, title: string): Promise { const content = (await readContent(file, 'utf8')).replace(/ @@ -1819,14 +1822,29 @@ export async function getListHtml(webview: Webview, file: string, title: string) white-space: pre-wrap; } button { + display: flex; + align-items: center; border: 0; - padding: 2px 8px; - color: var(--vscode-button-foreground); - background-color: var(--vscode-button-background); + padding: 2px; + color: var(--vscode-foreground); + background: transparent; cursor: pointer; } button:hover { - background-color: var(--vscode-button-hoverBackground); + background-color: var(--vscode-toolbar-hoverBackground); + } + button img { + width: 16px; + height: 16px; + } + .light-icon { + display: none; + } + body.vscode-light .dark-icon { + display: none; + } + body.vscode-light .light-icon { + display: block; } @@ -1853,7 +1871,9 @@ export async function getListHtml(webview: Webview, file: string, title: string) if (item.viewable) { const button = document.createElement('button'); - button.textContent = 'View'; + button.title = 'View'; + button.setAttribute('aria-label', 'View'); + button.innerHTML = ''; button.addEventListener('click', () => { vscode.postMessage({ message: 'listview/view', index: item.index }); }); From dd26ffa0c2cdb372e7b3350fa9b64321aa598a69 Mon Sep 17 00:00:00 2001 From: "4111978+Fred-Wu@users.noreply.github.com" <4111978+Fred-Wu@users.noreply.github.com> Date: Sun, 20 Sep 2026 16:39:13 +1000 Subject: [PATCH 04/30] Reuse list viewer panels by title --- sess/R/hooks.R | 20 +++++++++++--------- src/session.ts | 25 +++++++++++++++++++------ 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/sess/R/hooks.R b/sess/R/hooks.R index 4577b70b0..c1e50c7db 100644 --- a/sess/R/hooks.R +++ b/sess/R/hooks.R @@ -110,21 +110,24 @@ runtime_start <- function(use_rstudioapi = TRUE, return(invisible(NULL)) } - if (dataview_is_table(x)) { + view_type <- if (dataview_is_table(x)) "table" else if (is.list(x)) "list" else "object" + if (view_type != "object") { title_key <- paste(as.character(title), collapse = "\n") + registry_key <- paste0(view_type, ":", title_key) 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) + 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(title_key, id, envir = dataview_registry) + assign(registry_key, id, envir = dataview_registry) } id } + } + if (view_type == "table") { registration <- dataview_register(x, view_id = view_id) notify_client("dataview", list( @@ -133,12 +136,11 @@ runtime_start <- function(use_rstudioapi = TRUE, type = "json", view_id = registration$view_id )) - } else if (is.list(x)) { - view_id <- dataview_new_id() + } else if (view_type == "list") { .sess_env$dataviews[[view_id]] <- list( type = "list", data = x, - title = paste(as.character(title), collapse = "\n") + title = title_key ) x_names <- names(x) children <- lapply(seq_along(x), function(index) { diff --git a/src/session.ts b/src/session.ts index f108ec79e..d114555c8 100644 --- a/src/session.ts +++ b/src/session.ts @@ -1047,6 +1047,16 @@ export async function showDataView(source: string, type: string, title: string, const content = await getTableHtml(panel.webview, file || undefined, title); panel.webview.html = content; } else if (source === 'list') { + if (viewId) { + const existing = dynamicDataViewPanels.get(viewId); + if (existing) { + existing.title = title; + existing.reveal(ViewColumn[viewer as keyof typeof ViewColumn], true); + existing.webview.html = await getListHtml(existing.webview, file, title); + return; + } + } + const panel = window.createWebviewPanel('dataview', title, { preserveFocus: true, @@ -1056,11 +1066,11 @@ export async function showDataView(source: string, type: string, title: string, enableScripts: true, enableFindWidget: true, retainContextWhenHidden: true, - localResourceRoots: [Uri.file(resDir), Uri.file(extensionContext.asAbsolutePath('images/icons'))], + localResourceRoots: [Uri.file(extensionContext.asAbsolutePath('images/icons'))], }); - const content = await getListHtml(panel.webview, file, title); panel.iconPath = new UriIcon('open-preview'); if (viewId) { + dynamicDataViewPanels.set(viewId, panel); panel.webview.onDidReceiveMessage((message: { message?: string; index?: number }) => { if (message.message === 'listview/view' && typeof message.index === 'number' && Number.isInteger(message.index)) { void sessionRequest({ @@ -1070,13 +1080,16 @@ export async function showDataView(source: string, type: string, title: string, } }); panel.onDidDispose(() => { + if (dynamicDataViewPanels.get(viewId) === panel) { + dynamicDataViewPanels.delete(viewId); + } void sessionRequest({ method: 'dataview_dispose', params: { view_id: viewId }, }); }); } - panel.webview.html = content; + panel.webview.html = await getListHtml(panel.webview, file, title); } else { await commands.executeCommand('vscode.open', Uri.file(file), { preserveFocus: true, @@ -1780,10 +1793,10 @@ export async function getTableHtml(webview: Webview, file: string | undefined, t } export async function getListHtml(webview: Webview, file: string, title: string): Promise { - const content = (await readContent(file, 'utf8')).replace(/ From c66d423d71ebf0fbf027a44c8f6f45b0e2912aa0 Mon Sep 17 00:00:00 2001 From: "4111978+Fred-Wu@users.noreply.github.com" <4111978+Fred-Wu@users.noreply.github.com> Date: Sun, 20 Sep 2026 20:16:49 +1000 Subject: [PATCH 05/30] Nested View icons now appear for all supported values --- sess/R/handlers.R | 28 +++++++++++++++----- sess/R/hooks.R | 41 +++++++++++++++++++++++++----- sess/inst/tinytest/test-dataview.R | 9 ++++++- 3 files changed, 63 insertions(+), 15 deletions(-) diff --git a/sess/R/handlers.R b/sess/R/handlers.R index 7ddf74966..b3e2b1c70 100644 --- a/sess/R/handlers.R +++ b/sess/R/handlers.R @@ -120,7 +120,7 @@ workspace_child_item <- function(object, str, selector) { class = paste(class(object), collapse = ", "), type = typeof(object), has_children = workspace_child_count(object) > 0L, - viewable = is.list(object) || dataview_is_table(object), + viewable = TRUE, selector = selector ) } @@ -213,19 +213,33 @@ get_workspace_children <- function(name, path = list(), start = 1L) { handle_listview_view <- function(view_id, index) { state <- .sess_env$dataviews[[as.character(view_id)]] + if (is.null(state) || !identical(state$type, "list")) { + return(FALSE) + } index <- as.integer(index) - if (is.null(state) || !identical(state$type, "list") || - is.na(index) || index < 1L || index > length(state$data)) { + child_count <- if (state$kind == "index") length(state$data) else length(state$names) + if (length(index) != 1L || is.na(index) || index < 1L || index > child_count) { return(FALSE) } - child_names <- names(state$data) - child_name <- if (is.null(child_names)) NULL else child_names[[index]] - title <- if (!is.null(child_name) && !is.na(child_name) && nzchar(child_name)) { + 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))) { + return(FALSE) + } + child <- switch(state$kind, + name = get(child_name, envir = state$data, inherits = FALSE), + slot = methods::slot(state$data, child_name), + index = state$data[[index]] + ) + 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, "]]") } - utils::View(state$data[[index]], title = title) + utils::View(child, title = title) TRUE } diff --git a/sess/R/hooks.R b/sess/R/hooks.R index c1e50c7db..5f8cd92ba 100644 --- a/sess/R/hooks.R +++ b/sess/R/hooks.R @@ -110,7 +110,13 @@ runtime_start <- function(use_rstudioapi = TRUE, return(invisible(NULL)) } - view_type <- if (dataview_is_table(x)) "table" else if (is.list(x)) "list" else "object" + view_type <- if (dataview_is_table(x)) { + "table" + } else if (is.list(x) || is.environment(x) || isS4(x)) { + "list" + } else { + "object" + } if (view_type != "object") { title_key <- paste(as.character(title), collapse = "\n") registry_key <- paste0(view_type, ":", title_key) @@ -137,18 +143,39 @@ runtime_start <- function(use_rstudioapi = TRUE, view_id = registration$view_id )) } else if (view_type == "list") { + child_kind <- if (is.environment(x)) "name" else if (isS4(x)) "slot" else "index" + x_names <- switch(child_kind, + name = workspace_env_names(x), + slot = methods::slotNames(x), + index = names(x) + ) .sess_env$dataviews[[view_id]] <- list( type = "list", data = x, - title = title_key + title = title_key, + kind = child_kind, + names = x_names ) - x_names <- names(x) - children <- lapply(seq_along(x), function(index) { - child <- x[[index]] + indices <- if (child_kind == "index") seq_along(x) else seq_along(x_names) + children <- lapply(indices, function(index) { + child_name <- if (is.null(x_names)) NULL else x_names[[index]] + label <- if (child_kind == "slot") { + paste0("@ ", child_name) + } else { + workspace_child_label(child_name, index) + } + if (child_kind == "name" && bindingIsActive(child_name, x)) { + return(list(label = label, str = "(active-binding)", viewable = FALSE, index = index)) + } + child <- switch(child_kind, + name = get(child_name, envir = x, inherits = FALSE), + slot = methods::slot(x, child_name), + index = x[[index]] + ) list( - label = workspace_child_label(if (is.null(x_names)) NULL else x_names[[index]], index), + label = label, str = trimws(try_capture_str(child)), - viewable = is.list(child) || dataview_is_table(child), + viewable = TRUE, index = index ) }) diff --git a/sess/inst/tinytest/test-dataview.R b/sess/inst/tinytest/test-dataview.R index a8a21d4c9..f611530b8 100644 --- a/sess/inst/tinytest/test-dataview.R +++ b/sess/inst/tinytest/test-dataview.R @@ -323,10 +323,17 @@ local({ expect_identical(serialize(df, NULL), original) }) -# Workspace children expose the View action only for supported table objects. +# 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) }) From a44eb76e45ac0a87bbb6b4166c3ac498c35da221 Mon Sep 17 00:00:00 2001 From: "4111978+Fred-Wu@users.noreply.github.com" <4111978+Fred-Wu@users.noreply.github.com> Date: Sun, 20 Sep 2026 21:49:25 +1000 Subject: [PATCH 06/30] Load list viewer items on demand using workspace pagination --- sess/R/handlers.R | 116 +++++++++++++++-------------- sess/R/hooks.R | 26 ------- sess/R/server.R | 2 +- sess/inst/tinytest/test-dataview.R | 31 ++++++++ src/session.ts | 113 ++++++++++++++++++++-------- 5 files changed, 174 insertions(+), 114 deletions(-) diff --git a/sess/R/handlers.R b/sess/R/handlers.R index b3e2b1c70..f810dbc19 100644 --- a/sess/R/handlers.R +++ b/sess/R/handlers.R @@ -143,72 +143,76 @@ handle_workspace_view <- function(name, path = list()) { TRUE } -get_workspace_children <- function(name, path = list(), start = 1L) { +get_workspace_children <- function(name = NULL, path = list(), start = 1L, view_id = NULL) { tryCatch({ - object <- workspace_object(name, path) - child_count <- workspace_child_count(object) - if (child_count == 0L) { - return(list(children = I(list()), next_start = NULL)) + 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") + object <- state$data + kind <- state$kind + child_names <- state$names } - + child_count <- if (kind == "index") 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)) } - 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 (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 <- switch(kind, + name = get(child_name, envir = object, inherits = FALSE), + slot = methods::slot(object, child_name), + index = object[[index]] + ) + summary <- trimws(try_capture_str(child)) + if (!is.null(view_id)) { + list(label = label, str = summary, viewable = TRUE, index = index) + } else { + selector <- if (kind == "index") { + list(kind = kind, value = index, 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) - ) + list(kind = kind, value = child_name) } - }) - } 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, name = child_name) - ) - }) - } - - list( - children = I(children), - next_start = if (end < child_count) end + 1L else NULL - ) - }, error = function(e) list(children = I(list()), next_start = NULL)) + 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) + }) } handle_listview_view <- function(view_id, index) { diff --git a/sess/R/hooks.R b/sess/R/hooks.R index 5f8cd92ba..ca395fd4a 100644 --- a/sess/R/hooks.R +++ b/sess/R/hooks.R @@ -156,34 +156,8 @@ runtime_start <- function(use_rstudioapi = TRUE, kind = child_kind, names = x_names ) - indices <- if (child_kind == "index") seq_along(x) else seq_along(x_names) - children <- lapply(indices, function(index) { - child_name <- if (is.null(x_names)) NULL else x_names[[index]] - label <- if (child_kind == "slot") { - paste0("@ ", child_name) - } else { - workspace_child_label(child_name, index) - } - if (child_kind == "name" && bindingIsActive(child_name, x)) { - return(list(label = label, str = "(active-binding)", viewable = FALSE, index = index)) - } - child <- switch(child_kind, - name = get(child_name, envir = x, inherits = FALSE), - slot = methods::slot(x, child_name), - index = x[[index]] - ) - list( - label = label, - str = trimws(try_capture_str(child)), - viewable = TRUE, - index = index - ) - }) - file_path <- tempfile(tmpdir = .sess_env$tempdir, fileext = ".json") - jsonlite::write_json(list(children = I(children)), file_path, auto_unbox = TRUE) notify_client("dataview", list( title = title, - file = file_path, source = "list", type = "json", view_id = view_id diff --git a/sess/R/server.R b/sess/R/server.R index 353859879..f97bec091 100644 --- a/sess/R/server.R +++ b/sess/R/server.R @@ -450,7 +450,7 @@ 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), diff --git a/sess/inst/tinytest/test-dataview.R b/sess/inst/tinytest/test-dataview.R index f611530b8..977121b47 100644 --- a/sess/inst/tinytest/test-dataview.R +++ b/sess/inst/tinytest/test-dataview.R @@ -337,3 +337,34 @@ local({ 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) +}) diff --git a/src/session.ts b/src/session.ts index d114555c8..51ada9d52 100644 --- a/src/session.ts +++ b/src/session.ts @@ -1052,7 +1052,7 @@ export async function showDataView(source: string, type: string, title: string, if (existing) { existing.title = title; existing.reveal(ViewColumn[viewer as keyof typeof ViewColumn], true); - existing.webview.html = await getListHtml(existing.webview, file, title); + existing.webview.html = getListHtml(existing.webview, title); return; } } @@ -1071,12 +1071,24 @@ export async function showDataView(source: string, type: string, title: string, panel.iconPath = new UriIcon('open-preview'); if (viewId) { dynamicDataViewPanels.set(viewId, panel); - panel.webview.onDidReceiveMessage((message: { message?: string; index?: number }) => { + panel.webview.onDidReceiveMessage(async (message: { message?: string; index?: number; start?: number; requestId?: number }) => { if (message.message === 'listview/view' && typeof message.index === 'number' && Number.isInteger(message.index)) { void sessionRequest({ method: 'listview_view', params: { view_id: viewId, index: message.index }, }); + } 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 }, + }) as { children?: unknown; next_start?: number | null } | undefined; + void panel.webview.postMessage({ + message: 'listview/page', + requestId: message.requestId, + ...page, + error: Array.isArray(page?.children) ? undefined : 'Unable to load items. Check the R session and try again.', + }); } }); panel.onDidDispose(() => { @@ -1089,7 +1101,7 @@ export async function showDataView(source: string, type: string, title: string, }); }); } - panel.webview.html = await getListHtml(panel.webview, file, title); + panel.webview.html = getListHtml(panel.webview, title); } else { await commands.executeCommand('vscode.open', Uri.file(file), { preserveFocus: true, @@ -1792,8 +1804,7 @@ export async function getTableHtml(webview: Webview, file: string | undefined, t `; } -export async function getListHtml(webview: Webview, file: string, title: string): Promise { - const content = (await fs.readFile(file, 'utf8')).replace(/
+ +
From 82efd24c2237bc4a7ea136cac99c723ac6323a6e Mon Sep 17 00:00:00 2001 From: "4111978+Fred-Wu@users.noreply.github.com" <4111978+Fred-Wu@users.noreply.github.com> Date: Sat, 3 Oct 2026 18:45:35 +1000 Subject: [PATCH 07/30] Improve List Viewer - add expandable controls for nested objects in List Viewer - associate nested List / Data Viewers with their top level object, so only one viewer is used for each viewer type - add navigation controls to go back to the previous view or directly to a parent view. --- README.md | 4 + images/icons/arrow-left.svg | 3 + images/icons/chevron-down.svg | 3 + images/icons/chevron-right.svg | 3 + sess/R/handlers.R | 198 ++++++++++++++++++---- sess/R/hooks.R | 66 ++++---- sess/R/server.R | 3 +- sess/inst/tinytest/test-ipc.R | 6 +- sess/inst/tinytest/test-listview.R | 172 +++++++++++++++++++ src/interactive/manager.ts | 2 +- src/listViewer.ts | 176 ++++++++++++++++++++ src/session.ts | 182 +++++++++++--------- src/test/suite/listViewer.test.ts | 213 ++++++++++++++++++++++++ src/test/suite/listViewerPanels.test.ts | 79 +++++++++ 14 files changed, 961 insertions(+), 149 deletions(-) create mode 100644 images/icons/arrow-left.svg create mode 100644 images/icons/chevron-down.svg create mode 100644 images/icons/chevron-right.svg create mode 100644 sess/inst/tinytest/test-listview.R create mode 100644 src/listViewer.ts create mode 100644 src/test/suite/listViewer.test.ts create mode 100644 src/test/suite/listViewerPanels.test.ts diff --git a/README.md b/README.md index a226fd305..5f2c13bf1 100644 --- a/README.md +++ b/README.md @@ -138,3 +138,7 @@ Call `View(x)` to open a table. The viewer loads rows on demand and applies colu ## Persistent R Interactive This experimental feature provides native VS Code Interactive windows with independent plain-R or arf sessions, including adoption of existing arf sessions over Remote SSH. See the [setup and usage guide](https://github.com/REditorSupport/vscode-R/wiki/R-Interactive) for session switching, persistence, rich outputs, and platform requirements. + +## Icon attribution + +The list viewer’s navigation and chevron icons are from Microsoft’s [VS Code Codicons](https://github.com/microsoft/vscode-codicons), licensed under [CC BY 4.0](https://creativecommons.org/licenses/by/4.0/). The icon artwork is unchanged. diff --git a/images/icons/arrow-left.svg b/images/icons/arrow-left.svg new file mode 100644 index 000000000..91773a7ec --- /dev/null +++ b/images/icons/arrow-left.svg @@ -0,0 +1,3 @@ + + \ No newline at end of file diff --git a/images/icons/chevron-down.svg b/images/icons/chevron-down.svg new file mode 100644 index 000000000..389873c46 --- /dev/null +++ b/images/icons/chevron-down.svg @@ -0,0 +1,3 @@ + + \ No newline at end of file diff --git a/images/icons/chevron-right.svg b/images/icons/chevron-right.svg new file mode 100644 index 000000000..d08afa0b5 --- /dev/null +++ b/images/icons/chevron-right.svg @@ -0,0 +1,3 @@ + + \ No newline at end of file diff --git a/sess/R/handlers.R b/sess/R/handlers.R index f810dbc19..eb1a68755 100644 --- a/sess/R/handlers.R +++ b/sess/R/handlers.R @@ -133,16 +133,163 @@ workspace_child_label <- function(name, index) { } } -handle_workspace_view <- function(name, path = list()) { - title <- paste0(c(name, vapply(path, function(selector) switch(selector$kind, - index = if (!is.null(selector$name) && !is.na(selector$name) && nzchar(selector$name)) paste0("$", selector$name) else paste0("[[", selector$value, "]]"), - name = paste0("$", selector$value), - slot = paste0("@", selector$value) - ), "")), collapse = "") - utils::View(workspace_object(name, path), title = title) +# 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()) { + suffix <- vapply(path, function(selector) { + switch(selector$kind, + index = { + if (!is.null(selector$name) && !is.na(selector$name) && nzchar(selector$name)) { + paste0("$", selector$name) + } else { + paste0("[[", selector$value, "]]") + } + }, + name = paste0("$", selector$value), + slot = paste0("@", selector$value) + ) + }, "") + title <- paste0(c(name, suffix), collapse = "") + root <- listview_state(workspace_object(name), name, name) + context <- listview_context(root, path) + workspace_show_view(workspace_object(name, path), title, name, context) +} + +listview_is_list <- function(object) { + !dataview_is_table(object) && + (is.list(object) || is.pairlist(object) || is.environment(object) || isS4(object)) +} + +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) { + path <- list() + for (selector in selectors) { + location <- listview_location(root, path) + index <- if (selector$kind == "index" && is.numeric(selector$value)) { + selector$value + } else { + match(selector$value, location$names) + } + path <- c(path, list(index)) + } + location <- listview_location(root, path) + list(root = root, navigation = list( + title = location$title, path = I(path), breadcrumbs = I(location$breadcrumbs) + )) +} + +# Direct View(x$a) calls can also start with a complete breadcrumb path. +listview_expression_context <- function(expression, envir, owner) { + tryCatch({ + selectors <- list() + while (is.call(expression) && length(expression) == 3L && + is.symbol(expression[[1L]])) { + operator <- as.character(expression[[1L]]) + if (!operator %in% c("$", "[[", "@")) return(NULL) + value <- expression[[3L]] + if (operator %in% c("$", "@") && is.symbol(value)) value <- as.character(value) + if (length(value) != 1L || !(is.character(value) || is.numeric(value))) return(NULL) + selectors <- c(list(list(kind = "index", value = value)), selectors) + expression <- expression[[2L]] + } + if (!is.symbol(expression)) return(NULL) + name <- as.character(expression) + root <- listview_state(get(name, envir = envir, inherits = TRUE), name, owner) + listview_context(root, selectors) + }, error = function(e) NULL) +} + +listview_navigation <- function(state, path = list()) { + location <- listview_location(state, path) + if (!listview_is_list(location$data) && length(path)) stop("Not a list view") + list(title = location$title, path = I(path), breadcrumbs = I(location$breadcrumbs)) +} + +handle_listview_navigate <- function(view_id, path = list()) { + state <- dataview_get_state(view_id) + if (!identical(state$type, "list")) stop("Not a list view") + listview_navigation(state, path) +} + +listview_location <- function(state, path = list()) { + visited <- integer() + state$breadcrumbs <- list(list(label = state$title, path = I(list()))) + for (index in path) { + 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" + state$names <- switch(state$kind, + name = workspace_env_names(child), + slot = methods::slotNames(child), + index = names(child) + ) + visited <- c(visited, index) + 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))))) + # Environment bindings can disappear while their rows remain visible. + if (state$kind == "name" && is.environment(state$child_names)) { + key <- paste(visited, collapse = "/") + if (exists(key, envir = state$child_names, inherits = FALSE)) { + state$names <- get(key, envir = state$child_names, inherits = FALSE) + } else { + assign(key, state$names, envir = state$child_names) + } + } + } + state +} + get_workspace_children <- function(name = NULL, path = list(), start = 1L, view_id = NULL) { tryCatch({ if (is.null(view_id)) { @@ -156,6 +303,7 @@ get_workspace_children <- function(name = NULL, path = list(), start = 1L, view_ } 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 kind <- state$kind child_names <- state$names @@ -198,7 +346,10 @@ get_workspace_children <- function(name = NULL, path = list(), start = 1L, view_ ) summary <- trimws(try_capture_str(child)) if (!is.null(view_id)) { - list(label = label, str = summary, viewable = TRUE, index = index) + list( + label = label, str = summary, viewable = TRUE, index = index, + has_children = workspace_child_count(child) > 0L + ) } else { selector <- if (kind == "index") { list(kind = kind, value = index, name = child_name) @@ -215,36 +366,17 @@ get_workspace_children <- function(name = NULL, path = list(), start = 1L, view_ }) } -handle_listview_view <- function(view_id, index) { +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) } - index <- as.integer(index) - child_count <- if (state$kind == "index") length(state$data) else length(state$names) - if (length(index) != 1L || is.na(index) || index < 1L || index > child_count) { - return(FALSE) + location <- tryCatch(listview_location(state, c(path, list(index))), error = function(e) NULL) + if (is.null(location)) return(FALSE) + if (listview_is_list(location$data)) { + return(listview_navigation(state, c(path, list(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))) { - return(FALSE) - } - child <- switch(state$kind, - name = get(child_name, envir = state$data, inherits = FALSE), - slot = methods::slot(state$data, child_name), - index = state$data[[index]] - ) - 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, "]]") - } - utils::View(child, title = title) - TRUE + workspace_show_view(location$data, location$title, state$owner %||% state$title) } handle_hover <- function(expr_str) { diff --git a/sess/R/hooks.R b/sess/R/hooks.R index ca395fd4a..d0f1e36ca 100644 --- a/sess/R/hooks.R +++ b/sess/R/hooks.R @@ -103,7 +103,17 @@ 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)) { @@ -112,25 +122,24 @@ runtime_start <- function(use_rstudioapi = TRUE, view_type <- if (dataview_is_table(x)) { "table" - } else if (is.list(x) || is.environment(x) || isS4(x)) { + } else if (is.list(x) || is.pairlist(x) || is.environment(x) || isS4(x)) { "list" } else { "object" } - if (view_type != "object") { - title_key <- paste(as.character(title), collapse = "\n") - registry_key <- paste0(view_type, ":", title_key) - 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 + 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") { @@ -143,29 +152,26 @@ runtime_start <- function(use_rstudioapi = TRUE, view_id = registration$view_id )) } else if (view_type == "list") { - child_kind <- if (is.environment(x)) "name" else if (isS4(x)) "slot" else "index" - x_names <- switch(child_kind, - name = workspace_env_names(x), - slot = methods::slotNames(x), - index = names(x) - ) - .sess_env$dataviews[[view_id]] <- list( - type = "list", - data = x, - title = title_key, - kind = child_kind, - names = x_names - ) + context <- .sess_env$listview_context + if (is.null(context) && missing(title)) { + context <- listview_expression_context(original_expression, parent.frame(), owner) + } + root <- if (is.null(context)) listview_state(x, title_key, owner) else context$root + navigation <- if (is.null(context)) listview_navigation(root) else context$navigation + .sess_env$dataviews[[view_id]] <- root notify_client("dataview", list( title = title, source = "list", type = "json", - view_id = view_id + view_id = view_id, + 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 f97bec091..6fb92ca9d 100644 --- a/sess/R/server.R +++ b/sess/R/server.R @@ -455,7 +455,8 @@ dispatch_message <- function(line) { "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_view" = function(p) handle_listview_view(p$view_id, p$index), + "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/inst/tinytest/test-ipc.R b/sess/inst/tinytest/test-ipc.R index ef862e299..8ed00b95d 100644 --- a/sess/inst/tinytest/test-ipc.R +++ b/sess/inst/tinytest/test-ipc.R @@ -264,10 +264,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 +284,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.R b/sess/inst/tinytest/test-listview.R new file mode 100644 index 000000000..867d5b0a2 --- /dev/null +++ b/sess/inst/tinytest/test-listview.R @@ -0,0 +1,172 @@ +# 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") + 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 = c(root, other), envir = .GlobalEnv) + lapply(pipe, close) + }, add = TRUE) + runtime$con <- pipe[[2L]] + sess:::runtime_start(use_rstudioapi = FALSE, use_httpgd = FALSE, use_jgd = FALSE) + 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. + 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 + ))) + messages <- strsplit(processx::conn_read_chars(pipe[[1L]]), "\n", fixed = TRUE)[[1L]] + responses <- lapply(messages[nzchar(messages)], jsonlite::fromJSON, simplifyVector = FALSE) + 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)) + + # 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") + expect_equal(direct_context$navigation$path, list(1L, 1L), check.attributes = FALSE) + expect_equal(direct_context$navigation$title, "x$a$b") + utils::View(x$df) + direct_table <- id("table", "x") + utils::View(x$a$df) + expect_identical(id("table", "x"), direct_table) + utils::View(x$a$b$value) + text_id <- id("object", "x") + text_file <- runtime$dataviews[[text_id]]$file + utils::View(x$a$df$nested) + expect_identical(id("object", "x"), text_id) + expect_identical(runtime$dataviews[[text_id]]$file, text_file) + expect_equal(readLines(text_file), "2L") + unlink(text_file) + + # 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))) + 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") +}) diff --git a/src/interactive/manager.ts b/src/interactive/manager.ts index 3b9112265..199beddca 100644 --- a/src/interactive/manager.ts +++ b/src/interactive/manager.ts @@ -1184,7 +1184,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); 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 000000000..898f2377f --- /dev/null +++ b/src/listViewer.ts @@ -0,0 +1,176 @@ +export interface ListViewNavigation { + title: string; + path: number[]; + breadcrumbs: { label: string; path: number[] }[]; +} + +/** Script shared by the list webview and its interaction tests. */ +export function getListViewerScript(generation: number, initial: ListViewNavigation = { + title: '', path: [], breadcrumbs: [{ label: '', path: [] }], +}): string { + return ` + const vscode = acquireVsCodeApi(); + const generation = ${generation}; + 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; + let page = pages.get(key); + if (!page) { + const element = document.createElement('div'); + page = { element, scrollTop: 0, loader: createPage(element, navigation.path) }; + 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 = 'breadcrumb-separator'; + 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, generation, requestId, ...params }); + } + + back.addEventListener('click', () => { + if (history.length) navigate('listview/navigate', { path: history[history.length - 1].path }, true); + }); + + function createPage(container, path) { + 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'; + const arrow = document.createElement('span'); + arrow.className = '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); + button.innerHTML = document.getElementById('view-icon').innerHTML; + 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); + const page = createPage(children, [...path, item.index]); + entry.addEventListener('toggle', () => { + if (entry.open) 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', generation, 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.generation !== generation) return; + const receive = pending.get(message.requestId); + if (receive) { + pending.delete(message.requestId); + receive(message); + } + }); + showNavigation(${JSON.stringify(initial).replace(/ { +export async function showDataView(source: string, type: string, title: string, file: string, viewer: string, viewId?: string, navigation?: ListViewNavigation, owner = activeSession): Promise { resDir ??= path.join(extensionContext.extensionPath, 'dist', 'resources'); console.info(`[showDataView] source: ${source}, type: ${type}, title: ${title}, file: ${file}, viewer: ${viewer}, viewId: ${String(viewId ?? '')}`); @@ -1021,7 +1020,7 @@ export async function showDataView(source: string, type: string, title: string, const existing = dynamicDataViewPanels.get(`${owner?.sessionId ?? ''}:${viewId}`); if (existing) { existing.title = title; - existing.reveal(ViewColumn[viewer as keyof typeof ViewColumn], true); + existing.reveal(existing.viewColumn, true); const content = await getTableHtml(existing.webview, undefined, title); existing.webview.html = `${content}\n`; return; @@ -1051,8 +1050,8 @@ export async function showDataView(source: string, type: string, title: string, const existing = dynamicDataViewPanels.get(viewId); if (existing) { existing.title = title; - existing.reveal(ViewColumn[viewer as keyof typeof ViewColumn], true); - existing.webview.html = getListHtml(existing.webview, title); + existing.reveal(existing.viewColumn, true); + existing.webview.html = getListHtml(existing.webview, title, navigation); return; } } @@ -1071,20 +1070,40 @@ export async function showDataView(source: string, type: string, title: string, panel.iconPath = new UriIcon('open-preview'); if (viewId) { dynamicDataViewPanels.set(viewId, panel); - panel.webview.onDidReceiveMessage(async (message: { message?: string; index?: number; start?: number; requestId?: number }) => { - if (message.message === 'listview/view' && typeof message.index === 'number' && Number.isInteger(message.index)) { - void sessionRequest({ - method: 'listview_view', - params: { view_id: viewId, index: message.index }, + panel.webview.onDidReceiveMessage(async (message: { + message?: string; index?: number; start?: number; requestId?: number; + path?: number[]; generation?: number; + }) => { + 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 }, + }) as ListViewNavigation | boolean | undefined; + const navigation = result && typeof result === 'object' && Array.isArray(result.breadcrumbs) + ? result : undefined; + if (navigation) { + panel.title = navigation.title; + } + void panel.webview.postMessage({ + message: 'listview/navigation', + generation: message.generation, + 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 }, + params: { view_id: viewId, start: message.start, path: message.path }, }) as { children?: unknown; next_start?: number | null } | undefined; void panel.webview.postMessage({ message: 'listview/page', + generation: message.generation, requestId: message.requestId, ...page, error: Array.isArray(page?.children) ? undefined : 'Unable to load items. Check the R session and try again.', @@ -1101,7 +1120,7 @@ export async function showDataView(source: string, type: string, title: string, }); }); } - panel.webview.html = getListHtml(panel.webview, title); + panel.webview.html = getListHtml(panel.webview, title, navigation); } else { await commands.executeCommand('vscode.open', Uri.file(file), { preserveFocus: true, @@ -1804,10 +1823,19 @@ export async function getTableHtml(webview: Webview, file: string | undefined, t `; } -export function getListHtml(webview: Webview, title: string): string { +export function getListHtml(webview: Webview, title: string, navigation?: ListViewNavigation): string { const icon = new UriIcon('open-preview-codicon'); const darkIcon = webview.asWebviewUri(icon.dark).toString(); const lightIcon = webview.asWebviewUri(icon.light).toString(); + const chevronIcon = webview.asWebviewUri( + Uri.file(extensionContext.asAbsolutePath('images/icons/chevron-right.svg')) + ).toString(); + const backIcon = webview.asWebviewUri( + Uri.file(extensionContext.asAbsolutePath('images/icons/arrow-left.svg')) + ).toString(); + const expandedChevronIcon = webview.asWebviewUri( + Uri.file(extensionContext.asAbsolutePath('images/icons/chevron-down.svg')) + ).toString(); return ` @@ -1819,11 +1847,47 @@ export function getListHtml(webview: Webview, title: string): string { + +
- -
+ @@ -2192,6 +2211,7 @@ async function handleNotification(message: Record, socket: IpcS String(params.file ?? ''), viewer, params.view_id ? String(params.view_id) : undefined, + params.navigation as ListViewNavigation | undefined, ); } } diff --git a/src/test/suite/listViewer.test.ts b/src/test/suite/listViewer.test.ts new file mode 100644 index 000000000..93af77731 --- /dev/null +++ b/src/test/suite/listViewer.test.ts @@ -0,0 +1,213 @@ +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; + generation: 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 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: { + 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 }; +} + +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', 'breadcrumb-separator', 'breadcrumb', 'breadcrumb-separator', '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(messages.length, 1); + entry.open = true; + entry.fire('toggle'); + 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('ignores old viewer responses and hides open buttons for unavailable items', () => { + const { root, messages, reply } = createViewer(); + reply(messages[0], { generation: 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 000000000..868e35578 --- /dev/null +++ b/src/test/suite/listViewerPanels.test.ts @@ -0,0 +1,79 @@ +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(); + }); + + 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'); + 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'); + assert.strictEqual(panels[0].title, 'x$a$b'); + await session.showDataView('list', 'json', 'y', '', 'Two', 'test-list-y'); + assert.strictEqual(create.callCount, 3); + 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('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 }], 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', [], false); + assert.notStrictEqual(unavailable.contextValue, 'viewableNode'); + }); +}); From c242a633d160ca0c68448fd121f7f5b773be8402 Mon Sep 17 00:00:00 2001 From: Fred-Wu <4111978+Fred-Wu@users.noreply.github.com> Date: Sat, 3 Oct 2026 20:23:08 +1000 Subject: [PATCH 08/30] Fix nested viewer routing and icons --- sess/R/handlers.R | 8 ++++++++ sess/inst/tinytest/test-listview.R | 21 ++++++++++++++++++++- src/session.ts | 2 +- src/test/suite/listViewerPanels.test.ts | 6 ++++++ 4 files changed, 35 insertions(+), 2 deletions(-) diff --git a/sess/R/handlers.R b/sess/R/handlers.R index eb1a68755..9fb4f9935 100644 --- a/sess/R/handlers.R +++ b/sess/R/handlers.R @@ -233,6 +233,14 @@ listview_navigation <- function(state, path = list()) { 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 + )) + } listview_navigation(state, path) } diff --git a/sess/inst/tinytest/test-listview.R b/sess/inst/tinytest/test-listview.R index 867d5b0a2..78ada573b 100644 --- a/sess/inst/tinytest/test-listview.R +++ b/sess/inst/tinytest/test-listview.R @@ -6,13 +6,14 @@ local({ 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 = c(root, other), envir = .GlobalEnv) + rm(list = c(root, other, table_root), envir = .GlobalEnv) lapply(pipe, close) }, add = TRUE) runtime$con <- pipe[[2L]] @@ -89,6 +90,24 @@ local({ 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(2L, "nested")) + ))) + table_list_id <- id("list", table_root) + 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") diff --git a/src/session.ts b/src/session.ts index e1ed9e878..9625b32f4 100644 --- a/src/session.ts +++ b/src/session.ts @@ -1067,7 +1067,7 @@ export async function showDataView(source: string, type: string, title: string, retainContextWhenHidden: true, localResourceRoots: [Uri.file(extensionContext.asAbsolutePath('images/icons'))], }); - panel.iconPath = new UriIcon('open-preview'); + panel.iconPath = new UriIcon('preview'); if (viewId) { dynamicDataViewPanels.set(viewId, panel); panel.webview.onDidReceiveMessage(async (message: { diff --git a/src/test/suite/listViewerPanels.test.ts b/src/test/suite/listViewerPanels.test.ts index 868e35578..fa23aeee8 100644 --- a/src/test/suite/listViewerPanels.test.ts +++ b/src/test/suite/listViewerPanels.test.ts @@ -47,12 +47,18 @@ suite('List viewer panels', () => { 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); From 3d0dee0ab7a44be662690a337d2ac36a35dcf062 Mon Sep 17 00:00:00 2001 From: Fred-Wu <4111978+Fred-Wu@users.noreply.github.com> Date: Sat, 3 Oct 2026 20:48:00 +1000 Subject: [PATCH 09/30] Page vector values in viewer --- sess/R/handlers.R | 42 ++++++++++++++++++++----- sess/R/hooks.R | 7 +++-- sess/inst/tinytest/test-listview.R | 31 ++++++++++++++++++ src/listViewer.ts | 13 +++++--- src/session.ts | 29 ++++++++++++----- src/test/suite/listViewer.test.ts | 19 +++++++++-- src/test/suite/listViewerPanels.test.ts | 4 ++- 7 files changed, 121 insertions(+), 24 deletions(-) diff --git a/sess/R/handlers.R b/sess/R/handlers.R index 9fb4f9935..0eb2dca94 100644 --- a/sess/R/handlers.R +++ b/sess/R/handlers.R @@ -232,7 +232,7 @@ listview_navigation <- function(state, path = list()) { handle_listview_navigate <- function(view_id, path = list()) { state <- dataview_get_state(view_id) - if (!identical(state$type, "list")) stop("Not a list view") + if (!state$type %in% c("list", "vector")) stop("Not a list view") location <- listview_location(state, path) if (dataview_is_table(location$data)) { return(workspace_show_view( @@ -241,6 +241,15 @@ handle_listview_navigate <- function(view_id, path = list()) { state$owner %||% state$title )) } + if (identical(state$type, "vector") && listview_is_list(location$data)) { + navigation <- listview_navigation(state, path) + return(workspace_show_view( + location$data, + location$title, + state$owner %||% state$title, + list(root = state, navigation = navigation) + )) + } listview_navigation(state, path) } @@ -310,13 +319,22 @@ get_workspace_children <- function(name = NULL, path = list(), start = 1L, view_ ) } else { state <- dataview_get_state(view_id) - if (!identical(state$type, "list")) stop("Not a list view") + if (!state$type %in% c("list", "vector")) stop("Not a list view") + vector_view <- identical(state$type, "vector") state <- listview_location(state, path) object <- state$data kind <- state$kind child_names <- state$names } - child_count <- if (kind == "index") workspace_child_count(object) else length(child_names) + child_count <- if (kind == "index") { + if (!is.null(view_id) && isTRUE(vector_view)) { + 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) { @@ -325,7 +343,9 @@ get_workspace_children <- function(name = NULL, path = list(), start = 1L, view_ children <- lapply(seq.int(start, end), function(index) { child_name <- if (is.null(child_names)) NULL else child_names[[index]] - label <- if (kind == "slot") { + label <- if (!is.null(view_id) && isTRUE(vector_view)) { + paste0("[", index, "]") + } else if (kind == "slot") { paste0("@ ", child_name) } else { workspace_child_label(child_name, index) @@ -352,11 +372,19 @@ get_workspace_children <- function(name = NULL, path = list(), start = 1L, view_ slot = methods::slot(object, child_name), index = object[[index]] ) - summary <- trimws(try_capture_str(child)) + summary <- if (!is.null(view_id) && isTRUE(vector_view)) { + if (is.character(child)) { + encodeString(child, quote = "\"", na.encode = TRUE) + } else { + paste(format(child, trim = TRUE), collapse = " ") + } + } else { + trimws(try_capture_str(child)) + } if (!is.null(view_id)) { list( - label = label, str = summary, viewable = TRUE, index = index, - has_children = workspace_child_count(child) > 0L + label = label, str = summary, viewable = !isTRUE(vector_view), index = index, + has_children = !isTRUE(vector_view) && workspace_child_count(child) > 0L ) } else { selector <- if (kind == "index") { diff --git a/sess/R/hooks.R b/sess/R/hooks.R index d0f1e36ca..699321634 100644 --- a/sess/R/hooks.R +++ b/sess/R/hooks.R @@ -124,6 +124,8 @@ runtime_start <- function(use_rstudioapi = TRUE, "table" } else if (is.list(x) || is.pairlist(x) || is.environment(x) || isS4(x)) { "list" + } else if (is.atomic(x) && length(x) > 1L) { + "vector" } else { "object" } @@ -151,17 +153,18 @@ runtime_start <- function(use_rstudioapi = TRUE, type = "json", view_id = registration$view_id )) - } else if (view_type == "list") { + } else if (view_type %in% c("list", "vector")) { context <- .sess_env$listview_context if (is.null(context) && missing(title)) { context <- listview_expression_context(original_expression, parent.frame(), owner) } root <- if (is.null(context)) listview_state(x, title_key, owner) else context$root + root$type <- view_type navigation <- if (is.null(context)) listview_navigation(root) else context$navigation .sess_env$dataviews[[view_id]] <- root notify_client("dataview", list( title = title, - source = "list", + source = view_type, type = "json", view_id = view_id, navigation = navigation diff --git a/sess/inst/tinytest/test-listview.R b/sess/inst/tinytest/test-listview.R index 78ada573b..4a82dc864 100644 --- a/sess/inst/tinytest/test-listview.R +++ b/sess/inst/tinytest/test-listview.R @@ -96,6 +96,21 @@ local({ 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("vector", table_root) + 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")) @@ -123,6 +138,22 @@ local({ 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("vector", "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") + utils::View(x$a$b$value) text_id <- id("object", "x") text_file <- runtime$dataviews[[text_id]]$file diff --git a/src/listViewer.ts b/src/listViewer.ts index 898f2377f..addff67d3 100644 --- a/src/listViewer.ts +++ b/src/listViewer.ts @@ -7,10 +7,11 @@ export interface ListViewNavigation { /** Script shared by the list webview and its interaction tests. */ export function getListViewerScript(generation: number, initial: ListViewNavigation = { title: '', path: [], breadcrumbs: [{ label: '', path: [] }], -}): string { +}, vector = false): string { return ` const vscode = acquireVsCodeApi(); const generation = ${generation}; + const vector = ${vector}; const pending = new Map(); let nextRequestId = 0; const list = document.getElementById('list'); @@ -114,10 +115,12 @@ export function getListViewerScript(generation: number, initial: ListViewNavigat const entry = document.createElement(expandable ? 'details' : 'div'); const row = document.createElement(expandable ? 'summary' : 'div'); row.className = 'item'; - const arrow = document.createElement('span'); - arrow.className = 'arrow'; - arrow.setAttribute('aria-hidden', 'true'); - row.appendChild(arrow); + if (!vector) { + const arrow = document.createElement('span'); + arrow.className = 'arrow'; + arrow.setAttribute('aria-hidden', 'true'); + row.appendChild(arrow); + } const label = document.createElement('span'); label.className = 'label'; label.textContent = item.label; diff --git a/src/session.ts b/src/session.ts index 9625b32f4..7b793c7c7 100644 --- a/src/session.ts +++ b/src/session.ts @@ -1045,13 +1045,15 @@ export async function showDataView(source: string, type: string, title: string, } const content = await getTableHtml(panel.webview, file || undefined, title); panel.webview.html = content; - } else if (source === 'list') { + } else if (source === 'list' || source === 'vector') { if (viewId) { const existing = dynamicDataViewPanels.get(viewId); if (existing) { existing.title = title; existing.reveal(existing.viewColumn, true); - existing.webview.html = getListHtml(existing.webview, title, navigation); + existing.webview.html = getListHtml( + existing.webview, title, navigation, source === 'vector' + ); return; } } @@ -1120,7 +1122,9 @@ export async function showDataView(source: string, type: string, title: string, }); }); } - panel.webview.html = getListHtml(panel.webview, title, navigation); + panel.webview.html = getListHtml( + panel.webview, title, navigation, source === 'vector' + ); } else { await commands.executeCommand('vscode.open', Uri.file(file), { preserveFocus: true, @@ -1823,7 +1827,12 @@ export async function getTableHtml(webview: Webview, file: string | undefined, t `; } -export function getListHtml(webview: Webview, title: string, navigation?: ListViewNavigation): string { +export function getListHtml( + webview: Webview, + title: string, + navigation?: ListViewNavigation, + vector = false +): string { const icon = new UriIcon('open-preview-codicon'); const darkIcon = webview.asWebviewUri(icon.dark).toString(); const lightIcon = webview.asWebviewUri(icon.light).toString(); @@ -1914,6 +1923,12 @@ export function getListHtml(webview: Webview, title: string, navigation?: ListVi color: var(--vscode-symbolIcon-fieldForeground); white-space: nowrap; } + body.vector .label { + min-width: 64px; + } + body.vector .item { + gap: 8px; + } .str { flex: 1; color: var(--vscode-descriptionForeground); @@ -1953,10 +1968,10 @@ export function getListHtml(webview: Webview, title: string, navigation?: ListVi } - +
@@ -1964,7 +1979,7 @@ export function getListHtml(webview: Webview, title: string, navigation?: ListVi diff --git a/src/test/suite/listViewer.test.ts b/src/test/suite/listViewer.test.ts index 93af77731..5bf6d47e8 100644 --- a/src/test/suite/listViewer.test.ts +++ b/src/test/suite/listViewer.test.ts @@ -35,7 +35,7 @@ interface Request { function createViewer(initial: ListViewNavigation = { title: 'x', path: [], breadcrumbs: [{ label: 'x', path: [] }], -}) { +}, vector = false) { const root = new Element('div'); const back = new Element('button'); const breadcrumbs = new Element('nav'); @@ -43,7 +43,7 @@ function createViewer(initial: ListViewNavigation = { const elements: Record = { list: root, back, breadcrumbs, 'navigation-status': status }; const messages: Request[] = []; let receive: (event: unknown) => void = () => undefined; - vm.runInNewContext(getListViewerScript(7, initial), { + vm.runInNewContext(getListViewerScript(7, initial, vector), { acquireVsCodeApi: () => ({ postMessage: (message: Request) => { messages.push(JSON.parse(JSON.stringify(message)) as Request); } }), @@ -197,6 +197,21 @@ suite('List viewer', () => { assert.strictEqual(secondPage.children[1].hidden, true); }); + test('renders vector values as simple indexed rows', () => { + const viewer = createViewer(undefined, true); + viewer.reply(viewer.messages[0], { + children: [{ + label: '[1]', str: '12.4', index: 1, + viewable: false, has_children: false, + }], + }); + const row = viewer.root.children[0].children[0].children[0]; + assert.deepStrictEqual(row.children.map(child => child.className), ['label', 'str']); + assert.strictEqual(row.children[0].textContent, '[1]'); + assert.strictEqual(row.children[1].textContent, '12.4'); + assert.ok(!row.children.some(child => child.tag === 'button')); + }); + test('ignores old viewer responses and hides open buttons for unavailable items', () => { const { root, messages, reply } = createViewer(); reply(messages[0], { generation: 6, children: [{ label: 'stale' }] }); diff --git a/src/test/suite/listViewerPanels.test.ts b/src/test/suite/listViewerPanels.test.ts index fa23aeee8..418f009c6 100644 --- a/src/test/suite/listViewerPanels.test.ts +++ b/src/test/suite/listViewerPanels.test.ts @@ -62,11 +62,13 @@ suite('List viewer panels', () => { 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('vector', 'json', 'x$id', '', 'Two', 'test-vector-x'); + assert.strictEqual(create.callCount, 4); const first = panels.shift(); assert.ok(first); first.dispose(); await session.showDataView('list', 'json', 'x', '', 'Two', 'test-list-x'); - assert.strictEqual(create.callCount, 4); + assert.strictEqual(create.callCount, 5); }); test('supported workspace children retain open actions alongside expansion', () => { From 4aee993b4cb8795b3135758fc788489b57c3e018 Mon Sep 17 00:00:00 2001 From: "4111978+Fred-Wu@users.noreply.github.com" <4111978+Fred-Wu@users.noreply.github.com> Date: Sat, 3 Oct 2026 21:21:46 +1000 Subject: [PATCH 10/30] Fix lint error --- src/listViewer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/listViewer.ts b/src/listViewer.ts index addff67d3..7f0be625a 100644 --- a/src/listViewer.ts +++ b/src/listViewer.ts @@ -11,7 +11,7 @@ export function getListViewerScript(generation: number, initial: ListViewNavigat return ` const vscode = acquireVsCodeApi(); const generation = ${generation}; - const vector = ${vector}; + const vector = ${String(vector)}; const pending = new Map(); let nextRequestId = 0; const list = document.getElementById('list'); From 98511f4a13c5d58c7fc487c65448510365d919cd Mon Sep 17 00:00:00 2001 From: "4111978+Fred-Wu@users.noreply.github.com" <4111978+Fred-Wu@users.noreply.github.com> Date: Sun, 4 Oct 2026 12:19:52 +1100 Subject: [PATCH 11/30] fix(viewer): unify list and vector navigation - Reuse one list viewer panel and identity per root object - Preserve breadcrumbs, Back history, and cached pages for vectors - Render POSIXlt values without recursive expansion - Ignore stale requests and replies after panel reuse - Resolve workspace paths once and reuse environment-name snapshots - Create child page controls only when expanded - Remove unused jQuery and JSON viewer dependencies and assets - Add regression tests for navigation, reuse, and lazy loading --- esbuild.js | 3 - package.json | 2 - pnpm-lock.yaml | 16 --- sess/R/handlers.R | 127 ++++++++++------------ sess/R/hooks.R | 13 ++- sess/inst/tinytest/test-listview.R | 139 +++++++++++++++++++++++- src/listViewer.ts | 17 ++- src/session.ts | 35 ++++-- src/test/suite/listViewer.test.ts | 70 +++++++++++- src/test/suite/listViewerPanels.test.ts | 48 +++++++- 10 files changed, 346 insertions(+), 124 deletions(-) diff --git a/esbuild.js b/esbuild.js index 1ab93a298..29df03330 100644 --- a/esbuild.js +++ b/esbuild.js @@ -10,9 +10,6 @@ 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' diff --git a/package.json b/package.json index b247feced..b7ea83415 100644 --- a/package.json +++ b/package.json @@ -2573,8 +2573,6 @@ "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 e6a235f9d..a9078eafa 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -29,12 +29,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 @@ -1728,12 +1722,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==} @@ -4327,10 +4315,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@3.15.2: diff --git a/sess/R/handlers.R b/sess/R/handlers.R index 0eb2dca94..e506d788b 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)) @@ -148,28 +151,21 @@ workspace_show_view <- function(object, title, owner, context = NULL) { } handle_workspace_view <- function(name, path = list()) { - suffix <- vapply(path, function(selector) { - switch(selector$kind, - index = { - if (!is.null(selector$name) && !is.na(selector$name) && nzchar(selector$name)) { - paste0("$", selector$name) - } else { - paste0("[[", selector$value, "]]") - } - }, - name = paste0("$", selector$value), - slot = paste0("@", selector$value) - ) - }, "") - title <- paste0(c(name, suffix), collapse = "") root <- listview_state(workspace_object(name), name, name) context <- listview_context(root, path) - workspace_show_view(workspace_object(name, path), title, name, context) + 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) || + (is.atomic(object) && length(object) > 1L)) } -listview_is_list <- function(object) { +listview_is_vector <- function(object) { !dataview_is_table(object) && - (is.list(object) || is.pairlist(object) || is.environment(object) || isS4(object)) + (inherits(object, "POSIXlt") || (!isS4(object) && is.atomic(object) && length(object) > 1L)) } listview_state <- function(object, title, owner) { @@ -187,20 +183,8 @@ listview_state <- function(object, title, owner) { # Resolve workspace selectors once; navigation paths are relative to the retained root. listview_context <- function(root, selectors) { - path <- list() - for (selector in selectors) { - location <- listview_location(root, path) - index <- if (selector$kind == "index" && is.numeric(selector$value)) { - selector$value - } else { - match(selector$value, location$names) - } - path <- c(path, list(index)) - } - location <- listview_location(root, path) - list(root = root, navigation = list( - title = location$title, path = I(path), breadcrumbs = I(location$breadcrumbs) - )) + location <- listview_location(root, selectors, resolve_selectors = TRUE) + list(root = root, data = location$data, navigation = listview_navigation(location)) } # Direct View(x$a) calls can also start with a complete breadcrumb path. @@ -224,15 +208,16 @@ listview_expression_context <- function(expression, envir, owner) { }, error = function(e) NULL) } -listview_navigation <- function(state, path = list()) { - location <- listview_location(state, path) - if (!listview_is_list(location$data) && length(path)) stop("Not a list view") - list(title = location$title, path = I(path), breadcrumbs = I(location$breadcrumbs)) +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 (!state$type %in% c("list", "vector")) stop("Not a list view") + 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( @@ -241,22 +226,21 @@ handle_listview_navigate <- function(view_id, path = list()) { state$owner %||% state$title )) } - if (identical(state$type, "vector") && listview_is_list(location$data)) { - navigation <- listview_navigation(state, path) - return(workspace_show_view( - location$data, - location$title, - state$owner %||% state$title, - list(root = state, navigation = navigation) - )) - } - listview_navigation(state, path) + if (!listview_supported(location$data)) stop("Not a list view") + listview_navigation(location) } -listview_location <- function(state, path = list()) { +listview_location <- function(state, path = list(), resolve_selectors = FALSE) { visited <- integer() state$breadcrumbs <- list(list(label = state$title, path = I(list()))) for (index in path) { + if (resolve_selectors) { + index <- if (index$kind == "index" && is.numeric(index$value)) { + index$value + } 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) { @@ -282,33 +266,37 @@ listview_location <- function(state, path = list()) { } 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 = workspace_env_names(child), + 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) ) - visited <- c(visited, index) 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))))) - # Environment bindings can disappear while their rows remain visible. - if (state$kind == "name" && is.environment(state$child_names)) { - key <- paste(visited, collapse = "/") - if (exists(key, envir = state$child_names, inherits = FALSE)) { - state$names <- get(key, envir = state$child_names, inherits = FALSE) - } else { - assign(key, state$names, envir = state$child_names) - } - } } + state$path <- as.list(visited) state } 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" @@ -319,15 +307,15 @@ get_workspace_children <- function(name = NULL, path = list(), start = 1L, view_ ) } else { state <- dataview_get_state(view_id) - if (!state$type %in% c("list", "vector")) stop("Not a list view") - vector_view <- identical(state$type, "vector") + 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 <- state$kind child_names <- state$names } child_count <- if (kind == "index") { - if (!is.null(view_id) && isTRUE(vector_view)) { + if (vector_rows) { length(object) } else { workspace_child_count(object) @@ -343,7 +331,7 @@ get_workspace_children <- function(name = NULL, path = list(), start = 1L, view_ children <- lapply(seq.int(start, end), function(index) { child_name <- if (is.null(child_names)) NULL else child_names[[index]] - label <- if (!is.null(view_id) && isTRUE(vector_view)) { + label <- if (vector_rows) { paste0("[", index, "]") } else if (kind == "slot") { paste0("@ ", child_name) @@ -372,7 +360,7 @@ get_workspace_children <- function(name = NULL, path = list(), start = 1L, view_ slot = methods::slot(object, child_name), index = object[[index]] ) - summary <- if (!is.null(view_id) && isTRUE(vector_view)) { + summary <- if (vector_rows) { if (is.character(child)) { encodeString(child, quote = "\"", na.encode = TRUE) } else { @@ -383,8 +371,8 @@ get_workspace_children <- function(name = NULL, path = list(), start = 1L, view_ } if (!is.null(view_id)) { list( - label = label, str = summary, viewable = !isTRUE(vector_view), index = index, - has_children = !isTRUE(vector_view) && workspace_child_count(child) > 0L + label = label, str = summary, viewable = !vector_rows, index = index, + has_children = !vector_rows && workspace_child_count(child) > 0L ) } else { selector <- if (kind == "index") { @@ -407,10 +395,11 @@ handle_listview_view <- function(view_id, index, path = list()) { if (is.null(state) || !identical(state$type, "list")) { return(FALSE) } - location <- tryCatch(listview_location(state, c(path, list(index))), error = function(e) NULL) + path <- c(path, list(index)) + location <- tryCatch(listview_location(state, path), error = function(e) NULL) if (is.null(location)) return(FALSE) - if (listview_is_list(location$data)) { - return(listview_navigation(state, c(path, list(index)))) + if (listview_supported(location$data)) { + return(listview_navigation(location)) } workspace_show_view(location$data, location$title, state$owner %||% state$title) } diff --git a/sess/R/hooks.R b/sess/R/hooks.R index 699321634..45ea5533c 100644 --- a/sess/R/hooks.R +++ b/sess/R/hooks.R @@ -122,10 +122,8 @@ runtime_start <- function(use_rstudioapi = TRUE, view_type <- if (dataview_is_table(x)) { "table" - } else if (is.list(x) || is.pairlist(x) || is.environment(x) || isS4(x)) { + } else if (listview_supported(x)) { "list" - } else if (is.atomic(x) && length(x) > 1L) { - "vector" } else { "object" } @@ -153,14 +151,17 @@ runtime_start <- function(use_rstudioapi = TRUE, type = "json", view_id = registration$view_id )) - } else if (view_type %in% c("list", "vector")) { + } 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) } root <- if (is.null(context)) listview_state(x, title_key, owner) else context$root - root$type <- view_type - navigation <- if (is.null(context)) listview_navigation(root) else context$navigation + navigation <- if (is.null(context)) { + listview_navigation(listview_location(root)) + } else { + context$navigation + } .sess_env$dataviews[[view_id]] <- root notify_client("dataview", list( title = title, diff --git a/sess/inst/tinytest/test-listview.R b/sess/inst/tinytest/test-listview.R index 4a82dc864..d41db1eb9 100644 --- a/sess/inst/tinytest/test-listview.R +++ b/sess/inst/tinytest/test-listview.R @@ -24,13 +24,22 @@ local({ 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 ))) - messages <- strsplit(processx::conn_read_chars(pipe[[1L]]), "\n", fixed = TRUE)[[1L]] - responses <- lapply(messages[nzchar(messages)], jsonlite::fromJSON, simplifyVector = FALSE) + responses <- read_messages() replies <- Filter(function(message) identical(message$id, "list-test"), responses) expect_length(replies, 1L) expect_null(replies[[1L]]$error) @@ -100,7 +109,8 @@ local({ name = table_root, path = list(selector(1L, "id")) ))) - vector_id <- id("vector", table_root) + 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, @@ -116,6 +126,8 @@ local({ 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, @@ -139,7 +151,7 @@ local({ utils::View(x$a$df) expect_identical(id("table", "x"), direct_table) utils::View(seq_len(501L)) - direct_vector <- id("vector", "seq_len(501L)") + direct_vector <- id("list", "seq_len(501L)") first_vector_page <- sess:::get_workspace_children( view_id = direct_vector, start = 1L @@ -163,6 +175,84 @@ local({ expect_equal(readLines(text_file), "2L") 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)) @@ -220,3 +310,44 @@ local({ 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/src/listViewer.ts b/src/listViewer.ts index 7f0be625a..34d8526d2 100644 --- a/src/listViewer.ts +++ b/src/listViewer.ts @@ -2,16 +2,17 @@ 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(generation: number, initial: ListViewNavigation = { title: '', path: [], breadcrumbs: [{ label: '', path: [] }], -}, vector = false): string { +}): string { return ` const vscode = acquireVsCodeApi(); const generation = ${generation}; - const vector = ${String(vector)}; const pending = new Map(); let nextRequestId = 0; const list = document.getElementById('list'); @@ -31,10 +32,11 @@ export function getListViewerScript(generation: number, initial: ListViewNavigat 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) }; + page = { element, scrollTop: 0, loader: createPage(element, navigation.path, navigation.vector) }; pages.set(key, page); } list.replaceChildren(page.element); @@ -80,7 +82,7 @@ export function getListViewerScript(generation: number, initial: ListViewNavigat if (history.length) navigate('listview/navigate', { path: history[history.length - 1].path }, true); }); - function createPage(container, path) { + function createPage(container, path, vector = false) { const rows = document.createElement('div'); const more = document.createElement('button'); more.className = 'load-more'; @@ -147,9 +149,12 @@ export function getListViewerScript(generation: number, initial: ListViewNavigat const children = document.createElement('div'); children.className = 'children'; entry.appendChild(children); - const page = createPage(children, [...path, item.index]); + let page; entry.addEventListener('toggle', () => { - if (entry.open) page.loadOnce(); + if (entry.open) { + page ??= createPage(children, [...path, item.index]); + page.loadOnce(); + } }); } rows.appendChild(entry); diff --git a/src/session.ts b/src/session.ts index 7b793c7c7..58072c8b8 100644 --- a/src/session.ts +++ b/src/session.ts @@ -220,6 +220,7 @@ interface DataViewRequestMessage { } const dynamicDataViewPanels = new Map(); +const listViewGenerations = new WeakMap(); let dynamicDataViewReloadRevision = 0; function escapeHtml(text: string): string { @@ -1045,14 +1046,14 @@ export async function showDataView(source: string, type: string, title: string, } const content = await getTableHtml(panel.webview, file || undefined, title); panel.webview.html = content; - } else if (source === 'list' || source === 'vector') { + } else if (source === 'list') { if (viewId) { const existing = dynamicDataViewPanels.get(viewId); if (existing) { existing.title = title; existing.reveal(existing.viewColumn, true); existing.webview.html = getListHtml( - existing.webview, title, navigation, source === 'vector' + existing.webview, title, navigation ); return; } @@ -1076,6 +1077,9 @@ export async function showDataView(source: string, type: string, title: string, message?: string; index?: number; start?: number; requestId?: number; path?: number[]; generation?: number; }) => { + if (message.generation !== listViewGenerations.get(panel.webview)) { + return; + } if (!Array.isArray(message.path) || !message.path.every(index => Number.isSafeInteger(index) && index > 0)) { return; } @@ -1085,6 +1089,9 @@ export async function showDataView(source: string, type: string, title: string, method: message.message === 'listview/navigate' ? 'listview_navigate' : 'listview_view', params: { view_id: viewId, index: message.index, path: message.path }, }) as ListViewNavigation | boolean | undefined; + if (message.generation !== listViewGenerations.get(panel.webview)) { + return; + } const navigation = result && typeof result === 'object' && Array.isArray(result.breadcrumbs) ? result : undefined; if (navigation) { @@ -1103,6 +1110,9 @@ export async function showDataView(source: string, type: string, title: string, method: 'workspace_children', params: { view_id: viewId, start: message.start, path: message.path }, }) as { children?: unknown; next_start?: number | null } | undefined; + if (message.generation !== listViewGenerations.get(panel.webview)) { + return; + } void panel.webview.postMessage({ message: 'listview/page', generation: message.generation, @@ -1113,9 +1123,11 @@ export async function showDataView(source: string, type: string, title: string, } }); panel.onDidDispose(() => { - if (dynamicDataViewPanels.get(viewId) === panel) { - dynamicDataViewPanels.delete(viewId); + listViewGenerations.delete(panel.webview); + if (dynamicDataViewPanels.get(viewId) !== panel) { + return; } + dynamicDataViewPanels.delete(viewId); void sessionRequest({ method: 'dataview_dispose', params: { view_id: viewId }, @@ -1123,7 +1135,7 @@ export async function showDataView(source: string, type: string, title: string, }); } panel.webview.html = getListHtml( - panel.webview, title, navigation, source === 'vector' + panel.webview, title, navigation ); } else { await commands.executeCommand('vscode.open', Uri.file(file), { @@ -1830,9 +1842,10 @@ export async function getTableHtml(webview: Webview, file: string | undefined, t export function getListHtml( webview: Webview, title: string, - navigation?: ListViewNavigation, - vector = false + navigation?: ListViewNavigation ): string { + const generation = ++dynamicDataViewReloadRevision; + listViewGenerations.set(webview, generation); const icon = new UriIcon('open-preview-codicon'); const darkIcon = webview.asWebviewUri(icon.dark).toString(); const lightIcon = webview.asWebviewUri(icon.light).toString(); @@ -1968,18 +1981,18 @@ export function getListHtml( } - +
diff --git a/src/test/suite/listViewer.test.ts b/src/test/suite/listViewer.test.ts index 5bf6d47e8..06d9ef815 100644 --- a/src/test/suite/listViewer.test.ts +++ b/src/test/suite/listViewer.test.ts @@ -35,19 +35,27 @@ interface Request { function createViewer(initial: ListViewNavigation = { title: 'x', path: [], breadcrumbs: [{ label: 'x', path: [] }], -}, vector = false) { +}) { 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, vector), { + 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'), }, @@ -56,7 +64,10 @@ function createViewer(initial: ListViewNavigation = { 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 }; + return { + get root() { return root.children[0]; }, + viewport: root, back, breadcrumbs, status, messages, reply, bodyClasses, + }; } suite('List viewer', () => { @@ -143,9 +154,11 @@ suite('List viewer', () => { 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; @@ -198,7 +211,9 @@ suite('List viewer', () => { }); test('renders vector values as simple indexed rows', () => { - const viewer = createViewer(undefined, true); + const viewer = createViewer({ + title: 'x', path: [], breadcrumbs: [{ label: 'x', path: [] }], vector: true, + }); viewer.reply(viewer.messages[0], { children: [{ label: '[1]', str: '12.4', index: 1, @@ -212,6 +227,53 @@ suite('List viewer', () => { 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], { generation: 6, children: [{ label: 'stale' }] }); diff --git a/src/test/suite/listViewerPanels.test.ts b/src/test/suite/listViewerPanels.test.ts index 418f009c6..9bec1de2a 100644 --- a/src/test/suite/listViewerPanels.test.ts +++ b/src/test/suite/listViewerPanels.test.ts @@ -62,13 +62,55 @@ suite('List viewer panels', () => { 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('vector', 'json', 'x$id', '', 'Two', 'test-vector-x'); - assert.strictEqual(create.callCount, 4); + 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, 5); + 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 generation = () => Number(/const generation = (\d+)/.exec(panel.webview.html)?.[1]); + await session.showDataView('list', 'json', 'x', '', 'Two', 'test-list-generation'); + + for (const message of ['listview/navigate', 'listview/page']) { + const request = { message, generation: generation(), 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-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', generation: generation(), requestId: 2, path: [], start: 1 }); + sinon.assert.calledOnce(postMessage); + const response = postMessage.firstCall.args[0] as { generation: number }; + assert.strictEqual(response.generation, generation()); }); test('supported workspace children retain open actions alongside expansion', () => { From 6d0ab112ce1b1134180f0452ac5f14c6d71a59cd Mon Sep 17 00:00:00 2001 From: "4111978+Fred-Wu@users.noreply.github.com" <4111978+Fred-Wu@users.noreply.github.com> Date: Sun, 4 Oct 2026 12:35:04 +1100 Subject: [PATCH 12/30] Implement session-aware data/list viewers --- src/interactive/backend.ts | 4 +- src/interactive/manager.ts | 2 +- src/session.ts | 80 ++++---- src/test/suite/dataViewerSessions.test.ts | 224 ++++++++++++++++++++++ 4 files changed, 268 insertions(+), 42 deletions(-) create mode 100644 src/test/suite/dataViewerSessions.test.ts diff --git a/src/interactive/backend.ts b/src/interactive/backend.ts index b4460e964..a4fd57403 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/manager.ts b/src/interactive/manager.ts index 199beddca..57c0c4098 100644 --- a/src/interactive/manager.ts +++ b/src/interactive/manager.ts @@ -1184,7 +1184,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), undefined, 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/session.ts b/src/session.ts index 58072c8b8..69dd3d5fc 100644 --- a/src/session.ts +++ b/src/session.ts @@ -234,8 +234,24 @@ 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}`; +function registerDataViewPanel(panel: vscode.WebviewPanel, key: string, viewId: string, sessionId: string | null): void { + dynamicDataViewPanels.set(key, panel); + panel.onDidDispose(() => { + listViewGenerations.delete(panel.webview); + if (dynamicDataViewPanels.get(key) !== panel) { + return; + } + dynamicDataViewPanels.delete(key); + // Interactive transcripts retain this handle after the expanded viewer closes. + if (sessions.get(sessionId ?? '')?.requester) { return; } + void sessionRequest({ + method: 'dataview_dispose', + params: { view_id: viewId }, + }, sessionId); + }); +} + +function attachDynamicDataViewBridge(panel: vscode.WebviewPanel, viewId: string, sessionId: string | null): void { const postResponse = (requestId: number, ok: boolean, result?: unknown, error?: string) => { void panel.webview.postMessage({ message: 'dataview/response', @@ -257,7 +273,7 @@ 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'); } @@ -275,7 +291,7 @@ 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') { @@ -290,19 +306,6 @@ function attachDynamicDataViewBridge(panel: vscode.WebviewPanel, viewId: string, postResponse(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 { @@ -1012,13 +1015,18 @@ export function openExternalBrowser(): void { } } -export async function showDataView(source: string, type: string, title: string, file: string, viewer: string, viewId?: string, navigation?: ListViewNavigation, 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, +): 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) { existing.title = title; existing.reveal(existing.viewColumn, true); @@ -1041,14 +1049,14 @@ 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); + 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(viewId); + const existing = dynamicDataViewPanels.get(panelKey); if (existing) { existing.title = title; existing.reveal(existing.viewColumn, true); @@ -1072,7 +1080,7 @@ export async function showDataView(source: string, type: string, title: string, }); panel.iconPath = new UriIcon('preview'); if (viewId) { - dynamicDataViewPanels.set(viewId, panel); + registerDataViewPanel(panel, panelKey, viewId, sessionId); panel.webview.onDidReceiveMessage(async (message: { message?: string; index?: number; start?: number; requestId?: number; path?: number[]; generation?: number; @@ -1088,7 +1096,7 @@ export async function showDataView(source: string, type: string, title: string, const result = await sessionRequest({ method: message.message === 'listview/navigate' ? 'listview_navigate' : 'listview_view', params: { view_id: viewId, index: message.index, path: message.path }, - }) as ListViewNavigation | boolean | undefined; + }, sessionId) as ListViewNavigation | boolean | undefined; if (message.generation !== listViewGenerations.get(panel.webview)) { return; } @@ -1109,7 +1117,7 @@ export async function showDataView(source: string, type: string, title: string, const page = await sessionRequest({ method: 'workspace_children', params: { view_id: viewId, start: message.start, path: message.path }, - }) as { children?: unknown; next_start?: number | null } | undefined; + }, sessionId) as { children?: unknown; next_start?: number | null } | undefined; if (message.generation !== listViewGenerations.get(panel.webview)) { return; } @@ -1122,17 +1130,6 @@ export async function showDataView(source: string, type: string, title: string, }); } }); - panel.onDidDispose(() => { - listViewGenerations.delete(panel.webview); - if (dynamicDataViewPanels.get(viewId) !== panel) { - return; - } - dynamicDataViewPanels.delete(viewId); - void sessionRequest({ - method: 'dataview_dispose', - params: { view_id: viewId }, - }); - }); } panel.webview.html = getListHtml( panel.webview, title, navigation @@ -2240,6 +2237,7 @@ async function handleNotification(message: Record, socket: IpcS viewer, params.view_id ? String(params.view_id) : undefined, params.navigation as ListViewNavigation | undefined, + socket._sessionId ?? null, ); } } @@ -2383,10 +2381,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 000000000..3d7b576bb --- /dev/null +++ b/src/test/suite/dataViewerSessions.test.ts @@ -0,0 +1,224 @@ +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 }; +} + +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: 1, 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): Promise { + const count = panels.length; + const html = existing?.panel.webview.html; + notify(client, 'dataview', { + source, type: 'json', title: 'x', view_id: `same-${source}-id`, + 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 generation = Number(/const generation = (\d+)/.exec(panel.panel.webview.html)?.[1]); + await panel.receive({ generation, 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); + await open(a, 'table', aTable); + 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); + + 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.strictEqual(viewerRequests(a).length, 5); + await open(a, 'list', aList); + await open(a, 'table', aTable); + }); + + 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); + }); +}); From 2bfdc43f133e6b4679473ef844cc20c597b7a0a6 Mon Sep 17 00:00:00 2001 From: "4111978+Fred-Wu@users.noreply.github.com" <4111978+Fred-Wu@users.noreply.github.com> Date: Sun, 4 Oct 2026 12:50:39 +1100 Subject: [PATCH 13/30] Fixed viewer not reopening after cleanup --- src/session.ts | 22 +++++++----- src/test/suite/listViewerPanels.test.ts | 47 +++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/src/session.ts b/src/session.ts index 69dd3d5fc..821bbf4a2 100644 --- a/src/session.ts +++ b/src/session.ts @@ -235,9 +235,11 @@ function escapeHtml(text: string): string { } function registerDataViewPanel(panel: vscode.WebviewPanel, key: string, viewId: string, sessionId: string | null): void { + // The panel's webview getter throws once onDidDispose fires. + const webview = panel.webview; dynamicDataViewPanels.set(key, panel); panel.onDidDispose(() => { - listViewGenerations.delete(panel.webview); + listViewGenerations.delete(webview); if (dynamicDataViewPanels.get(key) !== panel) { return; } @@ -252,8 +254,9 @@ function registerDataViewPanel(panel: vscode.WebviewPanel, key: string, viewId: } function attachDynamicDataViewBridge(panel: vscode.WebviewPanel, viewId: string, sessionId: string | null): void { + const webview = panel.webview; const postResponse = (requestId: number, ok: boolean, result?: unknown, error?: string) => { - void panel.webview.postMessage({ + void webview.postMessage({ message: 'dataview/response', requestId, ok, @@ -262,7 +265,7 @@ 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') { return; @@ -1081,11 +1084,12 @@ export async function showDataView( panel.iconPath = new UriIcon('preview'); if (viewId) { registerDataViewPanel(panel, panelKey, viewId, sessionId); - panel.webview.onDidReceiveMessage(async (message: { + const webview = panel.webview; + webview.onDidReceiveMessage(async (message: { message?: string; index?: number; start?: number; requestId?: number; path?: number[]; generation?: number; }) => { - if (message.generation !== listViewGenerations.get(panel.webview)) { + if (message.generation !== listViewGenerations.get(webview)) { return; } if (!Array.isArray(message.path) || !message.path.every(index => Number.isSafeInteger(index) && index > 0)) { @@ -1097,7 +1101,7 @@ export async function showDataView( 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.generation !== listViewGenerations.get(panel.webview)) { + if (message.generation !== listViewGenerations.get(webview)) { return; } const navigation = result && typeof result === 'object' && Array.isArray(result.breadcrumbs) @@ -1105,7 +1109,7 @@ export async function showDataView( if (navigation) { panel.title = navigation.title; } - void panel.webview.postMessage({ + void webview.postMessage({ message: 'listview/navigation', generation: message.generation, requestId: message.requestId, @@ -1118,10 +1122,10 @@ export async function showDataView( method: 'workspace_children', params: { view_id: viewId, start: message.start, path: message.path }, }, sessionId) as { children?: unknown; next_start?: number | null } | undefined; - if (message.generation !== listViewGenerations.get(panel.webview)) { + if (message.generation !== listViewGenerations.get(webview)) { return; } - void panel.webview.postMessage({ + void webview.postMessage({ message: 'listview/page', generation: message.generation, requestId: message.requestId, diff --git a/src/test/suite/listViewerPanels.test.ts b/src/test/suite/listViewerPanels.test.ts index 9bec1de2a..ad757b28d 100644 --- a/src/test/suite/listViewerPanels.test.ts +++ b/src/test/suite/listViewerPanels.test.ts @@ -22,6 +22,53 @@ suite('List viewer panels', () => { 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 generation = Number(/const generation = (\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', requestId: 1 })] + : ['listview/page', 'listview/navigate'].map(message => listener({ + message, generation, 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) => { From 65d61d1ec83daac929d3849850adcb48f405683f Mon Sep 17 00:00:00 2001 From: "4111978+Fred-Wu@users.noreply.github.com" <4111978+Fred-Wu@users.noreply.github.com> Date: Sun, 4 Oct 2026 13:01:26 +1100 Subject: [PATCH 14/30] fix(viewer): avoid evaluating active root bindings twice --- sess/R/handlers.R | 10 ++++++++-- sess/inst/tinytest/test-listview.R | 29 +++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/sess/R/handlers.R b/sess/R/handlers.R index e506d788b..e303ab327 100644 --- a/sess/R/handlers.R +++ b/sess/R/handlers.R @@ -201,9 +201,15 @@ listview_expression_context <- function(expression, envir, owner) { selectors <- c(list(list(kind = "index", value = value)), selectors) expression <- expression[[2L]] } - if (!is.symbol(expression)) return(NULL) + if (!length(selectors) || !is.symbol(expression)) return(NULL) name <- as.character(expression) - root <- listview_state(get(name, envir = envir, inherits = TRUE), name, owner) + while (!identical(envir, emptyenv()) && !exists(name, envir = envir, inherits = FALSE)) { + envir <- parent.env(envir) + } + # View has already evaluated its argument; rebuilding an active root would + # evaluate it again and could display a different value. + if (identical(envir, emptyenv()) || bindingIsActive(name, envir)) return(NULL) + root <- listview_state(get(name, envir = envir, inherits = FALSE), name, owner) listview_context(root, selectors) }, error = function(e) NULL) } diff --git a/sess/inst/tinytest/test-listview.R b/sess/inst/tinytest/test-listview.R index d41db1eb9..89ea2f26e 100644 --- a/sess/inst/tinytest/test-listview.R +++ b/sess/inst/tinytest/test-listview.R @@ -146,6 +146,35 @@ local({ direct_context <- sess:::listview_expression_context(quote(x$a$b), environment(), "x") expect_equal(direct_context$navigation$path, list(1L, 1L), check.attributes = FALSE) expect_equal(direct_context$navigation$title, "x$a$b") + + # 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) + } + } + utils::View(x$df) direct_table <- id("table", "x") utils::View(x$a$df) From 7582cc911e6b6fcaf78ea00b18cb02db1b2d0338 Mon Sep 17 00:00:00 2001 From: "4111978+Fred-Wu@users.noreply.github.com" <4111978+Fred-Wu@users.noreply.github.com> Date: Sun, 4 Oct 2026 14:14:37 +1100 Subject: [PATCH 15/30] fix(viewer): preserve custom extraction semantics --- sess/R/handlers.R | 70 +++++++++++++---- sess/R/hooks.R | 2 +- sess/inst/tinytest/test-listview.R | 118 ++++++++++++++++++++++++++++- 3 files changed, 172 insertions(+), 18 deletions(-) diff --git a/sess/R/handlers.R b/sess/R/handlers.R index e303ab327..726b09b53 100644 --- a/sess/R/handlers.R +++ b/sess/R/handlers.R @@ -182,35 +182,66 @@ listview_state <- function(object, title, owner) { } # Resolve workspace selectors once; navigation paths are relative to the retained root. -listview_context <- function(root, selectors) { - location <- listview_location(root, selectors, resolve_selectors = TRUE) +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) { +listview_expression_context <- function(expression, envir, owner, value) { tryCatch({ selectors <- list() while (is.call(expression) && length(expression) == 3L && is.symbol(expression[[1L]])) { operator <- as.character(expression[[1L]]) if (!operator %in% c("$", "[[", "@")) return(NULL) - value <- expression[[3L]] - if (operator %in% c("$", "@") && is.symbol(value)) value <- as.character(value) - if (length(value) != 1L || !(is.character(value) || is.numeric(value))) return(NULL) - selectors <- c(list(list(kind = "index", value = value)), selectors) + 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) - while (!identical(envir, emptyenv()) && !exists(name, envir = envir, inherits = FALSE)) { - envir <- parent.env(envir) + object <- listview_binding(name, envir) + if (is.null(object) || !listview_structural_extraction(object, selectors[[1L]]$kind, envir)) { + return(NULL) } - # View has already evaluated its argument; rebuilding an active root would - # evaluate it again and could display a different value. - if (identical(envir, emptyenv()) || bindingIsActive(name, envir)) return(NULL) - root <- listview_state(get(name, envir = envir, inherits = FALSE), name, owner) - listview_context(root, selectors) + 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) } @@ -236,13 +267,20 @@ handle_listview_navigate <- function(view_id, path = list()) { listview_navigation(location) } -listview_location <- function(state, path = list(), resolve_selectors = FALSE) { +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) { - index <- if (index$kind == "index" && is.numeric(index$value)) { + 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) } diff --git a/sess/R/hooks.R b/sess/R/hooks.R index 45ea5533c..569621944 100644 --- a/sess/R/hooks.R +++ b/sess/R/hooks.R @@ -154,7 +154,7 @@ runtime_start <- function(use_rstudioapi = TRUE, } 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) + 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)) { diff --git a/sess/inst/tinytest/test-listview.R b/sess/inst/tinytest/test-listview.R index 89ea2f26e..71747a504 100644 --- a/sess/inst/tinytest/test-listview.R +++ b/sess/inst/tinytest/test-listview.R @@ -143,9 +143,14 @@ local({ 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") + 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)) { @@ -175,6 +180,117 @@ local({ } } + # 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) From 6647d3f3bef29f5aa09fce843c105b70c8985b42 Mon Sep 17 00:00:00 2001 From: "4111978+Fred-Wu@users.noreply.github.com" <4111978+Fred-Wu@users.noreply.github.com> Date: Sun, 4 Oct 2026 20:42:14 +1100 Subject: [PATCH 16/30] fix(viewer): preserve names for named vectors while empty or missing names retain [index] labels. --- sess/R/handlers.R | 6 +++++- sess/inst/tinytest/test-listview.R | 26 ++++++++++++++++++++++++++ src/test/suite/listViewer.test.ts | 22 +++++++++++++--------- 3 files changed, 44 insertions(+), 10 deletions(-) diff --git a/sess/R/handlers.R b/sess/R/handlers.R index 726b09b53..a53609bbf 100644 --- a/sess/R/handlers.R +++ b/sess/R/handlers.R @@ -376,7 +376,11 @@ get_workspace_children <- function(name = NULL, path = list(), start = 1L, view_ children <- lapply(seq.int(start, end), function(index) { child_name <- if (is.null(child_names)) NULL else child_names[[index]] label <- if (vector_rows) { - paste0("[", index, "]") + if (!is.null(child_name) && !is.na(child_name) && nzchar(child_name)) { + child_name + } else { + paste0("[", index, "]") + } } else if (kind == "slot") { paste0("@ ", child_name) } else { diff --git a/sess/inst/tinytest/test-listview.R b/sess/inst/tinytest/test-listview.R index 71747a504..0b560d6cb 100644 --- a/sess/inst/tinytest/test-listview.R +++ b/sess/inst/tinytest/test-listview.R @@ -311,6 +311,32 @@ local({ 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) text_id <- id("object", "x") text_file <- runtime$dataviews[[text_id]]$file diff --git a/src/test/suite/listViewer.test.ts b/src/test/suite/listViewer.test.ts index 06d9ef815..d0e9e6ace 100644 --- a/src/test/suite/listViewer.test.ts +++ b/src/test/suite/listViewer.test.ts @@ -210,21 +210,25 @@ suite('List viewer', () => { assert.strictEqual(secondPage.children[1].hidden, true); }); - test('renders vector values as simple indexed rows', () => { + 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: [{ - label: '[1]', str: '12.4', index: 1, + 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')); }); - const row = viewer.root.children[0].children[0].children[0]; - assert.deepStrictEqual(row.children.map(child => child.className), ['label', 'str']); - assert.strictEqual(row.children[0].textContent, '[1]'); - 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', () => { From 89adf4ff9bfec734bd21a7d7a8873d2ba8b4b918 Mon Sep 17 00:00:00 2001 From: "4111978+Fred-Wu@users.noreply.github.com" <4111978+Fred-Wu@users.noreply.github.com> Date: Sun, 4 Oct 2026 21:32:04 +1100 Subject: [PATCH 17/30] fix(viewer): - reuse data viewer formatting for list and vector values - route scalar value to the shared list viewer for consistency --- sess/R/handlers.R | 64 +++++++++--- sess/inst/tinytest/test-listview-format.R | 113 ++++++++++++++++++++++ sess/inst/tinytest/test-listview.R | 58 ++++++++++- 3 files changed, 219 insertions(+), 16 deletions(-) create mode 100644 sess/inst/tinytest/test-listview-format.R diff --git a/sess/R/handlers.R b/sess/R/handlers.R index a53609bbf..6a658ef02 100644 --- a/sess/R/handlers.R +++ b/sess/R/handlers.R @@ -160,12 +160,11 @@ handle_workspace_view <- function(name, path = list()) { listview_supported <- function(object) { !dataview_is_table(object) && (is.list(object) || is.pairlist(object) || is.environment(object) || isS4(object) || - (is.atomic(object) && length(object) > 1L)) + listview_is_vector(object)) } listview_is_vector <- function(object) { - !dataview_is_table(object) && - (inherits(object, "POSIXlt") || (!isS4(object) && is.atomic(object) && length(object) > 1L)) + !dataview_is_table(object) && (is.atomic(object) || inherits(object, "POSIXlt")) } listview_state <- function(object, title, owner) { @@ -338,6 +337,50 @@ listview_location <- function(state, path = list(), resolve_selectors = FALSE, 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 @@ -355,8 +398,8 @@ get_workspace_children <- function(name = NULL, path = list(), start = 1L, view_ state <- listview_location(state, path) object <- state$data vector_rows <- listview_is_vector(object) - kind <- state$kind - child_names <- state$names + 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) { @@ -372,6 +415,7 @@ get_workspace_children <- function(name = NULL, path = list(), start = 1L, view_ 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 <- lapply(seq.int(start, end), function(index) { child_name <- if (is.null(child_names)) NULL else child_names[[index]] @@ -403,19 +447,15 @@ get_workspace_children <- function(name = NULL, path = list(), start = 1L, view_ type = unavailable, has_children = FALSE )) } - child <- switch(kind, + 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) { - if (is.character(child)) { - encodeString(child, quote = "\"", na.encode = TRUE) - } else { - paste(format(child, trim = TRUE), collapse = " ") - } + formatted_values[[index - start + 1L]] } else { - trimws(try_capture_str(child)) + listview_summary(child) } if (!is.null(view_id)) { list( diff --git a/sess/inst/tinytest/test-listview-format.R b/sess/inst/tinytest/test-listview-format.R new file mode 100644 index 000000000..56f257c18 --- /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 index 0b560d6cb..2098e3a05 100644 --- a/sess/inst/tinytest/test-listview.R +++ b/sess/inst/tinytest/test-listview.R @@ -338,12 +338,62 @@ local({ } utils::View(x$a$b$value) - text_id <- id("object", "x") - text_file <- runtime$dataviews[[text_id]]$file + 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) - expect_identical(id("object", "x"), text_id) + 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(difftime(1, 2, 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), "2L") + expect_equal(readLines(text_file), deparse(functions$second)) unlink(text_file) # Vectors opened from expanded list rows retain the root and every breadcrumb. From 65ca4872a5a1fe55b625d4690a29a4c562fa8b83 Mon Sep 17 00:00:00 2001 From: Fred-Wu <4111978+Fred-Wu@users.noreply.github.com> Date: Sun, 4 Oct 2026 23:07:20 +1100 Subject: [PATCH 18/30] fix(viewer): prevent stale disposal from deleting reopened viewer state --- sess/R/handlers.R | 15 ++++++-- sess/R/hooks.R | 6 ++-- sess/inst/tinytest/test-dataview.R | 43 +++++++++++++++++++++++ sess/inst/tinytest/test-listview.R | 6 ++-- src/session.ts | 25 ++++++++++--- src/test/suite/dataViewerSessions.test.ts | 16 ++++++--- 6 files changed, 95 insertions(+), 16 deletions(-) diff --git a/sess/R/handlers.R b/sess/R/handlers.R index 6a658ef02..bf81338d3 100644 --- a/sess/R/handlers.R +++ b/sess/R/handlers.R @@ -804,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$instance <- (.sess_env$dataviews[[view_id]]$instance %||% 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() } @@ -816,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, + instance = state$instance, total_rows = state$total_rows, columns = dataview_columns(state) ) @@ -839,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$instance <- state$instance state <- current .sess_env$dataviews[[view_id]] <- state } @@ -1268,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]] + instance <- suppressWarnings(as.integer(params$instance %||% NA_integer_)) + if (!is.null(state) && identical(state$instance, instance)) { .sess_env$dataviews[[view_id]] <- NULL } TRUE diff --git a/sess/R/hooks.R b/sess/R/hooks.R index 569621944..7a2081a3d 100644 --- a/sess/R/hooks.R +++ b/sess/R/hooks.R @@ -149,7 +149,8 @@ runtime_start <- function(use_rstudioapi = TRUE, title = title, source = "table", type = "json", - view_id = registration$view_id + view_id = registration$view_id, + instance = registration$instance )) } else if (view_type == "list") { context <- .sess_env$listview_context @@ -162,12 +163,13 @@ runtime_start <- function(use_rstudioapi = TRUE, } else { context$navigation } - .sess_env$dataviews[[view_id]] <- root + root <- dataview_set_state(view_id, root) notify_client("dataview", list( title = title, source = view_type, type = "json", view_id = view_id, + instance = root$instance, navigation = navigation )) } else { diff --git a/sess/inst/tinytest/test-dataview.R b/sess/inst/tinytest/test-dataview.R index 977121b47..b4d464b44 100644 --- a/sess/inst/tinytest/test-dataview.R +++ b/sess/inst/tinytest/test-dataview.R @@ -368,3 +368,46 @@ local({ 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$instance > first$instance, info = name) + + expect_true(sess:::handle_dataview_dispose(list( + view_id = "replacement_test", instance = first$instance + )), info = name) + expect_identical( + runtime$dataviews$replacement_test$instance, replacement$instance, 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", instance = replacement$instance + )), info = name) + expect_null(runtime$dataviews$replacement_test, info = name) + } +}) diff --git a/sess/inst/tinytest/test-listview.R b/sess/inst/tinytest/test-listview.R index 2098e3a05..3ae56614d 100644 --- a/sess/inst/tinytest/test-listview.R +++ b/sess/inst/tinytest/test-listview.R @@ -358,7 +358,7 @@ local({ expect_length(notifications, 0L) # Scalars use the vector formatting route regardless of storage type or class. - for (sample in list(difftime(1, 2, units = "mins"), as.Date("2026-01-01"), + 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") @@ -478,7 +478,9 @@ local({ 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))) + expect_true(sess:::handle_dataview_dispose(list( + view_id = list_id, instance = runtime$dataviews[[list_id]]$instance + ))) expect_null(runtime$dataviews[[list_id]]) sess:::handle_workspace_view(root) expect_identical(id("list"), list_id) diff --git a/src/session.ts b/src/session.ts index 821bbf4a2..c7f338544 100644 --- a/src/session.ts +++ b/src/session.ts @@ -220,6 +220,7 @@ interface DataViewRequestMessage { } const dynamicDataViewPanels = new Map(); +const dynamicDataViewInstances = new WeakMap(); const listViewGenerations = new WeakMap(); let dynamicDataViewReloadRevision = 0; @@ -234,12 +235,20 @@ function escapeHtml(text: string): string { return text.replace(/[&<>"']/g, c => map[c]); } -function registerDataViewPanel(panel: vscode.WebviewPanel, key: string, viewId: string, sessionId: string | null): void { +function registerDataViewPanel( + panel: vscode.WebviewPanel, key: string, viewId: string, sessionId: string | null, + instance?: number, +): void { // The panel's webview getter throws once onDidDispose fires. const webview = panel.webview; dynamicDataViewPanels.set(key, panel); + if (instance !== undefined) { + dynamicDataViewInstances.set(panel, instance); + } panel.onDidDispose(() => { listViewGenerations.delete(webview); + const currentInstance = dynamicDataViewInstances.get(panel); + dynamicDataViewInstances.delete(panel); if (dynamicDataViewPanels.get(key) !== panel) { return; } @@ -248,7 +257,7 @@ function registerDataViewPanel(panel: vscode.WebviewPanel, key: string, viewId: if (sessions.get(sessionId ?? '')?.requester) { return; } void sessionRequest({ method: 'dataview_dispose', - params: { view_id: viewId }, + params: { view_id: viewId, instance: currentInstance }, }, sessionId); }); } @@ -1022,6 +1031,7 @@ export async function showDataView( source: string, type: string, title: string, file: string, viewer: string, viewId?: string, navigation?: ListViewNavigation, sessionId: string | null = activeSession?.sessionId ?? null, + instance?: 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 ?? '')}`); @@ -1031,6 +1041,9 @@ export async function showDataView( if (viewId) { const existing = dynamicDataViewPanels.get(panelKey); if (existing) { + if (instance !== undefined) { + dynamicDataViewInstances.set(existing, instance); + } existing.title = title; existing.reveal(existing.viewColumn, true); const content = await getTableHtml(existing.webview, undefined, title); @@ -1052,7 +1065,7 @@ export async function showDataView( }); panel.iconPath = new UriIcon('open-preview'); if (viewId) { - registerDataViewPanel(panel, panelKey, viewId, sessionId); + registerDataViewPanel(panel, panelKey, viewId, sessionId, instance); attachDynamicDataViewBridge(panel, viewId, sessionId); } const content = await getTableHtml(panel.webview, file || undefined, title); @@ -1061,6 +1074,9 @@ export async function showDataView( if (viewId) { const existing = dynamicDataViewPanels.get(panelKey); if (existing) { + if (instance !== undefined) { + dynamicDataViewInstances.set(existing, instance); + } existing.title = title; existing.reveal(existing.viewColumn, true); existing.webview.html = getListHtml( @@ -1083,7 +1099,7 @@ export async function showDataView( }); panel.iconPath = new UriIcon('preview'); if (viewId) { - registerDataViewPanel(panel, panelKey, viewId, sessionId); + registerDataViewPanel(panel, panelKey, viewId, sessionId, instance); const webview = panel.webview; webview.onDidReceiveMessage(async (message: { message?: string; index?: number; start?: number; requestId?: number; @@ -2242,6 +2258,7 @@ async function handleNotification(message: Record, socket: IpcS params.view_id ? String(params.view_id) : undefined, params.navigation as ListViewNavigation | undefined, socket._sessionId ?? null, + typeof params.instance === 'number' ? params.instance : undefined, ); } } diff --git a/src/test/suite/dataViewerSessions.test.ts b/src/test/suite/dataViewerSessions.test.ts index 3d7b576bb..457732dcc 100644 --- a/src/test/suite/dataViewerSessions.test.ts +++ b/src/test/suite/dataViewerSessions.test.ts @@ -11,7 +11,7 @@ import { mockExtensionContext } from '../common/mockvscode'; interface Request { id: number; method: string; - params?: { view_id?: string }; + params?: { view_id?: string; instance?: number }; } interface Client { @@ -137,11 +137,13 @@ suite('Viewer session ownership', () => { return client; } - async function open(client: Client, source: 'list' | 'table', existing?: Panel): Promise { + async function open( + client: Client, source: 'list' | 'table', existing?: Panel, instance = 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`, + source, type: 'json', title: 'x', view_id: `same-${source}-id`, instance, navigation: { title: 'x', path: [], breadcrumbs: [{ label: 'x', path: [] }] }, }); await waitFor(() => existing ? existing.panel.webview.html !== html : panels.length > count); @@ -166,8 +168,8 @@ suite('Viewer session ownership', () => { const bList = await open(b, 'list'); const bTable = await open(b, 'table'); assert.strictEqual(panels.length, 4); - await open(a, 'list', aList); - await open(a, 'table', aTable); + 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) => { @@ -185,11 +187,15 @@ suite('Viewer session ownership', () => { 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?.instance), + [2, 2]); assert.strictEqual(viewerRequests(a).length, 5); await open(a, 'list', aList); await open(a, 'table', aTable); From 1d66df6d6bed9e45388d1f33e248357bbac22ea1 Mon Sep 17 00:00:00 2001 From: Fred-Wu <4111978+Fred-Wu@users.noreply.github.com> Date: Sun, 4 Oct 2026 23:50:33 +1100 Subject: [PATCH 19/30] fix(viewer): isolate reloaded viewer responses --- sess/R/handlers.R | 8 +-- sess/R/hooks.R | 4 +- sess/inst/tinytest/test-dataview.R | 8 +-- sess/inst/tinytest/test-listview.R | 7 +- src/listViewer.ts | 10 +-- src/session.ts | 85 +++++++++++++---------- src/test/suite/dataViewerSessions.test.ts | 12 ++-- src/test/suite/listViewerPanels.test.ts | 58 +++++++++++++--- 8 files changed, 124 insertions(+), 68 deletions(-) diff --git a/sess/R/handlers.R b/sess/R/handlers.R index bf81338d3..e00907e62 100644 --- a/sess/R/handlers.R +++ b/sess/R/handlers.R @@ -808,7 +808,7 @@ dataview_set_state <- function(view_id, state) { if (is.null(.sess_env$dataviews)) { .sess_env$dataviews <- list() } - state$instance <- (.sess_env$dataviews[[view_id]]$instance %||% 0L) + 1L + state$state_generation <- (.sess_env$dataviews[[view_id]]$state_generation %||% 0L) + 1L .sess_env$dataviews[[view_id]] <- state state } @@ -824,7 +824,7 @@ dataview_register <- function(data, view_id = NULL, live = FALSE) { state <- dataview_set_state(view_id, state) list( view_id = view_id, - instance = state$instance, + state_generation = state$state_generation, total_rows = state$total_rows, columns = dataview_columns(state) ) @@ -1276,8 +1276,8 @@ handle_dataview_page <- function(params) { handle_dataview_dispose <- function(params) { view_id <- as.character(params$view_id %||% "") state <- if (is.null(.sess_env$dataviews)) NULL else .sess_env$dataviews[[view_id]] - instance <- suppressWarnings(as.integer(params$instance %||% NA_integer_)) - if (!is.null(state) && identical(state$instance, instance)) { + 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 7a2081a3d..a76ae3d55 100644 --- a/sess/R/hooks.R +++ b/sess/R/hooks.R @@ -150,7 +150,7 @@ runtime_start <- function(use_rstudioapi = TRUE, source = "table", type = "json", view_id = registration$view_id, - instance = registration$instance + state_generation = registration$state_generation )) } else if (view_type == "list") { context <- .sess_env$listview_context @@ -169,7 +169,7 @@ runtime_start <- function(use_rstudioapi = TRUE, source = view_type, type = "json", view_id = view_id, - instance = root$instance, + state_generation = root$state_generation, navigation = navigation )) } else { diff --git a/sess/inst/tinytest/test-dataview.R b/sess/inst/tinytest/test-dataview.R index b4d464b44..512998d20 100644 --- a/sess/inst/tinytest/test-dataview.R +++ b/sess/inst/tinytest/test-dataview.R @@ -393,20 +393,20 @@ local({ replacement <- sess:::dataview_set_state( "replacement_test", cases[[name]]$replacement ) - expect_true(replacement$instance > first$instance, info = name) + expect_true(replacement$state_generation > first$state_generation, info = name) expect_true(sess:::handle_dataview_dispose(list( - view_id = "replacement_test", instance = first$instance + view_id = "replacement_test", state_generation = first$state_generation )), info = name) expect_identical( - runtime$dataviews$replacement_test$instance, replacement$instance, info = name + 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", instance = replacement$instance + 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-listview.R b/sess/inst/tinytest/test-listview.R index 3ae56614d..252a46a17 100644 --- a/sess/inst/tinytest/test-listview.R +++ b/sess/inst/tinytest/test-listview.R @@ -13,7 +13,10 @@ local({ runtime$con <- previous_con runtime$dataviews <- previous_views runtime$dataview_registry <- previous_registry - rm(list = c(root, other, table_root), envir = .GlobalEnv) + 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]] @@ -479,7 +482,7 @@ local({ 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, instance = runtime$dataviews[[list_id]]$instance + view_id = list_id, state_generation = runtime$dataviews[[list_id]]$state_generation ))) expect_null(runtime$dataviews[[list_id]]) sess:::handle_workspace_view(root) diff --git a/src/listViewer.ts b/src/listViewer.ts index 34d8526d2..f680ebe9a 100644 --- a/src/listViewer.ts +++ b/src/listViewer.ts @@ -7,12 +7,12 @@ export interface ListViewNavigation { } /** Script shared by the list webview and its interaction tests. */ -export function getListViewerScript(generation: number, initial: ListViewNavigation = { +export function getListViewerScript(documentGeneration: number, initial: ListViewNavigation = { title: '', path: [], breadcrumbs: [{ label: '', path: [] }], }): string { return ` const vscode = acquireVsCodeApi(); - const generation = ${generation}; + const documentGeneration = ${documentGeneration}; const pending = new Map(); let nextRequestId = 0; const list = document.getElementById('list'); @@ -75,7 +75,7 @@ export function getListViewerScript(generation: number, initial: ListViewNavigat showNavigation(response.navigation, goingBack); } }); - vscode.postMessage({ message, generation, requestId, ...params }); + vscode.postMessage({ message, documentGeneration, requestId, ...params }); } back.addEventListener('click', () => { @@ -164,7 +164,7 @@ export function getListViewerScript(generation: number, initial: ListViewNavigat more.textContent = 'Load more'; status.textContent = rows.childElementCount ? '' : 'No items'; }); - vscode.postMessage({ message: 'listview/page', generation, requestId, path, start: nextStart }); + vscode.postMessage({ message: 'listview/page', documentGeneration, requestId, path, start: nextStart }); } more.addEventListener('click', loadPage); return { loadOnce: () => { if (!loaded) loadPage(); } }; @@ -172,7 +172,7 @@ export function getListViewerScript(generation: number, initial: ListViewNavigat window.addEventListener('message', (event) => { const message = event.data; - if (!['listview/page', 'listview/navigation'].includes(message.message) || message.generation !== generation) return; + if (!['listview/page', 'listview/navigation'].includes(message.message) || message.documentGeneration !== documentGeneration) return; const receive = pending.get(message.requestId); if (receive) { pending.delete(message.requestId); diff --git a/src/session.ts b/src/session.ts index c7f338544..160410b15 100644 --- a/src/session.ts +++ b/src/session.ts @@ -213,6 +213,7 @@ interface DataViewRequestMessage { message: 'dataview/request'; action: 'init' | 'page'; requestId: number; + documentGeneration: number; startRow?: number; endRow?: number; sortModel?: unknown[]; @@ -220,9 +221,9 @@ interface DataViewRequestMessage { } const dynamicDataViewPanels = new Map(); -const dynamicDataViewInstances = new WeakMap(); -const listViewGenerations = new WeakMap(); -let dynamicDataViewReloadRevision = 0; +const dynamicDataViewStateGenerations = new WeakMap(); +const documentGenerations = new WeakMap(); +let documentGenerationRevision = 0; function escapeHtml(text: string): string { const map: Record = { @@ -237,18 +238,18 @@ function escapeHtml(text: string): string { function registerDataViewPanel( panel: vscode.WebviewPanel, key: string, viewId: string, sessionId: string | null, - instance?: number, + stateGeneration?: number, ): void { // The panel's webview getter throws once onDidDispose fires. const webview = panel.webview; dynamicDataViewPanels.set(key, panel); - if (instance !== undefined) { - dynamicDataViewInstances.set(panel, instance); + if (stateGeneration !== undefined) { + dynamicDataViewStateGenerations.set(panel, stateGeneration); } panel.onDidDispose(() => { - listViewGenerations.delete(webview); - const currentInstance = dynamicDataViewInstances.get(panel); - dynamicDataViewInstances.delete(panel); + documentGenerations.delete(webview); + const currentStateGeneration = dynamicDataViewStateGenerations.get(panel); + dynamicDataViewStateGenerations.delete(panel); if (dynamicDataViewPanels.get(key) !== panel) { return; } @@ -257,16 +258,22 @@ function registerDataViewPanel( if (sessions.get(sessionId ?? '')?.requester) { return; } void sessionRequest({ method: 'dataview_dispose', - params: { view_id: viewId, instance: currentInstance }, + 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 = (requestId: number, ok: boolean, result?: unknown, error?: string) => { + 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, @@ -276,7 +283,9 @@ function attachDynamicDataViewBridge(panel: vscode.WebviewPanel, viewId: string, 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; } @@ -289,7 +298,7 @@ function attachDynamicDataViewBridge(panel: vscode.WebviewPanel, viewId: string, if (!result || !Array.isArray(result.columns) || typeof result.totalRows !== 'number') { throw new Error('Invalid dataview_init response'); } - postResponse(msg.requestId, true, result); + postResponse(msg.documentGeneration, msg.requestId, true, result); return; } @@ -309,13 +318,13 @@ function attachDynamicDataViewBridge(panel: vscode.WebviewPanel, viewId: string, typeof result.totalUnfiltered !== 'number') { throw new Error('Invalid dataview_page response'); } - 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)); } }); } @@ -1031,7 +1040,7 @@ export async function showDataView( source: string, type: string, title: string, file: string, viewer: string, viewId?: string, navigation?: ListViewNavigation, sessionId: string | null = activeSession?.sessionId ?? null, - instance?: number, + 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 ?? '')}`); @@ -1041,13 +1050,12 @@ export async function showDataView( if (viewId) { const existing = dynamicDataViewPanels.get(panelKey); if (existing) { - if (instance !== undefined) { - dynamicDataViewInstances.set(existing, instance); + if (stateGeneration !== undefined) { + dynamicDataViewStateGenerations.set(existing, stateGeneration); } existing.title = title; existing.reveal(existing.viewColumn, true); - const content = await getTableHtml(existing.webview, undefined, title); - existing.webview.html = `${content}\n`; + existing.webview.html = await getTableHtml(existing.webview, undefined, title); return; } } @@ -1065,7 +1073,7 @@ export async function showDataView( }); panel.iconPath = new UriIcon('open-preview'); if (viewId) { - registerDataViewPanel(panel, panelKey, viewId, sessionId, instance); + registerDataViewPanel(panel, panelKey, viewId, sessionId, stateGeneration); attachDynamicDataViewBridge(panel, viewId, sessionId); } const content = await getTableHtml(panel.webview, file || undefined, title); @@ -1074,8 +1082,8 @@ export async function showDataView( if (viewId) { const existing = dynamicDataViewPanels.get(panelKey); if (existing) { - if (instance !== undefined) { - dynamicDataViewInstances.set(existing, instance); + if (stateGeneration !== undefined) { + dynamicDataViewStateGenerations.set(existing, stateGeneration); } existing.title = title; existing.reveal(existing.viewColumn, true); @@ -1099,13 +1107,13 @@ export async function showDataView( }); panel.iconPath = new UriIcon('preview'); if (viewId) { - registerDataViewPanel(panel, panelKey, viewId, sessionId, instance); + registerDataViewPanel(panel, panelKey, viewId, sessionId, stateGeneration); const webview = panel.webview; webview.onDidReceiveMessage(async (message: { message?: string; index?: number; start?: number; requestId?: number; - path?: number[]; generation?: number; + path?: number[]; documentGeneration?: number; }) => { - if (message.generation !== listViewGenerations.get(webview)) { + if (message.documentGeneration !== documentGenerations.get(webview)) { return; } if (!Array.isArray(message.path) || !message.path.every(index => Number.isSafeInteger(index) && index > 0)) { @@ -1117,7 +1125,7 @@ export async function showDataView( 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.generation !== listViewGenerations.get(webview)) { + if (message.documentGeneration !== documentGenerations.get(webview)) { return; } const navigation = result && typeof result === 'object' && Array.isArray(result.breadcrumbs) @@ -1127,7 +1135,7 @@ export async function showDataView( } void webview.postMessage({ message: 'listview/navigation', - generation: message.generation, + documentGeneration: message.documentGeneration, requestId: message.requestId, navigation, error: result ? undefined : 'Unable to open this item. Check the R session and try again.', @@ -1138,12 +1146,12 @@ export async function showDataView( method: 'workspace_children', params: { view_id: viewId, start: message.start, path: message.path }, }, sessionId) as { children?: unknown; next_start?: number | null } | undefined; - if (message.generation !== listViewGenerations.get(webview)) { + if (message.documentGeneration !== documentGenerations.get(webview)) { return; } void webview.postMessage({ message: 'listview/page', - generation: message.generation, + documentGeneration: message.documentGeneration, requestId: message.requestId, ...page, error: Array.isArray(page?.children) ? undefined : 'Unable to load items. Check the R session and try again.', @@ -1167,6 +1175,8 @@ export async function showDataView( 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 ` @@ -1344,6 +1354,7 @@ export async function getTableHtml(webview: Webview, file: string | undefined, t @@ -2258,7 +2271,7 @@ async function handleNotification(message: Record, socket: IpcS params.view_id ? String(params.view_id) : undefined, params.navigation as ListViewNavigation | undefined, socket._sessionId ?? null, - typeof params.instance === 'number' ? params.instance : undefined, + typeof params.state_generation === 'number' ? params.state_generation : undefined, ); } } diff --git a/src/test/suite/dataViewerSessions.test.ts b/src/test/suite/dataViewerSessions.test.ts index 457732dcc..42e2bbeac 100644 --- a/src/test/suite/dataViewerSessions.test.ts +++ b/src/test/suite/dataViewerSessions.test.ts @@ -11,7 +11,7 @@ import { mockExtensionContext } from '../common/mockvscode'; interface Request { id: number; method: string; - params?: { view_id?: string; instance?: number }; + params?: { view_id?: string; state_generation?: number }; } interface Client { @@ -138,12 +138,12 @@ suite('Viewer session ownership', () => { } async function open( - client: Client, source: 'list' | 'table', existing?: Panel, instance = 1, + 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`, instance, + 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); @@ -154,8 +154,8 @@ suite('Viewer session ownership', () => { } async function send(panel: Panel, message: Record): Promise { - const generation = Number(/const generation = (\d+)/.exec(panel.panel.webview.html)?.[1]); - await panel.receive({ generation, requestId: 1, path: [], start: 1, ...message }); + 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'); @@ -194,7 +194,7 @@ suite('Viewer session ownership', () => { 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?.instance), + 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); diff --git a/src/test/suite/listViewerPanels.test.ts b/src/test/suite/listViewerPanels.test.ts index ad757b28d..4aeafd8a2 100644 --- a/src/test/suite/listViewerPanels.test.ts +++ b/src/test/suite/listViewerPanels.test.ts @@ -56,13 +56,13 @@ suite('List viewer panels', () => { 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 generation = Number(/const generation = (\d+)/.exec(panel.webview.html)?.[1]); + 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', requestId: 1 })] : ['listview/page', 'listview/navigate'].map(message => listener({ - message, generation, requestId: 1, path: [], start: 1, + message, documentGeneration, requestId: 1, path: [], start: 1, })); panel.dispose(); await Promise.all(pending); @@ -140,24 +140,64 @@ suite('List viewer panels', () => { } as unknown as vscode.WebviewPanel; panels.push(panel); sandbox.stub(vscode.window, 'createWebviewPanel').returns(panel); - const generation = () => Number(/const generation = (\d+)/.exec(panel.webview.html)?.[1]); - await session.showDataView('list', 'json', 'x', '', 'Two', 'test-list-generation'); + 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, generation: generation(), requestId: 1, path: [], start: 1 }; + 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-generation'); + 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', generation: generation(), requestId: 2, path: [], start: 1 }); + await receive({ message: 'listview/page', documentGeneration: documentGeneration(), requestId: 2, path: [], start: 1 }); sinon.assert.calledOnce(postMessage); - const response = postMessage.firstCall.args[0] as { generation: number }; - assert.strictEqual(response.generation, generation()); + 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', () => { From 13997ce2e5a2e852c0b0eaef44b6a4dabcf99300 Mon Sep 17 00:00:00 2001 From: Fred-Wu <4111978+Fred-Wu@users.noreply.github.com> Date: Mon, 5 Oct 2026 00:15:30 +1100 Subject: [PATCH 20/30] fix(viewer): update runtime start test arguments --- sess/inst/tinytest/test-listview.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sess/inst/tinytest/test-listview.R b/sess/inst/tinytest/test-listview.R index 252a46a17..abb071211 100644 --- a/sess/inst/tinytest/test-listview.R +++ b/sess/inst/tinytest/test-listview.R @@ -20,7 +20,7 @@ local({ lapply(pipe, close) }, add = TRUE) runtime$con <- pipe[[2L]] - sess:::runtime_start(use_rstudioapi = FALSE, use_httpgd = FALSE, use_jgd = FALSE) + 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) From 906376552490e4e92d55706d5d72d2a6a0cdc14e Mon Sep 17 00:00:00 2001 From: Fred-Wu <4111978+Fred-Wu@users.noreply.github.com> Date: Mon, 5 Oct 2026 00:19:03 +1100 Subject: [PATCH 21/30] fix(viewer): update viewer lifecycle test --- sess/inst/tinytest/test-ipc.R | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sess/inst/tinytest/test-ipc.R b/sess/inst/tinytest/test-ipc.R index 8ed00b95d..372e34f0a 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)), From 2afc534a69db372e24069762bfdf743b8b2f7907 Mon Sep 17 00:00:00 2001 From: Fred-Wu <4111978+Fred-Wu@users.noreply.github.com> Date: Mon, 5 Oct 2026 00:24:02 +1100 Subject: [PATCH 22/30] fix(viewer): update document generation test --- src/test/suite/listViewer.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/suite/listViewer.test.ts b/src/test/suite/listViewer.test.ts index d0e9e6ace..5ec49ffa0 100644 --- a/src/test/suite/listViewer.test.ts +++ b/src/test/suite/listViewer.test.ts @@ -26,7 +26,7 @@ class Element { interface Request { message: string; - generation: number; + documentGeneration: number; requestId: number; path: number[]; index?: number; @@ -280,7 +280,7 @@ suite('List viewer', () => { test('ignores old viewer responses and hides open buttons for unavailable items', () => { const { root, messages, reply } = createViewer(); - reply(messages[0], { generation: 6, children: [{ label: 'stale' }] }); + reply(messages[0], { documentGeneration: 6, children: [{ label: 'stale' }] }); assert.strictEqual(root.children[0].childElementCount, 0); reply(messages[0], { children: [ { label: 'removed', viewable: false }, From 15066ab90522b535c25827a227c78cd718abdb03 Mon Sep 17 00:00:00 2001 From: Fred-Wu <4111978+Fred-Wu@users.noreply.github.com> Date: Mon, 5 Oct 2026 01:01:41 +1100 Subject: [PATCH 23/30] fix(viewer): cover pending data viewer request --- src/test/suite/listViewerPanels.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/suite/listViewerPanels.test.ts b/src/test/suite/listViewerPanels.test.ts index 4aeafd8a2..0b84e18ec 100644 --- a/src/test/suite/listViewerPanels.test.ts +++ b/src/test/suite/listViewerPanels.test.ts @@ -60,7 +60,9 @@ suite('List viewer panels', () => { 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', requestId: 1 })] + ? [listener({ + message: 'dataview/request', action: 'page', documentGeneration, requestId: 1, + })] : ['listview/page', 'listview/navigate'].map(message => listener({ message, documentGeneration, requestId: 1, path: [], start: 1, })); From b2d72f819b3b2ffac14b2df30195b607cd6bffde Mon Sep 17 00:00:00 2001 From: "4111978+Fred-Wu@users.noreply.github.com" <4111978+Fred-Wu@users.noreply.github.com> Date: Mon, 5 Oct 2026 01:51:18 +1100 Subject: [PATCH 24/30] Bumped sess IPC to version 2 --- sess/R/handlers.R | 2 +- sess/R/hooks.R | 3 +- sess/R/server.R | 2 +- sess/README.md | 19 +++++++++++-- .../tinytest/test-interactive-large-table.R | 3 ++ sess/inst/tinytest/test-session-contract.R | 2 +- src/interactive/backends/sessBridge.ts | 2 +- src/interactive/manager.ts | 13 +++++++++ src/session.ts | 4 +-- src/test/suite/dataViewerSessions.test.ts | 24 +++++++++++++++- src/test/suite/interactiveEditor.test.ts | 28 +++++++++++++++++++ src/test/suite/session.test.ts | 20 ++++++------- src/test/suite/workspaceViewer.test.ts | 19 +++++++++++++ src/workspaceViewer.ts | 4 +++ 14 files changed, 124 insertions(+), 21 deletions(-) diff --git a/sess/R/handlers.R b/sess/R/handlers.R index e00907e62..bc96216ed 100644 --- a/sess/R/handlers.R +++ b/sess/R/handlers.R @@ -845,7 +845,7 @@ dataview_get_state <- function(view_id, refresh = FALSE) { !identical(state$total_rows, current$total_rows)) { current$live <- TRUE current$revision <- revision - current$instance <- state$instance + current$state_generation <- state$state_generation state <- current .sess_env$dataviews[[view_id]] <- state } diff --git a/sess/R/hooks.R b/sess/R/hooks.R index a76ae3d55..549c59ab6 100644 --- a/sess/R/hooks.R +++ b/sess/R/hooks.R @@ -116,7 +116,8 @@ runtime_start <- function(use_rstudioapi = TRUE, } 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)) } diff --git a/sess/R/server.R b/sess/R/server.R index 6fb92ca9d..1882ad2f0 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(), diff --git a/sess/README.md b/sess/README.md index f2b948687..c999fe1ad 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-interactive-large-table.R b/sess/inst/tinytest/test-interactive-large-table.R index 8e1e9ff09..45799fead 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-session-contract.R b/sess/inst/tinytest/test-session-contract.R index d19ecee83..3518957fc 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/interactive/backends/sessBridge.ts b/src/interactive/backends/sessBridge.ts index 3d1e7baae..d19e50cdf 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 57c0c4098..692753deb 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,18 @@ 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 viewer = util.config().get>('session.viewers.viewColumn')?.view ?? 'Two'; + if (viewer !== 'Disable' && params.source && params.type && params.title) { + await session.showDataView( + String(params.source), String(params.type), String(params.title), String(params.file ?? ''), viewer, + params.view_id ? String(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); } diff --git a/src/session.ts b/src/session.ts index 160410b15..2e5553f48 100644 --- a/src/session.ts +++ b/src/session.ts @@ -100,7 +100,7 @@ let info: SessionInfo; export let globalPipePath: string | undefined; export let workspaceFile: string; -const SESS_PROTOCOL_VERSION = 1; +const SESS_PROTOCOL_VERSION = 2; const sessions = new Map(); const documentSessions = new Map(); @@ -255,7 +255,7 @@ function registerDataViewPanel( } dynamicDataViewPanels.delete(key); // Interactive transcripts retain this handle after the expanded viewer closes. - if (sessions.get(sessionId ?? '')?.requester) { return; } + if (currentStateGeneration === undefined && sessions.get(sessionId ?? '')?.requester) { return; } void sessionRequest({ method: 'dataview_dispose', params: { view_id: viewId, state_generation: currentStateGeneration }, diff --git a/src/test/suite/dataViewerSessions.test.ts b/src/test/suite/dataViewerSessions.test.ts index 42e2bbeac..495676bdd 100644 --- a/src/test/suite/dataViewerSessions.test.ts +++ b/src/test/suite/dataViewerSessions.test.ts @@ -127,7 +127,7 @@ suite('Viewer session ownership', () => { socket.once('error', reject); }); notify(client, 'attach', { - protocol_version: 1, session_id: id, host: 'viewer-test-host', pid: id, + 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 && @@ -201,6 +201,28 @@ suite('Viewer session ownership', () => { 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'); diff --git a/src/test/suite/interactiveEditor.test.ts b/src/test/suite/interactiveEditor.test.ts index e1d344d0f..566c82541 100644 --- a/src/test/suite/interactiveEditor.test.ts +++ b/src/test/suite/interactiveEditor.test.ts @@ -1179,6 +1179,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/session.test.ts b/src/test/suite/session.test.ts index 9194d4730..75ecb71d9 100644 --- a/src/test/suite/session.test.ts +++ b/src/test/suite/session.test.ts @@ -276,7 +276,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'); @@ -315,7 +315,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', @@ -651,7 +651,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`); @@ -709,7 +709,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: '' }, @@ -866,7 +866,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', @@ -914,7 +914,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', @@ -949,7 +949,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', @@ -997,11 +997,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(); } diff --git a/src/test/suite/workspaceViewer.test.ts b/src/test/suite/workspaceViewer.test.ts index 8bec4cfc1..ab0c4e13d 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 a1583253a..78cae3a71 100644 --- a/src/workspaceViewer.ts +++ b/src/workspaceViewer.ts @@ -496,6 +496,10 @@ export async function loadWorkspace(): Promise { export async function viewItem(node: GlobalEnvItem): Promise { 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 }, From be2dc6a7a6f89d037f3e33c392d67d417c3b27f8 Mon Sep 17 00:00:00 2001 From: "4111978+Fred-Wu@users.noreply.github.com" <4111978+Fred-Wu@users.noreply.github.com> Date: Mon, 5 Oct 2026 10:18:36 +1100 Subject: [PATCH 25/30] refactor(viewer): use packaged codicons and consolidate icon notices Replace copied List Viewer control SVGs with @vscode/codicons. Bundle its CSS, font, and licenses. Move attribution from README into third-party notices covering Codicons, existing viewer tab icons, and the R logo. --- .vscodeignore | 12 ++++++ README.md | 4 -- ThirdPartyNotices.txt | 32 ++++++++++++++ esbuild.js | 6 ++- images/icons/arrow-left.svg | 3 -- images/icons/chevron-down.svg | 3 -- images/icons/chevron-right.svg | 3 -- images/icons/dark/open-preview-codicon.svg | 1 - images/icons/light/open-preview-codicon.svg | 1 - package.json | 1 + pnpm-lock.yaml | 8 ++++ src/listViewer.ts | 9 ++-- src/session.ts | 47 ++++----------------- src/test/suite/listViewer.test.ts | 2 +- src/test/suite/listViewerPanels.test.ts | 4 +- 15 files changed, 75 insertions(+), 61 deletions(-) create mode 100644 ThirdPartyNotices.txt delete mode 100644 images/icons/arrow-left.svg delete mode 100644 images/icons/chevron-down.svg delete mode 100644 images/icons/chevron-right.svg delete mode 100644 images/icons/dark/open-preview-codicon.svg delete mode 100644 images/icons/light/open-preview-codicon.svg diff --git a/.vscodeignore b/.vscodeignore index 9837514aa..2dd953f64 100644 --- a/.vscodeignore +++ b/.vscodeignore @@ -3,11 +3,17 @@ !README.md !CHANGELOG.md !LICENSE +<<<<<<< HEAD !R/*.R !R/cppProperties/** !R/help/** !R/rmarkdown/** !R/template/** +======= +!ThirdPartyNotices.txt +!R/** +!sess/** +>>>>>>> 109dc25 (refactor(viewer): use packaged codicons and consolidate icon notices) !html/**/*.ejs !html/**/*.css !html/**/*.js @@ -18,4 +24,10 @@ !dist/**/*.js !dist/**/*.css !dist/**/*.ejs +<<<<<<< HEAD !dist/resources/sess/** +======= +!dist/resources/codicon.ttf +!dist/resources/LICENSE +!dist/resources/LICENSE-CODE +>>>>>>> 109dc25 (refactor(viewer): use packaged codicons and consolidate icon notices) diff --git a/README.md b/README.md index 5f2c13bf1..a226fd305 100644 --- a/README.md +++ b/README.md @@ -138,7 +138,3 @@ Call `View(x)` to open a table. The viewer loads rows on demand and applies colu ## Persistent R Interactive This experimental feature provides native VS Code Interactive windows with independent plain-R or arf sessions, including adoption of existing arf sessions over Remote SSH. See the [setup and usage guide](https://github.com/REditorSupport/vscode-R/wiki/R-Interactive) for session switching, persistence, rich outputs, and platform requirements. - -## Icon attribution - -The list viewer’s navigation and chevron icons are from Microsoft’s [VS Code Codicons](https://github.com/microsoft/vscode-codicons), licensed under [CC BY 4.0](https://creativecommons.org/licenses/by/4.0/). The icon artwork is unchanged. diff --git a/ThirdPartyNotices.txt b/ThirdPartyNotices.txt new file mode 100644 index 000000000..a81ca18c7 --- /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 29df03330..cfa0d554b 100644 --- a/esbuild.js +++ b/esbuild.js @@ -12,7 +12,11 @@ function copyResources() { const resources = [ './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/images/icons/arrow-left.svg b/images/icons/arrow-left.svg deleted file mode 100644 index 91773a7ec..000000000 --- a/images/icons/arrow-left.svg +++ /dev/null @@ -1,3 +0,0 @@ - - \ No newline at end of file diff --git a/images/icons/chevron-down.svg b/images/icons/chevron-down.svg deleted file mode 100644 index 389873c46..000000000 --- a/images/icons/chevron-down.svg +++ /dev/null @@ -1,3 +0,0 @@ - - \ No newline at end of file diff --git a/images/icons/chevron-right.svg b/images/icons/chevron-right.svg deleted file mode 100644 index d08afa0b5..000000000 --- a/images/icons/chevron-right.svg +++ /dev/null @@ -1,3 +0,0 @@ - - \ No newline at end of file diff --git a/images/icons/dark/open-preview-codicon.svg b/images/icons/dark/open-preview-codicon.svg deleted file mode 100644 index 5dc0bba6b..000000000 --- a/images/icons/dark/open-preview-codicon.svg +++ /dev/null @@ -1 +0,0 @@ - diff --git a/images/icons/light/open-preview-codicon.svg b/images/icons/light/open-preview-codicon.svg deleted file mode 100644 index 9e363217a..000000000 --- a/images/icons/light/open-preview-codicon.svg +++ /dev/null @@ -1 +0,0 @@ - diff --git a/package.json b/package.json index b7ea83415..abd3e8a22 100644 --- a/package.json +++ b/package.json @@ -2566,6 +2566,7 @@ "typescript": "~5.8.3" }, "dependencies": { + "@vscode/codicons": "0.0.46-24", "ag-grid-community": "^36.2.0", "cheerio": "1.0.0-rc.12", "crypto": "^1.0.1", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a9078eafa..98e84f028 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -8,6 +8,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 @@ -749,6 +752,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'} @@ -3252,6 +3258,8 @@ snapshots: transitivePeerDependencies: - supports-color + '@vscode/codicons@0.0.46-24': {} + '@vscode/test-cli@0.0.12': dependencies: '@types/mocha': 10.0.10 diff --git a/src/listViewer.ts b/src/listViewer.ts index f680ebe9a..a1aaa09e8 100644 --- a/src/listViewer.ts +++ b/src/listViewer.ts @@ -46,7 +46,7 @@ export function getListViewerScript(documentGeneration: number, initial: ListVie navigation.breadcrumbs.forEach((crumb, index) => { if (index) { const separator = document.createElement('span'); - separator.className = 'breadcrumb-separator'; + separator.className = 'codicon codicon-chevron-right'; separator.setAttribute('aria-hidden', 'true'); breadcrumbs.appendChild(separator); } @@ -119,7 +119,7 @@ export function getListViewerScript(documentGeneration: number, initial: ListVie row.className = 'item'; if (!vector) { const arrow = document.createElement('span'); - arrow.className = 'arrow'; + arrow.className = expandable ? 'arrow codicon codicon-chevron-right' : 'arrow'; arrow.setAttribute('aria-hidden', 'true'); row.appendChild(arrow); } @@ -136,7 +136,10 @@ export function getListViewerScript(documentGeneration: number, initial: ListVie const button = document.createElement('button'); button.title = 'View'; button.setAttribute('aria-label', 'View ' + item.label); - button.innerHTML = document.getElementById('view-icon').innerHTML; + 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(); diff --git a/src/session.ts b/src/session.ts index 2e5553f48..a01dc9125 100644 --- a/src/session.ts +++ b/src/session.ts @@ -1103,7 +1103,7 @@ export async function showDataView( enableScripts: true, enableFindWidget: true, retainContextWhenHidden: true, - localResourceRoots: [Uri.file(extensionContext.asAbsolutePath('images/icons'))], + localResourceRoots: [Uri.file(resDir)], }); panel.iconPath = new UriIcon('preview'); if (viewId) { @@ -1876,17 +1876,8 @@ export function getListHtml( ): string { const documentGeneration = ++documentGenerationRevision; documentGenerations.set(webview, documentGeneration); - const icon = new UriIcon('open-preview-codicon'); - const darkIcon = webview.asWebviewUri(icon.dark).toString(); - const lightIcon = webview.asWebviewUri(icon.light).toString(); - const chevronIcon = webview.asWebviewUri( - Uri.file(extensionContext.asAbsolutePath('images/icons/chevron-right.svg')) - ).toString(); - const backIcon = webview.asWebviewUri( - Uri.file(extensionContext.asAbsolutePath('images/icons/arrow-left.svg')) - ).toString(); - const expandedChevronIcon = webview.asWebviewUri( - Uri.file(extensionContext.asAbsolutePath('images/icons/chevron-down.svg')) + const codicons = webview.asWebviewUri( + Uri.file(path.join(resDir, 'codicon.css')) ).toString(); return ` @@ -1896,6 +1887,7 @@ export function getListHtml( ${escapeHtml(title)} +
-