From f3467236072658ecc4854b6acc9a2cc50bcb1a1e Mon Sep 17 00:00:00 2001 From: "Waisman, Yiftah" Date: Sun, 10 Nov 2024 21:12:31 +0200 Subject: [PATCH 01/17] accept an array in compile commands schema --- Extension/c_cpp_properties.schema.json | 17 +++++++++++++++-- Extension/package.json | 15 ++++++++++++++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/Extension/c_cpp_properties.schema.json b/Extension/c_cpp_properties.schema.json index a07d242f0c..8059198f6d 100644 --- a/Extension/c_cpp_properties.schema.json +++ b/Extension/c_cpp_properties.schema.json @@ -70,9 +70,22 @@ ] }, "compileCommands": { + "oneOf": [ + { + "type": "string", + "default": "" + }, + { + "type": "array", + "items": { + "type": "string", + "uniqueItems": true + }, + "default": [] + } + ], "markdownDescription": "Full path to `compile_commands.json` file for the workspace.", - "descriptionHint": "Markdown text between `` should not be translated or localized (they represent literal text) and the capitalization, spacing, and punctuation (including the ``) should not be altered.", - "type": "string" + "descriptionHint": "Markdown text between `` should not be translated or localized (they represent literal text) and the capitalization, spacing, and punctuation (including the ``) should not be altered." }, "includePath": { "markdownDescription": "A list of paths for the IntelliSense engine to use while searching for included headers. Searching on these paths is not recursive. Specify `**` to indicate recursive search. For example, `${workspaceFolder}/**` will search through all subdirectories while `${workspaceFolder}` will not. Usually, this should not include system includes; instead, set `C_Cpp.default.compilerPath`.", diff --git a/Extension/package.json b/Extension/package.json index 733e2f4298..e77b2f2fff 100644 --- a/Extension/package.json +++ b/Extension/package.json @@ -693,7 +693,20 @@ "scope": "machine-overridable" }, "C_Cpp.default.compileCommands": { - "type": "string", + "oneOf": [ + { + "type": "string", + "default": "" + }, + { + "type": "array", + "items": { + "type": "string", + "uniqueItems": true + }, + "default": [] + } + ], "markdownDescription": "%c_cpp.configuration.default.compileCommands.markdownDescription%", "scope": "machine-overridable" }, From 2dc0fa328091c21de7439b43c3868a2ba4954b1f Mon Sep 17 00:00:00 2001 From: yiftahw <63462505+yiftahw@users.noreply.github.com> Date: Sun, 10 Nov 2024 22:08:11 +0200 Subject: [PATCH 02/17] handle settings.json of multiple compile commands --- Extension/src/LanguageServer/settings.ts | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/Extension/src/LanguageServer/settings.ts b/Extension/src/LanguageServer/settings.ts index 8e8d6c0654..d61208a4d6 100644 --- a/Extension/src/LanguageServer/settings.ts +++ b/Extension/src/LanguageServer/settings.ts @@ -399,10 +399,23 @@ export class CppSettings extends Settings { public get defaultDotconfig(): string | undefined { return changeBlankStringToUndefined(this.getAsStringOrUndefined("default.dotConfig")); } public get defaultMacFrameworkPath(): string[] | undefined { return this.getArrayOfStringsWithUndefinedDefault("default.macFrameworkPath"); } public get defaultWindowsSdkVersion(): string | undefined { return changeBlankStringToUndefined(this.getAsStringOrUndefined("default.windowsSdkVersion")); } - public get defaultCompileCommands(): string | undefined { return changeBlankStringToUndefined(this.getAsStringOrUndefined("default.compileCommands")); } public get defaultForcedInclude(): string[] | undefined { return this.getArrayOfStringsWithUndefinedDefault("default.forcedInclude"); } public get defaultIntelliSenseMode(): string | undefined { return this.getAsStringOrUndefined("default.intelliSenseMode"); } public get defaultCompilerPath(): string | null { return this.getAsString("default.compilerPath", true); } + public get defaultCompileCommands(): string[] | undefined { + // Try to get the value as a string. + const value: string | undefined = this.getAsStringOrUndefined("default.compileCommands"); + if (value !== undefined) { + if (changeBlankStringToUndefined(value) === undefined) { + return undefined; + } + return [value]; + } + + // value is not a string, try to get it as an array of strings instead. + return this.getAsArrayOfStringsOrUndefined("default.compileCommands"); + } + public set defaultCompilerPath(value: string) { const defaultCompilerPathStr: string = "default.compilerPath"; From 02a28121427f1a6144b29c908c2dad59c2124510 Mon Sep 17 00:00:00 2001 From: yiftahw <63462505+yiftahw@users.noreply.github.com> Date: Sun, 10 Nov 2024 22:30:22 +0200 Subject: [PATCH 03/17] update schemas --- Extension/c_cpp_properties.schema.json | 4 ++-- Extension/package.json | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Extension/c_cpp_properties.schema.json b/Extension/c_cpp_properties.schema.json index 8059198f6d..b090cb5279 100644 --- a/Extension/c_cpp_properties.schema.json +++ b/Extension/c_cpp_properties.schema.json @@ -78,9 +78,9 @@ { "type": "array", "items": { - "type": "string", - "uniqueItems": true + "type": "string" }, + "uniqueItems": true, "default": [] } ], diff --git a/Extension/package.json b/Extension/package.json index e77b2f2fff..d0898b17da 100644 --- a/Extension/package.json +++ b/Extension/package.json @@ -701,9 +701,9 @@ { "type": "array", "items": { - "type": "string", - "uniqueItems": true + "type": "string" }, + "uniqueItems": true, "default": [] } ], From 280aee15907b8606c4f22b32882bc21a002cd31e Mon Sep 17 00:00:00 2001 From: yiftahw <63462505+yiftahw@users.noreply.github.com> Date: Sun, 10 Nov 2024 22:43:09 +0200 Subject: [PATCH 04/17] an array is returned from default settings only if non-empty with non-empty strings --- Extension/src/LanguageServer/settings.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Extension/src/LanguageServer/settings.ts b/Extension/src/LanguageServer/settings.ts index d61208a4d6..39f5166410 100644 --- a/Extension/src/LanguageServer/settings.ts +++ b/Extension/src/LanguageServer/settings.ts @@ -413,7 +413,12 @@ export class CppSettings extends Settings { } // value is not a string, try to get it as an array of strings instead. - return this.getAsArrayOfStringsOrUndefined("default.compileCommands"); + var valueArray: string[] | undefined = this.getAsArrayOfStringsOrUndefined("default.compileCommands"); + valueArray = valueArray?.filter((value) => value !== ""); + if (valueArray?.length === 0) { + return undefined; + } + return valueArray; } @@ -651,7 +656,7 @@ export class CppSettings extends Settings { } if (isArrayOfString(value)) { - if (setting.items.enum && !allowUndefinedEnums) { + if (setting.items?.enum !== undefined && !allowUndefinedEnums) { if (!value.every(x => this.isValidEnum(setting.items.enum, x))) { return setting.default; } From b934a929c7bd7bddd78e5a6d857b823065d88757 Mon Sep 17 00:00:00 2001 From: yiftahw <63462505+yiftahw@users.noreply.github.com> Date: Mon, 11 Nov 2024 18:23:29 +0200 Subject: [PATCH 05/17] basic handling of compile commands array in configurations --- .../src/LanguageServer/configurations.ts | 46 ++++++++++--------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/Extension/src/LanguageServer/configurations.ts b/Extension/src/LanguageServer/configurations.ts index 3339514cc2..0094e068da 100644 --- a/Extension/src/LanguageServer/configurations.ts +++ b/Extension/src/LanguageServer/configurations.ts @@ -78,8 +78,8 @@ export interface Configuration { defines?: string[]; intelliSenseMode?: string; intelliSenseModeIsExplicit?: boolean; - compileCommandsInCppPropertiesJson?: string; - compileCommands?: string; + compileCommandsInCppPropertiesJson?: string[]; + compileCommands?: string[]; forcedInclude?: string[]; configurationProviderInCppPropertiesJson?: string; configurationProvider?: string; @@ -136,7 +136,7 @@ export class CppProperties { private currentConfigurationIndex: PersistentFolderState | undefined; private configFileWatcher: vscode.FileSystemWatcher | null = null; private configFileWatcherFallbackTime: Date = new Date(); // Used when file watching fails. - private compileCommandsFile: vscode.Uri | undefined | null = undefined; + private compileCommandsFiles: Set = new Set(); private compileCommandsFileWatchers: fs.FSWatcher[] = []; private compileCommandsFileWatcherFallbackTime: Date = new Date(); // Used when file watching fails. private defaultCompilerPath: string | null = null; @@ -389,7 +389,7 @@ export class CppProperties { configuration.windowsSdkVersion = this.defaultWindowsSdkVersion; } if (isUnset(settings.defaultCompilerPath) && this.defaultCompilerPath && - (isUnset(settings.defaultCompileCommands) || settings.defaultCompileCommands === "") && !configuration.compileCommands) { + (isUnset(settings.defaultCompileCommands) || settings.defaultCompileCommands?.length === 0) && !configuration.compileCommands) { // compile_commands.json already specifies a compiler. compilerPath overrides the compile_commands.json compiler so // don't set a default when compileCommands is in use. @@ -684,7 +684,7 @@ export class CppProperties { this.parsePropertiesFile(); // Clear out any modifications we may have made internally. const config: Configuration | undefined = this.CurrentConfiguration; if (config) { - config.compileCommands = path; + config.compileCommands = [path]; this.writeToJson(); } // Any time parsePropertiesFile is called, configurationJson gets @@ -938,7 +938,7 @@ export class CppProperties { configuration.macFrameworkPath = this.updateConfigurationStringArray(configuration.macFrameworkPath, settings.defaultMacFrameworkPath, env); configuration.windowsSdkVersion = this.updateConfigurationString(configuration.windowsSdkVersion, settings.defaultWindowsSdkVersion, env); configuration.forcedInclude = this.updateConfigurationPathsArray(configuration.forcedInclude, settings.defaultForcedInclude, env, false); - configuration.compileCommands = this.updateConfigurationString(configuration.compileCommands, settings.defaultCompileCommands, env); + configuration.compileCommands = this.updateConfigurationStringArray(configuration.compileCommands, settings.defaultCompileCommands, env); configuration.compilerArgs = this.updateConfigurationStringArray(configuration.compilerArgs, settings.defaultCompilerArgs, env); configuration.cStandard = this.updateConfigurationString(configuration.cStandard, settings.defaultCStandard, env); configuration.cppStandard = this.updateConfigurationString(configuration.cppStandard, settings.defaultCppStandard, env); @@ -1092,7 +1092,7 @@ export class CppProperties { } if (configuration.compileCommands) { - configuration.compileCommands = this.resolvePath(configuration.compileCommands); + configuration.compileCommands = configuration.compileCommands.map((path: string) => this.resolvePath(path)); } if (configuration.forcedInclude) { @@ -1121,12 +1121,12 @@ export class CppProperties { this.compileCommandsFileWatchers = []; // reset it const filePaths: Set = new Set(); this.configurationJson.configurations.forEach(c => { - if (c.compileCommands) { - const fileSystemCompileCommandsPath: string = this.resolvePath(c.compileCommands); + c.compileCommands?.forEach((path: string) => { + const fileSystemCompileCommandsPath: string = this.resolvePath(path); if (fs.existsSync(fileSystemCompileCommandsPath)) { filePaths.add(fileSystemCompileCommandsPath); } - } + }) }); try { filePaths.forEach((path: string) => { @@ -2302,23 +2302,25 @@ export class CppProperties { public checkCompileCommands(): void { // Check for changes in case of file watcher failure. - const compileCommands: string | undefined = this.CurrentConfiguration?.compileCommands; + const compileCommands: string[] | undefined = this.CurrentConfiguration?.compileCommands; if (!compileCommands) { return; } - const compileCommandsFile: string | undefined = this.resolvePath(compileCommands); - fs.stat(compileCommandsFile, (err, stats) => { - if (err) { - if (err.code === "ENOENT" && this.compileCommandsFile) { - this.compileCommandsFileWatchers = []; // reset file watchers + compileCommands.forEach((path: string) => { + const compileCommandsFile: string | undefined = this.resolvePath(path); + fs.stat(compileCommandsFile, (err, stats) => { + if (err) { + if (err.code === "ENOENT" && this.compileCommandsFiles.has(compileCommandsFile)) { + this.compileCommandsFileWatchers = []; // reset file watchers + this.onCompileCommandsChanged(compileCommandsFile); + this.compileCommandsFiles.delete(compileCommandsFile); // File deleted + } + } else if (stats.mtime > this.compileCommandsFileWatcherFallbackTime) { + this.compileCommandsFileWatcherFallbackTime = new Date(); this.onCompileCommandsChanged(compileCommandsFile); - this.compileCommandsFile = null; // File deleted + this.compileCommandsFiles.add(compileCommandsFile); // File created. } - } else if (stats.mtime > this.compileCommandsFileWatcherFallbackTime) { - this.compileCommandsFileWatcherFallbackTime = new Date(); - this.onCompileCommandsChanged(compileCommandsFile); - this.compileCommandsFile = vscode.Uri.file(compileCommandsFile); // File created. - } + }); }); } From 0c8ccb1137f2850c5ff8e64345a722c84effbe98 Mon Sep 17 00:00:00 2001 From: "Waisman, Yiftah" Date: Wed, 13 Nov 2024 19:08:47 +0200 Subject: [PATCH 06/17] fixes after merge --- .../src/LanguageServer/configurations.ts | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/Extension/src/LanguageServer/configurations.ts b/Extension/src/LanguageServer/configurations.ts index 122526207b..c86164c7db 100644 --- a/Extension/src/LanguageServer/configurations.ts +++ b/Extension/src/LanguageServer/configurations.ts @@ -1092,11 +1092,13 @@ export class CppProperties { } if (configuration.compileCommands) { - configuration.compileCommands = configuration.compileCommands.map((path: string) => this.resolvePath(path)); - if (!this.compileCommandsFileWatcherFallbackTime.has(configuration.compileCommands)) { - // Start tracking the fallback time for a new path. - this.compileCommandsFileWatcherFallbackTime.set(configuration.compileCommands, new Date()); - } + configuration.compileCommands = configuration.compileCommands?.map((path: string) => this.resolvePath(path)); + configuration.compileCommands?.forEach((path: string) => { + if (!this.compileCommandsFileWatcherFallbackTime.has(path)) { + // Start tracking the fallback time for a new path. + this.compileCommandsFileWatcherFallbackTime.set(path, new Date()); + } + }); } if (configuration.forcedInclude) { @@ -1120,10 +1122,12 @@ export class CppProperties { // Instead, we clear entries that are no longer relevant. const trackedCompileCommandsPaths: Set = new Set(); this.configurationJson?.configurations.forEach((config: Configuration) => { - const path = this.resolvePath(config.compileCommands); - if (path.length > 0) { - trackedCompileCommandsPaths.add(path); - } + config.compileCommands?.forEach((path: string) => { + const compileCommandsFile = this.resolvePath(path); + if (compileCommandsFile.length > 0) { + trackedCompileCommandsPaths.add(compileCommandsFile); + } + }); }); for (const path of this.compileCommandsFileWatcherFallbackTime.keys()) { @@ -1145,9 +1149,9 @@ export class CppProperties { const filePaths: Set = new Set(); this.configurationJson.configurations.forEach(c => { c.compileCommands?.forEach((path: string) => { - const fileSystemCompileCommandsPath: string = this.resolvePath(path); - if (fs.existsSync(fileSystemCompileCommandsPath)) { - filePaths.add(fileSystemCompileCommandsPath); + const compileCommandsFile: string = this.resolvePath(path); + if (fs.existsSync(compileCommandsFile)) { + filePaths.add(compileCommandsFile); } }) }); From 8e841836f2e6f95891e7cd790cc1af5e05914889 Mon Sep 17 00:00:00 2001 From: Yiftah Waisman Date: Thu, 14 Nov 2024 06:52:45 +0200 Subject: [PATCH 07/17] yarn lint --- Extension/src/LanguageServer/configurations.ts | 2 +- Extension/src/LanguageServer/settings.ts | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/Extension/src/LanguageServer/configurations.ts b/Extension/src/LanguageServer/configurations.ts index c86164c7db..2059db3929 100644 --- a/Extension/src/LanguageServer/configurations.ts +++ b/Extension/src/LanguageServer/configurations.ts @@ -1153,7 +1153,7 @@ export class CppProperties { if (fs.existsSync(compileCommandsFile)) { filePaths.add(compileCommandsFile); } - }) + }); }); try { filePaths.forEach((path: string) => { diff --git a/Extension/src/LanguageServer/settings.ts b/Extension/src/LanguageServer/settings.ts index 39f5166410..5420a6204d 100644 --- a/Extension/src/LanguageServer/settings.ts +++ b/Extension/src/LanguageServer/settings.ts @@ -401,7 +401,6 @@ export class CppSettings extends Settings { public get defaultWindowsSdkVersion(): string | undefined { return changeBlankStringToUndefined(this.getAsStringOrUndefined("default.windowsSdkVersion")); } public get defaultForcedInclude(): string[] | undefined { return this.getArrayOfStringsWithUndefinedDefault("default.forcedInclude"); } public get defaultIntelliSenseMode(): string | undefined { return this.getAsStringOrUndefined("default.intelliSenseMode"); } - public get defaultCompilerPath(): string | null { return this.getAsString("default.compilerPath", true); } public get defaultCompileCommands(): string[] | undefined { // Try to get the value as a string. const value: string | undefined = this.getAsStringOrUndefined("default.compileCommands"); @@ -413,14 +412,14 @@ export class CppSettings extends Settings { } // value is not a string, try to get it as an array of strings instead. - var valueArray: string[] | undefined = this.getAsArrayOfStringsOrUndefined("default.compileCommands"); + let valueArray: string[] | undefined = this.getAsArrayOfStringsOrUndefined("default.compileCommands"); valueArray = valueArray?.filter((value) => value !== ""); if (valueArray?.length === 0) { return undefined; } return valueArray; } - + public get defaultCompilerPath(): string | null { return this.getAsString("default.compilerPath", true); } public set defaultCompilerPath(value: string) { const defaultCompilerPathStr: string = "default.compilerPath"; From 22de1e0172396d03e6fac41d4b517a6e9b137306 Mon Sep 17 00:00:00 2001 From: Yiftah Waisman Date: Thu, 14 Nov 2024 07:20:14 +0200 Subject: [PATCH 08/17] unify check behavior --- Extension/src/LanguageServer/configurations.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Extension/src/LanguageServer/configurations.ts b/Extension/src/LanguageServer/configurations.ts index 2059db3929..6d2a8a22e5 100644 --- a/Extension/src/LanguageServer/configurations.ts +++ b/Extension/src/LanguageServer/configurations.ts @@ -389,7 +389,8 @@ export class CppProperties { configuration.windowsSdkVersion = this.defaultWindowsSdkVersion; } if (isUnset(settings.defaultCompilerPath) && this.defaultCompilerPath && - (isUnset(settings.defaultCompileCommands) || settings.defaultCompileCommands?.length === 0) && !configuration.compileCommands) { + (isUnset(settings.defaultCompileCommands) || settings.defaultCompileCommands?.length === 0) && + (isUnset(configuration.compileCommands) || configuration.compileCommands?.length === 0)) { // compile_commands.json already specifies a compiler. compilerPath overrides the compile_commands.json compiler so // don't set a default when compileCommands is in use. From 62a891566ab7eb6fa990def27b2e8888844b826c Mon Sep 17 00:00:00 2001 From: Yiftah Waisman Date: Thu, 14 Nov 2024 07:27:45 +0200 Subject: [PATCH 09/17] update desription in schema --- Extension/c_cpp_properties.schema.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Extension/c_cpp_properties.schema.json b/Extension/c_cpp_properties.schema.json index b090cb5279..7cc7f14790 100644 --- a/Extension/c_cpp_properties.schema.json +++ b/Extension/c_cpp_properties.schema.json @@ -84,7 +84,7 @@ "default": [] } ], - "markdownDescription": "Full path to `compile_commands.json` file for the workspace.", + "markdownDescription": "Full path or a list of full paths to `compile_commands.json` files for the workspace.", "descriptionHint": "Markdown text between `` should not be translated or localized (they represent literal text) and the capitalization, spacing, and punctuation (including the ``) should not be altered." }, "includePath": { From d7e2a4acae6ddd72ac6fbd73ec6eb8a3aa3a4e32 Mon Sep 17 00:00:00 2001 From: Yiftah Waisman Date: Thu, 14 Nov 2024 10:03:12 +0200 Subject: [PATCH 10/17] handle parsed values from c_cpp_properties.json --- .../src/LanguageServer/configurations.ts | 19 +++++++++++++++++++ Extension/src/LanguageServer/settings.ts | 2 +- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/Extension/src/LanguageServer/configurations.ts b/Extension/src/LanguageServer/configurations.ts index 6d2a8a22e5..e47c834df3 100644 --- a/Extension/src/LanguageServer/configurations.ts +++ b/Extension/src/LanguageServer/configurations.ts @@ -1446,6 +1446,25 @@ export class CppProperties { } } } + + // Configuration.compileCommands is allowed to be defined as a string in the schema, but we send an array to the language server. + // For having a predictable behvaior, we convert it here to an array of strings. + for (let i: number = 0; i < newJson.configurations.length; i++) { + let compileCommandsInCppPropertiesJson: string | string[] | undefined = newJson.configurations[i].compileCommands; + if (util.isString(compileCommandsInCppPropertiesJson) && compileCommandsInCppPropertiesJson.length > 0) { + newJson.configurations[i].compileCommands = [compileCommandsInCppPropertiesJson]; + } else if (util.isArrayOfString(compileCommandsInCppPropertiesJson)) { + compileCommandsInCppPropertiesJson = compileCommandsInCppPropertiesJson.filter((value: string) => value.length > 0); + if (compileCommandsInCppPropertiesJson.length > 0) { + newJson.configurations[i].compileCommands = compileCommandsInCppPropertiesJson; + } else { + newJson.configurations[i].compileCommands = undefined; + } + } else { + newJson.configurations[i].compileCommands = undefined; + } + } + this.configurationJson = newJson; if (this.CurrentConfigurationIndex < 0 || this.CurrentConfigurationIndex >= newJson.configurations.length) { const index: number | undefined = this.getConfigIndexForPlatform(newJson); diff --git a/Extension/src/LanguageServer/settings.ts b/Extension/src/LanguageServer/settings.ts index 5420a6204d..159ef07177 100644 --- a/Extension/src/LanguageServer/settings.ts +++ b/Extension/src/LanguageServer/settings.ts @@ -413,7 +413,7 @@ export class CppSettings extends Settings { // value is not a string, try to get it as an array of strings instead. let valueArray: string[] | undefined = this.getAsArrayOfStringsOrUndefined("default.compileCommands"); - valueArray = valueArray?.filter((value) => value !== ""); + valueArray = valueArray?.filter((value: string) => value.length > 0); if (valueArray?.length === 0) { return undefined; } From b1614decd7974988ab852a945b5863a00e98b8a4 Mon Sep 17 00:00:00 2001 From: Yiftah Waisman Date: Thu, 14 Nov 2024 10:07:16 +0200 Subject: [PATCH 11/17] reverse logic to default as undefined --- Extension/src/LanguageServer/configurations.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Extension/src/LanguageServer/configurations.ts b/Extension/src/LanguageServer/configurations.ts index e47c834df3..937df2d89f 100644 --- a/Extension/src/LanguageServer/configurations.ts +++ b/Extension/src/LanguageServer/configurations.ts @@ -1451,17 +1451,14 @@ export class CppProperties { // For having a predictable behvaior, we convert it here to an array of strings. for (let i: number = 0; i < newJson.configurations.length; i++) { let compileCommandsInCppPropertiesJson: string | string[] | undefined = newJson.configurations[i].compileCommands; + newJson.configurations[i].compileCommands = undefined; if (util.isString(compileCommandsInCppPropertiesJson) && compileCommandsInCppPropertiesJson.length > 0) { newJson.configurations[i].compileCommands = [compileCommandsInCppPropertiesJson]; } else if (util.isArrayOfString(compileCommandsInCppPropertiesJson)) { compileCommandsInCppPropertiesJson = compileCommandsInCppPropertiesJson.filter((value: string) => value.length > 0); if (compileCommandsInCppPropertiesJson.length > 0) { newJson.configurations[i].compileCommands = compileCommandsInCppPropertiesJson; - } else { - newJson.configurations[i].compileCommands = undefined; } - } else { - newJson.configurations[i].compileCommands = undefined; } } From 7fe9904daf3473095e65028bab90b9ffa3953eeb Mon Sep 17 00:00:00 2001 From: yiftahw <63462505+yiftahw@users.noreply.github.com> Date: Thu, 14 Nov 2024 20:12:29 +0200 Subject: [PATCH 12/17] squiggles for multi paths --- .../src/LanguageServer/configurations.ts | 35 ++++++++++++------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/Extension/src/LanguageServer/configurations.ts b/Extension/src/LanguageServer/configurations.ts index 937df2d89f..5768c3b82f 100644 --- a/Extension/src/LanguageServer/configurations.ts +++ b/Extension/src/LanguageServer/configurations.ts @@ -1417,6 +1417,18 @@ export class CppProperties { return; } + private forceCompileCommandsAsArray(compileCommandsInCppPropertiesJson: any): string[] | undefined { + if (util.isString(compileCommandsInCppPropertiesJson) && compileCommandsInCppPropertiesJson.length > 0) { + return [compileCommandsInCppPropertiesJson]; + } else if (util.isArrayOfString(compileCommandsInCppPropertiesJson)) { + const filteredArray: string[] = compileCommandsInCppPropertiesJson.filter(value => value.length > 0); + if (filteredArray.length > 0) { + return filteredArray; + } + } + return undefined + } + private parsePropertiesFile(): boolean { if (!this.propertiesFile) { this.configurationJson = undefined; @@ -1450,16 +1462,7 @@ export class CppProperties { // Configuration.compileCommands is allowed to be defined as a string in the schema, but we send an array to the language server. // For having a predictable behvaior, we convert it here to an array of strings. for (let i: number = 0; i < newJson.configurations.length; i++) { - let compileCommandsInCppPropertiesJson: string | string[] | undefined = newJson.configurations[i].compileCommands; - newJson.configurations[i].compileCommands = undefined; - if (util.isString(compileCommandsInCppPropertiesJson) && compileCommandsInCppPropertiesJson.length > 0) { - newJson.configurations[i].compileCommands = [compileCommandsInCppPropertiesJson]; - } else if (util.isArrayOfString(compileCommandsInCppPropertiesJson)) { - compileCommandsInCppPropertiesJson = compileCommandsInCppPropertiesJson.filter((value: string) => value.length > 0); - if (compileCommandsInCppPropertiesJson.length > 0) { - newJson.configurations[i].compileCommands = compileCommandsInCppPropertiesJson; - } - } + newJson.configurations[i].compileCommands = this.forceCompileCommandsAsArray(newJson.configurations[i].compileCommands); } this.configurationJson = newJson; @@ -1811,6 +1814,11 @@ export class CppProperties { const configurations: ConfigurationJson = jsonc.parse(configurationsText, undefined, true) as any; const currentConfiguration: Configuration = configurations.configurations[this.CurrentConfigurationIndex]; + // Configuration.compileCommands is allowed to be defined as a string in the schema, but we send an array to the language server. + // For having a predictable behvaior, we convert it here to an array of strings. + // Squiggles are still handled for both cases. + currentConfiguration.compileCommands = this.forceCompileCommandsAsArray(currentConfiguration.compileCommands); + let curTextStartOffset: number = 0; if (!currentConfiguration.name) { return; @@ -1937,9 +1945,10 @@ export class CppProperties { } } } - if (currentConfiguration.compileCommands) { - paths.push(`${currentConfiguration.compileCommands}`); - } + + currentConfiguration.compileCommands?.forEach((file: string) => { + paths.push(`${file}`) + }); if (currentConfiguration.compilerPath) { // Unlike other cases, compilerPath may not start or end with " due to trimming of whitespace and the possibility of compiler args. From 82c4e5386f378bb716cf2cfb420927b6833802c1 Mon Sep 17 00:00:00 2001 From: yiftahw <63462505+yiftahw@users.noreply.github.com> Date: Thu, 14 Nov 2024 20:16:04 +0200 Subject: [PATCH 13/17] remove not needed optional guard --- Extension/src/LanguageServer/configurations.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Extension/src/LanguageServer/configurations.ts b/Extension/src/LanguageServer/configurations.ts index 5768c3b82f..ea64235fc3 100644 --- a/Extension/src/LanguageServer/configurations.ts +++ b/Extension/src/LanguageServer/configurations.ts @@ -1093,8 +1093,8 @@ export class CppProperties { } if (configuration.compileCommands) { - configuration.compileCommands = configuration.compileCommands?.map((path: string) => this.resolvePath(path)); - configuration.compileCommands?.forEach((path: string) => { + configuration.compileCommands = configuration.compileCommands.map((path: string) => this.resolvePath(path)); + configuration.compileCommands.forEach((path: string) => { if (!this.compileCommandsFileWatcherFallbackTime.has(path)) { // Start tracking the fallback time for a new path. this.compileCommandsFileWatcherFallbackTime.set(path, new Date()); @@ -1962,6 +1962,8 @@ export class CppProperties { const forcedeIncludeEnd: number = forcedIncludeStart === -1 ? -1 : curText.indexOf("]", forcedIncludeStart); const compileCommandsStart: number = curText.search(/\s*\"compileCommands\"\s*:\s*\"/); const compileCommandsEnd: number = compileCommandsStart === -1 ? -1 : curText.indexOf('"', curText.indexOf('"', curText.indexOf(":", compileCommandsStart)) + 1); + // const compileCommandsArrayStart: number = curText.search(/\s*\"compileCommands\"\s*:\s*\[/); + // const compileCommandsArrayEnd: number = compileCommandsArrayStart === -1 ? -1 : curText.indexOf("]", curText.indexOf("[", curText.indexOf(":", compileCommandsArrayStart)) + 1); const compilerPathStart: number = curText.search(/\s*\"compilerPath\"\s*:\s*\"/); const compilerPathValueStart: number = curText.indexOf('"', curText.indexOf(":", compilerPathStart)); const compilerPathEnd: number = compilerPathStart === -1 ? -1 : curText.indexOf('"', compilerPathValueStart + 1) + 1; From 69c40137df9feddccce6f349928d09b47f9f30f3 Mon Sep 17 00:00:00 2001 From: yiftahw <63462505+yiftahw@users.noreply.github.com> Date: Thu, 14 Nov 2024 21:00:33 +0200 Subject: [PATCH 14/17] squiggles finalized --- .../src/LanguageServer/configurations.ts | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/Extension/src/LanguageServer/configurations.ts b/Extension/src/LanguageServer/configurations.ts index ea64235fc3..c7af63acd2 100644 --- a/Extension/src/LanguageServer/configurations.ts +++ b/Extension/src/LanguageServer/configurations.ts @@ -1898,9 +1898,9 @@ export class CppProperties { curText = curText.substring(0, nextNameStart2); } if (this.prevSquiggleMetrics.get(currentConfiguration.name) === undefined) { - this.prevSquiggleMetrics.set(currentConfiguration.name, { PathNonExistent: 0, PathNotAFile: 0, PathNotADirectory: 0, CompilerPathMissingQuotes: 0, CompilerModeMismatch: 0, MultiplePathsNotAllowed: 0 }); + this.prevSquiggleMetrics.set(currentConfiguration.name, { PathNonExistent: 0, PathNotAFile: 0, PathNotADirectory: 0, CompilerPathMissingQuotes: 0, CompilerModeMismatch: 0, MultiplePathsNotAllowed: 0, MultiplePathsShouldBeSeparated: 0 }); } - const newSquiggleMetrics: { [key: string]: number } = { PathNonExistent: 0, PathNotAFile: 0, PathNotADirectory: 0, CompilerPathMissingQuotes: 0, CompilerModeMismatch: 0, MultiplePathsNotAllowed: 0 }; + const newSquiggleMetrics: { [key: string]: number } = { PathNonExistent: 0, PathNotAFile: 0, PathNotADirectory: 0, CompilerPathMissingQuotes: 0, CompilerModeMismatch: 0, MultiplePathsNotAllowed: 0, MultiplePathsShouldBeSeparated: 0 }; const isWindows: boolean = os.platform() === 'win32'; // TODO: Add other squiggles. @@ -1962,8 +1962,8 @@ export class CppProperties { const forcedeIncludeEnd: number = forcedIncludeStart === -1 ? -1 : curText.indexOf("]", forcedIncludeStart); const compileCommandsStart: number = curText.search(/\s*\"compileCommands\"\s*:\s*\"/); const compileCommandsEnd: number = compileCommandsStart === -1 ? -1 : curText.indexOf('"', curText.indexOf('"', curText.indexOf(":", compileCommandsStart)) + 1); - // const compileCommandsArrayStart: number = curText.search(/\s*\"compileCommands\"\s*:\s*\[/); - // const compileCommandsArrayEnd: number = compileCommandsArrayStart === -1 ? -1 : curText.indexOf("]", curText.indexOf("[", curText.indexOf(":", compileCommandsArrayStart)) + 1); + const compileCommandsArrayStart: number = curText.search(/\s*\"compileCommands\"\s*:\s*\[/); + const compileCommandsArrayEnd: number = compileCommandsArrayStart === -1 ? -1 : curText.indexOf("]", curText.indexOf("[", curText.indexOf(":", compileCommandsArrayStart)) + 1); const compilerPathStart: number = curText.search(/\s*\"compilerPath\"\s*:\s*\"/); const compilerPathValueStart: number = curText.indexOf('"', curText.indexOf(":", compilerPathStart)); const compilerPathEnd: number = compilerPathStart === -1 ? -1 : curText.indexOf('"', compilerPathValueStart + 1) + 1; @@ -2153,6 +2153,19 @@ export class CppProperties { continue; } + message = localize("path.is.not.a.file", "Path is not a file: {0}", expandedPaths[0]); + newSquiggleMetrics.PathNotAFile++; + } + } else if (curOffset >= compileCommandsArrayStart && curOffset <= compileCommandsArrayEnd) { + if (expandedPaths.length > 1) { + message = localize("multiple.paths.should.be.separate.entries", "Multiple paths should be separate entries in the array."); + newSquiggleMetrics.MultiplePathsShouldBeSeparated++; + } else { + const resolvedPath = this.resolvePath(expandedPaths[0]); + if (util.checkFileExistsSync(resolvedPath)) { + continue; + } + message = localize("path.is.not.a.file", "Path is not a file: {0}", expandedPaths[0]); newSquiggleMetrics.PathNotAFile++; } @@ -2235,6 +2248,9 @@ export class CppProperties { if (newSquiggleMetrics.MultiplePathsNotAllowed !== this.prevSquiggleMetrics.get(currentConfiguration.name)?.MultiplePathsNotAllowed) { changedSquiggleMetrics.MultiplePathsNotAllowed = newSquiggleMetrics.MultiplePathsNotAllowed; } + if (newSquiggleMetrics.MultiplePathsShouldBeSeparated !== this.prevSquiggleMetrics.get(currentConfiguration.name)?.MultiplePathsShouldBeSeparated) { + changedSquiggleMetrics.MultiplePathsShouldBeSeparated = newSquiggleMetrics.MultiplePathsShouldBeSeparated; + } if (Object.keys(changedSquiggleMetrics).length > 0) { telemetry.logLanguageServerEvent("ConfigSquiggles", undefined, changedSquiggleMetrics); } From d2bf51d66bc4e5b7737f61a7cad4202b1de64de0 Mon Sep 17 00:00:00 2001 From: yiftahw <63462505+yiftahw@users.noreply.github.com> Date: Thu, 14 Nov 2024 21:30:20 +0200 Subject: [PATCH 15/17] paths should be separated telemetry enabled for a single string --- Extension/src/LanguageServer/configurations.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Extension/src/LanguageServer/configurations.ts b/Extension/src/LanguageServer/configurations.ts index c7af63acd2..8ff027a944 100644 --- a/Extension/src/LanguageServer/configurations.ts +++ b/Extension/src/LanguageServer/configurations.ts @@ -2142,8 +2142,7 @@ export class CppProperties { newSquiggleMetrics.PathNonExistent++; } else { // Check for file versus path mismatches. - if ((curOffset >= forcedIncludeStart && curOffset <= forcedeIncludeEnd) || - (curOffset >= compileCommandsStart && curOffset <= compileCommandsEnd)) { + if (curOffset >= forcedIncludeStart && curOffset <= forcedeIncludeEnd) { if (expandedPaths.length > 1) { message = localize("multiple.paths.not.allowed", "Multiple paths are not allowed."); newSquiggleMetrics.MultiplePathsNotAllowed++; @@ -2156,9 +2155,10 @@ export class CppProperties { message = localize("path.is.not.a.file", "Path is not a file: {0}", expandedPaths[0]); newSquiggleMetrics.PathNotAFile++; } - } else if (curOffset >= compileCommandsArrayStart && curOffset <= compileCommandsArrayEnd) { + } else if ((curOffset >= compileCommandsStart && curOffset <= compileCommandsEnd) || + (curOffset >= compileCommandsArrayStart && curOffset <= compileCommandsArrayEnd)) { if (expandedPaths.length > 1) { - message = localize("multiple.paths.should.be.separate.entries", "Multiple paths should be separate entries in the array."); + message = localize("multiple.paths.should.be.separate.entries", "Multiple paths should be separate entries in an array."); newSquiggleMetrics.MultiplePathsShouldBeSeparated++; } else { const resolvedPath = this.resolvePath(expandedPaths[0]); From 6aa66f66fc192f0b809d411f3ac89d30b135ff38 Mon Sep 17 00:00:00 2001 From: yiftahw <63462505+yiftahw@users.noreply.github.com> Date: Thu, 14 Nov 2024 21:49:00 +0200 Subject: [PATCH 16/17] yarn lint --- Extension/src/LanguageServer/configurations.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Extension/src/LanguageServer/configurations.ts b/Extension/src/LanguageServer/configurations.ts index 8ff027a944..f0dc73b659 100644 --- a/Extension/src/LanguageServer/configurations.ts +++ b/Extension/src/LanguageServer/configurations.ts @@ -1426,7 +1426,7 @@ export class CppProperties { return filteredArray; } } - return undefined + return undefined; } private parsePropertiesFile(): boolean { @@ -1947,7 +1947,7 @@ export class CppProperties { } currentConfiguration.compileCommands?.forEach((file: string) => { - paths.push(`${file}`) + paths.push(`${file}`); }); if (currentConfiguration.compilerPath) { From 45bb04197067aa3761d915c48dd0a8745ff0477c Mon Sep 17 00:00:00 2001 From: yiftahw <63462505+yiftahw@users.noreply.github.com> Date: Thu, 14 Nov 2024 23:12:43 +0200 Subject: [PATCH 17/17] update UI to handle multiple compile commands files --- Extension/src/LanguageServer/settingsPanel.ts | 2 +- Extension/ui/settings.html | 7 ++++--- Extension/ui/settings.ts | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/Extension/src/LanguageServer/settingsPanel.ts b/Extension/src/LanguageServer/settingsPanel.ts index 4522091d02..a13ef8109c 100644 --- a/Extension/src/LanguageServer/settingsPanel.ts +++ b/Extension/src/LanguageServer/settingsPanel.ts @@ -337,7 +337,7 @@ export class SettingsPanel { this.configValues.macFrameworkPath = splitEntries(message.value); break; case elementId.compileCommands: - this.configValues.compileCommands = message.value || undefined; + this.configValues.compileCommands = splitEntries(message.value); break; case elementId.dotConfig: this.configValues.dotConfig = message.value || undefined; diff --git a/Extension/ui/settings.html b/Extension/ui/settings.html index 01ea0d9e89..8abe8ef48c 100644 --- a/Extension/ui/settings.html +++ b/Extension/ui/settings.html @@ -672,11 +672,12 @@
Compile commands
- The full path to the compile_commands.json file for the workspace. The include paths and defines discovered in this file will be used instead of the values set for includePath and defines settings. If the compile commands database does not contain an entry for the translation unit that corresponds to the file you opened in the editor, then a warning message will appear and the extension will use the includePath and defines settings instead. + A list of paths to compile_commands.json files for the workspace. The include paths and defines discovered in these files will be used instead of the values set for includePath and defines settings. If the compile commands database does not contain an entry for the translation unit that corresponds to the file you opened in the editor, then a warning message will appear and the extension will use the includePath and defines settings instead.
- -
+
One compile commands path per line.
+ +
diff --git a/Extension/ui/settings.ts b/Extension/ui/settings.ts index e70516b27a..fc75f4cbdf 100644 --- a/Extension/ui/settings.ts +++ b/Extension/ui/settings.ts @@ -295,7 +295,7 @@ class SettingsApp { // Advanced settings (document.getElementById(elementId.windowsSdkVersion)).value = config.windowsSdkVersion ? config.windowsSdkVersion : ""; (document.getElementById(elementId.macFrameworkPath)).value = joinEntries(config.macFrameworkPath); - (document.getElementById(elementId.compileCommands)).value = config.compileCommands ? config.compileCommands : ""; + (document.getElementById(elementId.compileCommands)).value = joinEntries(config.compileCommands); (document.getElementById(elementId.mergeConfigurations)).checked = config.mergeConfigurations; (document.getElementById(elementId.configurationProvider)).value = config.configurationProvider ? config.configurationProvider : ""; (document.getElementById(elementId.forcedInclude)).value = joinEntries(config.forcedInclude);