Skip to content

Show R session info in data and list viewer tooltips - #1848

Merged
Fred-Wu merged 3 commits into
REditorSupport:mainfrom
Fred-Wu:feature/viewer-source-pid
Oct 8, 2026
Merged

Fred-Wu merged 3 commits into
REditorSupport:mainfrom
Fred-Wu:feature/viewer-source-pid

Conversation

@Fred-Wu

@Fred-Wu Fred-Wu commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Replaces #1828 .
Closes #1827

Summary

PR #1828 aims to identify the source R session of each Data Viewer and List Viewer when multiple sessions are running. However, using one shared status bar for both the focused viewer’s source session and the active R session introduces complex focus and state handling.

This PR replaces #1828 by displaying source session information in a tooltip within each viewer, leaving the status bar’s active-session behavior unchanged.

Adds an info icon to Data Viewer and List Viewer showing the source session’s R version and PID, using the status bar format: R version: PID.

  • Shows details on hover or keyboard focus; Escape dismisses the tooltip.
  • Keeps session details when detached while the R process remains running.
  • Shows R: (not attached) after the source R process exits.
  • Preserves the active session and existing status bar selection behavior.
    Validation: build, TypeScript checks, lint, and 26 focused viewer tests passed. Hover and keyboard dismissal were also verified in a browser.
image image

@Fred-Wu
Fred-Wu marked this pull request as ready for review October 7, 2026 23:26

@eitsupi eitsupi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review by ChatGPT.

I reviewed this with particular attention to the design problems and review findings from #1828. I think this is a substantially better design.

The important improvement is that the two concepts are now kept separate: the status bar continues to represent the active R session, while each Data/List Viewer exposes the session that originally produced that viewer. Because viewer focus no longer participates in status-bar/session selection, the races found in #1828 around workspace refreshes, viewer blur, terminal focus/selection, Interactive activation, and delayed terminal PID resolution are avoided structurally rather than being handled by an increasingly complex focus/state machine.

I also checked the new viewer-session lifecycle handling and tests. The source version/PID is captured independently of later session selection, disconnect alone does not incorrectly imply process exit, confirmed Interactive exits are propagated, and the viewer update does not alter activeSession or the existing status-bar behavior. The shared viewerSession implementation for Data Viewer and List Viewer also looks appropriately scoped.

One minor design concern is that attachViewerSessionBridge() creates a per-viewer timer and uses process.kill(pid, 0) for local process liveness. An event-driven/shared lifecycle mechanism would be cleaner in the longer term and would avoid PID-reuse edge cases. However, given the requirement to distinguish transport detachment from actual R process exit, I don't consider this a blocker for this PR.

Overall, I found no actionable correctness issue in the current implementation. This addresses the original goal with considerably less coupling than #1828, and I think this approach is preferable.

@eitsupi

eitsupi commented Oct 7, 2026

Copy link
Copy Markdown
Member

Since @renkun-ken reviewed #1828, I'd like to wait for @renkun-ken's opinion.

@Fred-Wu

Fred-Wu commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Since @renkun-ken reviewed #1828, I'd like to wait for @renkun-ken's opinion.

If this is accepted, I am thinking where to put that info icon on Plot Viewer and HTML Widget Viewer if they are made session-aware later.

@renkun-ken renkun-ken left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 855b274. I found no actionable correctness issues and support this approach. Keeping the source session in each viewer's tooltip avoids the focus and status-bar reconciliation problems from #1828 while leaving active-session selection unchanged.

The shared process monitor also addresses the earlier per-viewer polling concern: viewers of the same source share one probe, reconnects retain the process identity, transport detachment does not imply exit, and the last viewer releases its monitoring. Confirmed Interactive exits update the tooltip directly, including when the host cannot be probed locally.

Validation: local extension bundling, TypeScript checking, and TypeScript lint passed. All 27 focused viewer tests and 7 process-monitor tests passed, as did the changed Interactive nested-table integration test with two live R sessions. Two additional checks using real VS Code 1.140.0 List Viewer and Data Viewer panels confirmed that terminating their live R source while they are hidden updates the actual tooltip to R: (not attached) when revealed, without changing the active session. CI build, lint, and macOS/Linux/Windows tests are green for this revision.

The full Node suite was not validated locally: sandbox socket restrictions and a missing renv installation prevented several unrelated runtime tests from running successfully.

@Fred-Wu
Fred-Wu merged commit 2a6ab2a into REditorSupport:main Oct 8, 2026
5 checks passed
@Fred-Wu
Fred-Wu deleted the feature/viewer-source-pid branch October 8, 2026 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(Feat): update status bar PID when switching between Data and List Viewers from different sessions

3 participants