Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
@WardenGnaw had some feedback about dealing with some of the other properties in the launch.json config and shared a commit in MIEngine where these things might have been handled. The cases where the config asks for something other than a local launch of a program should be handled in some way (either fail the launch session or fall back to regular debugging mode). |
| }; | ||
| } | ||
|
|
||
| suite('Run Without Debugging Integration Test', function (): void { |
There was a problem hiding this comment.
Should this be with other tests somewhere (other than common.ts)?
| assert.fail(`Unsupported test platform: ${process.platform}`); | ||
| } | ||
|
|
||
| async function createBreakpointAtReturnStatement(sourceUri: vscode.Uri): Promise<vscode.SourceBreakpoint> { |
There was a problem hiding this comment.
Maybe debugger related helpers should go in Debugger\utils.ts ?
| assert.strictEqual(util.hasMsvcEnvironment(), true, 'MSVC environment not set correctly.'); | ||
| } | ||
|
|
||
| async function compileProgram(workspacePath: string, sourcePath: string, outputPath: string): Promise<void> { |
There was a problem hiding this comment.
What about other needed compiler args?
We also kick off builds in cppBuildTaskProvider.ts . It considers some other details there, such as other compiler args and ensuring ansi color coding is used in the output (which we could also get if we fixed #9681 , so we could use node-pty ).
|
|
||
| function runProcess(command: string, args: string[], cwd: string, env?: NodeJS.ProcessEnv): Promise<ProcessResult> { | ||
| return new Promise((resolve, reject) => { | ||
| const child = cp.spawn(command, args, { cwd, env }); |
There was a problem hiding this comment.
If you're quoting args and using spawn, you'll probably want to pass shell: true. Default would be false, and it wouldn't expect there to be escaping/quoting. We also have spawnChildProcess and execChildProcess. There is also direct use of exec after the call to buildShellCommandLine in cppBuildTaskProvider.ts.
I remember dealing with the whole shell-escaping can of worms once upon a time, and at least one additional spawn codepath (In Garrett's Process.ts) has snuck in since. I believe the right expectation is:
- If we receive a command line from a user, that is a shell concept and must already contain shell quoting/escaping if required. We shouldn't add/remove quoting/escaping if we need to pass it on as a shell command line. If we need to parse it into individual args, we need to remove the quoting/escaping.
- If we receive individual arguments (in an array), that is not a shell concept, and those arguments should not contain shell quoting/escaping. If we need to pass it on a command line through a shell, we need to add quoting/escaping.
4 participants
Fixes: #1201
Supports both the Play button in the editor (run active file) and configurations specified in launch.json. macOS support needs to be tested - I don't currently have access to a machine.
Co-authored by GitHub Copilot