Skip to content

Commit 300de6a

Browse files
CopilotOmotola
andauthored
Support string arrays in kit cmakeSettings for semicolon-separated CMake lists (#4823)
* Support string arrays in kit cmakeSettings for semicolon-separated CMake lists (#4503) Co-authored-by: Omotola <[email protected]> * Restore original yarn.lock Co-authored-by: Omotola <[email protected]> * Fix schema description for consistency Co-authored-by: Omotola <[email protected]> * Extract cmakeify to pure module for testability and improve documentation Co-authored-by: Omotola <[email protected]> * Add comment explaining why localization is omitted in cmakeValue.ts Co-authored-by: Omotola <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: Omotola <[email protected]> Co-authored-by: Omotola <[email protected]>
1 parent 140a888 commit 300de6a

8 files changed

Lines changed: 243 additions & 41 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ Features:
1616
- Support `targetName` argument for launch-target command substitutions (`cmake.launchTargetPath`, etc.) via `${input:...}` variables, enabling build-before-run for non-active executable targets without changing the active launch target. [#4656](https://github.com/microsoft/vscode-cmake-tools/issues/4656)
1717
- Complete Bookmarks context menu with "Set Build Target", "Set Launch/Debug Target", "Open CMakeLists.txt", and "Run Utility Target" actions to match the Project Outline. [#4788](https://github.com/microsoft/vscode-cmake-tools/issues/4788)
1818
- Relax `intelliSenseMode` validation in CMake Presets. [#4815](https://github.com/microsoft/vscode-cmake-tools/issues/4815) [@halflifefan](https://github.com/halflifefan)
19+
- Support string arrays in kit `cmakeSettings` to pass CMake lists without escaping semicolons (e.g., `"LLVM_ENABLE_PROJECTS": ["clang", "lld"]`). [#4503](https://github.com/microsoft/vscode-cmake-tools/issues/4503)
1920

2021
Improvements:
2122
- Add `.github/copilot-instructions.md` to ground GitHub Copilot in the repo's architecture and coding conventions.

docs/kits.md

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,32 @@ The following additional options may be specified:
151151
152152
> This setting is most useful when the toolchain file respects additional options that can be passed as cache variables.
153153
154+
> **Semicolon Handling:**
155+
>
156+
> CMake uses semicolons as list separators. The type of value you use determines how semicolons are handled:
157+
>
158+
> - **String values**: Semicolons are **escaped** (`;``\;`) to prevent CMake from treating them as list separators. Use this for single values that happen to contain semicolons.
159+
>
160+
> - **Array values**: Elements are joined with semicolons **without escaping**, producing a proper CMake list. Use this when you intentionally want to pass a CMake list.
161+
>
162+
> **Example - Passing a CMake list (use array notation):**
163+
> ```jsonc
164+
> "cmakeSettings": {
165+
> // Array notation → semicolons NOT escaped → creates CMake list
166+
> "LLVM_ENABLE_PROJECTS": ["clang", "lld"]
167+
> // Produces: -DLLVM_ENABLE_PROJECTS:STRING=clang;lld
168+
> }
169+
> ```
170+
>
171+
> **Example - String with semicolons escaped:**
172+
> ```jsonc
173+
> "cmakeSettings": {
174+
> // String notation → semicolons ARE escaped → treated as single value
175+
> "MY_VAR": "value;with;semicolons"
176+
> // Produces: -DMY_VAR:STRING=value\;with\;semicolons
177+
> }
178+
> ```
179+
154180
`environmentVariables`
155181
156182
> A JSON object of key-value pairs specifying additional environment variables to be defined when using this kit.

schemas/kits-schema.json

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,19 @@
7272
"type": "object",
7373
"patternProperties": {
7474
".*": {
75-
"description": "Value for the CMake Setting"
75+
"oneOf": [
76+
{
77+
"type": "string",
78+
"description": "Value for the CMake Setting. Semicolons in strings are escaped."
79+
},
80+
{
81+
"type": "array",
82+
"items": {
83+
"type": "string"
84+
},
85+
"description": "Values joined with semicolons to form a CMake list without escaping."
86+
}
87+
]
7688
}
7789
}
7890
},

src/cmakeValue.ts

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
/**
2+
* Pure utility functions for CMake value conversion.
3+
* This module has NO dependencies on 'vscode' or 'vscode-nls', making it safe
4+
* to import in backend tests that cannot load the vscode module.
5+
*/
6+
7+
/**
8+
* Represents a CMake cache variable value with its type.
9+
*/
10+
export interface CMakeValue {
11+
type: ('UNKNOWN' | 'BOOL' | 'STRING' | 'FILEPATH' | 'PATH' | ''); // There are more types, but we don't care ATM
12+
value: string;
13+
}
14+
15+
/**
16+
* Escape a string so it can be used as a regular expression
17+
*/
18+
function escapeStringForRegex(str: string): string {
19+
return str.replace(/([.*+?^=!:${}()|\[\]\/\\])/g, '\\$1');
20+
}
21+
22+
/**
23+
* Replace all occurrences of `needle` in `str` with `what`
24+
*/
25+
function replaceAll(str: string, needle: string, what: string): string {
26+
const pattern = escapeStringForRegex(needle);
27+
const re = new RegExp(pattern, 'g');
28+
return str.replace(re, what);
29+
}
30+
31+
/**
32+
* Checks if the given value is a string.
33+
*/
34+
function isString(x: unknown): x is string {
35+
return Object.prototype.toString.call(x) === "[object String]";
36+
}
37+
38+
/**
39+
* Converts a given value to a CMake-compatible value.
40+
*
41+
* **Semicolon Handling:**
42+
* - **String values**: Semicolons are escaped (`;` → `\;`) to prevent CMake from
43+
* interpreting them as list separators. Use this for single values that happen
44+
* to contain semicolons.
45+
* - **Array values**: Elements are joined with semicolons WITHOUT escaping,
46+
* producing a proper CMake list. Use this when you intentionally want a CMake
47+
* list (e.g., `LLVM_ENABLE_PROJECTS`).
48+
*
49+
* @param value The value to convert. Can be:
50+
* - `boolean`: Converts to CMake BOOL ("TRUE" or "FALSE")
51+
* - `string`: Converts to STRING with semicolons escaped
52+
* - `number`: Converts to STRING
53+
* - `string[]`: Joins elements with `;` to form a CMake list (no escaping)
54+
* - `CMakeValue`: Passes through unchanged
55+
* @returns A CMakeValue object with the appropriate type and value.
56+
* @throws An error if the input value is invalid or cannot be converted.
57+
*
58+
* @example
59+
* // String with semicolon - ESCAPED (for single values containing semicolons)
60+
* cmakeify("clang;lld")
61+
* // Returns: { type: 'STRING', value: 'clang\\;lld' }
62+
* // Produces: -DVAR:STRING=clang\;lld
63+
*
64+
* @example
65+
* // Array - NOT escaped (for CMake lists)
66+
* cmakeify(["clang", "lld"])
67+
* // Returns: { type: 'STRING', value: 'clang;lld' }
68+
* // Produces: -DVAR:STRING=clang;lld (a proper CMake list)
69+
*/
70+
export function cmakeify(value: (string | boolean | number | string[] | CMakeValue)): CMakeValue {
71+
const ret: CMakeValue = {
72+
type: 'UNKNOWN',
73+
value: ''
74+
};
75+
if (value === true || value === false) {
76+
ret.type = 'BOOL';
77+
ret.value = value ? 'TRUE' : 'FALSE';
78+
} else if (isString(value)) {
79+
ret.type = 'STRING';
80+
// String values have semicolons escaped to prevent CMake list interpretation
81+
ret.value = replaceAll(value, ';', '\\;');
82+
} else if (typeof value === 'number') {
83+
ret.type = 'STRING';
84+
ret.value = value.toString();
85+
} else if (value instanceof Array) {
86+
ret.type = 'STRING';
87+
// Array values are joined with semicolons WITHOUT escaping to form CMake lists
88+
ret.value = value.join(';');
89+
} else if (Object.getOwnPropertyNames(value).filter(e => e === 'type' || e === 'value').length === 2) {
90+
ret.type = value.type;
91+
ret.value = value.value;
92+
} else {
93+
// Note: This error message is not localized because this module must remain
94+
// free of vscode-nls dependencies to support import in backend unit tests.
95+
// This error is rare (only occurs with malformed input) and primarily aids debugging.
96+
throw new Error(`Invalid value to convert to cmake value: ${JSON.stringify(value)}`);
97+
}
98+
return ret;
99+
}

src/drivers/cmakeDriver.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1848,7 +1848,7 @@ export abstract class CMakeDriver implements vscode.Disposable {
18481848
}
18491849
if (this._kit.cmakeSettings) {
18501850
util.objectPairs(this._kit.cmakeSettings)
1851-
.forEach(([key, value]) => settingMap[key] = util.cmakeify(value as string));
1851+
.forEach(([key, value]) => settingMap[key] = util.cmakeify(value));
18521852
}
18531853

18541854
return util.objectPairs(settingMap).map(([key, value]) => {

src/kits/kit.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,11 @@ export interface Kit extends KitDetect {
105105
preferredGenerator?: CMakeGenerator;
106106

107107
/**
108-
* Additional settings to pass to CMake
108+
* Additional settings to pass to CMake.
109+
* Values can be strings or string arrays. String arrays are joined with
110+
* semicolons to form CMake lists (without escaping).
109111
*/
110-
cmakeSettings?: { [key: string]: string };
112+
cmakeSettings?: { [key: string]: string | string[] };
111113

112114
/**
113115
* Additional environment variables for the kit

src/util.ts

Lines changed: 3 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -280,43 +280,9 @@ export function product<T>(arrays: T[][]): T[][] {
280280
[[]] as T[][]);
281281
}
282282

283-
export interface CMakeValue {
284-
type: ('UNKNOWN' | 'BOOL' | 'STRING' | 'FILEPATH' | 'PATH' | ''); // There are more types, but we don't care ATM
285-
value: string;
286-
}
287-
288-
/**
289-
* Converts a given value to a CMake-compatible value.
290-
* The function determines the type of the input value and converts it to a corresponding CMakeValue object.
291-
* @param value The value to convert. It can be a string, boolean, number, string array, or CMakeValue.
292-
* @returns A CMakeValue object with the appropriate type and value.
293-
* @throws An error if the input value is invalid or cannot be converted to a CMakeValue.
294-
*/
295-
export function cmakeify(value: (string | boolean | number | string[] | CMakeValue)): CMakeValue {
296-
const ret: CMakeValue = {
297-
type: 'UNKNOWN',
298-
value: ''
299-
};
300-
if (value === true || value === false) {
301-
ret.type = 'BOOL';
302-
ret.value = value ? 'TRUE' : 'FALSE';
303-
} else if (isString(value)) {
304-
ret.type = 'STRING';
305-
ret.value = replaceAll(value, ';', '\\;');
306-
} else if (typeof value === 'number') {
307-
ret.type = 'STRING';
308-
ret.value = value.toString();
309-
} else if (value instanceof Array) {
310-
ret.type = 'STRING';
311-
ret.value = value.join(';');
312-
} else if (Object.getOwnPropertyNames(value).filter(e => e === 'type' || e === 'value').length === 2) {
313-
ret.type = value.type;
314-
ret.value = value.value;
315-
} else {
316-
throw new Error(localize('invalid.value', 'Invalid value to convert to cmake value: {0}', JSON.stringify(value)));
317-
}
318-
return ret;
319-
}
283+
// Re-export CMakeValue types and function from the pure module (no vscode dependency)
284+
// This allows backward compatibility for existing imports from @cmt/util
285+
export { CMakeValue, cmakeify } from '@cmt/cmakeValue';
320286

321287
/**
322288
* Terminates a child process and its descendant processes.
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
import { expect } from 'chai';
2+
import { cmakeify, CMakeValue } from '@cmt/cmakeValue';
3+
4+
/**
5+
* Tests for the cmakeify function that converts values to CMake cache variable format.
6+
*
7+
* This tests the REAL implementation from @cmt/cmakeValue (not a mirrored copy).
8+
* The cmakeValue module is pure TypeScript with no vscode dependencies, making it
9+
* safe to import in backend tests.
10+
*
11+
* Key behavior tested:
12+
* - String values have semicolons ESCAPED (`;` → `\;`) to prevent CMake list interpretation
13+
* - Array values are joined with semicolons WITHOUT escaping to form CMake lists
14+
*/
15+
16+
suite('[cmakeify]', () => {
17+
18+
test('Boolean true converts to BOOL TRUE', () => {
19+
const result = cmakeify(true);
20+
expect(result.type).to.equal('BOOL');
21+
expect(result.value).to.equal('TRUE');
22+
});
23+
24+
test('Boolean false converts to BOOL FALSE', () => {
25+
const result = cmakeify(false);
26+
expect(result.type).to.equal('BOOL');
27+
expect(result.value).to.equal('FALSE');
28+
});
29+
30+
test('Simple string converts to STRING', () => {
31+
const result = cmakeify('hello');
32+
expect(result.type).to.equal('STRING');
33+
expect(result.value).to.equal('hello');
34+
});
35+
36+
test('String with semicolon is escaped', () => {
37+
const result = cmakeify('clang;lld');
38+
expect(result.type).to.equal('STRING');
39+
expect(result.value).to.equal('clang\\;lld');
40+
});
41+
42+
test('Number converts to STRING', () => {
43+
const result = cmakeify(42);
44+
expect(result.type).to.equal('STRING');
45+
expect(result.value).to.equal('42');
46+
});
47+
48+
test('String array joins with semicolons without escaping', () => {
49+
const result = cmakeify(['clang', 'lld']);
50+
expect(result.type).to.equal('STRING');
51+
expect(result.value).to.equal('clang;lld');
52+
});
53+
54+
test('String array with multiple elements creates CMake list', () => {
55+
const result = cmakeify(['clang', 'lld', 'compiler-rt', 'libcxx']);
56+
expect(result.type).to.equal('STRING');
57+
expect(result.value).to.equal('clang;lld;compiler-rt;libcxx');
58+
});
59+
60+
test('Empty array produces empty string', () => {
61+
const result = cmakeify([]);
62+
expect(result.type).to.equal('STRING');
63+
expect(result.value).to.equal('');
64+
});
65+
66+
test('Single element array produces single value', () => {
67+
const result = cmakeify(['clang']);
68+
expect(result.type).to.equal('STRING');
69+
expect(result.value).to.equal('clang');
70+
});
71+
72+
test('CMakeValue passthrough preserves type and value', () => {
73+
const input: CMakeValue = { type: 'FILEPATH', value: '/path/to/file' };
74+
const result = cmakeify(input);
75+
expect(result.type).to.equal('FILEPATH');
76+
expect(result.value).to.equal('/path/to/file');
77+
});
78+
79+
test('LLVM_ENABLE_PROJECTS use case - array notation avoids escaping', () => {
80+
// This is the main use case from issue #4503
81+
const result = cmakeify(['clang', 'lld']);
82+
expect(result.type).to.equal('STRING');
83+
// Array notation should NOT escape semicolons - they are joined directly
84+
expect(result.value).to.equal('clang;lld');
85+
// This would produce: -DLLVM_ENABLE_PROJECTS:STRING=clang;lld
86+
});
87+
88+
test('LLVM_ENABLE_PROJECTS use case - string notation escapes semicolons', () => {
89+
// When using string notation, semicolons are escaped
90+
const result = cmakeify('clang;lld');
91+
expect(result.type).to.equal('STRING');
92+
// String notation DOES escape semicolons
93+
expect(result.value).to.equal('clang\\;lld');
94+
// This would produce: -DLLVM_ENABLE_PROJECTS:STRING=clang\;lld
95+
});
96+
});

0 commit comments

Comments
 (0)