Skip to content

Commit cb3734f

Browse files
fix(terminal): resolve profile path[] to first existing candidate (#119)
VS Code selects the first terminal-profile path candidate that exists on disk; mirror that instead of always taking index 0, falling back to the first non-empty candidate when none exist. Addresses CodeRabbit review on #277.
1 parent 4f45c7c commit cb3734f

2 files changed

Lines changed: 40 additions & 4 deletions

File tree

src/integrations/terminal/Terminal.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { existsSync } from "fs"
2+
13
import * as vscode from "vscode"
24
import pWaitFor from "p-wait-for"
35

@@ -277,9 +279,12 @@ export class Terminal extends BaseTerminal {
277279
return undefined
278280
}
279281

280-
// A `path` may be a single string or an array of candidate paths (VS Code
281-
// picks the first that exists). We pass the first candidate to createTerminal.
282-
const pathValue = Array.isArray(profile.path) ? profile.path[0] : profile.path
282+
// A `path` may be a single string or an array of candidate paths. VS Code
283+
// picks the first candidate that exists on disk, so mirror that: prefer the
284+
// first existing path, otherwise fall back to the first non-empty candidate.
285+
const candidates = Array.isArray(profile.path) ? profile.path : [profile.path]
286+
const nonEmpty = candidates.filter((p): p is string => typeof p === "string" && p.length > 0)
287+
const pathValue = nonEmpty.find((p) => existsSync(p)) ?? nonEmpty[0]
283288

284289
if (!pathValue) {
285290
// Profiles defined only by `source` (e.g. "PowerShell") can't be mapped to

src/integrations/terminal/__tests__/TerminalProfile.spec.ts

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
// npx vitest run src/integrations/terminal/__tests__/TerminalProfile.spec.ts
22

3+
import { existsSync } from "fs"
4+
35
import * as vscode from "vscode"
46

57
import { Terminal } from "../Terminal"
@@ -9,6 +11,12 @@ vi.mock("execa", () => ({
911
execa: vi.fn(),
1012
}))
1113

14+
vi.mock("fs", () => ({
15+
existsSync: vi.fn(() => false),
16+
}))
17+
18+
const mockedExistsSync = existsSync as unknown as ReturnType<typeof vi.fn>
19+
1220
describe("Terminal inline terminal profile (#119)", () => {
1321
// VS Code's getConfiguration/createTerminal are overloaded, so the precise
1422
// spy MockInstance type isn't worth fighting in a test — `any` keeps it simple.
@@ -44,6 +52,9 @@ describe("Terminal inline terminal profile (#119)", () => {
4452

4553
beforeEach(() => {
4654
createTerminalSpy = vi.spyOn(vscode.window, "createTerminal").mockImplementation(() => mockTerminal())
55+
// Default: no candidate path exists on disk unless a test says otherwise.
56+
mockedExistsSync.mockReset()
57+
mockedExistsSync.mockReturnValue(false)
4758
// Reset to default (unset) before each test.
4859
Terminal.setTerminalProfile(undefined)
4960
})
@@ -117,14 +128,34 @@ describe("Terminal inline terminal profile (#119)", () => {
117128
})
118129
})
119130

120-
it("uses the first path candidate when path is an array", () => {
131+
it("picks the first existing path candidate when path is an array", () => {
121132
stubProfiles({
122133
windows: {
123134
"Git Bash": {
124135
path: ["C:\\missing\\bash.exe", "C:\\Program Files\\Git\\bin\\bash.exe"],
125136
},
126137
},
127138
})
139+
// Only the second candidate exists on disk; VS Code would pick it.
140+
mockedExistsSync.mockImplementation((p: string) => p === "C:\\Program Files\\Git\\bin\\bash.exe")
141+
142+
Terminal.setTerminalProfile("Git Bash")
143+
144+
expect(Terminal.getProfileShell("win32")).toEqual({
145+
shellPath: "C:\\Program Files\\Git\\bin\\bash.exe",
146+
shellArgs: undefined,
147+
})
148+
})
149+
150+
it("falls back to the first non-empty candidate when none of the paths exist", () => {
151+
stubProfiles({
152+
windows: {
153+
"Git Bash": {
154+
path: ["C:\\missing\\bash.exe", "C:\\also-missing\\bash.exe"],
155+
},
156+
},
157+
})
158+
// existsSync defaults to false for every candidate.
128159

129160
Terminal.setTerminalProfile("Git Bash")
130161

0 commit comments

Comments
 (0)