Skip to content

Commit bd97021

Browse files
sean-mcmanussean-mcmanus
authored
Apply mergeConfigurations for browse.path, auto-update the config, and avoid accumulating old configs (#13678)
* Apply mergeConfigurations for browse.path. * Fix a bug with old merge properties being added.
1 parent ad8c3d8 commit bd97021

File tree

7 files changed

+104
-28
lines changed

7 files changed

+104
-28
lines changed

‎Extension/c_cpp_properties.schema.json

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -180,12 +180,9 @@
180180
"type": "string"
181181
},
182182
"mergeConfigurations": {
183-
"markdownDescription": "Set to `true` to merge include paths, defines, and forced includes with those from a configuration provider.",
184-
"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.",
185-
"type": [
186-
"boolean",
187-
"string"
188-
]
183+
"markdownDescription": "Set to `true` to merge `includePath`, `defines`, `forcedInclude`, and `browse.path` with those received from the configuration provider.",
184+
"descriptionHint": "{Locked=\"`true`\"} {Locked=\"`includePath`\"} {Locked=\"`defines`\"} {Locked=\"`forcedInclude`\"} {Locked=\"`browse.path`\"}",
185+
"type": "boolean"
189186
},
190187
"browse": {
191188
"type": "object",

‎Extension/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -841,6 +841,7 @@
841841
},
842842
"C_Cpp.default.mergeConfigurations": {
843843
"type": "boolean",
844+
"default": false,
844845
"markdownDescription": "%c_cpp.configuration.default.mergeConfigurations.markdownDescription%",
845846
"scope": "resource"
846847
},

‎Extension/src/LanguageServer/client.ts

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import { CloseAction, DidOpenTextDocumentParams, ErrorAction, LanguageClientOpti
3232
import { LanguageClient, ServerOptions } from 'vscode-languageclient/node';
3333
import * as nls from 'vscode-nls';
3434
import { DebugConfigurationProvider } from '../Debugger/configurationProvider';
35-
import { CustomConfigurationProvider1, getCustomConfigProviders, isSameProviderExtensionId } from '../LanguageServer/customProviders';
3635
import { ManualPromise } from '../Utility/Async/manualPromise';
3736
import { ManualSignal } from '../Utility/Async/manualSignal';
3837
import { logAndReturn } from '../Utility/Async/returns';
@@ -57,6 +56,7 @@ import {
5756
import { Location, TextEdit, WorkspaceEdit } from './commonTypes';
5857
import * as configs from './configurations';
5958
import { CopilotCompletionContextFeatures, CopilotCompletionContextProvider } from './copilotCompletionContextProvider';
59+
import { CustomConfigurationProvider1, getCustomConfigProviders, isSameProviderExtensionId } from './customProviders';
6060
import { DataBinding } from './dataBinding';
6161
import { cachedEditorConfigSettings, getEditorConfigSettings } from './editorConfig';
6262
import { CppSourceStr, clients, configPrefix, initializeIntervalTimer, updateLanguageConfigurations, usesCrashHandler, watchForCrashes } from './extension';
@@ -889,6 +889,11 @@ export class DefaultClient implements Client {
889889
private settingsTracker: SettingsTracker;
890890
private loggingLevel: number = 1;
891891
private configurationProvider?: string;
892+
private mergeConfigurations: boolean = false;
893+
private includePath?: string[];
894+
private defines?: string[];
895+
private forcedInclude?: string[];
896+
private browsePath?: string[];
892897
private hoverProvider: HoverProvider | undefined;
893898
private copilotHoverProvider: CopilotHoverProvider | undefined;
894899
private copilotCompletionProvider?: CopilotCompletionContextProvider;
@@ -2181,6 +2186,7 @@ export class DefaultClient implements Client {
21812186
if (configs && configs.length > 0 && configs[0]) {
21822187
const fileConfiguration: configs.Configuration | undefined = this.configuration.CurrentConfiguration;
21832188
if (fileConfiguration?.mergeConfigurations) {
2189+
configs = deepCopy(configs);
21842190
configs.forEach(config => {
21852191
if (fileConfiguration.includePath) {
21862192
fileConfiguration.includePath.forEach(p => {
@@ -3139,11 +3145,45 @@ export class DefaultClient implements Client {
31393145
const configName: string | undefined = configurations[params.currentConfiguration].name ?? "";
31403146
this.model.activeConfigName.setValueIfActive(configName);
31413147
const newProvider: string | undefined = this.configuration.CurrentConfigurationProvider;
3142-
if (!isSameProviderExtensionId(newProvider, this.configurationProvider)) {
3143-
if (this.configurationProvider) {
3148+
const previousProvider: string | undefined = this.configurationProvider;
3149+
let updateCustomConfigs: boolean = false;
3150+
if (!isSameProviderExtensionId(previousProvider, newProvider)) {
3151+
this.configurationProvider = newProvider;
3152+
updateCustomConfigs = true;
3153+
}
3154+
if (newProvider !== undefined) {
3155+
const newMergeConfigurations: boolean = this.configuration.CurrentMergeConfigurations;
3156+
if (this.mergeConfigurations !== newMergeConfigurations) {
3157+
this.mergeConfigurations = newMergeConfigurations;
3158+
updateCustomConfigs = true;
3159+
}
3160+
if (newMergeConfigurations) {
3161+
const newIncludePath: string[] | undefined = this.configuration.CurrentIncludePath;
3162+
if (!util.equals(this.includePath, newIncludePath)) {
3163+
this.includePath = newIncludePath;
3164+
updateCustomConfigs = true;
3165+
}
3166+
const newDefines: string[] | undefined = this.configuration.CurrentDefines;
3167+
if (!util.equals(this.defines, newDefines)) {
3168+
this.defines = newDefines;
3169+
updateCustomConfigs = true;
3170+
}
3171+
const newForcedInclude: string[] | undefined = this.configuration.CurrentForcedInclude;
3172+
if (!util.equals(this.forcedInclude, newForcedInclude)) {
3173+
this.forcedInclude = newForcedInclude;
3174+
updateCustomConfigs = true;
3175+
}
3176+
const newBrowsePath: string[] | undefined = this.configuration.CurrentBrowsePath;
3177+
if (!util.equals(this.browsePath, newBrowsePath)) {
3178+
this.browsePath = newBrowsePath;
3179+
updateCustomConfigs = true;
3180+
}
3181+
}
3182+
}
3183+
if (updateCustomConfigs) {
3184+
if (previousProvider) {
31443185
void this.clearCustomBrowseConfiguration().catch(logAndReturn.undefined);
31453186
}
3146-
this.configurationProvider = newProvider;
31473187
void this.updateCustomBrowseConfiguration().catch(logAndReturn.undefined);
31483188
void this.updateCustomConfigurations().catch(logAndReturn.undefined);
31493189
}

‎Extension/src/LanguageServer/configurations.ts

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ export interface Configuration {
8383
forcedInclude?: string[];
8484
configurationProviderInCppPropertiesJson?: string;
8585
configurationProvider?: string;
86-
mergeConfigurations?: boolean | string;
86+
mergeConfigurations?: boolean;
8787
browse?: Browse;
8888
recursiveIncludes?: RecursiveIncludes;
8989
customConfigurationVariables?: { [key: string]: string };
@@ -204,6 +204,41 @@ export class CppProperties {
204204
return new CppSettings(this.rootUri).defaultConfigurationProvider;
205205
}
206206

207+
public get CurrentMergeConfigurations(): boolean {
208+
if (this.CurrentConfiguration?.mergeConfigurations) {
209+
return this.CurrentConfiguration.mergeConfigurations;
210+
}
211+
return new CppSettings(this.rootUri).defaultMergeConfigurations;
212+
}
213+
214+
public get CurrentIncludePath(): string[] | undefined {
215+
if (this.CurrentConfiguration?.includePath) {
216+
return this.CurrentConfiguration.includePath;
217+
}
218+
return new CppSettings(this.rootUri).defaultIncludePath;
219+
}
220+
221+
public get CurrentDefines(): string[] | undefined {
222+
if (this.CurrentConfiguration?.defines) {
223+
return this.CurrentConfiguration.defines;
224+
}
225+
return new CppSettings(this.rootUri).defaultDefines;
226+
}
227+
228+
public get CurrentForcedInclude(): string[] | undefined {
229+
if (this.CurrentConfiguration?.forcedInclude) {
230+
return this.CurrentConfiguration.forcedInclude;
231+
}
232+
return new CppSettings(this.rootUri).defaultForcedInclude;
233+
}
234+
235+
public get CurrentBrowsePath(): string[] | undefined {
236+
if (this.CurrentConfiguration?.browse?.path) {
237+
return this.CurrentConfiguration.browse.path;
238+
}
239+
return new CppSettings(this.rootUri).defaultBrowsePath;
240+
}
241+
207242
public get ConfigurationNames(): string[] | undefined {
208243
const result: string[] = [];
209244
if (this.configurationJson) {

‎Extension/src/LanguageServer/settings.ts

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,7 @@ export class CppSettings extends Settings {
440440
public get defaultCStandard(): string | undefined { return this.getAsStringOrUndefined("default.cStandard"); }
441441
public get defaultCppStandard(): string | undefined { return this.getAsStringOrUndefined("default.cppStandard"); }
442442
public get defaultConfigurationProvider(): string | undefined { return changeBlankStringToUndefined(this.getAsStringOrUndefined("default.configurationProvider")); }
443-
public get defaultMergeConfigurations(): boolean | undefined { return this.getAsBooleanOrUndefined("default.mergeConfigurations"); }
443+
public get defaultMergeConfigurations(): boolean { return this.getAsBoolean("default.mergeConfigurations"); }
444444
public get defaultBrowsePath(): string[] | undefined { return this.getArrayOfStringsWithUndefinedDefault("default.browse.path"); }
445445
public get defaultDatabaseFilename(): string | undefined { return changeBlankStringToUndefined(this.getAsStringOrUndefined("default.browse.databaseFilename")); }
446446
public get defaultLimitSymbolsToIncludedHeaders(): boolean { return this.getAsBoolean("default.browse.limitSymbolsToIncludedHeaders"); }
@@ -569,21 +569,6 @@ export class CppSettings extends Settings {
569569
return undefined;
570570
}
571571

572-
// Returns the value of a setting as a boolean with proper type validation and checks for valid enum values while returning an undefined value if necessary.
573-
private getAsBooleanOrUndefined(settingName: string): boolean | undefined {
574-
const value: any = super.Section.get<any>(settingName);
575-
const setting = getRawSetting("C_Cpp." + settingName, true);
576-
if (setting.default !== undefined) {
577-
console.error(`Default value for ${settingName} is expected to be undefined.`);
578-
}
579-
580-
if (isBoolean(value)) {
581-
return value;
582-
}
583-
584-
return undefined;
585-
}
586-
587572
private isValidDefault(isValid: (x: any) => boolean, value: any, allowNull: boolean): boolean {
588573
return isValid(value) || (allowNull && value === null);
589574
}

‎Extension/src/common.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1810,3 +1810,21 @@ export function findExePathInArgs(args: CommandString[]): string | undefined {
18101810
export function getVsCodeVersion(): number[] {
18111811
return vscode.version.split('.').map(num => parseInt(num, undefined));
18121812
}
1813+
1814+
export function equals(array1: string[] | undefined, array2: string[] | undefined): boolean {
1815+
if (array1 === undefined && array2 === undefined) {
1816+
return true;
1817+
}
1818+
if (array1 === undefined || array2 === undefined) {
1819+
return false;
1820+
}
1821+
if (array1.length !== array2.length) {
1822+
return false;
1823+
}
1824+
for (let i: number = 0; i < array1.length; ++i) {
1825+
if (array1[i] !== array2[i]) {
1826+
return false;
1827+
}
1828+
}
1829+
return true;
1830+
}

‎Extension/ui/settings.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -687,7 +687,7 @@
687687
<div class="section-title" data-loc-id="merge.configurations">Merge configurations</div>
688688
<div>
689689
<input type="checkbox" id="mergeConfigurations" style="vertical-align: middle; transform: scale(1.5)">
690-
<span data-loc-id="merge.configurations.description">When <code>true</code> (or checked), merge include paths, defines, and forced includes with those from a configuration provider.</span>
690+
<span data-loc-id="merge.configurations.description">When <code>true</code> (or checked), merge <code>includePath</code>, <code>defines</code>, <code>forcedInclude</code>, and <code>browse.path</code> with those received from the configuration provider.</span>
691691
</input>
692692
</div>
693693
</div>

0 commit comments

Comments
 (0)