From f6608f63f21a671525b6f82641e585529f4483ab Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Mon, 5 Jun 2023 10:21:48 +0200 Subject: [PATCH 1/3] editors - respect user configured `editorLargeFileConfirmation` --- .../common/services/textResourceConfiguration.ts | 12 +++++++++++- .../services/textResourceConfigurationService.ts | 5 +++++ .../standalone/browser/standaloneServices.ts | 5 +++++ .../platform/configuration/common/configuration.ts | 9 +++++++++ .../test/common/configurationService.test.ts | 5 ++++- .../files/browser/editors/fileEditorInput.ts | 14 ++++++++++---- .../test/browser/workbenchTestServices.ts | 6 +++++- 7 files changed, 49 insertions(+), 7 deletions(-) diff --git a/src/vs/editor/common/services/textResourceConfiguration.ts b/src/vs/editor/common/services/textResourceConfiguration.ts index 0f4522529707b..0921cba713d10 100644 --- a/src/vs/editor/common/services/textResourceConfiguration.ts +++ b/src/vs/editor/common/services/textResourceConfiguration.ts @@ -6,7 +6,7 @@ import { Event } from 'vs/base/common/event'; import { URI } from 'vs/base/common/uri'; import { IPosition } from 'vs/editor/common/core/position'; -import { ConfigurationTarget } from 'vs/platform/configuration/common/configuration'; +import { ConfigurationTarget, IConfigurationValue } from 'vs/platform/configuration/common/configuration'; import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; export const ITextResourceConfigurationService = createDecorator('textResourceConfigurationService'); @@ -50,6 +50,16 @@ export interface ITextResourceConfigurationService { getValue(resource: URI | undefined, section?: string): T; getValue(resource: URI | undefined, position?: IPosition, section?: string): T; + /** + * Inspects the values of the section for the given resource by applying language overrides. + * + * @param resource - Resource for which the configuration has to be fetched. + * @param position - Position in the resource for which configuration has to be fetched. + * @param section - Section of the configuration. + * + */ + inspect(resource: URI | undefined, position: IPosition | null, section: string): IConfigurationValue>; + /** * Update the configuration value for the given resource at the effective location. * diff --git a/src/vs/editor/common/services/textResourceConfigurationService.ts b/src/vs/editor/common/services/textResourceConfigurationService.ts index 4069117941bc1..89acdd09e8b66 100644 --- a/src/vs/editor/common/services/textResourceConfigurationService.ts +++ b/src/vs/editor/common/services/textResourceConfigurationService.ts @@ -106,6 +106,11 @@ export class TextResourceConfigurationService extends Disposable implements ITex return this.configurationService.getValue(section, { resource, overrideIdentifier: language }); } + inspect(resource: URI | undefined, position: IPosition | null, section: string): IConfigurationValue> { + const language = resource ? this.getLanguage(resource, position) : undefined; + return this.configurationService.inspect(section, { resource, overrideIdentifier: language }); + } + private getLanguage(resource: URI, position: IPosition | null): string | null { const model = this.modelService.getModel(resource); if (model) { diff --git a/src/vs/editor/standalone/browser/standaloneServices.ts b/src/vs/editor/standalone/browser/standaloneServices.ts index d5d02b4de2e81..63ade6f21438e 100644 --- a/src/vs/editor/standalone/browser/standaloneServices.ts +++ b/src/vs/editor/standalone/browser/standaloneServices.ts @@ -717,6 +717,11 @@ class StandaloneResourceConfigurationService implements ITextResourceConfigurati }); } + inspect(resource: URI | undefined, position: IPosition | null, section: string): IConfigurationValue> { + const language = resource ? this.getLanguage(resource, position) : undefined; + return this.configurationService.inspect(section, { resource, overrideIdentifier: language }); + } + private getLanguage(resource: URI, position: IPosition | null): string | null { const model = this.modelService.getModel(resource); if (model) { diff --git a/src/vs/platform/configuration/common/configuration.ts b/src/vs/platform/configuration/common/configuration.ts index d922c48fd6075..38c4631b18530 100644 --- a/src/vs/platform/configuration/common/configuration.ts +++ b/src/vs/platform/configuration/common/configuration.ts @@ -99,6 +99,15 @@ export interface IConfigurationValue { readonly overrideIdentifiers?: string[]; } +export function isConfiguredByUser(configValue: IConfigurationValue): configValue is IConfigurationValue & { value: T } { + return configValue.applicationValue !== undefined || + configValue.userValue !== undefined || + configValue.userLocalValue !== undefined || + configValue.userRemoteValue !== undefined || + configValue.workspaceValue !== undefined || + configValue.workspaceFolderValue !== undefined; +} + export interface IConfigurationUpdateOptions { /** * If `true`, do not notifies the error to user by showing the message box. Default is `false`. diff --git a/src/vs/platform/configuration/test/common/configurationService.test.ts b/src/vs/platform/configuration/test/common/configurationService.test.ts index 8e9162e6e8979..74039ed651fbf 100644 --- a/src/vs/platform/configuration/test/common/configurationService.test.ts +++ b/src/vs/platform/configuration/test/common/configurationService.test.ts @@ -10,7 +10,7 @@ import { DisposableStore } from 'vs/base/common/lifecycle'; import { Schemas } from 'vs/base/common/network'; import { URI } from 'vs/base/common/uri'; import { runWithFakedTimers } from 'vs/base/test/common/timeTravelScheduler'; -import { ConfigurationTarget } from 'vs/platform/configuration/common/configuration'; +import { ConfigurationTarget, isConfiguredByUser } from 'vs/platform/configuration/common/configuration'; import { Extensions as ConfigurationExtensions, IConfigurationRegistry } from 'vs/platform/configuration/common/configurationRegistry'; import { ConfigurationService } from 'vs/platform/configuration/common/configurationService'; import { IFileService } from 'vs/platform/files/common/files'; @@ -200,11 +200,13 @@ suite('ConfigurationService', () => { assert.strictEqual(res.value, undefined); assert.strictEqual(res.defaultValue, undefined); assert.strictEqual(res.userValue, undefined); + assert.strictEqual(isConfiguredByUser(res), false); res = testObject.inspect('lookup.service.testSetting'); assert.strictEqual(res.defaultValue, 'isSet'); assert.strictEqual(res.value, 'isSet'); assert.strictEqual(res.userValue, undefined); + assert.strictEqual(isConfiguredByUser(res), false); await fileService.writeFile(settingsResource, VSBuffer.fromString('{ "lookup.service.testSetting": "bar" }')); @@ -213,6 +215,7 @@ suite('ConfigurationService', () => { assert.strictEqual(res.defaultValue, 'isSet'); assert.strictEqual(res.userValue, 'bar'); assert.strictEqual(res.value, 'bar'); + assert.strictEqual(isConfiguredByUser(res), true); })); diff --git a/src/vs/workbench/contrib/files/browser/editors/fileEditorInput.ts b/src/vs/workbench/contrib/files/browser/editors/fileEditorInput.ts index 671b31d850e29..dcd2479ca4adf 100644 --- a/src/vs/workbench/contrib/files/browser/editors/fileEditorInput.ts +++ b/src/vs/workbench/contrib/files/browser/editors/fileEditorInput.ts @@ -24,6 +24,7 @@ import { Schemas } from 'vs/base/common/network'; import { createTextBufferFactory } from 'vs/editor/common/model/textModel'; import { IPathService } from 'vs/workbench/services/path/common/pathService'; import { ITextResourceConfigurationService } from 'vs/editor/common/services/textResourceConfiguration'; +import { isConfiguredByUser } from 'vs/platform/configuration/common/configuration'; const enum ForceOpenAs { None, @@ -372,16 +373,21 @@ export class FileEditorInput extends AbstractTextResourceEditorInput implements return options.limits; // respect passed in limits if any } + // We want to determine the large file configuration based on the best defaults + // for the resource but also respecting user settings. We only apply user settings + // if explicitly configured by the user. Otherwise we pick the best limit for the + // resource scheme. + const defaultSizeLimit = getLargeFileConfirmationLimit(this.resource); let configuredSizeLimit: number | undefined = undefined; - const configuredSizeLimitMb = this.textResourceConfigurationService.getValue(this.resource, 'workbench.editorLargeFileConfirmation'); - if (typeof configuredSizeLimitMb === 'number') { - configuredSizeLimit = configuredSizeLimitMb * ByteSize.MB; // normalize to MB + const configuredSizeLimitMb = this.textResourceConfigurationService.inspect(this.resource, null, 'workbench.editorLargeFileConfirmation'); + if (isConfiguredByUser(configuredSizeLimitMb)) { + configuredSizeLimit = configuredSizeLimitMb.value * ByteSize.MB; // normalize to MB } return { - size: Math.max(defaultSizeLimit, configuredSizeLimit ?? defaultSizeLimit) // pick the highest limit + size: configuredSizeLimit ?? defaultSizeLimit }; } diff --git a/src/vs/workbench/test/browser/workbenchTestServices.ts b/src/vs/workbench/test/browser/workbenchTestServices.ts index 55fb0e1f6bd53..68191eae2f6bc 100644 --- a/src/vs/workbench/test/browser/workbenchTestServices.ts +++ b/src/vs/workbench/test/browser/workbenchTestServices.ts @@ -14,7 +14,7 @@ import { EditorInputWithOptions, IEditorIdentifier, IUntitledTextResourceEditorI import { EditorServiceImpl, IEditorGroupView, IEditorGroupsAccessor, IEditorGroupTitleHeight } from 'vs/workbench/browser/parts/editor/editor'; import { Event, Emitter } from 'vs/base/common/event'; import { IResolvedWorkingCopyBackup, IWorkingCopyBackupService } from 'vs/workbench/services/workingCopy/common/workingCopyBackup'; -import { IConfigurationService, ConfigurationTarget } from 'vs/platform/configuration/common/configuration'; +import { IConfigurationService, ConfigurationTarget, IConfigurationValue } from 'vs/platform/configuration/common/configuration'; import { IWorkbenchLayoutService, PanelAlignment, Parts, Position as PartPosition } from 'vs/workbench/services/layout/browser/layoutService'; import { TextModelResolverService } from 'vs/workbench/services/textmodelResolver/common/textModelResolverService'; import { ITextModelService } from 'vs/editor/common/services/resolverService'; @@ -1335,6 +1335,10 @@ export class TestTextResourceConfigurationService implements ITextResourceConfig return this.configurationService.getValue(section, { resource }); } + inspect(resource: URI | undefined, position: IPosition | null, section: string): IConfigurationValue> { + return this.configurationService.inspect(section, { resource }); + } + updateValue(resource: URI, key: string, value: any, configurationTarget?: ConfigurationTarget): Promise { return this.configurationService.updateValue(key, value); } From a2888b933c6b169b84d70fd3d8f2ffd83fd7f025 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Mon, 5 Jun 2023 12:36:10 +0200 Subject: [PATCH 2/3] Update src/vs/platform/configuration/common/configuration.ts Co-authored-by: Sandeep Somavarapu --- src/vs/platform/configuration/common/configuration.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/platform/configuration/common/configuration.ts b/src/vs/platform/configuration/common/configuration.ts index 38c4631b18530..eb419a6929a3d 100644 --- a/src/vs/platform/configuration/common/configuration.ts +++ b/src/vs/platform/configuration/common/configuration.ts @@ -99,7 +99,7 @@ export interface IConfigurationValue { readonly overrideIdentifiers?: string[]; } -export function isConfiguredByUser(configValue: IConfigurationValue): configValue is IConfigurationValue & { value: T } { +export function isConfigured(configValue: IConfigurationValue): configValue is IConfigurationValue & { value: T } { return configValue.applicationValue !== undefined || configValue.userValue !== undefined || configValue.userLocalValue !== undefined || From 87578141dcfb34adf5e29edc3db4cc9aee5efb7d Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Mon, 5 Jun 2023 12:36:59 +0200 Subject: [PATCH 3/3] . --- .../test/common/configurationService.test.ts | 8 ++++---- .../contrib/files/browser/editors/fileEditorInput.ts | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/vs/platform/configuration/test/common/configurationService.test.ts b/src/vs/platform/configuration/test/common/configurationService.test.ts index 74039ed651fbf..5337e51d66781 100644 --- a/src/vs/platform/configuration/test/common/configurationService.test.ts +++ b/src/vs/platform/configuration/test/common/configurationService.test.ts @@ -10,7 +10,7 @@ import { DisposableStore } from 'vs/base/common/lifecycle'; import { Schemas } from 'vs/base/common/network'; import { URI } from 'vs/base/common/uri'; import { runWithFakedTimers } from 'vs/base/test/common/timeTravelScheduler'; -import { ConfigurationTarget, isConfiguredByUser } from 'vs/platform/configuration/common/configuration'; +import { ConfigurationTarget, isConfigured } from 'vs/platform/configuration/common/configuration'; import { Extensions as ConfigurationExtensions, IConfigurationRegistry } from 'vs/platform/configuration/common/configurationRegistry'; import { ConfigurationService } from 'vs/platform/configuration/common/configurationService'; import { IFileService } from 'vs/platform/files/common/files'; @@ -200,13 +200,13 @@ suite('ConfigurationService', () => { assert.strictEqual(res.value, undefined); assert.strictEqual(res.defaultValue, undefined); assert.strictEqual(res.userValue, undefined); - assert.strictEqual(isConfiguredByUser(res), false); + assert.strictEqual(isConfigured(res), false); res = testObject.inspect('lookup.service.testSetting'); assert.strictEqual(res.defaultValue, 'isSet'); assert.strictEqual(res.value, 'isSet'); assert.strictEqual(res.userValue, undefined); - assert.strictEqual(isConfiguredByUser(res), false); + assert.strictEqual(isConfigured(res), false); await fileService.writeFile(settingsResource, VSBuffer.fromString('{ "lookup.service.testSetting": "bar" }')); @@ -215,7 +215,7 @@ suite('ConfigurationService', () => { assert.strictEqual(res.defaultValue, 'isSet'); assert.strictEqual(res.userValue, 'bar'); assert.strictEqual(res.value, 'bar'); - assert.strictEqual(isConfiguredByUser(res), true); + assert.strictEqual(isConfigured(res), true); })); diff --git a/src/vs/workbench/contrib/files/browser/editors/fileEditorInput.ts b/src/vs/workbench/contrib/files/browser/editors/fileEditorInput.ts index dcd2479ca4adf..016ca6bcb031c 100644 --- a/src/vs/workbench/contrib/files/browser/editors/fileEditorInput.ts +++ b/src/vs/workbench/contrib/files/browser/editors/fileEditorInput.ts @@ -24,7 +24,7 @@ import { Schemas } from 'vs/base/common/network'; import { createTextBufferFactory } from 'vs/editor/common/model/textModel'; import { IPathService } from 'vs/workbench/services/path/common/pathService'; import { ITextResourceConfigurationService } from 'vs/editor/common/services/textResourceConfiguration'; -import { isConfiguredByUser } from 'vs/platform/configuration/common/configuration'; +import { isConfigured } from 'vs/platform/configuration/common/configuration'; const enum ForceOpenAs { None, @@ -382,7 +382,7 @@ export class FileEditorInput extends AbstractTextResourceEditorInput implements let configuredSizeLimit: number | undefined = undefined; const configuredSizeLimitMb = this.textResourceConfigurationService.inspect(this.resource, null, 'workbench.editorLargeFileConfirmation'); - if (isConfiguredByUser(configuredSizeLimitMb)) { + if (isConfigured(configuredSizeLimitMb)) { configuredSizeLimit = configuredSizeLimitMb.value * ByteSize.MB; // normalize to MB }