From 41bc88c1b1f3a5dd5e911ff86b1c79d2f0af6e34 Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Wed, 3 Aug 2022 01:27:24 -0400 Subject: [PATCH] Reworks how modes apply to configuration - Adds override support to configuration - Splits configuration.get into get & getAll - Allows passing array to configuration.changed Avoids getting the entire configuration on startup --- src/commands/switchMode.ts | 8 +- src/configuration.ts | 74 ++++++++----- src/container.ts | 177 ++++++++++++++++++------------- src/extension.ts | 18 ++-- src/quickpicks/modePicker.ts | 10 +- src/webviews/settings/settingsWebview.ts | 2 +- src/webviews/webviewWithConfigBase.ts | 2 +- 7 files changed, 171 insertions(+), 120 deletions(-) diff --git a/src/commands/switchMode.ts b/src/commands/switchMode.ts index fb1d9bc..84065e2 100644 --- a/src/commands/switchMode.ts +++ b/src/commands/switchMode.ts @@ -25,15 +25,15 @@ export class SwitchModeCommand extends Command { cc.exitDetails = ` \u2014 mode=${pick.key ?? ''}`; } - const active = this.container.config.mode.active; + const active = configuration.get('mode.active'); if (active === pick.key) return; // Check if we have applied any annotations and clear them if we won't be applying them again if (active != null && active.length !== 0) { - const activeAnnotations = this.container.config.modes?.[active].annotations; + const modes = configuration.get('modes'); + const activeAnnotations = modes?.[active].annotations; if (activeAnnotations != null) { - const newAnnotations = - pick.key != null ? this.container.config.modes?.[pick.key].annotations : undefined; + const newAnnotations = pick.key != null ? modes?.[pick.key].annotations : undefined; if (activeAnnotations !== newAnnotations) { await this.container.fileAnnotations.clearAll(); } diff --git a/src/configuration.ts b/src/configuration.ts index c45f27b..926cacb 100644 --- a/src/configuration.ts +++ b/src/configuration.ts @@ -14,9 +14,10 @@ import { areEqual } from './system/object'; const configPrefix = 'gitlens'; -export interface ConfigurationWillChangeEvent { - change: ConfigurationChangeEvent; - transform?(e: ConfigurationChangeEvent): ConfigurationChangeEvent; +interface ConfigurationOverrides { + get(section: T, value: ConfigPathValue): ConfigPathValue; + getAll(config: Config): Config; + onChange(e: ConfigurationChangeEvent): ConfigurationChangeEvent; } export class Configuration { @@ -36,8 +37,8 @@ export class Configuration { return this._onDidChangeAny.event; } - private _onWillChange = new EventEmitter(); - get onWillChange(): Event { + private _onWillChange = new EventEmitter(); + get onWillChange(): Event { return this._onWillChange.event; } @@ -48,37 +49,46 @@ export class Configuration { return; } - const evt: ConfigurationWillChangeEvent = { - change: e, - }; - this._onWillChange.fire(evt); + this._onWillChange.fire(e); - if (evt.transform !== undefined) { - e = evt.transform(e); + if (this._overrides?.onChange != null) { + e = this._overrides.onChange(e); } this._onDidChangeAny.fire(e); this._onDidChange.fire(e); } - get(): Config; + private _overrides: Partial | undefined; + + applyOverrides(overrides: ConfigurationOverrides): void { + this._overrides = overrides; + } + + clearOverrides(): void { + if (this._overrides == null) return; + + // Don't clear the "onChange" override as we need to keep it until the stack unwinds (so the the event propagates with the override) + this._overrides.get = undefined; + this._overrides.getAll = undefined; + queueMicrotask(() => (this._overrides = undefined)); + } + get( section: T, scope?: ConfigurationScope | null, defaultValue?: ConfigPathValue, - ): ConfigPathValue; - get( - section?: T, - scope?: ConfigurationScope | null, - defaultValue?: ConfigPathValue, - ): Config | ConfigPathValue { - return defaultValue === undefined - ? workspace - .getConfiguration(section === undefined ? undefined : configPrefix, scope) - .get>(section === undefined ? configPrefix : section)! - : workspace - .getConfiguration(section === undefined ? undefined : configPrefix, scope) - .get>(section === undefined ? configPrefix : section, defaultValue)!; + ): ConfigPathValue { + const value = + defaultValue === undefined + ? workspace.getConfiguration(configPrefix, scope).get>(section)! + : workspace.getConfiguration(configPrefix, scope).get>(section, defaultValue)!; + return this._overrides?.get == null ? value : this._overrides.get(section, value); + } + + getAll(skipOverrides?: boolean): Config { + const config = workspace.getConfiguration().get(configPrefix)!; + return skipOverrides || this._overrides?.getAll == null ? config : this._overrides.getAll(config); } getAny(section: string, scope?: ConfigurationScope | null): T | undefined; @@ -91,15 +101,19 @@ export class Configuration { changed( e: ConfigurationChangeEvent | undefined, - section: T, + section: T | T[], scope?: ConfigurationScope | null | undefined, ): boolean { - return e?.affectsConfiguration(`${configPrefix}.${section}`, scope!) ?? true; + if (e == null) return true; + + return Array.isArray(section) + ? section.some(s => e.affectsConfiguration(`${configPrefix}.${s}`, scope!)) + : e.affectsConfiguration(`${configPrefix}.${section}`, scope!); } inspect>(section: T, scope?: ConfigurationScope | null) { return workspace - .getConfiguration(section === undefined ? undefined : configPrefix, scope) + .getConfiguration(configPrefix, scope) .inspect(section === undefined ? configPrefix : section); } @@ -243,6 +257,10 @@ export class Configuration { } } + matches(match: T, section: ConfigPath, value: unknown): value is ConfigPathValue { + return match === section; + } + name(section: T): string { return section; } diff --git a/src/container.ts b/src/container.ts index 2b6fd7f..2019134 100644 --- a/src/container.ts +++ b/src/container.ts @@ -1,11 +1,4 @@ -import { - ConfigurationChangeEvent, - ConfigurationScope, - Event, - EventEmitter, - ExtensionContext, - ExtensionMode, -} from 'vscode'; +import { ConfigurationChangeEvent, Event, EventEmitter, ExtensionContext, ExtensionMode } from 'vscode'; import { getSupportedGitProviders } from '@env/providers'; import { Autolinks } from './annotations/autolinks'; import { FileAnnotationController } from './annotations/fileAnnotationController'; @@ -18,10 +11,10 @@ import { AnnotationsToggleMode, Config, configuration, - ConfigurationWillChangeEvent, DateSource, DateStyle, FileAnnotationType, + ModeConfig, } from './configuration'; import { Commands } from './constants'; import { GitFileSystemProvider } from './git/fsProvider'; @@ -71,17 +64,17 @@ export class Container { if (Container.#instance != null) return (Container.#instance as any)[prop]; // Allow access to config before we are initialized - if (prop === 'config') return configuration.get(); + if (prop === 'config') return configuration.getAll(); // debugger; throw new Error('Container is not initialized'); }, }); - static create(context: ExtensionContext, cfg: Config, insiders: boolean) { + static create(context: ExtensionContext, insiders: boolean) { if (Container.#instance != null) throw new Error('Container is already initialized'); - Container.#instance = new Container(context, cfg, insiders); + Container.#instance = new Container(context, insiders); return Container.#instance; } @@ -145,17 +138,13 @@ export class Container { }, }; - private _configsAffectedByMode: string[] | undefined; - private _applyModeConfigurationTransformBound: - | ((e: ConfigurationChangeEvent) => ConfigurationChangeEvent) - | undefined; - + private _configAffectedByModeRegex: RegExp | undefined; private _terminalLinks: GitTerminalLinkProvider | undefined; - private constructor(context: ExtensionContext, config: Config, insiders: boolean) { + private constructor(context: ExtensionContext, insiders: boolean) { this._context = context; - this._config = this.applyMode(config); this._insiders = insiders; + this.ensureModeApplied(); context.subscriptions.push((this._storage = new Storage(this._context))); @@ -205,7 +194,7 @@ export class Container { context.subscriptions.push((this._homeView = new HomeWebviewView(this))); context.subscriptions.push((this._timelineView = new TimelineWebviewView(this))); - if (config.terminalLinks.enabled) { + if (configuration.get('terminalLinks.enabled')) { context.subscriptions.push((this._terminalLinks = new GitTerminalLinkProvider(this))); } @@ -214,7 +203,7 @@ export class Container { if (!configuration.changed(e, 'terminalLinks.enabled')) return; this._terminalLinks?.dispose(); - if (this.config.terminalLinks.enabled) { + if (configuration.get('terminalLinks.enabled')) { context.subscriptions.push((this._terminalLinks = new GitTerminalLinkProvider(this))); } }), @@ -241,22 +230,20 @@ export class Container { this._git.registrationComplete(); } - private onConfigurationChanging(e: ConfigurationWillChangeEvent) { + private onConfigurationChanging(e: ConfigurationChangeEvent) { this._config = undefined; + this._mode = undefined; - if (configuration.changed(e.change, 'outputLevel')) { + if (configuration.changed(e, 'outputLevel')) { Logger.logLevel = configuration.get('outputLevel'); } - if (configuration.changed(e.change, 'defaultGravatarsStyle')) { + if (configuration.changed(e, 'defaultGravatarsStyle')) { resetAvatarCache('fallback'); } - if (configuration.changed(e.change, 'mode') || configuration.changed(e.change, 'modes')) { - if (this._applyModeConfigurationTransformBound == null) { - this._applyModeConfigurationTransformBound = this.applyModeConfigurationTransform.bind(this); - } - e.transform = this._applyModeConfigurationTransformBound; + if (configuration.changed(e, 'mode')) { + this.ensureModeApplied(); } } @@ -304,7 +291,7 @@ export class Container { private _config: Config | undefined; get config() { if (this._config == null) { - this._config = this.applyMode(configuration.get()); + this._config = configuration.getAll(); } return this._config; } @@ -580,25 +567,32 @@ export class Container { return this._worktreesView; } - private applyMode(config: Config) { - if (!config.mode.active) return config; + private _mode: ModeConfig | undefined; + get mode() { + if (this._mode == null) { + this._mode = configuration.get('modes')?.[configuration.get('mode.active')]; + } + return this._mode; + } + + private ensureModeApplied() { + const mode = this.mode; + if (mode == null) { + configuration.clearOverrides(); - const mode = config.modes?.[config.mode.active]; - if (mode == null) return config; + return; + } if (mode.annotations != null) { let command: Commands | undefined; switch (mode.annotations) { case 'blame': - config.blame.toggleMode = AnnotationsToggleMode.Window; command = Commands.ToggleFileBlame; break; case 'changes': - config.changes.toggleMode = AnnotationsToggleMode.Window; command = Commands.ToggleFileChanges; break; case 'heatmap': - config.heatmap.toggleMode = AnnotationsToggleMode.Window; command = Commands.ToggleFileHeatmap; break; } @@ -608,50 +602,87 @@ export class Container { type: mode.annotations as FileAnnotationType, on: true, }; - // Make sure to delay the execution by a bit so that the configuration changes get propegated first + // Make sure to delay the execution by a bit so that the configuration changes get propagated first setTimeout(() => executeCommand(command!, commandArgs), 50); } } - if (mode.codeLens != null) { - config.codeLens.enabled = mode.codeLens; - } + // Apply any required configuration overrides + configuration.applyOverrides({ + get: (section, value) => { + if (mode.annotations != null) { + if (configuration.matches(`${mode.annotations}.toggleMode`, section, value)) { + value = AnnotationsToggleMode.Window as typeof value; + return value; + } - if (mode.currentLine != null) { - config.currentLine.enabled = mode.currentLine; - } + if (configuration.matches(mode.annotations, section, value)) { + value.toggleMode = AnnotationsToggleMode.Window; + return value; + } + } - if (mode.hovers != null) { - config.hovers.enabled = mode.hovers; - } + for (const key of ['codeLens', 'currentLine', 'hovers', 'statusBar'] as const) { + if (mode[key] != null) { + if (configuration.matches(`${key}.enabled`, section, value)) { + value = mode[key] as NonNullable; + return value; + } else if (configuration.matches(key, section, value)) { + value.enabled = mode[key]!; + return value; + } + } + } - if (mode.statusBar != null) { - config.statusBar.enabled = mode.statusBar; - } + return value; + }, + getAll: cfg => { + if (mode.annotations != null) { + cfg[mode.annotations].toggleMode = AnnotationsToggleMode.Window; + } - return config; - } - - private applyModeConfigurationTransform(e: ConfigurationChangeEvent): ConfigurationChangeEvent { - if (this._configsAffectedByMode == null) { - this._configsAffectedByMode = [ - `gitlens.${configuration.name('mode')}`, - `gitlens.${configuration.name('modes')}`, - `gitlens.${configuration.name('blame.toggleMode')}`, - `gitlens.${configuration.name('changes.toggleMode')}`, - `gitlens.${configuration.name('codeLens')}`, - `gitlens.${configuration.name('currentLine')}`, - `gitlens.${configuration.name('heatmap.toggleMode')}`, - `gitlens.${configuration.name('hovers')}`, - `gitlens.${configuration.name('statusBar')}`, - ]; - } + if (mode.codeLens != null) { + cfg.codeLens.enabled = mode.codeLens; + } + + if (mode.currentLine != null) { + cfg.currentLine.enabled = mode.currentLine; + } - const original = e.affectsConfiguration; - return { - ...e, - affectsConfiguration: (section: string, scope?: ConfigurationScope) => - this._configsAffectedByMode?.some(n => section.startsWith(n)) ? true : original(section, scope), - }; + if (mode.hovers != null) { + cfg.hovers.enabled = mode.hovers; + } + + if (mode.statusBar != null) { + cfg.statusBar.enabled = mode.statusBar; + } + + return cfg; + }, + onChange: e => { + // When the mode or modes change, we will simulate that all the affected configuration also changed + if (configuration.changed(e, ['mode', 'modes'])) { + if (this._configAffectedByModeRegex == null) { + this._configAffectedByModeRegex = new RegExp( + `^gitlens\\.(?:${configuration.name('mode')}|${configuration.name( + 'modes', + )}|${configuration.name('blame')}|${configuration.name('changes')}|${configuration.name( + 'heatmap', + )}|${configuration.name('codeLens')}|${configuration.name( + 'currentLine', + )}|${configuration.name('hovers')}|${configuration.name('statusBar')})\\b`, + ); + } + + const original = e.affectsConfiguration; + e = { + ...e, + affectsConfiguration: (section, scope) => + this._configAffectedByModeRegex!.test(section) ? true : original(section, scope), + }; + } + return e; + }, + }); } } diff --git a/src/extension.ts b/src/extension.ts index 6a864a3..aa1c6d9 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -24,6 +24,7 @@ export async function activate(context: ExtensionContext): Promise 2020.0.0'); + const outputLevel = configuration.get('outputLevel'); Logger.configure(context, configuration.get('outputLevel'), o => { if (GitUri.is(o)) { return `GitUri(${o.toString(true)}${o.repoPath ? ` repoPath=${o.repoPath}` : ''}${ @@ -95,19 +96,19 @@ export async function activate(context: ExtensionContext): Promise { - if (!e.affectsConfiguration('gitlens.defaultDateLocale')) return; - setDefaultDateLocales(configuration.get('defaultDateLocale', undefined, env.language)); + if (configuration.changed(e, 'defaultDateLocale')) { + setDefaultDateLocales(configuration.get('defaultDateLocale', undefined, env.language)); + } }), ); // await migrateSettings(context, previousVersion); - const container = Container.create(context, cfg, insiders); + const container = Container.create(context, insiders); once(container.onReady)(() => { context.subscriptions.push(...registerCommands(container)); registerBuiltInActionRunners(container); @@ -122,9 +123,9 @@ export async function activate(context: ExtensionContext): Promise { - if (cfg.outputLevel !== OutputLevel.Debug) return; + if (configuration.get('outputLevel') !== OutputLevel.Debug) return; if (!container.insiders) { if (await Messages.showDebugLoggingWarningMessage()) { @@ -143,9 +144,10 @@ export async function activate(context: ExtensionContext): Promise { - if (Container.instance.config.modes == null) return undefined; + const modes = configuration.get('modes'); + if (modes == null) return undefined; - const modes = Container.instance.config.modes; const modeKeys = Object.keys(modes); if (modeKeys.length === 0) return undefined; - const mode = Container.instance.config.mode.active; + const mode = configuration.get('mode.active'); const items = modeKeys.map(key => { const modeCfg = modes[key]; @@ -28,7 +28,7 @@ export namespace ModePicker { return item; }); - if (mode) { + if (mode && modes[mode] != null) { items.splice(0, 0, { label: `Exit ${modes[mode].name} mode`, key: undefined, diff --git a/src/webviews/settings/settingsWebview.ts b/src/webviews/settings/settingsWebview.ts index 48d0c32..070edbb 100644 --- a/src/webviews/settings/settingsWebview.ts +++ b/src/webviews/settings/settingsWebview.ts @@ -54,7 +54,7 @@ export class SettingsWebview extends WebviewWithConfigBase { return { // Make sure to get the raw config, not from the container which has the modes mixed in - config: configuration.get(), + config: configuration.getAll(true), customSettings: this.getCustomSettings(), scope: 'user', scopes: scopes, diff --git a/src/webviews/webviewWithConfigBase.ts b/src/webviews/webviewWithConfigBase.ts index 7d1ed08..a7a2430 100644 --- a/src/webviews/webviewWithConfigBase.ts +++ b/src/webviews/webviewWithConfigBase.ts @@ -241,7 +241,7 @@ export abstract class WebviewWithConfigBase extends WebviewBase { private notifyDidChangeConfiguration() { // Make sure to get the raw config, not from the container which has the modes mixed in return this.notify(DidChangeConfigurationNotificationType, { - config: configuration.get(), + config: configuration.getAll(true), customSettings: this.getCustomSettings(), }); }