Skip to content

Commit 9ff1f2a

Browse files
eleanorjboydeleanorjboyd
authored
Enhance environment manager settings updates to avoid unnecessary write to settings.json (#1120)
fixes #1118
1 parent d84e05b commit 9ff1f2a

File tree

2 files changed

+457
-30
lines changed

2 files changed

+457
-30
lines changed

‎src/features/settings/settingHelpers.ts‎

Lines changed: 126 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ export function isDefaultEnvManagerBroken(): boolean {
4242
}
4343

4444
export function getDefaultEnvManagerSetting(wm: PythonProjectManager, scope?: Uri): string {
45-
const config = workspace.getConfiguration('python-envs', scope);
45+
const config = getConfiguration('python-envs', scope);
4646
const settings = getSettings(wm, config, scope);
4747
if (settings && settings.envManager.length > 0) {
4848
return settings.envManager;
@@ -69,7 +69,7 @@ export function getDefaultPkgManagerSetting(
6969
scope?: ConfigurationScope | null,
7070
defaultId?: string,
7171
): string {
72-
const config = workspace.getConfiguration('python-envs', scope);
72+
const config = getConfiguration('python-envs', scope);
7373

7474
const settings = getSettings(wm, config, scope);
7575
if (settings && settings.packageManager.length > 0) {
@@ -127,8 +127,13 @@ export async function setAllManagerSettings(edits: EditAllManagerSettings[]): Pr
127127
const promises: Thenable<void>[] = [];
128128

129129
workspaces.forEach((es, w) => {
130-
const config = workspace.getConfiguration('python-envs', w);
130+
const config = getConfiguration('python-envs', w);
131131
const overrides = config.get<PythonProjectSettings[]>('pythonProjects', []);
132+
const projectsInspect = config.inspect<PythonProjectSettings[]>('pythonProjects');
133+
const existingProjectsSetting =
134+
projectsInspect?.workspaceFolderValue ?? projectsInspect?.workspaceValue ?? undefined;
135+
const originalOverridesLength = overrides.length;
136+
132137
es.forEach((e) => {
133138
const pwPath = path.normalize(e.project.uri.fsPath);
134139
const index = overrides.findIndex((s) => path.resolve(w.uri.fsPath, s.path) === pwPath);
@@ -142,33 +147,72 @@ export async function setAllManagerSettings(edits: EditAllManagerSettings[]): Pr
142147
packageManager: e.packageManager,
143148
});
144149
} else {
145-
if (config.get('defaultEnvManager') !== e.envManager) {
150+
// Only write settings if:
151+
// 1. There's already an explicit setting (we're updating it), OR
152+
// 2. The new value is not the implicit fallback (system manager is the fallback)
153+
const isSystemManager = e.envManager === 'ms-python.python:system';
154+
const envManagerInspect = config.inspect<string>('defaultEnvManager');
155+
const hasExplicitEnvManager =
156+
envManagerInspect?.workspaceFolderValue !== undefined ||
157+
envManagerInspect?.workspaceValue !== undefined;
158+
159+
// Write if changing an existing setting, OR if setting to non-system manager
160+
if ((hasExplicitEnvManager || !isSystemManager) && config.get('defaultEnvManager') !== e.envManager) {
146161
promises.push(config.update('defaultEnvManager', e.envManager, ConfigurationTarget.Workspace));
147162
}
148-
if (config.get('defaultPackageManager') !== e.packageManager) {
163+
164+
const pkgManagerInspect = config.inspect<string>('defaultPackageManager');
165+
const hasExplicitPkgManager =
166+
pkgManagerInspect?.workspaceFolderValue !== undefined ||
167+
pkgManagerInspect?.workspaceValue !== undefined;
168+
// For package manager, write if there's an explicit setting OR if env manager is being written
169+
if (
170+
(hasExplicitPkgManager || !isSystemManager) &&
171+
config.get('defaultPackageManager') !== e.packageManager
172+
) {
149173
promises.push(
150174
config.update('defaultPackageManager', e.packageManager, ConfigurationTarget.Workspace),
151175
);
152176
}
153177
}
154178
});
155-
promises.push(
156-
config.update(
157-
'pythonProjects',
158-
overrides,
159-
workspaceFile ? ConfigurationTarget.WorkspaceFolder : ConfigurationTarget.Workspace,
160-
),
161-
);
179+
180+
// Only write pythonProjects if:
181+
// 1. There was already an explicit setting OR
182+
// 2. adding new project entries
183+
const shouldWriteProjects = existingProjectsSetting !== undefined || overrides.length > originalOverridesLength;
184+
if (shouldWriteProjects) {
185+
promises.push(
186+
config.update(
187+
'pythonProjects',
188+
overrides,
189+
workspaceFile ? ConfigurationTarget.WorkspaceFolder : ConfigurationTarget.Workspace,
190+
),
191+
);
192+
}
162193
});
163194

164-
const config = workspace.getConfiguration('python-envs', undefined);
195+
const config = getConfiguration('python-envs', undefined);
165196
edits
166197
.filter((e) => !e.project)
167198
.forEach((e) => {
168-
if (config.get('defaultEnvManager') !== e.envManager) {
199+
// Only write global settings if:
200+
// 1. There's already an explicit global setting (we're updating it), OR
201+
// 2. The new value is not the implicit fallback (system manager)
202+
const isSystemManager = e.envManager === 'ms-python.python:system';
203+
const envManagerInspect = config.inspect<string>('defaultEnvManager');
204+
const hasExplicitGlobalEnvManager = envManagerInspect?.globalValue !== undefined;
205+
206+
if ((hasExplicitGlobalEnvManager || !isSystemManager) && config.get('defaultEnvManager') !== e.envManager) {
169207
promises.push(config.update('defaultEnvManager', e.envManager, ConfigurationTarget.Global));
170208
}
171-
if (config.get('defaultPackageManager') !== e.packageManager) {
209+
210+
const pkgManagerInspect = config.inspect<string>('defaultPackageManager');
211+
const hasExplicitGlobalPkgManager = pkgManagerInspect?.globalValue !== undefined;
212+
if (
213+
(hasExplicitGlobalPkgManager || !isSystemManager) &&
214+
config.get('defaultPackageManager') !== e.packageManager
215+
) {
172216
promises.push(config.update('defaultPackageManager', e.packageManager, ConfigurationTarget.Global));
173217
}
174218
});
@@ -209,25 +253,47 @@ export async function setEnvironmentManager(edits: EditEnvManagerSettings[]): Pr
209253
const promises: Thenable<void>[] = [];
210254

211255
workspaces.forEach((es, w) => {
212-
const config = workspace.getConfiguration('python-envs', w.uri);
256+
const config = getConfiguration('python-envs', w.uri);
213257
const overrides = config.get<PythonProjectSettings[]>('pythonProjects', []);
258+
const projectsInspect = config.inspect<PythonProjectSettings[]>('pythonProjects');
259+
const existingProjectsSetting = projectsInspect?.workspaceValue ?? undefined;
260+
const originalOverridesLength = overrides.length;
261+
let projectsModified = false;
262+
214263
es.forEach((e) => {
215264
const pwPath = path.normalize(e.project.uri.fsPath);
216265
const index = overrides.findIndex((s) => path.resolve(w.uri.fsPath, s.path) === pwPath);
217266
if (index >= 0) {
218267
overrides[index].envManager = e.envManager;
219-
} else if (config.get('defaultEnvManager') !== e.envManager) {
220-
promises.push(config.update('defaultEnvManager', e.envManager, ConfigurationTarget.Workspace));
268+
projectsModified = true;
269+
} else {
270+
// Only write settings if updating existing OR setting non-system manager
271+
const isSystemManager = e.envManager === 'ms-python.python:system';
272+
const envManagerInspect = config.inspect<string>('defaultEnvManager');
273+
const hasExplicitEnvManager = envManagerInspect?.workspaceValue !== undefined;
274+
if ((hasExplicitEnvManager || !isSystemManager) && config.get('defaultEnvManager') !== e.envManager) {
275+
promises.push(config.update('defaultEnvManager', e.envManager, ConfigurationTarget.Workspace));
276+
}
221277
}
222278
});
223-
promises.push(config.update('pythonProjects', overrides, ConfigurationTarget.Workspace));
279+
280+
// Only write pythonProjects if there was an explicit setting or we modified entries
281+
const shouldWriteProjects =
282+
existingProjectsSetting !== undefined || overrides.length > originalOverridesLength || projectsModified;
283+
if (shouldWriteProjects) {
284+
promises.push(config.update('pythonProjects', overrides, ConfigurationTarget.Workspace));
285+
}
224286
});
225287

226-
const config = workspace.getConfiguration('python-envs', undefined);
288+
const config = getConfiguration('python-envs', undefined);
227289
edits
228290
.filter((e) => !e.project)
229291
.forEach((e) => {
230-
if (config.get('defaultEnvManager') !== e.envManager) {
292+
// Only write global settings if updating existing OR setting non-system manager
293+
const isSystemManager = e.envManager === 'ms-python.python:system';
294+
const envManagerInspect = config.inspect<string>('defaultEnvManager');
295+
const hasExplicitGlobalEnvManager = envManagerInspect?.globalValue !== undefined;
296+
if ((hasExplicitGlobalEnvManager || !isSystemManager) && config.get('defaultEnvManager') !== e.envManager) {
231297
promises.push(config.update('defaultEnvManager', e.envManager, ConfigurationTarget.Global));
232298
}
233299
});
@@ -271,25 +337,55 @@ export async function setPackageManager(edits: EditPackageManagerSettings[]): Pr
271337
const promises: Thenable<void>[] = [];
272338

273339
workspaces.forEach((es, w) => {
274-
const config = workspace.getConfiguration('python-envs', w.uri);
340+
const config = getConfiguration('python-envs', w.uri);
275341
const overrides = config.get<PythonProjectSettings[]>('pythonProjects', []);
342+
const projectsInspect = config.inspect<PythonProjectSettings[]>('pythonProjects');
343+
const existingProjectsSetting = projectsInspect?.workspaceValue ?? undefined;
344+
const originalOverridesLength = overrides.length;
345+
let projectsModified = false;
346+
276347
es.forEach((e) => {
277348
const pwPath = path.normalize(e.project.uri.fsPath);
278349
const index = overrides.findIndex((s) => path.resolve(w.uri.fsPath, s.path) === pwPath);
279350
if (index >= 0) {
280351
overrides[index].packageManager = e.packageManager;
281-
} else if (config.get('defaultPackageManager') !== e.packageManager) {
282-
promises.push(config.update('defaultPackageManager', e.packageManager, ConfigurationTarget.Workspace));
352+
projectsModified = true;
353+
} else {
354+
// Only write settings if updating existing OR setting non-default package manager
355+
const isPipManager = e.packageManager === 'ms-python.python:pip';
356+
const pkgManagerInspect = config.inspect<string>('defaultPackageManager');
357+
const hasExplicitPkgManager = pkgManagerInspect?.workspaceValue !== undefined;
358+
if (
359+
(hasExplicitPkgManager || !isPipManager) &&
360+
config.get('defaultPackageManager') !== e.packageManager
361+
) {
362+
promises.push(
363+
config.update('defaultPackageManager', e.packageManager, ConfigurationTarget.Workspace),
364+
);
365+
}
283366
}
284367
});
285-
promises.push(config.update('pythonProjects', overrides, ConfigurationTarget.Workspace));
368+
369+
// Only write pythonProjects if there was an explicit setting or we modified entries
370+
const shouldWriteProjects =
371+
existingProjectsSetting !== undefined || overrides.length > originalOverridesLength || projectsModified;
372+
if (shouldWriteProjects) {
373+
promises.push(config.update('pythonProjects', overrides, ConfigurationTarget.Workspace));
374+
}
286375
});
287376

288-
const config = workspace.getConfiguration('python-envs', undefined);
377+
const config = getConfiguration('python-envs', undefined);
289378
edits
290379
.filter((e) => !e.project)
291380
.forEach((e) => {
292-
if (config.get('defaultPackageManager') !== e.packageManager) {
381+
// Only write global settings if updating existing OR setting non-default package manager
382+
const isPipManager = e.packageManager === 'ms-python.python:pip';
383+
const pkgManagerInspect = config.inspect<string>('defaultPackageManager');
384+
const hasExplicitGlobalPkgManager = pkgManagerInspect?.globalValue !== undefined;
385+
if (
386+
(hasExplicitGlobalPkgManager || !isPipManager) &&
387+
config.get('defaultPackageManager') !== e.packageManager
388+
) {
293389
promises.push(config.update('defaultPackageManager', e.packageManager, ConfigurationTarget.Global));
294390
}
295391
});
@@ -307,7 +403,7 @@ export interface EditProjectSettings {
307403
export async function addPythonProjectSetting(edits: EditProjectSettings[]): Promise<void> {
308404
const noWorkspace: EditProjectSettings[] = [];
309405
const workspaces = new Map<WorkspaceFolder, EditProjectSettings[]>();
310-
const globalConfig = workspace.getConfiguration('python-envs', undefined);
406+
const globalConfig = getConfiguration('python-envs', undefined);
311407
const envManager = globalConfig.get<string>('defaultEnvManager', DEFAULT_ENV_MANAGER_ID);
312408
const pkgManager = globalConfig.get<string>('defaultPackageManager', DEFAULT_PACKAGE_MANAGER_ID);
313409

@@ -328,7 +424,7 @@ export async function addPythonProjectSetting(edits: EditProjectSettings[]): Pro
328424

329425
const promises: Thenable<void>[] = [];
330426
workspaces.forEach((es, w) => {
331-
const config = workspace.getConfiguration('python-envs', w.uri);
427+
const config = getConfiguration('python-envs', w.uri);
332428
const overrides = config.get<PythonProjectSettings[]>('pythonProjects', []);
333429
es.forEach((e) => {
334430
if (isMultiroot) {
@@ -376,7 +472,7 @@ export async function removePythonProjectSetting(edits: EditProjectSettings[]):
376472

377473
const promises: Thenable<void>[] = [];
378474
workspaces.forEach((es, w) => {
379-
const config = workspace.getConfiguration('python-envs', w.uri);
475+
const config = getConfiguration('python-envs', w.uri);
380476
const overrides = config.get<PythonProjectSettings[]>('pythonProjects', []);
381477
es.forEach((e) => {
382478
const pwPath = path.normalize(e.project.uri.fsPath);

0 commit comments

Comments
 (0)