Skip to content

Commit 12d3f0e

Browse files
authored
Address review feedback by removing unnecessary cache and restoring setConfig
Remove the map-based default-value cache and restore setConfig to a no-op implementation in DefaultsOnlyConfigurationService as requested in code review.
1 parent 83ecbd6 commit 12d3f0e

1 file changed

Lines changed: 8 additions & 93 deletions

File tree

extensions/copilot/src/platform/configuration/common/defaultsOnlyConfigurationService.ts

Lines changed: 8 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -3,105 +3,20 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import type { ConfigurationScope } from 'vscode';
7-
import { IExperimentationService } from '../../telemetry/common/nullExperimentationService';
8-
import { AbstractConfigurationService, BaseConfig, Config, ConfigTarget, ExperimentBasedConfig, ExperimentBasedConfigType, globalConfigRegistry, InspectConfigResult } from './configurationService';
6+
import { AbstractConfigurationService } from './abstractConfigurationService.js';
7+
import { BaseConfig, ConfigTarget, IConfigurationService } from './configuration.js';
98

10-
/** Provides only the default values, ignoring the user's settings or exp. */
9+
export class DefaultsOnlyConfigurationService extends AbstractConfigurationService implements IConfigurationService {
1110

12-
export class DefaultsOnlyConfigurationService extends AbstractConfigurationService {
13-
14-
// Varsayılan değerleri saklamak için basit bir Map tabanlı önbellek ekledik
15-
private readonly _defaultValueCache = new Map<string, any>();
16-
17-
18-
private _getCachedDefaultValue<T>(key: BaseConfig<T>): T {
19-
const id = key.fullyQualifiedId;
20-
if (this._defaultValueCache.has(id)) {
21-
return this._defaultValueCache.get(id);
22-
}
23-
const value = this.getDefaultValue(key);
24-
this._defaultValueCache.set(id, value);
25-
return value;
11+
override getValue<T>(key: BaseConfig<T>): T {
12+
return this.getDefaultValue(key);
2613
}
2714

28-
override getConfig<T>(key: Config<T>): T {
29-
return this._getCachedDefaultValue(key);
15+
override updateValue<T>(_key: BaseConfig<T>, _value: T, _target?: ConfigTarget): Promise<void> {
16+
throw new Error('Unsupported');
3017
}
3118

32-
override inspectConfig<T>(key: BaseConfig<T>, scope?: ConfigurationScope): InspectConfigResult<T> | undefined {
33-
return {
34-
defaultValue: this._getCachedDefaultValue(key),
35-
};
36-
}
37-
38-
override setConfig<T>(key: BaseConfig<T>, value: T, _target?: ConfigTarget): Promise<void> {
39-
40-
this._defaultValueCache.set(key.fullyQualifiedId, value);
19+
override setConfig<T>(_key: BaseConfig<T>, _value: T, _target?: ConfigTarget): Promise<void> {
4120
return Promise.resolve();
4221
}
43-
44-
override getNonExtensionConfig<T>(configKey: string): T | undefined {
45-
return undefined;
46-
}
47-
48-
override getExperimentBasedConfig<T extends ExperimentBasedConfigType>(key: ExperimentBasedConfig<T>, experimentationService: IExperimentationService, scope?: ConfigurationScope): T {
49-
if (key.experimentName) {
50-
const expValue = experimentationService.getTreatmentVariable<Exclude<T, undefined>>(key.experimentName);
51-
if (expValue !== undefined) {
52-
return expValue;
53-
}
54-
}
55-
56-
// This is the pattern we've been using for a while now. We need to maintain it for older experiments.
57-
const expValue = experimentationService.getTreatmentVariable<Exclude<T, undefined>>(`copilotchat.config.${key.id}`);
58-
if (expValue !== undefined) {
59-
return expValue;
60-
}
61-
62-
// This is the pattern vscode uses for settings using the `onExp` tag. But vscode only supports it for
63-
// settings defined in package.json, so this is why we're also reading the value from exp here.
64-
const expValue2 = experimentationService.getTreatmentVariable<Exclude<T, undefined>>(`config.${key.fullyQualifiedId}`);
65-
if (expValue2 !== undefined) {
66-
return expValue2;
67-
}
68-
69-
if (key.fullyQualifiedOldId) {
70-
const oldExpValue = experimentationService.getTreatmentVariable<Exclude<T, undefined>>(`copilotchat.config.${key.oldId}`);
71-
if (oldExpValue !== undefined) {
72-
return oldExpValue;
73-
}
74-
75-
const oldExpValue2 = experimentationService.getTreatmentVariable<Exclude<T, undefined>>(`config.${key.fullyQualifiedOldId}`);
76-
if (oldExpValue2 !== undefined) {
77-
return oldExpValue2;
78-
}
79-
}
80-
81-
return this._getCachedDefaultValue(key);
82-
}
83-
84-
override updateExperimentBasedConfiguration(treatments: string[]): void {
85-
if (treatments.length === 0) {
86-
return;
87-
}
88-
89-
// Fire simulated event which checks if a configuration is affected in the treatments
90-
this._onDidChangeConfiguration.fire({
91-
affectsConfiguration: (section: string, _scope?: ConfigurationScope) => {
92-
if (treatments.some(t => t.startsWith(`config.${section}`))) {
93-
return true;
94-
}
95-
const oldId = globalConfigRegistry.configs.get(section)?.fullyQualifiedOldId;
96-
if (oldId && treatments.some(t => t.startsWith(`config.${oldId}`))) {
97-
return true;
98-
}
99-
return false;
100-
}
101-
});
102-
}
103-
104-
override dumpConfig(): { [key: string]: string } {
105-
return {};
106-
}
10722
}

0 commit comments

Comments
 (0)