From f137f258d865276ffb34994ad733faaa9fc4dcde Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 18 Aug 2023 05:14:14 -0700 Subject: [PATCH 01/34] Fix typo plural -> singular --- .../contrib/accessibility/browser/accessibilityConfiguration.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/accessibility/browser/accessibilityConfiguration.ts b/src/vs/workbench/contrib/accessibility/browser/accessibilityConfiguration.ts index 91f562646d6..0a7043a5cd8 100644 --- a/src/vs/workbench/contrib/accessibility/browser/accessibilityConfiguration.ts +++ b/src/vs/workbench/contrib/accessibility/browser/accessibilityConfiguration.ts @@ -84,7 +84,7 @@ const configuration: IConfigurationNode = { ...baseProperty }, [AccessibilitySettingId.UnfocusedViewOpacity]: { - description: localize('unfocusedViewOpacity', 'The opacity fraction (0.2 to 1.0) to use for unfocused editors and terminals. This will dim inactive views to make the focused views more obvious.'), + description: localize('unfocusedViewOpacity', 'The opacity fraction (0.2 to 1.0) to use for unfocused editors and terminals. This will dim inactive views to make the focused view more obvious.'), type: 'number', minimum: 0.2, maximum: 1, From 6336ee75304eaac32897d8e74c451863d877cb28 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Fri, 18 Aug 2023 08:04:19 -0700 Subject: [PATCH 02/34] eng: cleanup some leaks around around editors and workbenchInstantiationService (#190623) * inline chat: fix leaks found in testing Fixes some leaks I found while looking at https://github.com/microsoft/vscode/pull/190444, by adding ensureNoDisposablesAreLeakedInTestSuite to the suite. I don't have a dev setup for inline chat, so I have not tested this beyond running tests and verifying the fix * eng: cleanup some leaks around around editors and workbenchInstantiationService I added a disposable leak tracker to a test that used `workbenchInstantiationService`. This fixes the baseline leaks and some extra leaks with that function. * fix build * rm too-eager leak checker * remove forgotten busy loop --- src/vs/base/browser/ui/grid/gridview.ts | 1 + src/vs/base/browser/ui/splitview/splitview.ts | 4 +-- src/vs/base/browser/ui/toolbar/toolbar.ts | 1 + src/vs/base/common/stream.ts | 31 ++++++++++++++----- src/vs/base/test/common/stream.test.ts | 8 +++-- .../diffEditorWidget2/diffEditorEditors.ts | 4 +-- src/vs/editor/common/model/textModel.ts | 4 ++- .../suggest/browser/suggestController.ts | 2 +- src/vs/platform/actions/browser/buttonbar.ts | 2 +- .../platform/checksum/node/checksumService.ts | 12 +++++-- src/vs/platform/files/common/fileService.ts | 12 +++++-- .../node/diskFileSystemProviderServer.ts | 11 ++++--- src/vs/workbench/browser/dnd.ts | 6 ++-- .../browser/parts/editor/editorGroupView.ts | 2 +- .../parts/editor/editorGroupWatermark.ts | 2 +- .../browser/parts/editor/tabsTitleControl.ts | 10 +++--- .../common/editor/editorGroupModel.ts | 2 +- .../browser/editors/textFileEditorTracker.ts | 2 +- .../browser/textFileEditorTracker.test.ts | 3 ++ .../browser/inlineChatController.ts | 3 +- .../inlineChat/browser/inlineChatSession.ts | 2 ++ .../inlineChat/browser/inlineChatWidget.ts | 8 ++--- .../test/browser/inlineChatController.test.ts | 26 ++++------------ .../markers/test/browser/markersModel.test.ts | 3 ++ .../editor/browser/codeEditorService.ts | 4 +-- .../services/editor/browser/editorService.ts | 6 ++-- .../browser/browserTextFileService.ts | 2 +- .../textfile/browser/textFileService.ts | 7 +++-- .../common/textFileSaveParticipant.ts | 5 ++- .../test/browser/textFileEditorModel.test.ts | 4 +-- .../storedFileWorkingCopySaveParticipant.ts | 1 + .../workingCopyFileOperationParticipant.ts | 1 + .../test/browser/workbenchTestServices.ts | 13 ++++---- 33 files changed, 124 insertions(+), 80 deletions(-) diff --git a/src/vs/base/browser/ui/grid/gridview.ts b/src/vs/base/browser/ui/grid/gridview.ts index c89c6a7a063..9445c64286a 100644 --- a/src/vs/base/browser/ui/grid/gridview.ts +++ b/src/vs/base/browser/ui/grid/gridview.ts @@ -705,6 +705,7 @@ class BranchNode implements ISplitView, IDisposable { this.splitviewSashResetDisposable.dispose(); this.childrenSashResetDisposable.dispose(); this.childrenChangeDisposable.dispose(); + this.onDidScrollDisposable.dispose(); this.splitview.dispose(); } } diff --git a/src/vs/base/browser/ui/splitview/splitview.ts b/src/vs/base/browser/ui/splitview/splitview.ts index b822db43751..28f18d42537 100644 --- a/src/vs/base/browser/ui/splitview/splitview.ts +++ b/src/vs/base/browser/ui/splitview/splitview.ts @@ -565,11 +565,11 @@ export class SplitView extends Disposable { this.sashContainer = append(this.el, $('.sash-container')); this.viewContainer = $('.split-view-container'); - this.scrollable = new Scrollable({ + this.scrollable = this._register(new Scrollable({ forceIntegerValues: true, smoothScrollDuration: 125, scheduleAtNextAnimationFrame - }); + })); this.scrollableElement = this._register(new SmoothScrollableElement(this.viewContainer, { vertical: this.orientation === Orientation.VERTICAL ? (options.scrollbarVisibility ?? ScrollbarVisibility.Auto) : ScrollbarVisibility.Hidden, horizontal: this.orientation === Orientation.HORIZONTAL ? (options.scrollbarVisibility ?? ScrollbarVisibility.Auto) : ScrollbarVisibility.Hidden diff --git a/src/vs/base/browser/ui/toolbar/toolbar.ts b/src/vs/base/browser/ui/toolbar/toolbar.ts index 1a6089b5958..67a2401ecfe 100644 --- a/src/vs/base/browser/ui/toolbar/toolbar.ts +++ b/src/vs/base/browser/ui/toolbar/toolbar.ts @@ -212,6 +212,7 @@ export class ToolBar extends Disposable { override dispose(): void { this.clear(); + this.disposables.dispose(); super.dispose(); } } diff --git a/src/vs/base/common/stream.ts b/src/vs/base/common/stream.ts index 9f1039c0e52..dca56d44d38 100644 --- a/src/vs/base/common/stream.ts +++ b/src/vs/base/common/stream.ts @@ -519,13 +519,14 @@ export function consumeStream(stream: ReadableStreamEvents, reducer return new Promise((resolve, reject) => { const chunks: T[] = []; - listenStream(stream, { + const l = listenStream(stream, { onData: chunk => { if (reducer) { chunks.push(chunk); } }, onError: error => { + l.dispose(); if (reducer) { reject(error); } else { @@ -533,6 +534,7 @@ export function consumeStream(stream: ReadableStreamEvents, reducer } }, onEnd: () => { + l.dispose(); if (reducer) { resolve(reducer(chunks)); } else { @@ -570,15 +572,18 @@ export interface IStreamListener { export function listenStream(stream: ReadableStreamEvents, listener: IStreamListener): IDisposable { let destroyed = false; + // error and end events are in the next microtask so that a stream that is + // closed synchronously (e.g from a memory `toStream`) can get its disposable + // and destroy the stream. stream.on('error', error => { if (!destroyed) { - listener.onError(error); + queueMicrotask(() => listener.onError(error)); } }); stream.on('end', () => { if (!destroyed) { - listener.onEnd(); + queueMicrotask(() => listener.onEnd()); } }); @@ -692,10 +697,16 @@ export function toReadable(t: T): Readable { export function transform(stream: ReadableStreamEvents, transformer: ITransformer, reducer: IReducer): ReadableStream { const target = newWriteableStream(reducer); - listenStream(stream, { + const l = listenStream(stream, { onData: data => target.write(transformer.data(data)), - onError: error => target.error(transformer.error ? transformer.error(error) : error), - onEnd: () => target.end() + onError: error => { + l.dispose(); + target.error(transformer.error ? transformer.error(error) : error); + }, + onEnd: () => { + l.dispose(); + target.end(); + } }); return target; @@ -740,7 +751,7 @@ export function prefixedStream(prefix: T, stream: ReadableStream, reducer: const target = newWriteableStream(reducer); - listenStream(stream, { + const l = listenStream(stream, { onData: data => { // Handle prefix only once @@ -752,8 +763,12 @@ export function prefixedStream(prefix: T, stream: ReadableStream, reducer: return target.write(data); }, - onError: error => target.error(error), + onError: error => { + l.dispose(); + target.error(error); + }, onEnd: () => { + l.dispose(); // Handle prefix only once if (!prefixHandled) { diff --git a/src/vs/base/test/common/stream.test.ts b/src/vs/base/test/common/stream.test.ts index 78c38d691a4..fe69e603827 100644 --- a/src/vs/base/test/common/stream.test.ts +++ b/src/vs/base/test/common/stream.test.ts @@ -315,14 +315,14 @@ suite('Stream', () => { assert.strictEqual(consumed, undefined); }); - test('listenStream', () => { + test('listenStream', async () => { const stream = newWriteableStream(strings => strings.join()); let error = false; let end = false; let data = ''; - listenStream(stream, { + const l = listenStream(stream, { onData: d => { data = d; }, @@ -345,10 +345,14 @@ suite('Stream', () => { assert.strictEqual(end, false); stream.error(new Error()); + await new Promise(r => queueMicrotask(r)); assert.strictEqual(error, true); stream.end('Final Bit'); + await new Promise(r => queueMicrotask(r)); assert.strictEqual(end, true); + + l.dispose(); }); test('listenStream - dispose', () => { diff --git a/src/vs/editor/browser/widget/diffEditorWidget2/diffEditorEditors.ts b/src/vs/editor/browser/widget/diffEditorWidget2/diffEditorEditors.ts index cce99a94271..ee764c81aed 100644 --- a/src/vs/editor/browser/widget/diffEditorWidget2/diffEditorEditors.ts +++ b/src/vs/editor/browser/widget/diffEditorWidget2/diffEditorEditors.ts @@ -35,8 +35,8 @@ export class DiffEditorEditors extends Disposable { ) { super(); - this.original = this._createLeftHandSideEditor(_options.editorOptions.get(), codeEditorWidgetOptions.originalEditor || {}); - this.modified = this._createRightHandSideEditor(_options.editorOptions.get(), codeEditorWidgetOptions.modifiedEditor || {}); + this.original = this._register(this._createLeftHandSideEditor(_options.editorOptions.get(), codeEditorWidgetOptions.originalEditor || {})); + this.modified = this._register(this._createRightHandSideEditor(_options.editorOptions.get(), codeEditorWidgetOptions.modifiedEditor || {})); this._register(autorunHandleChanges({ createEmptyChangeSummary: () => ({} as IDiffEditorConstructionOptions), diff --git a/src/vs/editor/common/model/textModel.ts b/src/vs/editor/common/model/textModel.ts index b600bd7e3bb..409216fb9d9 100644 --- a/src/vs/editor/common/model/textModel.ts +++ b/src/vs/editor/common/model/textModel.ts @@ -67,19 +67,21 @@ export function createTextBufferFactoryFromStream(stream: ITextStream | VSBuffer let done = false; - listenStream(stream, { + const l = listenStream(stream, { onData: chunk => { builder.acceptChunk((typeof chunk === 'string') ? chunk : chunk.toString()); }, onError: error => { if (!done) { done = true; + l.dispose(); reject(error); } }, onEnd: () => { if (!done) { done = true; + l.dispose(); resolve(builder.finish()); } } diff --git a/src/vs/editor/contrib/suggest/browser/suggestController.ts b/src/vs/editor/contrib/suggest/browser/suggestController.ts index 887fb23cb2d..6450e989bce 100644 --- a/src/vs/editor/contrib/suggest/browser/suggestController.ts +++ b/src/vs/editor/contrib/suggest/browser/suggestController.ts @@ -143,7 +143,7 @@ export class SuggestController implements IEditorContribution { // context key: update insert/replace mode const ctxInsertMode = SuggestContext.InsertMode.bindTo(_contextKeyService); ctxInsertMode.set(editor.getOption(EditorOption.suggest).insertMode); - this.model.onDidTrigger(() => ctxInsertMode.set(editor.getOption(EditorOption.suggest).insertMode)); + this._toDispose.add(this.model.onDidTrigger(() => ctxInsertMode.set(editor.getOption(EditorOption.suggest).insertMode))); this.widget = this._toDispose.add(new IdleValue(() => { diff --git a/src/vs/platform/actions/browser/buttonbar.ts b/src/vs/platform/actions/browser/buttonbar.ts index cb6ab4c31df..4965d6bb5cd 100644 --- a/src/vs/platform/actions/browser/buttonbar.ts +++ b/src/vs/platform/actions/browser/buttonbar.ts @@ -55,7 +55,7 @@ export class MenuWorkbenchButtonBar extends ButtonBar { 'workbenchActionExecuted', { id: e.action.id, from: options.telemetrySource! } ); - }, this._store); + }, undefined, this._store); } const conifgProvider: IButtonConfigProvider = options?.buttonConfigProvider ?? (() => ({ showLabel: true })); diff --git a/src/vs/platform/checksum/node/checksumService.ts b/src/vs/platform/checksum/node/checksumService.ts index e4214019ff1..0554a503960 100644 --- a/src/vs/platform/checksum/node/checksumService.ts +++ b/src/vs/platform/checksum/node/checksumService.ts @@ -20,10 +20,16 @@ export class ChecksumService implements IChecksumService { return new Promise((resolve, reject) => { const hash = createHash('md5'); - listenStream(stream, { + const l = listenStream(stream, { onData: data => hash.update(data.buffer), - onError: error => reject(error), - onEnd: () => resolve(hash.digest('base64').replace(/=+$/, '')) + onError: error => { + l.dispose(); + reject(error); + }, + onEnd: () => { + l.dispose(); + resolve(hash.digest('base64').replace(/=+$/, '')); + } }); }); } diff --git a/src/vs/platform/files/common/fileService.ts b/src/vs/platform/files/common/fileService.ts index e88ff723b8b..dc22a8203e1 100644 --- a/src/vs/platform/files/common/fileService.ts +++ b/src/vs/platform/files/common/fileService.ts @@ -1228,7 +1228,7 @@ export class FileService extends Disposable implements IFileService { } return new Promise((resolve, reject) => { - listenStream(stream, { + const l = listenStream(stream, { onData: async chunk => { // pause stream to perform async write operation @@ -1248,8 +1248,14 @@ export class FileService extends Disposable implements IFileService { // handler again before finishing. setTimeout(() => stream.resume()); }, - onError: error => reject(error), - onEnd: () => resolve() + onError: error => { + l.dispose(); + reject(error); + }, + onEnd: () => { + l.dispose(); + resolve(); + } }); }); } diff --git a/src/vs/platform/files/node/diskFileSystemProviderServer.ts b/src/vs/platform/files/node/diskFileSystemProviderServer.ts index b7e81ab4491..989a714094b 100644 --- a/src/vs/platform/files/node/diskFileSystemProviderServer.ts +++ b/src/vs/platform/files/node/diskFileSystemProviderServer.ts @@ -106,16 +106,19 @@ export abstract class AbstractDiskFileSystemProviderChannel extends Disposabl // Ensure to cancel the read operation when there is no more // listener on the other side to prevent unneeded work. - cts.cancel(); + cts.dispose(true); } }); const fileStream = this.provider.readFileStream(resource, opts, cts.token); - listenStream(fileStream, { + const l = listenStream(fileStream, { onData: chunk => emitter.fire(VSBuffer.wrap(chunk)), - onError: error => emitter.fire(error), + onError: error => { + l.dispose(); + emitter.fire(error); + }, onEnd: () => { - + l.dispose(); // Forward event emitter.fire('end'); diff --git a/src/vs/workbench/browser/dnd.ts b/src/vs/workbench/browser/dnd.ts index c196ac2132e..a025892f899 100644 --- a/src/vs/workbench/browser/dnd.ts +++ b/src/vs/workbench/browser/dnd.ts @@ -12,7 +12,7 @@ import { ITreeDragOverReaction } from 'vs/base/browser/ui/tree/tree'; import { coalesce } from 'vs/base/common/arrays'; import { UriList, VSDataTransfer } from 'vs/base/common/dataTransfer'; import { Emitter } from 'vs/base/common/event'; -import { Disposable, DisposableStore, IDisposable } from 'vs/base/common/lifecycle'; +import { Disposable, DisposableStore, IDisposable, markAsSingleton } from 'vs/base/common/lifecycle'; import { stringify } from 'vs/base/common/marshalling'; import { Mimes } from 'vs/base/common/mime'; import { FileAccess, Schemas } from 'vs/base/common/network'; @@ -427,8 +427,10 @@ export class CompositeDragAndDropObserver extends Disposable { static get INSTANCE(): CompositeDragAndDropObserver { if (!CompositeDragAndDropObserver.instance) { CompositeDragAndDropObserver.instance = new CompositeDragAndDropObserver(); + markAsSingleton(CompositeDragAndDropObserver.instance); } + return CompositeDragAndDropObserver.instance; } @@ -523,7 +525,7 @@ export class CompositeDragAndDropObserver extends Disposable { if (callbacks.onDragEnd) { this.onDragEnd.event(e => { callbacks.onDragEnd!(e); - }); + }, this, disposableStore); } return this._register(disposableStore); diff --git a/src/vs/workbench/browser/parts/editor/editorGroupView.ts b/src/vs/workbench/browser/parts/editor/editorGroupView.ts index c80fa632d3e..01b92a4b7e2 100644 --- a/src/vs/workbench/browser/parts/editor/editorGroupView.ts +++ b/src/vs/workbench/browser/parts/editor/editorGroupView.ts @@ -244,7 +244,7 @@ export class EditorGroupView extends Themable implements IEditorGroupView { const groupEditorsCountContext = EditorGroupEditorsCountContext.bindTo(this.scopedContextKeyService); const groupLockedContext = ActiveEditorGroupLockedContext.bindTo(this.scopedContextKeyService); - const activeEditorListener = new MutableDisposable(); + const activeEditorListener = this._register(new MutableDisposable()); const observeActiveEditor = () => { activeEditorListener.clear(); diff --git a/src/vs/workbench/browser/parts/editor/editorGroupWatermark.ts b/src/vs/workbench/browser/parts/editor/editorGroupWatermark.ts index 55d6506abab..ff8a2778c7c 100644 --- a/src/vs/workbench/browser/parts/editor/editorGroupWatermark.ts +++ b/src/vs/workbench/browser/parts/editor/editorGroupWatermark.ts @@ -90,7 +90,7 @@ export class EditorGroupWatermark extends Disposable { } private registerListeners(): void { - this.lifecycleService.onDidShutdown(() => this.dispose()); + this._register(this.lifecycleService.onDidShutdown(() => this.dispose())); this._register(this.configurationService.onDidChangeConfiguration(e => { if (e.affectsConfiguration('workbench.tips.enabled')) { diff --git a/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts b/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts index d712b1a146b..3272b3e4839 100644 --- a/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts +++ b/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts @@ -184,7 +184,7 @@ export class TabsTitleControl extends TitleControl { this.updateTabSizing(false); // Tabs Scrollbar - this.tabsScrollbar = this._register(this.createTabsScrollbar(this.tabsContainer)); + this.tabsScrollbar = this.createTabsScrollbar(this.tabsContainer); this.tabsAndActionsContainer.appendChild(this.tabsScrollbar.getDomNode()); // Tabs Container listeners @@ -206,19 +206,19 @@ export class TabsTitleControl extends TitleControl { } private createTabsScrollbar(scrollable: HTMLElement): ScrollableElement { - const tabsScrollbar = new ScrollableElement(scrollable, { + const tabsScrollbar = this._register(new ScrollableElement(scrollable, { horizontal: ScrollbarVisibility.Auto, horizontalScrollbarSize: this.getTabsScrollbarSizing(), vertical: ScrollbarVisibility.Hidden, scrollYToX: true, useShadows: false - }); + })); - tabsScrollbar.onScroll(e => { + this._register(tabsScrollbar.onScroll(e => { if (e.scrollLeftChanged) { scrollable.scrollLeft = e.scrollLeft; } - }); + })); return tabsScrollbar; } diff --git a/src/vs/workbench/common/editor/editorGroupModel.ts b/src/vs/workbench/common/editor/editorGroupModel.ts index 7fc1ae6cbc0..0a31b47bcf7 100644 --- a/src/vs/workbench/common/editor/editorGroupModel.ts +++ b/src/vs/workbench/common/editor/editorGroupModel.ts @@ -404,7 +404,7 @@ export class EditorGroupModel extends Disposable { } private registerEditorListeners(editor: EditorInput): void { - const listeners = new DisposableStore(); + const listeners = this._register(new DisposableStore()); // Re-emit disposal of editor input as our own event listeners.add(Event.once(editor.onWillDispose)(() => { diff --git a/src/vs/workbench/contrib/files/browser/editors/textFileEditorTracker.ts b/src/vs/workbench/contrib/files/browser/editors/textFileEditorTracker.ts index fb876ba670a..afa94056cd5 100644 --- a/src/vs/workbench/contrib/files/browser/editors/textFileEditorTracker.ts +++ b/src/vs/workbench/contrib/files/browser/editors/textFileEditorTracker.ts @@ -47,7 +47,7 @@ export class TextFileEditorTracker extends Disposable implements IWorkbenchContr this._register(this.hostService.onDidChangeFocus(hasFocus => hasFocus ? this.reloadVisibleTextFileEditors() : undefined)); // Lifecycle - this.lifecycleService.onDidShutdown(() => this.dispose()); + this._register(this.lifecycleService.onDidShutdown(() => this.dispose())); } //#region Text File: Ensure every dirty text and untitled file is opened in an editor diff --git a/src/vs/workbench/contrib/files/test/browser/textFileEditorTracker.test.ts b/src/vs/workbench/contrib/files/test/browser/textFileEditorTracker.test.ts index bc69b29a381..b0afe7cc554 100644 --- a/src/vs/workbench/contrib/files/test/browser/textFileEditorTracker.test.ts +++ b/src/vs/workbench/contrib/files/test/browser/textFileEditorTracker.test.ts @@ -77,6 +77,7 @@ suite('Files - TextFileEditorTracker', () => { instantiationService.stub(IWorkspaceTrustRequestService, new TestWorkspaceTrustRequestService(false)); const editorService: EditorService = instantiationService.createInstance(EditorService); + disposables.add(editorService); instantiationService.stub(IEditorService, editorService); const accessor = instantiationService.createInstance(TestServiceAccessor); @@ -93,6 +94,7 @@ suite('Files - TextFileEditorTracker', () => { const resource = toResource.call(this, '/path/index.txt'); const model = await accessor.textFileService.files.resolve(resource) as IResolvedTextFileEditorModel; + disposables.add(model); model.textEditorModel.setValue('Super Good'); assert.strictEqual(snapshotToString(model.createSnapshot()!), 'Super Good'); @@ -141,6 +143,7 @@ suite('Files - TextFileEditorTracker', () => { } const model = await accessor.textFileService.files.resolve(resource) as IResolvedTextFileEditorModel; + disposables.add(model); model.textEditorModel.setValue('Super Good'); diff --git a/src/vs/workbench/contrib/inlineChat/browser/inlineChatController.ts b/src/vs/workbench/contrib/inlineChat/browser/inlineChatController.ts index 104830cc852..f91de7e3ee8 100644 --- a/src/vs/workbench/contrib/inlineChat/browser/inlineChatController.ts +++ b/src/vs/workbench/contrib/inlineChat/browser/inlineChatController.ts @@ -100,7 +100,7 @@ export class InlineChatController implements IEditorContribution { private _messages = this._store.add(new Emitter()); - private readonly _sessionStore: DisposableStore = new DisposableStore(); + private readonly _sessionStore: DisposableStore = this._store.add(new DisposableStore()); private readonly _stashedSession: MutableDisposable = this._store.add(new MutableDisposable()); private _activeSession?: Session; private _strategy?: EditModeStrategy; @@ -146,6 +146,7 @@ export class InlineChatController implements IEditorContribution { } dispose(): void { + this._strategy?.dispose(); this._stashedSession.clear(); this.finishExistingSession(); this._store.dispose(); diff --git a/src/vs/workbench/contrib/inlineChat/browser/inlineChatSession.ts b/src/vs/workbench/contrib/inlineChat/browser/inlineChatSession.ts index 434aea1f4d2..fcbb998dc79 100644 --- a/src/vs/workbench/contrib/inlineChat/browser/inlineChatSession.ts +++ b/src/vs/workbench/contrib/inlineChat/browser/inlineChatSession.ts @@ -385,6 +385,8 @@ export interface IInlineChatSessionService { // recordings(): readonly Recording[]; + + dispose(): void; } type SessionData = { diff --git a/src/vs/workbench/contrib/inlineChat/browser/inlineChatWidget.ts b/src/vs/workbench/contrib/inlineChat/browser/inlineChatWidget.ts index bf16c0aec05..8619088ce96 100644 --- a/src/vs/workbench/contrib/inlineChat/browser/inlineChatWidget.ts +++ b/src/vs/workbench/contrib/inlineChat/browser/inlineChatWidget.ts @@ -230,7 +230,7 @@ export class InlineChatWidget { })); const uri = URI.from({ scheme: 'vscode', authority: 'inline-chat', path: `/inline-chat/model${InlineChatWidget._modelPool++}.txt` }); - this._inputModel = this._modelService.getModel(uri) ?? this._modelService.createModel('', null, uri); + this._inputModel = this._store.add(this._modelService.getModel(uri) ?? this._modelService.createModel('', null, uri)); this._inputEditor.setModel(this._inputModel); // --- context keys @@ -359,13 +359,13 @@ export class InlineChatWidget { this._store.add(feedbackToolbar); // preview editors - this._previewDiffEditor = new IdleValue(() => this._store.add(_instantiationService.createInstance(EmbeddedDiffEditorWidget2, this._elements.previewDiff, { + this._previewDiffEditor = this._store.add(new IdleValue(() => this._store.add(_instantiationService.createInstance(EmbeddedDiffEditorWidget2, this._elements.previewDiff, { ..._previewEditorEditorOptions, onlyShowAccessibleDiffViewer: this._accessibilityService.isScreenReaderOptimized(), - }, { modifiedEditor: codeEditorWidgetOptions, originalEditor: codeEditorWidgetOptions }, parentEditor))); + }, { modifiedEditor: codeEditorWidgetOptions, originalEditor: codeEditorWidgetOptions }, parentEditor)))); this._previewCreateTitle = this._store.add(_instantiationService.createInstance(ResourceLabel, this._elements.previewCreateTitle, { supportIcons: true })); - this._previewCreateEditor = new IdleValue(() => this._store.add(_instantiationService.createInstance(EmbeddedCodeEditorWidget, this._elements.previewCreate, _previewEditorEditorOptions, codeEditorWidgetOptions, parentEditor))); + this._previewCreateEditor = this._store.add(new IdleValue(() => this._store.add(_instantiationService.createInstance(EmbeddedCodeEditorWidget, this._elements.previewCreate, _previewEditorEditorOptions, codeEditorWidgetOptions, parentEditor)))); this._elements.message.tabIndex = 0; this._elements.message.ariaLabel = this._accessibleViewService.getOpenAriaHint(AccessibilityVerbositySettingId.InlineChat); diff --git a/src/vs/workbench/contrib/inlineChat/test/browser/inlineChatController.test.ts b/src/vs/workbench/contrib/inlineChat/test/browser/inlineChatController.test.ts index ab5e4b27e1b..7a9beea7519 100644 --- a/src/vs/workbench/contrib/inlineChat/test/browser/inlineChatController.test.ts +++ b/src/vs/workbench/contrib/inlineChat/test/browser/inlineChatController.test.ts @@ -24,7 +24,6 @@ import { IEditorProgressService, IProgressRunner } from 'vs/platform/progress/co import { mock } from 'vs/base/test/common/mock'; import { Emitter, Event } from 'vs/base/common/event'; import { equals } from 'vs/base/common/arrays'; -import { timeout } from 'vs/base/common/async'; import { IChatAccessibilityService } from 'vs/workbench/contrib/chat/browser/chat'; import { IChatResponseViewModel } from 'vs/workbench/contrib/chat/common/chatViewModel'; import { IAccessibleViewService } from 'vs/workbench/contrib/accessibility/browser/accessibleView'; @@ -114,11 +113,11 @@ suite('InteractiveChatController', function () { }] ); - instaService = workbenchInstantiationService(undefined, store).createChild(serviceCollection); - inlineChatSessionService = instaService.get(IInlineChatSessionService); + instaService = store.add(workbenchInstantiationService(undefined, store).createChild(serviceCollection)); + inlineChatSessionService = store.add(instaService.get(IInlineChatSessionService)); - model = instaService.get(IModelService).createModel('Hello\nWorld\nHello Again\nHello World\n', null); - editor = instantiateTestCodeEditor(instaService, model); + model = store.add(instaService.get(IModelService).createModel('Hello\nWorld\nHello Again\nHello World\n', null)); + editor = store.add(instantiateTestCodeEditor(instaService, model)); store.add(inlineChatService.addProvider({ debugName: 'Unit Test', @@ -142,8 +141,6 @@ suite('InteractiveChatController', function () { }); teardown(function () { - editor.dispose(); - model.dispose(); store.clear(); ctrl?.dispose(); }); @@ -295,19 +292,8 @@ suite('InteractiveChatController', function () { wholeRange: new Range(3, 1, 3, 3) }; }, - async provideResponse(session, request) { - - // SLOW response - await timeout(50000); - - return { - type: InlineChatResponseType.EditorEdit, - id: Math.random(), - edits: [{ - range: new Range(1, 1, 1, 1), // EDIT happens outside of whole range - text: `${request.prompt}\n${request.prompt}` - }] - }; + provideResponse(session, request) { + return new Promise(() => { }); } }); store.add(d); diff --git a/src/vs/workbench/contrib/markers/test/browser/markersModel.test.ts b/src/vs/workbench/contrib/markers/test/browser/markersModel.test.ts index 9cb2c9dc650..b8334c948d6 100644 --- a/src/vs/workbench/contrib/markers/test/browser/markersModel.test.ts +++ b/src/vs/workbench/contrib/markers/test/browser/markersModel.test.ts @@ -8,6 +8,7 @@ import { URI } from 'vs/base/common/uri'; import { IMarker, MarkerSeverity, IRelatedInformation } from 'vs/platform/markers/common/markers'; import { MarkersModel, Marker, ResourceMarkers, RelatedInformation } from 'vs/workbench/contrib/markers/browser/markersModel'; import { groupBy } from 'vs/base/common/collections'; +import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils'; class TestMarkersModel extends MarkersModel { @@ -27,6 +28,8 @@ class TestMarkersModel extends MarkersModel { suite('MarkersModel Test', () => { + ensureNoDisposablesAreLeakedInTestSuite(); + test('marker ids are unique', function () { const marker1 = anErrorWithRange(3); const marker2 = anErrorWithRange(3); diff --git a/src/vs/workbench/services/editor/browser/codeEditorService.ts b/src/vs/workbench/services/editor/browser/codeEditorService.ts index 657f203312c..9930f85f58c 100644 --- a/src/vs/workbench/services/editor/browser/codeEditorService.ts +++ b/src/vs/workbench/services/editor/browser/codeEditorService.ts @@ -25,8 +25,8 @@ export class CodeEditorService extends AbstractCodeEditorService { ) { super(themeService); - this.registerCodeEditorOpenHandler(this.doOpenCodeEditor.bind(this)); - this.registerCodeEditorOpenHandler(this.doOpenCodeEditorFromDiff.bind(this)); + this._register(this.registerCodeEditorOpenHandler(this.doOpenCodeEditor.bind(this))); + this._register(this.registerCodeEditorOpenHandler(this.doOpenCodeEditorFromDiff.bind(this))); } getActiveCodeEditor(): ICodeEditor | null { diff --git a/src/vs/workbench/services/editor/browser/editorService.ts b/src/vs/workbench/services/editor/browser/editorService.ts index e241e493f65..f997b3ecfbd 100644 --- a/src/vs/workbench/services/editor/browser/editorService.ts +++ b/src/vs/workbench/services/editor/browser/editorService.ts @@ -83,9 +83,9 @@ export class EditorService extends Disposable implements EditorServiceImpl { // Editor & group changes this.editorGroupService.whenReady.then(() => this.onEditorGroupsReady()); - this.editorGroupService.onDidChangeActiveGroup(group => this.handleActiveEditorChange(group)); - this.editorGroupService.onDidAddGroup(group => this.registerGroupListeners(group as IEditorGroupView)); - this.editorsObserver.onDidMostRecentlyActiveEditorsChange(() => this._onDidMostRecentlyActiveEditorsChange.fire()); + this._register(this.editorGroupService.onDidChangeActiveGroup(group => this.handleActiveEditorChange(group))); + this._register(this.editorGroupService.onDidAddGroup(group => this.registerGroupListeners(group as IEditorGroupView))); + this._register(this.editorsObserver.onDidMostRecentlyActiveEditorsChange(() => this._onDidMostRecentlyActiveEditorsChange.fire())); // Out of workspace file watchers this._register(this.onDidVisibleEditorsChange(() => this.handleVisibleEditorsChange())); diff --git a/src/vs/workbench/services/textfile/browser/browserTextFileService.ts b/src/vs/workbench/services/textfile/browser/browserTextFileService.ts index 78957c01afd..188ca299d5f 100644 --- a/src/vs/workbench/services/textfile/browser/browserTextFileService.ts +++ b/src/vs/workbench/services/textfile/browser/browserTextFileService.ts @@ -54,7 +54,7 @@ export class BrowserTextFileService extends AbstractTextFileService { private registerListeners(): void { // Lifecycle - this.lifecycleService.onBeforeShutdown(event => event.veto(this.onBeforeShutdown(), 'veto.textFiles')); + this._register(this.lifecycleService.onBeforeShutdown(event => event.veto(this.onBeforeShutdown(), 'veto.textFiles'))); } private onBeforeShutdown(): boolean { diff --git a/src/vs/workbench/services/textfile/browser/textFileService.ts b/src/vs/workbench/services/textfile/browser/textFileService.ts index 45015baa458..f80681a7058 100644 --- a/src/vs/workbench/services/textfile/browser/textFileService.ts +++ b/src/vs/workbench/services/textfile/browser/textFileService.ts @@ -89,7 +89,7 @@ export abstract class AbstractTextFileService extends Disposable implements ITex private provideDecorations(): void { // Text file model decorations - this.decorationsService.registerDecorationsProvider(new class extends Disposable implements IDecorationsProvider { + const provider = new class extends Disposable implements IDecorationsProvider { readonly label = localize('textFileModelDecorations', "Text File Model Decorations"); @@ -160,7 +160,10 @@ export abstract class AbstractTextFileService extends Disposable implements ITex return undefined; } - }(this.files)); + }(this.files); + + this._register(provider); + this._register(this.decorationsService.registerDecorationsProvider(provider)); } //#endregin diff --git a/src/vs/workbench/services/textfile/common/textFileSaveParticipant.ts b/src/vs/workbench/services/textfile/common/textFileSaveParticipant.ts index ab3cb0a0e8a..12affdd638f 100644 --- a/src/vs/workbench/services/textfile/common/textFileSaveParticipant.ts +++ b/src/vs/workbench/services/textfile/common/textFileSaveParticipant.ts @@ -60,11 +60,14 @@ export class TextFileSaveParticipant extends Disposable { model.textEditorModel?.pushStackElement(); }, () => { // user cancel - cts.dispose(true); + cts.cancel(); + }).finally(() => { + cts.dispose(); }); } override dispose(): void { this.saveParticipants.splice(0, this.saveParticipants.length); + super.dispose(); } } diff --git a/src/vs/workbench/services/textfile/test/browser/textFileEditorModel.test.ts b/src/vs/workbench/services/textfile/test/browser/textFileEditorModel.test.ts index 7595d417a5f..0dcc1e614ee 100644 --- a/src/vs/workbench/services/textfile/test/browser/textFileEditorModel.test.ts +++ b/src/vs/workbench/services/textfile/test/browser/textFileEditorModel.test.ts @@ -638,8 +638,8 @@ suite('Files - TextFileEditorModel', () => { }); test('save() and isDirty() - proper with check for mtimes', async function () { - const input1 = createFileEditorInput(instantiationService, toResource.call(this, '/path/index_async2.txt')); - const input2 = createFileEditorInput(instantiationService, toResource.call(this, '/path/index_async.txt')); + const input1 = disposables.add(createFileEditorInput(instantiationService, toResource.call(this, '/path/index_async2.txt'))); + const input2 = disposables.add(createFileEditorInput(instantiationService, toResource.call(this, '/path/index_async.txt'))); const model1 = await input1.resolve() as TextFileEditorModel; const model2 = await input2.resolve() as TextFileEditorModel; diff --git a/src/vs/workbench/services/workingCopy/common/storedFileWorkingCopySaveParticipant.ts b/src/vs/workbench/services/workingCopy/common/storedFileWorkingCopySaveParticipant.ts index 18be856eb47..f66950e4281 100644 --- a/src/vs/workbench/services/workingCopy/common/storedFileWorkingCopySaveParticipant.ts +++ b/src/vs/workbench/services/workingCopy/common/storedFileWorkingCopySaveParticipant.ts @@ -69,5 +69,6 @@ export class StoredFileWorkingCopySaveParticipant extends Disposable { override dispose(): void { this.saveParticipants.splice(0, this.saveParticipants.length); + super.dispose(); } } diff --git a/src/vs/workbench/services/workingCopy/common/workingCopyFileOperationParticipant.ts b/src/vs/workbench/services/workingCopy/common/workingCopyFileOperationParticipant.ts index e75246b8b83..3d0f521c340 100644 --- a/src/vs/workbench/services/workingCopy/common/workingCopyFileOperationParticipant.ts +++ b/src/vs/workbench/services/workingCopy/common/workingCopyFileOperationParticipant.ts @@ -46,5 +46,6 @@ export class WorkingCopyFileOperationParticipant extends Disposable { override dispose(): void { this.participants.clear(); + super.dispose(); } } diff --git a/src/vs/workbench/test/browser/workbenchTestServices.ts b/src/vs/workbench/test/browser/workbenchTestServices.ts index 0fffe75c46f..c566bc46401 100644 --- a/src/vs/workbench/test/browser/workbenchTestServices.ts +++ b/src/vs/workbench/test/browser/workbenchTestServices.ts @@ -283,15 +283,16 @@ export function workbenchInstantiationService( instantiationService.stub(IUndoRedoService, instantiationService.createInstance(UndoRedoService)); const themeService = new TestThemeService(); instantiationService.stub(IThemeService, themeService); - instantiationService.stub(ILanguageConfigurationService, new TestLanguageConfigurationService()); + instantiationService.stub(ILanguageConfigurationService, disposables.add(new TestLanguageConfigurationService())); instantiationService.stub(IModelService, disposables.add(instantiationService.createInstance(ModelService))); const fileService = overrides?.fileService ? overrides.fileService(instantiationService) : new TestFileService(); instantiationService.stub(IFileService, fileService); const uriIdentityService = new UriIdentityService(fileService); + disposables.add(uriIdentityService); instantiationService.stub(IFilesConfigurationService, disposables.add(new TestFilesConfigurationService(contextKeyService, configService, workspaceContextService, environmentService, uriIdentityService, fileService))); instantiationService.stub(IUriIdentityService, uriIdentityService); - const userDataProfilesService = instantiationService.stub(IUserDataProfilesService, new UserDataProfilesService(environmentService, fileService, uriIdentityService, new NullLogService())); - instantiationService.stub(IUserDataProfileService, new UserDataProfileService(userDataProfilesService.defaultProfile, userDataProfilesService)); + const userDataProfilesService = instantiationService.stub(IUserDataProfilesService, disposables.add(new UserDataProfilesService(environmentService, fileService, uriIdentityService, new NullLogService()))); + instantiationService.stub(IUserDataProfileService, disposables.add(new UserDataProfileService(userDataProfilesService.defaultProfile, userDataProfilesService))); instantiationService.stub(IWorkingCopyBackupService, overrides?.workingCopyBackupService ? overrides?.workingCopyBackupService(instantiationService) : new TestWorkingCopyBackupService()); instantiationService.stub(ITelemetryService, NullTelemetryService); instantiationService.stub(INotificationService, new TestNotificationService()); @@ -305,7 +306,7 @@ export function workbenchInstantiationService( instantiationService.stub(ITextFileService, overrides?.textFileService ? overrides.textFileService(instantiationService) : disposables.add(instantiationService.createInstance(TestTextFileService))); instantiationService.stub(IHostService, instantiationService.createInstance(TestHostService)); instantiationService.stub(ITextModelService, disposables.add(instantiationService.createInstance(TextModelResolverService))); - instantiationService.stub(ILoggerService, new TestLoggerService(TestEnvironmentService.logsHome)); + instantiationService.stub(ILoggerService, disposables.add(new TestLoggerService(TestEnvironmentService.logsHome))); instantiationService.stub(ILogService, new NullLogService()); const editorGroupService = new TestEditorGroupsService([new TestEditorGroupView(0)]); instantiationService.stub(IEditorGroupsService, editorGroupService); @@ -314,10 +315,10 @@ export function workbenchInstantiationService( instantiationService.stub(IEditorService, editorService); instantiationService.stub(IWorkingCopyEditorService, disposables.add(instantiationService.createInstance(WorkingCopyEditorService))); instantiationService.stub(IEditorResolverService, disposables.add(instantiationService.createInstance(EditorResolverService))); - const textEditorService = overrides?.textEditorService ? overrides.textEditorService(instantiationService) : instantiationService.createInstance(TextEditorService); + const textEditorService = overrides?.textEditorService ? overrides.textEditorService(instantiationService) : disposables.add(instantiationService.createInstance(TextEditorService)); instantiationService.stub(ITextEditorService, textEditorService); instantiationService.stub(ICodeEditorService, disposables.add(new CodeEditorService(editorService, themeService, configService))); - instantiationService.stub(IPaneCompositePartService, new TestPaneCompositeService()); + instantiationService.stub(IPaneCompositePartService, disposables.add(new TestPaneCompositeService())); instantiationService.stub(IListService, new TestListService()); const hoverService = instantiationService.stub(IHoverService, instantiationService.createInstance(TestHoverService)); instantiationService.stub(IQuickInputService, disposables.add(new QuickInputService(configService, instantiationService, keybindingService, contextKeyService, themeService, layoutService, hoverService))); From 7ebb304bbf8e4eaf2a4c71ee6122672c10f988c5 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 18 Aug 2023 08:11:46 -0700 Subject: [PATCH 03/34] Add enabled dim unfocused setting, tweak enabled setting Part of #30522 --- .../browser/accessibilityConfiguration.ts | 60 +++++++++++++++---- .../unfocusedViewDimmingContribution.ts | 39 +++++++----- 2 files changed, 70 insertions(+), 29 deletions(-) diff --git a/src/vs/workbench/contrib/accessibility/browser/accessibilityConfiguration.ts b/src/vs/workbench/contrib/accessibility/browser/accessibilityConfiguration.ts index 0a7043a5cd8..aac405116e1 100644 --- a/src/vs/workbench/contrib/accessibility/browser/accessibilityConfiguration.ts +++ b/src/vs/workbench/contrib/accessibility/browser/accessibilityConfiguration.ts @@ -4,9 +4,10 @@ *--------------------------------------------------------------------------------------------*/ import { localize } from 'vs/nls'; -import { Extensions, IConfigurationNode, IConfigurationRegistry } from 'vs/platform/configuration/common/configurationRegistry'; +import { ConfigurationScope, Extensions, IConfigurationNode, IConfigurationRegistry } from 'vs/platform/configuration/common/configurationRegistry'; import { Registry } from 'vs/platform/registry/common/platform'; import { RawContextKey } from 'vs/platform/contextkey/common/contextkey'; +import { workbenchConfigurationNodeBase } from 'vs/workbench/common/configuration'; export const accessibilityHelpIsShown = new RawContextKey('accessibilityHelpIsShown', false, true); export const accessibleViewIsShown = new RawContextKey('accessibleViewIsShown', false, true); @@ -14,8 +15,19 @@ export const accessibleViewSupportsNavigation = new RawContextKey('acce export const accessibleViewVerbosityEnabled = new RawContextKey('accessibleViewVerbosityEnabled', false, true); export const accessibleViewGoToSymbolSupported = new RawContextKey('accessibleViewGoToSymbolSupported', false, true); -export const enum AccessibilitySettingId { - UnfocusedViewOpacity = 'accessibility.unfocusedViewOpacity' +/** + * Miscellaneous settings tagged with accessibility and implemented in the accessibility contrib but + * were better to live under workbench for discoverability. + */ +export const enum AccessibilityWorkbenchSettingId { + ViewDimUnfocusedEnabled = 'workbench.view.dimUnfocused.enabled', + ViewDimUnfocusedOpacity = 'workbench.view.dimUnfocused.opacity' +} + +export const enum ViewDimUnfocusedOpacityProperties { + Default = 0.75, + Minimum = 0.2, + Maximum = 1 } export const enum AccessibilityVerbositySettingId { @@ -82,19 +94,41 @@ const configuration: IConfigurationNode = { [AccessibilityVerbositySettingId.EditorUntitledHint]: { description: localize('verbosity.editor.untitledhint', 'Provide information about relevant actions in an untitled text editor.'), ...baseProperty - }, - [AccessibilitySettingId.UnfocusedViewOpacity]: { - description: localize('unfocusedViewOpacity', 'The opacity fraction (0.2 to 1.0) to use for unfocused editors and terminals. This will dim inactive views to make the focused view more obvious.'), - type: 'number', - minimum: 0.2, - maximum: 1, - default: 1, - tags: ['accessibility'] } + // [AccessibilitySettingId.UnfocusedViewOpacity]: { + // description: localize('unfocusedViewOpacity', 'The opacity fraction (0.2 to 1.0) to use for unfocused editors and terminals. This will dim inactive views to make the focused view more obvious.'), + // type: 'number', + // minimum: 0.2, + // maximum: 1, + // default: 1, + // tags: ['accessibility'] + // } } }; export function registerAccessibilityConfiguration() { - const configurationRegistry = Registry.as(Extensions.Configuration); - configurationRegistry.registerConfiguration(configuration); + const registry = Registry.as(Extensions.Configuration); + registry.registerConfiguration(configuration); + + registry.registerConfiguration({ + ...workbenchConfigurationNodeBase, + properties: { + [AccessibilityWorkbenchSettingId.ViewDimUnfocusedEnabled]: { + description: localize('dimUnfocusedEnabled', 'Whether to dim unfocused editors and terminals, making the focused view more obvious.'), + type: 'boolean', + default: false, + tags: ['accessibility'], + scope: ConfigurationScope.MACHINE, + }, + [AccessibilityWorkbenchSettingId.ViewDimUnfocusedOpacity]: { + description: localize('dimUnfocusedOpacity', 'The opacity fraction (0.2 to 1.0) to use for unfocused editors and terminals. This will only take effect when {0} is enabled.', `\`#${AccessibilityWorkbenchSettingId.ViewDimUnfocusedEnabled}#\``), + type: 'number', + minimum: ViewDimUnfocusedOpacityProperties.Minimum, + maximum: ViewDimUnfocusedOpacityProperties.Maximum, + default: ViewDimUnfocusedOpacityProperties.Default, + tags: ['accessibility'], + scope: ConfigurationScope.MACHINE, + } + } + }); } diff --git a/src/vs/workbench/contrib/accessibility/browser/unfocusedViewDimmingContribution.ts b/src/vs/workbench/contrib/accessibility/browser/unfocusedViewDimmingContribution.ts index e865074afda..847eb63fd2d 100644 --- a/src/vs/workbench/contrib/accessibility/browser/unfocusedViewDimmingContribution.ts +++ b/src/vs/workbench/contrib/accessibility/browser/unfocusedViewDimmingContribution.ts @@ -8,7 +8,7 @@ import { Disposable, toDisposable } from 'vs/base/common/lifecycle'; import { clamp } from 'vs/base/common/numbers'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { IWorkbenchContribution } from 'vs/workbench/common/contributions'; -import { AccessibilitySettingId } from 'vs/workbench/contrib/accessibility/browser/accessibilityConfiguration'; +import { AccessibilityWorkbenchSettingId, ViewDimUnfocusedOpacityProperties } from 'vs/workbench/contrib/accessibility/browser/accessibilityConfiguration'; export class UnfocusedViewDimmingContribution extends Disposable implements IWorkbenchContribution { constructor( @@ -22,24 +22,22 @@ export class UnfocusedViewDimmingContribution extends Disposable implements IWor this._register(toDisposable(() => elStyle.remove())); this._register(Event.runAndSubscribe(configurationService.onDidChangeConfiguration, e => { - if (e && !e.affectsConfiguration(AccessibilitySettingId.UnfocusedViewOpacity)) { + if (e && !e.affectsConfiguration(AccessibilityWorkbenchSettingId.ViewDimUnfocusedEnabled) && !e.affectsConfiguration(AccessibilityWorkbenchSettingId.ViewDimUnfocusedOpacity)) { return; } - let opacity: number; - const opacityConfig = configurationService.getValue(AccessibilitySettingId.UnfocusedViewOpacity); - if (typeof opacityConfig !== 'number') { - opacity = 1; - } else { - opacity = clamp(opacityConfig, 0.2, 1); - } - let cssTextContent = ''; - // Only add the styles if the feature is used - if (opacity !== 1) { - const rules = new Set(); + const enabled = ensureBoolean(configurationService.getValue(AccessibilityWorkbenchSettingId.ViewDimUnfocusedEnabled), false); + if (enabled) { + const opacity = clamp( + ensureNumber(configurationService.getValue(AccessibilityWorkbenchSettingId.ViewDimUnfocusedOpacity), ViewDimUnfocusedOpacityProperties.Default), + ViewDimUnfocusedOpacityProperties.Minimum, + ViewDimUnfocusedOpacityProperties.Maximum + ); + if (opacity !== 1) { + const rules = new Set(); const filterRule = `filter: opacity(${opacity});`; // Terminal tabs rules.add(`.monaco-workbench .pane-body.integrated-terminal:not(:focus-within) .tabs-container { ${filterRule} }`); @@ -49,11 +47,20 @@ export class UnfocusedViewDimmingContribution extends Disposable implements IWor rules.add(`.monaco-workbench .editor-instance:not(:focus-within) .monaco-editor { ${filterRule} }`); // Terminal editors rules.add(`.monaco-workbench .editor-instance:not(:focus-within) .terminal-wrapper { ${filterRule} }`); + cssTextContent = [...rules].join('\n'); } - cssTextContent = [...rules].join('\n'); - } - elStyle.textContent = cssTextContent; + elStyle.textContent = cssTextContent; + } })); } } + + +function ensureBoolean(value: unknown, defaultValue: boolean): boolean { + return typeof value === 'boolean' ? value : defaultValue; +} + +function ensureNumber(value: unknown, defaultValue: number): number { + return typeof value === 'number' ? value : defaultValue; +} From 42b6e31801cd4d8d327be1b5ec16582fc06b3137 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 18 Aug 2023 08:18:03 -0700 Subject: [PATCH 04/34] Don't add dim unfocused element if not used Fixes #30522 --- .../unfocusedViewDimmingContribution.ts | 28 +++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/src/vs/workbench/contrib/accessibility/browser/unfocusedViewDimmingContribution.ts b/src/vs/workbench/contrib/accessibility/browser/unfocusedViewDimmingContribution.ts index 847eb63fd2d..1dc2b4b3605 100644 --- a/src/vs/workbench/contrib/accessibility/browser/unfocusedViewDimmingContribution.ts +++ b/src/vs/workbench/contrib/accessibility/browser/unfocusedViewDimmingContribution.ts @@ -11,15 +11,14 @@ import { IWorkbenchContribution } from 'vs/workbench/common/contributions'; import { AccessibilityWorkbenchSettingId, ViewDimUnfocusedOpacityProperties } from 'vs/workbench/contrib/accessibility/browser/accessibilityConfiguration'; export class UnfocusedViewDimmingContribution extends Disposable implements IWorkbenchContribution { + private _styleElement?: HTMLStyleElement; + constructor( @IConfigurationService configurationService: IConfigurationService, ) { super(); - const elStyle = document.createElement('style'); - elStyle.className = 'accessibilityUnfocusedViewOpacity'; - document.head.appendChild(elStyle); - this._register(toDisposable(() => elStyle.remove())); + this._register(toDisposable(() => this._removeStyleElement())); this._register(Event.runAndSubscribe(configurationService.onDidChangeConfiguration, e => { if (e && !e.affectsConfiguration(AccessibilityWorkbenchSettingId.ViewDimUnfocusedEnabled) && !e.affectsConfiguration(AccessibilityWorkbenchSettingId.ViewDimUnfocusedOpacity)) { @@ -50,10 +49,29 @@ export class UnfocusedViewDimmingContribution extends Disposable implements IWor cssTextContent = [...rules].join('\n'); } - elStyle.textContent = cssTextContent; + } + + if (cssTextContent.length === 0) { + this._removeStyleElement(); + } else { + this._getStyleElement().textContent = cssTextContent; } })); } + + private _getStyleElement(): HTMLStyleElement { + if (!this._styleElement) { + this._styleElement = document.createElement('style'); + this._styleElement.className = 'accessibilityUnfocusedViewOpacity'; + document.head.appendChild(this._styleElement); + } + return this._styleElement; + } + + private _removeStyleElement(): void { + this._styleElement?.remove(); + this._styleElement = undefined; + } } From f0f36dff5efa066df997197e1d3a78e2727b70a9 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 18 Aug 2023 08:19:47 -0700 Subject: [PATCH 05/34] Remove commented out setting --- .../accessibility/browser/accessibilityConfiguration.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/vs/workbench/contrib/accessibility/browser/accessibilityConfiguration.ts b/src/vs/workbench/contrib/accessibility/browser/accessibilityConfiguration.ts index aac405116e1..b97be1b44df 100644 --- a/src/vs/workbench/contrib/accessibility/browser/accessibilityConfiguration.ts +++ b/src/vs/workbench/contrib/accessibility/browser/accessibilityConfiguration.ts @@ -95,14 +95,6 @@ const configuration: IConfigurationNode = { description: localize('verbosity.editor.untitledhint', 'Provide information about relevant actions in an untitled text editor.'), ...baseProperty } - // [AccessibilitySettingId.UnfocusedViewOpacity]: { - // description: localize('unfocusedViewOpacity', 'The opacity fraction (0.2 to 1.0) to use for unfocused editors and terminals. This will dim inactive views to make the focused view more obvious.'), - // type: 'number', - // minimum: 0.2, - // maximum: 1, - // default: 1, - // tags: ['accessibility'] - // } } }; From 3fe5a470363153bdf415eb39fca7cda471e08cda Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Fri, 18 Aug 2023 08:40:33 -0700 Subject: [PATCH 06/34] eng: revert #190623 until everyone can review it (#190752) This reverts commit 6336ee75304eaac32897d8e74c451863d877cb28. --- src/vs/base/browser/ui/grid/gridview.ts | 1 - src/vs/base/browser/ui/splitview/splitview.ts | 4 +-- src/vs/base/browser/ui/toolbar/toolbar.ts | 1 - src/vs/base/common/stream.ts | 31 +++++-------------- src/vs/base/test/common/stream.test.ts | 8 ++--- .../diffEditorWidget2/diffEditorEditors.ts | 4 +-- src/vs/editor/common/model/textModel.ts | 4 +-- .../suggest/browser/suggestController.ts | 2 +- src/vs/platform/actions/browser/buttonbar.ts | 2 +- .../platform/checksum/node/checksumService.ts | 12 ++----- src/vs/platform/files/common/fileService.ts | 12 ++----- .../node/diskFileSystemProviderServer.ts | 11 +++---- src/vs/workbench/browser/dnd.ts | 6 ++-- .../browser/parts/editor/editorGroupView.ts | 2 +- .../parts/editor/editorGroupWatermark.ts | 2 +- .../browser/parts/editor/tabsTitleControl.ts | 10 +++--- .../common/editor/editorGroupModel.ts | 2 +- .../browser/editors/textFileEditorTracker.ts | 2 +- .../browser/textFileEditorTracker.test.ts | 3 -- .../browser/inlineChatController.ts | 3 +- .../inlineChat/browser/inlineChatSession.ts | 2 -- .../inlineChat/browser/inlineChatWidget.ts | 8 ++--- .../test/browser/inlineChatController.test.ts | 26 ++++++++++++---- .../markers/test/browser/markersModel.test.ts | 3 -- .../editor/browser/codeEditorService.ts | 4 +-- .../services/editor/browser/editorService.ts | 6 ++-- .../browser/browserTextFileService.ts | 2 +- .../textfile/browser/textFileService.ts | 7 ++--- .../common/textFileSaveParticipant.ts | 5 +-- .../test/browser/textFileEditorModel.test.ts | 4 +-- .../storedFileWorkingCopySaveParticipant.ts | 1 - .../workingCopyFileOperationParticipant.ts | 1 - .../test/browser/workbenchTestServices.ts | 13 ++++---- 33 files changed, 80 insertions(+), 124 deletions(-) diff --git a/src/vs/base/browser/ui/grid/gridview.ts b/src/vs/base/browser/ui/grid/gridview.ts index 9445c64286a..c89c6a7a063 100644 --- a/src/vs/base/browser/ui/grid/gridview.ts +++ b/src/vs/base/browser/ui/grid/gridview.ts @@ -705,7 +705,6 @@ class BranchNode implements ISplitView, IDisposable { this.splitviewSashResetDisposable.dispose(); this.childrenSashResetDisposable.dispose(); this.childrenChangeDisposable.dispose(); - this.onDidScrollDisposable.dispose(); this.splitview.dispose(); } } diff --git a/src/vs/base/browser/ui/splitview/splitview.ts b/src/vs/base/browser/ui/splitview/splitview.ts index 28f18d42537..b822db43751 100644 --- a/src/vs/base/browser/ui/splitview/splitview.ts +++ b/src/vs/base/browser/ui/splitview/splitview.ts @@ -565,11 +565,11 @@ export class SplitView extends Disposable { this.sashContainer = append(this.el, $('.sash-container')); this.viewContainer = $('.split-view-container'); - this.scrollable = this._register(new Scrollable({ + this.scrollable = new Scrollable({ forceIntegerValues: true, smoothScrollDuration: 125, scheduleAtNextAnimationFrame - })); + }); this.scrollableElement = this._register(new SmoothScrollableElement(this.viewContainer, { vertical: this.orientation === Orientation.VERTICAL ? (options.scrollbarVisibility ?? ScrollbarVisibility.Auto) : ScrollbarVisibility.Hidden, horizontal: this.orientation === Orientation.HORIZONTAL ? (options.scrollbarVisibility ?? ScrollbarVisibility.Auto) : ScrollbarVisibility.Hidden diff --git a/src/vs/base/browser/ui/toolbar/toolbar.ts b/src/vs/base/browser/ui/toolbar/toolbar.ts index 67a2401ecfe..1a6089b5958 100644 --- a/src/vs/base/browser/ui/toolbar/toolbar.ts +++ b/src/vs/base/browser/ui/toolbar/toolbar.ts @@ -212,7 +212,6 @@ export class ToolBar extends Disposable { override dispose(): void { this.clear(); - this.disposables.dispose(); super.dispose(); } } diff --git a/src/vs/base/common/stream.ts b/src/vs/base/common/stream.ts index dca56d44d38..9f1039c0e52 100644 --- a/src/vs/base/common/stream.ts +++ b/src/vs/base/common/stream.ts @@ -519,14 +519,13 @@ export function consumeStream(stream: ReadableStreamEvents, reducer return new Promise((resolve, reject) => { const chunks: T[] = []; - const l = listenStream(stream, { + listenStream(stream, { onData: chunk => { if (reducer) { chunks.push(chunk); } }, onError: error => { - l.dispose(); if (reducer) { reject(error); } else { @@ -534,7 +533,6 @@ export function consumeStream(stream: ReadableStreamEvents, reducer } }, onEnd: () => { - l.dispose(); if (reducer) { resolve(reducer(chunks)); } else { @@ -572,18 +570,15 @@ export interface IStreamListener { export function listenStream(stream: ReadableStreamEvents, listener: IStreamListener): IDisposable { let destroyed = false; - // error and end events are in the next microtask so that a stream that is - // closed synchronously (e.g from a memory `toStream`) can get its disposable - // and destroy the stream. stream.on('error', error => { if (!destroyed) { - queueMicrotask(() => listener.onError(error)); + listener.onError(error); } }); stream.on('end', () => { if (!destroyed) { - queueMicrotask(() => listener.onEnd()); + listener.onEnd(); } }); @@ -697,16 +692,10 @@ export function toReadable(t: T): Readable { export function transform(stream: ReadableStreamEvents, transformer: ITransformer, reducer: IReducer): ReadableStream { const target = newWriteableStream(reducer); - const l = listenStream(stream, { + listenStream(stream, { onData: data => target.write(transformer.data(data)), - onError: error => { - l.dispose(); - target.error(transformer.error ? transformer.error(error) : error); - }, - onEnd: () => { - l.dispose(); - target.end(); - } + onError: error => target.error(transformer.error ? transformer.error(error) : error), + onEnd: () => target.end() }); return target; @@ -751,7 +740,7 @@ export function prefixedStream(prefix: T, stream: ReadableStream, reducer: const target = newWriteableStream(reducer); - const l = listenStream(stream, { + listenStream(stream, { onData: data => { // Handle prefix only once @@ -763,12 +752,8 @@ export function prefixedStream(prefix: T, stream: ReadableStream, reducer: return target.write(data); }, - onError: error => { - l.dispose(); - target.error(error); - }, + onError: error => target.error(error), onEnd: () => { - l.dispose(); // Handle prefix only once if (!prefixHandled) { diff --git a/src/vs/base/test/common/stream.test.ts b/src/vs/base/test/common/stream.test.ts index fe69e603827..78c38d691a4 100644 --- a/src/vs/base/test/common/stream.test.ts +++ b/src/vs/base/test/common/stream.test.ts @@ -315,14 +315,14 @@ suite('Stream', () => { assert.strictEqual(consumed, undefined); }); - test('listenStream', async () => { + test('listenStream', () => { const stream = newWriteableStream(strings => strings.join()); let error = false; let end = false; let data = ''; - const l = listenStream(stream, { + listenStream(stream, { onData: d => { data = d; }, @@ -345,14 +345,10 @@ suite('Stream', () => { assert.strictEqual(end, false); stream.error(new Error()); - await new Promise(r => queueMicrotask(r)); assert.strictEqual(error, true); stream.end('Final Bit'); - await new Promise(r => queueMicrotask(r)); assert.strictEqual(end, true); - - l.dispose(); }); test('listenStream - dispose', () => { diff --git a/src/vs/editor/browser/widget/diffEditorWidget2/diffEditorEditors.ts b/src/vs/editor/browser/widget/diffEditorWidget2/diffEditorEditors.ts index ee764c81aed..cce99a94271 100644 --- a/src/vs/editor/browser/widget/diffEditorWidget2/diffEditorEditors.ts +++ b/src/vs/editor/browser/widget/diffEditorWidget2/diffEditorEditors.ts @@ -35,8 +35,8 @@ export class DiffEditorEditors extends Disposable { ) { super(); - this.original = this._register(this._createLeftHandSideEditor(_options.editorOptions.get(), codeEditorWidgetOptions.originalEditor || {})); - this.modified = this._register(this._createRightHandSideEditor(_options.editorOptions.get(), codeEditorWidgetOptions.modifiedEditor || {})); + this.original = this._createLeftHandSideEditor(_options.editorOptions.get(), codeEditorWidgetOptions.originalEditor || {}); + this.modified = this._createRightHandSideEditor(_options.editorOptions.get(), codeEditorWidgetOptions.modifiedEditor || {}); this._register(autorunHandleChanges({ createEmptyChangeSummary: () => ({} as IDiffEditorConstructionOptions), diff --git a/src/vs/editor/common/model/textModel.ts b/src/vs/editor/common/model/textModel.ts index 409216fb9d9..b600bd7e3bb 100644 --- a/src/vs/editor/common/model/textModel.ts +++ b/src/vs/editor/common/model/textModel.ts @@ -67,21 +67,19 @@ export function createTextBufferFactoryFromStream(stream: ITextStream | VSBuffer let done = false; - const l = listenStream(stream, { + listenStream(stream, { onData: chunk => { builder.acceptChunk((typeof chunk === 'string') ? chunk : chunk.toString()); }, onError: error => { if (!done) { done = true; - l.dispose(); reject(error); } }, onEnd: () => { if (!done) { done = true; - l.dispose(); resolve(builder.finish()); } } diff --git a/src/vs/editor/contrib/suggest/browser/suggestController.ts b/src/vs/editor/contrib/suggest/browser/suggestController.ts index 6450e989bce..887fb23cb2d 100644 --- a/src/vs/editor/contrib/suggest/browser/suggestController.ts +++ b/src/vs/editor/contrib/suggest/browser/suggestController.ts @@ -143,7 +143,7 @@ export class SuggestController implements IEditorContribution { // context key: update insert/replace mode const ctxInsertMode = SuggestContext.InsertMode.bindTo(_contextKeyService); ctxInsertMode.set(editor.getOption(EditorOption.suggest).insertMode); - this._toDispose.add(this.model.onDidTrigger(() => ctxInsertMode.set(editor.getOption(EditorOption.suggest).insertMode))); + this.model.onDidTrigger(() => ctxInsertMode.set(editor.getOption(EditorOption.suggest).insertMode)); this.widget = this._toDispose.add(new IdleValue(() => { diff --git a/src/vs/platform/actions/browser/buttonbar.ts b/src/vs/platform/actions/browser/buttonbar.ts index 4965d6bb5cd..cb6ab4c31df 100644 --- a/src/vs/platform/actions/browser/buttonbar.ts +++ b/src/vs/platform/actions/browser/buttonbar.ts @@ -55,7 +55,7 @@ export class MenuWorkbenchButtonBar extends ButtonBar { 'workbenchActionExecuted', { id: e.action.id, from: options.telemetrySource! } ); - }, undefined, this._store); + }, this._store); } const conifgProvider: IButtonConfigProvider = options?.buttonConfigProvider ?? (() => ({ showLabel: true })); diff --git a/src/vs/platform/checksum/node/checksumService.ts b/src/vs/platform/checksum/node/checksumService.ts index 0554a503960..e4214019ff1 100644 --- a/src/vs/platform/checksum/node/checksumService.ts +++ b/src/vs/platform/checksum/node/checksumService.ts @@ -20,16 +20,10 @@ export class ChecksumService implements IChecksumService { return new Promise((resolve, reject) => { const hash = createHash('md5'); - const l = listenStream(stream, { + listenStream(stream, { onData: data => hash.update(data.buffer), - onError: error => { - l.dispose(); - reject(error); - }, - onEnd: () => { - l.dispose(); - resolve(hash.digest('base64').replace(/=+$/, '')); - } + onError: error => reject(error), + onEnd: () => resolve(hash.digest('base64').replace(/=+$/, '')) }); }); } diff --git a/src/vs/platform/files/common/fileService.ts b/src/vs/platform/files/common/fileService.ts index dc22a8203e1..e88ff723b8b 100644 --- a/src/vs/platform/files/common/fileService.ts +++ b/src/vs/platform/files/common/fileService.ts @@ -1228,7 +1228,7 @@ export class FileService extends Disposable implements IFileService { } return new Promise((resolve, reject) => { - const l = listenStream(stream, { + listenStream(stream, { onData: async chunk => { // pause stream to perform async write operation @@ -1248,14 +1248,8 @@ export class FileService extends Disposable implements IFileService { // handler again before finishing. setTimeout(() => stream.resume()); }, - onError: error => { - l.dispose(); - reject(error); - }, - onEnd: () => { - l.dispose(); - resolve(); - } + onError: error => reject(error), + onEnd: () => resolve() }); }); } diff --git a/src/vs/platform/files/node/diskFileSystemProviderServer.ts b/src/vs/platform/files/node/diskFileSystemProviderServer.ts index 989a714094b..b7e81ab4491 100644 --- a/src/vs/platform/files/node/diskFileSystemProviderServer.ts +++ b/src/vs/platform/files/node/diskFileSystemProviderServer.ts @@ -106,19 +106,16 @@ export abstract class AbstractDiskFileSystemProviderChannel extends Disposabl // Ensure to cancel the read operation when there is no more // listener on the other side to prevent unneeded work. - cts.dispose(true); + cts.cancel(); } }); const fileStream = this.provider.readFileStream(resource, opts, cts.token); - const l = listenStream(fileStream, { + listenStream(fileStream, { onData: chunk => emitter.fire(VSBuffer.wrap(chunk)), - onError: error => { - l.dispose(); - emitter.fire(error); - }, + onError: error => emitter.fire(error), onEnd: () => { - l.dispose(); + // Forward event emitter.fire('end'); diff --git a/src/vs/workbench/browser/dnd.ts b/src/vs/workbench/browser/dnd.ts index a025892f899..c196ac2132e 100644 --- a/src/vs/workbench/browser/dnd.ts +++ b/src/vs/workbench/browser/dnd.ts @@ -12,7 +12,7 @@ import { ITreeDragOverReaction } from 'vs/base/browser/ui/tree/tree'; import { coalesce } from 'vs/base/common/arrays'; import { UriList, VSDataTransfer } from 'vs/base/common/dataTransfer'; import { Emitter } from 'vs/base/common/event'; -import { Disposable, DisposableStore, IDisposable, markAsSingleton } from 'vs/base/common/lifecycle'; +import { Disposable, DisposableStore, IDisposable } from 'vs/base/common/lifecycle'; import { stringify } from 'vs/base/common/marshalling'; import { Mimes } from 'vs/base/common/mime'; import { FileAccess, Schemas } from 'vs/base/common/network'; @@ -427,10 +427,8 @@ export class CompositeDragAndDropObserver extends Disposable { static get INSTANCE(): CompositeDragAndDropObserver { if (!CompositeDragAndDropObserver.instance) { CompositeDragAndDropObserver.instance = new CompositeDragAndDropObserver(); - markAsSingleton(CompositeDragAndDropObserver.instance); } - return CompositeDragAndDropObserver.instance; } @@ -525,7 +523,7 @@ export class CompositeDragAndDropObserver extends Disposable { if (callbacks.onDragEnd) { this.onDragEnd.event(e => { callbacks.onDragEnd!(e); - }, this, disposableStore); + }); } return this._register(disposableStore); diff --git a/src/vs/workbench/browser/parts/editor/editorGroupView.ts b/src/vs/workbench/browser/parts/editor/editorGroupView.ts index 01b92a4b7e2..c80fa632d3e 100644 --- a/src/vs/workbench/browser/parts/editor/editorGroupView.ts +++ b/src/vs/workbench/browser/parts/editor/editorGroupView.ts @@ -244,7 +244,7 @@ export class EditorGroupView extends Themable implements IEditorGroupView { const groupEditorsCountContext = EditorGroupEditorsCountContext.bindTo(this.scopedContextKeyService); const groupLockedContext = ActiveEditorGroupLockedContext.bindTo(this.scopedContextKeyService); - const activeEditorListener = this._register(new MutableDisposable()); + const activeEditorListener = new MutableDisposable(); const observeActiveEditor = () => { activeEditorListener.clear(); diff --git a/src/vs/workbench/browser/parts/editor/editorGroupWatermark.ts b/src/vs/workbench/browser/parts/editor/editorGroupWatermark.ts index ff8a2778c7c..55d6506abab 100644 --- a/src/vs/workbench/browser/parts/editor/editorGroupWatermark.ts +++ b/src/vs/workbench/browser/parts/editor/editorGroupWatermark.ts @@ -90,7 +90,7 @@ export class EditorGroupWatermark extends Disposable { } private registerListeners(): void { - this._register(this.lifecycleService.onDidShutdown(() => this.dispose())); + this.lifecycleService.onDidShutdown(() => this.dispose()); this._register(this.configurationService.onDidChangeConfiguration(e => { if (e.affectsConfiguration('workbench.tips.enabled')) { diff --git a/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts b/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts index 3272b3e4839..d712b1a146b 100644 --- a/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts +++ b/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts @@ -184,7 +184,7 @@ export class TabsTitleControl extends TitleControl { this.updateTabSizing(false); // Tabs Scrollbar - this.tabsScrollbar = this.createTabsScrollbar(this.tabsContainer); + this.tabsScrollbar = this._register(this.createTabsScrollbar(this.tabsContainer)); this.tabsAndActionsContainer.appendChild(this.tabsScrollbar.getDomNode()); // Tabs Container listeners @@ -206,19 +206,19 @@ export class TabsTitleControl extends TitleControl { } private createTabsScrollbar(scrollable: HTMLElement): ScrollableElement { - const tabsScrollbar = this._register(new ScrollableElement(scrollable, { + const tabsScrollbar = new ScrollableElement(scrollable, { horizontal: ScrollbarVisibility.Auto, horizontalScrollbarSize: this.getTabsScrollbarSizing(), vertical: ScrollbarVisibility.Hidden, scrollYToX: true, useShadows: false - })); + }); - this._register(tabsScrollbar.onScroll(e => { + tabsScrollbar.onScroll(e => { if (e.scrollLeftChanged) { scrollable.scrollLeft = e.scrollLeft; } - })); + }); return tabsScrollbar; } diff --git a/src/vs/workbench/common/editor/editorGroupModel.ts b/src/vs/workbench/common/editor/editorGroupModel.ts index 0a31b47bcf7..7fc1ae6cbc0 100644 --- a/src/vs/workbench/common/editor/editorGroupModel.ts +++ b/src/vs/workbench/common/editor/editorGroupModel.ts @@ -404,7 +404,7 @@ export class EditorGroupModel extends Disposable { } private registerEditorListeners(editor: EditorInput): void { - const listeners = this._register(new DisposableStore()); + const listeners = new DisposableStore(); // Re-emit disposal of editor input as our own event listeners.add(Event.once(editor.onWillDispose)(() => { diff --git a/src/vs/workbench/contrib/files/browser/editors/textFileEditorTracker.ts b/src/vs/workbench/contrib/files/browser/editors/textFileEditorTracker.ts index afa94056cd5..fb876ba670a 100644 --- a/src/vs/workbench/contrib/files/browser/editors/textFileEditorTracker.ts +++ b/src/vs/workbench/contrib/files/browser/editors/textFileEditorTracker.ts @@ -47,7 +47,7 @@ export class TextFileEditorTracker extends Disposable implements IWorkbenchContr this._register(this.hostService.onDidChangeFocus(hasFocus => hasFocus ? this.reloadVisibleTextFileEditors() : undefined)); // Lifecycle - this._register(this.lifecycleService.onDidShutdown(() => this.dispose())); + this.lifecycleService.onDidShutdown(() => this.dispose()); } //#region Text File: Ensure every dirty text and untitled file is opened in an editor diff --git a/src/vs/workbench/contrib/files/test/browser/textFileEditorTracker.test.ts b/src/vs/workbench/contrib/files/test/browser/textFileEditorTracker.test.ts index b0afe7cc554..bc69b29a381 100644 --- a/src/vs/workbench/contrib/files/test/browser/textFileEditorTracker.test.ts +++ b/src/vs/workbench/contrib/files/test/browser/textFileEditorTracker.test.ts @@ -77,7 +77,6 @@ suite('Files - TextFileEditorTracker', () => { instantiationService.stub(IWorkspaceTrustRequestService, new TestWorkspaceTrustRequestService(false)); const editorService: EditorService = instantiationService.createInstance(EditorService); - disposables.add(editorService); instantiationService.stub(IEditorService, editorService); const accessor = instantiationService.createInstance(TestServiceAccessor); @@ -94,7 +93,6 @@ suite('Files - TextFileEditorTracker', () => { const resource = toResource.call(this, '/path/index.txt'); const model = await accessor.textFileService.files.resolve(resource) as IResolvedTextFileEditorModel; - disposables.add(model); model.textEditorModel.setValue('Super Good'); assert.strictEqual(snapshotToString(model.createSnapshot()!), 'Super Good'); @@ -143,7 +141,6 @@ suite('Files - TextFileEditorTracker', () => { } const model = await accessor.textFileService.files.resolve(resource) as IResolvedTextFileEditorModel; - disposables.add(model); model.textEditorModel.setValue('Super Good'); diff --git a/src/vs/workbench/contrib/inlineChat/browser/inlineChatController.ts b/src/vs/workbench/contrib/inlineChat/browser/inlineChatController.ts index f91de7e3ee8..104830cc852 100644 --- a/src/vs/workbench/contrib/inlineChat/browser/inlineChatController.ts +++ b/src/vs/workbench/contrib/inlineChat/browser/inlineChatController.ts @@ -100,7 +100,7 @@ export class InlineChatController implements IEditorContribution { private _messages = this._store.add(new Emitter()); - private readonly _sessionStore: DisposableStore = this._store.add(new DisposableStore()); + private readonly _sessionStore: DisposableStore = new DisposableStore(); private readonly _stashedSession: MutableDisposable = this._store.add(new MutableDisposable()); private _activeSession?: Session; private _strategy?: EditModeStrategy; @@ -146,7 +146,6 @@ export class InlineChatController implements IEditorContribution { } dispose(): void { - this._strategy?.dispose(); this._stashedSession.clear(); this.finishExistingSession(); this._store.dispose(); diff --git a/src/vs/workbench/contrib/inlineChat/browser/inlineChatSession.ts b/src/vs/workbench/contrib/inlineChat/browser/inlineChatSession.ts index fcbb998dc79..434aea1f4d2 100644 --- a/src/vs/workbench/contrib/inlineChat/browser/inlineChatSession.ts +++ b/src/vs/workbench/contrib/inlineChat/browser/inlineChatSession.ts @@ -385,8 +385,6 @@ export interface IInlineChatSessionService { // recordings(): readonly Recording[]; - - dispose(): void; } type SessionData = { diff --git a/src/vs/workbench/contrib/inlineChat/browser/inlineChatWidget.ts b/src/vs/workbench/contrib/inlineChat/browser/inlineChatWidget.ts index 8619088ce96..bf16c0aec05 100644 --- a/src/vs/workbench/contrib/inlineChat/browser/inlineChatWidget.ts +++ b/src/vs/workbench/contrib/inlineChat/browser/inlineChatWidget.ts @@ -230,7 +230,7 @@ export class InlineChatWidget { })); const uri = URI.from({ scheme: 'vscode', authority: 'inline-chat', path: `/inline-chat/model${InlineChatWidget._modelPool++}.txt` }); - this._inputModel = this._store.add(this._modelService.getModel(uri) ?? this._modelService.createModel('', null, uri)); + this._inputModel = this._modelService.getModel(uri) ?? this._modelService.createModel('', null, uri); this._inputEditor.setModel(this._inputModel); // --- context keys @@ -359,13 +359,13 @@ export class InlineChatWidget { this._store.add(feedbackToolbar); // preview editors - this._previewDiffEditor = this._store.add(new IdleValue(() => this._store.add(_instantiationService.createInstance(EmbeddedDiffEditorWidget2, this._elements.previewDiff, { + this._previewDiffEditor = new IdleValue(() => this._store.add(_instantiationService.createInstance(EmbeddedDiffEditorWidget2, this._elements.previewDiff, { ..._previewEditorEditorOptions, onlyShowAccessibleDiffViewer: this._accessibilityService.isScreenReaderOptimized(), - }, { modifiedEditor: codeEditorWidgetOptions, originalEditor: codeEditorWidgetOptions }, parentEditor)))); + }, { modifiedEditor: codeEditorWidgetOptions, originalEditor: codeEditorWidgetOptions }, parentEditor))); this._previewCreateTitle = this._store.add(_instantiationService.createInstance(ResourceLabel, this._elements.previewCreateTitle, { supportIcons: true })); - this._previewCreateEditor = this._store.add(new IdleValue(() => this._store.add(_instantiationService.createInstance(EmbeddedCodeEditorWidget, this._elements.previewCreate, _previewEditorEditorOptions, codeEditorWidgetOptions, parentEditor)))); + this._previewCreateEditor = new IdleValue(() => this._store.add(_instantiationService.createInstance(EmbeddedCodeEditorWidget, this._elements.previewCreate, _previewEditorEditorOptions, codeEditorWidgetOptions, parentEditor))); this._elements.message.tabIndex = 0; this._elements.message.ariaLabel = this._accessibleViewService.getOpenAriaHint(AccessibilityVerbositySettingId.InlineChat); diff --git a/src/vs/workbench/contrib/inlineChat/test/browser/inlineChatController.test.ts b/src/vs/workbench/contrib/inlineChat/test/browser/inlineChatController.test.ts index 7a9beea7519..ab5e4b27e1b 100644 --- a/src/vs/workbench/contrib/inlineChat/test/browser/inlineChatController.test.ts +++ b/src/vs/workbench/contrib/inlineChat/test/browser/inlineChatController.test.ts @@ -24,6 +24,7 @@ import { IEditorProgressService, IProgressRunner } from 'vs/platform/progress/co import { mock } from 'vs/base/test/common/mock'; import { Emitter, Event } from 'vs/base/common/event'; import { equals } from 'vs/base/common/arrays'; +import { timeout } from 'vs/base/common/async'; import { IChatAccessibilityService } from 'vs/workbench/contrib/chat/browser/chat'; import { IChatResponseViewModel } from 'vs/workbench/contrib/chat/common/chatViewModel'; import { IAccessibleViewService } from 'vs/workbench/contrib/accessibility/browser/accessibleView'; @@ -113,11 +114,11 @@ suite('InteractiveChatController', function () { }] ); - instaService = store.add(workbenchInstantiationService(undefined, store).createChild(serviceCollection)); - inlineChatSessionService = store.add(instaService.get(IInlineChatSessionService)); + instaService = workbenchInstantiationService(undefined, store).createChild(serviceCollection); + inlineChatSessionService = instaService.get(IInlineChatSessionService); - model = store.add(instaService.get(IModelService).createModel('Hello\nWorld\nHello Again\nHello World\n', null)); - editor = store.add(instantiateTestCodeEditor(instaService, model)); + model = instaService.get(IModelService).createModel('Hello\nWorld\nHello Again\nHello World\n', null); + editor = instantiateTestCodeEditor(instaService, model); store.add(inlineChatService.addProvider({ debugName: 'Unit Test', @@ -141,6 +142,8 @@ suite('InteractiveChatController', function () { }); teardown(function () { + editor.dispose(); + model.dispose(); store.clear(); ctrl?.dispose(); }); @@ -292,8 +295,19 @@ suite('InteractiveChatController', function () { wholeRange: new Range(3, 1, 3, 3) }; }, - provideResponse(session, request) { - return new Promise(() => { }); + async provideResponse(session, request) { + + // SLOW response + await timeout(50000); + + return { + type: InlineChatResponseType.EditorEdit, + id: Math.random(), + edits: [{ + range: new Range(1, 1, 1, 1), // EDIT happens outside of whole range + text: `${request.prompt}\n${request.prompt}` + }] + }; } }); store.add(d); diff --git a/src/vs/workbench/contrib/markers/test/browser/markersModel.test.ts b/src/vs/workbench/contrib/markers/test/browser/markersModel.test.ts index b8334c948d6..9cb2c9dc650 100644 --- a/src/vs/workbench/contrib/markers/test/browser/markersModel.test.ts +++ b/src/vs/workbench/contrib/markers/test/browser/markersModel.test.ts @@ -8,7 +8,6 @@ import { URI } from 'vs/base/common/uri'; import { IMarker, MarkerSeverity, IRelatedInformation } from 'vs/platform/markers/common/markers'; import { MarkersModel, Marker, ResourceMarkers, RelatedInformation } from 'vs/workbench/contrib/markers/browser/markersModel'; import { groupBy } from 'vs/base/common/collections'; -import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils'; class TestMarkersModel extends MarkersModel { @@ -28,8 +27,6 @@ class TestMarkersModel extends MarkersModel { suite('MarkersModel Test', () => { - ensureNoDisposablesAreLeakedInTestSuite(); - test('marker ids are unique', function () { const marker1 = anErrorWithRange(3); const marker2 = anErrorWithRange(3); diff --git a/src/vs/workbench/services/editor/browser/codeEditorService.ts b/src/vs/workbench/services/editor/browser/codeEditorService.ts index 9930f85f58c..657f203312c 100644 --- a/src/vs/workbench/services/editor/browser/codeEditorService.ts +++ b/src/vs/workbench/services/editor/browser/codeEditorService.ts @@ -25,8 +25,8 @@ export class CodeEditorService extends AbstractCodeEditorService { ) { super(themeService); - this._register(this.registerCodeEditorOpenHandler(this.doOpenCodeEditor.bind(this))); - this._register(this.registerCodeEditorOpenHandler(this.doOpenCodeEditorFromDiff.bind(this))); + this.registerCodeEditorOpenHandler(this.doOpenCodeEditor.bind(this)); + this.registerCodeEditorOpenHandler(this.doOpenCodeEditorFromDiff.bind(this)); } getActiveCodeEditor(): ICodeEditor | null { diff --git a/src/vs/workbench/services/editor/browser/editorService.ts b/src/vs/workbench/services/editor/browser/editorService.ts index f997b3ecfbd..e241e493f65 100644 --- a/src/vs/workbench/services/editor/browser/editorService.ts +++ b/src/vs/workbench/services/editor/browser/editorService.ts @@ -83,9 +83,9 @@ export class EditorService extends Disposable implements EditorServiceImpl { // Editor & group changes this.editorGroupService.whenReady.then(() => this.onEditorGroupsReady()); - this._register(this.editorGroupService.onDidChangeActiveGroup(group => this.handleActiveEditorChange(group))); - this._register(this.editorGroupService.onDidAddGroup(group => this.registerGroupListeners(group as IEditorGroupView))); - this._register(this.editorsObserver.onDidMostRecentlyActiveEditorsChange(() => this._onDidMostRecentlyActiveEditorsChange.fire())); + this.editorGroupService.onDidChangeActiveGroup(group => this.handleActiveEditorChange(group)); + this.editorGroupService.onDidAddGroup(group => this.registerGroupListeners(group as IEditorGroupView)); + this.editorsObserver.onDidMostRecentlyActiveEditorsChange(() => this._onDidMostRecentlyActiveEditorsChange.fire()); // Out of workspace file watchers this._register(this.onDidVisibleEditorsChange(() => this.handleVisibleEditorsChange())); diff --git a/src/vs/workbench/services/textfile/browser/browserTextFileService.ts b/src/vs/workbench/services/textfile/browser/browserTextFileService.ts index 188ca299d5f..78957c01afd 100644 --- a/src/vs/workbench/services/textfile/browser/browserTextFileService.ts +++ b/src/vs/workbench/services/textfile/browser/browserTextFileService.ts @@ -54,7 +54,7 @@ export class BrowserTextFileService extends AbstractTextFileService { private registerListeners(): void { // Lifecycle - this._register(this.lifecycleService.onBeforeShutdown(event => event.veto(this.onBeforeShutdown(), 'veto.textFiles'))); + this.lifecycleService.onBeforeShutdown(event => event.veto(this.onBeforeShutdown(), 'veto.textFiles')); } private onBeforeShutdown(): boolean { diff --git a/src/vs/workbench/services/textfile/browser/textFileService.ts b/src/vs/workbench/services/textfile/browser/textFileService.ts index f80681a7058..45015baa458 100644 --- a/src/vs/workbench/services/textfile/browser/textFileService.ts +++ b/src/vs/workbench/services/textfile/browser/textFileService.ts @@ -89,7 +89,7 @@ export abstract class AbstractTextFileService extends Disposable implements ITex private provideDecorations(): void { // Text file model decorations - const provider = new class extends Disposable implements IDecorationsProvider { + this.decorationsService.registerDecorationsProvider(new class extends Disposable implements IDecorationsProvider { readonly label = localize('textFileModelDecorations', "Text File Model Decorations"); @@ -160,10 +160,7 @@ export abstract class AbstractTextFileService extends Disposable implements ITex return undefined; } - }(this.files); - - this._register(provider); - this._register(this.decorationsService.registerDecorationsProvider(provider)); + }(this.files)); } //#endregin diff --git a/src/vs/workbench/services/textfile/common/textFileSaveParticipant.ts b/src/vs/workbench/services/textfile/common/textFileSaveParticipant.ts index 12affdd638f..ab3cb0a0e8a 100644 --- a/src/vs/workbench/services/textfile/common/textFileSaveParticipant.ts +++ b/src/vs/workbench/services/textfile/common/textFileSaveParticipant.ts @@ -60,14 +60,11 @@ export class TextFileSaveParticipant extends Disposable { model.textEditorModel?.pushStackElement(); }, () => { // user cancel - cts.cancel(); - }).finally(() => { - cts.dispose(); + cts.dispose(true); }); } override dispose(): void { this.saveParticipants.splice(0, this.saveParticipants.length); - super.dispose(); } } diff --git a/src/vs/workbench/services/textfile/test/browser/textFileEditorModel.test.ts b/src/vs/workbench/services/textfile/test/browser/textFileEditorModel.test.ts index 0dcc1e614ee..7595d417a5f 100644 --- a/src/vs/workbench/services/textfile/test/browser/textFileEditorModel.test.ts +++ b/src/vs/workbench/services/textfile/test/browser/textFileEditorModel.test.ts @@ -638,8 +638,8 @@ suite('Files - TextFileEditorModel', () => { }); test('save() and isDirty() - proper with check for mtimes', async function () { - const input1 = disposables.add(createFileEditorInput(instantiationService, toResource.call(this, '/path/index_async2.txt'))); - const input2 = disposables.add(createFileEditorInput(instantiationService, toResource.call(this, '/path/index_async.txt'))); + const input1 = createFileEditorInput(instantiationService, toResource.call(this, '/path/index_async2.txt')); + const input2 = createFileEditorInput(instantiationService, toResource.call(this, '/path/index_async.txt')); const model1 = await input1.resolve() as TextFileEditorModel; const model2 = await input2.resolve() as TextFileEditorModel; diff --git a/src/vs/workbench/services/workingCopy/common/storedFileWorkingCopySaveParticipant.ts b/src/vs/workbench/services/workingCopy/common/storedFileWorkingCopySaveParticipant.ts index f66950e4281..18be856eb47 100644 --- a/src/vs/workbench/services/workingCopy/common/storedFileWorkingCopySaveParticipant.ts +++ b/src/vs/workbench/services/workingCopy/common/storedFileWorkingCopySaveParticipant.ts @@ -69,6 +69,5 @@ export class StoredFileWorkingCopySaveParticipant extends Disposable { override dispose(): void { this.saveParticipants.splice(0, this.saveParticipants.length); - super.dispose(); } } diff --git a/src/vs/workbench/services/workingCopy/common/workingCopyFileOperationParticipant.ts b/src/vs/workbench/services/workingCopy/common/workingCopyFileOperationParticipant.ts index 3d0f521c340..e75246b8b83 100644 --- a/src/vs/workbench/services/workingCopy/common/workingCopyFileOperationParticipant.ts +++ b/src/vs/workbench/services/workingCopy/common/workingCopyFileOperationParticipant.ts @@ -46,6 +46,5 @@ export class WorkingCopyFileOperationParticipant extends Disposable { override dispose(): void { this.participants.clear(); - super.dispose(); } } diff --git a/src/vs/workbench/test/browser/workbenchTestServices.ts b/src/vs/workbench/test/browser/workbenchTestServices.ts index c566bc46401..0fffe75c46f 100644 --- a/src/vs/workbench/test/browser/workbenchTestServices.ts +++ b/src/vs/workbench/test/browser/workbenchTestServices.ts @@ -283,16 +283,15 @@ export function workbenchInstantiationService( instantiationService.stub(IUndoRedoService, instantiationService.createInstance(UndoRedoService)); const themeService = new TestThemeService(); instantiationService.stub(IThemeService, themeService); - instantiationService.stub(ILanguageConfigurationService, disposables.add(new TestLanguageConfigurationService())); + instantiationService.stub(ILanguageConfigurationService, new TestLanguageConfigurationService()); instantiationService.stub(IModelService, disposables.add(instantiationService.createInstance(ModelService))); const fileService = overrides?.fileService ? overrides.fileService(instantiationService) : new TestFileService(); instantiationService.stub(IFileService, fileService); const uriIdentityService = new UriIdentityService(fileService); - disposables.add(uriIdentityService); instantiationService.stub(IFilesConfigurationService, disposables.add(new TestFilesConfigurationService(contextKeyService, configService, workspaceContextService, environmentService, uriIdentityService, fileService))); instantiationService.stub(IUriIdentityService, uriIdentityService); - const userDataProfilesService = instantiationService.stub(IUserDataProfilesService, disposables.add(new UserDataProfilesService(environmentService, fileService, uriIdentityService, new NullLogService()))); - instantiationService.stub(IUserDataProfileService, disposables.add(new UserDataProfileService(userDataProfilesService.defaultProfile, userDataProfilesService))); + const userDataProfilesService = instantiationService.stub(IUserDataProfilesService, new UserDataProfilesService(environmentService, fileService, uriIdentityService, new NullLogService())); + instantiationService.stub(IUserDataProfileService, new UserDataProfileService(userDataProfilesService.defaultProfile, userDataProfilesService)); instantiationService.stub(IWorkingCopyBackupService, overrides?.workingCopyBackupService ? overrides?.workingCopyBackupService(instantiationService) : new TestWorkingCopyBackupService()); instantiationService.stub(ITelemetryService, NullTelemetryService); instantiationService.stub(INotificationService, new TestNotificationService()); @@ -306,7 +305,7 @@ export function workbenchInstantiationService( instantiationService.stub(ITextFileService, overrides?.textFileService ? overrides.textFileService(instantiationService) : disposables.add(instantiationService.createInstance(TestTextFileService))); instantiationService.stub(IHostService, instantiationService.createInstance(TestHostService)); instantiationService.stub(ITextModelService, disposables.add(instantiationService.createInstance(TextModelResolverService))); - instantiationService.stub(ILoggerService, disposables.add(new TestLoggerService(TestEnvironmentService.logsHome))); + instantiationService.stub(ILoggerService, new TestLoggerService(TestEnvironmentService.logsHome)); instantiationService.stub(ILogService, new NullLogService()); const editorGroupService = new TestEditorGroupsService([new TestEditorGroupView(0)]); instantiationService.stub(IEditorGroupsService, editorGroupService); @@ -315,10 +314,10 @@ export function workbenchInstantiationService( instantiationService.stub(IEditorService, editorService); instantiationService.stub(IWorkingCopyEditorService, disposables.add(instantiationService.createInstance(WorkingCopyEditorService))); instantiationService.stub(IEditorResolverService, disposables.add(instantiationService.createInstance(EditorResolverService))); - const textEditorService = overrides?.textEditorService ? overrides.textEditorService(instantiationService) : disposables.add(instantiationService.createInstance(TextEditorService)); + const textEditorService = overrides?.textEditorService ? overrides.textEditorService(instantiationService) : instantiationService.createInstance(TextEditorService); instantiationService.stub(ITextEditorService, textEditorService); instantiationService.stub(ICodeEditorService, disposables.add(new CodeEditorService(editorService, themeService, configService))); - instantiationService.stub(IPaneCompositePartService, disposables.add(new TestPaneCompositeService())); + instantiationService.stub(IPaneCompositePartService, new TestPaneCompositeService()); instantiationService.stub(IListService, new TestListService()); const hoverService = instantiationService.stub(IHoverService, instantiationService.createInstance(TestHoverService)); instantiationService.stub(IQuickInputService, disposables.add(new QuickInputService(configService, instantiationService, keybindingService, contextKeyService, themeService, layoutService, hoverService))); From e40344eba7a43267a2c0e70cbbde29d955d42771 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Fri, 18 Aug 2023 09:00:21 -0700 Subject: [PATCH 07/34] ports: rename 'local address' -> 'forwarded address' (#190755) Caused some confusion in https://github.com/microsoft/vscode/issues/189678#issuecomment-1684091172, and it hasn't really been a "local address" in all cases for a while, e.g. on vscode.dev. --- src/vs/workbench/contrib/remote/browser/tunnelView.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/remote/browser/tunnelView.ts b/src/vs/workbench/contrib/remote/browser/tunnelView.ts index 037e25ff458..8f94225be44 100644 --- a/src/vs/workbench/contrib/remote/browser/tunnelView.ts +++ b/src/vs/workbench/contrib/remote/browser/tunnelView.ts @@ -218,8 +218,8 @@ class PortColumn implements ITableColumn { } class LocalAddressColumn implements ITableColumn { - readonly label: string = nls.localize('tunnel.addressColumn.label', "Local Address"); - readonly tooltip: string = nls.localize('tunnel.addressColumn.tooltip', "The address that the forwarded port is available at locally."); + readonly label: string = nls.localize('tunnel.addressColumn.label', "Forwarded Address"); + readonly tooltip: string = nls.localize('tunnel.addressColumn.tooltip', "The address that the forwarded port is available at."); readonly weight: number = 1; readonly templateId: string = 'actionbar'; project(row: ITunnelItem): ActionBarCell { From 5e6c5a167a7ac02c4818d6bf6442160f73a968e1 Mon Sep 17 00:00:00 2001 From: meganrogge Date: Fri, 18 Aug 2023 09:32:35 -0700 Subject: [PATCH 08/34] fix #190716 --- .../browser/accessibilityContributions.ts | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/vs/workbench/contrib/accessibility/browser/accessibilityContributions.ts b/src/vs/workbench/contrib/accessibility/browser/accessibilityContributions.ts index ccfc48a979b..21ef08a3bd6 100644 --- a/src/vs/workbench/contrib/accessibility/browser/accessibilityContributions.ts +++ b/src/vs/workbench/contrib/accessibility/browser/accessibilityContributions.ts @@ -301,6 +301,13 @@ export class InlineCompletionsAccessibleViewContribution extends Disposable { if (!ghostText) { return false; } + const accept = () => { + model.accept(editor).then(() => { + alert('Accepted'); + model.stop(); + editor.focus(); + }); + }; this._options.language = editor.getModel()?.getLanguageId() ?? undefined; accessibleViewService.show({ verbositySettingKey: AccessibilityVerbositySettingId.InlineCompletions, @@ -315,17 +322,18 @@ export class InlineCompletionsAccessibleViewContribution extends Disposable { previous() { model.previous().then(() => show()); }, + onKeyDown: (e) => { + if (e.ctrlKey && e.browserEvent.key === '/') { + accept(); + } + }, actions: [ { id: 'inlineCompletions.accept', - label: localize('inlineCompletions.accept', "Accept Completion"), - tooltip: localize('inlineCompletions.accept', "Accept Completion"), + label: localize('inlineCompletions.accept', "Accept Completion (Ctrl+/)"), + tooltip: localize('inlineCompletions.accept', "Accept Completion (Ctrl+/)"), run: () => { - model.accept(editor).then(() => { - alert('Accepted'); - model.stop(); - editor.focus(); - }); + accept(); }, class: ThemeIcon.asClassName(Codicon.check), enabled: true From bd60cc529ca82647a759c3c90a63bd1f8495c7b3 Mon Sep 17 00:00:00 2001 From: Aaron Munger Date: Fri, 18 Aug 2023 10:43:43 -0700 Subject: [PATCH 09/34] allow copying cell output images from context menu --- extensions/ipynb/package.json | 254 +++++++++--------- extensions/ipynb/package.nls.json | 1 + extensions/notebook-renderers/src/index.ts | 1 + .../browser/controller/cellOutputActions.ts | 62 +++-- .../view/renderers/backLayerWebView.ts | 4 +- 5 files changed, 182 insertions(+), 140 deletions(-) diff --git a/extensions/ipynb/package.json b/extensions/ipynb/package.json index ce667a5d277..f9251ae54e8 100644 --- a/extensions/ipynb/package.json +++ b/extensions/ipynb/package.json @@ -1,124 +1,134 @@ { - "name": "ipynb", - "displayName": "%displayName%", - "description": "%description%", - "publisher": "vscode", - "version": "1.0.0", - "license": "MIT", - "engines": { - "vscode": "^1.57.0" - }, - "enabledApiProposals": [ - "documentPaste", - "diffContentOptions", - "dropMetadata" - ], - "activationEvents": [ - "onNotebook:jupyter-notebook", - "onNotebookSerializer:interactive" - ], - "extensionKind": [ - "workspace", - "ui" - ], - "main": "./out/ipynbMain.js", - "browser": "./dist/browser/ipynbMain.js", - "capabilities": { - "virtualWorkspaces": true, - "untrustedWorkspaces": { - "supported": true - } - }, - "contributes": { - "configuration": [ - { - "properties": { - "ipynb.pasteImagesAsAttachments.enabled": { - "type": "boolean", - "scope": "resource", - "markdownDescription": "%ipynb.pasteImagesAsAttachments.enabled%", - "default": true - } - } - } - ], - "commands": [ - { - "command": "ipynb.newUntitledIpynb", - "title": "%newUntitledIpynb.title%", - "shortTitle": "%newUntitledIpynb.shortTitle%", - "category": "Create" - }, - { - "command": "ipynb.openIpynbInNotebookEditor", - "title": "%openIpynbInNotebookEditor.title%" - }, - { - "command": "ipynb.cleanInvalidImageAttachment", - "title": "%cleanInvalidImageAttachment.title%" - } - ], - "notebooks": [ - { - "type": "jupyter-notebook", - "displayName": "Jupyter Notebook", - "selector": [ - { - "filenamePattern": "*.ipynb" - } - ], - "priority": "default" - } - ], - "notebookRenderer": [ - { - "id": "vscode.markdown-it-cell-attachment-renderer", - "displayName": "%markdownAttachmentRenderer.displayName%", - "entrypoint": { - "extends": "vscode.markdown-it-renderer", - "path": "./notebook-out/cellAttachmentRenderer.js" - } - } - ], - "menus": { - "file/newFile": [ - { - "command": "ipynb.newUntitledIpynb", - "group": "notebook" - } - ], - "commandPalette": [ - { - "command": "ipynb.newUntitledIpynb" - }, - { - "command": "ipynb.openIpynbInNotebookEditor", - "when": "false" - }, - { - "command": "ipynb.cleanInvalidImageAttachment", - "when": "false" - } - ] - } - }, - "scripts": { - "compile": "npx gulp compile-extension:ipynb && npm run build-notebook", - "watch": "npx gulp watch-extension:ipynb", - "build-notebook": "node ./esbuild" - }, - "dependencies": { - "@enonic/fnv-plus": "^1.3.0", - "detect-indent": "^6.0.0", - "uuid": "^8.3.2" - }, - "devDependencies": { - "@jupyterlab/nbformat": "^3.2.9", - "@types/markdown-it": "12.2.3", - "@types/uuid": "^8.3.1" - }, - "repository": { - "type": "git", - "url": "https://github.com/microsoft/vscode.git" - } + "name": "ipynb", + "displayName": "%displayName%", + "description": "%description%", + "publisher": "vscode", + "version": "1.0.0", + "license": "MIT", + "engines": { + "vscode": "^1.57.0" + }, + "enabledApiProposals": [ + "documentPaste", + "diffContentOptions", + "dropMetadata" + ], + "activationEvents": [ + "onNotebook:jupyter-notebook", + "onNotebookSerializer:interactive" + ], + "extensionKind": [ + "workspace", + "ui" + ], + "main": "./out/ipynbMain.js", + "browser": "./dist/browser/ipynbMain.js", + "capabilities": { + "virtualWorkspaces": true, + "untrustedWorkspaces": { + "supported": true + } + }, + "contributes": { + "configuration": [ + { + "properties": { + "ipynb.pasteImagesAsAttachments.enabled": { + "type": "boolean", + "scope": "resource", + "markdownDescription": "%ipynb.pasteImagesAsAttachments.enabled%", + "default": true + } + } + } + ], + "commands": [ + { + "command": "ipynb.newUntitledIpynb", + "title": "%newUntitledIpynb.title%", + "shortTitle": "%newUntitledIpynb.shortTitle%", + "category": "Create" + }, + { + "command": "ipynb.openIpynbInNotebookEditor", + "title": "%openIpynbInNotebookEditor.title%" + }, + { + "command": "ipynb.cleanInvalidImageAttachment", + "title": "%cleanInvalidImageAttachment.title%" + }, + { + "command": "notebook.cellOutput.copyToClipboard", + "title": "%copyOutputToClipboard.title%" + } + ], + "notebooks": [ + { + "type": "jupyter-notebook", + "displayName": "Jupyter Notebook", + "selector": [ + { + "filenamePattern": "*.ipynb" + } + ], + "priority": "default" + } + ], + "notebookRenderer": [ + { + "id": "vscode.markdown-it-cell-attachment-renderer", + "displayName": "%markdownAttachmentRenderer.displayName%", + "entrypoint": { + "extends": "vscode.markdown-it-renderer", + "path": "./notebook-out/cellAttachmentRenderer.js" + } + } + ], + "menus": { + "file/newFile": [ + { + "command": "ipynb.newUntitledIpynb", + "group": "notebook" + } + ], + "commandPalette": [ + { + "command": "ipynb.newUntitledIpynb" + }, + { + "command": "ipynb.openIpynbInNotebookEditor", + "when": "false" + }, + { + "command": "ipynb.cleanInvalidImageAttachment", + "when": "false" + } + ], + "webview/context": [ + { + "command": "notebook.cellOutput.copyToClipboard", + "when": "webviewId == 'notebook.output' && webviewSection == 'image'" + } + ] + } + }, + "scripts": { + "compile": "npx gulp compile-extension:ipynb && npm run build-notebook", + "watch": "npx gulp watch-extension:ipynb", + "build-notebook": "node ./esbuild" + }, + "dependencies": { + "@enonic/fnv-plus": "^1.3.0", + "detect-indent": "^6.0.0", + "uuid": "^8.3.2" + }, + "devDependencies": { + "@jupyterlab/nbformat": "^3.2.9", + "@types/markdown-it": "12.2.3", + "@types/uuid": "^8.3.1" + }, + "repository": { + "type": "git", + "url": "https://github.com/microsoft/vscode.git" + } } diff --git a/extensions/ipynb/package.nls.json b/extensions/ipynb/package.nls.json index bd8e0ab1da0..45aa2aa03e8 100644 --- a/extensions/ipynb/package.nls.json +++ b/extensions/ipynb/package.nls.json @@ -6,6 +6,7 @@ "newUntitledIpynb.shortTitle": "Jupyter Notebook", "openIpynbInNotebookEditor.title": "Open IPYNB File In Notebook Editor", "cleanInvalidImageAttachment.title": "Clean Invalid Image Attachment Reference", + "copyOutputToClipboard.title": "Copy Output to Clipboard", "markdownAttachmentRenderer.displayName": { "message": "Markdown-It ipynb Cell Attachment renderer", "comment": [ diff --git a/extensions/notebook-renderers/src/index.ts b/extensions/notebook-renderers/src/index.ts index 090e9719420..c5b4a631afc 100644 --- a/extensions/notebook-renderers/src/index.ts +++ b/extensions/notebook-renderers/src/index.ts @@ -37,6 +37,7 @@ function renderImage(outputInfo: OutputItem, element: HTMLElement): IDisposable if (alt) { image.alt = alt; } + image.setAttribute('data-vscode-context', JSON.stringify({ webviewSection: 'image', outputId: outputInfo.id, 'preventDefaultContextMenuItems': true })); const display = document.createElement('div'); display.classList.add('display'); display.appendChild(image); diff --git a/src/vs/workbench/contrib/notebook/browser/controller/cellOutputActions.ts b/src/vs/workbench/contrib/notebook/browser/controller/cellOutputActions.ts index d71503c9cb6..f6f859db2a4 100644 --- a/src/vs/workbench/contrib/notebook/browser/controller/cellOutputActions.ts +++ b/src/vs/workbench/contrib/notebook/browser/controller/cellOutputActions.ts @@ -5,39 +5,51 @@ import { ServicesAccessor } from 'vs/editor/browser/editorExtensions'; import { localize } from 'vs/nls'; -import { MenuId, registerAction2 } from 'vs/platform/actions/common/actions'; +import { Action2, MenuId, registerAction2 } from 'vs/platform/actions/common/actions'; import { IClipboardService } from 'vs/platform/clipboard/common/clipboardService'; -import { INotebookOutputActionContext, NotebookAction } from 'vs/workbench/contrib/notebook/browser/controller/coreActions'; +import { INotebookOutputActionContext, NOTEBOOK_ACTIONS_CATEGORY } from 'vs/workbench/contrib/notebook/browser/controller/coreActions'; import { NOTEBOOK_CELL_HAS_OUTPUTS } from 'vs/workbench/contrib/notebook/common/notebookContextKeys'; import * as icons from 'vs/workbench/contrib/notebook/browser/notebookIcons'; import { ILogService } from 'vs/platform/log/common/log'; import { copyCellOutput } from 'vs/workbench/contrib/notebook/browser/contrib/clipboard/cellOutputClipboard'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; -import { ICellViewModel, INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; +import { ICellOutputViewModel, ICellViewModel, INotebookEditor, getNotebookEditorFromEditorPane } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; +import { CellKind } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CodeCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel'; export const COPY_OUTPUT_COMMAND_ID = 'notebook.cellOutput.copyToClipboard'; -registerAction2(class CopyCellOutputAction extends NotebookAction { +registerAction2(class CopyCellOutputAction extends Action2 { constructor() { - super( - { - id: COPY_OUTPUT_COMMAND_ID, - title: localize('notebookActions.copyOutput', "Copy Output to Clipboard"), - menu: { - id: MenuId.NotebookOutputToolbar, - when: NOTEBOOK_CELL_HAS_OUTPUTS - }, - icon: icons.copyIcon, - }); + super({ + id: COPY_OUTPUT_COMMAND_ID, + title: localize('notebookActions.copyOutput', "Copy Output to Clipboard"), + menu: { + id: MenuId.NotebookOutputToolbar, + when: NOTEBOOK_CELL_HAS_OUTPUTS + }, + category: NOTEBOOK_ACTIONS_CATEGORY, + icon: icons.copyIcon, + }); } - async runWithContext(accessor: ServicesAccessor, context: INotebookOutputActionContext): Promise { - const outputViewModel = context.outputViewModel; + async run(accessor: ServicesAccessor, outputContext: INotebookOutputActionContext | { outputViewModel: ICellOutputViewModel }): Promise { + const editorService = accessor.get(IEditorService); + let outputViewModel: ICellOutputViewModel | undefined; + + if ('outputId' in outputContext && typeof outputContext.outputId === 'string') { + outputViewModel = getOutputViewModelFromId(outputContext.outputId, editorService); + } else { + outputViewModel = outputContext.outputViewModel; + } + + if (!outputViewModel) { + return; + } const mimeType = outputViewModel.pickedMimeType?.mimeType; if (mimeType?.startsWith('image/')) { - const editorService = accessor.get(IEditorService); const editor = editorService.activeEditorPane?.getControl() as INotebookEditor; await editor.focusNotebookCell(outputViewModel.cellViewModel as ICellViewModel, 'output', { skipReveal: true, outputId: outputViewModel.model.outputId }); editor.copyOutputImage(outputViewModel); @@ -48,4 +60,20 @@ registerAction2(class CopyCellOutputAction extends NotebookAction { copyCellOutput(mimeType, outputViewModel, clipboardService, logService); } } + }); + +function getOutputViewModelFromId(outputId: string, editorService: IEditorService): ICellOutputViewModel | undefined { + const notebookViewModel = getNotebookEditorFromEditorPane(editorService.activeEditorPane)?.getViewModel(); + if (notebookViewModel) { + const codeCells = notebookViewModel.viewCells.filter(cell => cell.cellKind === CellKind.Code) as CodeCellViewModel[]; + for (const cell of codeCells) { + const output = cell.outputsViewModels.find(output => output.model.outputId === outputId); + if (output) { + return output; + } + } + } + + return undefined; +} 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 1b67d83daf6..b2359655e6d 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts @@ -1074,10 +1074,12 @@ export class BackLayerWebView extends Themable { allowScripts: true, localResourceRoots: this.localResourceRootsCache, }, - extension: undefined + extension: undefined, + providedViewType: 'notebook.output' }); webview.setHtml(content); + webview.setContextKeyService(this.contextKeyService); return webview; } From f1b9a60d62b66fb02db467d294250a150a0720d1 Mon Sep 17 00:00:00 2001 From: Aaron Munger Date: Fri, 18 Aug 2023 10:55:06 -0700 Subject: [PATCH 10/34] disable default context menu items for the rest of the output webview --- .../contrib/notebook/browser/view/renderers/webviewPreloads.ts | 1 + 1 file changed, 1 insertion(+) 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 ca2ad71955c..92bf95a2111 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts @@ -2561,6 +2561,7 @@ async function webviewPreloads(ctx: PreloadContext) { ) { this.element = document.createElement('div'); this.element.classList.add('output_container'); + this.element.setAttribute('data-vscode-context', JSON.stringify({ 'preventDefaultContextMenuItems': true })); this.element.style.position = 'absolute'; this.element.style.overflow = 'hidden'; } From 967f27ba3a0758f64ce26ff14cd47acf446ee009 Mon Sep 17 00:00:00 2001 From: hsfzxjy Date: Sat, 19 Aug 2023 02:03:33 +0800 Subject: [PATCH 11/34] Fix overflow of setting list (#190721) Don't set height for AbstractListSettingWidget Fixes #187467 --- .../workbench/contrib/preferences/browser/settingsWidgets.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/vs/workbench/contrib/preferences/browser/settingsWidgets.ts b/src/vs/workbench/contrib/preferences/browser/settingsWidgets.ts index 94dc42655cd..ad2a9b4a633 100644 --- a/src/vs/workbench/contrib/preferences/browser/settingsWidgets.ts +++ b/src/vs/workbench/contrib/preferences/browser/settingsWidgets.ts @@ -208,18 +208,13 @@ export abstract class AbstractListSettingWidget extend } const header = this.renderHeader(); - const ITEM_HEIGHT = 24; - let listHeight = ITEM_HEIGHT * this.model.items.length; if (header) { - listHeight += ITEM_HEIGHT; this.listElement.appendChild(header); } this.rowElements = this.model.items.map((item, i) => this.renderDataOrEditItem(item, i, focused)); this.rowElements.forEach(rowElement => this.listElement.appendChild(rowElement)); - - this.listElement.style.height = listHeight + 'px'; } protected createBasicSelectBox(value: IObjectEnumData): SelectBox { From 7ebfc44283ae07e41147dffa65eb910739494f7e Mon Sep 17 00:00:00 2001 From: Aaron Munger Date: Fri, 18 Aug 2023 11:04:43 -0700 Subject: [PATCH 12/34] removed formatting changes --- extensions/ipynb/package.json | 244 +++++++++++++++++----------------- 1 file changed, 122 insertions(+), 122 deletions(-) diff --git a/extensions/ipynb/package.json b/extensions/ipynb/package.json index f9251ae54e8..f5e25ac3695 100644 --- a/extensions/ipynb/package.json +++ b/extensions/ipynb/package.json @@ -1,134 +1,134 @@ { - "name": "ipynb", - "displayName": "%displayName%", - "description": "%description%", - "publisher": "vscode", - "version": "1.0.0", - "license": "MIT", - "engines": { - "vscode": "^1.57.0" - }, - "enabledApiProposals": [ - "documentPaste", - "diffContentOptions", - "dropMetadata" - ], - "activationEvents": [ - "onNotebook:jupyter-notebook", - "onNotebookSerializer:interactive" - ], - "extensionKind": [ - "workspace", - "ui" - ], - "main": "./out/ipynbMain.js", - "browser": "./dist/browser/ipynbMain.js", - "capabilities": { - "virtualWorkspaces": true, - "untrustedWorkspaces": { - "supported": true - } - }, - "contributes": { - "configuration": [ - { - "properties": { - "ipynb.pasteImagesAsAttachments.enabled": { - "type": "boolean", - "scope": "resource", - "markdownDescription": "%ipynb.pasteImagesAsAttachments.enabled%", - "default": true - } - } - } - ], - "commands": [ - { - "command": "ipynb.newUntitledIpynb", - "title": "%newUntitledIpynb.title%", - "shortTitle": "%newUntitledIpynb.shortTitle%", - "category": "Create" - }, - { - "command": "ipynb.openIpynbInNotebookEditor", - "title": "%openIpynbInNotebookEditor.title%" - }, - { - "command": "ipynb.cleanInvalidImageAttachment", - "title": "%cleanInvalidImageAttachment.title%" - }, + "name": "ipynb", + "displayName": "%displayName%", + "description": "%description%", + "publisher": "vscode", + "version": "1.0.0", + "license": "MIT", + "engines": { + "vscode": "^1.57.0" + }, + "enabledApiProposals": [ + "documentPaste", + "diffContentOptions", + "dropMetadata" + ], + "activationEvents": [ + "onNotebook:jupyter-notebook", + "onNotebookSerializer:interactive" + ], + "extensionKind": [ + "workspace", + "ui" + ], + "main": "./out/ipynbMain.js", + "browser": "./dist/browser/ipynbMain.js", + "capabilities": { + "virtualWorkspaces": true, + "untrustedWorkspaces": { + "supported": true + } + }, + "contributes": { + "configuration": [ + { + "properties": { + "ipynb.pasteImagesAsAttachments.enabled": { + "type": "boolean", + "scope": "resource", + "markdownDescription": "%ipynb.pasteImagesAsAttachments.enabled%", + "default": true + } + } + } + ], + "commands": [ + { + "command": "ipynb.newUntitledIpynb", + "title": "%newUntitledIpynb.title%", + "shortTitle": "%newUntitledIpynb.shortTitle%", + "category": "Create" + }, + { + "command": "ipynb.openIpynbInNotebookEditor", + "title": "%openIpynbInNotebookEditor.title%" + }, + { + "command": "ipynb.cleanInvalidImageAttachment", + "title": "%cleanInvalidImageAttachment.title%" + }, { "command": "notebook.cellOutput.copyToClipboard", "title": "%copyOutputToClipboard.title%" } - ], - "notebooks": [ - { - "type": "jupyter-notebook", - "displayName": "Jupyter Notebook", - "selector": [ - { - "filenamePattern": "*.ipynb" - } - ], - "priority": "default" - } - ], - "notebookRenderer": [ - { - "id": "vscode.markdown-it-cell-attachment-renderer", - "displayName": "%markdownAttachmentRenderer.displayName%", - "entrypoint": { - "extends": "vscode.markdown-it-renderer", - "path": "./notebook-out/cellAttachmentRenderer.js" - } - } - ], - "menus": { - "file/newFile": [ - { - "command": "ipynb.newUntitledIpynb", - "group": "notebook" - } - ], - "commandPalette": [ - { - "command": "ipynb.newUntitledIpynb" - }, - { - "command": "ipynb.openIpynbInNotebookEditor", - "when": "false" - }, - { - "command": "ipynb.cleanInvalidImageAttachment", - "when": "false" - } - ], + ], + "notebooks": [ + { + "type": "jupyter-notebook", + "displayName": "Jupyter Notebook", + "selector": [ + { + "filenamePattern": "*.ipynb" + } + ], + "priority": "default" + } + ], + "notebookRenderer": [ + { + "id": "vscode.markdown-it-cell-attachment-renderer", + "displayName": "%markdownAttachmentRenderer.displayName%", + "entrypoint": { + "extends": "vscode.markdown-it-renderer", + "path": "./notebook-out/cellAttachmentRenderer.js" + } + } + ], + "menus": { + "file/newFile": [ + { + "command": "ipynb.newUntitledIpynb", + "group": "notebook" + } + ], + "commandPalette": [ + { + "command": "ipynb.newUntitledIpynb" + }, + { + "command": "ipynb.openIpynbInNotebookEditor", + "when": "false" + }, + { + "command": "ipynb.cleanInvalidImageAttachment", + "when": "false" + } + ], "webview/context": [ { "command": "notebook.cellOutput.copyToClipboard", "when": "webviewId == 'notebook.output' && webviewSection == 'image'" } ] - } - }, - "scripts": { - "compile": "npx gulp compile-extension:ipynb && npm run build-notebook", - "watch": "npx gulp watch-extension:ipynb", - "build-notebook": "node ./esbuild" - }, - "dependencies": { - "@enonic/fnv-plus": "^1.3.0", - "detect-indent": "^6.0.0", - "uuid": "^8.3.2" - }, - "devDependencies": { - "@jupyterlab/nbformat": "^3.2.9", - "@types/markdown-it": "12.2.3", - "@types/uuid": "^8.3.1" - }, - "repository": { - "type": "git", - "url": "https://github.com/microsoft/vscode.git" - } + } + }, + "scripts": { + "compile": "npx gulp compile-extension:ipynb && npm run build-notebook", + "watch": "npx gulp watch-extension:ipynb", + "build-notebook": "node ./esbuild" + }, + "dependencies": { + "@enonic/fnv-plus": "^1.3.0", + "detect-indent": "^6.0.0", + "uuid": "^8.3.2" + }, + "devDependencies": { + "@jupyterlab/nbformat": "^3.2.9", + "@types/markdown-it": "12.2.3", + "@types/uuid": "^8.3.1" + }, + "repository": { + "type": "git", + "url": "https://github.com/microsoft/vscode.git" + } } From c73043e10b5b083e081e427978dbba179668e233 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Fri, 18 Aug 2023 11:11:23 -0700 Subject: [PATCH 13/34] terminal: enable link detectors in detached terminals (#190699) Initially the direction was to make `TerminalLinkManager` more general and use it explicitly in detached terminals. But I ended up getting nudged in the direction of making terminalContribs work with detached terminals instead. All the link detection logic is in terminalContrib which is a layer that can't be imported from the main terminal code. And it also depends on the raw xterm instance, which we don't want to expose to callers, so I wouldn't want to have the caller manually do something like new `TerminalLinkManager(deatchedXterm.raw, ...)` So that made me think I should either make a new contrib system for detached terminals, or allow existing contribs to signal that they could run for detached terminals too. In the PR, I did the latter. ![](https://memes.peet.io/img/23-08-cce27de3-f2da-45cf-acee-6731480722ed.png) --- .../terminal/browser/detachedTerminal.ts | 101 ++++++++++++++++++ .../contrib/terminal/browser/terminal.ts | 29 ++++- .../terminal/browser/terminalExtensions.ts | 25 +++-- .../terminal/browser/terminalService.ts | 18 ++-- .../contrib/terminal/common/terminal.ts | 7 +- .../browser/terminal.links.contribution.ts | 30 +++--- .../links/browser/terminalLinkManager.ts | 16 +-- .../testing/browser/testingOutputPeek.ts | 36 ++++--- 8 files changed, 204 insertions(+), 58 deletions(-) create mode 100644 src/vs/workbench/contrib/terminal/browser/detachedTerminal.ts diff --git a/src/vs/workbench/contrib/terminal/browser/detachedTerminal.ts b/src/vs/workbench/contrib/terminal/browser/detachedTerminal.ts new file mode 100644 index 00000000000..9902a5483e5 --- /dev/null +++ b/src/vs/workbench/contrib/terminal/browser/detachedTerminal.ts @@ -0,0 +1,101 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Delayer } from 'vs/base/common/async'; +import { onUnexpectedError } from 'vs/base/common/errors'; +import { Disposable } from 'vs/base/common/lifecycle'; +import { OperatingSystem } from 'vs/base/common/platform'; +import { MicrotaskDelay } from 'vs/base/common/symbols'; +import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; +import { TerminalCapabilityStore } from 'vs/platform/terminal/common/capabilities/terminalCapabilityStore'; +import { IMergedEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariable'; +import { ITerminalBackend } from 'vs/platform/terminal/common/terminal'; +import { IDetachedTerminalInstance, IDetachedXTermOptions, IDetachedXtermTerminal, ITerminalContribution, IXtermAttachToElementOptions } from 'vs/workbench/contrib/terminal/browser/terminal'; +import { TerminalExtensionsRegistry } from 'vs/workbench/contrib/terminal/browser/terminalExtensions'; +import { TerminalWidgetManager } from 'vs/workbench/contrib/terminal/browser/widgets/widgetManager'; +import { XtermTerminal } from 'vs/workbench/contrib/terminal/browser/xterm/xtermTerminal'; +import { IEnvironmentVariableInfo } from 'vs/workbench/contrib/terminal/common/environmentVariable'; +import { ITerminalProcessInfo, ProcessState } from 'vs/workbench/contrib/terminal/common/terminal'; + +export class DeatachedTerminal extends Disposable implements IDetachedTerminalInstance { + private readonly _widgets = this._register(new TerminalWidgetManager()); + public readonly capabilities = new TerminalCapabilityStore(); + private readonly _contributions: Map = new Map(); + + public get xterm(): IDetachedXtermTerminal { + return this._xterm; + } + + constructor( + private readonly _xterm: XtermTerminal, + options: IDetachedXTermOptions, + @IInstantiationService instantiationService: IInstantiationService, + ) { + super(); + this._register(_xterm); + + // Initialize contributions + const contributionDescs = TerminalExtensionsRegistry.getTerminalContributions(); + for (const desc of contributionDescs) { + if (this._contributions.has(desc.id)) { + onUnexpectedError(new Error(`Cannot have two terminal contributions with the same id ${desc.id}`)); + continue; + } + if (desc.canRunInDetachedTerminals === false) { + continue; + } + + let contribution: ITerminalContribution; + try { + contribution = instantiationService.createInstance(desc.ctor, this, options.processInfo, this._widgets); + this._contributions.set(desc.id, contribution); + this._register(contribution); + } catch (err) { + onUnexpectedError(err); + } + } + + // xterm is already by the time DetachedTerminal is created, so trigger everything + // on the next microtask, allowing the caller to do any extra initialization + this._register(new Delayer(MicrotaskDelay)).trigger(() => { + for (const contr of this._contributions.values()) { + contr.xtermReady?.(this._xterm); + } + }); + } + + attachToElement(container: HTMLElement, options?: Partial | undefined): void { + const screenElement = this._xterm.attachToElement(container, options); + this._widgets.attachToElement(screenElement); + } +} + +/** + * Implements {@link ITerminalProcessInfo} for a detached terminal where most + * properties are stubbed. Properties are mutable and can be updated by + * the instantiator. + */ +export class DetachedProcessInfo implements ITerminalProcessInfo { + processState = ProcessState.Running; + ptyProcessReady = Promise.resolve(); + shellProcessId: number | undefined; + remoteAuthority: string | undefined; + os: OperatingSystem | undefined; + userHome: string | undefined; + initialCwd = ''; + environmentVariableInfo: IEnvironmentVariableInfo | undefined; + persistentProcessId: number | undefined; + shouldPersist = false; + hasWrittenData = false; + hasChildProcesses = false; + backend: ITerminalBackend | undefined; + capabilities = new TerminalCapabilityStore(); + shellIntegrationNonce = ''; + extEnvironmentVariableCollection: IMergedEnvironmentVariableCollection | undefined; + + constructor(initialValues: Partial) { + Object.assign(this, initialValues); + } +} diff --git a/src/vs/workbench/contrib/terminal/browser/terminal.ts b/src/vs/workbench/contrib/terminal/browser/terminal.ts index 6d199b0cd13..61b92bd54d9 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminal.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminal.ts @@ -22,7 +22,7 @@ import { IEditableData } from 'vs/workbench/common/views'; import { ITerminalStatusList } from 'vs/workbench/contrib/terminal/browser/terminalStatusList'; import { ScrollPosition } from 'vs/workbench/contrib/terminal/browser/xterm/markNavigationAddon'; import { XtermTerminal } from 'vs/workbench/contrib/terminal/browser/xterm/xtermTerminal'; -import { IRegisterContributedProfileArgs, IRemoteTerminalAttachTarget, IStartExtensionTerminalRequest, ITerminalConfigHelper, ITerminalFont, ITerminalProcessExtHostProxy } from 'vs/workbench/contrib/terminal/common/terminal'; +import { IRegisterContributedProfileArgs, IRemoteTerminalAttachTarget, IStartExtensionTerminalRequest, ITerminalConfigHelper, ITerminalFont, ITerminalProcessExtHostProxy, ITerminalProcessInfo } from 'vs/workbench/contrib/terminal/common/terminal'; import { EditorGroupColumn } from 'vs/workbench/services/editor/common/editorGroupColumn'; import { ISimpleSelectedSuggestion } from 'vs/workbench/services/suggest/browser/simpleSuggestWidget'; import type { IMarker, Terminal as RawXtermTerminal } from 'xterm'; @@ -146,8 +146,32 @@ export interface IDetachedXTermOptions { colorProvider: IXtermColorProvider; capabilities?: ITerminalCapabilityStore; readonly?: boolean; + processInfo: ITerminalProcessInfo; } +/** + * A {@link ITerminalInstance}-like object that emulates a subset of + * capabilities. This instance is returned from {@link ITerminalService.createDetachedTerminal} + * to represent terminals that appear in other parts of the VS Code UI outside + * of the "Terminal" view or editors. + */ +export interface IDetachedTerminalInstance extends IDisposable { + readonly xterm: IDetachedXtermTerminal; + readonly capabilities: ITerminalCapabilityStore; + + /** + * Attached the terminal to the given element. This should be preferred over + * calling {@link IXtermTerminal.attachToElement} so that extra DOM elements + * for contributions are initialized. + * + * @param container Container the terminal will be rendered in + * @param options Additional options for mounting the terminal in an element + */ + attachToElement(container: HTMLElement, options?: Partial): void; +} + +export const isDetachedTerminalInstance = (t: ITerminalInstance | IDetachedTerminalInstance): t is IDetachedTerminalInstance => typeof (t as ITerminalInstance).instanceId === 'number'; + export interface ITerminalService extends ITerminalInstanceHost { readonly _serviceBrand: undefined; @@ -192,7 +216,7 @@ export interface ITerminalService extends ITerminalInstanceHost { * tracked as a terminal instance. * @params options The options to create the terminal with */ - createDetachedXterm(options: IDetachedXTermOptions): Promise; + createDetachedTerminal(options: IDetachedXTermOptions): Promise; /** * Creates a raw terminal instance, this should not be used outside of the terminal part. @@ -1107,7 +1131,6 @@ export interface IXtermTerminal extends IDisposable { } export interface IDetachedXtermTerminal extends IXtermTerminal { - /** * Writes data to the terminal. * @param data data to write diff --git a/src/vs/workbench/contrib/terminal/browser/terminalExtensions.ts b/src/vs/workbench/contrib/terminal/browser/terminalExtensions.ts index a9e85fa6eb4..6657524b166 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalExtensions.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalExtensions.ts @@ -5,19 +5,24 @@ import { BrandedService, IConstructorSignature } from 'vs/platform/instantiation/common/instantiation'; import { Registry } from 'vs/platform/registry/common/platform'; -import { ITerminalContribution, ITerminalInstance } from 'vs/workbench/contrib/terminal/browser/terminal'; +import { IDetachedTerminalInstance, ITerminalContribution, ITerminalInstance } from 'vs/workbench/contrib/terminal/browser/terminal'; import { TerminalWidgetManager } from 'vs/workbench/contrib/terminal/browser/widgets/widgetManager'; -import { ITerminalProcessManager } from 'vs/workbench/contrib/terminal/common/terminal'; +import { ITerminalProcessInfo, ITerminalProcessManager } from 'vs/workbench/contrib/terminal/common/terminal'; +/** Constructor compatible with full terminal instances, is assignable to {@link DetachedCompatibleTerminalContributionCtor} */ export type TerminalContributionCtor = IConstructorSignature; +/** Constructor compatible with detached terminals */ +export type DetachedCompatibleTerminalContributionCtor = IConstructorSignature; -export interface ITerminalContributionDescription { - readonly id: string; - readonly ctor: TerminalContributionCtor; -} +export type ITerminalContributionDescription = { readonly id: string } & ( + | { readonly canRunInDetachedTerminals: false; readonly ctor: TerminalContributionCtor } + | { readonly canRunInDetachedTerminals: true; readonly ctor: DetachedCompatibleTerminalContributionCtor } +); -export function registerTerminalContribution(id: string, ctor: { new(instance: ITerminalInstance, processManager: ITerminalProcessManager, widgetManager: TerminalWidgetManager, ...services: Services): ITerminalContribution }): void { - TerminalContributionRegistry.INSTANCE.registerTerminalContribution(id, ctor); +export function registerTerminalContribution(id: string, ctor: { new(instance: ITerminalInstance, processManager: ITerminalProcessManager, widgetManager: TerminalWidgetManager, ...services: Services): ITerminalContribution }, canRunInDetachedTerminals?: false): void; +export function registerTerminalContribution(id: string, ctor: { new(instance: ITerminalInstance, processManager: ITerminalProcessInfo, widgetManager: TerminalWidgetManager, ...services: Services): ITerminalContribution }, canRunInDetachedTerminals: true): void; +export function registerTerminalContribution(id: string, ctor: { new(instance: ITerminalInstance, processManager: ITerminalProcessManager, widgetManager: TerminalWidgetManager, ...services: Services): ITerminalContribution }, canRunInDetachedTerminals = false): void { + TerminalContributionRegistry.INSTANCE.registerTerminalContribution({ id, ctor, canRunInDetachedTerminals } as ITerminalContributionDescription); } export namespace TerminalExtensionsRegistry { @@ -35,8 +40,8 @@ class TerminalContributionRegistry { constructor() { } - public registerTerminalContribution(id: string, ctor: { new(instance: ITerminalInstance, processManager: ITerminalProcessManager, widgetManager: TerminalWidgetManager, ...services: Services): ITerminalContribution }): void { - this._terminalContributions.push({ id, ctor: ctor as TerminalContributionCtor }); + public registerTerminalContribution(description: ITerminalContributionDescription): void { + this._terminalContributions.push(description); } public getTerminalContributions(): ITerminalContributionDescription[] { diff --git a/src/vs/workbench/contrib/terminal/browser/terminalService.ts b/src/vs/workbench/contrib/terminal/browser/terminalService.ts index 23b4cbdfd3d..2423c3958b1 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalService.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalService.ts @@ -29,7 +29,7 @@ import { ThemeIcon } from 'vs/base/common/themables'; import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace'; import { VirtualWorkspaceContext } from 'vs/workbench/common/contextkeys'; import { IEditableData, IViewsService } from 'vs/workbench/common/views'; -import { ICreateTerminalOptions, IDetachedXTermOptions, IDetachedXtermTerminal, IRequestAddInstanceToGroupEvent, ITerminalEditorService, ITerminalGroup, ITerminalGroupService, ITerminalInstance, ITerminalInstanceHost, ITerminalInstanceService, ITerminalLocationOptions, ITerminalService, ITerminalServiceNativeDelegate, IXtermTerminal, TerminalConnectionState, TerminalEditorLocation } from 'vs/workbench/contrib/terminal/browser/terminal'; +import { ICreateTerminalOptions, IDetachedTerminalInstance, IDetachedXTermOptions, IRequestAddInstanceToGroupEvent, ITerminalEditorService, ITerminalGroup, ITerminalGroupService, ITerminalInstance, ITerminalInstanceHost, ITerminalInstanceService, ITerminalLocationOptions, ITerminalService, ITerminalServiceNativeDelegate, IXtermTerminal, TerminalConnectionState, TerminalEditorLocation } from 'vs/workbench/contrib/terminal/browser/terminal'; import { getCwdForSplit } from 'vs/workbench/contrib/terminal/browser/terminalActions'; import { TerminalConfigHelper } from 'vs/workbench/contrib/terminal/browser/terminalConfigHelper'; import { TerminalEditorInput } from 'vs/workbench/contrib/terminal/browser/terminalEditorInput'; @@ -52,6 +52,7 @@ import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; import { TerminalCapabilityStore } from 'vs/platform/terminal/common/capabilities/terminalCapabilityStore'; import { ITimerService } from 'vs/workbench/services/timer/browser/timerService'; import { mark } from 'vs/base/common/performance'; +import { DeatachedTerminal } from 'vs/workbench/contrib/terminal/browser/detachedTerminal'; export class TerminalService extends Disposable implements ITerminalService { declare _serviceBrand: undefined; @@ -1018,9 +1019,9 @@ export class TerminalService extends Disposable implements ITerminalService { return this._createTerminal(shellLaunchConfig, location, options); } - async createDetachedXterm(options: IDetachedXTermOptions): Promise { + async createDetachedTerminal(options: IDetachedXTermOptions): Promise { const ctor = await TerminalInstance.getXtermConstructor(this._keybindingService, this._contextKeyService); - const instance = this._instantiationService.createInstance( + const xterm = this._instantiationService.createInstance( XtermTerminal, ctor, this._configHelper, @@ -1034,13 +1035,16 @@ export class TerminalService extends Disposable implements ITerminalService { ); if (options.readonly) { - instance.raw.attachCustomKeyEventHandler(() => false); + xterm.raw.attachCustomKeyEventHandler(() => false); } - this._detachedXterms.add(instance); - instance.onDidDispose(() => this._detachedXterms.delete(instance)); + this._detachedXterms.add(xterm); + const l = xterm.onDidDispose(() => { + this._detachedXterms.delete(xterm); + l.dispose(); + }); - return instance; + return new DeatachedTerminal(xterm, options, this._instantiationService); } private async _resolveCwd(shellLaunchConfig: IShellLaunchConfig, splitActiveTerminal: boolean, options?: ICreateTerminalOptions): Promise { diff --git a/src/vs/workbench/contrib/terminal/common/terminal.ts b/src/vs/workbench/contrib/terminal/common/terminal.ts index 3f528b3908c..77e37c5b574 100644 --- a/src/vs/workbench/contrib/terminal/common/terminal.ts +++ b/src/vs/workbench/contrib/terminal/common/terminal.ts @@ -253,7 +253,8 @@ export interface IDefaultShellAndArgsRequest { callback: (shell: string, args: string[] | string | undefined) => void; } -export interface ITerminalProcessManager extends IDisposable { +/** Read-only process information that can apply to detached terminals. */ +export interface ITerminalProcessInfo { readonly processState: ProcessState; readonly ptyProcessReady: Promise; readonly shellProcessId: number | undefined; @@ -270,7 +271,11 @@ export interface ITerminalProcessManager extends IDisposable { readonly capabilities: ITerminalCapabilityStore; readonly shellIntegrationNonce: string; readonly extEnvironmentVariableCollection: IMergedEnvironmentVariableCollection | undefined; +} +export const isTerminalProcessManager = (t: ITerminalProcessInfo | ITerminalProcessManager): t is ITerminalProcessManager => typeof (t as ITerminalProcessManager).write === 'function'; + +export interface ITerminalProcessManager extends IDisposable, ITerminalProcessInfo { readonly onPtyDisconnect: Event; readonly onPtyReconnect: Event; diff --git a/src/vs/workbench/contrib/terminalContrib/links/browser/terminal.links.contribution.ts b/src/vs/workbench/contrib/terminalContrib/links/browser/terminal.links.contribution.ts index bfc8d964116..5d144353e9f 100644 --- a/src/vs/workbench/contrib/terminalContrib/links/browser/terminal.links.contribution.ts +++ b/src/vs/workbench/contrib/terminalContrib/links/browser/terminal.links.contribution.ts @@ -10,11 +10,11 @@ import { ContextKeyExpr } from 'vs/platform/contextkey/common/contextkey'; import { InstantiationType, registerSingleton } from 'vs/platform/instantiation/common/extensions'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegistry'; -import { ITerminalContribution, ITerminalInstance, IXtermTerminal } from 'vs/workbench/contrib/terminal/browser/terminal'; +import { IDetachedTerminalInstance, ITerminalContribution, ITerminalInstance, IXtermTerminal, isDetachedTerminalInstance } from 'vs/workbench/contrib/terminal/browser/terminal'; import { registerActiveInstanceAction } from 'vs/workbench/contrib/terminal/browser/terminalActions'; import { registerTerminalContribution } from 'vs/workbench/contrib/terminal/browser/terminalExtensions'; import { TerminalWidgetManager } from 'vs/workbench/contrib/terminal/browser/widgets/widgetManager'; -import { ITerminalProcessManager, TerminalCommandId } from 'vs/workbench/contrib/terminal/common/terminal'; +import { ITerminalProcessInfo, ITerminalProcessManager, TerminalCommandId, isTerminalProcessManager } from 'vs/workbench/contrib/terminal/common/terminal'; import { TerminalContextKeys } from 'vs/workbench/contrib/terminal/common/terminalContextKey'; import { terminalStrings } from 'vs/workbench/contrib/terminal/common/terminalStrings'; import { ITerminalLinkProviderService } from 'vs/workbench/contrib/terminalContrib/links/browser/links'; @@ -40,8 +40,8 @@ class TerminalLinkContribution extends DisposableStore implements ITerminalContr private _linkResolver: TerminalLinkResolver; constructor( - private readonly _instance: ITerminalInstance, - private readonly _processManager: ITerminalProcessManager, + private readonly _instance: ITerminalInstance | IDetachedTerminalInstance, + private readonly _processManager: ITerminalProcessManager | ITerminalProcessInfo, private readonly _widgetManager: TerminalWidgetManager, @IInstantiationService private readonly _instantiationService: IInstantiationService, @ITerminalLinkProviderService private readonly _terminalLinkProviderService: ITerminalLinkProviderService @@ -52,18 +52,24 @@ class TerminalLinkContribution extends DisposableStore implements ITerminalContr xtermReady(xterm: IXtermTerminal & { raw: RawXtermTerminal }): void { const linkManager = this._instantiationService.createInstance(TerminalLinkManager, xterm.raw, this._processManager, this._instance.capabilities, this._linkResolver); - this._processManager.onProcessReady(() => { + if (isTerminalProcessManager(this._processManager)) { + this._processManager.onProcessReady(() => { + linkManager.setWidgetManager(this._widgetManager); + }); + } else { linkManager.setWidgetManager(this._widgetManager); - }); + } this._linkManager = this.add(linkManager); // Attach the link provider(s) to the instance and listen for changes - for (const linkProvider of this._terminalLinkProviderService.linkProviders) { - this._linkManager.registerExternalLinkProvider(linkProvider.provideLinks.bind(linkProvider, this._instance)); + if (!isDetachedTerminalInstance(this._instance)) { + for (const linkProvider of this._terminalLinkProviderService.linkProviders) { + this._linkManager.registerExternalLinkProvider(linkProvider.provideLinks.bind(linkProvider, this._instance)); + } + this.add(this._terminalLinkProviderService.onDidAddLinkProvider(e => { + linkManager.registerExternalLinkProvider(e.provideLinks.bind(e, this._instance as ITerminalInstance)); + })); } - this.add(this._terminalLinkProviderService.onDidAddLinkProvider(e => { - linkManager.registerExternalLinkProvider(e.provideLinks.bind(e, this._instance)); - })); // TODO: Currently only a single link provider is supported; the one registered by the ext host this.add(this._terminalLinkProviderService.onDidRemoveLinkProvider(e => { linkManager.dispose(); @@ -97,7 +103,7 @@ class TerminalLinkContribution extends DisposableStore implements ITerminalContr } } -registerTerminalContribution(TerminalLinkContribution.ID, TerminalLinkContribution); +registerTerminalContribution(TerminalLinkContribution.ID, TerminalLinkContribution, true); const category = terminalStrings.actionCategory; diff --git a/src/vs/workbench/contrib/terminalContrib/links/browser/terminalLinkManager.ts b/src/vs/workbench/contrib/terminalContrib/links/browser/terminalLinkManager.ts index 3cce34e79b6..93105d6e3c0 100644 --- a/src/vs/workbench/contrib/terminalContrib/links/browser/terminalLinkManager.ts +++ b/src/vs/workbench/contrib/terminalContrib/links/browser/terminalLinkManager.ts @@ -25,7 +25,7 @@ import { ILinkHoverTargetOptions, TerminalHover } from 'vs/workbench/contrib/ter import { TerminalWidgetManager } from 'vs/workbench/contrib/terminal/browser/widgets/widgetManager'; import { IXtermCore } from 'vs/workbench/contrib/terminal/browser/xterm-private'; import { ITerminalCapabilityStore } from 'vs/platform/terminal/common/capabilities/capabilities'; -import { ITerminalConfiguration, ITerminalProcessManager, TERMINAL_CONFIG_SECTION } from 'vs/workbench/contrib/terminal/common/terminal'; +import { ITerminalConfiguration, ITerminalProcessInfo, TERMINAL_CONFIG_SECTION } from 'vs/workbench/contrib/terminal/common/terminal'; import { IHoverAction } from 'vs/workbench/services/hover/browser/hover'; import type { ILink, ILinkProvider, IViewportRange, Terminal } from 'xterm'; import { convertBufferRangeToViewport } from 'vs/workbench/contrib/terminalContrib/links/browser/terminalLinkHelpers'; @@ -47,7 +47,7 @@ export class TerminalLinkManager extends DisposableStore { constructor( private readonly _xterm: Terminal, - private readonly _processManager: ITerminalProcessManager, + private readonly _processInfo: ITerminalProcessInfo, capabilities: ITerminalCapabilityStore, private readonly _linkResolver: ITerminalLinkResolver, @IConfigurationService private readonly _configurationService: IConfigurationService, @@ -65,15 +65,15 @@ export class TerminalLinkManager extends DisposableStore { enableFileLinks = false; break; case 'notRemote': - enableFileLinks = !this._processManager.remoteAuthority; + enableFileLinks = !this._processInfo.remoteAuthority; break; } // Setup link detectors in their order of priority - this._setupLinkDetector(TerminalUriLinkDetector.id, this._instantiationService.createInstance(TerminalUriLinkDetector, this._xterm, this._processManager, this._linkResolver)); + this._setupLinkDetector(TerminalUriLinkDetector.id, this._instantiationService.createInstance(TerminalUriLinkDetector, this._xterm, this._processInfo, this._linkResolver)); if (enableFileLinks) { - this._setupLinkDetector(TerminalMultiLineLinkDetector.id, this._instantiationService.createInstance(TerminalMultiLineLinkDetector, this._xterm, this._processManager, this._linkResolver)); - this._setupLinkDetector(TerminalLocalLinkDetector.id, this._instantiationService.createInstance(TerminalLocalLinkDetector, this._xterm, capabilities, this._processManager, this._linkResolver)); + this._setupLinkDetector(TerminalMultiLineLinkDetector.id, this._instantiationService.createInstance(TerminalMultiLineLinkDetector, this._xterm, this._processInfo, this._linkResolver)); + this._setupLinkDetector(TerminalLocalLinkDetector.id, this._instantiationService.createInstance(TerminalLocalLinkDetector, this._xterm, capabilities, this._processInfo, this._linkResolver)); } this._setupLinkDetector(TerminalWordLinkDetector.id, this.add(this._instantiationService.createInstance(TerminalWordLinkDetector, this._xterm))); @@ -83,8 +83,8 @@ export class TerminalLinkManager extends DisposableStore { this._openers.set(TerminalBuiltinLinkType.LocalFile, localFileOpener); this._openers.set(TerminalBuiltinLinkType.LocalFolderInWorkspace, localFolderInWorkspaceOpener); this._openers.set(TerminalBuiltinLinkType.LocalFolderOutsideWorkspace, this._instantiationService.createInstance(TerminalLocalFolderOutsideWorkspaceLinkOpener)); - this._openers.set(TerminalBuiltinLinkType.Search, this._instantiationService.createInstance(TerminalSearchLinkOpener, capabilities, this._processManager.initialCwd, localFileOpener, localFolderInWorkspaceOpener, () => this._processManager.os || OS)); - this._openers.set(TerminalBuiltinLinkType.Url, this._instantiationService.createInstance(TerminalUrlLinkOpener, !!this._processManager.remoteAuthority)); + this._openers.set(TerminalBuiltinLinkType.Search, this._instantiationService.createInstance(TerminalSearchLinkOpener, capabilities, this._processInfo.initialCwd, localFileOpener, localFolderInWorkspaceOpener, () => this._processInfo.os || OS)); + this._openers.set(TerminalBuiltinLinkType.Url, this._instantiationService.createInstance(TerminalUrlLinkOpener, !!this._processInfo.remoteAuthority)); this._registerStandardLinkProviders(); diff --git a/src/vs/workbench/contrib/testing/browser/testingOutputPeek.ts b/src/vs/workbench/contrib/testing/browser/testingOutputPeek.ts index 047d8486c0a..6223c1c211f 100644 --- a/src/vs/workbench/contrib/testing/browser/testingOutputPeek.ts +++ b/src/vs/workbench/contrib/testing/browser/testingOutputPeek.ts @@ -74,7 +74,8 @@ import { IViewPaneOptions, ViewPane } from 'vs/workbench/browser/parts/views/vie import { EditorModel } from 'vs/workbench/common/editor/editorModel'; import { PANEL_BACKGROUND, SIDE_BAR_BACKGROUND } from 'vs/workbench/common/theme'; import { IViewDescriptorService, IViewsService, ViewContainerLocation } from 'vs/workbench/common/views'; -import { IDetachedXtermTerminal, ITerminalService } from 'vs/workbench/contrib/terminal/browser/terminal'; +import { DetachedProcessInfo } from 'vs/workbench/contrib/terminal/browser/detachedTerminal'; +import { IDetachedTerminalInstance, ITerminalService } from 'vs/workbench/contrib/terminal/browser/terminal'; import { getXtermScaledDimensions } from 'vs/workbench/contrib/terminal/browser/xterm/xtermTerminal'; import { TERMINAL_BACKGROUND_COLOR } from 'vs/workbench/contrib/terminal/common/terminalColorRegistry'; import { flatTestItemDelimiter } from 'vs/workbench/contrib/testing/browser/explorerProjections/display'; @@ -1352,7 +1353,7 @@ class TerminalMessagePeek extends Disposable implements IPeekOutputRenderer { private readonly xtermLayoutDelayer = this._register(new Delayer(50)); /** Active terminal instance. */ - private readonly terminal = this._register(new MutableDisposable()); + private readonly terminal = this._register(new MutableDisposable()); /** Listener for streaming result data */ private readonly outputDataListener = this._register(new MutableDisposable()); @@ -1369,11 +1370,11 @@ class TerminalMessagePeek extends Disposable implements IPeekOutputRenderer { private async makeTerminal() { const prev = this.terminal.value; if (prev) { - prev.clearBuffer(); - prev.clearSearchDecorations(); + prev.xterm.clearBuffer(); + prev.xterm.clearSearchDecorations(); // clearBuffer tries to retain the prompt line, but this doesn't exist for tests. // So clear the screen (J) and move to home (H) to ensure previous data is cleaned up. - prev.write(`\x1b[2J\x1b[0;0H`); + prev.xterm.write(`\x1b[2J\x1b[0;0H`); return prev; } @@ -1387,11 +1388,12 @@ class TerminalMessagePeek extends Disposable implements IPeekOutputRenderer { updateCwd: () => { }, }); - return this.terminal.value = await this.terminalService.createDetachedXterm({ + return this.terminal.value = await this.terminalService.createDetachedTerminal({ rows: 10, cols: 80, readonly: true, capabilities, + processInfo: new DetachedProcessInfo({ initialCwd: cwd.value }), colorProvider: { getBackgroundColor: theme => { const terminalBackground = theme.getColor(TERMINAL_BACKGROUND_COLOR); @@ -1440,17 +1442,17 @@ class TerminalMessagePeek extends Disposable implements IPeekOutputRenderer { } } }, - doListenForMoreData: (output, result, terminal) => result.onChange(e => { + doListenForMoreData: (output, result, { xterm }) => result.onChange(e => { if (e.reason === TestResultItemChangeReason.NewMessage && e.item.item.extId === testItem.extId && e.message.type === TestMessageType.Output) { for (const chunk of output.getRangeIter(e.message.offset, e.message.length)) { - terminal.write(chunk.buffer); + xterm.write(chunk.buffer); } } }), }); if (subject instanceof MessageSubject && subject.message.type === TestMessageType.Output && subject.message.marker !== undefined) { - terminal?.selectMarkedRange(getMarkId(subject.message.marker, true), getMarkId(subject.message.marker, false), /* scrollIntoView= */ true); + terminal?.xterm.selectMarkedRange(getMarkId(subject.message.marker, true), getMarkId(subject.message.marker, false), /* scrollIntoView= */ true); } } @@ -1464,7 +1466,7 @@ class TerminalMessagePeek extends Disposable implements IPeekOutputRenderer { this.updateCwd(Iterable.find(result.tests, t => !!t.item.uri)?.item.uri); return task.output.buffers; }, - doListenForMoreData: (task, _result, terminal) => task.output.onDidWriteData(e => terminal.write(e.buffer)), + doListenForMoreData: (task, _result, { xterm }) => task.output.onDidWriteData(e => xterm.write(e.buffer)), }); } @@ -1472,7 +1474,7 @@ class TerminalMessagePeek extends Disposable implements IPeekOutputRenderer { subject: InspectSubject; getTarget: (result: ITestResult) => T | undefined; doInitialWrite: (target: T, result: LiveTestResult) => Iterable; - doListenForMoreData: (target: T, result: LiveTestResult, terminal: IDetachedXtermTerminal) => IDisposable | undefined; + doListenForMoreData: (target: T, result: LiveTestResult, terminal: IDetachedTerminalInstance) => IDisposable | undefined; }) { const result = opts.subject.result; const target = opts.getTarget(result); @@ -1488,7 +1490,7 @@ class TerminalMessagePeek extends Disposable implements IPeekOutputRenderer { for (const chunk of opts.doInitialWrite(target, result)) { didWriteData ||= chunk.byteLength > 0; pendingWrites.value++; - terminal.write(chunk.buffer, () => pendingWrites.value--); + terminal.xterm.write(chunk.buffer, () => pendingWrites.value--); } } else { this.writeNotice(terminal, localize('runNoOutputForPast', 'Test output is only available for new test runs.')); @@ -1525,12 +1527,12 @@ class TerminalMessagePeek extends Disposable implements IPeekOutputRenderer { } } - private writeNotice(terminal: IDetachedXtermTerminal, str: string) { - terminal.write(`\x1b[2m${str}\x1b[0m`); + private writeNotice(terminal: IDetachedTerminalInstance, str: string) { + terminal.xterm.write(`\x1b[2m${str}\x1b[0m`); } - private attachTerminalToDom(terminal: IDetachedXtermTerminal) { - terminal.write('\x1b[?25l'); // hide cursor + private attachTerminalToDom(terminal: IDetachedTerminalInstance) { + terminal.xterm.write('\x1b[?25l'); // hide cursor requestAnimationFrame(() => this.layoutTerminal(terminal)); terminal.attachToElement(this.container, { enableGpu: false }); } @@ -1549,7 +1551,7 @@ class TerminalMessagePeek extends Disposable implements IPeekOutputRenderer { } private layoutTerminal( - xterm: IDetachedXtermTerminal, + { xterm }: IDetachedTerminalInstance, width = this.dimensions?.width ?? this.container.clientWidth, height = this.dimensions?.height ?? this.container.clientHeight ) { From d1a2b7eac949d1c3064898e6cb8ed310445ceff0 Mon Sep 17 00:00:00 2001 From: Joyce Er Date: Fri, 18 Aug 2023 12:23:18 -0700 Subject: [PATCH 14/34] Add debug logging for Cloud Changes enablement (#190764) --- .../editSessions/browser/editSessions.contribution.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/vs/workbench/contrib/editSessions/browser/editSessions.contribution.ts b/src/vs/workbench/contrib/editSessions/browser/editSessions.contribution.ts index 0f6e359a961..2ac22f1d0d7 100644 --- a/src/vs/workbench/contrib/editSessions/browser/editSessions.contribution.ts +++ b/src/vs/workbench/contrib/editSessions/browser/editSessions.contribution.ts @@ -206,14 +206,17 @@ export class EditSessionsContribution extends Disposable implements IWorkbenchCo } else if (shouldAutoResumeOnReload) { // The application has previously launched via a protocol URL Continue On flow const hasApplicationLaunchedFromContinueOnFlow = this.storageService.getBoolean(EditSessionsContribution.APPLICATION_LAUNCHED_VIA_CONTINUE_ON_STORAGE_KEY, StorageScope.APPLICATION, false); + this.logService.info(`Prompting to enable cloud changes, has application previously launched from Continue On flow: ${hasApplicationLaunchedFromContinueOnFlow}`); const handlePendingEditSessions = () => { // display a badge in the accounts menu but do not prompt the user to sign in again + this.logService.info('Showing badge to enable cloud changes in accounts menu...'); this.updateAccountsMenuBadge(); this.pendingEditSessionsContext.set(true); // attempt a resume if we are in a pending state and the user just signed in const disposable = this.editSessionsStorageService.onDidSignIn(async () => { disposable.dispose(); + this.logService.info('Showing badge to enable cloud changes in accounts menu succeeded, resuming cloud changes...'); await this.progressService.withProgress(resumeProgressOptions, async (progress) => await this.resumeEditSession(undefined, true, undefined, undefined, progress)); this.storageService.remove(EditSessionsContribution.APPLICATION_LAUNCHED_VIA_CONTINUE_ON_STORAGE_KEY, StorageScope.APPLICATION); this.environmentService.continueOn = undefined; @@ -227,8 +230,10 @@ export class EditSessionsContribution extends Disposable implements IWorkbenchCo ) { // store the fact that we prompted the user this.storageService.store(EditSessionsContribution.APPLICATION_LAUNCHED_VIA_CONTINUE_ON_STORAGE_KEY, true, StorageScope.APPLICATION, StorageTarget.MACHINE); + this.logService.info('Prompting to enable cloud changes...'); await this.editSessionsStorageService.initialize('read'); if (this.editSessionsStorageService.isSignedIn) { + this.logService.info('Prompting to enable cloud changes succeeded, resuming cloud changes...'); await this.progressService.withProgress(resumeProgressOptions, async (progress) => await this.resumeEditSession(undefined, true, undefined, undefined, progress)); } else { handlePendingEditSessions(); @@ -239,6 +244,8 @@ export class EditSessionsContribution extends Disposable implements IWorkbenchCo ) { handlePendingEditSessions(); } + } else { + this.logService.debug('Auto resuming cloud changes disabled.'); } } From 4d0e68207ea272b279e5994be5211de691d2af17 Mon Sep 17 00:00:00 2001 From: Aaron Munger Date: Fri, 18 Aug 2023 13:27:06 -0700 Subject: [PATCH 15/34] fixed copying image in Interactive Window --- .../browser/controller/cellOutputActions.ts | 19 ++++--- .../browser/view/renderers/webviewPreloads.ts | 49 ++++++++++--------- 2 files changed, 37 insertions(+), 31 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/controller/cellOutputActions.ts b/src/vs/workbench/contrib/notebook/browser/controller/cellOutputActions.ts index f6f859db2a4..956222d7a36 100644 --- a/src/vs/workbench/contrib/notebook/browser/controller/cellOutputActions.ts +++ b/src/vs/workbench/contrib/notebook/browser/controller/cellOutputActions.ts @@ -35,10 +35,15 @@ registerAction2(class CopyCellOutputAction extends Action2 { async run(accessor: ServicesAccessor, outputContext: INotebookOutputActionContext | { outputViewModel: ICellOutputViewModel }): Promise { const editorService = accessor.get(IEditorService); - let outputViewModel: ICellOutputViewModel | undefined; + const notebookEditor = getNotebookEditorFromEditorPane(editorService.activeEditorPane); + if (!notebookEditor) { + return; + } + + let outputViewModel: ICellOutputViewModel | undefined; if ('outputId' in outputContext && typeof outputContext.outputId === 'string') { - outputViewModel = getOutputViewModelFromId(outputContext.outputId, editorService); + outputViewModel = getOutputViewModelFromId(outputContext.outputId, notebookEditor); } else { outputViewModel = outputContext.outputViewModel; } @@ -50,9 +55,9 @@ registerAction2(class CopyCellOutputAction extends Action2 { const mimeType = outputViewModel.pickedMimeType?.mimeType; if (mimeType?.startsWith('image/')) { - const editor = editorService.activeEditorPane?.getControl() as INotebookEditor; - await editor.focusNotebookCell(outputViewModel.cellViewModel as ICellViewModel, 'output', { skipReveal: true, outputId: outputViewModel.model.outputId }); - editor.copyOutputImage(outputViewModel); + const focusOptions = { skipReveal: true, outputId: outputViewModel.model.outputId }; + await notebookEditor.focusNotebookCell(outputViewModel.cellViewModel as ICellViewModel, 'output', focusOptions); + notebookEditor.copyOutputImage(outputViewModel); } else { const clipboardService = accessor.get(IClipboardService); const logService = accessor.get(ILogService); @@ -63,8 +68,8 @@ registerAction2(class CopyCellOutputAction extends Action2 { }); -function getOutputViewModelFromId(outputId: string, editorService: IEditorService): ICellOutputViewModel | undefined { - const notebookViewModel = getNotebookEditorFromEditorPane(editorService.activeEditorPane)?.getViewModel(); +function getOutputViewModelFromId(outputId: string, notebookEditor: INotebookEditor): ICellOutputViewModel | undefined { + const notebookViewModel = notebookEditor.getViewModel(); if (notebookViewModel) { const codeCells = notebookViewModel.viewCells.filter(cell => cell.cellKind === CellKind.Code) as CodeCellViewModel[]; for (const cell of codeCells) { 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 92bf95a2111..4715e64aa08 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts @@ -1362,33 +1362,38 @@ async function webviewPreloads(ctx: PreloadContext) { }); }; - const copyImage = async (image: HTMLImageElement, retries = 5) => { + const copyOutputImage = async (outputId: string, retries = 5) => { if (!document.hasFocus() && retries > 0) { // copyImage can be called from outside of the webview, which means this function may be running whilst the webview is gaining focus. // Since navigator.clipboard.write requires the document to be focused, we need to wait for focus. // We cannot use a listener, as there is a high chance the focus is gained during the setup of the listener resulting in us missing it. - setTimeout(() => { copyImage(image, retries - 1); }, 20); + setTimeout(() => { copyOutputImage(outputId, retries - 1); }, 20); return; } try { - await navigator.clipboard.write([new ClipboardItem({ - 'image/png': new Promise((resolve) => { - const canvas = document.createElement('canvas'); - if (canvas !== null) { - canvas.width = image.naturalWidth; - canvas.height = image.naturalHeight; - const context = canvas.getContext('2d'); - context?.drawImage(image, 0, 0); - } - canvas.toBlob((blob) => { - if (blob) { - resolve(blob); + const image = document.getElementById(outputId)?.querySelector('img'); + if (image) { + await navigator.clipboard.write([new ClipboardItem({ + 'image/png': new Promise((resolve) => { + const canvas = document.createElement('canvas'); + if (canvas !== null) { + canvas.width = image.naturalWidth; + canvas.height = image.naturalHeight; + const context = canvas.getContext('2d'); + context?.drawImage(image, 0, 0); } - canvas.remove(); - }, 'image/png'); - }) - })]); + canvas.toBlob((blob) => { + if (blob) { + resolve(blob); + } + canvas.remove(); + }, 'image/png'); + }) + })]); + } else { + console.error('Could not find image element to copy for output with id', outputId); + } } catch (e) { console.error('Could not copy image:', e); } @@ -1495,12 +1500,8 @@ async function webviewPreloads(ctx: PreloadContext) { break; } case 'copyImage': { - const image = document.getElementById(event.data.outputId)?.querySelector('img'); - if (image) { - await copyImage(image); - } else { - console.warn('Could not find image element to copy for output with id', event.data.outputId); - } + + await copyOutputImage(event.data.outputId); break; } From de63553929b5077bc8db7ad609da5924dd737230 Mon Sep 17 00:00:00 2001 From: meganrogge Date: Fri, 18 Aug 2023 13:37:40 -0700 Subject: [PATCH 16/34] add an action --- .../browser/accessibilityConfiguration.ts | 1 + .../browser/accessibilityContributions.ts | 28 +---------- .../accessibility/browser/accessibleView.ts | 6 ++- .../browser/accessibleViewActions.ts | 46 ++++++++++++++++++- .../common/accessibilityCommands.ts | 3 +- 5 files changed, 55 insertions(+), 29 deletions(-) diff --git a/src/vs/workbench/contrib/accessibility/browser/accessibilityConfiguration.ts b/src/vs/workbench/contrib/accessibility/browser/accessibilityConfiguration.ts index b97be1b44df..c8272bc512d 100644 --- a/src/vs/workbench/contrib/accessibility/browser/accessibilityConfiguration.ts +++ b/src/vs/workbench/contrib/accessibility/browser/accessibilityConfiguration.ts @@ -14,6 +14,7 @@ export const accessibleViewIsShown = new RawContextKey('accessibleViewI export const accessibleViewSupportsNavigation = new RawContextKey('accessibleViewSupportsNavigation', false, true); export const accessibleViewVerbosityEnabled = new RawContextKey('accessibleViewVerbosityEnabled', false, true); export const accessibleViewGoToSymbolSupported = new RawContextKey('accessibleViewGoToSymbolSupported', false, true); +export const accessibleViewCurrentProviderId = new RawContextKey('accessibleViewCurrentProviderId', undefined, undefined); /** * Miscellaneous settings tagged with accessibility and implemented in the accessibility contrib but diff --git a/src/vs/workbench/contrib/accessibility/browser/accessibilityContributions.ts b/src/vs/workbench/contrib/accessibility/browser/accessibilityContributions.ts index 21ef08a3bd6..8b1095ce97d 100644 --- a/src/vs/workbench/contrib/accessibility/browser/accessibilityContributions.ts +++ b/src/vs/workbench/contrib/accessibility/browser/accessibilityContributions.ts @@ -301,13 +301,6 @@ export class InlineCompletionsAccessibleViewContribution extends Disposable { if (!ghostText) { return false; } - const accept = () => { - model.accept(editor).then(() => { - alert('Accepted'); - model.stop(); - editor.focus(); - }); - }; this._options.language = editor.getModel()?.getLanguageId() ?? undefined; accessibleViewService.show({ verbositySettingKey: AccessibilityVerbositySettingId.InlineCompletions, @@ -322,29 +315,12 @@ export class InlineCompletionsAccessibleViewContribution extends Disposable { previous() { model.previous().then(() => show()); }, - onKeyDown: (e) => { - if (e.ctrlKey && e.browserEvent.key === '/') { - accept(); - } - }, - actions: [ - { - id: 'inlineCompletions.accept', - label: localize('inlineCompletions.accept', "Accept Completion (Ctrl+/)"), - tooltip: localize('inlineCompletions.accept', "Accept Completion (Ctrl+/)"), - run: () => { - accept(); - }, - class: ThemeIcon.asClassName(Codicon.check), - enabled: true - } - ], options: this._options }); return true; - }; + }; ContextKeyExpr.and(InlineCompletionContextKeys.inlineSuggestionVisible); return show(); - }, ContextKeyExpr.and(InlineCompletionContextKeys.inlineSuggestionVisible, EditorContextKeys.focus, EditorContextKeys.hasCodeActionsProvider) + }, ) ); } diff --git a/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts b/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts index 71fcfb7a5a3..c48af34270d 100644 --- a/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts +++ b/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts @@ -32,7 +32,7 @@ import { IOpenerService } from 'vs/platform/opener/common/opener'; import { IPickerQuickAccessItem } from 'vs/platform/quickinput/browser/pickerQuickAccess'; import { IQuickInputService } from 'vs/platform/quickinput/common/quickInput'; import { AccessibilityCommandId } from 'vs/workbench/contrib/accessibility/common/accessibilityCommands'; -import { AccessibilityVerbositySettingId, accessibilityHelpIsShown, accessibleViewGoToSymbolSupported, accessibleViewIsShown, accessibleViewSupportsNavigation, accessibleViewVerbosityEnabled } from 'vs/workbench/contrib/accessibility/browser/accessibilityConfiguration'; +import { AccessibilityVerbositySettingId, accessibilityHelpIsShown, accessibleViewCurrentProviderId, accessibleViewGoToSymbolSupported, accessibleViewIsShown, accessibleViewSupportsNavigation, accessibleViewVerbosityEnabled } from 'vs/workbench/contrib/accessibility/browser/accessibilityConfiguration'; import { getSimpleEditorOptions } from 'vs/workbench/contrib/codeEditor/browser/simpleEditorOptions'; import { IAction } from 'vs/base/common/actions'; import { createAndFillInActionBarActions } from 'vs/platform/actions/browser/menuEntryActionViewItem'; @@ -101,6 +101,7 @@ class AccessibleView extends Disposable { private _accessibleViewSupportsNavigation: IContextKey; private _accessibleViewVerbosityEnabled: IContextKey; private _accessibleViewGoToSymbolSupported: IContextKey; + private _accessibleViewCurrentProviderId: IContextKey; get editorWidget() { return this._editorWidget; } private _editorContainer: HTMLElement; @@ -126,6 +127,7 @@ class AccessibleView extends Disposable { this._accessibleViewSupportsNavigation = accessibleViewSupportsNavigation.bindTo(this._contextKeyService); this._accessibleViewVerbosityEnabled = accessibleViewVerbosityEnabled.bindTo(this._contextKeyService); this._accessibleViewGoToSymbolSupported = accessibleViewGoToSymbolSupported.bindTo(this._contextKeyService); + this._accessibleViewCurrentProviderId = accessibleViewCurrentProviderId.bindTo(this._contextKeyService); this._editorContainer = document.createElement('div'); this._editorContainer.classList.add('accessible-view'); @@ -182,6 +184,7 @@ class AccessibleView extends Disposable { onHide: () => { if (!showAccessibleViewHelp) { this._currentProvider = undefined; + this._accessibleViewCurrentProviderId.reset(); } } }; @@ -291,6 +294,7 @@ class AccessibleView extends Disposable { if (!showAccessibleViewHelp) { // don't overwrite the current provider this._currentProvider = provider; + this._accessibleViewCurrentProviderId.set(provider.verbositySettingKey); } this._updateContextKeys(provider, true); const value = this._configurationService.getValue(provider.verbositySettingKey); diff --git a/src/vs/workbench/contrib/accessibility/browser/accessibleViewActions.ts b/src/vs/workbench/contrib/accessibility/browser/accessibleViewActions.ts index b9a3d3f141e..ab3b221030d 100644 --- a/src/vs/workbench/contrib/accessibility/browser/accessibleViewActions.ts +++ b/src/vs/workbench/contrib/accessibility/browser/accessibleViewActions.ts @@ -11,8 +11,11 @@ import { Action2, MenuId, registerAction2 } from 'vs/platform/actions/common/act import { ContextKeyExpr } from 'vs/platform/contextkey/common/contextkey'; import { KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegistry'; import { AccessibilityCommandId } from 'vs/workbench/contrib/accessibility/common/accessibilityCommands'; -import { accessibilityHelpIsShown, accessibleViewGoToSymbolSupported, accessibleViewIsShown, accessibleViewSupportsNavigation, accessibleViewVerbosityEnabled } from 'vs/workbench/contrib/accessibility/browser/accessibilityConfiguration'; +import { AccessibilityVerbositySettingId, accessibilityHelpIsShown, accessibleViewCurrentProviderId, accessibleViewGoToSymbolSupported, accessibleViewIsShown, accessibleViewSupportsNavigation, accessibleViewVerbosityEnabled } from 'vs/workbench/contrib/accessibility/browser/accessibilityConfiguration'; import { IAccessibleViewService } from 'vs/workbench/contrib/accessibility/browser/accessibleView'; +import { ICodeEditorService } from 'vs/editor/browser/services/codeEditorService'; +import { InlineCompletionsController } from 'vs/editor/contrib/inlineCompletions/browser/inlineCompletionsController'; +import { alert } from 'vs/base/browser/ui/aria/aria'; const accessibleViewMenu = { id: MenuId.AccessibleView, @@ -153,3 +156,44 @@ class AccessibleViewDisableHintAction extends Action2 { } } registerAction2(AccessibleViewDisableHintAction); + +class AccessibleViewAcceptInlineCompletionAction extends Action2 { + constructor() { + super({ + id: AccessibilityCommandId.AccessibleViewAcceptInlineCompletionAction, + precondition: ContextKeyExpr.and(accessibleViewIsShown, ContextKeyExpr.equals(accessibleViewCurrentProviderId.key, AccessibilityVerbositySettingId.InlineCompletions)), + keybinding: { + primary: KeyMod.CtrlCmd | KeyCode.Slash, + mac: { primary: KeyMod.WinCtrl | KeyCode.Slash }, + weight: KeybindingWeight.WorkbenchContrib + }, + icon: Codicon.check, + menu: [ + commandPalette, + { + id: MenuId.AccessibleView, + group: 'navigation', + order: 0, + when: ContextKeyExpr.and(accessibleViewIsShown, ContextKeyExpr.equals(accessibleViewCurrentProviderId.key, AccessibilityVerbositySettingId.InlineCompletions)) + }], + title: localize('editor.action.accessibleViewAcceptInlineCompletionAction', "Accept Inline Completion") + }); + } + async run(accessor: ServicesAccessor): Promise { + const codeEditorService = accessor.get(ICodeEditorService); + const editor = codeEditorService.getActiveCodeEditor() || codeEditorService.getFocusedCodeEditor(); + if (!editor) { + return; + } + const model = InlineCompletionsController.get(editor)?.model.get(); + const state = model?.state.get(); + if (!model || !state) { + return; + } + await model.accept(editor); + alert('Accepted'); + model.stop(); + editor.focus(); + } +} +registerAction2(AccessibleViewAcceptInlineCompletionAction); diff --git a/src/vs/workbench/contrib/accessibility/common/accessibilityCommands.ts b/src/vs/workbench/contrib/accessibility/common/accessibilityCommands.ts index 8475978456d..6e73600a925 100644 --- a/src/vs/workbench/contrib/accessibility/common/accessibilityCommands.ts +++ b/src/vs/workbench/contrib/accessibility/common/accessibilityCommands.ts @@ -9,5 +9,6 @@ export const enum AccessibilityCommandId { DisableVerbosityHint = 'editor.action.accessibleViewDisableHint', GoToSymbol = 'editor.action.accessibleViewGoToSymbol', ShowNext = 'editor.action.accessibleViewNext', - ShowPrevious = 'editor.action.accessibleViewPrevious' + ShowPrevious = 'editor.action.accessibleViewPrevious', + AccessibleViewAcceptInlineCompletionAction = 'editor.action.accessibleViewAcceptInlineCompletion' } From 61d996db162360862effe0a3574f3a4b1b5d2ee7 Mon Sep 17 00:00:00 2001 From: meganrogge Date: Fri, 18 Aug 2023 13:40:36 -0700 Subject: [PATCH 17/34] improve consistency --- .../contrib/accessibility/browser/accessibleViewActions.ts | 2 +- .../contrib/accessibility/common/accessibilityCommands.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/accessibility/browser/accessibleViewActions.ts b/src/vs/workbench/contrib/accessibility/browser/accessibleViewActions.ts index ab3b221030d..530917842cc 100644 --- a/src/vs/workbench/contrib/accessibility/browser/accessibleViewActions.ts +++ b/src/vs/workbench/contrib/accessibility/browser/accessibleViewActions.ts @@ -160,7 +160,7 @@ registerAction2(AccessibleViewDisableHintAction); class AccessibleViewAcceptInlineCompletionAction extends Action2 { constructor() { super({ - id: AccessibilityCommandId.AccessibleViewAcceptInlineCompletionAction, + id: AccessibilityCommandId.AccessibleViewAcceptInlineCompletion, precondition: ContextKeyExpr.and(accessibleViewIsShown, ContextKeyExpr.equals(accessibleViewCurrentProviderId.key, AccessibilityVerbositySettingId.InlineCompletions)), keybinding: { primary: KeyMod.CtrlCmd | KeyCode.Slash, diff --git a/src/vs/workbench/contrib/accessibility/common/accessibilityCommands.ts b/src/vs/workbench/contrib/accessibility/common/accessibilityCommands.ts index 6e73600a925..2fb8316bdc8 100644 --- a/src/vs/workbench/contrib/accessibility/common/accessibilityCommands.ts +++ b/src/vs/workbench/contrib/accessibility/common/accessibilityCommands.ts @@ -10,5 +10,5 @@ export const enum AccessibilityCommandId { GoToSymbol = 'editor.action.accessibleViewGoToSymbol', ShowNext = 'editor.action.accessibleViewNext', ShowPrevious = 'editor.action.accessibleViewPrevious', - AccessibleViewAcceptInlineCompletionAction = 'editor.action.accessibleViewAcceptInlineCompletion' + AccessibleViewAcceptInlineCompletion = 'editor.action.accessibleViewAcceptInlineCompletion' } From 6909c0b4701ffe69ba919734dc088e8802effbba Mon Sep 17 00:00:00 2001 From: meganrogge Date: Fri, 18 Aug 2023 13:43:20 -0700 Subject: [PATCH 18/34] use id --- .../workbench/contrib/accessibility/browser/accessibleView.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts b/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts index c48af34270d..16095809d8c 100644 --- a/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts +++ b/src/vs/workbench/contrib/accessibility/browser/accessibleView.ts @@ -294,7 +294,7 @@ class AccessibleView extends Disposable { if (!showAccessibleViewHelp) { // don't overwrite the current provider this._currentProvider = provider; - this._accessibleViewCurrentProviderId.set(provider.verbositySettingKey); + this._accessibleViewCurrentProviderId.set(provider.verbositySettingKey.replaceAll('accessibility.verbosity.', '')); } this._updateContextKeys(provider, true); const value = this._configurationService.getValue(provider.verbositySettingKey); From dc2bcb11be090c957b881ee986f28783ff61824c Mon Sep 17 00:00:00 2001 From: meganrogge Date: Fri, 18 Aug 2023 14:05:25 -0700 Subject: [PATCH 19/34] fix #190766 --- .../browser/accessibleViewActions.ts | 32 +++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/src/vs/workbench/contrib/accessibility/browser/accessibleViewActions.ts b/src/vs/workbench/contrib/accessibility/browser/accessibleViewActions.ts index b9a3d3f141e..8e58dbb60b0 100644 --- a/src/vs/workbench/contrib/accessibility/browser/accessibleViewActions.ts +++ b/src/vs/workbench/contrib/accessibility/browser/accessibleViewActions.ts @@ -33,7 +33,12 @@ class AccessibleViewNextAction extends Action2 { primary: KeyMod.Alt | KeyCode.BracketRight, weight: KeybindingWeight.WorkbenchContrib }, - menu: [commandPalette, accessibleViewMenu], + menu: [ + commandPalette, + { + ...accessibleViewMenu, + when: ContextKeyExpr.and(accessibleViewIsShown, accessibleViewSupportsNavigation), + }], icon: Codicon.chevronRight, title: localize('editor.action.accessibleViewNext', "Show Next in Accessible View") }); @@ -55,7 +60,13 @@ class AccessibleViewPreviousAction extends Action2 { weight: KeybindingWeight.WorkbenchContrib }, icon: Codicon.chevronLeft, - menu: [commandPalette, accessibleViewMenu], + menu: [ + commandPalette, + { + ...accessibleViewMenu, + when: ContextKeyExpr.and(accessibleViewIsShown, accessibleViewSupportsNavigation), + } + ], title: localize('editor.action.accessibleViewPrevious', "Show Previous in Accessible View") }); } @@ -76,7 +87,13 @@ class AccessibleViewGoToSymbolAction extends Action2 { weight: KeybindingWeight.WorkbenchContrib + 10 }, icon: Codicon.symbolField, - menu: [commandPalette, accessibleViewMenu], + menu: [ + commandPalette, + { + ...accessibleViewMenu, + when: ContextKeyExpr.and(accessibleViewIsShown, accessibleViewSupportsNavigation), + } + ], title: localize('editor.action.accessibleViewGoToSymbol', "Go To Symbol in Accessible View") }); } @@ -140,11 +157,14 @@ class AccessibleViewDisableHintAction extends Action2 { weight: KeybindingWeight.WorkbenchContrib }, icon: Codicon.treeFilterClear, - menu: [commandPalette, + menu: [ + commandPalette, { id: MenuId.AccessibleView, - group: 'navigation' - }], + group: 'navigation', + when: ContextKeyExpr.and(ContextKeyExpr.or(accessibleViewIsShown, accessibilityHelpIsShown), accessibleViewVerbosityEnabled), + } + ], title: localize('editor.action.accessibleViewDisableHint', "Disable Accessible View Hint") }); } From e988d9d71c1356d0f0c1576da7b58943ff6b496d Mon Sep 17 00:00:00 2001 From: meganrogge Date: Fri, 18 Aug 2023 14:09:12 -0700 Subject: [PATCH 20/34] fix #189974 --- .../contrib/accessibility/browser/accessibleViewActions.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/vs/workbench/contrib/accessibility/browser/accessibleViewActions.ts b/src/vs/workbench/contrib/accessibility/browser/accessibleViewActions.ts index 530917842cc..9ebe9bb817e 100644 --- a/src/vs/workbench/contrib/accessibility/browser/accessibleViewActions.ts +++ b/src/vs/workbench/contrib/accessibility/browser/accessibleViewActions.ts @@ -76,6 +76,7 @@ class AccessibleViewGoToSymbolAction extends Action2 { precondition: ContextKeyExpr.and(accessibleViewIsShown, accessibleViewGoToSymbolSupported), keybinding: { primary: KeyMod.CtrlCmd | KeyMod.Shift | KeyCode.KeyO, + secondary: [KeyMod.CtrlCmd | KeyMod.Shift | KeyCode.Period], weight: KeybindingWeight.WorkbenchContrib + 10 }, icon: Codicon.symbolField, From 1d9febc37eebedc3a44c86bb131becf5f30dd0f3 Mon Sep 17 00:00:00 2001 From: Joyce Er Date: Fri, 18 Aug 2023 14:15:12 -0700 Subject: [PATCH 21/34] Render file tree during progressive rendering (#190697) * Render file tree during progressive rendering --- .../contrib/chat/browser/chatListRenderer.ts | 125 +++++++++++++----- .../contrib/chat/common/chatViewModel.ts | 17 +-- 2 files changed, 98 insertions(+), 44 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatListRenderer.ts b/src/vs/workbench/contrib/chat/browser/chatListRenderer.ts index add3f8bafb7..8a367015e9e 100644 --- a/src/vs/workbench/contrib/chat/browser/chatListRenderer.ts +++ b/src/vs/workbench/contrib/chat/browser/chatListRenderer.ts @@ -63,7 +63,7 @@ import { ChatFollowups } from 'vs/workbench/contrib/chat/browser/chatFollowups'; import { ChatEditorOptions } from 'vs/workbench/contrib/chat/browser/chatOptions'; import { CONTEXT_REQUEST, CONTEXT_RESPONSE, CONTEXT_RESPONSE_HAS_PROVIDER_ID, CONTEXT_RESPONSE_VOTE } from 'vs/workbench/contrib/chat/common/chatContextKeys'; import { IChatReplyFollowup, IChatResponseProgressFileTreeData, IChatService, ISlashCommand, InteractiveSessionVoteDirection } from 'vs/workbench/contrib/chat/common/chatService'; -import { IChatResponseViewModel, IChatWelcomeMessageViewModel, isRequestVM, isResponseVM, isWelcomeVM } from 'vs/workbench/contrib/chat/common/chatViewModel'; +import { IChatResponseMarkdownRenderData, IChatResponseRenderData, IChatResponseViewModel, IChatWelcomeMessageViewModel, isRequestVM, isResponseVM, isWelcomeVM } from 'vs/workbench/contrib/chat/common/chatViewModel'; import { IWordCountResult, getNWords } from 'vs/workbench/contrib/chat/common/chatWordCounter'; import { MenuPreventer } from 'vs/workbench/contrib/codeEditor/browser/menuPreventer'; import { SelectionClipboardContributionID } from 'vs/workbench/contrib/codeEditor/browser/selectionClipboard'; @@ -103,7 +103,6 @@ export interface IChatListItemRendererOptions { } export class ChatListItemRenderer extends Disposable implements ITreeRenderer { - static readonly cursorCharacter = '\u258c'; static readonly ID = 'item'; private readonly codeBlocksByResponseId = new Map(); @@ -399,29 +398,92 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer { + const renderedPart = renderedParts[index]; + // Is this part completely new? + if (!renderedPart) { + if (isInteractiveProgressTreeData(part)) { + partsToRender[index] = part; + } else { + const wordCountResult = this.getDataForProgressiveRender(element, part); + if (wordCountResult !== undefined) { + partsToRender[index] = { + renderedWordCount: wordCountResult.actualWordCount, + lastRenderTime: Date.now(), + isFullyRendered: wordCountResult.isFullString, + }; + wordCountResults[index] = wordCountResult; + } + } + } + + // Did this part go from being a placeholder string to resolved tree data? + else if (isInteractiveProgressTreeData(part) && !isInteractiveProgressTreeData(renderedPart)) { + partsToRender[index] = part; + } + + // Did this part's content change? + else if (!isInteractiveProgressTreeData(part) && !isInteractiveProgressTreeData(renderedPart) && !renderedPart.isFullyRendered) { + const wordCountResult = this.getDataForProgressiveRender(element, part, renderedPart); + if (wordCountResult !== undefined) { + partsToRender[index] = { + renderedWordCount: wordCountResult.actualWordCount, + lastRenderTime: Date.now(), + isFullyRendered: wordCountResult.isFullString, + }; + wordCountResults[index] = wordCountResult; + } + } + }); + + isFullyRendered = partsToRender.length === 0; + if (isFullyRendered && element.isComplete) { // Response is done and content is rendered, so do a normal render this.traceLayout('runProgressiveRender', `end progressive render, index=${index} and clearing renderData, response is complete, index=${index}`); element.renderData = undefined; disposables.clear(); this.basicRenderElement(element.response.value, element, index, templateData); - } else if (renderValue) { - element.renderData = { - renderedWordCount: renderValue.actualWordCount, - lastRenderTime: Date.now(), - isFullyRendered: renderValue.isFullString - }; + } else if (!isFullyRendered) { + let hasRenderedOneMarkdownBlock = false; + partsToRender.forEach((partToRender, index) => { + if (!partToRender) { + return; + } - const plusCursor = (renderValue.value.match(/```\s*$/) ? - renderValue.value + '\n\n' : - renderValue.value) + ` ${ChatListItemRenderer.cursorCharacter}`; - const result = this.renderMarkdown(new MarkdownString(plusCursor), element, disposables, templateData, true); - // Doing the progressive render - dom.clearNode(templateData.value); - templateData.value.appendChild(result.element); - disposables.add(result); + let result; + if (isInteractiveProgressTreeData(partToRender)) { + result = this.renderTreeData(partToRender, element, disposables, templateData, index); + } + + // Avoid doing progressive rendering for multiple markdown parts simultaneously + else if (!hasRenderedOneMarkdownBlock) { + const value = wordCountResults[index].value; + result = this.renderMarkdown(new MarkdownString(value), element, disposables, templateData, true); + hasRenderedOneMarkdownBlock = true; + } + + if (!result) { + return; + } + + // Doing the progressive render + renderedParts[index] = partToRender; + const existingElement = templateData.value.children[index]; + if (existingElement) { + templateData.value.replaceChild(result.element, existingElement); + } else { + templateData.value.appendChild(result.element); + } + disposables.add(result); + }); } else { // Nothing new to render, not done, keep waiting return false; @@ -435,7 +497,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer void } { @@ -558,8 +620,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer, index: number, templateData: IChatListItemTemplate): void { @@ -886,28 +947,16 @@ class CodeBlockPart extends Disposable implements IChatResultCodeBlockPart { } private setText(newText: string): void { - let currentText = this.textModel.getLinesContent().join('\n'); + const currentText = this.textModel.getLinesContent().join('\n'); if (newText === currentText) { return; } - let removedChars = 0; - if (currentText.endsWith(` ${ChatListItemRenderer.cursorCharacter}`)) { - removedChars = 2; - } else if (currentText.endsWith(ChatListItemRenderer.cursorCharacter)) { - removedChars = 1; - } - - if (removedChars > 0) { - currentText = currentText.slice(0, currentText.length - removedChars); - } - if (newText.startsWith(currentText)) { const text = newText.slice(currentText.length); const lastLine = this.textModel.getLineCount(); const lastCol = this.textModel.getLineMaxColumn(lastLine); - const insertAtCol = lastCol - removedChars; - this.textModel.applyEdits([{ range: new Range(lastLine, insertAtCol, lastLine, lastCol), text }]); + this.textModel.applyEdits([{ range: new Range(lastLine, lastCol, lastLine, lastCol), text }]); } else { // console.log(`Failed to optimize setText`); this.textModel.setValue(newText); @@ -1133,3 +1182,7 @@ class ChatListTreeDataSource implements IAsyncDataSource acc += ('label' in part ? 0 : part.renderedWordCount), 0); if (!this.isComplete) { - this.trace('onDidChange', `Update- got ${wordCount} words over ${timeDiff}ms = ${impliedWordLoadRate} words/s. ${this.renderData?.renderedWordCount} words are rendered.`); + this.trace('onDidChange', `Update- got ${wordCount} words over ${timeDiff}ms = ${impliedWordLoadRate} words/s. ${renderedWordCount} words are rendered.`); this._contentUpdateTimings = { loadingStartTime: this._contentUpdateTimings!.loadingStartTime, lastUpdateTime: now, @@ -336,7 +341,7 @@ export class ChatResponseViewModel extends Disposable implements IChatResponseVi impliedWordLoadRate }; } else { - this.trace(`onDidChange`, `Done- got ${wordCount} words over ${timeDiff}ms = ${impliedWordLoadRate} words/s. ${this.renderData?.renderedWordCount} words are rendered.`); + this.trace(`onDidChange`, `Done- got ${wordCount} words over ${timeDiff}ms = ${impliedWordLoadRate} words/s. ${renderedWordCount} words are rendered.`); } } else { this.logService.warn('ChatResponseViewModel#onDidChange: got model update but contentUpdateTimings is not initialized'); @@ -344,10 +349,6 @@ export class ChatResponseViewModel extends Disposable implements IChatResponseVi // new data -> new id, new content to render this._modelChangeCount++; - if (this.renderData) { - this.renderData.isFullyRendered = false; - this.renderData.lastRenderTime = Date.now(); - } this._onDidChange.fire(); })); From f6c280baf84d9a6c6f9f5911d809f96a1df9ce3b Mon Sep 17 00:00:00 2001 From: meganrogge Date: Fri, 18 Aug 2023 14:16:17 -0700 Subject: [PATCH 22/34] fix #188755 --- .../contrib/accessibility/browser/accessibilityContributions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/accessibility/browser/accessibilityContributions.ts b/src/vs/workbench/contrib/accessibility/browser/accessibilityContributions.ts index 8b1095ce97d..3d6968b88c4 100644 --- a/src/vs/workbench/contrib/accessibility/browser/accessibilityContributions.ts +++ b/src/vs/workbench/contrib/accessibility/browser/accessibilityContributions.ts @@ -54,7 +54,7 @@ export class EditorAccessibilityHelpContribution extends Disposable { codeEditor = codeEditorService.getActiveCodeEditor()!; } accessibleViewService.show(instantiationService.createInstance(AccessibilityHelpProvider, codeEditor)); - })); + }, EditorContextKeys.focus)); } } From 41a5fc5945ad1f97173b85e643f906d73ddb729e Mon Sep 17 00:00:00 2001 From: meganrogge Date: Fri, 18 Aug 2023 14:37:03 -0700 Subject: [PATCH 23/34] fix #190718 --- .../accessibility/browser/accessibilityContributions.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/accessibility/browser/accessibilityContributions.ts b/src/vs/workbench/contrib/accessibility/browser/accessibilityContributions.ts index 3d6968b88c4..80cdebaab8e 100644 --- a/src/vs/workbench/contrib/accessibility/browser/accessibilityContributions.ts +++ b/src/vs/workbench/contrib/accessibility/browser/accessibilityContributions.ts @@ -282,6 +282,7 @@ export class InlineCompletionsAccessibleViewContribution extends Disposable { this._register(AccessibleViewAction.addImplementation(95, 'inline-completions', accessor => { const accessibleViewService = accessor.get(IAccessibleViewService); const codeEditorService = accessor.get(ICodeEditorService); + const contextViewService = accessor.get(IContextViewService); const show = () => { const editor = codeEditorService.getActiveCodeEditor() || codeEditorService.getFocusedCodeEditor(); if (!editor) { @@ -310,10 +311,12 @@ export class InlineCompletionsAccessibleViewContribution extends Disposable { editor.focus(); }, next() { - model.next().then(() => show()); + contextViewService.hideContextView(); + setTimeout(() => model.next().then(() => show()), 50); }, previous() { - model.previous().then(() => show()); + contextViewService.hideContextView(); + setTimeout(() => model.previous().then(() => show()), 50); }, options: this._options }); From 349c93df2a8e531d0353853ca08af968a121033f Mon Sep 17 00:00:00 2001 From: Raymond Zhao <7199958+rzhao271@users.noreply.github.com> Date: Fri, 18 Aug 2023 15:20:46 -0700 Subject: [PATCH 24/34] Add progressbar to Settings editor (#190708) --- .../preferences/browser/settingsEditor2.ts | 86 ++++++++++--------- 1 file changed, 44 insertions(+), 42 deletions(-) diff --git a/src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts b/src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts index 1dac2bd1e62..d0666ca8be5 100644 --- a/src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts +++ b/src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts @@ -66,6 +66,7 @@ import { IWorkbenchAssignmentService } from 'vs/workbench/services/assignment/co import { IProductService } from 'vs/platform/product/common/productService'; import { IEnvironmentService } from 'vs/platform/environment/common/environment'; import { registerNavigableContainer } from 'vs/workbench/browser/actions/widgetNavigationCommands'; +import { IEditorProgressService } from 'vs/platform/progress/common/progress'; export const enum SettingsFocusContext { @@ -237,6 +238,7 @@ export class SettingsEditor2 extends EditorPane { @IProductService private readonly productService: IProductService, @IEnvironmentService private readonly environmentService: IEnvironmentService, @IExtensionGalleryService private readonly extensionGalleryService: IExtensionGalleryService, + @IEditorProgressService private readonly editorProgressService: IEditorProgressService, ) { super(SettingsEditor2.ID, telemetryService, themeService, storageService); this.delayedFilterLogging = new Delayer(1000); @@ -566,6 +568,9 @@ export class SettingsEditor2 extends EditorPane { this.searchWidget.updateAriaLabel(label); } + /** + * Render the header of the Settings editor, which includes the content above the splitview. + */ private createHeader(parent: HTMLElement): void { this.headerContainer = DOM.append(parent, $('.settings-header')); @@ -1622,19 +1627,20 @@ export class SettingsEditor2 extends EditorPane { // Trigger the local search. If it didn't find an exact match, trigger the remote search. const searchInProgress = this.searchInProgress = new CancellationTokenSource(); - return this.localSearchDelayer.trigger(() => { + return this.localSearchDelayer.trigger(async () => { if (searchInProgress && !searchInProgress.token.isCancellationRequested) { - return this.localFilterPreferences(query).then(result => { - if (result && !result.exactMatch) { - this.remoteSearchThrottle.trigger(() => { - return searchInProgress && !searchInProgress.token.isCancellationRequested ? - this.remoteSearchPreferences(query, this.searchInProgress!.token) : - Promise.resolve(); - }); - } - }); - } else { - return Promise.resolve(); + const progressRunner = this.editorProgressService.show(true); + const result = await this.localFilterPreferences(query); + if (result && !result.exactMatch) { + this.remoteSearchThrottle.trigger(async () => { + if (searchInProgress && !searchInProgress.token.isCancellationRequested) { + await this.remoteSearchPreferences(query, this.searchInProgress!.token); + } + progressRunner.done(); + }); + } else { + progressRunner.done(); + } } }); } @@ -1654,36 +1660,32 @@ export class SettingsEditor2 extends EditorPane { ]).then(() => { }); } - private filterOrSearchPreferences(query: string, type: SearchResultIdx, searchProvider?: ISearchProvider, token?: CancellationToken): Promise { - return this._filterOrSearchPreferencesModel(query, this.defaultSettingsEditorModel, searchProvider, token).then(result => { - if (token && token.isCancellationRequested) { - // Handle cancellation like this because cancellation is lost inside the search provider due to async/await - return null; - } - - if (!this.searchResultModel) { - this.searchResultModel = this.instantiationService.createInstance(SearchResultModel, this.viewState, this.workspaceTrustManagementService.isWorkspaceTrusted()); - // Must be called before this.renderTree() - // to make sure the search results count is set. - this.searchResultModel.setResult(type, result); - this.tocTreeModel.currentSearchModel = this.searchResultModel; - this.onSearchModeToggled(); - } else { - this.searchResultModel.setResult(type, result); - this.tocTreeModel.update(); - } - - if (type === SearchResultIdx.Local) { - this.tocTree.setFocus([]); - this.viewState.filterToCategory = undefined; - this.tocTree.expandAll(); - } - - this.settingsTree.scrollTop = 0; - this.refreshTOCTree(); - this.renderTree(undefined, true); - return result; - }); + private async filterOrSearchPreferences(query: string, type: SearchResultIdx, searchProvider?: ISearchProvider, token?: CancellationToken): Promise { + const result = await this._filterOrSearchPreferencesModel(query, this.defaultSettingsEditorModel, searchProvider, token); + if (token?.isCancellationRequested) { + // Handle cancellation like this because cancellation is lost inside the search provider due to async/await + return null; + } + if (!this.searchResultModel) { + this.searchResultModel = this.instantiationService.createInstance(SearchResultModel, this.viewState, this.workspaceTrustManagementService.isWorkspaceTrusted()); + // Must be called before this.renderTree() + // to make sure the search results count is set. + this.searchResultModel.setResult(type, result); + this.tocTreeModel.currentSearchModel = this.searchResultModel; + this.onSearchModeToggled(); + } else { + this.searchResultModel.setResult(type, result); + this.tocTreeModel.update(); + } + if (type === SearchResultIdx.Local) { + this.tocTree.setFocus([]); + this.viewState.filterToCategory = undefined; + this.tocTree.expandAll(); + } + this.settingsTree.scrollTop = 0; + this.refreshTOCTree(); + this.renderTree(undefined, true); + return result; } private renderResultCountMessages() { From e92a7f0f6968abd596434b57a7647e36c1ed5eaf Mon Sep 17 00:00:00 2001 From: Tyler James Leonhardt Date: Fri, 18 Aug 2023 15:32:17 -0700 Subject: [PATCH 25/34] Remove titlebar and include side buttons in quick chat (#190780) This adds buttons to the right of the chat box. These buttons were previously on the quick pick's titlebar, but in order to have a smooth transition between Command Palette and Quick Chat we wanted to move away from the title bar. --- .../platform/quickinput/browser/quickInput.ts | 3 +- .../browser/actions/chatQuickInputActions.ts | 274 ++++-------------- .../contrib/chat/browser/chat.contribution.ts | 4 +- src/vs/workbench/contrib/chat/browser/chat.ts | 8 + .../contrib/chat/browser/chatQuick.ts | 209 +++++++++++++ 5 files changed, 281 insertions(+), 217 deletions(-) create mode 100644 src/vs/workbench/contrib/chat/browser/chatQuick.ts diff --git a/src/vs/platform/quickinput/browser/quickInput.ts b/src/vs/platform/quickinput/browser/quickInput.ts index 072f2f56e89..c56674f2f78 100644 --- a/src/vs/platform/quickinput/browser/quickInput.ts +++ b/src/vs/platform/quickinput/browser/quickInput.ts @@ -1264,8 +1264,7 @@ export class QuickWidget extends QuickInput implements IQuickWidget { const visibilities: Visibilities = { title: !!this.title || !!this.step || !!this.buttons.length, - description: !!this.description || !!this.step, - progressBar: true + description: !!this.description || !!this.step }; this.ui.setVisibilities(visibilities); diff --git a/src/vs/workbench/contrib/chat/browser/actions/chatQuickInputActions.ts b/src/vs/workbench/contrib/chat/browser/actions/chatQuickInputActions.ts index c644cc34fbe..31c6361beaa 100644 --- a/src/vs/workbench/contrib/chat/browser/actions/chatQuickInputActions.ts +++ b/src/vs/workbench/contrib/chat/browser/actions/chatQuickInputActions.ts @@ -3,32 +3,71 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import * as dom from 'vs/base/browser/dom'; -import { CancellationToken } from 'vs/base/common/cancellation'; import { Codicon } from 'vs/base/common/codicons'; import { KeyCode, KeyMod } from 'vs/base/common/keyCodes'; -import { Disposable, DisposableStore } from 'vs/base/common/lifecycle'; -import { ThemeIcon } from 'vs/base/common/themables'; import { localize } from 'vs/nls'; import { Action2, MenuId, registerAction2 } from 'vs/platform/actions/common/actions'; -import { ICommandService } from 'vs/platform/commands/common/commands'; -import { ContextKeyExpr, IContextKeyService, IScopedContextKeyService } from 'vs/platform/contextkey/common/contextkey'; -import { IInstantiationService, ServicesAccessor } from 'vs/platform/instantiation/common/instantiation'; -import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection'; +import { ContextKeyExpr } from 'vs/platform/contextkey/common/contextkey'; +import { ServicesAccessor } from 'vs/platform/instantiation/common/instantiation'; import { KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegistry'; -import { IQuickInputService, IQuickWidget } from 'vs/platform/quickinput/common/quickInput'; -import { editorBackground, editorForeground, inputBackground } from 'vs/platform/theme/common/colorRegistry'; import { CHAT_CATEGORY } from 'vs/workbench/contrib/chat/browser/actions/chatActions'; -import { IChatWidgetService } from 'vs/workbench/contrib/chat/browser/chat'; -import { IChatViewOptions } from 'vs/workbench/contrib/chat/browser/chatViewPane'; -import { ChatWidget } from 'vs/workbench/contrib/chat/browser/chatWidget'; +import { IQuickChatService } from 'vs/workbench/contrib/chat/browser/chat'; import { CONTEXT_PROVIDER_EXISTS } from 'vs/workbench/contrib/chat/common/chatContextKeys'; -import { ChatModel } from 'vs/workbench/contrib/chat/common/chatModel'; import { IChatService } from 'vs/workbench/contrib/chat/common/chatService'; -export const ASK_QUICK_QUESTION_ACTION_ID = 'chat.action.askQuickQuestion'; +export const ASK_QUICK_QUESTION_ACTION_ID = 'workbench.action.quickchat.toggle'; export function registerQuickChatActions() { registerAction2(QuickChatGlobalAction); + + registerAction2(class OpenInChatViewAction extends Action2 { + constructor() { + super({ + id: 'workbench.action.quickchat.openInChatView', + title: { + value: localize('chat.openInChatView.label', "Open in Chat View"), + original: 'Open in Chat View' + }, + f1: false, + category: CHAT_CATEGORY, + icon: Codicon.commentDiscussion, + menu: { + id: MenuId.ChatInputSide, + group: 'navigation', + order: 10 + } + }); + } + + run(accessor: ServicesAccessor) { + const quickChatService = accessor.get(IQuickChatService); + quickChatService.openInChatView(); + } + }); + + registerAction2(class CloseQuickChatAction extends Action2 { + constructor() { + super({ + id: 'workbench.action.quickchat.close', + title: { + value: localize('chat.closeQuickChat.label', "Close Quick Chat"), + original: 'Close Quick Chat' + }, + f1: false, + category: CHAT_CATEGORY, + icon: Codicon.close, + menu: { + id: MenuId.ChatInputSide, + group: 'navigation', + order: 20 + } + }); + } + + run(accessor: ServicesAccessor) { + const quickChatService = accessor.get(IQuickChatService); + quickChatService.close(); + } + }); } class QuickChatGlobalAction extends Action2 { @@ -56,13 +95,13 @@ class QuickChatGlobalAction extends Action2 { }); } - override async run(accessor: ServicesAccessor, query: string): Promise { + override run(accessor: ServicesAccessor, query?: string): void { const chatService = accessor.get(IChatService); - const commandService = accessor.get(ICommandService); + const quickChatService = accessor.get(IQuickChatService); // Grab the first provider and run its command const info = chatService.getProviderInfos()[0]; if (info) { - await commandService.executeCommand(`workbench.action.openQuickChat.${info.id}`, query); + quickChatService.toggle(info.id, query); } } } @@ -77,10 +116,6 @@ class QuickChatGlobalAction extends Action2 { */ export function getQuickChatActionForProvider(id: string, label: string) { return class AskQuickChatAction extends Action2 { - _currentTimer: any | undefined; - _input: IQuickWidget | undefined; - _currentChat: QuickChat | undefined; - constructor() { super({ id: `workbench.action.openQuickChat.${id}`, @@ -90,198 +125,9 @@ export function getQuickChatActionForProvider(id: string, label: string) { }); } - override run(accessor: ServicesAccessor, query: string): void { - const quickInputService = accessor.get(IQuickInputService); - const chatService = accessor.get(IChatService); - const instantiationService = accessor.get(IInstantiationService); - - // First things first, clear the existing timer that will dispose the session - clearTimeout(this._currentTimer); - this._currentTimer = undefined; - - // If the input is already shown, hide it. This provides a toggle behavior of the quick pick - if (this._input !== undefined) { - this._input.hide(); - return; - } - - // Check if any providers are available. If not, show nothing - // This shouldn't be needed because of the precondition, but just in case - const providerInfo = chatService.getProviderInfos()[0]; - if (!providerInfo) { - return; - } - - const disposableStore = new DisposableStore(); - - //#region Setup quick pick - - this._input = quickInputService.createQuickWidget(); - disposableStore.add(this._input); - - const containerSession = dom.$('.interactive-session'); - this._input.widget = containerSession; - - this._currentChat ??= instantiationService.createInstance(QuickChat, { - providerId: providerInfo.id, - }); - // show needs to come before the current chat rendering - this._input.show(); - this._currentChat.render(containerSession); - - const clearButton = { - iconClass: ThemeIcon.asClassName(Codicon.clearAll), - tooltip: localize('clear', "Clear"), - }; - this._input.buttons = [ - clearButton, - { - iconClass: ThemeIcon.asClassName(Codicon.commentDiscussion), - tooltip: localize('openInChat', "Open In Chat View"), - } - ]; - this._input.title = providerInfo.displayName; - - disposableStore.add(this._input.onDidHide(() => { - disposableStore.dispose(); - this._input = undefined; - this._currentTimer = setTimeout(() => { - this._currentChat?.dispose(); - this._currentChat = undefined; - }, 1000 * 30); // 30 seconds - })); - - disposableStore.add(this._input.onDidTriggerButton((e) => { - if (e === clearButton) { - this._currentChat?.clear(); - } else { - this._currentChat?.openChatView(); - } - })); - - //#endregion - - this._currentChat.focus(); - - if (query) { - this._currentChat.setValue(query); - this._currentChat.acceptInput(); - } + override run(accessor: ServicesAccessor, query?: string): void { + const quickChatService = accessor.get(IQuickChatService); + quickChatService.toggle(id, query); } }; } - -class QuickChat extends Disposable { - private widget!: ChatWidget; - private model: ChatModel | undefined; - private _currentQuery: string | undefined; - - private _scopedContextKeyService!: IScopedContextKeyService; - get scopedContextKeyService() { - return this._scopedContextKeyService; - } - - constructor( - private readonly _options: IChatViewOptions, - @IInstantiationService private readonly instantiationService: IInstantiationService, - @IContextKeyService private readonly contextKeyService: IContextKeyService, - @IChatService private readonly chatService: IChatService, - @IChatWidgetService private readonly _chatWidgetService: IChatWidgetService - ) { - super(); - } - - clear() { - this.model?.dispose(); - this.model = undefined; - this.updateModel(); - this.widget.inputEditor.setValue(''); - } - - focus(): void { - if (this.widget) { - this.widget.focusInput(); - } - } - - render(parent: HTMLElement): void { - this._scopedContextKeyService?.dispose(); - this._scopedContextKeyService = this._register(this.contextKeyService.createScoped(parent)); - const scopedInstantiationService = this.instantiationService.createChild(new ServiceCollection([IContextKeyService, this.scopedContextKeyService])); - this.widget?.dispose(); - this.widget = this._register( - scopedInstantiationService.createInstance( - ChatWidget, - { resource: true, renderInputOnTop: true, renderStyle: 'compact' }, - { - listForeground: editorForeground, - listBackground: editorBackground, - inputEditorBackground: inputBackground, - resultEditorBackground: editorBackground - })); - this.widget.render(parent); - this.widget.setVisible(true); - this.widget.setDynamicChatTreeItemLayout(2, 600); - this.updateModel(); - if (this._currentQuery) { - this.widget.inputEditor.setSelection({ - startLineNumber: 1, - startColumn: 1, - endLineNumber: 1, - endColumn: this._currentQuery.length + 1 - }); - } - - this.registerListeners(); - } - - private registerListeners(): void { - this._register(this.widget.inputEditor.onDidChangeModelContent((e) => { - this._currentQuery = this.widget.inputEditor.getValue(); - })); - this._register(this.widget.onDidClear(() => this.clear())); - } - - async acceptInput(): Promise { - return this.widget.acceptInput(); - } - - async openChatView(): Promise { - const widget = await this._chatWidgetService.revealViewForProvider(this._options.providerId); - if (!widget?.viewModel || !this.model) { - return; - } - - for (const request of this.model.getRequests()) { - if (request.response?.response.value || request.response?.errorDetails) { - this.chatService.addCompleteRequest(widget.viewModel.sessionId, - request.message as string, - { - message: request.response.response.asString(), - errorDetails: request.response.errorDetails - }); - } else if (request.message) { - - } - } - - const value = this.widget.inputEditor.getValue(); - if (value) { - widget.inputEditor.setValue(value); - } - widget.focusInput(); - } - - setValue(value: string): void { - this.widget.inputEditor.setValue(value); - } - - private updateModel(): void { - this.model ??= this.chatService.startSession(this._options.providerId, CancellationToken.None); - if (!this.model) { - throw new Error('Could not start chat session'); - } - - this.widget.setModel(this.model, { inputValue: this._currentQuery }); - } -} diff --git a/src/vs/workbench/contrib/chat/browser/chat.contribution.ts b/src/vs/workbench/contrib/chat/browser/chat.contribution.ts index 01068dfc64f..09d6c46267b 100644 --- a/src/vs/workbench/contrib/chat/browser/chat.contribution.ts +++ b/src/vs/workbench/contrib/chat/browser/chat.contribution.ts @@ -22,7 +22,7 @@ import { registerChatExecuteActions } from 'vs/workbench/contrib/chat/browser/ac import { registerQuickChatActions } from 'vs/workbench/contrib/chat/browser/actions/chatQuickInputActions'; import { registerChatTitleActions } from 'vs/workbench/contrib/chat/browser/actions/chatTitleActions'; import { registerChatExportActions } from 'vs/workbench/contrib/chat/browser/actions/chatImportExport'; -import { IChatAccessibilityService, IChatWidget, IChatWidgetService } from 'vs/workbench/contrib/chat/browser/chat'; +import { IChatAccessibilityService, IChatWidget, IChatWidgetService, IQuickChatService } from 'vs/workbench/contrib/chat/browser/chat'; import { ChatContributionService } from 'vs/workbench/contrib/chat/browser/chatContributionServiceImpl'; import { ChatEditor, IChatEditorOptions } from 'vs/workbench/contrib/chat/browser/chatEditor'; import { ChatEditorInput, ChatEditorInputSerializer } from 'vs/workbench/contrib/chat/browser/chatEditorInput'; @@ -52,6 +52,7 @@ import { AccessibleViewAction } from 'vs/workbench/contrib/accessibility/browser import { ICommandService } from 'vs/platform/commands/common/commands'; import { ChatVariablesService, IChatVariablesService } from 'vs/workbench/contrib/chat/common/chatVariables'; import { registerChatFileTreeActions } from 'vs/workbench/contrib/chat/browser/actions/chatFileTreeActions'; +import { QuickChatService } from 'vs/workbench/contrib/chat/browser/chatQuick'; // Register configuration const configurationRegistry = Registry.as(ConfigurationExtensions.Configuration); @@ -258,6 +259,7 @@ registerClearActions(); registerSingleton(IChatService, ChatService, InstantiationType.Delayed); registerSingleton(IChatContributionService, ChatContributionService, InstantiationType.Delayed); registerSingleton(IChatWidgetService, ChatWidgetService, InstantiationType.Delayed); +registerSingleton(IQuickChatService, QuickChatService, InstantiationType.Delayed); registerSingleton(IChatAccessibilityService, ChatAccessibilityService, InstantiationType.Delayed); registerSingleton(IChatWidgetHistoryService, ChatWidgetHistoryService, InstantiationType.Delayed); registerSingleton(IChatProviderService, ChatProviderService, InstantiationType.Delayed); diff --git a/src/vs/workbench/contrib/chat/browser/chat.ts b/src/vs/workbench/contrib/chat/browser/chat.ts index 59e1f4181ef..8e1c6ee182c 100644 --- a/src/vs/workbench/contrib/chat/browser/chat.ts +++ b/src/vs/workbench/contrib/chat/browser/chat.ts @@ -11,6 +11,7 @@ import { URI } from 'vs/base/common/uri'; import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; export const IChatWidgetService = createDecorator('chatWidgetService'); +export const IQuickChatService = createDecorator('quickChatService'); export const IChatAccessibilityService = createDecorator('chatAccessibilityService'); export interface IChatWidgetService { @@ -32,6 +33,13 @@ export interface IChatWidgetService { getWidgetBySessionId(sessionId: string): IChatWidget | undefined; } +export interface IQuickChatService { + readonly _serviceBrand: undefined; + toggle(providerId: string, query?: string): void; + focus(): void; + close(): void; + openInChatView(): void; +} export interface IChatAccessibilityService { readonly _serviceBrand: undefined; diff --git a/src/vs/workbench/contrib/chat/browser/chatQuick.ts b/src/vs/workbench/contrib/chat/browser/chatQuick.ts new file mode 100644 index 00000000000..ec90707236b --- /dev/null +++ b/src/vs/workbench/contrib/chat/browser/chatQuick.ts @@ -0,0 +1,209 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as dom from 'vs/base/browser/dom'; +import { CancellationToken } from 'vs/base/common/cancellation'; +import { Disposable, DisposableStore } from 'vs/base/common/lifecycle'; +import { IContextKeyService, IScopedContextKeyService } from 'vs/platform/contextkey/common/contextkey'; +import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; +import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection'; +import { IQuickInputService, IQuickWidget } from 'vs/platform/quickinput/common/quickInput'; +import { editorBackground, editorForeground, inputBackground } from 'vs/platform/theme/common/colorRegistry'; +import { IChatWidgetService, IQuickChatService } from 'vs/workbench/contrib/chat/browser/chat'; +import { IChatViewOptions } from 'vs/workbench/contrib/chat/browser/chatViewPane'; +import { ChatWidget } from 'vs/workbench/contrib/chat/browser/chatWidget'; +import { ChatModel } from 'vs/workbench/contrib/chat/common/chatModel'; +import { IChatService } from 'vs/workbench/contrib/chat/common/chatService'; + +export class QuickChatService implements IQuickChatService { + readonly _serviceBrand: undefined; + + _input: IQuickWidget | undefined; + _currentChat: QuickChat | undefined; + + constructor( + @IQuickInputService private readonly quickInputService: IQuickInputService, + @IChatService private readonly chatService: IChatService, + @IInstantiationService private readonly instantiationService: IInstantiationService, + ) { } + + get focused(): boolean { + const widget = this._input?.widget as HTMLElement; + if (!widget) { + return false; + } + return dom.isAncestor(document.activeElement, widget); + } + + toggle(providerId: string, query?: string | undefined): void { + // If the input is already shown, hide it. This provides a toggle behavior of the quick pick + if (this.focused) { + this.close(); + return; + } + + // Check if any providers are available. If not, show nothing + // This shouldn't be needed because of the precondition, but just in case + const providerInfo = this.chatService.getProviderInfos().find(info => info.id === providerId); + if (!providerInfo) { + return; + } + + const disposableStore = new DisposableStore(); + + this._input = this.quickInputService.createQuickWidget(); + this._input.contextKey = 'chatInputVisible'; + this._input.ignoreFocusOut = true; + disposableStore.add(this._input); + + const containerSession = dom.$('.interactive-session'); + this._input.widget = containerSession; + + this._currentChat ??= this.instantiationService.createInstance(QuickChat, { + providerId: providerInfo.id, + }); + + // show needs to come before the current chat rendering + this._input.show(); + this._currentChat.render(containerSession); + + disposableStore.add(this._input.onDidHide(() => { + disposableStore.dispose(); + this._input = undefined; + })); + + this._currentChat.focus(); + + if (query) { + this._currentChat.setValue(query); + this._currentChat.acceptInput(); + } + } + focus(): void { + this._currentChat?.focus(); + } + close(): void { + this._input?.dispose(); + } + async openInChatView(): Promise { + await this._currentChat?.openChatView(); + this.close(); + } +} + +class QuickChat extends Disposable { + private widget!: ChatWidget; + private model: ChatModel | undefined; + private _currentQuery: string | undefined; + + private _scopedContextKeyService!: IScopedContextKeyService; + get scopedContextKeyService() { + return this._scopedContextKeyService; + } + + constructor( + private readonly _options: IChatViewOptions, + @IInstantiationService private readonly instantiationService: IInstantiationService, + @IContextKeyService private readonly contextKeyService: IContextKeyService, + @IChatService private readonly chatService: IChatService, + @IChatWidgetService private readonly _chatWidgetService: IChatWidgetService + ) { + super(); + } + + clear() { + this.model?.dispose(); + this.model = undefined; + this.updateModel(); + this.widget.inputEditor.setValue(''); + } + + focus(): void { + if (this.widget) { + this.widget.focusInput(); + } + } + + render(parent: HTMLElement): void { + this._scopedContextKeyService?.dispose(); + this._scopedContextKeyService = this._register(this.contextKeyService.createScoped(parent)); + const scopedInstantiationService = this.instantiationService.createChild(new ServiceCollection([IContextKeyService, this.scopedContextKeyService])); + this.widget?.dispose(); + this.widget = this._register( + scopedInstantiationService.createInstance( + ChatWidget, + { resource: true, renderInputOnTop: true, renderStyle: 'compact' }, + { + listForeground: editorForeground, + listBackground: editorBackground, + inputEditorBackground: inputBackground, + resultEditorBackground: editorBackground + })); + this.widget.render(parent); + this.widget.setVisible(true); + this.widget.setDynamicChatTreeItemLayout(2, 600); + this.updateModel(); + if (this._currentQuery) { + this.widget.inputEditor.setSelection({ + startLineNumber: 1, + startColumn: 1, + endLineNumber: 1, + endColumn: this._currentQuery.length + 1 + }); + } + + this.registerListeners(); + } + + private registerListeners(): void { + this._register(this.widget.inputEditor.onDidChangeModelContent((e) => { + this._currentQuery = this.widget.inputEditor.getValue(); + })); + this._register(this.widget.onDidClear(() => this.clear())); + } + + async acceptInput(): Promise { + return this.widget.acceptInput(); + } + + async openChatView(): Promise { + const widget = await this._chatWidgetService.revealViewForProvider(this._options.providerId); + if (!widget?.viewModel || !this.model) { + return; + } + + for (const request of this.model.getRequests()) { + if (request.response?.response.value || request.response?.errorDetails) { + this.chatService.addCompleteRequest(widget.viewModel.sessionId, + request.message as string, + { + message: request.response.response.asString(), + errorDetails: request.response.errorDetails + }); + } else if (request.message) { + + } + } + + const value = this.widget.inputEditor.getValue(); + if (value) { + widget.inputEditor.setValue(value); + } + widget.focusInput(); + } + + setValue(value: string): void { + this.widget.inputEditor.setValue(value); + } + + private updateModel(): void { + this.model ??= this.chatService.startSession(this._options.providerId, CancellationToken.None); + if (!this.model) { + throw new Error('Could not start chat session'); + } + + this.widget.setModel(this.model, { inputValue: this._currentQuery }); + } +} From 6562544dc3cf77eff45113e17162fa2dc60b8dff Mon Sep 17 00:00:00 2001 From: Tyler James Leonhardt Date: Fri, 18 Aug 2023 16:03:52 -0700 Subject: [PATCH 26/34] Don't use `remoteCredentialsService` when client specified a `secretStorageProvider` (#190781) * Don't use `remoteCredentialsService` when client specified a `secretStorageProvider` This is a temporary fix for https://github.com/microsoft/vscode/issues/190537 until we clean up the last of the CredentialsService (which should be in debt week in Sept). If the client embedder declared a secretStorageProvider, they likely don't want the old credentialsProvider to go over the remote connection... so we use an in-memory provider instead. Fixed https://github.com/microsoft/vscode/issues/190537 * remove stale comment --- .../services/credentials/browser/credentialsService.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/vs/workbench/services/credentials/browser/credentialsService.ts b/src/vs/workbench/services/credentials/browser/credentialsService.ts index 2ee077e32a2..80a551f4cc3 100644 --- a/src/vs/workbench/services/credentials/browser/credentialsService.ts +++ b/src/vs/workbench/services/credentials/browser/credentialsService.ts @@ -30,15 +30,18 @@ export class BrowserCredentialsService extends Disposable implements ICredential ) { super(); - if (environmentService.remoteAuthority && !environmentService.options?.credentialsProvider) { + if ( + environmentService.remoteAuthority + && !environmentService.options?.credentialsProvider + && !environmentService.options?.secretStorageProvider + ) { // If we have a remote authority but the embedder didn't provide a credentialsProvider, // we can use the CredentialsService on the remote side const remoteCredentialsService = ProxyChannel.toService(remoteAgentService.getConnection()!.getChannel('credentials')); this.credentialsProvider = remoteCredentialsService; this._secretStoragePrefix = remoteCredentialsService.getSecretStoragePrefix(); } else { - // fall back to InMemoryCredentialsProvider if none was given to us. This should really only be used - // when running tests. + // fall back to InMemoryCredentialsProvider if none was given to us. this.credentialsProvider = environmentService.options?.credentialsProvider ?? new InMemoryCredentialsProvider(); this._secretStoragePrefix = Promise.resolve(this.productService.urlProtocol); } From b2a482303cdb82bbc14a9677cf5e5caaf8e07646 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 18 Aug 2023 16:38:52 -0700 Subject: [PATCH 27/34] xterm@5.3.0-beta.46 --- package.json | 6 +++--- remote/package.json | 6 +++--- remote/web/package.json | 4 ++-- remote/web/yarn.lock | 16 ++++++++-------- remote/yarn.lock | 24 ++++++++++++------------ yarn.lock | 24 ++++++++++++------------ 6 files changed, 40 insertions(+), 40 deletions(-) diff --git a/package.json b/package.json index 97143500048..7c5d9cb3444 100644 --- a/package.json +++ b/package.json @@ -95,14 +95,14 @@ "vscode-oniguruma": "1.7.0", "vscode-regexpp": "^3.1.0", "vscode-textmate": "9.0.0", - "xterm": "5.3.0-beta.42", + "xterm": "5.3.0-beta.46", "xterm-addon-canvas": "0.5.0-beta.9", "xterm-addon-image": "0.6.0-beta.2", "xterm-addon-search": "0.13.0-beta.8", "xterm-addon-serialize": "0.11.0-beta.8", "xterm-addon-unicode11": "0.5.0", - "xterm-addon-webgl": "0.16.0-beta.14", - "xterm-headless": "5.3.0-beta.42", + "xterm-addon-webgl": "0.16.0-beta.16", + "xterm-headless": "5.3.0-beta.46", "yauzl": "^2.9.2", "yazl": "^2.4.3" }, diff --git a/remote/package.json b/remote/package.json index 0edbe410766..28cf60a0252 100644 --- a/remote/package.json +++ b/remote/package.json @@ -27,14 +27,14 @@ "vscode-oniguruma": "1.7.0", "vscode-regexpp": "^3.1.0", "vscode-textmate": "9.0.0", - "xterm": "5.3.0-beta.42", + "xterm": "5.3.0-beta.46", "xterm-addon-canvas": "0.5.0-beta.9", "xterm-addon-image": "0.6.0-beta.2", "xterm-addon-search": "0.13.0-beta.8", "xterm-addon-serialize": "0.11.0-beta.8", "xterm-addon-unicode11": "0.5.0", - "xterm-addon-webgl": "0.16.0-beta.14", - "xterm-headless": "5.3.0-beta.42", + "xterm-addon-webgl": "0.16.0-beta.16", + "xterm-headless": "5.3.0-beta.46", "yauzl": "^2.9.2", "yazl": "^2.4.3" } diff --git a/remote/web/package.json b/remote/web/package.json index 9c1b107dd78..bd22a24243a 100644 --- a/remote/web/package.json +++ b/remote/web/package.json @@ -11,11 +11,11 @@ "tas-client-umd": "0.1.8", "vscode-oniguruma": "1.7.0", "vscode-textmate": "9.0.0", - "xterm": "5.3.0-beta.42", + "xterm": "5.3.0-beta.46", "xterm-addon-canvas": "0.5.0-beta.9", "xterm-addon-image": "0.6.0-beta.2", "xterm-addon-search": "0.13.0-beta.8", "xterm-addon-unicode11": "0.5.0", - "xterm-addon-webgl": "0.16.0-beta.14" + "xterm-addon-webgl": "0.16.0-beta.16" } } diff --git a/remote/web/yarn.lock b/remote/web/yarn.lock index a5a1fd6c1ee..b57b5aa165d 100644 --- a/remote/web/yarn.lock +++ b/remote/web/yarn.lock @@ -88,12 +88,12 @@ xterm-addon-unicode11@0.5.0: resolved "https://registry.yarnpkg.com/xterm-addon-unicode11/-/xterm-addon-unicode11-0.5.0.tgz#41c0d96acc1e3bb6c6596eee64e163b6bca74be7" integrity sha512-Jm4/g4QiTxiKiTbYICQgC791ubhIZyoIwxAIgOW8z8HWFNY+lwk+dwaKEaEeGBfM48Vk8fklsUW9u/PlenYEBg== -xterm-addon-webgl@0.16.0-beta.14: - version "0.16.0-beta.14" - resolved "https://registry.yarnpkg.com/xterm-addon-webgl/-/xterm-addon-webgl-0.16.0-beta.14.tgz#de5bf6c97e31f62d13907a875ffc6be154bc4a02" - integrity sha512-cj+rQWqOeZYJ6JMjMuTTKu3OnP+1D+sqSZPuwZAd6s4NkY4dFXFQlKxJ2yvDHOU/jrgpIYR1c3Cnu0AOWZpS3w== +xterm-addon-webgl@0.16.0-beta.16: + version "0.16.0-beta.16" + resolved "https://registry.yarnpkg.com/xterm-addon-webgl/-/xterm-addon-webgl-0.16.0-beta.16.tgz#2c31308f8c7f636576720adca529297f8fff3224" + integrity sha512-k0ZSwpBVtxXaqUc29CmlfFgIK7LEYAmwcDC3QVp2ESZo2JHYuQrWFpImjbzbYu0ON6qqcfbX8SnbzRid/XfGUg== -xterm@5.3.0-beta.42: - version "5.3.0-beta.42" - resolved "https://registry.yarnpkg.com/xterm/-/xterm-5.3.0-beta.42.tgz#f0806ebe9b033530c54ac5b03955827fb729c38f" - integrity sha512-3OLXTfwtSy9UKo8Uzm3/kd7qBg0JD/qFpotg8iKJ2aB9zx0fkJnysl4aIuNleUJzgvuxzBUHkcRZUmEw1XCKdw== +xterm@5.3.0-beta.46: + version "5.3.0-beta.46" + resolved "https://registry.yarnpkg.com/xterm/-/xterm-5.3.0-beta.46.tgz#9bd2b2a588c88ae64f1989ca3bf5bd9116554b5a" + integrity sha512-keB3C6sXm56ug1+hylAQiNyZP4YtPgd2W+Gu/ZKwJMk5haD25DEnenBYL++Cl3+sNGYxOGS42QlmYj6Mrq1LPw== diff --git a/remote/yarn.lock b/remote/yarn.lock index 15f25782773..e222314587d 100644 --- a/remote/yarn.lock +++ b/remote/yarn.lock @@ -902,20 +902,20 @@ xterm-addon-unicode11@0.5.0: resolved "https://registry.yarnpkg.com/xterm-addon-unicode11/-/xterm-addon-unicode11-0.5.0.tgz#41c0d96acc1e3bb6c6596eee64e163b6bca74be7" integrity sha512-Jm4/g4QiTxiKiTbYICQgC791ubhIZyoIwxAIgOW8z8HWFNY+lwk+dwaKEaEeGBfM48Vk8fklsUW9u/PlenYEBg== -xterm-addon-webgl@0.16.0-beta.14: - version "0.16.0-beta.14" - resolved "https://registry.yarnpkg.com/xterm-addon-webgl/-/xterm-addon-webgl-0.16.0-beta.14.tgz#de5bf6c97e31f62d13907a875ffc6be154bc4a02" - integrity sha512-cj+rQWqOeZYJ6JMjMuTTKu3OnP+1D+sqSZPuwZAd6s4NkY4dFXFQlKxJ2yvDHOU/jrgpIYR1c3Cnu0AOWZpS3w== +xterm-addon-webgl@0.16.0-beta.16: + version "0.16.0-beta.16" + resolved "https://registry.yarnpkg.com/xterm-addon-webgl/-/xterm-addon-webgl-0.16.0-beta.16.tgz#2c31308f8c7f636576720adca529297f8fff3224" + integrity sha512-k0ZSwpBVtxXaqUc29CmlfFgIK7LEYAmwcDC3QVp2ESZo2JHYuQrWFpImjbzbYu0ON6qqcfbX8SnbzRid/XfGUg== -xterm-headless@5.3.0-beta.42: - version "5.3.0-beta.42" - resolved "https://registry.yarnpkg.com/xterm-headless/-/xterm-headless-5.3.0-beta.42.tgz#714d167dc0af010067420956ffca80149dddeca9" - integrity sha512-3xhyy9DaPpadd1iiuKjOn4Zvntnx8WSBMEXPJbrORmy3ZbIwmLf/HYvg7iZfVreSDH+UFlm06uq3ictXpv7YvA== +xterm-headless@5.3.0-beta.46: + version "5.3.0-beta.46" + resolved "https://registry.yarnpkg.com/xterm-headless/-/xterm-headless-5.3.0-beta.46.tgz#3f941f673e3c61aad2705e4b34b42fbe467aed48" + integrity sha512-e/VbZKrfyD1TTxOY5/jah3dUih6q8nTO5v5GQV7YOASp+yyI0XHKiDjLrCyFPQOLTKxCcRutMHvr+CoCcPpcHg== -xterm@5.3.0-beta.42: - version "5.3.0-beta.42" - resolved "https://registry.yarnpkg.com/xterm/-/xterm-5.3.0-beta.42.tgz#f0806ebe9b033530c54ac5b03955827fb729c38f" - integrity sha512-3OLXTfwtSy9UKo8Uzm3/kd7qBg0JD/qFpotg8iKJ2aB9zx0fkJnysl4aIuNleUJzgvuxzBUHkcRZUmEw1XCKdw== +xterm@5.3.0-beta.46: + version "5.3.0-beta.46" + resolved "https://registry.yarnpkg.com/xterm/-/xterm-5.3.0-beta.46.tgz#9bd2b2a588c88ae64f1989ca3bf5bd9116554b5a" + integrity sha512-keB3C6sXm56ug1+hylAQiNyZP4YtPgd2W+Gu/ZKwJMk5haD25DEnenBYL++Cl3+sNGYxOGS42QlmYj6Mrq1LPw== yallist@^4.0.0: version "4.0.0" diff --git a/yarn.lock b/yarn.lock index 17b3729bc6a..b2b8a437cc0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10784,20 +10784,20 @@ xterm-addon-unicode11@0.5.0: resolved "https://registry.yarnpkg.com/xterm-addon-unicode11/-/xterm-addon-unicode11-0.5.0.tgz#41c0d96acc1e3bb6c6596eee64e163b6bca74be7" integrity sha512-Jm4/g4QiTxiKiTbYICQgC791ubhIZyoIwxAIgOW8z8HWFNY+lwk+dwaKEaEeGBfM48Vk8fklsUW9u/PlenYEBg== -xterm-addon-webgl@0.16.0-beta.14: - version "0.16.0-beta.14" - resolved "https://registry.yarnpkg.com/xterm-addon-webgl/-/xterm-addon-webgl-0.16.0-beta.14.tgz#de5bf6c97e31f62d13907a875ffc6be154bc4a02" - integrity sha512-cj+rQWqOeZYJ6JMjMuTTKu3OnP+1D+sqSZPuwZAd6s4NkY4dFXFQlKxJ2yvDHOU/jrgpIYR1c3Cnu0AOWZpS3w== +xterm-addon-webgl@0.16.0-beta.16: + version "0.16.0-beta.16" + resolved "https://registry.yarnpkg.com/xterm-addon-webgl/-/xterm-addon-webgl-0.16.0-beta.16.tgz#2c31308f8c7f636576720adca529297f8fff3224" + integrity sha512-k0ZSwpBVtxXaqUc29CmlfFgIK7LEYAmwcDC3QVp2ESZo2JHYuQrWFpImjbzbYu0ON6qqcfbX8SnbzRid/XfGUg== -xterm-headless@5.3.0-beta.42: - version "5.3.0-beta.42" - resolved "https://registry.yarnpkg.com/xterm-headless/-/xterm-headless-5.3.0-beta.42.tgz#714d167dc0af010067420956ffca80149dddeca9" - integrity sha512-3xhyy9DaPpadd1iiuKjOn4Zvntnx8WSBMEXPJbrORmy3ZbIwmLf/HYvg7iZfVreSDH+UFlm06uq3ictXpv7YvA== +xterm-headless@5.3.0-beta.46: + version "5.3.0-beta.46" + resolved "https://registry.yarnpkg.com/xterm-headless/-/xterm-headless-5.3.0-beta.46.tgz#3f941f673e3c61aad2705e4b34b42fbe467aed48" + integrity sha512-e/VbZKrfyD1TTxOY5/jah3dUih6q8nTO5v5GQV7YOASp+yyI0XHKiDjLrCyFPQOLTKxCcRutMHvr+CoCcPpcHg== -xterm@5.3.0-beta.42: - version "5.3.0-beta.42" - resolved "https://registry.yarnpkg.com/xterm/-/xterm-5.3.0-beta.42.tgz#f0806ebe9b033530c54ac5b03955827fb729c38f" - integrity sha512-3OLXTfwtSy9UKo8Uzm3/kd7qBg0JD/qFpotg8iKJ2aB9zx0fkJnysl4aIuNleUJzgvuxzBUHkcRZUmEw1XCKdw== +xterm@5.3.0-beta.46: + version "5.3.0-beta.46" + resolved "https://registry.yarnpkg.com/xterm/-/xterm-5.3.0-beta.46.tgz#9bd2b2a588c88ae64f1989ca3bf5bd9116554b5a" + integrity sha512-keB3C6sXm56ug1+hylAQiNyZP4YtPgd2W+Gu/ZKwJMk5haD25DEnenBYL++Cl3+sNGYxOGS42QlmYj6Mrq1LPw== y18n@^3.2.1: version "3.2.2" From 3cc27e7f16de312602f0b92c0d24c488d3200402 Mon Sep 17 00:00:00 2001 From: Logan Ramos Date: Sun, 20 Aug 2023 09:51:01 -0700 Subject: [PATCH 28/34] Make telemetry more resilient to bad networks (#190810) * More resiliance towards bad network * Spelling --- src/vs/platform/telemetry/node/1dsAppender.ts | 102 +++++++++--------- 1 file changed, 54 insertions(+), 48 deletions(-) diff --git a/src/vs/platform/telemetry/node/1dsAppender.ts b/src/vs/platform/telemetry/node/1dsAppender.ts index 94f0dc30692..63805143d49 100644 --- a/src/vs/platform/telemetry/node/1dsAppender.ts +++ b/src/vs/platform/telemetry/node/1dsAppender.ts @@ -8,15 +8,24 @@ import { streamToBuffer } from 'vs/base/common/buffer'; import { CancellationToken } from 'vs/base/common/cancellation'; import { IRequestOptions } from 'vs/base/parts/request/common/request'; import { IRequestService } from 'vs/platform/request/common/request'; +import * as https from 'https'; import { AbstractOneDataSystemAppender, IAppInsightsCore } from 'vs/platform/telemetry/common/1dsAppender'; +type OnCompleteFunc = (status: number, headers: { [headerName: string]: string }, response?: string) => void; + +interface IResponseData { + headers: { [headerName: string]: string }; + statusCode: number; + responseData: string; +} + /** * Completes a request to submit telemetry to the server utilizing the request service * @param options The options which will be used to make the request * @param requestService The request service * @returns An object containing the headers, statusCode, and responseData */ -async function makeTelemetryRequest(options: IRequestOptions, requestService: IRequestService) { +async function makeTelemetryRequest(options: IRequestOptions, requestService: IRequestService): Promise { const response = await requestService.request(options, CancellationToken.None); const responseData = (await streamToBuffer(response.stream)).toString(); const statusCode = response.res.statusCode ?? 200; @@ -31,30 +40,57 @@ async function makeTelemetryRequest(options: IRequestOptions, requestService: IR /** * Complete a request to submit telemetry to the server utilizing the https module. Only used when the request service is not available * @param options The options which will be used to make the request - * @param httpsModule The https node module * @returns An object containing the headers, statusCode, and responseData */ -function makeLegacyTelemetryRequest(options: IRequestOptions, httpsModule: typeof import('https')) { +async function makeLegacyTelemetryRequest(options: IRequestOptions): Promise { const httpsOptions = { method: options.type, headers: options.headers }; - const req = httpsModule.request(options.url ?? '', httpsOptions, res => { - res.on('data', function (responseData) { - return { - headers: res.headers as Record, - statusCode: res.statusCode ?? 200, - responseData: responseData.toString() - }; + const responsePromise = new Promise((resolve, reject) => { + const req = https.request(options.url ?? '', httpsOptions, res => { + res.on('data', function (responseData) { + resolve({ + headers: res.headers as Record, + statusCode: res.statusCode ?? 200, + responseData: responseData.toString() + }); + }); + // On response with error send status of 0 and a blank response to oncomplete so we can retry events + res.on('error', function (err) { + reject(err); + }); }); - // On response with error send status of 0 and a blank response to oncomplete so we can retry events - res.on('error', function (err) { - throw err; + req.write(options.data, (err) => { + if (err) { + reject(err); + } }); + req.end(); }); - req.write(options.data); - req.end(); - return; + return responsePromise; +} + +async function sendPostAsync(requestService: IRequestService | undefined, payload: IPayloadData, oncomplete: OnCompleteFunc) { + const telemetryRequestData = typeof payload.data === 'string' ? payload.data : new TextDecoder().decode(payload.data); + const requestOptions: IRequestOptions = { + type: 'POST', + headers: { + ...payload.headers, + 'Content-Type': 'application/json', + 'Content-Length': Buffer.byteLength(payload.data).toString() + }, + url: payload.urlString, + data: telemetryRequestData + }; + + try { + const responseData = requestService ? await makeTelemetryRequest(requestOptions, requestService) : await makeLegacyTelemetryRequest(requestOptions); + oncomplete(responseData.statusCode, responseData.headers, responseData.responseData); + } catch { + // If it errors out, send status of 0 and a blank response to oncomplete so we can retry events + oncomplete(0, {}); + } } @@ -67,41 +103,11 @@ export class OneDataSystemAppender extends AbstractOneDataSystemAppender { defaultData: { [key: string]: any } | null, iKeyOrClientFactory: string | (() => IAppInsightsCore), // allow factory function for testing ) { - let httpsModule: typeof import('https') | undefined; - if (!requestService) { - httpsModule = require('https'); - } // Override the way events get sent since node doesn't have XHTMLRequest const customHttpXHROverride: IXHROverride = { sendPOST: (payload: IPayloadData, oncomplete) => { - - const telemetryRequestData = typeof payload.data === 'string' ? payload.data : new TextDecoder().decode(payload.data); - const requestOptions: IRequestOptions = { - type: 'POST', - headers: { - ...payload.headers, - 'Content-Type': 'application/json', - 'Content-Length': Buffer.byteLength(payload.data).toString() - }, - url: payload.urlString, - data: telemetryRequestData - }; - - try { - if (requestService) { - makeTelemetryRequest(requestOptions, requestService).then(({ statusCode, headers, responseData }) => { - oncomplete(statusCode, headers, responseData); - }); - } else { - if (!httpsModule) { - throw new Error('https module is undefined'); - } - makeLegacyTelemetryRequest(requestOptions, httpsModule); - } - } catch { - // If it errors out, send status of 0 and a blank response to oncomplete so we can retry events - oncomplete(0, {}); - } + // Fire off the async request without awaiting it + sendPostAsync(requestService, payload, oncomplete); } }; From f125afbc800ec611f5a9ab1333c769832ce424b3 Mon Sep 17 00:00:00 2001 From: Tyler James Leonhardt Date: Sun, 20 Aug 2023 09:51:31 -0700 Subject: [PATCH 29/34] Move contributing to Command Center to registration time & have Chat show up in Command Center (#190786) --- .../platform/quickinput/common/quickAccess.ts | 13 +++ .../browser/actions/chatQuickInputActions.ts | 15 +-- .../contrib/chat/browser/chat.contribution.ts | 12 -- src/vs/workbench/contrib/chat/browser/chat.ts | 3 +- .../browser/chatContributionServiceImpl.ts | 5 +- .../contrib/chat/browser/chatQuick.ts | 10 +- .../quickaccess/gotoSymbolQuickAccess.ts | 12 +- .../debug/browser/debug.contribution.ts | 6 +- .../browser/quickAccess.contribution.ts | 8 +- .../search/browser/anythingQuickAccess.ts | 104 ++++++++---------- .../search/browser/search.contribution.ts | 6 +- .../tasks/browser/task.contribution.ts | 2 +- 12 files changed, 98 insertions(+), 98 deletions(-) diff --git a/src/vs/platform/quickinput/common/quickAccess.ts b/src/vs/platform/quickinput/common/quickAccess.ts index e5c5dee9173..47dc660daca 100644 --- a/src/vs/platform/quickinput/common/quickAccess.ts +++ b/src/vs/platform/quickinput/common/quickAccess.ts @@ -125,6 +125,19 @@ export interface IQuickAccessProviderHelp { * The command to bring up this quick access provider. */ readonly commandId?: string; + + /** + * The order of help entries in the Command Center. + * Lower values will be placed above higher values. + * No value will hide this help entry from the Command Center. + */ + readonly commandCenterOrder?: number; + + /** + * An optional label to use for the Command Center entry. If not set + * the description will be used instead. + */ + readonly commandCenterLabel?: string; } export interface IQuickAccessProviderDescriptor { diff --git a/src/vs/workbench/contrib/chat/browser/actions/chatQuickInputActions.ts b/src/vs/workbench/contrib/chat/browser/actions/chatQuickInputActions.ts index 31c6361beaa..07575215554 100644 --- a/src/vs/workbench/contrib/chat/browser/actions/chatQuickInputActions.ts +++ b/src/vs/workbench/contrib/chat/browser/actions/chatQuickInputActions.ts @@ -7,13 +7,11 @@ import { Codicon } from 'vs/base/common/codicons'; import { KeyCode, KeyMod } from 'vs/base/common/keyCodes'; import { localize } from 'vs/nls'; import { Action2, MenuId, registerAction2 } from 'vs/platform/actions/common/actions'; -import { ContextKeyExpr } from 'vs/platform/contextkey/common/contextkey'; import { ServicesAccessor } from 'vs/platform/instantiation/common/instantiation'; import { KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegistry'; import { CHAT_CATEGORY } from 'vs/workbench/contrib/chat/browser/actions/chatActions'; import { IQuickChatService } from 'vs/workbench/contrib/chat/browser/chat'; import { CONTEXT_PROVIDER_EXISTS } from 'vs/workbench/contrib/chat/common/chatContextKeys'; -import { IChatService } from 'vs/workbench/contrib/chat/common/chatService'; export const ASK_QUICK_QUESTION_ACTION_ID = 'workbench.action.quickchat.toggle'; export function registerQuickChatActions() { @@ -85,24 +83,13 @@ class QuickChatGlobalAction extends Action2 { linux: { primary: KeyMod.CtrlCmd | KeyMod.Shift | KeyMod.Alt | KeyCode.KeyI } - }, - menu: { - id: MenuId.LayoutControlMenu, - group: '0_workbench_toggles', - when: ContextKeyExpr.notEquals('config.chat.experimental.defaultMode', 'chatView'), - order: 0 } }); } override run(accessor: ServicesAccessor, query?: string): void { - const chatService = accessor.get(IChatService); const quickChatService = accessor.get(IQuickChatService); - // Grab the first provider and run its command - const info = chatService.getProviderInfos()[0]; - if (info) { - quickChatService.toggle(info.id, query); - } + quickChatService.toggle(undefined, query); } } diff --git a/src/vs/workbench/contrib/chat/browser/chat.contribution.ts b/src/vs/workbench/contrib/chat/browser/chat.contribution.ts index 09d6c46267b..9e6bf370a27 100644 --- a/src/vs/workbench/contrib/chat/browser/chat.contribution.ts +++ b/src/vs/workbench/contrib/chat/browser/chat.contribution.ts @@ -86,18 +86,6 @@ configurationRegistry.registerConfiguration({ type: 'number', description: nls.localize('interactiveSession.editor.lineHeight', "Controls the line height in pixels in chat codeblocks. Use 0 to compute the line height from the font size."), default: 0 - }, - 'chat.experimental.defaultMode': { - type: 'string', - tags: ['experimental'], - enum: ['chatView', 'quickQuestion', 'both'], - enumDescriptions: [ - nls.localize('interactiveSession.defaultMode.chatView', "Use the chat view as the default mode. Displays the chat icon in the Activity Bar."), - nls.localize('interactiveSession.defaultMode.quickQuestion', "Use the quick question as the default mode. Displays the chat icon in the Title Bar."), - nls.localize('interactiveSession.defaultMode.both', "Displays the chat icon in the Activity Bar and the Title Bar which open their respective chat modes.") - ], - description: nls.localize('interactiveSession.defaultMode', "Controls the default mode of the chat experience."), - default: 'chatView' } } }); diff --git a/src/vs/workbench/contrib/chat/browser/chat.ts b/src/vs/workbench/contrib/chat/browser/chat.ts index 8e1c6ee182c..f0587cfbc84 100644 --- a/src/vs/workbench/contrib/chat/browser/chat.ts +++ b/src/vs/workbench/contrib/chat/browser/chat.ts @@ -35,7 +35,8 @@ export interface IChatWidgetService { export interface IQuickChatService { readonly _serviceBrand: undefined; - toggle(providerId: string, query?: string): void; + enabled: boolean; + toggle(providerId?: string, query?: string): void; focus(): void; close(): void; openInChatView(): void; diff --git a/src/vs/workbench/contrib/chat/browser/chatContributionServiceImpl.ts b/src/vs/workbench/contrib/chat/browser/chatContributionServiceImpl.ts index 66759974390..000bc774029 100644 --- a/src/vs/workbench/contrib/chat/browser/chatContributionServiceImpl.ts +++ b/src/vs/workbench/contrib/chat/browser/chatContributionServiceImpl.ts @@ -126,10 +126,7 @@ export class ChatContributionService implements IChatContributionService { canToggleVisibility: false, canMoveView: true, ctorDescriptor: new SyncDescriptor(ChatViewPane, [{ providerId: providerDescriptor.id }]), - when: ContextKeyExpr.and( - ContextKeyExpr.deserialize(providerDescriptor.when), - ContextKeyExpr.notEquals('config.chat.experimental.defaultMode', 'quickQuestion') - ) + when: ContextKeyExpr.deserialize(providerDescriptor.when) }]; Registry.as(ViewExtensions.ViewsRegistry).registerViews(viewDescriptor, viewContainer); diff --git a/src/vs/workbench/contrib/chat/browser/chatQuick.ts b/src/vs/workbench/contrib/chat/browser/chatQuick.ts index ec90707236b..e43c684102d 100644 --- a/src/vs/workbench/contrib/chat/browser/chatQuick.ts +++ b/src/vs/workbench/contrib/chat/browser/chatQuick.ts @@ -29,6 +29,10 @@ export class QuickChatService implements IQuickChatService { @IInstantiationService private readonly instantiationService: IInstantiationService, ) { } + get enabled(): boolean { + return this.chatService.getProviderInfos().length > 0; + } + get focused(): boolean { const widget = this._input?.widget as HTMLElement; if (!widget) { @@ -37,7 +41,7 @@ export class QuickChatService implements IQuickChatService { return dom.isAncestor(document.activeElement, widget); } - toggle(providerId: string, query?: string | undefined): void { + toggle(providerId?: string, query?: string | undefined): void { // If the input is already shown, hide it. This provides a toggle behavior of the quick pick if (this.focused) { this.close(); @@ -46,7 +50,9 @@ export class QuickChatService implements IQuickChatService { // Check if any providers are available. If not, show nothing // This shouldn't be needed because of the precondition, but just in case - const providerInfo = this.chatService.getProviderInfos().find(info => info.id === providerId); + const providerInfo = providerId + ? this.chatService.getProviderInfos().find(info => info.id === providerId) + : this.chatService.getProviderInfos()[0]; if (!providerInfo) { return; } diff --git a/src/vs/workbench/contrib/codeEditor/browser/quickaccess/gotoSymbolQuickAccess.ts b/src/vs/workbench/contrib/codeEditor/browser/quickaccess/gotoSymbolQuickAccess.ts index d6be4855c01..0a16cc4822a 100644 --- a/src/vs/workbench/contrib/codeEditor/browser/quickaccess/gotoSymbolQuickAccess.ts +++ b/src/vs/workbench/contrib/codeEditor/browser/quickaccess/gotoSymbolQuickAccess.ts @@ -273,7 +273,15 @@ Registry.as(QuickaccessExtensions.Quickaccess).registerQui contextKey: 'inFileSymbolsPicker', placeholder: localize('gotoSymbolQuickAccessPlaceholder', "Type the name of a symbol to go to."), helpEntries: [ - { description: localize('gotoSymbolQuickAccess', "Go to Symbol in Editor"), prefix: AbstractGotoSymbolQuickAccessProvider.PREFIX, commandId: GotoSymbolAction.ID }, - { description: localize('gotoSymbolByCategoryQuickAccess', "Go to Symbol in Editor by Category"), prefix: AbstractGotoSymbolQuickAccessProvider.PREFIX_BY_CATEGORY } + { + description: localize('gotoSymbolQuickAccess', "Go to Symbol in Editor"), + prefix: AbstractGotoSymbolQuickAccessProvider.PREFIX, + commandId: GotoSymbolAction.ID, + commandCenterOrder: 40 + }, + { + description: localize('gotoSymbolByCategoryQuickAccess', "Go to Symbol in Editor by Category"), + prefix: AbstractGotoSymbolQuickAccessProvider.PREFIX_BY_CATEGORY + } ] }); diff --git a/src/vs/workbench/contrib/debug/browser/debug.contribution.ts b/src/vs/workbench/contrib/debug/browser/debug.contribution.ts index 3b1eebc2929..c0d1213a3c3 100644 --- a/src/vs/workbench/contrib/debug/browser/debug.contribution.ts +++ b/src/vs/workbench/contrib/debug/browser/debug.contribution.ts @@ -77,7 +77,11 @@ Registry.as(QuickAccessExtensions.Quickaccess).registerQui prefix: DEBUG_QUICK_ACCESS_PREFIX, contextKey: 'inLaunchConfigurationsPicker', placeholder: nls.localize('startDebugPlaceholder', "Type the name of a launch configuration to run."), - helpEntries: [{ description: nls.localize('startDebuggingHelp', "Start Debugging"), commandId: SELECT_AND_START_ID }] + helpEntries: [{ + description: nls.localize('startDebuggingHelp', "Start Debugging"), + commandId: SELECT_AND_START_ID, + commandCenterOrder: 50 + }] }); // Register quick access for debug console diff --git a/src/vs/workbench/contrib/quickaccess/browser/quickAccess.contribution.ts b/src/vs/workbench/contrib/quickaccess/browser/quickAccess.contribution.ts index 85a5e04afd2..61a4af6eac8 100644 --- a/src/vs/workbench/contrib/quickaccess/browser/quickAccess.contribution.ts +++ b/src/vs/workbench/contrib/quickaccess/browser/quickAccess.contribution.ts @@ -24,7 +24,11 @@ quickAccessRegistry.registerQuickAccessProvider({ ctor: HelpQuickAccessProvider, prefix: HelpQuickAccessProvider.PREFIX, placeholder: localize('helpQuickAccessPlaceholder', "Type '{0}' to get help on the actions you can take from here.", HelpQuickAccessProvider.PREFIX), - helpEntries: [{ description: localize('helpQuickAccess', "Show all Quick Access Providers") }] + helpEntries: [{ + description: localize('helpQuickAccess', "Show all Quick Access Providers"), + commandCenterOrder: 70, + commandCenterLabel: localize('more', 'More') + }] }); quickAccessRegistry.registerQuickAccessProvider({ @@ -40,7 +44,7 @@ quickAccessRegistry.registerQuickAccessProvider({ prefix: CommandsQuickAccessProvider.PREFIX, contextKey: 'inCommandsPicker', placeholder: localize('commandsQuickAccessPlaceholder', "Type the name of a command to run."), - helpEntries: [{ description: localize('commandsQuickAccess', "Show and Run Commands"), commandId: ShowAllCommandsAction.ID }] + helpEntries: [{ description: localize('commandsQuickAccess', "Show and Run Commands"), commandId: ShowAllCommandsAction.ID, commandCenterOrder: 20 }] }); //#endregion diff --git a/src/vs/workbench/contrib/search/browser/anythingQuickAccess.ts b/src/vs/workbench/contrib/search/browser/anythingQuickAccess.ts index ccd7471df75..94fc88fe959 100644 --- a/src/vs/workbench/contrib/search/browser/anythingQuickAccess.ts +++ b/src/vs/workbench/contrib/search/browser/anythingQuickAccess.ts @@ -40,7 +40,7 @@ import { Schemas } from 'vs/base/common/network'; import { IFilesConfigurationService, AutoSaveMode } from 'vs/workbench/services/filesConfiguration/common/filesConfigurationService'; import { ResourceMap } from 'vs/base/common/map'; import { SymbolsQuickAccessProvider } from 'vs/workbench/contrib/search/browser/symbolsQuickAccess'; -import { AnythingQuickAccessProviderRunOptions, DefaultQuickAccessFilterValue } from 'vs/platform/quickinput/common/quickAccess'; +import { AnythingQuickAccessProviderRunOptions, DefaultQuickAccessFilterValue, Extensions, IQuickAccessRegistry } from 'vs/platform/quickinput/common/quickAccess'; import { IWorkbenchQuickAccessConfiguration } from 'vs/workbench/browser/quickaccess'; import { GotoSymbolQuickAccessProvider } from 'vs/workbench/contrib/codeEditor/browser/quickaccess/gotoSymbolQuickAccess'; import { ITextModelService } from 'vs/editor/common/services/resolverService'; @@ -52,11 +52,11 @@ import { Codicon } from 'vs/base/common/codicons'; import { ThemeIcon } from 'vs/base/common/themables'; import { IUriIdentityService } from 'vs/platform/uriIdentity/common/uriIdentity'; import { stripIcons } from 'vs/base/common/iconLabels'; -import { HelpQuickAccessProvider } from 'vs/platform/quickinput/browser/helpQuickAccess'; -import { CommandsQuickAccessProvider } from 'vs/workbench/contrib/quickaccess/browser/commandsQuickAccess'; -import { DEBUG_QUICK_ACCESS_PREFIX } from 'vs/workbench/contrib/debug/browser/debugCommands'; -import { TasksQuickAccessProvider } from 'vs/workbench/contrib/tasks/browser/tasksQuickAccess'; import { Lazy } from 'vs/base/common/lazy'; +import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; +import { Registry } from 'vs/platform/registry/common/platform'; +import { ASK_QUICK_QUESTION_ACTION_ID } from 'vs/workbench/contrib/chat/browser/actions/chatQuickInputActions'; +import { IQuickChatService } from 'vs/workbench/contrib/chat/browser/chat'; interface IAnythingQuickPickItem extends IPickerQuickAccessItem, IQuickPickItemWithResource { } @@ -187,6 +187,8 @@ export class AnythingQuickAccessProvider extends PickerQuickAccessProvider Registry.as(Extensions.Quickaccess)); private getHelpPicks(query: IPreparedQuery, token: CancellationToken, runOptions?: AnythingQuickAccessProviderRunOptions): IAnythingQuickPickItem[] { if (query.normalized) { return []; // If there's a filter, we don't show the help } - type IHelpAnythingQuickPickItem = IAnythingQuickPickItem & { prefix: string }; - const providers: Array = this.helpQuickAccess.getQuickAccessProviders(); - const mapOfProviders = new Map(); - for (const provider of providers) { - mapOfProviders.set(provider.prefix, provider); + type IHelpAnythingQuickPickItem = IAnythingQuickPickItem & { commandCenterOrder: number }; + const providers: IHelpAnythingQuickPickItem[] = this.lazyRegistry.value.getQuickAccessProviders() + .filter(p => p.helpEntries.some(h => h.commandCenterOrder !== undefined)) + .flatMap(provider => provider.helpEntries + .filter(h => h.commandCenterOrder !== undefined) + .map(helpEntry => { + const providerSpecificOptions: AnythingQuickAccessProviderRunOptions | undefined = { + ...runOptions, + includeHelp: provider.prefix === AnythingQuickAccessProvider.PREFIX ? false : runOptions?.includeHelp + }; + + const label = helpEntry.commandCenterLabel ?? helpEntry.description!; + return { + label, + description: helpEntry.prefix ?? provider.prefix, + commandCenterOrder: helpEntry.commandCenterOrder!, + keybinding: helpEntry.commandId ? this.keybindingService.lookupKeybinding(helpEntry.commandId) : undefined, + ariaLabel: localize('helpPickAriaLabel', "{0}, {1}", label, helpEntry.description), + accept: () => { + this.quickInputService.quickAccess.show(provider.prefix, { + preserveValue: true, + providerOptions: providerSpecificOptions + }); + } + }; + })); + + // TODO: There has to be a better place for this, but it's the first time we are adding a non-quick access provider + // to the command center, so for now, let's do this. + if (this.quickChatService.enabled) { + providers.push({ + label: localize('chat', "Open Quick Chat"), + commandCenterOrder: 30, + keybinding: this.keybindingService.lookupKeybinding(ASK_QUICK_QUESTION_ACTION_ID), + accept: () => this.quickChatService.toggle() + }); } - const importantProviders: Array = []; - const AddProvider = (prefix: string, modifications: Partial = {}) => { - if (mapOfProviders.has(prefix)) { - const provider = mapOfProviders.get(prefix)!; - - // We swap the label and description in this to emphasize the ability - // not the prefix. - provider.label = provider.description!; - provider.description = provider.prefix; - - // If the user chooses 'Go to File' the help should go away as if they were - // entering a new mode - const providerSpecificOptions: AnythingQuickAccessProviderRunOptions | undefined = { - ...runOptions, - includeHelp: provider.prefix === AnythingQuickAccessProvider.PREFIX ? false : runOptions?.includeHelp - }; - - importantProviders.push({ - ...mapOfProviders.get(prefix)!, - ...modifications, - accept: () => { - this.quickInputService.quickAccess.show(provider.prefix, { - preserveValue: true, - providerOptions: providerSpecificOptions - }); - } - }); - } - }; - - // TODO@TylerLeonhardt ideally this hardcoded list and hardcoded dependency moves - // into a provider model where when I register a quick access provider I can enlist - // for showing up in command center - - // Acts as the ordering too - AddProvider(AnythingQuickAccessProvider.PREFIX); - AddProvider(CommandsQuickAccessProvider.PREFIX); - AddProvider(GotoSymbolQuickAccessProvider.PREFIX); - AddProvider(DEBUG_QUICK_ACCESS_PREFIX); - AddProvider(TasksQuickAccessProvider.PREFIX); - AddProvider(HelpQuickAccessProvider.PREFIX, { - // More concise - label: localize('more', 'More') - }); - - return importantProviders; + return providers.sort((a, b) => a.commandCenterOrder - b.commandCenterOrder); } //#endregion diff --git a/src/vs/workbench/contrib/search/browser/search.contribution.ts b/src/vs/workbench/contrib/search/browser/search.contribution.ts index ec2d3dd0103..d868e5f6575 100644 --- a/src/vs/workbench/contrib/search/browser/search.contribution.ts +++ b/src/vs/workbench/contrib/search/browser/search.contribution.ts @@ -107,7 +107,11 @@ quickAccessRegistry.registerQuickAccessProvider({ prefix: AnythingQuickAccessProvider.PREFIX, placeholder: nls.localize('anythingQuickAccessPlaceholder', "Search files by name (append {0} to go to line or {1} to go to symbol)", AbstractGotoLineQuickAccessProvider.PREFIX, GotoSymbolQuickAccessProvider.PREFIX), contextKey: defaultQuickAccessContextKeyValue, - helpEntries: [{ description: nls.localize('anythingQuickAccess', "Go to File"), commandId: 'workbench.action.quickOpen' }] + helpEntries: [{ + description: nls.localize('anythingQuickAccess', "Go to File"), + commandId: 'workbench.action.quickOpen', + commandCenterOrder: 10 + }] }); quickAccessRegistry.registerQuickAccessProvider({ diff --git a/src/vs/workbench/contrib/tasks/browser/task.contribution.ts b/src/vs/workbench/contrib/tasks/browser/task.contribution.ts index d20bb9d4370..27bdc71350a 100644 --- a/src/vs/workbench/contrib/tasks/browser/task.contribution.ts +++ b/src/vs/workbench/contrib/tasks/browser/task.contribution.ts @@ -408,7 +408,7 @@ quickAccessRegistry.registerQuickAccessProvider({ prefix: TasksQuickAccessProvider.PREFIX, contextKey: tasksPickerContextKey, placeholder: nls.localize('tasksQuickAccessPlaceholder', "Type the name of a task to run."), - helpEntries: [{ description: nls.localize('tasksQuickAccessHelp', "Run Task") }] + helpEntries: [{ description: nls.localize('tasksQuickAccessHelp', "Run Task"), commandCenterOrder: 60 }] }); // tasks.json validation From 9f7783206a19c2b08fbfce27e6d1b17fd1d308ec Mon Sep 17 00:00:00 2001 From: Henning Dieterichs Date: Sat, 19 Aug 2023 13:15:46 +0200 Subject: [PATCH 30/34] Fixes #https://github.com/microsoft/monaco-editor/issues/4129 --- .../diffEditorWidget2/diffEditorViewModel.ts | 53 +++++++++---------- 1 file changed, 24 insertions(+), 29 deletions(-) diff --git a/src/vs/editor/browser/widget/diffEditorWidget2/diffEditorViewModel.ts b/src/vs/editor/browser/widget/diffEditorWidget2/diffEditorViewModel.ts index 4833d3de651..448ccb5b27e 100644 --- a/src/vs/editor/browser/widget/diffEditorWidget2/diffEditorViewModel.ts +++ b/src/vs/editor/browser/widget/diffEditorWidget2/diffEditorViewModel.ts @@ -105,40 +105,36 @@ export class DiffEditorViewModel extends Disposable implements IDiffEditorViewMo this._register(model.modified.onDidChangeContent((e) => { const diff = this._diff.get(); - if (!diff) { - return; - } - - const textEdits = TextEditInfo.fromModelContentChanges(e.changes); - const result = applyModifiedEdits(this._lastDiff!, textEdits, model.original, model.modified); - if (result) { - this._lastDiff = result; - transaction(tx => { - this._diff.set(DiffState.fromDiffResult(this._lastDiff!), tx); - updateUnchangedRegions(result, tx); - const currentSyncedMovedText = this.syncedMovedTexts.get(); - this.syncedMovedTexts.set(currentSyncedMovedText ? this._lastDiff!.moves.find(m => m.lineRangeMapping.modified.intersect(currentSyncedMovedText.lineRangeMapping.modified)) : undefined, tx); - }); + if (diff) { + const textEdits = TextEditInfo.fromModelContentChanges(e.changes); + const result = applyModifiedEdits(this._lastDiff!, textEdits, model.original, model.modified); + if (result) { + this._lastDiff = result; + transaction(tx => { + this._diff.set(DiffState.fromDiffResult(this._lastDiff!), tx); + updateUnchangedRegions(result, tx); + const currentSyncedMovedText = this.syncedMovedTexts.get(); + this.syncedMovedTexts.set(currentSyncedMovedText ? this._lastDiff!.moves.find(m => m.lineRangeMapping.modified.intersect(currentSyncedMovedText.lineRangeMapping.modified)) : undefined, tx); + }); + } } debouncer.schedule(); })); this._register(model.original.onDidChangeContent((e) => { const diff = this._diff.get(); - if (!diff) { - return; - } - - const textEdits = TextEditInfo.fromModelContentChanges(e.changes); - const result = applyOriginalEdits(this._lastDiff!, textEdits, model.original, model.modified); - if (result) { - this._lastDiff = result; - transaction(tx => { - this._diff.set(DiffState.fromDiffResult(this._lastDiff!), tx); - updateUnchangedRegions(result, tx); - const currentSyncedMovedText = this.syncedMovedTexts.get(); - this.syncedMovedTexts.set(currentSyncedMovedText ? this._lastDiff!.moves.find(m => m.lineRangeMapping.modified.intersect(currentSyncedMovedText.lineRangeMapping.modified)) : undefined, tx); - }); + if (diff) { + const textEdits = TextEditInfo.fromModelContentChanges(e.changes); + const result = applyOriginalEdits(this._lastDiff!, textEdits, model.original, model.modified); + if (result) { + this._lastDiff = result; + transaction(tx => { + this._diff.set(DiffState.fromDiffResult(this._lastDiff!), tx); + updateUnchangedRegions(result, tx); + const currentSyncedMovedText = this.syncedMovedTexts.get(); + this.syncedMovedTexts.set(currentSyncedMovedText ? this._lastDiff!.moves.find(m => m.lineRangeMapping.modified.intersect(currentSyncedMovedText.lineRangeMapping.modified)) : undefined, tx); + }); + } } debouncer.schedule(); @@ -175,7 +171,6 @@ export class DiffEditorViewModel extends Disposable implements IDiffEditorViewMo result = applyOriginalEdits(result, originalTextEditInfos, model.original, model.modified) ?? result; result = applyModifiedEdits(result, modifiedTextEditInfos, model.original, model.modified) ?? result; - transaction(tx => { updateUnchangedRegions(result, tx); From afa5e38f0ce3fb98c0ffc206468de46c80785aba Mon Sep 17 00:00:00 2001 From: Henning Dieterichs Date: Mon, 21 Aug 2023 11:12:51 +0200 Subject: [PATCH 31/34] Fixes #190727 --- src/vs/workbench/api/common/extHostLanguageFeatures.ts | 3 +-- .../vscode.proposed.inlineCompletionsAdditions.d.ts | 5 ----- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/vs/workbench/api/common/extHostLanguageFeatures.ts b/src/vs/workbench/api/common/extHostLanguageFeatures.ts index 7a32009ac07..cd8b2c1de2c 100644 --- a/src/vs/workbench/api/common/extHostLanguageFeatures.ts +++ b/src/vs/workbench/api/common/extHostLanguageFeatures.ts @@ -1217,7 +1217,6 @@ class InlineCompletionAdapter extends InlineCompletionAdapterBase { const normalizedResult = Array.isArray(result) ? result : result.items; const commands = this._isAdditionsProposedApiEnabled ? Array.isArray(result) ? [] : result.commands || [] : []; - const suppressSuggestions = this._isAdditionsProposedApiEnabled && !Array.isArray(result) ? result.suppressSuggestions : undefined; const enableForwardStability = this._isAdditionsProposedApiEnabled && !Array.isArray(result) ? result.enableForwardStability : undefined; let disposableStore: DisposableStore | undefined = undefined; @@ -1255,7 +1254,7 @@ class InlineCompletionAdapter extends InlineCompletionAdapterBase { } return this._commands.toInternal(c, disposableStore); }), - suppressSuggestions, + suppressSuggestions: false, enableForwardStability, }; } diff --git a/src/vscode-dts/vscode.proposed.inlineCompletionsAdditions.d.ts b/src/vscode-dts/vscode.proposed.inlineCompletionsAdditions.d.ts index c38c4e23671..88dc3ef60c7 100644 --- a/src/vscode-dts/vscode.proposed.inlineCompletionsAdditions.d.ts +++ b/src/vscode-dts/vscode.proposed.inlineCompletionsAdditions.d.ts @@ -62,11 +62,6 @@ declare module 'vscode' { */ commands?: Command[]; - /** - * When set, overrides the user setting of `editor.inlineSuggest.suppressSuggestions`. - */ - suppressSuggestions?: boolean; - /** * When set and the user types a suggestion without derivating from it, the inline suggestion is not updated. * Defaults to false (might change). From c44c644cf38d9263cf730249766266866af1e5fc Mon Sep 17 00:00:00 2001 From: Martin Aeschlimann Date: Mon, 21 Aug 2023 12:13:09 +0200 Subject: [PATCH 32/34] [folding] Fold all regions except selected is misnamed (#190732) --- src/vs/editor/contrib/folding/browser/folding.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/vs/editor/contrib/folding/browser/folding.ts b/src/vs/editor/contrib/folding/browser/folding.ts index ee79563f1f5..6134f4fa401 100644 --- a/src/vs/editor/contrib/folding/browser/folding.ts +++ b/src/vs/editor/contrib/folding/browser/folding.ts @@ -908,13 +908,13 @@ class UnfoldAllRegionsAction extends FoldingAction { } } -class FoldAllRegionsExceptAction extends FoldingAction { +class FoldAllExceptAction extends FoldingAction { constructor() { super({ id: 'editor.foldAllExcept', - label: nls.localize('foldAllExcept.label', "Fold All Regions Except Selected"), - alias: 'Fold All Regions Except Selected', + label: nls.localize('foldAllExcept.label', "Fold All Except Selected"), + alias: 'Fold All Except Selected', precondition: CONTEXT_FOLDING_ENABLED, kbOpts: { kbExpr: EditorContextKeys.editorTextFocus, @@ -931,13 +931,13 @@ class FoldAllRegionsExceptAction extends FoldingAction { } -class UnfoldAllRegionsExceptAction extends FoldingAction { +class UnfoldAllExceptAction extends FoldingAction { constructor() { super({ id: 'editor.unfoldAllExcept', - label: nls.localize('unfoldAllExcept.label', "Unfold All Regions Except Selected"), - alias: 'Unfold All Regions Except Selected', + label: nls.localize('unfoldAllExcept.label', "Unfold All Except Selected"), + alias: 'Unfold All Except Selected', precondition: CONTEXT_FOLDING_ENABLED, kbOpts: { kbExpr: EditorContextKeys.editorTextFocus, @@ -1194,8 +1194,8 @@ registerEditorAction(UnfoldAllAction); registerEditorAction(FoldAllBlockCommentsAction); registerEditorAction(FoldAllRegionsAction); registerEditorAction(UnfoldAllRegionsAction); -registerEditorAction(FoldAllRegionsExceptAction); -registerEditorAction(UnfoldAllRegionsExceptAction); +registerEditorAction(FoldAllExceptAction); +registerEditorAction(UnfoldAllExceptAction); registerEditorAction(ToggleFoldAction); registerEditorAction(GotoParentFoldAction); registerEditorAction(GotoPreviousFoldAction); From 829932cb62b2216d8311c2981918b92b13b8a026 Mon Sep 17 00:00:00 2001 From: Martin Aeschlimann Date: Mon, 21 Aug 2023 12:13:51 +0200 Subject: [PATCH 33/34] improve json schema settings descriptions (#190735) --- extensions/json-language-features/package.nls.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/extensions/json-language-features/package.nls.json b/extensions/json-language-features/package.nls.json index 4091dad211a..df68b3f8eac 100644 --- a/extensions/json-language-features/package.nls.json +++ b/extensions/json-language-features/package.nls.json @@ -2,9 +2,9 @@ "displayName": "JSON Language Features", "description": "Provides rich language support for JSON files.", "json.schemas.desc": "Associate schemas to JSON files in the current project.", - "json.schemas.url.desc": "A URL to a schema or a relative path to a schema in the current directory", - "json.schemas.fileMatch.desc": "An array of file patterns to match against when resolving JSON files to schemas. `*` can be used as a wildcard. Exclusion patterns can also be defined and start with '!'. A file matches when there is at least one matching pattern and the last matching pattern is not an exclusion pattern.", - "json.schemas.fileMatch.item.desc": "A file pattern that can contain '*' to match against when resolving JSON files to schemas.", + "json.schemas.url.desc": "A URL or absolute file path to a schema. Can be a relative path in workspace and workspace folder settings.", + "json.schemas.fileMatch.desc": "An array of file patterns to match against when resolving JSON files to schemas. `*` and '**' can be used as a wildcard. Exclusion patterns can also be defined and start with '!'. A file matches when there is at least one matching pattern and the last matching pattern is not an exclusion pattern.", + "json.schemas.fileMatch.item.desc": "A file pattern that can contain '*' and '**' to match against when resolving JSON files to schemas. When beginning with '!', it defines an exclusion pattern.", "json.schemas.schema.desc": "The schema definition for the given URL. The schema only needs to be provided to avoid accesses to the schema URL.", "json.format.enable.desc": "Enable/disable default JSON formatter", "json.format.keepLines.desc" : "Keep all existing new lines when formatting.", From 69b2435e14e5dbd442df58efcc72c28ad81e1ac2 Mon Sep 17 00:00:00 2001 From: Martin Aeschlimann Date: Mon, 21 Aug 2023 12:14:11 +0200 Subject: [PATCH 34/34] set a user agent when attempting to retrieve $schema JSON Schemas (#190726) --- .../client/src/node/jsonClientMain.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/extensions/json-language-features/client/src/node/jsonClientMain.ts b/extensions/json-language-features/client/src/node/jsonClientMain.ts index 10895276e1f..457a40f6a74 100644 --- a/extensions/json-language-features/client/src/node/jsonClientMain.ts +++ b/extensions/json-language-features/client/src/node/jsonClientMain.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { ExtensionContext, OutputChannel, window, workspace, l10n } from 'vscode'; +import { ExtensionContext, OutputChannel, window, workspace, l10n, env } from 'vscode'; import { startClient, LanguageClientConstructor, SchemaRequestService, languageServerDescription } from '../jsonClient'; import { ServerOptions, TransportKind, LanguageClientOptions, LanguageClient, BaseLanguageClient } from 'vscode-languageclient/node'; @@ -129,7 +129,10 @@ async function getSchemaRequestService(context: ExtensionContext, log: Log): Pro const isXHRResponse = (error: any): error is XHRResponse => typeof error?.status === 'number'; const request = async (uri: string, etag?: string): Promise => { - const headers: Headers = { 'Accept-Encoding': 'gzip, deflate' }; + const headers: Headers = { + 'Accept-Encoding': 'gzip, deflate', + 'User-Agent': `${env.appName} (${env.appHost})` + }; if (etag) { headers['If-None-Match'] = etag; }