Skip to content

fix: handle Windows cmd encoding for ipconfig and other commands #7804

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

fix: handle Windows cmd encoding for ipconfig and other commands #7804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions src/integrations/terminal/ExecaTerminalProcess.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,25 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
try {
this.isHot = true

// On Windows, wrap the command with chcp to ensure UTF-8 output
let actualCommand = command
if (process.platform === "win32") {
// Set code page to UTF-8 (65001) before running the command
// and restore it afterwards to avoid affecting other programs
Copy link
Contributor Author

@roomote roomote bot Sep 9, 2025

Choose a reason for hiding this comment

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

The comment mentions "restore it afterwards" but I don't see where we're restoring the original code page. Should we either:

  1. Remove this misleading comment, or
  2. Actually implement code page restoration (e.g., chcp 65001 >nul 2>&1 && %s && chcp %original% >nul 2>&1)?

actualCommand = `chcp 65001 >nul 2>&1 && ${command}`
Copy link
Contributor Author

@roomote roomote bot Sep 9, 2025

Choose a reason for hiding this comment

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

Is this intentional? When using VSCode terminal integration, commands get wrapped with chcp twice - once here in ExecaTerminalProcess and once in TerminalProcess. This results in: chcp 65001 >nul 2>&1 && chcp 65001 >nul 2>&1 && ipconfig

Perhaps we should only apply this in ExecaTerminalProcess since TerminalProcess handles VSCode's integrated terminal?

}

this.subprocess = execa({
shell: true,
cwd: this.terminal.getCurrentWorkingDirectory(),
all: true,
env: {
...process.env,
// Ensure UTF-8 encoding for Ruby, CocoaPods, etc.
// Ensure UTF-8 encoding for Ruby, CocoaPods, etc. (Unix-like systems)
LANG: "en_US.UTF-8",
LC_ALL: "en_US.UTF-8",
},
})`${command}`
})`${actualCommand}`

this.pid = this.subprocess.pid

Expand All @@ -74,9 +82,21 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
const rawStream = this.subprocess.iterable({ from: "all", preserveNewlines: true })

// Wrap the stream to ensure all chunks are strings (execa can return Uint8Array)
// On Windows, we need to handle potential encoding issues
const stream = (async function* () {
for await (const chunk of rawStream) {
yield typeof chunk === "string" ? chunk : new TextDecoder().decode(chunk)
if (typeof chunk === "string") {
yield chunk
} else {
// For Windows cmd output, try to decode with UTF-8 first
// If that fails, fall back to Windows-1252 (common Windows encoding)
try {
yield new TextDecoder("utf-8", { fatal: true }).decode(chunk)
} catch {
// Fallback to Windows-1252 if UTF-8 decoding fails
yield new TextDecoder("windows-1252", { fatal: false }).decode(chunk)
Copy link
Contributor Author

@roomote roomote bot Sep 9, 2025

Choose a reason for hiding this comment

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

When the fallback to Windows-1252 occurs, we're silently handling the encoding issue. Would it be helpful to add logging here to track when UTF-8 decoding fails? This could help debug encoding issues in production:

}
}
}
})()

Expand Down
8 changes: 8 additions & 0 deletions src/integrations/terminal/Terminal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,14 @@ export class Terminal extends BaseTerminal {
VTE_VERSION: "0",
}

// On Windows, set the console output code page to UTF-8
// This helps with proper encoding of command outputs like ipconfig
if (process.platform === "win32") {
env.PYTHONIOENCODING = "utf-8"
Copy link
Contributor Author

@roomote roomote bot Sep 9, 2025

Choose a reason for hiding this comment

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

This environment variable only affects new Python processes. Should we document that existing Python processes won't pick up this change? Also, is there a reason we're only setting this for Python and not other interpreters that might have similar issues?

// Note: We can't set CHCP directly here as it's a command, not an env var
// The actual chcp command will be handled in the terminal execution
}

// Set Oh My Zsh shell integration if enabled
if (Terminal.getTerminalZshOhMy()) {
env.ITERM_SHELL_INTEGRATION_INSTALLED = "Yes"
Expand Down
12 changes: 12 additions & 0 deletions src/integrations/terminal/TerminalProcess.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,17 @@ export class TerminalProcess extends BaseTerminalProcess {
(defaultWindowsShellProfile === null ||
(defaultWindowsShellProfile as string)?.toLowerCase().includes("powershell"))

const isCmd =
process.platform === "win32" &&
defaultWindowsShellProfile !== null &&
(defaultWindowsShellProfile as string)?.toLowerCase().includes("cmd")

if (isPowerShell) {
let commandToExecute = command

// Set UTF-8 encoding for PowerShell
commandToExecute = `[Console]::OutputEncoding = [System.Text.Encoding]::UTF8; ${commandToExecute}`
Copy link
Contributor Author

@roomote roomote bot Sep 9, 2025

Choose a reason for hiding this comment

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

Could we add a comment explaining why PowerShell uses while cmd uses ? This would help future maintainers understand the different approaches:

[Console]::OutputEncoding = [System.Text.Encoding]::UTF8;


// Only add the PowerShell counter workaround if enabled
if (Terminal.getPowershellCounter()) {
commandToExecute += ` ; "(Roo/PS Workaround: ${this.terminal.cmdCounter++})" > $null`
Expand All @@ -127,6 +135,10 @@ export class TerminalProcess extends BaseTerminalProcess {
commandToExecute += ` ; start-sleep -milliseconds ${Terminal.getCommandDelay()}`
}

terminal.shellIntegration.executeCommand(commandToExecute)
} else if (isCmd) {
// For Windows cmd, set code page to UTF-8 before executing the command
const commandToExecute = `chcp 65001 >nul 2>&1 && ${command}`
Copy link
Contributor Author

@roomote roomote bot Sep 9, 2025

Choose a reason for hiding this comment

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

Same issue as in ExecaTerminalProcess - this creates duplicate chcp commands when both terminal processes are involved. Should we consolidate this logic to avoid redundancy?

terminal.shellIntegration.executeCommand(commandToExecute)
} else {
terminal.shellIntegration.executeCommand(command)
Expand Down
Loading