Skip to content

Commit 0488016

Browse files
committed
Enhance shlex parsing and add unit tests for POSIX and Windows (#4898)
* enhance shlex parsing and add unit tests for POSIX and Windows modes * fix changelog --------- Co-authored-by: Hannia Valera <[email protected]>
1 parent a5ecb12 commit 0488016

5 files changed

Lines changed: 281 additions & 16 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
## 1.23.52
44

55
Bug Fixes:
6+
- 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)
67
- 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)
78
- 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)
89

src/drivers/cmakeDriver.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,7 @@ export abstract class CMakeDriver implements vscode.Disposable {
566566
const writeEmitter = new vscode.EventEmitter<string>();
567567
const closeEmitter = new vscode.EventEmitter<number>();
568568
let activeProcess: proc.Subprocess | undefined;
569+
let processExitCode: number | undefined;
569570

570571
const pty: vscode.Pseudoterminal = {
571572
onDidWrite: writeEmitter.event,
@@ -581,18 +582,26 @@ export abstract class CMakeDriver implements vscode.Disposable {
581582
activeProcess.result.then(result => {
582583
activeProcess = undefined;
583584
const retc = result.retc ?? 0;
585+
processExitCode = retc;
584586
if (retc !== 0) {
585587
writeEmitter.fire(localize('compile.finished.with.error',
586588
'Compilation finished with error(s).') + '\r\n');
587589
} else {
588590
writeEmitter.fire(localize('compile.finished.successfully',
589591
'Compilation finished successfully.') + '\r\n');
590592
}
591-
closeEmitter.fire(retc);
593+
// Keep terminal open so user can see the output. Only close when
594+
// user presses a key or closes the terminal manually.
595+
// See https://github.com/microsoft/vscode-cmake-tools/issues/4896
596+
writeEmitter.fire(localize('press.any.key.to.close',
597+
'Press any key to close the terminal...') + '\r\n');
592598
}, (e: any) => {
593599
activeProcess = undefined;
600+
// Use positive exit code per VS Code PTY API convention
601+
processExitCode = 1;
594602
writeEmitter.fire((e?.message ?? String(e)) + '\r\n');
595-
closeEmitter.fire(-1);
603+
writeEmitter.fire(localize('press.any.key.to.close',
604+
'Press any key to close the terminal...') + '\r\n');
596605
});
597606
},
598607
close: () => {
@@ -602,6 +611,13 @@ export abstract class CMakeDriver implements vscode.Disposable {
602611
void util.termProc(activeProcess.child);
603612
activeProcess = undefined;
604613
}
614+
},
615+
handleInput: (_data: string) => {
616+
// Close the terminal when the user presses any key after the
617+
// process has finished. If the process is still running, ignore input.
618+
if (processExitCode !== undefined) {
619+
closeEmitter.fire(processExitCode);
620+
}
605621
}
606622
};
607623

src/shlex.ts

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@ export interface ShlexOptions {
55
/**
66
* Splits a string into an iterable of tokens, similar to how a shell would parse arguments.
77
* Handles quoting and escaping according to the specified mode ('windows' or 'posix').
8+
*
9+
* In POSIX mode, backslash escapes the following character outside of quotes (the backslash
10+
* is consumed). Inside double quotes, only $, `, ", \, and newline can be escaped.
11+
* Inside single quotes, backslash has no special meaning.
12+
*
813
* @param str The string to split into tokens.
914
* @param opt Optional options for splitting. If not provided, defaults to the platform-specific mode ('windows' or 'posix').
1015
* @returns An iterable of tokens.
@@ -18,20 +23,39 @@ export function* split(str: string, opt?: ShlexOptions): Iterable<string> {
1823
const escapeChars = '\\';
1924
let escapeChar: string | undefined;
2025
let token: string[] = [];
21-
let isSubQuote: boolean = false;
26+
let quoteChar: string | undefined; // Track which quote character we're inside (for POSIX)
2227

2328
for (let i = 0; i < str.length; ++i) {
2429
const char = str.charAt(i);
2530

2631
if (escapeChar) {
2732
if (char === '\n') {
28-
// Do nothing
29-
} else if (escapeChars.includes(char)) {
30-
token.push(char);
33+
// Line continuation: consume both backslash and newline
34+
} else if (opt.mode === 'posix') {
35+
// POSIX escape handling
36+
if (quoteChar === "'") {
37+
// Inside single quotes: backslash has no special meaning
38+
token.push(escapeChar, char);
39+
} else if (quoteChar === '"') {
40+
// Inside double quotes: only certain chars can be escaped
41+
// $, `, ", \, and newline
42+
if (char === '$' || char === '`' || char === '"' || char === '\\') {
43+
token.push(char);
44+
} else {
45+
token.push(escapeChar, char);
46+
}
47+
} else {
48+
// Outside quotes: backslash escapes any character
49+
token.push(char);
50+
}
3151
} else {
32-
token.push(escapeChar, char); // Append escape sequence
52+
// Windows mode: only backslash can be escaped
53+
if (escapeChars.includes(char)) {
54+
token.push(char);
55+
} else {
56+
token.push(escapeChar, char);
57+
}
3358
}
34-
// We parsed an escape seq. Reset to no escape
3559
escapeChar = undefined;
3660
continue;
3761
}
@@ -42,10 +66,10 @@ export function* split(str: string, opt?: ShlexOptions): Iterable<string> {
4266
continue;
4367
}
4468

45-
if (isSubQuote) {
46-
if (quoteChars.includes(char)) {
47-
// End of sub-quoted token
48-
isSubQuote = false;
69+
if (quoteChar) {
70+
if (char === quoteChar) {
71+
// End of quoted section
72+
quoteChar = undefined;
4973
token.push(char);
5074
continue;
5175
}
@@ -54,13 +78,13 @@ export function* split(str: string, opt?: ShlexOptions): Iterable<string> {
5478
}
5579

5680
if (quoteChars.includes(char)) {
57-
// Beginning of a subquoted token
58-
isSubQuote = true;
81+
// Beginning of a quoted section
82+
quoteChar = char;
5983
token.push(char);
6084
continue;
6185
}
6286

63-
if (!isSubQuote && /[\t \n\r\f]/.test(char)) {
87+
if (/[\t \n\r\f]/.test(char)) {
6488
if (token.length > 0) {
6589
yield token.join('');
6690
}
Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
import { expect } from 'chai';
2+
import * as shlex from '@cmt/shlex';
3+
4+
function splitWin(str: string): string[] {
5+
return [...shlex.split(str, { mode: 'windows' })];
6+
}
7+
8+
function splitUnix(str: string): string[] {
9+
return [...shlex.split(str, { mode: 'posix' })];
10+
}
11+
12+
suite('shlex testing (backend)', () => {
13+
suite('Windows shell splitting', () => {
14+
test('Basic token splitting', () => {
15+
const pairs: [string, string[]][] = [
16+
['foo', ['foo']],
17+
['foo bar', ['foo', 'bar']],
18+
['', []],
19+
['""', ['""']],
20+
[' Something ', ['Something']],
21+
['foo bar', ['foo', 'bar']],
22+
['"C:\\Program Files" something', ['"C:\\Program Files"', 'something']],
23+
['foo "" bar', ['foo', '""', 'bar']],
24+
[`\"'fo o'\" bar`, [`\"'fo o'\"`, 'bar']],
25+
[`'quote arg'`, [`'quote`, `arg'`]],
26+
['" fail"', ['" fail"']]
27+
];
28+
29+
for (const [cmd, expected] of pairs) {
30+
expect(splitWin(cmd)).to.deep.equal(expected, `Bad parse for string: ${cmd}`);
31+
}
32+
});
33+
34+
test('Windows mode: backslash only escapes backslash, not quotes', () => {
35+
// -DAWESOME=\"\\\"'fo o' bar\\\"\" should preserve all escapes
36+
const cmd = `-DAWESOME=\"\\\"'fo o' bar\\\"\"`;
37+
expect(splitWin(cmd)).to.deep.equal([`-DAWESOME=\"\\\"'fo o' bar\\\"\"`]);
38+
});
39+
});
40+
41+
suite('Posix shell splitting', () => {
42+
test('Basic token splitting', () => {
43+
const pairs: [string, string[]][] = [
44+
['foo', ['foo']],
45+
['foo bar', ['foo', 'bar']],
46+
['', []],
47+
['""', ['""']],
48+
[`''`, [`''`]],
49+
[' Something ', ['Something']],
50+
['foo bar', ['foo', 'bar']],
51+
['"C:\\Program Files" something', ['"C:\\Program Files"', 'something']],
52+
['foo "" bar', ['foo', '""', 'bar']],
53+
[`"fo o" bar`, [`"fo o"`, 'bar']],
54+
[`'quote arg'`, [`'quote arg'`]],
55+
['" fail"', ['" fail"']]
56+
];
57+
58+
for (const [cmd, expected] of pairs) {
59+
expect(splitUnix(cmd)).to.deep.equal(expected, `Bad parse for string: ${cmd}`);
60+
}
61+
});
62+
});
63+
64+
suite('Posix escape handling outside quotes', () => {
65+
test('\\\\ -> single backslash', () => {
66+
expect(splitUnix('foo\\\\bar')).to.deep.equal(['foo\\bar']);
67+
});
68+
69+
test('\\" -> literal quote', () => {
70+
expect(splitUnix('-DFOO=\\"bar\\"')).to.deep.equal(['-DFOO="bar"']);
71+
});
72+
73+
test('\\n (not newline char) -> n', () => {
74+
expect(splitUnix('foo\\nbar')).to.deep.equal(['foonbar']);
75+
});
76+
77+
test('Escaped space keeps token together', () => {
78+
expect(splitUnix('foo\\ bar')).to.deep.equal(['foo bar']);
79+
});
80+
81+
test('Line continuation (actual newline) - backslash+newline consumed', () => {
82+
expect(splitUnix('foo\\\nbar')).to.deep.equal(['foobar']);
83+
});
84+
85+
test('Compile command with escaped quotes (issue #4896)', () => {
86+
expect(splitUnix('-DIMGUI_USER_CONFIG=\\"frontends/sdl/imgui/sa2_imconfig.h\\"'))
87+
.to.deep.equal(['-DIMGUI_USER_CONFIG="frontends/sdl/imgui/sa2_imconfig.h"']);
88+
});
89+
});
90+
91+
suite('Posix escape handling inside double quotes', () => {
92+
test('Inside double quotes: \\\\ -> \\', () => {
93+
expect(splitUnix('"foo\\\\bar"')).to.deep.equal(['"foo\\bar"']);
94+
});
95+
96+
test('Inside double quotes: \\" -> "', () => {
97+
expect(splitUnix('"foo\\"bar"')).to.deep.equal(['"foo"bar"']);
98+
});
99+
100+
test('Inside double quotes: \\n (not escapable) -> \\n preserved', () => {
101+
expect(splitUnix('"foo\\nbar"')).to.deep.equal(['"foo\\nbar"']);
102+
});
103+
104+
test('Inside double quotes: \\$ -> $', () => {
105+
expect(splitUnix('"foo\\$bar"')).to.deep.equal(['"foo$bar"']);
106+
});
107+
108+
test('Inside double quotes: \\` -> `', () => {
109+
expect(splitUnix('"foo\\`bar"')).to.deep.equal(['"foo`bar"']);
110+
});
111+
});
112+
113+
suite('Posix escape handling inside single quotes', () => {
114+
test('Inside single quotes: backslash has no special meaning', () => {
115+
expect(splitUnix(`'foo\\bar'`)).to.deep.equal([`'foo\\bar'`]);
116+
});
117+
118+
test('Inside single quotes: \\" stays as \\"', () => {
119+
expect(splitUnix(`'foo\\"bar'`)).to.deep.equal([`'foo\\"bar'`]);
120+
});
121+
122+
test('Inside single quotes: \\\\ stays as \\\\', () => {
123+
expect(splitUnix(`'foo\\\\bar'`)).to.deep.equal([`'foo\\\\bar'`]);
124+
});
125+
});
126+
127+
// Cross-platform regression tests (macOS uses POSIX mode)
128+
suite('Cross-platform: mixed quotes (macOS/Linux POSIX)', () => {
129+
test('Double quote inside single quotes is literal', () => {
130+
// 'a"b' -> single token with literal "
131+
expect(splitUnix(`'a"b'`)).to.deep.equal([`'a"b'`]);
132+
});
133+
134+
test('Single quote inside double quotes is literal', () => {
135+
// "a'b" -> single token with literal '
136+
expect(splitUnix(`"a'b"`)).to.deep.equal([`"a'b"`]);
137+
});
138+
139+
test('Adjacent mixed quotes form single token', () => {
140+
// "foo"'bar' -> foo and bar concatenated
141+
expect(splitUnix(`"foo"'bar'`)).to.deep.equal([`"foo"'bar'`]);
142+
});
143+
144+
test('Complex mixed quoting with spaces', () => {
145+
// "foo bar"'baz qux' -> single token
146+
expect(splitUnix(`"foo bar"'baz qux'`)).to.deep.equal([`"foo bar"'baz qux'`]);
147+
});
148+
});
149+
150+
suite('Cross-platform: Windows path preservation', () => {
151+
test('Windows paths in Windows mode preserve backslashes', () => {
152+
expect(splitWin('"C:\\Users\\test\\file.cpp"')).to.deep.equal(['"C:\\Users\\test\\file.cpp"']);
153+
});
154+
155+
test('Windows trailing backslash in quoted path', () => {
156+
// "C:\path\" arg - the backslash before closing quote escapes the quote in Windows mode,
157+
// so the entire rest of the string becomes one token (this is existing behavior)
158+
expect(splitWin('"C:\\path\\" arg')).to.deep.equal(['"C:\\path\\" arg']);
159+
});
160+
161+
test('UNC paths in Windows mode', () => {
162+
// In Windows mode, \\ becomes \ (backslash escapes backslash)
163+
expect(splitWin('"\\\\server\\share\\file.cpp"')).to.deep.equal(['"\\server\\share\\file.cpp"']);
164+
});
165+
});
166+
});

test/unit-tests/shlex.test.ts

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ suite('shlex testing', () => {
2323
[`\"'fo o'\" bar`, [`\"'fo o'\"`, 'bar']],
2424
[`'quote arg'`, [`'quote`, `arg'`]],
2525
['" fail"', ['" fail"']],
26+
// Windows mode: backslash only escapes backslash, not quotes
2627
[`-DAWESOME=\"\\\"'fo o' bar\\\"\"`, [`-DAWESOME=\"\\\"'fo o' bar\\\"\"`]]
2728
];
2829

@@ -44,7 +45,64 @@ suite('shlex testing', () => {
4445
[`"fo o" bar`, [`"fo o"`, 'bar']],
4546
[`'quote arg'`, [`'quote arg'`]],
4647
['" fail"', ['" fail"']],
47-
[`-DAWESOME=\"\\\"fo o bar\\\"\"`, [`-DAWESOME=\"\\\"fo o bar\\\"\"`]]
48+
// POSIX mode: backslash outside quotes escapes any character (backslash consumed)
49+
// Input: -DAWESOME=\"\"fo o bar\"\" (escaped quotes outside quotes)
50+
// After escape processing: -DAWESOME=""fo o bar""
51+
// But the inner "" is an empty double-quoted section, so we get:
52+
[`-DAWESOME=\"\"fo o bar\"\"`, [`-DAWESOME=""fo`, 'o', 'bar""']]
53+
];
54+
55+
for (const [cmd, expected] of pairs) {
56+
expect(splitUnix(cmd)).to.eql(expected, `Bad parse for string: ${cmd}`);
57+
}
58+
});
59+
60+
test('Posix escape handling outside quotes', () => {
61+
const pairs: [string, string[]][] = [
62+
// \\ -> single backslash
63+
['foo\\\\bar', ['foo\\bar']],
64+
// \" -> literal quote
65+
['-DFOO=\\"bar\\"', ['-DFOO="bar"']],
66+
// \n (not newline char) -> n
67+
['foo\\nbar', ['foonbar']],
68+
// Escaped space keeps token together
69+
['foo\\ bar', ['foo bar']],
70+
// Line continuation (actual newline) - backslash+newline consumed
71+
['foo\\\nbar', ['foobar']],
72+
// Compile command with escaped quotes (issue #4896)
73+
['-DIMGUI_USER_CONFIG=\\"frontends/sdl/imgui/sa2_imconfig.h\\"', ['-DIMGUI_USER_CONFIG="frontends/sdl/imgui/sa2_imconfig.h"']]
74+
];
75+
76+
for (const [cmd, expected] of pairs) {
77+
expect(splitUnix(cmd)).to.eql(expected, `Bad parse for string: ${cmd}`);
78+
}
79+
});
80+
81+
test('Posix escape handling inside double quotes', () => {
82+
const pairs: [string, string[]][] = [
83+
// Inside double quotes: \\ -> \
84+
['"foo\\\\bar"', ['"foo\\bar"']],
85+
// Inside double quotes: \" -> "
86+
['"foo\\"bar"', ['"foo"bar"']],
87+
// Inside double quotes: \n (not escapable) -> \n preserved
88+
['"foo\\nbar"', ['"foo\\nbar"']],
89+
// Inside double quotes: \$ -> $
90+
['"foo\\$bar"', ['"foo$bar"']],
91+
// Inside double quotes: \` -> `
92+
['"foo\\`bar"', ['"foo`bar"']]
93+
];
94+
95+
for (const [cmd, expected] of pairs) {
96+
expect(splitUnix(cmd)).to.eql(expected, `Bad parse for string: ${cmd}`);
97+
}
98+
});
99+
100+
test('Posix escape handling inside single quotes', () => {
101+
const pairs: [string, string[]][] = [
102+
// Inside single quotes: backslash has no special meaning
103+
[`'foo\\bar'`, [`'foo\\bar'`]],
104+
[`'foo\\"bar'`, [`'foo\\"bar'`]],
105+
[`'foo\\\\bar'`, [`'foo\\\\bar'`]]
48106
];
49107

50108
for (const [cmd, expected] of pairs) {

0 commit comments

Comments
 (0)