From e2dd710d998f0538bde3dffbd330d33339aadc56 Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Wed, 12 Oct 2022 15:43:11 +0200 Subject: [PATCH 01/13] Revert "Always dispose test editor even if a test fails" This reverts commit 7f17834a595c7f0977ff6407994a8e4f531cc920. --- src/vs/editor/test/browser/testCodeEditor.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/vs/editor/test/browser/testCodeEditor.ts b/src/vs/editor/test/browser/testCodeEditor.ts index 9fb9e854b35..9278f28c685 100644 --- a/src/vs/editor/test/browser/testCodeEditor.ts +++ b/src/vs/editor/test/browser/testCodeEditor.ts @@ -133,7 +133,7 @@ function isTextModel(arg: ITextModel | string | string[] | ITextBufferFactory): function _withTestCodeEditor(arg: ITextModel | string | string[] | ITextBufferFactory, options: TestCodeEditorInstantiationOptions, callback: (editor: ITestCodeEditor, viewModel: ViewModel, instantiationService: TestInstantiationService) => void): void; function _withTestCodeEditor(arg: ITextModel | string | string[] | ITextBufferFactory, options: TestCodeEditorInstantiationOptions, callback: (editor: ITestCodeEditor, viewModel: ViewModel, instantiationService: TestInstantiationService) => Promise): Promise; -async function _withTestCodeEditor(arg: ITextModel | string | string[] | ITextBufferFactory, options: TestCodeEditorInstantiationOptions, callback: (editor: ITestCodeEditor, viewModel: ViewModel, instantiationService: TestInstantiationService) => Promise | void): Promise | void> { +function _withTestCodeEditor(arg: ITextModel | string | string[] | ITextBufferFactory, options: TestCodeEditorInstantiationOptions, callback: (editor: ITestCodeEditor, viewModel: ViewModel, instantiationService: TestInstantiationService) => Promise | void): Promise | void { const disposables = new DisposableStore(); const instantiationService = createCodeEditorServices(disposables, options.serviceCollection); delete options.serviceCollection; @@ -149,12 +149,12 @@ async function _withTestCodeEditor(arg: ITextModel | string | string[] | ITextBu const editor = disposables.add(instantiateTestCodeEditor(instantiationService, model, options)); const viewModel = editor.getViewModel()!; viewModel.setHasFocus(true); - try { - const result = callback(editor, editor.getViewModel()!, instantiationService); - await result; - } finally { - disposables.dispose(); + const result = callback(editor, editor.getViewModel()!, instantiationService); + if (result) { + return result.then(() => disposables.dispose()); } + + disposables.dispose(); } export function createCodeEditorServices(disposables: DisposableStore, services: ServiceCollection = new ServiceCollection()): TestInstantiationService { From 3c9854fe860a3e679aab5312674e8a8f4229017c Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Wed, 12 Oct 2022 22:35:13 +0200 Subject: [PATCH 02/13] Fixes #95328: Clamp the hover result range to make sure the hover stays on the same view line --- .../contrib/hover/browser/contentHover.ts | 21 ++++++-- .../hover/test/browser/contentHover.test.ts | 49 ++++++++++++++----- 2 files changed, 54 insertions(+), 16 deletions(-) diff --git a/src/vs/editor/contrib/hover/browser/contentHover.ts b/src/vs/editor/contrib/hover/browser/contentHover.ts index 30de80e0952..e0f2246f5a6 100644 --- a/src/vs/editor/contrib/hover/browser/contentHover.ts +++ b/src/vs/editor/contrib/hover/browser/contentHover.ts @@ -24,6 +24,7 @@ import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; import { Context as SuggestContext } from 'vs/editor/contrib/suggest/browser/suggest'; import { AsyncIterableObject } from 'vs/base/common/async'; import { EditorContextKeys } from 'vs/editor/common/editorContextKeys'; +import { Constants } from 'vs/base/common/uint'; const $ = dom.$; @@ -240,7 +241,7 @@ export class ContentHoverController extends Disposable { } private _renderMessages(anchor: HoverAnchor, messages: IHoverPart[]): void { - const { showAtPosition, showAtRange, highlightRange } = ContentHoverController.computeHoverRanges(anchor.range, messages); + const { showAtPosition, showAtRange, highlightRange } = ContentHoverController.computeHoverRanges(this._editor, anchor.range, messages); const disposables = new DisposableStore(); const statusBar = disposables.add(new EditorHoverStatusBar(this._keybindingService)); @@ -301,7 +302,19 @@ export class ContentHoverController extends Disposable { className: 'hoverHighlight' }); - public static computeHoverRanges(anchorRange: Range, messages: IHoverPart[]) { + public static computeHoverRanges(editor: ICodeEditor, anchorRange: Range, messages: IHoverPart[]) { + let startColumnBoundary = 1; + let endColumnBoundary = Constants.MAX_SAFE_SMALL_INTEGER; + if (editor.hasModel()) { + // Ensure the range is on the current view line + const viewModel = editor._getViewModel(); + const coordinatesConverter = viewModel.coordinatesConverter; + const anchorViewRange = coordinatesConverter.convertModelRangeToViewRange(anchorRange); + const anchorViewRangeStart = new Position(anchorViewRange.startLineNumber, viewModel.getLineMinColumn(anchorViewRange.startLineNumber)); + const anchorViewRangeEnd = new Position(anchorViewRange.endLineNumber, viewModel.getLineMaxColumn(anchorViewRange.endLineNumber)); + startColumnBoundary = coordinatesConverter.convertViewPositionToModelPosition(anchorViewRangeStart).column; + endColumnBoundary = coordinatesConverter.convertViewPositionToModelPosition(anchorViewRangeEnd).column; + } // The anchor range is always on a single line const anchorLineNumber = anchorRange.startLineNumber; let renderStartColumn = anchorRange.startColumn; @@ -313,8 +326,8 @@ export class ContentHoverController extends Disposable { highlightRange = Range.plusRange(highlightRange, msg.range); if (msg.range.startLineNumber === anchorLineNumber && msg.range.endLineNumber === anchorLineNumber) { // this message has a range that is completely sitting on the line of the anchor - renderStartColumn = Math.min(renderStartColumn, msg.range.startColumn); - renderEndColumn = Math.max(renderEndColumn, msg.range.endColumn); + renderStartColumn = Math.max(Math.min(renderStartColumn, msg.range.startColumn), startColumnBoundary); + renderEndColumn = Math.min(Math.max(renderEndColumn, msg.range.endColumn), endColumnBoundary); } if (msg.forceShowAtRange) { forceShowAtRange = msg.range; diff --git a/src/vs/editor/contrib/hover/test/browser/contentHover.test.ts b/src/vs/editor/contrib/hover/test/browser/contentHover.test.ts index 7e35cea943b..1dacdd5a203 100644 --- a/src/vs/editor/contrib/hover/test/browser/contentHover.test.ts +++ b/src/vs/editor/contrib/hover/test/browser/contentHover.test.ts @@ -8,20 +8,45 @@ import { ContentHoverController } from 'vs/editor/contrib/hover/browser/contentH import { Range } from 'vs/editor/common/core/range'; import { Position } from 'vs/editor/common/core/position'; import { IHoverPart } from 'vs/editor/contrib/hover/browser/hoverTypes'; +import { TestCodeEditorInstantiationOptions, withTestCodeEditor } from 'vs/editor/test/browser/testCodeEditor'; suite('Content Hover', () => { test('issue #151235: Gitlens hover shows up in the wrong place', () => { - const actual = ContentHoverController.computeHoverRanges( - new Range(5, 5, 5, 5), - [{ range: new Range(4, 1, 5, 6) }] - ); - assert.deepStrictEqual( - actual, - { - showAtPosition: new Position(5, 5), - showAtRange: new Range(5, 5, 5, 5), - highlightRange: new Range(4, 1, 5, 6) - } - ); + const text = 'just some text'; + withTestCodeEditor(text, {}, (editor) => { + const actual = ContentHoverController.computeHoverRanges( + editor, + new Range(5, 5, 5, 5), + [{ range: new Range(4, 1, 5, 6) }] + ); + assert.deepStrictEqual( + actual, + { + showAtPosition: new Position(5, 5), + showAtRange: new Range(5, 5, 5, 5), + highlightRange: new Range(4, 1, 5, 6) + } + ); + }); + }); + + test('issue #95328: Hover placement with word-wrap', () => { + const text = 'just some text'; + const opts: TestCodeEditorInstantiationOptions = { wordWrap: 'wordWrapColumn', wordWrapColumn: 6 }; + withTestCodeEditor(text, opts, (editor) => { + const actual = ContentHoverController.computeHoverRanges( + editor, + new Range(1, 8, 1, 8), + [{ range: new Range(1, 1, 1, 15) }] + ); + assert.deepStrictEqual( + actual, + { + showAtPosition: new Position(1, 6), + showAtRange: new Range(1, 6, 1, 11), + highlightRange: new Range(1, 1, 1, 15) + } + ); + }); }); }); From b5542cae84a18a46270e620214f49172d287154d Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Wed, 12 Oct 2022 15:23:09 -0700 Subject: [PATCH 03/13] Document more of lifecycle.ts (#163482) --- src/vs/base/common/lifecycle.ts | 66 ++++++++++++++++++++++++++++++++- 1 file changed, 64 insertions(+), 2 deletions(-) diff --git a/src/vs/base/common/lifecycle.ts b/src/vs/base/common/lifecycle.ts index 08e4da78072..d58b2fa2155 100644 --- a/src/vs/base/common/lifecycle.ts +++ b/src/vs/base/common/lifecycle.ts @@ -6,6 +6,8 @@ import { once } from 'vs/base/common/functional'; import { Iterable } from 'vs/base/common/iterator'; +// #region Disposable Tracking + /** * Enables logging of potentially leaked disposables. * @@ -108,14 +110,31 @@ export function markAsSingleton(singleton: T): T { return singleton; } +// #endregion + +/** + * An object that performs a cleanup operation when `.dispose()` is called. + * + * Some examples of how disposables are used: + * + * - An event listener that removes itself when `.dispose()` is called. + * - A resource such as a file system watcher that cleans up the resource when `.dispose()` is called. + * - The return value from registering a provider. When `.dispose()` is called, the provider is unregistered. + */ export interface IDisposable { dispose(): void; } +/** + * Check if `thing` is {@link IDisposable disposable}. + */ export function isDisposable(thing: E): thing is E & IDisposable { return typeof (thing).dispose === 'function' && (thing).dispose.length === 0; } +/** + * Disposes of the value(s) passed in. + */ export function dispose(disposable: T): T; export function dispose(disposable: T | undefined): T | undefined; export function dispose = Iterable>(disposables: A): A; @@ -157,12 +176,18 @@ export function disposeIfDisposable(disposables: return []; } +/** + * Combine multiple disposable values into a single {@link IDisposable}. + */ export function combinedDisposable(...disposables: IDisposable[]): IDisposable { const parent = toDisposable(() => dispose(disposables)); setParentOfDisposables(disposables, parent); return parent; } +/** + * Turn a function that implements dispose into an {@link IDisposable}. + */ export function toDisposable(fn: () => void): IDisposable { const self = trackDisposable({ dispose: once(() => { @@ -173,6 +198,13 @@ export function toDisposable(fn: () => void): IDisposable { return self; } +/** + * Manages a collection of disposable values. + * + * This is the preferred way to manage multiple disposables. A `DisposableStore` is safer to work with than an + * `IDisposable[]` as it considers edge cases, such as registering the same value multiple times or adding an item to a + * store that has already been disposed of. + */ export class DisposableStore implements IDisposable { static DISABLE_DISPOSED_WARNING = false; @@ -200,7 +232,7 @@ export class DisposableStore implements IDisposable { } /** - * Returns `true` if this object has been disposed + * @return `true` if this object has been disposed of. */ public get isDisposed(): boolean { return this._isDisposed; @@ -221,6 +253,9 @@ export class DisposableStore implements IDisposable { } } + /** + * Add a new {@link IDisposable disposable} to the collection. + */ public add(o: T): T { if (!o) { return o; @@ -242,8 +277,18 @@ export class DisposableStore implements IDisposable { } } +/** + * Abstract base class for a {@link IDisposable disposable} object. + * + * Subclasses can {@linkcode _register} disposables that will be automatically cleaned up when this object is disposed of. + */ export abstract class Disposable implements IDisposable { + /** + * A disposable that does nothing when it is disposed of. + * + * TODO: This should not be a static property. + */ static readonly None = Object.freeze({ dispose() { } }); protected readonly _store = new DisposableStore(); @@ -259,6 +304,9 @@ export abstract class Disposable implements IDisposable { this._store.dispose(); } + /** + * Adds `o` to the collection of disposables managed by this object. + */ protected _register(o: T): T { if ((o as unknown as Disposable) === this) { throw new Error('Cannot register a disposable on itself!'); @@ -297,7 +345,10 @@ export class MutableDisposable implements IDisposable { this._value = value; } - clear() { + /** + * Resets the stored value and disposed of the previously stored value. + */ + clear(): void { this.value = undefined; } @@ -456,12 +507,20 @@ export class DisposableMap implements ID trackDisposable(this); } + /** + * Disposes of all stored values and mark this object as disposed. + * + * Trying to use this object after it has been disposed of is an error. + */ dispose(): void { markAsDisposed(this); this._isDisposed = true; this.clearAndDisposeAll(); } + /** + * Disposes of all stored values and clear the map, but DO NOT mark this object as disposed. + */ clearAndDisposeAll(): void { if (!this._store.size) { return; @@ -494,6 +553,9 @@ export class DisposableMap implements ID this._store.set(key, value); } + /** + * Delete the value stored for `key` from this map and also dispose of it. + */ deleteAndDispose(key: K): void { this._store.get(key)?.dispose(); this._store.delete(key); From f5ced8231b65f7939c29e4cf23949951eb1e6596 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Wed, 12 Oct 2022 15:38:29 -0700 Subject: [PATCH 04/13] Small clean up for terminal tab list rendering lifecycle (#163485) Small clean up for terminal tab list lifecycle - Reuse the same `DisposableStore` across calls to `renderElement` - Make sure we call `dispose` on the actionbar instead of `clear`. This looks like an oversight --- .../terminal/browser/terminalTabsList.ts | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/browser/terminalTabsList.ts b/src/vs/workbench/contrib/terminal/browser/terminalTabsList.ts index 6baeb8f6596..82d9b1747e2 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalTabsList.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalTabsList.ts @@ -261,7 +261,8 @@ class TerminalTabsRenderer implements IListRenderer { e.stopImmediatePropagation(); @@ -456,16 +453,14 @@ class TerminalTabsRenderer implements IListRenderer Date: Wed, 12 Oct 2022 15:38:57 -0700 Subject: [PATCH 05/13] add accessibility setting tag (#163358) --- src/vs/editor/common/config/editorOptions.ts | 4 +++- .../workbench/browser/workbench.contribution.ts | 1 + .../audioCues/browser/audioCues.contribution.ts | 9 ++++++--- .../browser/preferences.contribution.ts | 17 +++++++++++++++++ .../preferences/browser/settingsEditor2.ts | 1 + .../terminal/common/terminalConfiguration.ts | 3 ++- .../electron-sandbox/desktop.contribution.ts | 3 ++- 7 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/vs/editor/common/config/editorOptions.ts b/src/vs/editor/common/config/editorOptions.ts index 6e08c683030..f3ac6a40cf9 100644 --- a/src/vs/editor/common/config/editorOptions.ts +++ b/src/vs/editor/common/config/editorOptions.ts @@ -1183,6 +1183,7 @@ class EditorAccessibilitySupport extends BaseEditorOption(ConfigurationExtensions.Con localize('workbench.reduceMotion.auto', "Render with reduced motion based on OS configuration."), ], default: 'auto', + tags: ['accessibility'], enum: ['on', 'off', 'auto'] }, 'workbench.layoutControl.enabled': { diff --git a/src/vs/workbench/contrib/audioCues/browser/audioCues.contribution.ts b/src/vs/workbench/contrib/audioCues/browser/audioCues.contribution.ts index 74b817d6c1a..4519b93f7e0 100644 --- a/src/vs/workbench/contrib/audioCues/browser/audioCues.contribution.ts +++ b/src/vs/workbench/contrib/audioCues/browser/audioCues.contribution.ts @@ -29,27 +29,30 @@ const audioCueFeatureBase: IConfigurationPropertySchema = { localize('audioCues.enabled.on', "Enable audio cue."), localize('audioCues.enabled.off', "Disable audio cue.") ], + tags: ['accessibility'] }; Registry.as(ConfigurationExtensions.Configuration).registerConfiguration({ 'properties': { 'audioCues.enabled': { markdownDeprecationMessage: 'Deprecated. Use the specific setting for each audio cue instead (`audioCues.*`).', + tags: ['accessibility'] }, 'audioCues.volume': { 'description': localize('audioCues.volume', "The volume of the audio cues in percent (0-100)."), 'type': 'number', 'minimum': 0, 'maximum': 100, - 'default': 70 + 'default': 70, + tags: ['accessibility'] }, 'audioCues.lineHasBreakpoint': { 'description': localize('audioCues.lineHasBreakpoint', "Plays a sound when the active line has a breakpoint."), - ...audioCueFeatureBase, + ...audioCueFeatureBase }, 'audioCues.lineHasInlineSuggestion': { 'description': localize('audioCues.lineHasInlineSuggestion', "Plays a sound when the active line has an inline suggestion."), - ...audioCueFeatureBase, + ...audioCueFeatureBase }, 'audioCues.lineHasError': { 'description': localize('audioCues.lineHasError', "Plays a sound when the active line has an error."), diff --git a/src/vs/workbench/contrib/preferences/browser/preferences.contribution.ts b/src/vs/workbench/contrib/preferences/browser/preferences.contribution.ts index e1aab482fb5..2e3ffc352c1 100644 --- a/src/vs/workbench/contrib/preferences/browser/preferences.contribution.ts +++ b/src/vs/workbench/contrib/preferences/browser/preferences.contribution.ts @@ -422,6 +422,23 @@ class PreferencesActionsContribution extends Disposable implements IWorkbenchCon return accessor.get(IPreferencesService).openWorkspaceSettings(args); } }); + + registerAction2(class extends Action2 { + constructor() { + super({ + id: 'workbench.action.openAccessibilitySettings', + title: { value: nls.localize('openAccessibilitySettings', "Open Accessibility Settings"), original: 'Open Accessibility Settings' }, + category, + menu: { + id: MenuId.CommandPalette, + when: WorkbenchStateContext.notEqualsTo('empty') + } + }); + } + async run(accessor: ServicesAccessor) { + await accessor.get(IPreferencesService).openSettings({ jsonEditor: false, query: '@tag:accessibility' }); + } + }); registerAction2(class extends Action2 { constructor() { super({ diff --git a/src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts b/src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts index 69f441beb3d..f12a3672b7b 100644 --- a/src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts +++ b/src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts @@ -114,6 +114,7 @@ export class SettingsEditor2 extends EditorPane { '@tag:sync', '@tag:usesOnlineServices', '@tag:telemetry', + '@tag:accessibility', `@${ID_SETTING_TAG}`, `@${EXTENSION_SETTING_TAG}`, `@${FEATURE_SETTING_TAG}scm`, diff --git a/src/vs/workbench/contrib/terminal/common/terminalConfiguration.ts b/src/vs/workbench/contrib/terminal/common/terminalConfiguration.ts index 790e1777020..ed2caa4f5c2 100644 --- a/src/vs/workbench/contrib/terminal/common/terminalConfiguration.ts +++ b/src/vs/workbench/contrib/terminal/common/terminalConfiguration.ts @@ -186,7 +186,8 @@ const terminalConfiguration: IConfigurationNode = { [TerminalSettingId.MinimumContrastRatio]: { markdownDescription: localize('terminal.integrated.minimumContrastRatio', "When set, the foreground color of each cell will change to try meet the contrast ratio specified. Note that this will not apply to `powerline` characters per #146406. Example values:\n\n- 1: Do nothing and use the standard theme colors.\n- 4.5: [WCAG AA compliance (minimum)](https://www.w3.org/TR/UNDERSTANDING-WCAG20/visual-audio-contrast-contrast.html) (default).\n- 7: [WCAG AAA compliance (enhanced)](https://www.w3.org/TR/UNDERSTANDING-WCAG20/visual-audio-contrast7.html).\n- 21: White on black or black on white."), type: 'number', - default: 4.5 + default: 4.5, + tags: ['accessibility'] }, [TerminalSettingId.FastScrollSensitivity]: { markdownDescription: localize('terminal.integrated.fastScrollSensitivity', "Scrolling speed multiplier when pressing `Alt`."), diff --git a/src/vs/workbench/electron-sandbox/desktop.contribution.ts b/src/vs/workbench/electron-sandbox/desktop.contribution.ts index 95e31623896..d2a4a62970b 100644 --- a/src/vs/workbench/electron-sandbox/desktop.contribution.ts +++ b/src/vs/workbench/electron-sandbox/desktop.contribution.ts @@ -170,7 +170,8 @@ import product from 'vs/platform/product/common/product'; 'type': 'number', 'default': 0, 'description': localize('zoomLevel', "Adjust the zoom level of the window. The original size is 0 and each increment above (e.g. 1) or below (e.g. -1) represents zooming 20% larger or smaller. You can also enter decimals to adjust the zoom level with a finer granularity."), - ignoreSync: true + ignoreSync: true, + tags: ['accessibility'] }, 'window.newWindowDimensions': { 'type': 'string', From 07b87405e7a370a90d190891c32768e717bc665e Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Wed, 12 Oct 2022 16:03:07 -0700 Subject: [PATCH 06/13] Don't depend on typescript protocol.d.ts (#163365) --- .../src/languageFeatures/completions.ts | 2 +- .../src/protocol.d.ts | 16 +++++++++++----- .../src/utils/configuration.ts | 7 ++++--- .../src/utils/tsconfig.ts | 2 +- 4 files changed, 17 insertions(+), 10 deletions(-) diff --git a/extensions/typescript-language-features/src/languageFeatures/completions.ts b/extensions/typescript-language-features/src/languageFeatures/completions.ts index a7b1e6c061c..68eb03dd690 100644 --- a/extensions/typescript-language-features/src/languageFeatures/completions.ts +++ b/extensions/typescript-language-features/src/languageFeatures/completions.ts @@ -521,7 +521,7 @@ class MyCompletionItem extends vscode.CompletionItem { } } -function getScriptKindDetails(tsEntry: protocol.CompletionEntry,): string | undefined { +function getScriptKindDetails(tsEntry: Proto.CompletionEntry,): string | undefined { if (!tsEntry.kindModifiers || tsEntry.kind !== PConst.Kind.script) { return; } diff --git a/extensions/typescript-language-features/src/protocol.d.ts b/extensions/typescript-language-features/src/protocol.d.ts index 7e9e45f2575..1aec9e082ed 100644 --- a/extensions/typescript-language-features/src/protocol.d.ts +++ b/extensions/typescript-language-features/src/protocol.d.ts @@ -1,13 +1,19 @@ -import * as Proto from 'typescript/lib/protocol'; -export = Proto; +import * as ts from 'typescript/lib/tsserverlibrary'; +export = ts.server.protocol; + declare enum ServerType { Syntax = 'syntax', Semantic = 'semantic', } -declare module 'typescript/lib/protocol' { - interface Response { - readonly _serverType?: ServerType; +declare module 'typescript/lib/tsserverlibrary' { + namespace server.protocol { + type TextInsertion = ts.TextInsertion; + type ScriptElementKind = ts.ScriptElementKind; + + interface Response { + readonly _serverType?: ServerType; + } } } diff --git a/extensions/typescript-language-features/src/utils/configuration.ts b/extensions/typescript-language-features/src/utils/configuration.ts index efa442a5d0d..d1bfe62f738 100644 --- a/extensions/typescript-language-features/src/utils/configuration.ts +++ b/extensions/typescript-language-features/src/utils/configuration.ts @@ -5,6 +5,7 @@ import * as vscode from 'vscode'; import * as objects from '../utils/objects'; +import * as Proto from '../protocol'; export enum TsServerLogLevel { Off, @@ -112,7 +113,7 @@ export interface TypeScriptServiceConfiguration { readonly enableProjectDiagnostics: boolean; readonly maxTsServerMemory: number; readonly enablePromptUseWorkspaceTsdk: boolean; - readonly watchOptions: protocol.WatchOptions | undefined; + readonly watchOptions: Proto.WatchOptions | undefined; readonly includePackageJsonAutoImports: 'auto' | 'on' | 'off' | undefined; readonly enableTsServerTracing: boolean; } @@ -196,8 +197,8 @@ export abstract class BaseServiceConfigurationProvider implements ServiceConfigu return configuration.get('typescript.tsserver.experimental.enableProjectDiagnostics', false); } - protected readWatchOptions(configuration: vscode.WorkspaceConfiguration): protocol.WatchOptions | undefined { - return configuration.get('typescript.tsserver.watchOptions'); + protected readWatchOptions(configuration: vscode.WorkspaceConfiguration): Proto.WatchOptions | undefined { + return configuration.get('typescript.tsserver.watchOptions'); } protected readIncludePackageJsonAutoImports(configuration: vscode.WorkspaceConfiguration): 'auto' | 'on' | 'off' | undefined { diff --git a/extensions/typescript-language-features/src/utils/tsconfig.ts b/extensions/typescript-language-features/src/utils/tsconfig.ts index baf1d4fd59b..1c71d659126 100644 --- a/extensions/typescript-language-features/src/utils/tsconfig.ts +++ b/extensions/typescript-language-features/src/utils/tsconfig.ts @@ -163,7 +163,7 @@ export async function openProjectConfigForFile( return; } - let res: ServerResponse.Response | undefined; + let res: ServerResponse.Response | undefined; try { res = await client.execute('projectInfo', { file, needFileNameList: false }, nulToken); } catch { From 7cfb34dff428cabc0a96ac223a8381f802dca9d7 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Wed, 12 Oct 2022 16:47:22 -0700 Subject: [PATCH 07/13] Freeze the notebook kernel context (#163488) This is an API object so it should not be extensible --- .../notebook/browser/view/renderers/webviewPreloads.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts index 6062c79818c..4533b9a5255 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts @@ -235,10 +235,10 @@ async function webviewPreloads(ctx: PreloadContext) { } function createKernelContext(): KernelPreloadContext { - return { + return Object.freeze({ onDidReceiveKernelMessage: onDidReceiveKernelMessage.event, postKernelMessage: (data: unknown) => postNotebookMessage('customKernelMessage', { message: data }), - }; + }); } const invokeSourceWithGlobals = (functionSrc: string, globals: { [name: string]: unknown }) => { From 24a2b8af8c7b99c4ef91b2b7f4ae97d313b7c9e3 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Wed, 12 Oct 2022 16:49:40 -0700 Subject: [PATCH 08/13] Re-apply: Clean up how the notebook renderer entrypoint is passed around (#163486) Revert "Revert "Clean up how the notebook renderer entrypoint is passed around (#163373)"" This reverts commit a3d8c337610dec6dbd24f4250639715327fb613f. Fixes #163415 --- .../browser/notebookExtensionPoint.ts | 4 +-- .../view/renderers/backLayerWebView.ts | 8 +++-- .../browser/view/renderers/webviewMessages.ts | 3 +- .../browser/view/renderers/webviewPreloads.ts | 8 ++--- .../contrib/notebook/common/notebookCommon.ts | 20 ++++++------ .../notebook/common/notebookOutputRenderer.ts | 31 +++++++------------ 6 files changed, 33 insertions(+), 41 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/notebookExtensionPoint.ts b/src/vs/workbench/contrib/notebook/browser/notebookExtensionPoint.ts index 651082499a8..f28fc03d29b 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookExtensionPoint.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookExtensionPoint.ts @@ -6,7 +6,7 @@ import { IJSONSchema } from 'vs/base/common/jsonSchema'; import * as nls from 'vs/nls'; import { ExtensionsRegistry } from 'vs/workbench/services/extensions/common/extensionsRegistry'; -import { NotebookEditorPriority, NotebookRendererEntrypoint, RendererMessagingSpec } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { NotebookEditorPriority, ContributedNotebookRendererEntrypoint, RendererMessagingSpec } from 'vs/workbench/contrib/notebook/common/notebookCommon'; const NotebookEditorContribution = Object.freeze({ type: 'type', @@ -36,7 +36,7 @@ export interface INotebookRendererContribution { readonly [NotebookRendererContribution.id]?: string; readonly [NotebookRendererContribution.displayName]: string; readonly [NotebookRendererContribution.mimeTypes]?: readonly string[]; - readonly [NotebookRendererContribution.entrypoint]: NotebookRendererEntrypoint; + readonly [NotebookRendererContribution.entrypoint]: ContributedNotebookRendererEntrypoint; readonly [NotebookRendererContribution.hardDependencies]: readonly string[]; readonly [NotebookRendererContribution.optionalDependencies]: readonly string[]; readonly [NotebookRendererContribution.requiresMessaging]: RendererMessagingSpec; diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts index f01090a0561..ba6810f0afb 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts @@ -403,12 +403,14 @@ export class BackLayerWebView extends Disposable { private getRendererData(): RendererMetadata[] { return this.notebookService.getRenderers().map((renderer): RendererMetadata => { - const entrypoint = this.asWebviewUri(renderer.entrypoint, renderer.extensionLocation).toString(); + const entrypoint = { + extends: renderer.entrypoint.extends, + path: this.asWebviewUri(renderer.entrypoint.path, renderer.extensionLocation).toString() + }; return { id: renderer.id, entrypoint, mimeTypes: renderer.mimeTypes, - extends: renderer.extends, messaging: renderer.messaging !== RendererMessagingSpec.Never, isBuiltin: renderer.isBuiltin }; @@ -923,7 +925,7 @@ var requirejs = (function() { const notebookDir = this.getNotebookBaseUri(); return [ ...this.notebookService.getNotebookProviderResourceRoots(), - ...this.notebookService.getRenderers().map(x => dirname(x.entrypoint)), + ...this.notebookService.getRenderers().map(x => dirname(x.entrypoint.path)), ...workspaceFolders, notebookDir, ...this.getBuiltinLocalResourceRoots() diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewMessages.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewMessages.ts index 79adc52e51e..8b0902ba129 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewMessages.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewMessages.ts @@ -275,9 +275,8 @@ export interface IUpdateControllerPreloadsMessage { export interface RendererMetadata { readonly id: string; - readonly entrypoint: string; + readonly entrypoint: { readonly extends: string | undefined; readonly path: string }; readonly mimeTypes: readonly string[]; - readonly extends: string | undefined; readonly messaging: boolean; readonly isBuiltin: boolean; } diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts index 4533b9a5255..4907be7643d 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts @@ -1314,7 +1314,7 @@ async function webviewPreloads(ctx: PreloadContext) { /** Inner function cached in the _loadPromise(). */ private async _load(): Promise { - const module: RendererModule = await __import(this.data.entrypoint); + const module: RendererModule = await __import(this.data.entrypoint.path); if (!module) { return; } @@ -1324,7 +1324,7 @@ async function webviewPreloads(ctx: PreloadContext) { // Load all renderers that extend this renderer await Promise.all( ctx.rendererData - .filter(d => d.extends === this.data.id) + .filter(d => d.entrypoint.extends === this.data.id) .map(async d => { const renderer = renderers.getRenderer(d.id); if (!renderer) { @@ -1436,7 +1436,7 @@ async function webviewPreloads(ctx: PreloadContext) { } private rendererEqual(a: webviewMessages.RendererMetadata, b: webviewMessages.RendererMetadata) { - if (a.entrypoint !== b.entrypoint || a.id !== b.id || a.extends !== b.extends || a.messaging !== b.messaging) { + if (a.id !== b.id || a.entrypoint.path !== b.entrypoint.path || a.entrypoint.extends !== b.entrypoint.extends || a.messaging !== b.messaging) { return false; } @@ -1497,7 +1497,7 @@ async function webviewPreloads(ctx: PreloadContext) { .find((renderer) => renderer.data.id === preferredRendererId); } else { const renderers = Array.from(this._renderers.values()) - .filter((renderer) => renderer.data.mimeTypes.includes(info.mime) && !renderer.data.extends); + .filter((renderer) => renderer.data.mimeTypes.includes(info.mime) && !renderer.data.entrypoint.extends); if (renderers.length) { // De-prioritize built-in renderers diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index 0e2f1fe9ea6..403a07a082f 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -75,7 +75,7 @@ export const RENDERER_EQUIVALENT_EXTENSIONS: ReadonlyMap; - extensionLocation: URI; - extensionId: ExtensionIdentifier; - messaging: RendererMessagingSpec; + readonly id: string; + readonly displayName: string; + readonly entrypoint: NotebookRendererEntrypoint; + readonly extensionLocation: URI; + readonly extensionId: ExtensionIdentifier; + readonly messaging: RendererMessagingSpec; readonly mimeTypes: readonly string[]; - readonly dependencies: readonly string[]; - readonly isBuiltin: boolean; matchesWithoutKernel(mimeType: string): NotebookRendererMatch; diff --git a/src/vs/workbench/contrib/notebook/common/notebookOutputRenderer.ts b/src/vs/workbench/contrib/notebook/common/notebookOutputRenderer.ts index e22c7fd486f..72c2e6ebf3e 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookOutputRenderer.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookOutputRenderer.ts @@ -8,7 +8,7 @@ import { Iterable } from 'vs/base/common/iterator'; import { joinPath } from 'vs/base/common/resources'; import { URI } from 'vs/base/common/uri'; import { ExtensionIdentifier, IExtensionDescription } from 'vs/platform/extensions/common/extensions'; -import { INotebookRendererInfo, NotebookRendererEntrypoint, NotebookRendererMatch, RendererMessagingSpec } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { INotebookRendererInfo, ContributedNotebookRendererEntrypoint, NotebookRendererMatch, RendererMessagingSpec, NotebookRendererEntrypoint } from 'vs/workbench/contrib/notebook/common/notebookCommon'; class DependencyList { private readonly value: ReadonlySet; @@ -19,10 +19,6 @@ class DependencyList { this.defined = this.value.size > 0; } - public values(): string[] { - return Array.from(this.value); - } - /** Gets whether any of the 'available' dependencies match the ones in this list */ public matches(available: ReadonlyArray) { // For now this is simple, but this may expand to support globs later @@ -34,17 +30,13 @@ class DependencyList { export class NotebookOutputRendererInfo implements INotebookRendererInfo { readonly id: string; - readonly extends?: string; - readonly entrypoint: URI; + readonly entrypoint: NotebookRendererEntrypoint; readonly displayName: string; readonly extensionLocation: URI; readonly extensionId: ExtensionIdentifier; readonly hardDependencies: DependencyList; readonly optionalDependencies: DependencyList; - /** @see RendererMessagingSpec */ readonly messaging: RendererMessagingSpec; - // todo: re-add preloads in pure renderer API - readonly preloads: ReadonlyArray = []; readonly mimeTypes: readonly string[]; private readonly mimeTypeGlobs: glob.ParsedPattern[]; @@ -54,7 +46,7 @@ export class NotebookOutputRendererInfo implements INotebookRendererInfo { constructor(descriptor: { readonly id: string; readonly displayName: string; - readonly entrypoint: NotebookRendererEntrypoint; + readonly entrypoint: ContributedNotebookRendererEntrypoint; readonly mimeTypes: readonly string[]; readonly extension: IExtensionDescription; readonly dependencies: readonly string[] | undefined; @@ -67,10 +59,15 @@ export class NotebookOutputRendererInfo implements INotebookRendererInfo { this.isBuiltin = descriptor.extension.isBuiltin; if (typeof descriptor.entrypoint === 'string') { - this.entrypoint = joinPath(this.extensionLocation, descriptor.entrypoint); + this.entrypoint = { + extends: undefined, + path: joinPath(this.extensionLocation, descriptor.entrypoint) + }; } else { - this.extends = descriptor.entrypoint.extends; - this.entrypoint = joinPath(this.extensionLocation, descriptor.entrypoint.path); + this.entrypoint = { + extends: descriptor.entrypoint.extends, + path: joinPath(this.extensionLocation, descriptor.entrypoint.path) + }; } this.displayName = descriptor.displayName; @@ -81,10 +78,6 @@ export class NotebookOutputRendererInfo implements INotebookRendererInfo { this.messaging = descriptor.requiresMessaging ?? RendererMessagingSpec.Never; } - public get dependencies(): string[] { - return this.hardDependencies.values(); - } - public matchesWithoutKernel(mimeType: string) { if (!this.matchesMimeTypeOnly(mimeType)) { return NotebookRendererMatch.Never; @@ -118,7 +111,7 @@ export class NotebookOutputRendererInfo implements INotebookRendererInfo { } private matchesMimeTypeOnly(mimeType: string) { - if (this.extends !== undefined) { + if (this.entrypoint.extends) { // We're extending another renderer return false; } From 5b7233c5e3977662532e77c96a3d0fcad1d25753 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Wed, 12 Oct 2022 17:01:38 -0700 Subject: [PATCH 09/13] Use for-of loop instead (#163491) --- .../contrib/webview/browser/webviewElement.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/vs/workbench/contrib/webview/browser/webviewElement.ts b/src/vs/workbench/contrib/webview/browser/webviewElement.ts index 45e735a28ed..433b3789392 100644 --- a/src/vs/workbench/contrib/webview/browser/webviewElement.ts +++ b/src/vs/workbench/contrib/webview/browser/webviewElement.ts @@ -545,15 +545,17 @@ export class WebviewElement extends Disposable implements IWebview, WebviewFindD parent.appendChild(this._webviewFindWidget.getDomNode()); } - [EventType.MOUSE_DOWN, EventType.MOUSE_MOVE, EventType.DROP].forEach(eventName => { + for (const eventName of [EventType.MOUSE_DOWN, EventType.MOUSE_MOVE, EventType.DROP]) { this._register(addDisposableListener(parent, eventName, () => { this.stopBlockingIframeDragEvents(); })); - }); + } - [parent, window].forEach(node => this._register(addDisposableListener(node as HTMLElement, EventType.DRAG_END, () => { - this.stopBlockingIframeDragEvents(); - }))); + for (const node of [parent, window]) { + this._register(addDisposableListener(node, EventType.DRAG_END, () => { + this.stopBlockingIframeDragEvents(); + })); + } parent.appendChild(this.element); } From a5e9b83d6550b3dd6ea749718597203b2e770585 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Wed, 12 Oct 2022 17:05:41 -0700 Subject: [PATCH 10/13] ci: fix wrong file in cli tests --- build/azure-pipelines/cli/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/azure-pipelines/cli/test.yml b/build/azure-pipelines/cli/test.yml index d17175e4758..70eb714c1d9 100644 --- a/build/azure-pipelines/cli/test.yml +++ b/build/azure-pipelines/cli/test.yml @@ -7,7 +7,7 @@ parameters: default: stable steps: - - template: ./install-rust.yml + - template: ./install-rust-posix.yml parameters: targets: [] channel: ${{ parameters.VSCODE_CLI_RUST_CHANNEL }} From 2457d9eb469d9fd56ed4ce1ce9887b4b0ef43730 Mon Sep 17 00:00:00 2001 From: Tyler James Leonhardt Date: Wed, 12 Oct 2022 17:08:27 -0700 Subject: [PATCH 11/13] Plumbing for desktop to pick up strings from language packs (#163494) Web will come in the next PR (hence the TODO) Also this includes the smallest translation change which will be the ultimate test that this is all working. --- .../github-authentication/src/githubServer.ts | 2 +- .../languagePacks/browser/languagePacks.ts | 14 +++++++++++ .../languagePacks/common/languagePacks.ts | 4 ++++ .../languagePacks/node/languagePacks.ts | 14 +++++++++++ .../api/browser/mainThreadLocalization.ts | 11 +++++++++ .../workbench/api/common/extHost.protocol.ts | 1 + .../api/common/extHostLocalizationService.ts | 23 ++++++++++--------- 7 files changed, 57 insertions(+), 12 deletions(-) diff --git a/extensions/github-authentication/src/githubServer.ts b/extensions/github-authentication/src/githubServer.ts index 94ce5429e78..5c6bff905db 100644 --- a/extensions/github-authentication/src/githubServer.ts +++ b/extensions/github-authentication/src/githubServer.ts @@ -156,7 +156,7 @@ export class GitHubServer implements IGitHubServer { // Used for showing a friendlier message to the user when the explicitly cancel a flow. let userCancelled: boolean | undefined; - const yes = localize('yes', "Yes"); + const yes = vscode.l10n.t('Yes'); const no = localize('no', "No"); const promptToContinue = async () => { if (userCancelled === undefined) { diff --git a/src/vs/platform/languagePacks/browser/languagePacks.ts b/src/vs/platform/languagePacks/browser/languagePacks.ts index 83b4c051e0f..f7fdae65552 100644 --- a/src/vs/platform/languagePacks/browser/languagePacks.ts +++ b/src/vs/platform/languagePacks/browser/languagePacks.ts @@ -3,9 +3,23 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { URI } from 'vs/base/common/uri'; import { ILanguagePackItem, LanguagePackBaseService } from 'vs/platform/languagePacks/common/languagePacks'; export class WebLanguagePacksService extends LanguagePackBaseService { + // TODO: support builtin extensions using unpkg service + // constructor( + // @IExtensionResourceLoaderService extensionResourceLoaderService: IExtensionResourceLoaderService, + // @IExtensionGalleryService extensionGalleryService: IExtensionGalleryService, + // @ILogService private readonly logService: ILogService + // ) { + // super(extensionGalleryService); + // } + + getTranslationsUri(id: string): Promise { + return Promise.resolve(undefined); + } + // Web doesn't have a concept of language packs, so we just return an empty array getInstalledLanguages(): Promise { return Promise.resolve([]); diff --git a/src/vs/platform/languagePacks/common/languagePacks.ts b/src/vs/platform/languagePacks/common/languagePacks.ts index d316ba89c3e..589e8678326 100644 --- a/src/vs/platform/languagePacks/common/languagePacks.ts +++ b/src/vs/platform/languagePacks/common/languagePacks.ts @@ -6,6 +6,7 @@ import { CancellationTokenSource } from 'vs/base/common/cancellation'; import { Disposable } from 'vs/base/common/lifecycle'; import { language } from 'vs/base/common/platform'; +import { URI } from 'vs/base/common/uri'; import { IQuickPickItem } from 'vs/base/parts/quickinput/common/quickInput'; import { localize } from 'vs/nls'; import { IExtensionGalleryService, IGalleryExtension } from 'vs/platform/extensionManagement/common/extensionManagement'; @@ -22,6 +23,7 @@ export interface ILanguagePackService { readonly _serviceBrand: undefined; getAvailableLanguages(): Promise>; getInstalledLanguages(): Promise>; + getTranslationsUri(id: string): Promise; getLocale(extension: IGalleryExtension): string | undefined; } @@ -32,6 +34,8 @@ export abstract class LanguagePackBaseService extends Disposable implements ILan super(); } + abstract getTranslationsUri(id: string): Promise; + abstract getInstalledLanguages(): Promise>; async getAvailableLanguages(): Promise { diff --git a/src/vs/platform/languagePacks/node/languagePacks.ts b/src/vs/platform/languagePacks/node/languagePacks.ts index defe0957b17..f2d2516c9bd 100644 --- a/src/vs/platform/languagePacks/node/languagePacks.ts +++ b/src/vs/platform/languagePacks/node/languagePacks.ts @@ -16,6 +16,8 @@ import { areSameExtensions } from 'vs/platform/extensionManagement/common/extens import { ILogService } from 'vs/platform/log/common/log'; import { ILocalizationContribution } from 'vs/platform/extensions/common/extensions'; import { ILanguagePackItem, LanguagePackBaseService } from 'vs/platform/languagePacks/common/languagePacks'; +import { locale } from 'vs/base/common/platform'; +import { URI } from 'vs/base/common/uri'; interface ILanguagePack { hash: string; @@ -48,6 +50,18 @@ export class NativeLanguagePackService extends LanguagePackBaseService { }); } + async getTranslationsUri(id: string): Promise { + const packs = await this.cache.getLanguagePacks(); + const pack = packs[locale!]; + if (!pack) { + this.logService.warn(`No language pack found for ${locale}`); + return undefined; + } + + const translation = pack.translations[id]; + return translation ? URI.file(translation) : undefined; + } + async getInstalledLanguages(): Promise> { const languagePacks = await this.cache.getLanguagePacks(); const languages = Object.keys(languagePacks).map(locale => { diff --git a/src/vs/workbench/api/browser/mainThreadLocalization.ts b/src/vs/workbench/api/browser/mainThreadLocalization.ts index 988fb8ba6c3..c3cdd418461 100644 --- a/src/vs/workbench/api/browser/mainThreadLocalization.ts +++ b/src/vs/workbench/api/browser/mainThreadLocalization.ts @@ -8,6 +8,7 @@ import { extHostNamedCustomer, IExtHostContext } from 'vs/workbench/services/ext import { URI, UriComponents } from 'vs/base/common/uri'; import { IFileService } from 'vs/platform/files/common/files'; import { Disposable } from 'vs/base/common/lifecycle'; +import { ILanguagePackService } from 'vs/platform/languagePacks/common/languagePacks'; @extHostNamedCustomer(MainContext.MainThreadLocalization) export class MainThreadLocalization extends Disposable implements MainThreadLocalizationShape { @@ -15,10 +16,20 @@ export class MainThreadLocalization extends Disposable implements MainThreadLoca constructor( extHostContext: IExtHostContext, @IFileService private readonly fileService: IFileService, + @ILanguagePackService private readonly languagePackService: ILanguagePackService ) { super(); } + async $fetchBuiltInBundleUri(id: string): Promise { + try { + const uri = await this.languagePackService.getTranslationsUri(id); + return uri; + } catch (e) { + return undefined; + } + } + async $fetchBundleContents(uriComponents: UriComponents): Promise { const contents = await this.fileService.readFile(URI.revive(uriComponents)); return contents.value.toString(); diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 3bb8699d408..95ff686a02b 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -2144,6 +2144,7 @@ export interface MainThreadThemingShape extends IDisposable { } export interface MainThreadLocalizationShape extends IDisposable { + $fetchBuiltInBundleUri(id: string): Promise; $fetchBundleContents(uriComponents: UriComponents): Promise; } diff --git a/src/vs/workbench/api/common/extHostLocalizationService.ts b/src/vs/workbench/api/common/extHostLocalizationService.ts index e2671689ca6..611add6ffce 100644 --- a/src/vs/workbench/api/common/extHostLocalizationService.ts +++ b/src/vs/workbench/api/common/extHostLocalizationService.ts @@ -60,7 +60,7 @@ export class ExtHostLocalizationService implements ExtHostLocalizationShape { async initializeLocalizedMessages(extension: IExtensionDescription): Promise { if (this.isDefaultLanguage // TODO: support builtin extensions - || !extension.l10n + || (!extension.l10n && !extension.isBuiltin) ) { return; } @@ -70,16 +70,17 @@ export class ExtHostLocalizationService implements ExtHostLocalizationShape { } let contents: { [key: string]: string } | undefined; - const bundleLocation = this.getBundleLocation(extension); - if (!bundleLocation) { + const bundleUri = await this.getBundleLocation(extension); + if (!bundleUri) { this.logService.error(`No bundle location found for extension ${extension.identifier.value}`); return; } - const bundleUri = URI.joinPath(bundleLocation, `bundle.l10n.${this.currentLanguage}.json`); try { const response = await this._proxy.$fetchBundleContents(bundleUri); - contents = JSON.parse(response); + const result = JSON.parse(response); + // 'contents.bundle' is a well-known key in the language pack json file that contains the _code_ translations for the extension + contents = extension.isBuiltin ? result.contents?.bundle : result; } catch (e) { this.logService.error(`Failed to load translations for ${extension.identifier.value} from ${bundleUri}: ${e.message}`); return; @@ -93,14 +94,14 @@ export class ExtHostLocalizationService implements ExtHostLocalizationShape { } } - private getBundleLocation(extension: IExtensionDescription): URI | undefined { - // TODO: support builtin extensions using IExtHostInitDataService - // if (extension.isBuiltin && this.initData.nlsBaseUrl) { - // return URI.joinPath(this.initData.nlsBaseUrl, extension.identifier.value, 'main'); - // } + private async getBundleLocation(extension: IExtensionDescription): Promise { + if (extension.isBuiltin) { + const uri = await this._proxy.$fetchBuiltInBundleUri(extension.identifier.value); + return URI.revive(uri); + } return extension.l10n - ? URI.joinPath(extension.extensionLocation, extension.l10n) + ? URI.joinPath(extension.extensionLocation, extension.l10n, `bundle.l10n.${this.currentLanguage}.json`) : undefined; } } From db9f5f9b9bdd06cc037607473513b59d85617082 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Wed, 12 Oct 2022 17:12:14 -0700 Subject: [PATCH 12/13] Fix electron webviews not always updating `_webviewKeyboardHandler` when focus changes (#163495) I believe any focus changes should also update the `_webviewKeyboardHandler`. Right now programatic focuses don't --- .../contrib/webview/browser/webviewElement.ts | 18 ++++++++---------- .../electron-sandbox/webviewElement.ts | 19 ++++++++++--------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/vs/workbench/contrib/webview/browser/webviewElement.ts b/src/vs/workbench/contrib/webview/browser/webviewElement.ts index 433b3789392..51b3d4db948 100644 --- a/src/vs/workbench/contrib/webview/browser/webviewElement.ts +++ b/src/vs/workbench/contrib/webview/browser/webviewElement.ts @@ -37,7 +37,7 @@ import { areWebviewContentOptionsEqual, IWebview, WebviewContentOptions, Webview import { WebviewFindDelegate, WebviewFindWidget } from 'vs/workbench/contrib/webview/browser/webviewFindWidget'; import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService'; -export const enum WebviewMessageChannels { +const enum WebviewMessageChannels { onmessage = 'onmessage', didClickLink = 'did-click-link', didScroll = 'did-scroll', @@ -303,14 +303,14 @@ export class WebviewElement extends Disposable implements IWebview, WebviewFindD this.handleFocusChange(true); })); - this._register(this.on(WebviewMessageChannels.wheel, (event: IMouseWheelEvent) => { - this._onDidWheel.fire(event); - })); - this._register(this.on(WebviewMessageChannels.didBlur, () => { this.handleFocusChange(false); })); + this._register(this.on(WebviewMessageChannels.wheel, (event: IMouseWheelEvent) => { + this._onDidWheel.fire(event); + })); + this._register(this.on(WebviewMessageChannels.didFind, (didFind: boolean) => { this._hasFindResult.fire(didFind); })); @@ -379,7 +379,6 @@ export class WebviewElement extends Disposable implements IWebview, WebviewFindD })); this._register(Event.runAndSubscribe(webviewThemeDataProvider.onThemeDataChanged, () => this.style())); - this._register(_accessibilityService.onDidChangeReducedMotion(() => this.style())); this._register(_accessibilityService.onDidChangeScreenReaderOptimized(() => this.style())); @@ -468,7 +467,7 @@ export class WebviewElement extends Disposable implements IWebview, WebviewFindD return this._send('message', { message, transfer }); } - protected async _send(channel: string, data?: any, transferable: Transferable[] = []): Promise { + private async _send(channel: string, data?: any, transferable: Transferable[] = []): Promise { if (this._state.type === WebviewState.Type.Initializing) { let resolve: (x: boolean) => void; const promise = new Promise(r => resolve = r); @@ -525,7 +524,6 @@ export class WebviewElement extends Disposable implements IWebview, WebviewFindD params.purpose = options.purpose; } - COI.addSearchParam(params, true, true); const queryString = new URLSearchParams(params).toString(); @@ -598,7 +596,7 @@ export class WebviewElement extends Disposable implements IWebview, WebviewFindD return false; } - protected on(channel: WebviewMessageChannels, handler: (data: T, e: MessageEvent) => void): IDisposable { + private on(channel: WebviewMessageChannels, handler: (data: T, e: MessageEvent) => void): IDisposable { let handlers = this._messageHandlers.get(channel); if (!handlers) { handlers = new Set(); @@ -726,7 +724,7 @@ export class WebviewElement extends Disposable implements IWebview, WebviewFindD this._webviewFindWidget?.updateTheme(this.webviewThemeDataProvider.getTheme()); } - private handleFocusChange(isFocused: boolean): void { + protected handleFocusChange(isFocused: boolean): void { this._focused = isFocused; if (isFocused) { this._onDidFocus.fire(); diff --git a/src/vs/workbench/contrib/webview/electron-sandbox/webviewElement.ts b/src/vs/workbench/contrib/webview/electron-sandbox/webviewElement.ts index c119358978c..bdc676cff07 100644 --- a/src/vs/workbench/contrib/webview/electron-sandbox/webviewElement.ts +++ b/src/vs/workbench/contrib/webview/electron-sandbox/webviewElement.ts @@ -23,7 +23,7 @@ import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; import { ITunnelService } from 'vs/platform/tunnel/common/tunnel'; import { FindInFrameOptions, IWebviewManagerService } from 'vs/platform/webview/common/webviewManagerService'; import { WebviewThemeDataProvider } from 'vs/workbench/contrib/webview/browser/themeing'; -import { WebviewElement, WebviewInitInfo, WebviewMessageChannels } from 'vs/workbench/contrib/webview/browser/webviewElement'; +import { WebviewElement, WebviewInitInfo } from 'vs/workbench/contrib/webview/browser/webviewElement'; import { WindowIgnoreMenuShortcutsManager } from 'vs/workbench/contrib/webview/electron-sandbox/windowIgnoreMenuShortcutsManager'; import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService'; @@ -68,14 +68,6 @@ export class ElectronWebviewElement extends WebviewElement { this._webviewMainService = ProxyChannel.toService(mainProcessService.getChannel('webview')); - this._register(this.on(WebviewMessageChannels.didFocus, () => { - this._webviewKeyboardHandler.didFocus(); - })); - - this._register(this.on(WebviewMessageChannels.didBlur, () => { - this._webviewKeyboardHandler.didBlur(); - })); - if (initInfo.options.enableFindWidget) { this._register(this.onDidHtmlChange((newContent) => { if (this._findStarted && this._cachedHtmlContent !== newContent) { @@ -167,4 +159,13 @@ export class ElectronWebviewElement extends WebviewElement { }); this._onDidStopFind.fire(); } + + protected override handleFocusChange(isFocused: boolean): void { + super.handleFocusChange(isFocused); + if (isFocused) { + this._webviewKeyboardHandler.didFocus(); + } else { + this._webviewKeyboardHandler.didBlur(); + } + } } From a8108049ab61b970f2ec1839dfb753054e07395e Mon Sep 17 00:00:00 2001 From: Tyler James Leonhardt Date: Wed, 12 Oct 2022 17:28:34 -0700 Subject: [PATCH 13/13] Allow extensions to use new API and it get included in Language Packs (#163493) * Allow extensions to use new API and it get included in Language Packs This leverages the l10n-dev package to analyze ts files for `l10n.t` calls. * delete console.logs --- build/lib/i18n.js | 38 +++++++++++++++--- build/lib/i18n.ts | 46 ++++++++++++++++++---- build/npm/update-localization-extension.js | 2 +- 3 files changed, 72 insertions(+), 14 deletions(-) diff --git a/build/lib/i18n.js b/build/lib/i18n.js index 2d407ad6f1f..30c75e7c1c3 100644 --- a/build/lib/i18n.js +++ b/build/lib/i18n.js @@ -4,7 +4,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ Object.defineProperty(exports, "__esModule", { value: true }); -exports.prepareIslFiles = exports.prepareI18nPackFiles = exports.createXlfFilesForIsl = exports.createXlfFilesForExtensions = exports.createXlfFilesForCoreBundle = exports.getResource = exports.processNlsFiles = exports.XLF = exports.Line = exports.externalExtensionsWithTranslations = exports.extraLanguages = exports.defaultLanguages = void 0; +exports.prepareIslFiles = exports.prepareI18nPackFiles = exports.createXlfFilesForIsl = exports.createXlfFilesForExtensions = exports.createXlfFilesForCoreBundle = exports.getResource = exports.processNlsFiles = exports.XLF = exports.Line = exports.extraLanguages = exports.defaultLanguages = void 0; const path = require("path"); const fs = require("fs"); const event_stream_1 = require("event-stream"); @@ -37,7 +37,7 @@ exports.extraLanguages = [ { id: 'tr', folderName: 'trk' } ]; // non built-in extensions also that are transifex and need to be part of the language packs -exports.externalExtensionsWithTranslations = { +const externalExtensionsWithTranslations = { 'vscode-chrome-debug': 'msjsdiag.debugger-for-chrome', 'vscode-node-debug': 'ms-vscode.node-debug', 'vscode-node-debug2': 'ms-vscode.node-debug2' @@ -508,6 +508,28 @@ function createXlfFilesForCoreBundle() { }); } exports.createXlfFilesForCoreBundle = createXlfFilesForCoreBundle; +function createL10nBundleForExtension(extensionName) { + const result = (0, event_stream_1.through)(); + gulp.src([ + `extensions/${extensionName}/src/**/*.ts`, + ]).pipe((0, event_stream_1.writeArray)((err, files) => { + if (err) { + result.emit('error', err); + return; + } + const json = (0, l10n_dev_1.getL10nJson)(files.map(file => { + return file.contents.toString('utf8'); + })); + if (Object.keys(json)) { + result.emit('data', new File({ + path: `${extensionName}/bundle.l10n.json`, + contents: Buffer.from(JSON.stringify(json), 'utf8') + })); + } + result.emit('end'); + })); + return result; +} function createXlfFilesForExtensions() { let counter = 0; let folderStreamEnded = false; @@ -530,7 +552,7 @@ function createXlfFilesForExtensions() { } return _l10nMap; } - gulp.src([`.build/extensions/${extensionName}/package.nls.json`, `.build/extensions/${extensionName}/**/nls.metadata.json`], { allowEmpty: true }).pipe((0, event_stream_1.through)(function (file) { + (0, event_stream_1.merge)(gulp.src([`.build/extensions/${extensionName}/package.nls.json`, `.build/extensions/${extensionName}/**/nls.metadata.json`], { allowEmpty: true }), createL10nBundleForExtension(extensionName)).pipe((0, event_stream_1.through)(function (file) { if (file.isBuffer()) { const buffer = file.contents; const basename = path.basename(file.path); @@ -554,6 +576,10 @@ function createXlfFilesForExtensions() { getL10nMap().set(`extensions/${extensionName}/${relPath}/${file}`, info); } } + else if (basename === 'bundle.l10n.json') { + const json = JSON.parse(buffer.toString('utf8')); + getL10nMap().set(`extensions/${extensionName}/bundle`, json); + } else { this.emit('error', new Error(`${file.path} is not a valid extension nls file`)); return; @@ -664,7 +690,7 @@ function getRecordFromL10nJsonFormat(l10nJsonFormat) { } return record; } -function prepareI18nPackFiles(externalExtensions, resultingTranslationPaths) { +function prepareI18nPackFiles(resultingTranslationPaths) { const parsePromises = []; const mainPack = { version: i18nPackVersion, contents: {} }; const extensionsPacks = {}; @@ -685,7 +711,7 @@ function prepareI18nPackFiles(externalExtensions, resultingTranslationPaths) { if (!extPack) { extPack = extensionsPacks[resource] = { version: i18nPackVersion, contents: {} }; } - const externalId = externalExtensions[resource]; + const externalId = externalExtensionsWithTranslations[resource]; if (!externalId) { // internal extension: remove 'extensions/extensionId/' segnent const secondSlash = path.indexOf('/', firstSlash + 1); extPack.contents[path.substring(secondSlash + 1)] = getRecordFromL10nJsonFormat(file.messages); @@ -713,7 +739,7 @@ function prepareI18nPackFiles(externalExtensions, resultingTranslationPaths) { for (const extension in extensionsPacks) { const translatedExtFile = createI18nFile(`extensions/${extension}`, extensionsPacks[extension]); this.queue(translatedExtFile); - const externalExtensionId = externalExtensions[extension]; + const externalExtensionId = externalExtensionsWithTranslations[extension]; if (externalExtensionId) { resultingTranslationPaths.push({ id: externalExtensionId, resourceName: `extensions/${extension}.i18n.json` }); } diff --git a/build/lib/i18n.ts b/build/lib/i18n.ts index 0045bb64fcf..f5510eda4d7 100644 --- a/build/lib/i18n.ts +++ b/build/lib/i18n.ts @@ -6,7 +6,7 @@ import * as path from 'path'; import * as fs from 'fs'; -import { through, ThroughStream } from 'event-stream'; +import { merge, through, ThroughStream, writeArray } from 'event-stream'; import * as File from 'vinyl'; import * as Is from 'is'; import * as xml2js from 'xml2js'; @@ -14,7 +14,7 @@ import * as gulp from 'gulp'; import * as fancyLog from 'fancy-log'; import * as ansiColors from 'ansi-colors'; import * as iconv from '@vscode/iconv-lite-umd'; -import { l10nJsonFormat, getL10nXlf, l10nJsonDetails, getL10nFilesFromXlf } from '@vscode/l10n-dev'; +import { l10nJsonFormat, getL10nXlf, l10nJsonDetails, getL10nFilesFromXlf, getL10nJson } from '@vscode/l10n-dev'; function log(message: any, ...rest: any[]): void { fancyLog(ansiColors.green('[i18n]'), message, ...rest); @@ -50,7 +50,7 @@ export const extraLanguages: Language[] = [ ]; // non built-in extensions also that are transifex and need to be part of the language packs -export const externalExtensionsWithTranslations = { +const externalExtensionsWithTranslations: Record = { 'vscode-chrome-debug': 'msjsdiag.debugger-for-chrome', 'vscode-node-debug': 'ms-vscode.node-debug', 'vscode-node-debug2': 'ms-vscode.node-debug2' @@ -586,6 +586,32 @@ export function createXlfFilesForCoreBundle(): ThroughStream { }); } +function createL10nBundleForExtension(extensionName: string): ThroughStream { + const result = through(); + gulp.src([ + `extensions/${extensionName}/src/**/*.ts`, + ]).pipe(writeArray((err, files: File[]) => { + if (err) { + result.emit('error', err); + return; + } + + const json = getL10nJson(files.map(file => { + return file.contents.toString('utf8'); + })); + + if (Object.keys(json)) { + result.emit('data', new File({ + path: `${extensionName}/bundle.l10n.json`, + contents: Buffer.from(JSON.stringify(json), 'utf8') + })); + } + result.emit('end'); + })); + + return result; +} + export function createXlfFilesForExtensions(): ThroughStream { let counter: number = 0; let folderStreamEnded: boolean = false; @@ -608,7 +634,10 @@ export function createXlfFilesForExtensions(): ThroughStream { } return _l10nMap; } - gulp.src([`.build/extensions/${extensionName}/package.nls.json`, `.build/extensions/${extensionName}/**/nls.metadata.json`], { allowEmpty: true }).pipe(through(function (file: File) { + merge( + gulp.src([`.build/extensions/${extensionName}/package.nls.json`, `.build/extensions/${extensionName}/**/nls.metadata.json`], { allowEmpty: true }), + createL10nBundleForExtension(extensionName) + ).pipe(through(function (file: File) { if (file.isBuffer()) { const buffer: Buffer = file.contents as Buffer; const basename = path.basename(file.path); @@ -631,6 +660,9 @@ export function createXlfFilesForExtensions(): ThroughStream { } getL10nMap().set(`extensions/${extensionName}/${relPath}/${file}`, info); } + } else if (basename === 'bundle.l10n.json') { + const json: l10nJsonFormat = JSON.parse(buffer.toString('utf8')); + getL10nMap().set(`extensions/${extensionName}/bundle`, json); } else { this.emit('error', new Error(`${file.path} is not a valid extension nls file`)); return; @@ -762,7 +794,7 @@ function getRecordFromL10nJsonFormat(l10nJsonFormat: l10nJsonFormat): Record, resultingTranslationPaths: TranslationPath[]): NodeJS.ReadWriteStream { +export function prepareI18nPackFiles(resultingTranslationPaths: TranslationPath[]): NodeJS.ReadWriteStream { const parsePromises: Promise[] = []; const mainPack: I18nPack = { version: i18nPackVersion, contents: {} }; const extensionsPacks: Record = {}; @@ -785,7 +817,7 @@ export function prepareI18nPackFiles(externalExtensions: Record, if (!extPack) { extPack = extensionsPacks[resource] = { version: i18nPackVersion, contents: {} }; } - const externalId = externalExtensions[resource]; + const externalId = externalExtensionsWithTranslations[resource]; if (!externalId) { // internal extension: remove 'extensions/extensionId/' segnent const secondSlash = path.indexOf('/', firstSlash + 1); extPack.contents[path.substring(secondSlash + 1)] = getRecordFromL10nJsonFormat(file.messages); @@ -814,7 +846,7 @@ export function prepareI18nPackFiles(externalExtensions: Record, const translatedExtFile = createI18nFile(`extensions/${extension}`, extensionsPacks[extension]); this.queue(translatedExtFile); - const externalExtensionId = externalExtensions[extension]; + const externalExtensionId = externalExtensionsWithTranslations[extension]; if (externalExtensionId) { resultingTranslationPaths.push({ id: externalExtensionId, resourceName: `extensions/${extension}.i18n.json` }); } else { diff --git a/build/npm/update-localization-extension.js b/build/npm/update-localization-extension.js index 9876882a6c4..b78674a7099 100644 --- a/build/npm/update-localization-extension.js +++ b/build/npm/update-localization-extension.js @@ -68,7 +68,7 @@ function update(options) { console.log(`Importing translations for ${languageId} form '${location}' to '${translationDataFolder}' ...`); let translationPaths = []; gulp.src(path.join(location, '**', languageId, '*.xlf'), { silent: false }) - .pipe(i18n.prepareI18nPackFiles(i18n.externalExtensionsWithTranslations, translationPaths)) + .pipe(i18n.prepareI18nPackFiles(translationPaths)) .on('error', (error) => { console.log(`Error occurred while importing translations:`); translationPaths = undefined;