feat(terminal): allow choosing the terminal profile for inline execution (#119)#277
Conversation
proyectoauraorg
requested review from
JamesRobert20,
edelauna,
hannesrudolph,
navedmerchant and
taltas
as code owners
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (32)
✅ Files skipped from review due to trivial changes (10)
🚧 Files skipped from review as they are similar to previous changes (19)
📝 WalkthroughWalkthroughAdds a selectable VS Code terminal profile for the inline terminal: schema and message types, profile storage and resolution, extension↔webview plumbing for discovery and persistence, settings UI with profile dropdown, tests for backend/frontend behavior, and translations across locales. ChangesTerminal profile selection feature
Sequence Diagram(s)sequenceDiagram
participant SettingsView
participant TerminalSettings
participant Extension
participant Terminal
participant VSCodeConfig
SettingsView->>TerminalSettings: pass terminalProfile prop
TerminalSettings->>Extension: postMessage(type: "requestTerminalProfiles")
Extension->>VSCodeConfig: read terminal.integrated.profiles.{windows,osx,linux}
VSCodeConfig-->>Extension: profile lists
Extension-->>TerminalSettings: postMessage(type: "terminalProfiles", profiles)
User->>TerminalSettings: select profile
TerminalSettings->>Extension: setCachedStateField("terminalProfile", name|undefined)
Extension->>Terminal: Terminal.setTerminalProfile(name|undefined)
Terminal->>VSCodeConfig: resolve configured profile -> shellPath/shellArgs/env
Terminal->>Terminal: create inline terminal with resolved options
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
webview-ui/src/components/settings/__tests__/TerminalSettings.profile.spec.tsx (1)
129-135: ⚡ Quick winAdd a save-path regression for “Default” profile clearing
This test stops at
setCachedStateField. It won’t catch serialization drops whenSettingsViewpostsupdateSettings. Add one integration assertion that clicking Save emits a clearable value forterminalProfile(not an omitted field).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webview-ui/src/components/settings/__tests__/TerminalSettings.profile.spec.tsx` around lines 129 - 135, The test currently only asserts setCachedStateField when selecting the "__default__" option but doesn’t verify what SettingsView posts on Save, allowing serialization regressions to omit the field; after firing the existing click on screen.getByTestId("option-__default__"), simulate clicking the Save control (e.g. the Save button used by SettingsView) and add an assertion that the mocked updateSettings/postMessage handler was called with a payload that explicitly includes terminalProfile: null (i.e. a clearable value) rather than omitting terminalProfile, referencing the existing setup and setCachedStateField helpers and the SettingsView updateSettings/postMessage entry point.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/integrations/terminal/BaseTerminal.ts`:
- Around line 306-308: The setter currently validates using profile.trim() but
stores the original untrimmed string; update BaseTerminal.setTerminalProfile to
persist a normalized value (use const normalized = profile?.trim();) and set
BaseTerminal.terminalProfile to normalized if normalized.length > 0, otherwise
undefined, so stored profile keys match lookups (reference:
BaseTerminal.setTerminalProfile and BaseTerminal.terminalProfile).
In `@src/integrations/terminal/Terminal.ts`:
- Around line 269-272: Terminal.getProfileShell() currently picks
profile.path[0] without checking filesystem existence; change it to iterate the
array when profile.path is an array and choose the first path that exists (use
fs.existsSync or the project's async existence helper) before passing to
createTerminal, while preserving fallback when no candidate exists; update unit
tests that assume index-0 selection for terminal.integrated.profiles.*.path[] to
assert selection of the first existing candidate.
In `@webview-ui/src/components/settings/SettingsView.tsx`:
- Line 400: When resetting settings to Default in SettingsView, don't leave
terminalProfile as undefined (which gets omitted during serialization);
explicitly assign a clear value such as an empty string (e.g. terminalProfile:
'') or a "Default" sentinel when building the settings object (the code paths
around terminalProfile in SettingsView / the reset handler), so the saved JSON
contains a persistent, clear value instead of omitting the key.
---
Nitpick comments:
In
`@webview-ui/src/components/settings/__tests__/TerminalSettings.profile.spec.tsx`:
- Around line 129-135: The test currently only asserts setCachedStateField when
selecting the "__default__" option but doesn’t verify what SettingsView posts on
Save, allowing serialization regressions to omit the field; after firing the
existing click on screen.getByTestId("option-__default__"), simulate clicking
the Save control (e.g. the Save button used by SettingsView) and add an
assertion that the mocked updateSettings/postMessage handler was called with a
payload that explicitly includes terminalProfile: null (i.e. a clearable value)
rather than omitting terminalProfile, referencing the existing setup and
setCachedStateField helpers and the SettingsView updateSettings/postMessage
entry point.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: db59c226-79a8-4714-b752-3545454c95be
📒 Files selected for processing (30)
packages/types/src/global-settings.tspackages/types/src/vscode-extension-host.tssrc/core/webview/ClineProvider.tssrc/core/webview/webviewMessageHandler.tssrc/integrations/terminal/BaseTerminal.tssrc/integrations/terminal/Terminal.tssrc/integrations/terminal/__tests__/TerminalProfile.spec.tswebview-ui/src/components/settings/SettingsView.tsxwebview-ui/src/components/settings/TerminalSettings.tsxwebview-ui/src/components/settings/__tests__/SettingsView.change-detection.spec.tsxwebview-ui/src/components/settings/__tests__/TerminalSettings.profile.spec.tsxwebview-ui/src/context/ExtensionStateContext.tsxwebview-ui/src/i18n/locales/ca/settings.jsonwebview-ui/src/i18n/locales/de/settings.jsonwebview-ui/src/i18n/locales/en/settings.jsonwebview-ui/src/i18n/locales/es/settings.jsonwebview-ui/src/i18n/locales/fr/settings.jsonwebview-ui/src/i18n/locales/hi/settings.jsonwebview-ui/src/i18n/locales/id/settings.jsonwebview-ui/src/i18n/locales/it/settings.jsonwebview-ui/src/i18n/locales/ja/settings.jsonwebview-ui/src/i18n/locales/ko/settings.jsonwebview-ui/src/i18n/locales/nl/settings.jsonwebview-ui/src/i18n/locales/pl/settings.jsonwebview-ui/src/i18n/locales/pt-BR/settings.jsonwebview-ui/src/i18n/locales/ru/settings.jsonwebview-ui/src/i18n/locales/tr/settings.jsonwebview-ui/src/i18n/locales/vi/settings.jsonwebview-ui/src/i18n/locales/zh-CN/settings.jsonwebview-ui/src/i18n/locales/zh-TW/settings.json
| public static setTerminalProfile(profile: string | undefined): void { | ||
| BaseTerminal.terminalProfile = profile && profile.trim().length > 0 ? profile : undefined | ||
| } |
There was a problem hiding this comment.
Persist the normalized profile name, not the raw input.
Line 307 validates with trim() but stores the untrimmed string. A value like " Git Bash " passes validation but won’t match profile keys later.
Suggested fix
public static setTerminalProfile(profile: string | undefined): void {
- BaseTerminal.terminalProfile = profile && profile.trim().length > 0 ? profile : undefined
+ const normalized = profile?.trim()
+ BaseTerminal.terminalProfile = normalized && normalized.length > 0 ? normalized : undefined
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public static setTerminalProfile(profile: string | undefined): void { | |
| BaseTerminal.terminalProfile = profile && profile.trim().length > 0 ? profile : undefined | |
| } | |
| public static setTerminalProfile(profile: string | undefined): void { | |
| const normalized = profile?.trim() | |
| BaseTerminal.terminalProfile = normalized && normalized.length > 0 ? normalized : undefined | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/integrations/terminal/BaseTerminal.ts` around lines 306 - 308, The setter
currently validates using profile.trim() but stores the original untrimmed
string; update BaseTerminal.setTerminalProfile to persist a normalized value
(use const normalized = profile?.trim();) and set BaseTerminal.terminalProfile
to normalized if normalized.length > 0, otherwise undefined, so stored profile
keys match lookups (reference: BaseTerminal.setTerminalProfile and
BaseTerminal.terminalProfile).
| terminalZshOhMy, | ||
| terminalZshP10k, | ||
| terminalZdotdir, | ||
| terminalProfile, |
There was a problem hiding this comment.
Persist a clear value for terminalProfile when resetting to Default
Line 400 can silently fail to clear a previously saved profile. terminalProfile becomes undefined for “Default”, and your own comment (Line 378) notes undefined is omitted during serialization.
💡 Suggested fix
- terminalProfile,
+ terminalProfile: terminalProfile ?? "",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| terminalProfile, | |
| terminalProfile: terminalProfile ?? "", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@webview-ui/src/components/settings/SettingsView.tsx` at line 400, When
resetting settings to Default in SettingsView, don't leave terminalProfile as
undefined (which gets omitted during serialization); explicitly assign a clear
value such as an empty string (e.g. terminalProfile: '') or a "Default" sentinel
when building the settings object (the code paths around terminalProfile in
SettingsView / the reset handler), so the saved JSON contains a persistent,
clear value instead of omitting the key.
edelauna
left a comment
edelauna
left a comment
There was a problem hiding this comment.
Nice this a good first pass - could you add a screenshot / screen recording of the terminal in action?
| useMount(() => vscode.postMessage({ type: "getVSCodeSetting", setting: "terminal.integrated.inheritEnv" })) | ||
| useMount(() => { | ||
| vscode.postMessage({ type: "getVSCodeSetting", setting: "terminal.integrated.inheritEnv" }) | ||
| PROFILE_SETTING_KEYS.forEach((setting) => vscode.postMessage({ type: "getVSCodeSetting", setting })) |
There was a problem hiding this comment.
Could we route these through a small allowlisted request that returns only profile names? The existing getVSCodeSetting handler reads any key supplied by the webview, and this dropdown only needs sanitized names.
There was a problem hiding this comment.
Done in 4fcbb4b5 — added a dedicated allowlisted requestTerminalProfiles message. The extension now reads terminal.integrated.profiles.* server-side and returns only the sanitized profile names (terminalProfiles → profiles: string[]), so the dropdown no longer routes through the generic getVSCodeSetting handler. The webview no longer supplies arbitrary setting keys for this. 🦘
| if (!pathValue) { | ||
| // Profiles defined only by `source` (e.g. "PowerShell") can't be mapped to | ||
| // a shell path here, so we fall back to the default terminal. | ||
| console.warn( | ||
| `[Terminal] Terminal profile "${profileName}" has no resolvable "path"; using default terminal.`, | ||
| ) | ||
| return undefined | ||
| } | ||
|
|
||
| const shellArgs = Array.isArray(profile.args) | ||
| ? profile.args | ||
| : typeof profile.args === "string" | ||
| ? [profile.args] | ||
| : undefined | ||
|
|
||
| return { shellPath: pathValue, shellArgs } |
There was a problem hiding this comment.
Do we want this to preserve the selected VS Code profile more fully? Profiles can include source and env, but this only returns shellPath/shellArgs, so some profiles shown in the dropdown may fall back or lose required environment.
There was a problem hiding this comment.
Done in 4fcbb4b5 — getProfileShell now also returns the profile's env (sanitized to string | null values; a null unsets the variable), and createTerminal merges it on top of the base env so profile-specific environment isn't lost. Source-only profiles (no resolvable path) still fall back to the default, since fully resolving a source to a shell path needs VS Code internals — I kept that behavior with a clearer comment. Added a test covering env preservation + sanitization.
| "profile": { | ||
| "label": "Inline terminal profile", | ||
| "default": "Default (VS Code default shell)", | ||
| "description": "Choose which VS Code terminal profile the inline terminal uses. Pick a UTF-8 shell such as Git Bash if the default shell garbles non-ASCII output (e.g. on Windows with a GBK code page). Leave on Default to keep VS Code's default behavior." |
There was a problem hiding this comment.
Can we clarify this copy against the nearby Use Inline Terminal description? That text says inline mode bypasses shell profiles, while this new setting says the inline terminal uses a VS Code terminal profile.
There was a problem hiding this comment.
Clarified in 4fcbb4b5 (en + es): the copy now states the Inline Terminal profile only selects the shell binary (path/args/env), while the Inline Terminal still bypasses shell integration, prompts, and plugins — so it no longer reads as contradicting the "Use Inline Terminal" description.
Thanks for the review! Here's the terminal profile selector in action: The new Inline terminal profile dropdown (Settings → Terminal → Advanced) set to zsh. 
I'm also working through your inline comments — the allowlisted profile-name request, fuller VS Code profile preservation, the copy clarification, and the updateSettings / startup-hydration tests. Will push those in a follow-up. 🦘 |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/integrations/terminal/Terminal.ts (1)
280-283:⚠️ Potential issue | 🟠 Major | ⚡ Quick winResolve
path[]using the first existing candidate, not index0.Current logic always picks
profile.path[0]. If that path is missing but a later candidate exists, profile resolution fails unnecessarily.In VS Code terminal profile settings (`terminal.integrated.profiles.windows|osx|linux`), when `path` is an array, does VS Code select the first existing executable path candidate?Proposed fix
+import { existsSync } from "fs" import * as vscode from "vscode" import pWaitFor from "p-wait-for" @@ - const pathValue = Array.isArray(profile.path) ? profile.path[0] : profile.path + const candidates = Array.isArray(profile.path) ? profile.path : [profile.path] + const firstNonEmpty = candidates.find((p): p is string => typeof p === "string" && p.length > 0) + const pathValue = + candidates.find((p): p is string => typeof p === "string" && p.length > 0 && existsSync(p)) ?? firstNonEmpty🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/integrations/terminal/Terminal.ts` around lines 280 - 283, The code in Terminal.ts currently picks profile.path[0] unconditionally which fails if that candidate doesn't exist; update the resolution for profile.path (used to compute pathValue passed to createTerminal) to iterate candidates and pick the first existing executable path (e.g., check each string in profile.path with fs.existsSync or fs.promises.access) and fall back to the original string when profile.path is not an array; ensure you update the variable referenced as pathValue and preserve existing behavior when no candidate exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/webview/webviewMessageHandler.ts`:
- Around line 1523-1538: The code currently iterates ["windows","osx","linux"]
and aggregates terminal profile names across all platforms; change it to only
read profiles for the active runtime platform so we don't surface non-resolvable
names. Detect the active platform (e.g. using process.platform and map
"win32"→"windows","darwin"→"osx","linux"→"linux"), then call
vscode.workspace.getConfiguration("terminal.integrated.profiles").get<Record<string,
unknown>>(mappedPlatform) and add keys from that single object into the existing
names Set before calling provider.postMessageToWebview({ type:
"terminalProfiles", profiles: Array.from(names).sort() }).
---
Duplicate comments:
In `@src/integrations/terminal/Terminal.ts`:
- Around line 280-283: The code in Terminal.ts currently picks profile.path[0]
unconditionally which fails if that candidate doesn't exist; update the
resolution for profile.path (used to compute pathValue passed to createTerminal)
to iterate candidates and pick the first existing executable path (e.g., check
each string in profile.path with fs.existsSync or fs.promises.access) and fall
back to the original string when profile.path is not an array; ensure you update
the variable referenced as pathValue and preserve existing behavior when no
candidate exists.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f6372979-a8cc-46a4-bc65-be490c0584dc
📒 Files selected for processing (10)
packages/types/src/vscode-extension-host.tssrc/core/webview/__tests__/ClineProvider.spec.tssrc/core/webview/__tests__/webviewMessageHandler.spec.tssrc/core/webview/webviewMessageHandler.tssrc/integrations/terminal/Terminal.tssrc/integrations/terminal/__tests__/TerminalProfile.spec.tswebview-ui/src/components/settings/TerminalSettings.tsxwebview-ui/src/components/settings/__tests__/TerminalSettings.profile.spec.tsxwebview-ui/src/i18n/locales/en/settings.jsonwebview-ui/src/i18n/locales/es/settings.json
✅ Files skipped from review due to trivial changes (1)
- webview-ui/src/i18n/locales/es/settings.json
| for (const platform of ["windows", "osx", "linux"] as const) { | ||
| const profiles = vscode.workspace | ||
| .getConfiguration("terminal.integrated.profiles") | ||
| .get<Record<string, unknown>>(platform) | ||
|
|
||
| if (profiles && typeof profiles === "object") { | ||
| for (const name of Object.keys(profiles)) { | ||
| names.add(name) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| await provider.postMessageToWebview({ | ||
| type: "terminalProfiles", | ||
| profiles: Array.from(names).sort(), | ||
| }) |
There was a problem hiding this comment.
Limit returned profile names to the active platform.
Collecting from windows, osx, and linux exposes non-resolvable profiles on the current machine, which can lead to confusing no-op selections.
Proposed fix
- const names = new Set<string>()
-
- for (const platform of ["windows", "osx", "linux"] as const) {
- const profiles = vscode.workspace
- .getConfiguration("terminal.integrated.profiles")
- .get<Record<string, unknown>>(platform)
-
- if (profiles && typeof profiles === "object") {
- for (const name of Object.keys(profiles)) {
- names.add(name)
- }
- }
- }
+ const platform =
+ process.platform === "win32" ? "windows" : process.platform === "darwin" ? "osx" : "linux"
+ const profiles = vscode.workspace
+ .getConfiguration("terminal.integrated.profiles")
+ .get<Record<string, unknown>>(platform)
+ const names =
+ profiles && typeof profiles === "object" ? Object.keys(profiles).sort() : []
await provider.postMessageToWebview({
type: "terminalProfiles",
- profiles: Array.from(names).sort(),
+ profiles: names,
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const platform of ["windows", "osx", "linux"] as const) { | |
| const profiles = vscode.workspace | |
| .getConfiguration("terminal.integrated.profiles") | |
| .get<Record<string, unknown>>(platform) | |
| if (profiles && typeof profiles === "object") { | |
| for (const name of Object.keys(profiles)) { | |
| names.add(name) | |
| } | |
| } | |
| } | |
| await provider.postMessageToWebview({ | |
| type: "terminalProfiles", | |
| profiles: Array.from(names).sort(), | |
| }) | |
| const platform = | |
| process.platform === "win32" ? "windows" : process.platform === "darwin" ? "osx" : "linux" | |
| const profiles = vscode.workspace | |
| .getConfiguration("terminal.integrated.profiles") | |
| .get<Record<string, unknown>>(platform) | |
| const names = | |
| profiles && typeof profiles === "object" ? Object.keys(profiles).sort() : [] | |
| await provider.postMessageToWebview({ | |
| type: "terminalProfiles", | |
| profiles: names, | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/webview/webviewMessageHandler.ts` around lines 1523 - 1538, The code
currently iterates ["windows","osx","linux"] and aggregates terminal profile
names across all platforms; change it to only read profiles for the active
runtime platform so we don't surface non-resolvable names. Detect the active
platform (e.g. using process.platform and map
"win32"→"windows","darwin"→"osx","linux"→"linux"), then call
vscode.workspace.getConfiguration("terminal.integrated.profiles").get<Record<string,
unknown>>(mappedPlatform) and add keys from that single object into the existing
names Set before calling provider.postMessageToWebview({ type:
"terminalProfiles", profiles: Array.from(names).sort() }).
…o-Code-Org#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 Zoo-Code-Org#277.
Add a 'terminalProfile' setting that lets users choose which VS Code terminal profile the inline terminal uses. On Windows the default cmd/PowerShell shell may use a non-UTF-8 code page (e.g. GBK) and garble output; selecting a UTF-8 profile such as Git Bash resolves this. The setting reuses VS Code's terminal profile concept: when set, the profile name is resolved against terminal.integrated.profiles.<platform> to derive shellPath/shellArgs for createTerminal. When empty/unset the default terminal behavior is preserved unchanged. Adds backend unit tests for profile resolution and a webview test for the settings dropdown wiring.
Zoo-Code-Org#119) - Route profile names through a dedicated allowlisted `requestTerminalProfiles` message instead of the generic `getVSCodeSetting` (which reads any key the webview supplies); the extension reads the profiles and returns only names. - Preserve the profile's `env` (sanitized to string/null; null unsets a var), merged onto the base env in createTerminal. - Clarify the setting copy (en + es) vs the 'Use Inline Terminal' description. - Add tests: updateSettings->setTerminalProfile bridge, resolveWebviewView startup hydration, and profile env preservation/sanitization.
…o-Code-Org#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 Zoo-Code-Org#277.
proyectoauraorg
force-pushed
the
feat/119-inline-terminal-profile
branch
from
92f45be to
cb3734f
Compare
|
Thanks for the review @edelauna! Responses: Screenshot/Recording: Will add a screen recording showing the terminal profile switching in action — targeting EOD. Code comments:
|
| </div> | ||
| </div> | ||
| <div className="flex flex-col gap-3 pl-3 border-l-2 border-vscode-button-background"> | ||
| <SearchableSetting |
There was a problem hiding this comment.
Should this block be wrapped in {terminalShellIntegrationDisabled && (...)} so the dropdown is hidden when the user has turned off the inline terminal? The other inline-terminal-only settings (inheritEnv, shellIntegrationTimeout) disappear under the !terminalShellIntegrationDisabled guard at line 213, but the profile picker stays visible and editable even when it has no effect.
The terminal-profile dropdown only affects inline execution, which is active when shell integration is disabled. It previously stayed visible and editable even when inline mode was off, where it has no effect. Guard it with terminalShellIntegrationDisabled (defaulting to shown, matching the checkbox), mirroring the inline-only settings below. Addresses PR Zoo-Code-Org#277 review (edelauna).
|
Done — the profile picker is now guarded by |
2 participants
Summary
On Windows the inline terminal defaults to cmd/PowerShell with a non-UTF-8 charset (e.g. GBK), which garbles output for UTF-8 projects (#119). This adds an opt-in setting to choose which VS Code terminal profile the inline (shell-integration) terminal uses — e.g. Git Bash for UTF-8.
Change
terminalProfilesetting (string; empty/unset = unchanged default behavior).Terminal.getProfileShell()resolves the chosen profile name against VS Code'sterminal.integrated.profiles.<platform>config and passes the derivedshellPath/shellArgstovscode.window.createTerminal. When unset,createTerminalis called exactly as before (no extra keys), so existing behavior — and the existingTerminalRegistrytests — are unaffected.Tests & i18n
TerminalProfile.spec.ts: profile resolution (unset→default, Git Bash on Windows, array/string args, osx section, not-found and source-only fallbacks,createTerminalintegration).TerminalSettings.profile.spec.tsx: mount fetches profile lists, nosetCachedStateFieldon init, selecting a profile sets the name, "Default" maps to undefined.terminal.profile.*added for all 18 locales.Closes #119
Summary by CodeRabbit
New Features
Localization
Tests