From a42b7a6be0127be704de1793c6907a057cbcb71f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Mar=C3=A9chal?= Date: Thu, 11 Feb 2021 18:08:10 -0500 Subject: [PATCH] fix PreferenceChangeEvent type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Despite precisely knowing for a given `preferenceName` what its associated type for `newValue` and `oldValue` should be, the current typing wasn't allowing TypeScript to properly infer the type. With this commit, we can do things like: declare const event: PreferenceChangeEvent<{ a: string b: number }> if (event.preferenceName === 'a') { event.newValue // type is `string` } The type is more complex, but it now represents how it is meant to be used rather than getting tsserver lost like before. This commit also fixes places where the typing was broken and makes use of the new information. Signed-off-by: Paul Maréchal --- .../sample-file-watching-contribution.ts | 2 +- .../browser/preferences/preference-proxy.ts | 59 +++++++++++++++++-- .../git/src/browser/diff/git-diff-widget.tsx | 8 +-- .../history/git-commit-detail-widget.tsx | 8 +-- .../src/browser/problem/problem-decorator.ts | 10 ++-- .../textmate/monaco-textmate-service.ts | 2 +- packages/output/src/common/output-channel.ts | 7 +-- .../preview/src/browser/preview-widget.ts | 2 +- packages/scm/src/browser/scm-widget.tsx | 8 +-- .../src/browser/terminal-preferences.ts | 7 ++- .../src/browser/terminal-widget-impl.ts | 18 +++--- .../src/browser/workspace-service.ts | 4 +- 12 files changed, 90 insertions(+), 45 deletions(-) diff --git a/examples/api-samples/src/browser/file-watching/sample-file-watching-contribution.ts b/examples/api-samples/src/browser/file-watching/sample-file-watching-contribution.ts index ac8ad1649bfe9..3a12e7f6b4a97 100644 --- a/examples/api-samples/src/browser/file-watching/sample-file-watching-contribution.ts +++ b/examples/api-samples/src/browser/file-watching/sample-file-watching-contribution.ts @@ -69,7 +69,7 @@ class SampleFileWatchingContribution implements FrontendApplicationContribution this.verbose = this.fileWatchingPreferences['sample.file-watching.verbose']; this.fileWatchingPreferences.onPreferenceChanged(e => { if (e.preferenceName === 'sample.file-watching.verbose') { - this.verbose = e.newValue!; + this.verbose = e.newValue; } }); } diff --git a/packages/core/src/browser/preferences/preference-proxy.ts b/packages/core/src/browser/preferences/preference-proxy.ts index 3fc56f6ba9568..f3f63b8f9bf8b 100644 --- a/packages/core/src/browser/preferences/preference-proxy.ts +++ b/packages/core/src/browser/preferences/preference-proxy.ts @@ -21,12 +21,61 @@ import { PreferenceService } from './preference-service'; import { PreferenceSchema, OverridePreferenceName } from './preference-contribution'; import { PreferenceScope } from './preference-scope'; -export interface PreferenceChangeEvent { - readonly preferenceName: keyof T; - readonly newValue?: T[keyof T]; - readonly oldValue?: T[keyof T]; +/** + * It is worth explaining the type for `PreferenceChangeEvent`: + * + * // Given T: + * type T = { a: string, b: number } + * + * // We construct a new type such as: + * type U = { + * a: { + * preferenceName: 'a' + * newValue: string + * oldValue?: string + * } + * b: { + * preferenceName: 'b' + * newValue: number + * oldValue?: number + * } + * } + * + * // Then we get the union of all values of U by selecting by `keyof T`: + * type V = U[keyof T] + * + * // Implementation: + * type PreferenceChangeEvent = { + * // Create a mapping where each key is a key from T, + * // -? normalizes optional typings to avoid getting + * // `undefined` as part of the final union: + * [K in keyof T]-?: { + * // In this object, K will take the value of each + * // independent key from T: + * preferenceName: K + * newValue: T[K] + * oldValue?: T[K] + * // Finally we create the union by doing so: + * }[keyof T] + * } + */ + +/** + * Union of all possible key/value pairs for a type `T` + */ +export type PreferenceChangeEvent = { affects(resourceUri?: string, overrideIdentifier?: string): boolean; -} +} & { + [K in keyof T]-?: { + readonly preferenceName: K; + readonly newValue: T[K]; + /** + * Undefined if the preference is set for the first time. + */ + // TODO: Use the default value instead of undefined? + readonly oldValue?: T[K]; + } +}[keyof T]; export interface PreferenceEventEmitter { readonly onPreferenceChanged: Event>; diff --git a/packages/git/src/browser/diff/git-diff-widget.tsx b/packages/git/src/browser/diff/git-diff-widget.tsx index f22d82075d5fe..bf667c0dcbcbe 100644 --- a/packages/git/src/browser/diff/git-diff-widget.tsx +++ b/packages/git/src/browser/diff/git-diff-widget.tsx @@ -16,7 +16,7 @@ import { inject, injectable, postConstruct } from 'inversify'; import { - BaseWidget, Widget, StatefulWidget, Panel, PanelLayout, Message, MessageLoop, PreferenceChangeEvent + BaseWidget, Widget, StatefulWidget, Panel, PanelLayout, Message, MessageLoop } from '@theia/core/lib/browser'; import { EditorManager, DiffNavigatorProvider } from '@theia/editor/lib/browser'; import { GitDiffTreeModel } from './git-diff-tree-model'; @@ -25,7 +25,7 @@ import { GitDiffHeaderWidget } from './git-diff-header-widget'; import { ScmService } from '@theia/scm/lib/browser/scm-service'; import { GitRepositoryProvider } from '../git-repository-provider'; import { ScmTreeWidget } from '@theia/scm/lib/browser/scm-tree-widget'; -import { ScmPreferences, ScmConfiguration } from '@theia/scm/lib/browser/scm-preferences'; +import { ScmPreferences } from '@theia/scm/lib/browser/scm-preferences'; /* eslint-disable @typescript-eslint/no-explicit-any */ @@ -76,9 +76,9 @@ export class GitDiffWidget extends BaseWidget implements StatefulWidget { this.containerLayout.addWidget(this.resourceWidget); this.updateViewMode(this.scmPreferences.get('scm.defaultViewMode')); - this.toDispose.push(this.scmPreferences.onPreferenceChanged((e: PreferenceChangeEvent) => { + this.toDispose.push(this.scmPreferences.onPreferenceChanged(e => { if (e.preferenceName === 'scm.defaultViewMode') { - this.updateViewMode(e.newValue!); + this.updateViewMode(e.newValue); } })); } diff --git a/packages/git/src/browser/history/git-commit-detail-widget.tsx b/packages/git/src/browser/history/git-commit-detail-widget.tsx index afba377fbad55..32e9e4bcd57fa 100644 --- a/packages/git/src/browser/history/git-commit-detail-widget.tsx +++ b/packages/git/src/browser/history/git-commit-detail-widget.tsx @@ -19,14 +19,14 @@ import { Message } from '@phosphor/messaging'; import { injectable, inject, postConstruct } from 'inversify'; import { - BaseWidget, Widget, StatefulWidget, Panel, PanelLayout, MessageLoop, PreferenceChangeEvent + BaseWidget, Widget, StatefulWidget, Panel, PanelLayout, MessageLoop } from '@theia/core/lib/browser'; import { GitCommitDetailWidgetOptions } from './git-commit-detail-widget-options'; import { GitCommitDetailHeaderWidget } from './git-commit-detail-header-widget'; import { ScmService } from '@theia/scm/lib/browser/scm-service'; import { GitDiffTreeModel } from '../diff/git-diff-tree-model'; import { ScmTreeWidget } from '@theia/scm/lib/browser/scm-tree-widget'; -import { ScmPreferences, ScmConfiguration } from '@theia/scm/lib/browser/scm-preferences'; +import { ScmPreferences } from '@theia/scm/lib/browser/scm-preferences'; @injectable() export class GitCommitDetailWidget extends BaseWidget implements StatefulWidget { @@ -76,9 +76,9 @@ export class GitCommitDetailWidget extends BaseWidget implements StatefulWidget this.containerLayout.addWidget(this.resourceWidget); this.updateViewMode(this.scmPreferences.get('scm.defaultViewMode')); - this.toDispose.push(this.scmPreferences.onPreferenceChanged((e: PreferenceChangeEvent) => { + this.toDispose.push(this.scmPreferences.onPreferenceChanged(e => { if (e.preferenceName === 'scm.defaultViewMode') { - this.updateViewMode(e.newValue!); + this.updateViewMode(e.newValue); } })); diff --git a/packages/markers/src/browser/problem/problem-decorator.ts b/packages/markers/src/browser/problem/problem-decorator.ts index d75bd15551881..697278854d95a 100644 --- a/packages/markers/src/browser/problem/problem-decorator.ts +++ b/packages/markers/src/browser/problem/problem-decorator.ts @@ -25,8 +25,7 @@ import { TreeDecorator, TreeDecoration } from '@theia/core/lib/browser/tree/tree import { FileStatNode } from '@theia/filesystem/lib/browser'; import { Marker } from '../../common/marker'; import { ProblemManager } from './problem-manager'; -import { ProblemPreferences, ProblemConfiguration } from './problem-preferences'; -import { PreferenceChangeEvent } from '@theia/core/lib/browser'; +import { ProblemPreferences } from './problem-preferences'; import { ProblemUtils } from './problem-utils'; @injectable() @@ -46,10 +45,9 @@ export class ProblemDecorator implements TreeDecorator { @postConstruct() protected init(): void { - this.problemPreferences.onPreferenceChanged((event: PreferenceChangeEvent) => { - const { preferenceName } = event; - if (preferenceName === 'problems.decorations.enabled') { - this.fireDidChangeDecorations((tree: Tree) => this.collectDecorators(tree)); + this.problemPreferences.onPreferenceChanged(event => { + if (event.preferenceName === 'problems.decorations.enabled') { + this.fireDidChangeDecorations(tree => this.collectDecorators(tree)); } }); } diff --git a/packages/monaco/src/browser/textmate/monaco-textmate-service.ts b/packages/monaco/src/browser/textmate/monaco-textmate-service.ts index a21c61965f08b..75fd443dc5781 100644 --- a/packages/monaco/src/browser/textmate/monaco-textmate-service.ts +++ b/packages/monaco/src/browser/textmate/monaco-textmate-service.ts @@ -103,7 +103,7 @@ export class MonacoTextmateService implements FrontendApplicationContribution { this.tokenizerOption.lineLimit = this.preferences['editor.maxTokenizationLineLength']; this.preferences.onPreferenceChanged(e => { if (e.preferenceName === 'editor.maxTokenizationLineLength') { - this.tokenizerOption.lineLimit = this.preferences['editor.maxTokenizationLineLength']; + this.tokenizerOption.lineLimit = e.newValue; } }); diff --git a/packages/output/src/common/output-channel.ts b/packages/output/src/common/output-channel.ts index 163083a312acf..7f7ee421cfd2e 100644 --- a/packages/output/src/common/output-channel.ts +++ b/packages/output/src/common/output-channel.ts @@ -25,7 +25,6 @@ import { MonacoTextModelService, IReference } from '@theia/monaco/lib/browser/mo import { OutputUri } from './output-uri'; import { OutputResource } from '../browser/output-resource'; import { OutputPreferences } from './output-preferences'; -import { OutputConfigSchema } from './output-preferences'; @injectable() export class OutputChannelManager implements Disposable, ResourceResolver { @@ -221,9 +220,9 @@ export class OutputChannel implements Disposable { this._maxLineNumber = this.preferences['output.maxChannelHistory']; this.toDispose.push(resource); this.toDispose.push(Disposable.create(() => this.decorationIds.clear())); - this.toDispose.push(this.preferences.onPreferenceChanged(({ preferenceName, newValue }) => { - if (preferenceName === 'output.maxChannelHistory') { - const maxLineNumber = newValue ? newValue : OutputConfigSchema.properties['output.maxChannelHistory'].default; + this.toDispose.push(this.preferences.onPreferenceChanged(event => { + if (event.preferenceName === 'output.maxChannelHistory') { + const maxLineNumber = event.newValue; if (this.maxLineNumber !== maxLineNumber) { this.maxLineNumber = maxLineNumber; } diff --git a/packages/preview/src/browser/preview-widget.ts b/packages/preview/src/browser/preview-widget.ts index 32e413f514183..ff5b69c707621 100644 --- a/packages/preview/src/browser/preview-widget.ts +++ b/packages/preview/src/browser/preview-widget.ts @@ -83,7 +83,7 @@ export class PreviewWidget extends BaseWidget implements Navigatable { this.scrollBeyondLastLine = !!this.editorPreferences['editor.scrollBeyondLastLine']; this.toDispose.push(this.editorPreferences.onPreferenceChanged(e => { if (e.preferenceName === 'editor.scrollBeyondLastLine') { - this.scrollBeyondLastLine = !!e.newValue; + this.scrollBeyondLastLine = e.newValue; this.forceUpdate(); } })); diff --git a/packages/scm/src/browser/scm-widget.tsx b/packages/scm/src/browser/scm-widget.tsx index 06b1aa9cf0d14..d87f2868827d1 100644 --- a/packages/scm/src/browser/scm-widget.tsx +++ b/packages/scm/src/browser/scm-widget.tsx @@ -20,14 +20,14 @@ import { Message } from '@phosphor/messaging'; import { injectable, inject, postConstruct } from 'inversify'; import { DisposableCollection } from '@theia/core/lib/common/disposable'; import { - BaseWidget, Widget, StatefulWidget, Panel, PanelLayout, MessageLoop, PreferenceChangeEvent, CompositeTreeNode, SelectableTreeNode, + BaseWidget, Widget, StatefulWidget, Panel, PanelLayout, MessageLoop, CompositeTreeNode, SelectableTreeNode, } from '@theia/core/lib/browser'; import { ScmCommitWidget } from './scm-commit-widget'; import { ScmAmendWidget } from './scm-amend-widget'; import { ScmNoRepositoryWidget } from './scm-no-repository-widget'; import { ScmService } from './scm-service'; import { ScmTreeWidget } from './scm-tree-widget'; -import { ScmPreferences, ScmConfiguration } from './scm-preferences'; +import { ScmPreferences } from './scm-preferences'; @injectable() export class ScmWidget extends BaseWidget implements StatefulWidget { @@ -78,9 +78,9 @@ export class ScmWidget extends BaseWidget implements StatefulWidget { this.refresh(); this.toDispose.push(this.scmService.onDidChangeSelectedRepository(() => this.refresh())); this.updateViewMode(this.scmPreferences.get('scm.defaultViewMode')); - this.toDispose.push(this.scmPreferences.onPreferenceChanged((e: PreferenceChangeEvent) => { + this.toDispose.push(this.scmPreferences.onPreferenceChanged(e => { if (e.preferenceName === 'scm.defaultViewMode') { - this.updateViewMode(e.newValue!); + this.updateViewMode(e.newValue); } })); diff --git a/packages/terminal/src/browser/terminal-preferences.ts b/packages/terminal/src/browser/terminal-preferences.ts index 8a67f8c38ba3b..7c6b45adddee2 100644 --- a/packages/terminal/src/browser/terminal-preferences.ts +++ b/packages/terminal/src/browser/terminal-preferences.ts @@ -151,6 +151,7 @@ export const TerminalConfigSchema: PreferenceSchema = { export interface TerminalConfiguration { 'terminal.enableCopy': boolean 'terminal.enablePaste': boolean + // xterm compatible, see https://xtermjs.org/docs/api/terminal/interfaces/iterminaloptions/ 'terminal.integrated.fontFamily': string 'terminal.integrated.fontSize': number 'terminal.integrated.fontWeight': FontWeight @@ -165,9 +166,9 @@ export interface TerminalConfiguration { 'terminal.integrated.cursorBlinking': boolean, 'terminal.integrated.cursorStyle': CursorStyleVSCode, 'terminal.integrated.cursorWidth': number, - 'terminal.integrated.shell.windows': string | undefined, - 'terminal.integrated.shell.osx': string | undefined, - 'terminal.integrated.shell.linux': string | undefined, + 'terminal.integrated.shell.windows': string | null | undefined, + 'terminal.integrated.shell.osx': string | null | undefined, + 'terminal.integrated.shell.linux': string | null | undefined, 'terminal.integrated.shellArgs.windows': string[], 'terminal.integrated.shellArgs.osx': string[], 'terminal.integrated.shellArgs.linux': string[], diff --git a/packages/terminal/src/browser/terminal-widget-impl.ts b/packages/terminal/src/browser/terminal-widget-impl.ts index 9f442189bf03d..373700c38482d 100644 --- a/packages/terminal/src/browser/terminal-widget-impl.ts +++ b/packages/terminal/src/browser/terminal-widget-impl.ts @@ -148,18 +148,16 @@ export class TerminalWidgetImpl extends TerminalWidget implements StatefulWidget const lastSeparator = change.preferenceName.lastIndexOf('.'); if (lastSeparator > 0) { let preferenceName = change.preferenceName.substr(lastSeparator + 1); - let preferenceValue = this.preferences[change.preferenceName]; + let preferenceValue = change.newValue; if (preferenceName === 'rendererType') { - const newRendererType: string = this.preferences[change.preferenceName] as string; + const newRendererType = preferenceValue as string; if (newRendererType !== this.getTerminalRendererType(newRendererType)) { - // given terminal renderer type is not supported or invalid + // Given terminal renderer type is not supported or invalid preferenceValue = DEFAULT_TERMINAL_RENDERER_TYPE; } - } - - // Convert the terminal preference into a valid `xterm` option. - if (preferenceName === 'cursorBlinking') { + } else if (preferenceName === 'cursorBlinking') { + // Convert the terminal preference into a valid `xterm` option preferenceName = 'cursorBlink'; } else if (preferenceName === 'cursorStyle') { preferenceValue = this.getCursorStyle(); @@ -635,9 +633,9 @@ export class TerminalWidgetImpl extends TerminalWidget implements StatefulWidget protected get shellPreferences(): IShellTerminalPreferences { return { shell: { - Windows: this.preferences['terminal.integrated.shell.windows'], - Linux: this.preferences['terminal.integrated.shell.linux'], - OSX: this.preferences['terminal.integrated.shell.osx'], + Windows: this.preferences['terminal.integrated.shell.windows'] ?? undefined, + Linux: this.preferences['terminal.integrated.shell.linux'] ?? undefined, + OSX: this.preferences['terminal.integrated.shell.osx'] ?? undefined, }, shellArgs: { Windows: this.preferences['terminal.integrated.shellArgs.windows'], diff --git a/packages/workspace/src/browser/workspace-service.ts b/packages/workspace/src/browser/workspace-service.ts index 8b792a774dcd3..5d6cf8de5e125 100644 --- a/packages/workspace/src/browser/workspace-service.ts +++ b/packages/workspace/src/browser/workspace-service.ts @@ -95,8 +95,8 @@ export class WorkspaceService implements FrontendApplicationContribution { this.updateWorkspace(); } }); - this.fsPreferences.onPreferenceChanged(e => { - if (e.preferenceName === 'files.watcherExclude') { + this.fsPreferences.onPreferenceChanged(event => { + if (event.preferenceName === 'files.watcherExclude') { this.refreshRootWatchers(); } });