Use PowerShell call operator when shell integration is unavailable#14585
Open
owevertonguedes wants to merge 1 commit into
Open
Use PowerShell call operator when shell integration is unavailable#14585owevertonguedes wants to merge 1 commit into
owevertonguedes wants to merge 1 commit into
Conversation
"Run C/C++ File" builds the command with buildShellCommandLine and sends it via sendText when the terminal has no shell integration. That path quotes a program path containing spaces but never prepends the call operator, so PowerShell evaluates the quoted path as a string literal and echoes it instead of running the program. The shell-integration path already handles this, so extract its PowerShell detection into isPowerShellTerminal() and reuse it in the sendText path. Closes microsoft#14583
Author
|
@microsoft-github-policy-service agree |
Member
|
@owevertonguedes, so are there versions of PowerShell that do not support shell integration, or is there some VS Code bug where it fails to activate? The code changes look reasonable, but I'd like to understand the case where shell integration doesn't work with PowerShell. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes “Run C/C++ File” on PowerShell terminals when shell integration is unavailable by ensuring the command is invoked with the PowerShell call operator so spaced executable paths are executed rather than echoed.
Changes:
- Extracts PowerShell terminal detection into a reusable
isPowerShellTerminal()helper. - Reuses that helper in the no-shell-integration fallback to prefix commands with
&for PowerShell.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
125
to
+128
| const cmdLine: string = buildShellCommandLine('', program, args, true); | ||
| this.terminal.sendText(cmdLine); | ||
| // buildShellCommandLine quotes the path, and PowerShell evaluates a quoted path as a string | ||
| // literal instead of running it, so the call operator is required to invoke it. | ||
| this.terminal.sendText(this.isPowerShellTerminal() ? `& ${cmdLine}` : cmdLine); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #14583
Problem
When "Run C/C++ File" targets a terminal without shell integration,
launchIntegratedTerminalfalls back tobuildShellCommandLine(...)+terminal.sendText(...). That fallback quotes a program path containing spaces but never prepends the PowerShell call operator, so PowerShell evaluates the quoted path as a string literal and echoes it back instead of executing the program:This matches the reported output exactly: had the shell-integration path run, the command would have been rendered as
& "C:\...\main.exe".The shell-integration path already handles PowerShell correctly (added in #14351), but its detection logic was inline and therefore unavailable to the fallback.
buildShellCommandLinealso carries an explicit// TODO: Support launching with PowerShell.Change
isPowerShellTerminal()helper (behavior unchanged, including the "assume PowerShell on Windows when the shell is unknown" fallback).sendTextpath to prepend&for PowerShell.No behavior change for cmd.exe or POSIX shells, or for terminals that do have shell integration.
Testing
tsc --noEmitpasses for the changed file, andeslintis clean.runWithoutDebugging.terminals.test.tsmatrix already covers a spaced executable across theundefined/Command Prompt/PowerShellprofiles, but it only exercises the shell-integration path, which is why this fallback regressed unnoticed.I could not exercise the Windows/PowerShell fallback end-to-end (no Windows machine available), so I would appreciate a careful look at that path in review.