From 96c64c96dc3267b4add8adf393673552f7de7f2e Mon Sep 17 00:00:00 2001 From: "Bruce A. MacNaughton" Date: Fri, 1 Jan 2021 13:54:27 -0800 Subject: [PATCH 01/11] add addNodeAddonIncludePaths --- Extension/package.json | 8 ++- Extension/package.nls.json | 1 + .../src/LanguageServer/configurations.ts | 51 ++++++++++++++++++- Extension/src/LanguageServer/settings.ts | 1 + 4 files changed, 59 insertions(+), 2 deletions(-) diff --git a/Extension/package.json b/Extension/package.json index 327895d60c..9966637ab7 100644 --- a/Extension/package.json +++ b/Extension/package.json @@ -1102,6 +1102,12 @@ "markdownDescription": "%c_cpp.configuration.vcpkg.enabled.markdownDescription%", "scope": "resource" }, + "C_Cpp.addNodeAddonIncludePaths": { + "type": "boolean", + "default": false, + "markdownDescription": "%c_cpp.configuration.addNodeAddonIncludePaths.description%", + "scope": "machine-overridable" + }, "C_Cpp.renameRequiresIdentifier": { "type": "boolean", "default": true, @@ -2679,4 +2685,4 @@ "integrity": "CF1A01AA75275F76800F6BC1D289F2066DCEBCD983376D344ABF6B03FDB8FEA0" } ] -} \ No newline at end of file +} diff --git a/Extension/package.nls.json b/Extension/package.nls.json index 60486fda1e..6e155b04d3 100644 --- a/Extension/package.nls.json +++ b/Extension/package.nls.json @@ -160,6 +160,7 @@ "c_cpp.configuration.enhancedColorization.description": "If enabled, code is colorized based on IntelliSense. This setting only applies if intelliSenseEngine is set to \"Default\".", "c_cpp.configuration.codeFolding.description": "If enabled, code folding ranges are provided by the language server.", "c_cpp.configuration.vcpkg.enabled.markdownDescription": "Enable integration services for the [vcpkg dependency manager](https://aka.ms/vcpkg/).", + "c_cpp.configuration.addNodeAddonIncludePaths.description": "Add include paths from nan and node-addon-api when they're dependencies.", "c_cpp.configuration.renameRequiresIdentifier.description": "If true, 'Rename Symbol' will require a valid C/C++ identifier.", "c_cpp.configuration.debugger.useBacktickCommandSubstitution.description": "If true, debugger shell command substitution will use obsolete backtick (`).", "c_cpp.contributes.views.cppReferencesView.title": "C/C++: Other references results", diff --git a/Extension/src/LanguageServer/configurations.ts b/Extension/src/LanguageServer/configurations.ts index 28a0b6d836..186e099aec 100644 --- a/Extension/src/LanguageServer/configurations.ts +++ b/Extension/src/LanguageServer/configurations.ts @@ -21,6 +21,8 @@ import * as nls from 'vscode-nls'; import { setTimeout } from 'timers'; import * as which from 'which'; +const pfs = fs.promises; + nls.config({ messageFormat: nls.MessageFormat.bundle, bundleFormat: nls.BundleFormat.standalone })(); const localize: nls.LocalizeFunc = nls.loadMessageBundle(); @@ -130,6 +132,8 @@ export class CppProperties { private defaultWindowsSdkVersion: string | null = null; private vcpkgIncludes: string[] = []; private vcpkgPathReady: boolean = false; + private nodeAddonIncludes: string[] = []; + private nodeAddonIncludePathsReady: boolean = false; private defaultIntelliSenseMode?: string; private defaultCustomConfigurationVariables?: { [key: string]: string }; private readonly configurationGlobPattern: string = "c_cpp_properties.json"; @@ -155,6 +159,7 @@ export class CppProperties { this.configFolder = path.join(rootPath, ".vscode"); this.diagnosticCollection = vscode.languages.createDiagnosticCollection(rootPath); this.buildVcpkgIncludePath(); + this.addNodeAddonIncludeLocations(rootPath); this.disposables.push(vscode.Disposable.from(this.configurationsChanged, this.selectionChanged, this.compileCommandsChanged)); } @@ -288,7 +293,8 @@ export class CppProperties { } private applyDefaultIncludePathsAndFrameworks(): void { - if (this.configurationIncomplete && this.defaultIncludes && this.defaultFrameworks && this.vcpkgPathReady) { + if (this.configurationIncomplete && this.defaultIncludes && this.defaultFrameworks && this.vcpkgPathReady + && this.nodeAddonIncludePathsReady) { const configuration: Configuration | undefined = this.CurrentConfiguration; if (configuration) { this.applyDefaultConfigurationValues(configuration); @@ -317,6 +323,9 @@ export class CppProperties { } else { configuration.includePath = [defaultFolder]; } + // check settings.addNodeAddonIncludePaths? + configuration.includePath = configuration.includePath.concat(this.nodeAddonIncludes); + // browse.path is not set by default anymore. When it is not set, the includePath will be used instead. if (isUnset(settings.defaultDefines)) { configuration.defines = (process.platform === 'win32') ? ["_DEBUG", "UNICODE", "_UNICODE"] : []; @@ -386,6 +395,46 @@ export class CppProperties { } } + private nodeAddonMap: {[dependency: string]: string} = { + "nan": "node --no-warnings -e \"require('nan')\"", + "node-addon-api": "node --no-warnings -p \"require('node-addon-api').include\"", + }; + + private async addNodeAddonIncludeLocations(rootPath: string) : Promise { + try { + + const settings: CppSettings = new CppSettings(this.rootUri); + if (settings.addNodeAddonIncludePaths) { + const package_json = JSON.parse(await pfs.readFile(path.join(rootPath, "package.json"), "utf8")); + dependencyLoop: + for (const dep in this.nodeAddonMap) { + if (dep in package_json.dependencies) { + const execCmd: string = this.nodeAddonMap[dep]; + let stdout = await util.execChildProcess(execCmd, rootPath); + if (!stdout) continue dependencyLoop; + + // cleanup newlines + if (stdout[stdout.length - 1] === "\n") { + stdout = stdout.slice(0, -1); + } + // node-addon-api returns a quoted string, e.g., '"/home/user/dir/node_modules/node-addon-api"'. + if (stdout[0] === "\"" && stdout[stdout.length - 1] === "\"") { + stdout = stdout.slice(1, -1); + } + if (stdout) { + this.nodeAddonIncludes.push(stdout); + } + } + } + } + } catch (error) { + console.log(error); + } finally { + this.nodeAddonIncludePathsReady = true; + this.handleConfigurationChange(); + } + } + private getConfigIndexForPlatform(config: any): number | undefined { if (!this.configurationJson) { return undefined; diff --git a/Extension/src/LanguageServer/settings.ts b/Extension/src/LanguageServer/settings.ts index 5e9e492422..14f02f0bd0 100644 --- a/Extension/src/LanguageServer/settings.ts +++ b/Extension/src/LanguageServer/settings.ts @@ -141,6 +141,7 @@ export class CppSettings extends Settings { public get preferredPathSeparator(): string | undefined { return super.Section.get("preferredPathSeparator"); } public get updateChannel(): string | undefined { return super.Section.get("updateChannel"); } public get vcpkgEnabled(): boolean | undefined { return super.Section.get("vcpkg.enabled"); } + public get addNodeAddonIncludePaths(): boolean | undefined { return super.Section.get("addNodeAddonIncludePaths"); } public get renameRequiresIdentifier(): boolean | undefined { return super.Section.get("renameRequiresIdentifier"); } public get defaultIncludePath(): string[] | undefined { return super.Section.get("default.includePath"); } public get defaultDefines(): string[] | undefined { return super.Section.get("default.defines"); } From e3797bdd926807080dd8a1e6643a5ef902db689c Mon Sep 17 00:00:00 2001 From: "Bruce A. MacNaughton" Date: Thu, 7 Jan 2021 07:42:53 -0800 Subject: [PATCH 02/11] partial review comments fixes --- Extension/package.json | 2 +- .../src/LanguageServer/configurations.ts | 57 +++++++++++-------- 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/Extension/package.json b/Extension/package.json index 9966637ab7..25958323c1 100644 --- a/Extension/package.json +++ b/Extension/package.json @@ -1106,7 +1106,7 @@ "type": "boolean", "default": false, "markdownDescription": "%c_cpp.configuration.addNodeAddonIncludePaths.description%", - "scope": "machine-overridable" + "scope": "resource" }, "C_Cpp.renameRequiresIdentifier": { "type": "boolean", diff --git a/Extension/src/LanguageServer/configurations.ts b/Extension/src/LanguageServer/configurations.ts index 186e099aec..6b6b85a5ea 100644 --- a/Extension/src/LanguageServer/configurations.ts +++ b/Extension/src/LanguageServer/configurations.ts @@ -21,7 +21,7 @@ import * as nls from 'vscode-nls'; import { setTimeout } from 'timers'; import * as which from 'which'; -const pfs = fs.promises; +const pfs: any = fs.promises; nls.config({ messageFormat: nls.MessageFormat.bundle, bundleFormat: nls.BundleFormat.standalone })(); const localize: nls.LocalizeFunc = nls.loadMessageBundle(); @@ -323,8 +323,9 @@ export class CppProperties { } else { configuration.includePath = [defaultFolder]; } - // check settings.addNodeAddonIncludePaths? - configuration.includePath = configuration.includePath.concat(this.nodeAddonIncludes); + if (isUnset(settings.addNodeAddonIncludePaths)) { + configuration.includePath = configuration.includePath.concat(this.nodeAddonIncludes); + } // browse.path is not set by default anymore. When it is not set, the includePath will be used instead. if (isUnset(settings.defaultDefines)) { @@ -396,39 +397,45 @@ export class CppProperties { } private nodeAddonMap: {[dependency: string]: string} = { - "nan": "node --no-warnings -e \"require('nan')\"", - "node-addon-api": "node --no-warnings -p \"require('node-addon-api').include\"", + "nan": "node --no-warnings -e \"require('nan')\"", + "node-addon-api": "node --no-warnings -p \"require('node-addon-api').include\"" }; - private async addNodeAddonIncludeLocations(rootPath: string) : Promise { + private async addNodeAddonIncludeLocations(rootPath: string): Promise { try { const settings: CppSettings = new CppSettings(this.rootUri); if (settings.addNodeAddonIncludePaths) { - const package_json = JSON.parse(await pfs.readFile(path.join(rootPath, "package.json"), "utf8")); - dependencyLoop: + const package_json: any = JSON.parse(await pfs.readFile(path.join(rootPath, "package.json"), "utf8")); for (const dep in this.nodeAddonMap) { - if (dep in package_json.dependencies) { - const execCmd: string = this.nodeAddonMap[dep]; - let stdout = await util.execChildProcess(execCmd, rootPath); - if (!stdout) continue dependencyLoop; - - // cleanup newlines - if (stdout[stdout.length - 1] === "\n") { - stdout = stdout.slice(0, -1); - } - // node-addon-api returns a quoted string, e.g., '"/home/user/dir/node_modules/node-addon-api"'. - if (stdout[0] === "\"" && stdout[stdout.length - 1] === "\"") { - stdout = stdout.slice(1, -1); - } - if (stdout) { - this.nodeAddonIncludes.push(stdout); - } + if (dep in package_json.dependencies) { + const execCmd: string = this.nodeAddonMap[dep]; + let stdout: string = await util.execChildProcess(execCmd, rootPath); + if (!stdout) { + continue; + } + + // cleanup newlines + if (stdout[stdout.length - 1] === "\n") { + stdout = stdout.slice(0, -1); + } + // node-addon-api returns a quoted string, e.g., '"/home/user/dir/node_modules/node-addon-api"'. + if (stdout[0] === "\"" && stdout[stdout.length - 1] === "\"") { + stdout = stdout.slice(1, -1); + } + if (stdout) { + // nan returns the path relative to the working directory + stdout = path.resolve(stdout); + if (!await util.checkDirectoryExists(stdout)) { + continue; + } + this.nodeAddonIncludes.push(stdout); + } } } } } catch (error) { - console.log(error); + console.log(error); } finally { this.nodeAddonIncludePathsReady = true; this.handleConfigurationChange(); From 94127398f71da66e433ed1eb680a66dadb82da91 Mon Sep 17 00:00:00 2001 From: "Bruce A. MacNaughton" Date: Mon, 11 Jan 2021 10:26:01 -0800 Subject: [PATCH 03/11] change scope to "application" --- Extension/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Extension/package.json b/Extension/package.json index 25958323c1..5f4b580e8c 100644 --- a/Extension/package.json +++ b/Extension/package.json @@ -1106,7 +1106,7 @@ "type": "boolean", "default": false, "markdownDescription": "%c_cpp.configuration.addNodeAddonIncludePaths.description%", - "scope": "resource" + "scope": "application" }, "C_Cpp.renameRequiresIdentifier": { "type": "boolean", From 9fb66ac6b7fde91cca578e438d7cafbc677aa342 Mon Sep 17 00:00:00 2001 From: "Bruce A. MacNaughton" Date: Mon, 11 Jan 2021 10:29:34 -0800 Subject: [PATCH 04/11] make setting dynamic - set at init and on changes - always read nan/node-addon-api paths --- .../src/LanguageServer/configurations.ts | 52 ++++++++----------- 1 file changed, 23 insertions(+), 29 deletions(-) diff --git a/Extension/src/LanguageServer/configurations.ts b/Extension/src/LanguageServer/configurations.ts index 6b6b85a5ea..2d42015f45 100644 --- a/Extension/src/LanguageServer/configurations.ts +++ b/Extension/src/LanguageServer/configurations.ts @@ -21,8 +21,6 @@ import * as nls from 'vscode-nls'; import { setTimeout } from 'timers'; import * as which from 'which'; -const pfs: any = fs.promises; - nls.config({ messageFormat: nls.MessageFormat.bundle, bundleFormat: nls.BundleFormat.standalone })(); const localize: nls.LocalizeFunc = nls.loadMessageBundle(); @@ -323,7 +321,8 @@ export class CppProperties { } else { configuration.includePath = [defaultFolder]; } - if (isUnset(settings.addNodeAddonIncludePaths)) { + + if (settings.addNodeAddonIncludePaths) { configuration.includePath = configuration.includePath.concat(this.nodeAddonIncludes); } @@ -404,33 +403,25 @@ export class CppProperties { private async addNodeAddonIncludeLocations(rootPath: string): Promise { try { - const settings: CppSettings = new CppSettings(this.rootUri); - if (settings.addNodeAddonIncludePaths) { - const package_json: any = JSON.parse(await pfs.readFile(path.join(rootPath, "package.json"), "utf8")); - for (const dep in this.nodeAddonMap) { - if (dep in package_json.dependencies) { - const execCmd: string = this.nodeAddonMap[dep]; - let stdout: string = await util.execChildProcess(execCmd, rootPath); - if (!stdout) { - continue; - } + const package_json: any = JSON.parse(await fs.promises.readFile(path.join(rootPath, "package.json"), "utf8")); + for (const dep in this.nodeAddonMap) { + if (dep in package_json.dependencies) { + const execCmd: string = this.nodeAddonMap[dep]; + let stdout: string = await util.execChildProcess(execCmd, rootPath); + if (!stdout) { + continue; + } - // cleanup newlines - if (stdout[stdout.length - 1] === "\n") { - stdout = stdout.slice(0, -1); - } - // node-addon-api returns a quoted string, e.g., '"/home/user/dir/node_modules/node-addon-api"'. - if (stdout[0] === "\"" && stdout[stdout.length - 1] === "\"") { - stdout = stdout.slice(1, -1); - } - if (stdout) { - // nan returns the path relative to the working directory - stdout = path.resolve(stdout); - if (!await util.checkDirectoryExists(stdout)) { - continue; - } - this.nodeAddonIncludes.push(stdout); - } + // cleanup newlines + if (stdout[stdout.length - 1] === "\n") { + stdout = stdout.slice(0, -1); + } + // node-addon-api returns a quoted string, e.g., '"/home/user/dir/node_modules/node-addon-api"'. + if (stdout[0] === "\"" && stdout[stdout.length - 1] === "\"") { + stdout = stdout.slice(1, -1); + } + if (stdout) { + this.nodeAddonIncludes.push(stdout); } } } @@ -683,6 +674,9 @@ export class CppProperties { const configuration: Configuration = this.configurationJson.configurations[i]; configuration.includePath = this.updateConfigurationStringArray(configuration.includePath, settings.defaultIncludePath, env); + if (settings.addNodeAddonIncludePaths) { + configuration.includePath = this.updateConfigurationStringArray(configuration.includePath, this.nodeAddonIncludes, env); + } configuration.defines = this.updateConfigurationStringArray(configuration.defines, settings.defaultDefines, env); configuration.macFrameworkPath = this.updateConfigurationStringArray(configuration.macFrameworkPath, settings.defaultMacFrameworkPath, env); configuration.windowsSdkVersion = this.updateConfigurationString(configuration.windowsSdkVersion, settings.defaultWindowsSdkVersion, env); From ae9b88de0421dbbd081d582591bd834b4ec8de41 Mon Sep 17 00:00:00 2001 From: "Bruce A. MacNaughton" Date: Mon, 11 Jan 2021 14:34:59 -0800 Subject: [PATCH 05/11] restructure addon include reads --- .../src/LanguageServer/configurations.ts | 69 ++++++++++++------- 1 file changed, 45 insertions(+), 24 deletions(-) diff --git a/Extension/src/LanguageServer/configurations.ts b/Extension/src/LanguageServer/configurations.ts index 2d42015f45..557378197b 100644 --- a/Extension/src/LanguageServer/configurations.ts +++ b/Extension/src/LanguageServer/configurations.ts @@ -401,36 +401,57 @@ export class CppProperties { }; private async addNodeAddonIncludeLocations(rootPath: string): Promise { - try { + let error: Error | undefined; + const package_json: any = await fs.promises.readFile(path.join(rootPath, "package.json"), "utf8") + .then(pdj => JSON.parse(pdj)) + .catch(e => (error = e)); - const package_json: any = JSON.parse(await fs.promises.readFile(path.join(rootPath, "package.json"), "utf8")); - for (const dep in this.nodeAddonMap) { - if (dep in package_json.dependencies) { - const execCmd: string = this.nodeAddonMap[dep]; - let stdout: string = await util.execChildProcess(execCmd, rootPath); - if (!stdout) { - continue; - } + if (!error) { + try { + for (const dep in this.nodeAddonMap) { + if (dep in package_json.dependencies) { + const execCmd: string = this.nodeAddonMap[dep]; + let stdout: string = await util.execChildProcess(execCmd, rootPath); + if (!stdout) { + continue; + } - // cleanup newlines - if (stdout[stdout.length - 1] === "\n") { - stdout = stdout.slice(0, -1); - } - // node-addon-api returns a quoted string, e.g., '"/home/user/dir/node_modules/node-addon-api"'. - if (stdout[0] === "\"" && stdout[stdout.length - 1] === "\"") { - stdout = stdout.slice(1, -1); - } - if (stdout) { - this.nodeAddonIncludes.push(stdout); + // cleanup newlines + if (stdout[stdout.length - 1] === "\n") { + stdout = stdout.slice(0, -1); + } + // node-addon-api returns a quoted string, e.g., '"/home/user/dir/node_modules/node-addon-api"'. + if (stdout[0] === "\"" && stdout[stdout.length - 1] === "\"") { + stdout = stdout.slice(1, -1); + } + + // at this time both node-addon-api and nan return "node_modules/" so this test is + // not needed. but this future proofs the code. + if (!await util.checkDirectoryExists(stdout)) { + // convert the path to an absolute path because nan returns a relative path causing the previous test + // to fail because this executes in vscode's working directory. + stdout = path.join(rootPath, stdout); + if (!await util.checkDirectoryExists(stdout)) { + error = new Error(`${dep} directory ${stdout} doesn't exist`); + stdout = ''; + } + } + if (stdout) { + this.nodeAddonIncludes.push(stdout); + } } } + } catch (e) { + error = e; } - } catch (error) { - console.log(error); - } finally { - this.nodeAddonIncludePathsReady = true; - this.handleConfigurationChange(); } + if (error) { + vscode.window.showErrorMessage('error.adding.node.addon.includes', error.message); + } + this.nodeAddonIncludePathsReady = true; + // it's possible that only one of two include paths failed, so proceed with the + // configuration change. + this.handleConfigurationChange(); } private getConfigIndexForPlatform(config: any): number | undefined { From 1dc03a7bac84bb85f1c5a18058bb0530158009fd Mon Sep 17 00:00:00 2001 From: "Bruce A. MacNaughton" Date: Tue, 12 Jan 2021 08:25:47 -0800 Subject: [PATCH 06/11] prompt for reload if no addon paths - make nodeAddonMap a local - fail silently - add nodeAddonIncludesFound() --- Extension/src/LanguageServer/client.ts | 5 +++ .../src/LanguageServer/configurations.ts | 36 +++++++++++-------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/Extension/src/LanguageServer/client.ts b/Extension/src/LanguageServer/client.ts index 9087b726b5..0ef0b69485 100644 --- a/Extension/src/LanguageServer/client.ts +++ b/Extension/src/LanguageServer/client.ts @@ -1367,6 +1367,11 @@ export class DefaultClient implements Client { this.semanticTokensProvider = undefined; } } + // if addNodeAddonIncludePaths was turned on but no includes have been found yet then 1) presume that nan + // or node-addon-api was installed so prompt for reload. + if (changedSettings["addNodeAddonIncludePaths"] && settings.addNodeAddonIncludePaths && this.configuration.nodeAddonIncludesFound() === 0) { + util.promptForReloadWindowDueToSettingsChange(); + } } this.configuration.onDidChangeSettings(); telemetry.logLanguageServerEvent("CppSettingsChange", changedSettings, undefined); diff --git a/Extension/src/LanguageServer/configurations.ts b/Extension/src/LanguageServer/configurations.ts index 557378197b..e189643d4e 100644 --- a/Extension/src/LanguageServer/configurations.ts +++ b/Extension/src/LanguageServer/configurations.ts @@ -157,7 +157,7 @@ export class CppProperties { this.configFolder = path.join(rootPath, ".vscode"); this.diagnosticCollection = vscode.languages.createDiagnosticCollection(rootPath); this.buildVcpkgIncludePath(); - this.addNodeAddonIncludeLocations(rootPath); + this.readNodeAddonIncludeLocations(rootPath); this.disposables.push(vscode.Disposable.from(this.configurationsChanged, this.selectionChanged, this.compileCommandsChanged)); } @@ -395,22 +395,27 @@ export class CppProperties { } } - private nodeAddonMap: {[dependency: string]: string} = { - "nan": "node --no-warnings -e \"require('nan')\"", - "node-addon-api": "node --no-warnings -p \"require('node-addon-api').include\"" - }; + public nodeAddonIncludesFound(): number { + return this.nodeAddonIncludes.length; + } - private async addNodeAddonIncludeLocations(rootPath: string): Promise { + private async readNodeAddonIncludeLocations(rootPath: string): Promise { let error: Error | undefined; const package_json: any = await fs.promises.readFile(path.join(rootPath, "package.json"), "utf8") .then(pdj => JSON.parse(pdj)) .catch(e => (error = e)); + const nodeAddonMap: { [dependency: string]: string } = { + "nan": "node --no-warnings -e \"require('nan')\"", + "node-addon-api": "node --no-warnings -p \"require('node-addon-api').include\"" + }; + + let added: number = 0; if (!error) { try { - for (const dep in this.nodeAddonMap) { + for (const dep in nodeAddonMap) { if (dep in package_json.dependencies) { - const execCmd: string = this.nodeAddonMap[dep]; + const execCmd: string = nodeAddonMap[dep]; let stdout: string = await util.execChildProcess(execCmd, rootPath); if (!stdout) { continue; @@ -425,11 +430,11 @@ export class CppProperties { stdout = stdout.slice(1, -1); } - // at this time both node-addon-api and nan return "node_modules/" so this test is - // not needed. but this future proofs the code. + // at this time both node-addon-api and nan return their own directory so this test is not really + // needed. but it does future proof the code. if (!await util.checkDirectoryExists(stdout)) { - // convert the path to an absolute path because nan returns a relative path causing the previous test - // to fail because this executes in vscode's working directory. + // nan returns a path relative to rootPath causing the previous check to fail because this code + // is executing in vscode's working directory. stdout = path.join(rootPath, stdout); if (!await util.checkDirectoryExists(stdout)) { error = new Error(`${dep} directory ${stdout} doesn't exist`); @@ -437,6 +442,7 @@ export class CppProperties { } } if (stdout) { + added += 1; this.nodeAddonIncludes.push(stdout); } } @@ -446,11 +452,11 @@ export class CppProperties { } } if (error) { - vscode.window.showErrorMessage('error.adding.node.addon.includes', error.message); + // if it's possible for the directories returned by nan or node-addon-api to fail then + // this should be uncommented. + // console.log('readNodeAddonIncludeLocations', error.message); } this.nodeAddonIncludePathsReady = true; - // it's possible that only one of two include paths failed, so proceed with the - // configuration change. this.handleConfigurationChange(); } From 5b28559cfb5d380f6ca1db79ba347820d9e0bb31 Mon Sep 17 00:00:00 2001 From: "Bruce A. MacNaughton" Date: Tue, 12 Jan 2021 13:08:05 -0800 Subject: [PATCH 07/11] log relevant errors - remove experimental artifact --- Extension/src/LanguageServer/configurations.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/Extension/src/LanguageServer/configurations.ts b/Extension/src/LanguageServer/configurations.ts index e189643d4e..05d3c96e27 100644 --- a/Extension/src/LanguageServer/configurations.ts +++ b/Extension/src/LanguageServer/configurations.ts @@ -401,8 +401,9 @@ export class CppProperties { private async readNodeAddonIncludeLocations(rootPath: string): Promise { let error: Error | undefined; + let pdjFound: boolean = false; const package_json: any = await fs.promises.readFile(path.join(rootPath, "package.json"), "utf8") - .then(pdj => JSON.parse(pdj)) + .then(pdj => {pdjFound = true; return JSON.parse(pdj); }) .catch(e => (error = e)); const nodeAddonMap: { [dependency: string]: string } = { @@ -410,7 +411,6 @@ export class CppProperties { "node-addon-api": "node --no-warnings -p \"require('node-addon-api').include\"" }; - let added: number = 0; if (!error) { try { for (const dep in nodeAddonMap) { @@ -442,7 +442,6 @@ export class CppProperties { } } if (stdout) { - added += 1; this.nodeAddonIncludes.push(stdout); } } @@ -451,10 +450,9 @@ export class CppProperties { error = e; } } - if (error) { - // if it's possible for the directories returned by nan or node-addon-api to fail then - // this should be uncommented. - // console.log('readNodeAddonIncludeLocations', error.message); + if (error && pdjFound) { + // only log an error if package.json exists. + console.log('readNodeAddonIncludeLocations', error.message); } this.nodeAddonIncludePathsReady = true; this.handleConfigurationChange(); From 1eabe7726095f4a74a154d5de71372b81266abef Mon Sep 17 00:00:00 2001 From: "Bruce A. MacNaughton" Date: Tue, 12 Jan 2021 15:00:48 -0800 Subject: [PATCH 08/11] use installed node for requires --- Extension/src/LanguageServer/configurations.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Extension/src/LanguageServer/configurations.ts b/Extension/src/LanguageServer/configurations.ts index 05d3c96e27..2d085c7162 100644 --- a/Extension/src/LanguageServer/configurations.ts +++ b/Extension/src/LanguageServer/configurations.ts @@ -406,9 +406,10 @@ export class CppProperties { .then(pdj => {pdjFound = true; return JSON.parse(pdj); }) .catch(e => (error = e)); + const pathToNode: string = which.sync("node"); const nodeAddonMap: { [dependency: string]: string } = { - "nan": "node --no-warnings -e \"require('nan')\"", - "node-addon-api": "node --no-warnings -p \"require('node-addon-api').include\"" + "nan": `${pathToNode} --no-warnings -e "require('nan')"`, + "node-addon-api": `${pathToNode} --no-warnings -p "require('node-addon-api').include"` }; if (!error) { From 0b3c932ddab4f6d3423ba2a2d10aba6406effe8f Mon Sep 17 00:00:00 2001 From: "Bruce A. MacNaughton" Date: Tue, 12 Jan 2021 16:03:43 -0800 Subject: [PATCH 09/11] remove incorrect include concat --- Extension/src/LanguageServer/configurations.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Extension/src/LanguageServer/configurations.ts b/Extension/src/LanguageServer/configurations.ts index 2d085c7162..0fc7a85e60 100644 --- a/Extension/src/LanguageServer/configurations.ts +++ b/Extension/src/LanguageServer/configurations.ts @@ -322,10 +322,6 @@ export class CppProperties { configuration.includePath = [defaultFolder]; } - if (settings.addNodeAddonIncludePaths) { - configuration.includePath = configuration.includePath.concat(this.nodeAddonIncludes); - } - // browse.path is not set by default anymore. When it is not set, the includePath will be used instead. if (isUnset(settings.defaultDefines)) { configuration.defines = (process.platform === 'win32') ? ["_DEBUG", "UNICODE", "_UNICODE"] : []; @@ -701,7 +697,8 @@ export class CppProperties { configuration.includePath = this.updateConfigurationStringArray(configuration.includePath, settings.defaultIncludePath, env); if (settings.addNodeAddonIncludePaths) { - configuration.includePath = this.updateConfigurationStringArray(configuration.includePath, this.nodeAddonIncludes, env); + const includePath: string[] = configuration.includePath || []; + configuration.includePath = includePath.concat(this.nodeAddonIncludes.filter(i => includePath.indexOf(i) < 0)); } configuration.defines = this.updateConfigurationStringArray(configuration.defines, settings.defaultDefines, env); configuration.macFrameworkPath = this.updateConfigurationStringArray(configuration.macFrameworkPath, settings.defaultMacFrameworkPath, env); From 892987c658c3eac68e186bd8bd70466892c10bb4 Mon Sep 17 00:00:00 2001 From: "Bruce A. MacNaughton" Date: Tue, 12 Jan 2021 16:15:59 -0800 Subject: [PATCH 10/11] quote pathToNode --- 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 0fc7a85e60..92b88d647a 100644 --- a/Extension/src/LanguageServer/configurations.ts +++ b/Extension/src/LanguageServer/configurations.ts @@ -404,8 +404,8 @@ export class CppProperties { const pathToNode: string = which.sync("node"); const nodeAddonMap: { [dependency: string]: string } = { - "nan": `${pathToNode} --no-warnings -e "require('nan')"`, - "node-addon-api": `${pathToNode} --no-warnings -p "require('node-addon-api').include"` + "nan": `"${pathToNode}" --no-warnings -e "require('nan')"`, + "node-addon-api": `"${pathToNode}" --no-warnings -p "require('node-addon-api').include"` }; if (!error) { From 04fc19c5eb296d3d0eede0eefbc941ede705aded Mon Sep 17 00:00:00 2001 From: "Bruce A. MacNaughton" Date: Wed, 13 Jan 2021 14:50:44 -0800 Subject: [PATCH 11/11] address review issues - remove unused flag - avoid unnecessary work - handle alternate include setting path --- .../src/LanguageServer/configurations.ts | 37 ++++++++++--------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/Extension/src/LanguageServer/configurations.ts b/Extension/src/LanguageServer/configurations.ts index 92b88d647a..fe86468acd 100644 --- a/Extension/src/LanguageServer/configurations.ts +++ b/Extension/src/LanguageServer/configurations.ts @@ -131,7 +131,6 @@ export class CppProperties { private vcpkgIncludes: string[] = []; private vcpkgPathReady: boolean = false; private nodeAddonIncludes: string[] = []; - private nodeAddonIncludePathsReady: boolean = false; private defaultIntelliSenseMode?: string; private defaultCustomConfigurationVariables?: { [key: string]: string }; private readonly configurationGlobPattern: string = "c_cpp_properties.json"; @@ -291,8 +290,7 @@ export class CppProperties { } private applyDefaultIncludePathsAndFrameworks(): void { - if (this.configurationIncomplete && this.defaultIncludes && this.defaultFrameworks && this.vcpkgPathReady - && this.nodeAddonIncludePathsReady) { + if (this.configurationIncomplete && this.defaultIncludes && this.defaultFrameworks && this.vcpkgPathReady) { const configuration: Configuration | undefined = this.CurrentConfiguration; if (configuration) { this.applyDefaultConfigurationValues(configuration); @@ -402,14 +400,14 @@ export class CppProperties { .then(pdj => {pdjFound = true; return JSON.parse(pdj); }) .catch(e => (error = e)); - const pathToNode: string = which.sync("node"); - const nodeAddonMap: { [dependency: string]: string } = { - "nan": `"${pathToNode}" --no-warnings -e "require('nan')"`, - "node-addon-api": `"${pathToNode}" --no-warnings -p "require('node-addon-api').include"` - }; - if (!error) { try { + const pathToNode: string = which.sync("node"); + const nodeAddonMap: { [dependency: string]: string } = { + "nan": `"${pathToNode}" --no-warnings -e "require('nan')"`, + "node-addon-api": `"${pathToNode}" --no-warnings -p "require('node-addon-api').include"` + }; + for (const dep in nodeAddonMap) { if (dep in package_json.dependencies) { const execCmd: string = nodeAddonMap[dep]; @@ -447,12 +445,14 @@ export class CppProperties { error = e; } } - if (error && pdjFound) { - // only log an error if package.json exists. - console.log('readNodeAddonIncludeLocations', error.message); + if (error) { + if (pdjFound) { + // only log an error if package.json exists. + console.log('readNodeAddonIncludeLocations', error.message); + } + } else { + this.handleConfigurationChange(); } - this.nodeAddonIncludePathsReady = true; - this.handleConfigurationChange(); } private getConfigIndexForPlatform(config: any): number | undefined { @@ -696,8 +696,10 @@ export class CppProperties { const configuration: Configuration = this.configurationJson.configurations[i]; configuration.includePath = this.updateConfigurationStringArray(configuration.includePath, settings.defaultIncludePath, env); + // in case includePath is reset below + const origIncludePath: string[] | undefined = configuration.includePath; if (settings.addNodeAddonIncludePaths) { - const includePath: string[] = configuration.includePath || []; + const includePath: string[] = origIncludePath || []; configuration.includePath = includePath.concat(this.nodeAddonIncludes.filter(i => includePath.indexOf(i) < 0)); } configuration.defines = this.updateConfigurationStringArray(configuration.defines, settings.defaultDefines, env); @@ -736,8 +738,9 @@ export class CppProperties { if (!configuration.windowsSdkVersion && !!this.defaultWindowsSdkVersion) { configuration.windowsSdkVersion = this.defaultWindowsSdkVersion; } - if (!configuration.includePath && !!this.defaultIncludes) { - configuration.includePath = this.defaultIncludes; + if (!origIncludePath && !!this.defaultIncludes) { + const includePath: string[] = configuration.includePath || []; + configuration.includePath = includePath.concat(this.defaultIncludes); } if (!configuration.macFrameworkPath && !!this.defaultFrameworks) { configuration.macFrameworkPath = this.defaultFrameworks;