Repository navigation
fix: focus the terminal created from the "R Terminal" profile - #1839
davidkane9 wants to merge 2 commits into
Conversation
A terminal created from the contributed profile (the terminal panel's "+" menu) is created by VS Code, not by createRTerm(), so the extension never called show() on it. VS Code does try to focus it, but the extension host fires off the creation without awaiting it ($createContributedProfileTerminal -> createTerminalFromOptions) and then focuses "the last terminal", which with a remote extension host (Codespaces, SSH, the web client) is still the previous terminal. Result: the new R terminal opens without focus, unlike every other profile and unlike "R: Create R Terminal". provideTerminalProfile() now records that it just handed VS Code a terminal, and onDidOpenTerminal focuses the first "R Interactive" terminal that opens while that request is pending (10 s expiry, so an aborted creation cannot steal focus later). Tests cover focus-once, no-pending, and expiry.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Pending requests are not uniquely correlated or counted, allowing overlapping or unrelated R terminals to consume the focus request.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Adds focus handling for R terminals created through the contributed terminal profile.
Changes:
- Tracks pending profile-terminal creation and focuses the opened terminal.
- Registers focus handling during terminal-open events.
- Adds regression tests and changelog documentation.
| File | Description |
|---|---|
src/rTerminal.ts |
Adds pending-request focus logic. |
src/extension.ts |
Connects profile creation and terminal-open handling. |
src/test/suite/terminal.test.ts |
Tests focus, inactivity, and expiry behavior. |
CHANGELOG.md |
Documents the fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| export async function provideTerminalProfile(resource?: vscode.Uri): Promise<vscode.TerminalProfile> { | ||
| const options = await makeTerminalOptions(resource); | ||
| profileTerminalPendingUntil = Date.now() + PROFILE_TERMINAL_PENDING_MS; |
|
Filed the underlying VS Code bug: microsoft/vscode#340185 (contributed-profile terminal creation is not awaited before VS Code focuses the "last" instance). This PR is the extension-side workaround until that lands. |
renkun-ken
left a comment
renkun-ken
left a comment
There was a problem hiding this comment.
Reviewed 07058c3. The workaround addresses the reported focus race, but the opened terminal needs to be correlated with the profile request before focus is taken. I reproduced a separate failure from the overlapping-request case already raised: an unrelated command-created R terminal can consume the request, override its preserve-focus behavior, and leave the actual profile terminal unfocused. Details are inline.
Validation: the 53 tests in terminal.test.ts pass in an isolated VS Code 1.110.0 extension host, and an additional diagnostic test confirms the incorrect event ordering described below. Type checking, lint, and the extension build pass. GitHub CI is also green.
| if (terminal.name !== 'R Interactive') { | ||
| return false; | ||
| } | ||
| profileTerminalPendingUntil = 0; | ||
| terminal.show(); |
There was a problem hiding this comment.
[P2] Match the opened terminal to its profile request before focusing it
R Interactive is also the name used by createRTerm(), including the createRTerm(true) path used by Run Selection and Restart R Terminal. If one of those terminals opens while a profile terminal is still pending on a slow remote host, this handler consumes the profile request and calls show() on the command terminal, overriding its preserve-focus behavior. When the actual profile terminal opens, the deadline has already been cleared, so the reported focus bug remains.
I reproduced this against the PR code by requesting a profile, calling createRTerm(true), and delivering the command terminal's open event first: its show calls are [true] followed by [], and focusProfileTerminal() then returns false for the terminal created from profile.options. This can also happen after an aborted profile creation, throughout the 10-second pending window. Please attach a unique identifier to each profile's terminal options and consume only the corresponding request when that identifier appears in the opened terminal's creationOptions; a name match alone (even with a queue of deadlines) cannot distinguish these paths. Add a regression test where an unrelated R terminal opens before the profile terminal.
Review (renkun-ken, Copilot): a name match could be consumed by an unrelated createRTerm(true) terminal (Run Selection, Restart) — overriding its preserve-focus and leaving the real profile terminal unfocused — and a single deadline could not represent several in-flight requests. Each request is now keyed by the options object handed to VS Code, which VS Code freezes and exposes unchanged as terminal.creationOptions; only the terminal whose creationOptions IS that object is focused, each request is consumed once, and stale requests expire. Regression tests: an unrelated R terminal opening first, several in-flight requests, no request, expiry.
|
Thanks, both points addressed in the follow-up commit. Correlation is now by identity of the options object: New regression tests: an unrelated R terminal opening before the profile terminal ( |
renkun-ken
left a comment
renkun-ken
left a comment
There was a problem hiding this comment.
Reviewed 15fd0a5. The options-identity correlation addresses my previous finding and the overlapping-request issue. I also verified it through a real contributed-profile creation in VS Code 1.110.0: terminal.creationOptions is the same frozen object returned by the provider, and the request is consumed exactly once. The previous createRTerm(true) reproduction now preserves focus and leaves the profile request intact.
One timing regression remains: the deadline is captured before asynchronous options preparation, so a slow preparation can exhaust the focus window before the profile is returned. Details and a deterministic reproduction are inline.
Validation: all 55 tests in terminal.test.ts pass, as do the previous regression reproduction, the slow-preparation diagnostic, and the real profile-creation check. Type checking, lint, and the extension build pass. All GitHub CI checks are green.
| export async function provideTerminalProfile(resource?: vscode.Uri, now: number = Date.now()): Promise<vscode.TerminalProfile> { | ||
| const options = await makeTerminalOptions(resource); | ||
| pendingProfileTerminals.set(options, now + PROFILE_TERMINAL_PENDING_MS); |
There was a problem hiding this comment.
[P2] Start the expiry window after terminal options are ready
The default now = Date.now() is evaluated when provideTerminalProfile() is called, before await makeTerminalOptions(resource). That preparation awaits R-path discovery and, with the session watcher enabled, session-server initialization and discovery-file filesystem operations. Its duration now counts against the 10-second window even though VS Code cannot begin creating this terminal until the profile is returned. Slow setup can therefore return an already-expired request, or leave too little time for the remote creation round trip, and the handler drops the matching terminal without focusing it. The previous implementation sampled the deadline after the await.
I reproduced this without passing the test-only now argument: stub Date.now() to return 1,000 at entry, advance it to 12,000 inside the awaited getRterm() stub, then open a terminal whose creationOptions === profile.options immediately after the provider returns. focusProfileTerminal() returns false and never calls show(). Please calculate the deadline after makeTerminalOptions() resolves (if injecting a clock for tests, evaluate that clock there), and cover time advancing during options preparation.
Problem
Creating an R terminal from the terminal panel's "+" menu → "R Terminal" (the contributed profile) opens the terminal without focus when the extension host is remote — GitHub Codespaces in the browser is where we hit it, and SSH / vscode.dev should behave the same. Every other profile (bash, zsh) takes focus, and so does "R: Create R Terminal" from the Command Palette, which makes the "+" path feel broken.
Cause
On the profile path the terminal is created by VS Code, not by
createRTerm(), so the extension never callsshow(). VS Code does try to focus it (terminalService.createContributedTerminalProfile→setActiveInstanceByIndex(instances.length - 1)→focusWhenReady()), but on the extension-host side$createContributedProfileTerminalcallscreateTerminalFromOptions(...)without awaiting it and returns. With a slow round trip to a remote extension host, "the last instance" at that moment is still the previous terminal; the new one registers afterwards, unfocused. On a local desktop the race is usually won, which is why this is mostly reported from Codespaces.Fix
provideTerminalProfile()records that it just handed VS Code a terminal;onDidOpenTerminalfocuses the firstR Interactiveterminal that opens while that request is pending (10 s expiry, so an aborted creation can never steal focus later). This mirrors whatcreateRTerm()already does for the command path. No new settings.Three tests in
terminal.test.ts: focus exactly once per profile request, nothing without a pending request, and expiry.Verified
pnpm run lint,tsc --noEmit, and the extension test suite locally (389 passing; the two Session Communication timeouts are local-environment only — nosessin the local R library — and unrelated).A separate upstream report to VS Code about the un-awaited creation would let this workaround go away eventually; this PR makes the extension behave correctly in the meantime.