-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Fix Bash path interpretation for Windows terminal #26125
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,8 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| // bug fix - Bash-VS Code path errors | ||
| import * as vscode from 'vscode'; | ||
| import { inject, injectable } from 'inversify'; | ||
| import { CancellationToken, Disposable, Event, EventEmitter, Terminal, TerminalShellExecution } from 'vscode'; | ||
| import '../../common/extensions'; | ||
|
|
@@ -67,15 +69,56 @@ export class TerminalService implements ITerminalService, Disposable { | |
| }); | ||
| } | ||
| } | ||
| public async sendCommand(command: string, args: string[], _?: CancellationToken): Promise<void> { | ||
| await this.ensureTerminal(); | ||
| const text = this.terminalHelper.buildCommandForTerminal(this.terminalShellType, command, args); | ||
|
|
||
| // fixed Path interpretation bug between Bash and VS Code | ||
| // fixed Path interpretation bug between Bash and VS Code | ||
| public async sendCommand(command: string, args: string[] = []): Promise<void> { | ||
| await this.ensureTerminal(); // <-- ADD THIS: Ensures terminal is booted up | ||
|
|
||
| if (!this.options?.hideFromUser) { | ||
| this.terminal!.show(true); | ||
| } | ||
|
|
||
| await this.executeCommand(text, false); | ||
| // Fetch the terminal settings from VS Code | ||
| const terminalSettings = vscode.workspace.getConfiguration('terminal.integrated'); | ||
| const defaultProfile = terminalSettings.get<string>('defaultProfile.windows') || ''; | ||
|
|
||
| // Check if the destination target is a Bash terminal | ||
| const isBashShell = defaultProfile.toLowerCase().includes('bash') || | ||
| command.toLowerCase().includes('bash.exe'); | ||
|
|
||
| let processedCommand = command; | ||
| let processedArgs = [...args]; | ||
|
|
||
| // If running on Windows but targeting Git Bash, swap backslashes to forward slashes! | ||
| if (process.platform === 'win32' && isBashShell) { | ||
| // Fix the executable binary path | ||
| processedCommand = processedCommand.replace(/\\/g, '/'); | ||
| processedCommand = processedCommand.replace(/^([A-Za-z]):/, (_, drive) => `/${drive.toLowerCase()}`); | ||
|
|
||
| // Fix the script file paths being sent as arguments | ||
| processedArgs = processedArgs.map(arg => { | ||
| let safeArg = arg.replace(/\\/g, '/'); | ||
| safeArg = safeArg.replace(/^([A-Za-z]):/, (_, drive) => `/${drive.toLowerCase()}`); | ||
| return safeArg; | ||
| }); | ||
| } | ||
|
|
||
| // Standard VS Code logic to stitch the command and arguments together | ||
| const text = processedArgs.reduce((p, c) => `${p} "${c}"`, processedCommand); | ||
|
|
||
| // Ship the cleanly escaped string to the terminal stream! | ||
| this.terminal!.sendText(text, true); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This replaces |
||
| } | ||
|
|
||
|
|
||
| // Standard VS Code logic to stitch the command and arguments together | ||
| const text = processedArgs.reduce((p, c) => `${p} "${c}"`, processedCommand); | ||
|
|
||
| // Ship the cleanly escaped string to the terminal stream! | ||
| this.terminal!.sendText(text, true); | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
These duplicated statements are directly in the class body, reference method-local variables, and introduce an extra closing brace, so the file cannot compile. Remove this entire duplicated block. |
||
|
|
||
| /** @deprecated */ | ||
| public async sendText(text: string): Promise<void> { | ||
| await this.ensureTerminal(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replacing backslashes in every argument treats opaque values such as regular expressions or Python snippets as paths and can corrupt them. Restrict conversion to known path operands or move normalization into an API that can identify paths.