diff --git a/CHANGELOG.md b/CHANGELOG.md index 80fbd17ff..567016af4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ Bug Fixes: ## 1.23.52 Bug Fixes: +- Fix "Compile File" terminal closing immediately, hiding compilation output. The terminal now stays open until the user presses a key + Fix POSIX shell escape handling in `compile_commands.json` parsing so that escaped quotes (e.g., `\"`) are correctly interpreted when compiling single files. [#4896](https://github.com/microsoft/vscode-cmake-tools/issues/4896) - Fix `cmake.exportCompileCommandsFile` set to `false` still passing `-DCMAKE_EXPORT_COMPILE_COMMANDS:BOOL=FALSE` instead of omitting the flag entirely, which caused CMake warnings for projects with `LANGUAGES NONE`. [#4893](https://github.com/microsoft/vscode-cmake-tools/issues/4893) - Fix regression where Visual Studio kits with an existing Ninja-based build cache would fail due to a generator mismatch. Ninja is now preferred again when available, stale VS kits derive the correct generator at runtime as a fallback, and the build directory is auto-cleaned on generator mismatches. [#4890](https://github.com/microsoft/vscode-cmake-tools/issues/4890) diff --git a/src/drivers/cmakeDriver.ts b/src/drivers/cmakeDriver.ts index 9276a7702..eeae19406 100644 --- a/src/drivers/cmakeDriver.ts +++ b/src/drivers/cmakeDriver.ts @@ -566,6 +566,7 @@ export abstract class CMakeDriver implements vscode.Disposable { const writeEmitter = new vscode.EventEmitter(); const closeEmitter = new vscode.EventEmitter(); let activeProcess: proc.Subprocess | undefined; + let processExitCode: number | undefined; const pty: vscode.Pseudoterminal = { onDidWrite: writeEmitter.event, @@ -581,6 +582,7 @@ export abstract class CMakeDriver implements vscode.Disposable { activeProcess.result.then(result => { activeProcess = undefined; const retc = result.retc ?? 0; + processExitCode = retc; if (retc !== 0) { writeEmitter.fire(localize('compile.finished.with.error', 'Compilation finished with error(s).') + '\r\n'); @@ -588,11 +590,18 @@ export abstract class CMakeDriver implements vscode.Disposable { writeEmitter.fire(localize('compile.finished.successfully', 'Compilation finished successfully.') + '\r\n'); } - closeEmitter.fire(retc); + // Keep terminal open so user can see the output. Only close when + // user presses a key or closes the terminal manually. + // See https://github.com/microsoft/vscode-cmake-tools/issues/4896 + writeEmitter.fire(localize('press.any.key.to.close', + 'Press any key to close the terminal...') + '\r\n'); }, (e: any) => { activeProcess = undefined; + // Use positive exit code per VS Code PTY API convention + processExitCode = 1; writeEmitter.fire((e?.message ?? String(e)) + '\r\n'); - closeEmitter.fire(-1); + writeEmitter.fire(localize('press.any.key.to.close', + 'Press any key to close the terminal...') + '\r\n'); }); }, close: () => { @@ -602,6 +611,13 @@ export abstract class CMakeDriver implements vscode.Disposable { void util.termProc(activeProcess.child); activeProcess = undefined; } + }, + handleInput: (_data: string) => { + // Close the terminal when the user presses any key after the + // process has finished. If the process is still running, ignore input. + if (processExitCode !== undefined) { + closeEmitter.fire(processExitCode); + } } }; diff --git a/src/shlex.ts b/src/shlex.ts index 387d99966..7418cfe87 100644 --- a/src/shlex.ts +++ b/src/shlex.ts @@ -5,6 +5,11 @@ export interface ShlexOptions { /** * Splits a string into an iterable of tokens, similar to how a shell would parse arguments. * Handles quoting and escaping according to the specified mode ('windows' or 'posix'). + * + * In POSIX mode, backslash escapes the following character outside of quotes (the backslash + * is consumed). Inside double quotes, only $, `, ", \, and newline can be escaped. + * Inside single quotes, backslash has no special meaning. + * * @param str The string to split into tokens. * @param opt Optional options for splitting. If not provided, defaults to the platform-specific mode ('windows' or 'posix'). * @returns An iterable of tokens. @@ -18,20 +23,39 @@ export function* split(str: string, opt?: ShlexOptions): Iterable { const escapeChars = '\\'; let escapeChar: string | undefined; let token: string[] = []; - let isSubQuote: boolean = false; + let quoteChar: string | undefined; // Track which quote character we're inside (for POSIX) for (let i = 0; i < str.length; ++i) { const char = str.charAt(i); if (escapeChar) { if (char === '\n') { - // Do nothing - } else if (escapeChars.includes(char)) { - token.push(char); + // Line continuation: consume both backslash and newline + } else if (opt.mode === 'posix') { + // POSIX escape handling + if (quoteChar === "'") { + // Inside single quotes: backslash has no special meaning + token.push(escapeChar, char); + } else if (quoteChar === '"') { + // Inside double quotes: only certain chars can be escaped + // $, `, ", \, and newline + if (char === '$' || char === '`' || char === '"' || char === '\\') { + token.push(char); + } else { + token.push(escapeChar, char); + } + } else { + // Outside quotes: backslash escapes any character + token.push(char); + } } else { - token.push(escapeChar, char); // Append escape sequence + // Windows mode: only backslash can be escaped + if (escapeChars.includes(char)) { + token.push(char); + } else { + token.push(escapeChar, char); + } } - // We parsed an escape seq. Reset to no escape escapeChar = undefined; continue; } @@ -42,10 +66,10 @@ export function* split(str: string, opt?: ShlexOptions): Iterable { continue; } - if (isSubQuote) { - if (quoteChars.includes(char)) { - // End of sub-quoted token - isSubQuote = false; + if (quoteChar) { + if (char === quoteChar) { + // End of quoted section + quoteChar = undefined; token.push(char); continue; } @@ -54,13 +78,13 @@ export function* split(str: string, opt?: ShlexOptions): Iterable { } if (quoteChars.includes(char)) { - // Beginning of a subquoted token - isSubQuote = true; + // Beginning of a quoted section + quoteChar = char; token.push(char); continue; } - if (!isSubQuote && /[\t \n\r\f]/.test(char)) { + if (/[\t \n\r\f]/.test(char)) { if (token.length > 0) { yield token.join(''); } diff --git a/test/unit-tests/backend/shlex.test.ts b/test/unit-tests/backend/shlex.test.ts new file mode 100644 index 000000000..5a6d07bda --- /dev/null +++ b/test/unit-tests/backend/shlex.test.ts @@ -0,0 +1,166 @@ +import { expect } from 'chai'; +import * as shlex from '@cmt/shlex'; + +function splitWin(str: string): string[] { + return [...shlex.split(str, { mode: 'windows' })]; +} + +function splitUnix(str: string): string[] { + return [...shlex.split(str, { mode: 'posix' })]; +} + +suite('shlex testing (backend)', () => { + suite('Windows shell splitting', () => { + test('Basic token splitting', () => { + const pairs: [string, string[]][] = [ + ['foo', ['foo']], + ['foo bar', ['foo', 'bar']], + ['', []], + ['""', ['""']], + [' Something ', ['Something']], + ['foo bar', ['foo', 'bar']], + ['"C:\\Program Files" something', ['"C:\\Program Files"', 'something']], + ['foo "" bar', ['foo', '""', 'bar']], + [`\"'fo o'\" bar`, [`\"'fo o'\"`, 'bar']], + [`'quote arg'`, [`'quote`, `arg'`]], + ['" fail"', ['" fail"']] + ]; + + for (const [cmd, expected] of pairs) { + expect(splitWin(cmd)).to.deep.equal(expected, `Bad parse for string: ${cmd}`); + } + }); + + test('Windows mode: backslash only escapes backslash, not quotes', () => { + // -DAWESOME=\"\\\"'fo o' bar\\\"\" should preserve all escapes + const cmd = `-DAWESOME=\"\\\"'fo o' bar\\\"\"`; + expect(splitWin(cmd)).to.deep.equal([`-DAWESOME=\"\\\"'fo o' bar\\\"\"`]); + }); + }); + + suite('Posix shell splitting', () => { + test('Basic token splitting', () => { + const pairs: [string, string[]][] = [ + ['foo', ['foo']], + ['foo bar', ['foo', 'bar']], + ['', []], + ['""', ['""']], + [`''`, [`''`]], + [' Something ', ['Something']], + ['foo bar', ['foo', 'bar']], + ['"C:\\Program Files" something', ['"C:\\Program Files"', 'something']], + ['foo "" bar', ['foo', '""', 'bar']], + [`"fo o" bar`, [`"fo o"`, 'bar']], + [`'quote arg'`, [`'quote arg'`]], + ['" fail"', ['" fail"']] + ]; + + for (const [cmd, expected] of pairs) { + expect(splitUnix(cmd)).to.deep.equal(expected, `Bad parse for string: ${cmd}`); + } + }); + }); + + suite('Posix escape handling outside quotes', () => { + test('\\\\ -> single backslash', () => { + expect(splitUnix('foo\\\\bar')).to.deep.equal(['foo\\bar']); + }); + + test('\\" -> literal quote', () => { + expect(splitUnix('-DFOO=\\"bar\\"')).to.deep.equal(['-DFOO="bar"']); + }); + + test('\\n (not newline char) -> n', () => { + expect(splitUnix('foo\\nbar')).to.deep.equal(['foonbar']); + }); + + test('Escaped space keeps token together', () => { + expect(splitUnix('foo\\ bar')).to.deep.equal(['foo bar']); + }); + + test('Line continuation (actual newline) - backslash+newline consumed', () => { + expect(splitUnix('foo\\\nbar')).to.deep.equal(['foobar']); + }); + + test('Compile command with escaped quotes (issue #4896)', () => { + expect(splitUnix('-DIMGUI_USER_CONFIG=\\"frontends/sdl/imgui/sa2_imconfig.h\\"')) + .to.deep.equal(['-DIMGUI_USER_CONFIG="frontends/sdl/imgui/sa2_imconfig.h"']); + }); + }); + + suite('Posix escape handling inside double quotes', () => { + test('Inside double quotes: \\\\ -> \\', () => { + expect(splitUnix('"foo\\\\bar"')).to.deep.equal(['"foo\\bar"']); + }); + + test('Inside double quotes: \\" -> "', () => { + expect(splitUnix('"foo\\"bar"')).to.deep.equal(['"foo"bar"']); + }); + + test('Inside double quotes: \\n (not escapable) -> \\n preserved', () => { + expect(splitUnix('"foo\\nbar"')).to.deep.equal(['"foo\\nbar"']); + }); + + test('Inside double quotes: \\$ -> $', () => { + expect(splitUnix('"foo\\$bar"')).to.deep.equal(['"foo$bar"']); + }); + + test('Inside double quotes: \\` -> `', () => { + expect(splitUnix('"foo\\`bar"')).to.deep.equal(['"foo`bar"']); + }); + }); + + suite('Posix escape handling inside single quotes', () => { + test('Inside single quotes: backslash has no special meaning', () => { + expect(splitUnix(`'foo\\bar'`)).to.deep.equal([`'foo\\bar'`]); + }); + + test('Inside single quotes: \\" stays as \\"', () => { + expect(splitUnix(`'foo\\"bar'`)).to.deep.equal([`'foo\\"bar'`]); + }); + + test('Inside single quotes: \\\\ stays as \\\\', () => { + expect(splitUnix(`'foo\\\\bar'`)).to.deep.equal([`'foo\\\\bar'`]); + }); + }); + + // Cross-platform regression tests (macOS uses POSIX mode) + suite('Cross-platform: mixed quotes (macOS/Linux POSIX)', () => { + test('Double quote inside single quotes is literal', () => { + // 'a"b' -> single token with literal " + expect(splitUnix(`'a"b'`)).to.deep.equal([`'a"b'`]); + }); + + test('Single quote inside double quotes is literal', () => { + // "a'b" -> single token with literal ' + expect(splitUnix(`"a'b"`)).to.deep.equal([`"a'b"`]); + }); + + test('Adjacent mixed quotes form single token', () => { + // "foo"'bar' -> foo and bar concatenated + expect(splitUnix(`"foo"'bar'`)).to.deep.equal([`"foo"'bar'`]); + }); + + test('Complex mixed quoting with spaces', () => { + // "foo bar"'baz qux' -> single token + expect(splitUnix(`"foo bar"'baz qux'`)).to.deep.equal([`"foo bar"'baz qux'`]); + }); + }); + + suite('Cross-platform: Windows path preservation', () => { + test('Windows paths in Windows mode preserve backslashes', () => { + expect(splitWin('"C:\\Users\\test\\file.cpp"')).to.deep.equal(['"C:\\Users\\test\\file.cpp"']); + }); + + test('Windows trailing backslash in quoted path', () => { + // "C:\path\" arg - the backslash before closing quote escapes the quote in Windows mode, + // so the entire rest of the string becomes one token (this is existing behavior) + expect(splitWin('"C:\\path\\" arg')).to.deep.equal(['"C:\\path\\" arg']); + }); + + test('UNC paths in Windows mode', () => { + // In Windows mode, \\ becomes \ (backslash escapes backslash) + expect(splitWin('"\\\\server\\share\\file.cpp"')).to.deep.equal(['"\\server\\share\\file.cpp"']); + }); + }); +}); diff --git a/test/unit-tests/shlex.test.ts b/test/unit-tests/shlex.test.ts index 22067027f..caf35b144 100644 --- a/test/unit-tests/shlex.test.ts +++ b/test/unit-tests/shlex.test.ts @@ -23,6 +23,7 @@ suite('shlex testing', () => { [`\"'fo o'\" bar`, [`\"'fo o'\"`, 'bar']], [`'quote arg'`, [`'quote`, `arg'`]], ['" fail"', ['" fail"']], + // Windows mode: backslash only escapes backslash, not quotes [`-DAWESOME=\"\\\"'fo o' bar\\\"\"`, [`-DAWESOME=\"\\\"'fo o' bar\\\"\"`]] ]; @@ -44,7 +45,64 @@ suite('shlex testing', () => { [`"fo o" bar`, [`"fo o"`, 'bar']], [`'quote arg'`, [`'quote arg'`]], ['" fail"', ['" fail"']], - [`-DAWESOME=\"\\\"fo o bar\\\"\"`, [`-DAWESOME=\"\\\"fo o bar\\\"\"`]] + // POSIX mode: backslash outside quotes escapes any character (backslash consumed) + // Input: -DAWESOME=\"\"fo o bar\"\" (escaped quotes outside quotes) + // After escape processing: -DAWESOME=""fo o bar"" + // But the inner "" is an empty double-quoted section, so we get: + [`-DAWESOME=\"\"fo o bar\"\"`, [`-DAWESOME=""fo`, 'o', 'bar""']] + ]; + + for (const [cmd, expected] of pairs) { + expect(splitUnix(cmd)).to.eql(expected, `Bad parse for string: ${cmd}`); + } + }); + + test('Posix escape handling outside quotes', () => { + const pairs: [string, string[]][] = [ + // \\ -> single backslash + ['foo\\\\bar', ['foo\\bar']], + // \" -> literal quote + ['-DFOO=\\"bar\\"', ['-DFOO="bar"']], + // \n (not newline char) -> n + ['foo\\nbar', ['foonbar']], + // Escaped space keeps token together + ['foo\\ bar', ['foo bar']], + // Line continuation (actual newline) - backslash+newline consumed + ['foo\\\nbar', ['foobar']], + // Compile command with escaped quotes (issue #4896) + ['-DIMGUI_USER_CONFIG=\\"frontends/sdl/imgui/sa2_imconfig.h\\"', ['-DIMGUI_USER_CONFIG="frontends/sdl/imgui/sa2_imconfig.h"']] + ]; + + for (const [cmd, expected] of pairs) { + expect(splitUnix(cmd)).to.eql(expected, `Bad parse for string: ${cmd}`); + } + }); + + test('Posix escape handling inside double quotes', () => { + const pairs: [string, string[]][] = [ + // Inside double quotes: \\ -> \ + ['"foo\\\\bar"', ['"foo\\bar"']], + // Inside double quotes: \" -> " + ['"foo\\"bar"', ['"foo"bar"']], + // Inside double quotes: \n (not escapable) -> \n preserved + ['"foo\\nbar"', ['"foo\\nbar"']], + // Inside double quotes: \$ -> $ + ['"foo\\$bar"', ['"foo$bar"']], + // Inside double quotes: \` -> ` + ['"foo\\`bar"', ['"foo`bar"']] + ]; + + for (const [cmd, expected] of pairs) { + expect(splitUnix(cmd)).to.eql(expected, `Bad parse for string: ${cmd}`); + } + }); + + test('Posix escape handling inside single quotes', () => { + const pairs: [string, string[]][] = [ + // Inside single quotes: backslash has no special meaning + [`'foo\\bar'`, [`'foo\\bar'`]], + [`'foo\\"bar'`, [`'foo\\"bar'`]], + [`'foo\\\\bar'`, [`'foo\\\\bar'`]] ]; for (const [cmd, expected] of pairs) {