From 1d5c97906e0f4e38cb5b51825bd2943560ab5741 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 28 Feb 2024 13:16:21 +0100 Subject: [PATCH 01/18] select outline element when double clicking (#206429) fixes https://github.com/microsoft/vscode/issues/206426 re https://github.com/microsoft/vscode/issues/206424 --- .../browser/parts/editor/breadcrumbsControl.ts | 4 ++-- .../browser/parts/editor/breadcrumbsPicker.ts | 2 +- .../browser/outline/documentSymbolsOutline.ts | 4 ++-- .../browser/quickaccess/gotoSymbolQuickAccess.ts | 2 +- .../contrib/outline/browser/outlinePane.ts | 16 ++++++++++++++-- .../services/outline/browser/outline.ts | 2 +- 6 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/vs/workbench/browser/parts/editor/breadcrumbsControl.ts b/src/vs/workbench/browser/parts/editor/breadcrumbsControl.ts index b97793d6ddd..1802e2eac64 100644 --- a/src/vs/workbench/browser/parts/editor/breadcrumbsControl.ts +++ b/src/vs/workbench/browser/parts/editor/breadcrumbsControl.ts @@ -517,7 +517,7 @@ export class BreadcrumbsControl { this._widget.setSelection(items[idx + 1], BreadcrumbsControl.Payload_Pick); } } else { - element.outline.reveal(element, { pinned }, group === SIDE_GROUP); + element.outline.reveal(element, { pinned }, group === SIDE_GROUP, false); } } @@ -860,7 +860,7 @@ KeybindingsRegistry.registerCommandAndKeybindingRule({ return (>input).reveal(element, { pinned: true, preserveFocus: false - }, true); + }, true, false); } } }); diff --git a/src/vs/workbench/browser/parts/editor/breadcrumbsPicker.ts b/src/vs/workbench/browser/parts/editor/breadcrumbsPicker.ts index 9ca0b4310a5..13c2df31119 100644 --- a/src/vs/workbench/browser/parts/editor/breadcrumbsPicker.ts +++ b/src/vs/workbench/browser/parts/editor/breadcrumbsPicker.ts @@ -507,7 +507,7 @@ export class BreadcrumbsOutlinePicker extends BreadcrumbsPicker { protected async _revealElement(element: any, options: IEditorOptions, sideBySide: boolean): Promise { this._onWillPickElement.fire(); const outline: IOutline = this._tree.getInput(); - await outline.reveal(element, options, sideBySide); + await outline.reveal(element, options, sideBySide, false); return true; } } diff --git a/src/vs/workbench/contrib/codeEditor/browser/outline/documentSymbolsOutline.ts b/src/vs/workbench/contrib/codeEditor/browser/outline/documentSymbolsOutline.ts index 557d9a025ec..dc43b9557b7 100644 --- a/src/vs/workbench/contrib/codeEditor/browser/outline/documentSymbolsOutline.ts +++ b/src/vs/workbench/contrib/codeEditor/browser/outline/documentSymbolsOutline.ts @@ -219,7 +219,7 @@ class DocumentSymbolsOutline implements IOutline { return this._outlineModel?.uri; } - async reveal(entry: DocumentSymbolItem, options: IEditorOptions, sideBySide: boolean): Promise { + async reveal(entry: DocumentSymbolItem, options: IEditorOptions, sideBySide: boolean, select: boolean): Promise { const model = OutlineModel.get(entry); if (!model || !(entry instanceof OutlineElement)) { return; @@ -228,7 +228,7 @@ class DocumentSymbolsOutline implements IOutline { resource: model.uri, options: { ...options, - selection: Range.collapseToStart(entry.symbol.selectionRange), + selection: select ? entry.symbol.range : Range.collapseToStart(entry.symbol.selectionRange), selectionRevealType: TextEditorSelectionRevealType.NearTopIfOutsideViewport, } }, this._editor, sideBySide); diff --git a/src/vs/workbench/contrib/codeEditor/browser/quickaccess/gotoSymbolQuickAccess.ts b/src/vs/workbench/contrib/codeEditor/browser/quickaccess/gotoSymbolQuickAccess.ts index 6788e70b877..71a1b593698 100644 --- a/src/vs/workbench/contrib/codeEditor/browser/quickaccess/gotoSymbolQuickAccess.ts +++ b/src/vs/workbench/contrib/codeEditor/browser/quickaccess/gotoSymbolQuickAccess.ts @@ -183,7 +183,7 @@ export class GotoSymbolQuickAccessProvider extends AbstractGotoSymbolQuickAccess picker.hide(); const [entry] = picker.selectedItems; if (entry && entries[entry.index]) { - outline.reveal(entries[entry.index].element, {}, false); + outline.reveal(entries[entry.index].element, {}, false, false); } })); diff --git a/src/vs/workbench/contrib/outline/browser/outlinePane.ts b/src/vs/workbench/contrib/outline/browser/outlinePane.ts index 6756a2dd0fe..d00db5854fb 100644 --- a/src/vs/workbench/contrib/outline/browser/outlinePane.ts +++ b/src/vs/workbench/contrib/outline/browser/outlinePane.ts @@ -6,7 +6,7 @@ import 'vs/css!./outlinePane'; import * as dom from 'vs/base/browser/dom'; import { ProgressBar } from 'vs/base/browser/ui/progressbar/progressbar'; -import { TimeoutTimer } from 'vs/base/common/async'; +import { TimeoutTimer, timeout } from 'vs/base/common/async'; import { IDisposable, toDisposable, DisposableStore, MutableDisposable } from 'vs/base/common/lifecycle'; import { LRUCache } from 'vs/base/common/map'; import { localize } from 'vs/nls'; @@ -304,7 +304,19 @@ export class OutlinePane extends ViewPane implements IOutlinePane { // feature: reveal outline selection in editor // on change -> reveal/select defining range - this._editorControlDisposables.add(tree.onDidOpen(e => newOutline.reveal(e.element, e.editorOptions, e.sideBySide))); + let idPool = 0; + this._editorControlDisposables.add(tree.onDidOpen(async e => { + const myId = ++idPool; + const isDoubleClick = e.browserEvent?.type === 'dblclick'; + if (!isDoubleClick) { + // workaround for https://github.com/microsoft/vscode/issues/206424 + await timeout(150); + if (myId !== idPool) { + return; + } + } + await newOutline.reveal(e.element, e.editorOptions, e.sideBySide, isDoubleClick); + })); // feature: reveal editor selection in outline const revealActiveElement = () => { if (!this._outlineViewState.followCursor || !newOutline.activeElement) { diff --git a/src/vs/workbench/services/outline/browser/outline.ts b/src/vs/workbench/services/outline/browser/outline.ts index 8447b30d8fe..4a30a3a206d 100644 --- a/src/vs/workbench/services/outline/browser/outline.ts +++ b/src/vs/workbench/services/outline/browser/outline.ts @@ -83,7 +83,7 @@ export interface IOutline { readonly activeElement: E | undefined; readonly onDidChange: Event; - reveal(entry: E, options: IEditorOptions, sideBySide: boolean): Promise | void; + reveal(entry: E, options: IEditorOptions, sideBySide: boolean, select: boolean): Promise | void; preview(entry: E): IDisposable; captureViewState(): IDisposable; dispose(): void; From 4a050aaac8f5a89c7be997db92a0cbceb7db2f2f Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Wed, 28 Feb 2024 15:29:29 +0100 Subject: [PATCH 02/18] support enabling builtin extensions (#206437) * support enabling builtin extensions * fix tests --- .../browser/extensionsWorkbenchService.ts | 83 ++++++++++--------- .../extensionsWorkbenchService.test.ts | 3 - 2 files changed, 43 insertions(+), 43 deletions(-) diff --git a/src/vs/workbench/contrib/extensions/browser/extensionsWorkbenchService.ts b/src/vs/workbench/contrib/extensions/browser/extensionsWorkbenchService.ts index b2e33dd00ae..c92d36d3ecf 100644 --- a/src/vs/workbench/contrib/extensions/browser/extensionsWorkbenchService.ts +++ b/src/vs/workbench/contrib/extensions/browser/extensionsWorkbenchService.ts @@ -52,7 +52,7 @@ import { TelemetryTrustedValue } from 'vs/platform/telemetry/common/telemetryUti import { ILifecycleService, LifecyclePhase } from 'vs/workbench/services/lifecycle/common/lifecycle'; import { IUserDataProfileService } from 'vs/workbench/services/userDataProfile/common/userDataProfile'; import { mainWindow } from 'vs/base/browser/window'; -import { IDialogService } from 'vs/platform/dialogs/common/dialogs'; +import { IDialogService, IPromptButton } from 'vs/platform/dialogs/common/dialogs'; interface IExtensionStateProvider { (extension: Extension): T; @@ -1650,17 +1650,19 @@ export class ExtensionsWorkbenchService extends Disposable implements IExtension } async install(arg: string | URI | IExtension, installOptions: InstallExtensionOptions = {}, progressLocation?: ProgressLocation): Promise { - let installable: URI | { extension: IExtension; gallery: IGalleryExtension }; + let installable: URI | IGalleryExtension | undefined; + let extension: IExtension | undefined; if (arg instanceof URI) { installable = arg; } else { let installableInfo: IExtensionInfo | undefined; let gallery: IGalleryExtension | undefined; - let extension: IExtension | undefined; if (isString(arg)) { - installableInfo = { id: arg, version: installOptions.version, preRelease: installOptions?.installPreReleaseVersion ?? this.preferPreReleases }; extension = this.local.find(e => areSameExtensions(e.identifier, { id: arg })); + if (!extension?.isBuiltin) { + installableInfo = { id: arg, version: installOptions.version, preRelease: installOptions.installPreReleaseVersion ?? this.preferPreReleases }; + } } else { extension = arg; gallery = arg.gallery; @@ -1672,47 +1674,44 @@ export class ExtensionsWorkbenchService extends Disposable implements IExtension const targetPlatform = extension?.server ? await extension.server.extensionManagementService.getTargetPlatform() : undefined; gallery = firstOrDefault(await this.galleryService.getExtensions([installableInfo], { targetPlatform }, CancellationToken.None)); } - if (!gallery) { - const id = isString(arg) ? arg : (arg).identifier.id; - if (installOptions.version) { - throw new Error(nls.localize('not found version', "Unable to install extension '{0}' because the requested version '{1}' is not found.", id, installOptions.version)); - } else { - throw new Error(nls.localize('not found', "Unable to install extension '{0}' because it is not found.", id)); - } - } - if (!extension) { + if (!extension && gallery) { extension = this.instantiationService.createInstance(Extension, ext => this.getExtensionState(ext), ext => this.getReloadStatus(ext), undefined, undefined, gallery); + Extensions.updateExtensionFromControlManifest(extension as Extension, await this.extensionManagementService.getExtensionsControlManifest()); } if (extension?.isMalicious) { throw new Error(nls.localize('malicious', "This extension is reported to be problematic.")); } - installable = { extension, gallery }; - if (installOptions.version) { - installOptions.installGivenVersion = true; + // Do not install if requested to enable and extension is already installed + if (!(installOptions.enable && extension?.local)) { + if (!gallery) { + const id = isString(arg) ? arg : (arg).identifier.id; + if (installOptions.version) { + throw new Error(nls.localize('not found version', "Unable to install extension '{0}' because the requested version '{1}' is not found.", id, installOptions.version)); + } else { + throw new Error(nls.localize('not found', "Unable to install extension '{0}' because it is not found.", id)); + } + } + installable = gallery; + if (installOptions.version) { + installOptions.installGivenVersion = true; + } } } - let extension: IExtension; - if (installable instanceof URI || !(installOptions.enable && installable.extension.local)) { + if (installable) { if (installOptions.justification) { const syncCheck = isUndefined(installOptions.isMachineScoped) && this.userDataSyncEnablementService.isEnabled() && this.userDataSyncEnablementService.isResourceEnabled(SyncResource.Extensions); + const buttons: IPromptButton[] = []; + buttons.push({ label: isString(installOptions.justification) ? nls.localize({ key: 'installButtonLabel', comment: ['&& denotes a mnemonic'] }, "&&Install Extension") : nls.localize({ key: 'installButtonLabelWithAction', comment: ['&& denotes a mnemonic'] }, "&&Install Extension and {0}", installOptions.justification.action), run: () => true }); + if (!extension) { + buttons.push({ label: nls.localize('open', "Open Extension"), run: () => { this.open(extension!); return false; } }); + } const result = await this.dialogService.prompt({ title: nls.localize('installExtensionTitle', "Install Extension"), - message: installable instanceof URI ? nls.localize('installVSIXMessage', "Would you like to install the extension?") : nls.localize('installExtensionMessage', "Would you like to install '{0}' extension from '{1}'?", installable.extension.displayName, installable.extension.publisherDisplayName), + message: extension ? nls.localize('installExtensionMessage', "Would you like to install '{0}' extension from '{1}'?", extension.displayName, extension.publisherDisplayName) : nls.localize('installVSIXMessage', "Would you like to install the extension?"), detail: isString(installOptions.justification) ? installOptions.justification : installOptions.justification.reason, cancelButton: true, - buttons: [{ - label: isString(installOptions.justification) ? nls.localize({ key: 'installButtonLabel', comment: ['&& denotes a mnemonic'] }, "&&Install Extension") : nls.localize({ key: 'installButtonLabelWithAction', comment: ['&& denotes a mnemonic'] }, "&&Install Extension and {0}", installOptions.justification.action), - run: () => true - }, { - label: nls.localize('open', "Open Extension"), - run: () => { - if (!(installable instanceof URI)) { - this.open(installable.extension); - } - return false; - } - }], + buttons, checkbox: syncCheck ? { label: nls.localize('sync extension', "Sync this extension"), checked: true, @@ -1725,11 +1724,15 @@ export class ExtensionsWorkbenchService extends Disposable implements IExtension installOptions.isMachineScoped = !result.checkboxChecked; } } - extension = await this.doInstall(installable instanceof URI ? installable : installable.extension, - () => installable instanceof URI ? this.installFromVSIX(installable, installOptions) : this.installFromGallery(installable.extension, installable.gallery, installOptions), - progressLocation); - } else { - extension = installable.extension; + if (installable instanceof URI) { + extension = await this.doInstall(undefined, () => this.installFromVSIX(installable, installOptions), progressLocation); + } else if (extension) { + extension = await this.doInstall(extension, () => this.installFromGallery(extension!, installable, installOptions), progressLocation); + } + } + + if (!extension) { + throw new Error(nls.localize('unknown', "Unable to install extension")); } if (installOptions.version) { @@ -1902,21 +1905,21 @@ export class ExtensionsWorkbenchService extends Disposable implements IExtension return extension; } - private doInstall(extension: IExtension | URI, installTask: () => Promise, progressLocation?: ProgressLocation): Promise { - const title = extension instanceof URI ? nls.localize('installing extension', 'Installing extension....') : nls.localize('installing named extension', "Installing '{0}' extension....", extension.displayName); + private doInstall(extension: IExtension | undefined, installTask: () => Promise, progressLocation?: ProgressLocation): Promise { + const title = extension ? nls.localize('installing named extension', "Installing '{0}' extension....", extension.displayName) : nls.localize('installing extension', 'Installing extension....'); return this.withProgress({ location: progressLocation ?? ProgressLocation.Extensions, title }, async () => { try { - if (!(extension instanceof URI)) { + if (extension) { this.installing.push(extension); this._onChange.fire(extension); } const local = await installTask(); return await this.waitAndGetInstalledExtension(local.identifier); } finally { - if (!(extension instanceof URI)) { + if (extension) { this.installing = this.installing.filter(e => e !== extension); // Trigger the change without passing the extension because it is replaced by a new instance. this._onChange.fire(undefined); diff --git a/src/vs/workbench/contrib/extensions/test/electron-sandbox/extensionsWorkbenchService.test.ts b/src/vs/workbench/contrib/extensions/test/electron-sandbox/extensionsWorkbenchService.test.ts index ab080153b5e..39d5869cd3c 100644 --- a/src/vs/workbench/contrib/extensions/test/electron-sandbox/extensionsWorkbenchService.test.ts +++ b/src/vs/workbench/contrib/extensions/test/electron-sandbox/extensionsWorkbenchService.test.ts @@ -365,7 +365,6 @@ suite('ExtensionsWorkbenchServiceTest', () => { const extension = page.firstPage[0]; assert.strictEqual(ExtensionState.Uninstalled, extension.state); - testObject.install(extension); const identifier = gallery.identifier; // Installing @@ -450,7 +449,6 @@ suite('ExtensionsWorkbenchServiceTest', () => { const extension = page.firstPage[0]; assert.strictEqual(ExtensionState.Uninstalled, extension.state); - testObject.install(extension); installEvent.fire({ identifier: gallery.identifier, source: gallery }); const promise = Event.toPromise(testObject.onChange); @@ -470,7 +468,6 @@ suite('ExtensionsWorkbenchServiceTest', () => { const extension = page.firstPage[0]; assert.strictEqual(ExtensionState.Uninstalled, extension.state); - testObject.install(extension); disposableStore.add(testObject.onChange(target)); // Installing From 85a26649de3502073cfcc86511d316761f025d4a Mon Sep 17 00:00:00 2001 From: Alex Ross Date: Wed, 28 Feb 2024 16:01:25 +0100 Subject: [PATCH 03/18] Add resource hint API for commenting range provider (#206444) Part of #185551 --- src/vs/editor/common/languages.ts | 8 +++++ .../api/browser/mainThreadComments.ts | 8 ++--- .../workbench/api/common/extHost.protocol.ts | 2 +- .../workbench/api/common/extHostComments.ts | 5 ++- .../comments/browser/commentService.ts | 14 +++++--- .../comments/browser/commentsController.ts | 33 +++++++++++++++---- .../common/extensionsApiProposals.ts | 1 + .../vscode.proposed.commentingRangeHint.d.ts | 16 +++++++++ 8 files changed, 70 insertions(+), 17 deletions(-) create mode 100644 src/vscode-dts/vscode.proposed.commentingRangeHint.d.ts diff --git a/src/vs/editor/common/languages.ts b/src/vs/editor/common/languages.ts index 16550bdef8d..57f11ed3e4e 100644 --- a/src/vs/editor/common/languages.ts +++ b/src/vs/editor/common/languages.ts @@ -1710,6 +1710,14 @@ export interface CommentInfo { commentingRanges: CommentingRanges; } + +/** + * @internal + */ +export interface CommentingRangeResourceHint { + schemes: readonly string[]; +} + /** * @internal */ diff --git a/src/vs/workbench/api/browser/mainThreadComments.ts b/src/vs/workbench/api/browser/mainThreadComments.ts index 538c754e1b3..4e369eb19da 100644 --- a/src/vs/workbench/api/browser/mainThreadComments.ts +++ b/src/vs/workbench/api/browser/mainThreadComments.ts @@ -370,8 +370,8 @@ export class MainThreadCommentController implements ICommentController { } } - updateCommentingRanges() { - this._commentService.updateCommentingRanges(this._uniqueId); + updateCommentingRanges(resourceHints?: languages.CommentingRangeResourceHint) { + this._commentService.updateCommentingRanges(this._uniqueId, resourceHints); } private getKnownThread(commentThreadHandle: number): MainThreadCommentThread { @@ -591,14 +591,14 @@ export class MainThreadComments extends Disposable implements MainThreadComments return provider.deleteCommentThread(commentThreadHandle); } - $updateCommentingRanges(handle: number) { + $updateCommentingRanges(handle: number, resourceHints?: languages.CommentingRangeResourceHint) { const provider = this._commentControllers.get(handle); if (!provider) { return; } - provider.updateCommentingRanges(); + provider.updateCommentingRanges(resourceHints); } private registerView(commentsViewAlreadyRegistered: boolean) { diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index f57a727dd33..dda01854f21 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -145,7 +145,7 @@ export interface MainThreadCommentsShape extends IDisposable { $createCommentThread(handle: number, commentThreadHandle: number, threadId: string, resource: UriComponents, range: IRange | ICellRange | undefined, extensionId: ExtensionIdentifier, isTemplate: boolean): languages.CommentThread | undefined; $updateCommentThread(handle: number, commentThreadHandle: number, threadId: string, resource: UriComponents, changes: CommentThreadChanges): void; $deleteCommentThread(handle: number, commentThreadHandle: number): void; - $updateCommentingRanges(handle: number): void; + $updateCommentingRanges(handle: number, resourceHints?: languages.CommentingRangeResourceHint): void; } export interface AuthenticationForceNewSessionOptions { diff --git a/src/vs/workbench/api/common/extHostComments.ts b/src/vs/workbench/api/common/extHostComments.ts index 9db6a543d21..3e20208c247 100644 --- a/src/vs/workbench/api/common/extHostComments.ts +++ b/src/vs/workbench/api/common/extHostComments.ts @@ -565,7 +565,10 @@ export function createExtHostComments(mainContext: IMainContext, commands: ExtHo set commentingRangeProvider(provider: vscode.CommentingRangeProvider | undefined) { this._commentingRangeProvider = provider; - proxy.$updateCommentingRanges(this.handle); + if (provider?.resourceHints) { + checkProposedApiEnabled(this._extension, 'commentingRangeHint'); + } + proxy.$updateCommentingRanges(this.handle, provider?.resourceHints); } private _reactionHandler?: ReactionHandler; diff --git a/src/vs/workbench/contrib/comments/browser/commentService.ts b/src/vs/workbench/contrib/comments/browser/commentService.ts index c4747664064..da253044635 100644 --- a/src/vs/workbench/contrib/comments/browser/commentService.ts +++ b/src/vs/workbench/contrib/comments/browser/commentService.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { CommentThreadChangedEvent, CommentInfo, Comment, CommentReaction, CommentingRanges, CommentThread, CommentOptions, PendingCommentThread } from 'vs/editor/common/languages'; +import { CommentThreadChangedEvent, CommentInfo, Comment, CommentReaction, CommentingRanges, CommentThread, CommentOptions, PendingCommentThread, CommentingRangeResourceHint } from 'vs/editor/common/languages'; import { createDecorator, IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { Event, Emitter } from 'vs/base/common/event'; import { Disposable, DisposableStore, IDisposable } from 'vs/base/common/lifecycle'; @@ -104,7 +104,7 @@ export interface ICommentService { disposeCommentThread(ownerId: string, threadId: string): void; getDocumentComments(resource: URI): Promise<(ICommentInfo | null)[]>; getNotebookComments(resource: URI): Promise<(INotebookCommentInfo | null)[]>; - updateCommentingRanges(ownerId: string): void; + updateCommentingRanges(ownerId: string, resourceHints?: CommentingRangeResourceHint): void; hasReactionHandler(owner: string): boolean; toggleReaction(owner: string, resource: URI, thread: CommentThread, comment: Comment, reaction: CommentReaction): Promise; setActiveEditingCommentThread(commentThread: CommentThread | null): void; @@ -172,6 +172,7 @@ export class CommentService extends Disposable implements ICommentService { public readonly commentsModel: ICommentsModel = this._commentsModel; private _commentingRangeResources = new Set(); // URIs + private _commentingRangeResourceHintSchemes = new Set(); // schemes constructor( @IInstantiationService protected readonly instantiationService: IInstantiationService, @@ -406,7 +407,12 @@ export class CommentService extends Disposable implements ICommentService { this._onDidUpdateNotebookCommentThreads.fire(evt); } - updateCommentingRanges(ownerId: string) { + updateCommentingRanges(ownerId: string, resourceHints?: CommentingRangeResourceHint) { + if (resourceHints?.schemes && resourceHints.schemes.length > 0) { + for (const scheme of resourceHints.schemes) { + this._commentingRangeResourceHintSchemes.add(scheme); + } + } this._workspaceHasCommenting.set(true); this._onDidUpdateCommentingRanges.fire({ owner: ownerId }); } @@ -518,6 +524,6 @@ export class CommentService extends Disposable implements ICommentService { } resourceHasCommentingRanges(resource: URI): boolean { - return this._commentingRangeResources.has(resource.toString()); + return this._commentingRangeResourceHintSchemes.has(resource.scheme) || this._commentingRangeResources.has(resource.toString()); } } diff --git a/src/vs/workbench/contrib/comments/browser/commentsController.ts b/src/vs/workbench/contrib/comments/browser/commentsController.ts index 1fdee1beb9f..35c0cd6eb7f 100644 --- a/src/vs/workbench/contrib/comments/browser/commentsController.ts +++ b/src/vs/workbench/contrib/comments/browser/commentsController.ts @@ -376,6 +376,7 @@ export class CommentController implements IEditorContribution { private _commentThreadRangeDecorator!: CommentThreadRangeDecorator; private mouseDownInfo: { lineNumber: number } | null = null; private _commentingRangeSpaceReserved = false; + private _commentingRangeAmountReserved = 0; private _computePromise: CancelablePromise> | null; private _addInProgress!: boolean; private _emptyThreadsToAddQueue: [Range | undefined, IEditorMouseEvent | undefined][] = []; @@ -1179,15 +1180,20 @@ export class CommentController implements IEditorContribution { return { extraEditorClassName, lineDecorationsWidth }; } - private getWithCommentsEditorOptions(editor: ICodeEditor, extraEditorClassName: string[], startingLineDecorationsWidth: number) { + private getWithCommentsLineDecorationWidth(editor: ICodeEditor, startingLineDecorationsWidth: number) { let lineDecorationsWidth = startingLineDecorationsWidth; const options = editor.getOptions(); if (options.get(EditorOption.folding) && options.get(EditorOption.showFoldingControls) !== 'never') { lineDecorationsWidth -= 11; } lineDecorationsWidth += 24; + this._commentingRangeAmountReserved = lineDecorationsWidth; + return this._commentingRangeAmountReserved; + } + + private getWithCommentsEditorOptions(editor: ICodeEditor, extraEditorClassName: string[], startingLineDecorationsWidth: number) { extraEditorClassName.push('inline-comment'); - return { lineDecorationsWidth, extraEditorClassName }; + return { lineDecorationsWidth: this.getWithCommentsLineDecorationWidth(editor, startingLineDecorationsWidth), extraEditorClassName }; } private updateEditorLayoutOptions(editor: ICodeEditor, extraEditorClassName: string[], lineDecorationsWidth: number) { @@ -1197,6 +1203,15 @@ export class CommentController implements IEditorContribution { }); } + private ensureCommentingRangeReservedAmount(editor: ICodeEditor) { + const existing = this.getExistingCommentEditorOptions(editor); + if (existing.lineDecorationsWidth !== this._commentingRangeAmountReserved) { + editor.updateOptions({ + lineDecorationsWidth: this.getWithCommentsLineDecorationWidth(editor, existing.lineDecorationsWidth) + }); + } + } + private tryUpdateReservedSpace(uri?: URI) { if (!this.editor) { return; @@ -1211,11 +1226,15 @@ export class CommentController implements IEditorContribution { const hasCommentsOrRanges = hasCommentsOrRangesInInfo || resourceHasCommentingRanges; - if (hasCommentsOrRanges && !this._commentingRangeSpaceReserved && this.commentService.isCommentingEnabled) { - this._commentingRangeSpaceReserved = true; - const { lineDecorationsWidth, extraEditorClassName } = this.getExistingCommentEditorOptions(this.editor); - const newOptions = this.getWithCommentsEditorOptions(this.editor, extraEditorClassName, lineDecorationsWidth); - this.updateEditorLayoutOptions(this.editor, newOptions.extraEditorClassName, newOptions.lineDecorationsWidth); + if (hasCommentsOrRanges && this.commentService.isCommentingEnabled) { + if (!this._commentingRangeSpaceReserved) { + this._commentingRangeSpaceReserved = true; + const { lineDecorationsWidth, extraEditorClassName } = this.getExistingCommentEditorOptions(this.editor); + const newOptions = this.getWithCommentsEditorOptions(this.editor, extraEditorClassName, lineDecorationsWidth); + this.updateEditorLayoutOptions(this.editor, newOptions.extraEditorClassName, newOptions.lineDecorationsWidth); + } else { + this.ensureCommentingRangeReservedAmount(this.editor); + } } else if ((!hasCommentsOrRanges || !this.commentService.isCommentingEnabled) && this._commentingRangeSpaceReserved) { this._commentingRangeSpaceReserved = false; const { lineDecorationsWidth, extraEditorClassName } = this.getExistingCommentEditorOptions(this.editor); diff --git a/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts b/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts index 7386529c446..071545be93b 100644 --- a/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts +++ b/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts @@ -21,6 +21,7 @@ export const allApiProposals = Object.freeze({ codeActionRanges: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.codeActionRanges.d.ts', codiconDecoration: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.codiconDecoration.d.ts', commentReactor: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.commentReactor.d.ts', + commentingRangeHint: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.commentingRangeHint.d.ts', commentsDraftState: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.commentsDraftState.d.ts', contribCommentEditorActionsMenu: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.contribCommentEditorActionsMenu.d.ts', contribCommentPeekContext: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.contribCommentPeekContext.d.ts', diff --git a/src/vscode-dts/vscode.proposed.commentingRangeHint.d.ts b/src/vscode-dts/vscode.proposed.commentingRangeHint.d.ts new file mode 100644 index 00000000000..595e4f0f62a --- /dev/null +++ b/src/vscode-dts/vscode.proposed.commentingRangeHint.d.ts @@ -0,0 +1,16 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +declare module 'vscode' { + + // @alexr00 https://github.com/microsoft/vscode/issues/185551 + + /** + * Commenting range provider for a {@link CommentController comment controller}. + */ + export interface CommentingRangeProvider { + readonly resourceHints?: { schemes: readonly string[] }; + } +} From cff275ae6414cf8ea7e4266b7e84e9eea4459161 Mon Sep 17 00:00:00 2001 From: Benjamin Christopher Simmonds <44439583+benibenj@users.noreply.github.com> Date: Wed, 28 Feb 2024 16:37:07 +0100 Subject: [PATCH 04/18] Adopt custom hover in settings and keybindings editor (#206440) * adopt custom hover in settings and keybindings editor * fiy smoketests --- .../ui/highlightedlabel/highlightedLabel.ts | 2 +- .../browser/ui/iconLabel/simpleIconLabel.ts | 17 ++++++++-- .../ui/keybindingLabel/keybindingLabel.ts | 17 ++++++---- src/vs/base/browser/ui/toggle/toggle.ts | 1 + .../editor/contrib/find/browser/findWidget.ts | 1 + .../browser/inlineCompletionsHintsWidget.ts | 2 +- .../browser/inlineEditHintsWidget.ts | 2 +- .../actionWidget/browser/actionList.ts | 2 +- .../quickinput/browser/quickInputList.ts | 1 + .../parts/editor/editorGroupWatermark.ts | 7 ++-- .../browser/parts/editor/editorPlaceholder.ts | 2 +- .../browser/parts/statusbar/statusbarItem.ts | 2 +- .../emptyTextEditorHint.ts | 6 ++-- .../browser/extensionFeaturesTab.ts | 13 +++++--- .../browser/view/cellParts/cellStatusPart.ts | 2 +- .../preferences/browser/keybindingWidgets.ts | 6 ++-- .../preferences/browser/keybindingsEditor.ts | 32 ++++++++++++------- .../preferences/browser/preferencesWidgets.ts | 8 +++-- .../settingsEditorSettingIndicators.ts | 20 ++++++------ .../preferences/browser/settingsTree.ts | 15 +++++---- .../preferences/browser/settingsWidgets.ts | 20 ++++++------ .../contrib/preferences/browser/tocTree.ts | 12 +++++-- test/automation/src/keybindings.ts | 6 ++-- 23 files changed, 127 insertions(+), 69 deletions(-) diff --git a/src/vs/base/browser/ui/highlightedlabel/highlightedLabel.ts b/src/vs/base/browser/ui/highlightedlabel/highlightedLabel.ts index 5aaa8bcc551..4847da97d2f 100644 --- a/src/vs/base/browser/ui/highlightedlabel/highlightedLabel.ts +++ b/src/vs/base/browser/ui/highlightedlabel/highlightedLabel.ts @@ -140,7 +140,7 @@ export class HighlightedLabel extends Disposable { } else { if (!this.customHover && this.title !== '') { const hoverDelegate = this.options?.hoverDelegate ?? getDefaultHoverDelegate('mouse'); - this.customHover = this._store.add(setupCustomHover(hoverDelegate, this.domNode, this.title)); + this.customHover = this._register(setupCustomHover(hoverDelegate, this.domNode, this.title)); } else if (this.customHover) { this.customHover.update(this.title); } diff --git a/src/vs/base/browser/ui/iconLabel/simpleIconLabel.ts b/src/vs/base/browser/ui/iconLabel/simpleIconLabel.ts index 659572d4ff8..dc35bd8a9ab 100644 --- a/src/vs/base/browser/ui/iconLabel/simpleIconLabel.ts +++ b/src/vs/base/browser/ui/iconLabel/simpleIconLabel.ts @@ -4,9 +4,14 @@ *--------------------------------------------------------------------------------------------*/ import { reset } from 'vs/base/browser/dom'; +import { getDefaultHoverDelegate } from 'vs/base/browser/ui/hover/hoverDelegateFactory'; +import { ICustomHover, setupCustomHover } from 'vs/base/browser/ui/hover/updatableHoverWidget'; import { renderLabelWithIcons } from 'vs/base/browser/ui/iconLabel/iconLabels'; +import { IDisposable } from 'vs/base/common/lifecycle'; -export class SimpleIconLabel { +export class SimpleIconLabel implements IDisposable { + + private hover?: ICustomHover; constructor( private readonly _container: HTMLElement @@ -17,6 +22,14 @@ export class SimpleIconLabel { } set title(title: string) { - this._container.title = title; + if (!this.hover && title) { + this.hover = setupCustomHover(getDefaultHoverDelegate('mouse'), this._container, title); + } else if (this.hover) { + this.hover.update(title); + } + } + + dispose(): void { + this.hover?.dispose(); } } diff --git a/src/vs/base/browser/ui/keybindingLabel/keybindingLabel.ts b/src/vs/base/browser/ui/keybindingLabel/keybindingLabel.ts index 431e33048cd..20eea5d8850 100644 --- a/src/vs/base/browser/ui/keybindingLabel/keybindingLabel.ts +++ b/src/vs/base/browser/ui/keybindingLabel/keybindingLabel.ts @@ -4,8 +4,11 @@ *--------------------------------------------------------------------------------------------*/ import * as dom from 'vs/base/browser/dom'; +import { getDefaultHoverDelegate } from 'vs/base/browser/ui/hover/hoverDelegateFactory'; +import { ICustomHover, setupCustomHover } from 'vs/base/browser/ui/hover/updatableHoverWidget'; import { UILabelProvider } from 'vs/base/common/keybindingLabels'; import { ResolvedKeybinding, ResolvedChord } from 'vs/base/common/keybindings'; +import { Disposable } from 'vs/base/common/lifecycle'; import { equals } from 'vs/base/common/objects'; import { OperatingSystem } from 'vs/base/common/platform'; import 'vs/css!./keybindingLabel'; @@ -50,18 +53,21 @@ export const unthemedKeybindingLabelOptions: KeybindingLabelOptions = { keybindingLabelShadow: undefined }; -export class KeybindingLabel { +export class KeybindingLabel extends Disposable { private domNode: HTMLElement; private options: KeybindingLabelOptions; private readonly keyElements = new Set(); + private hover: ICustomHover; private keybinding: ResolvedKeybinding | undefined; private matches: Matches | undefined; private didEverRender: boolean; constructor(container: HTMLElement, private os: OperatingSystem, options?: KeybindingLabelOptions) { + super(); + this.options = options || Object.create(null); const labelForeground = this.options.keybindingLabelForeground; @@ -71,6 +77,8 @@ export class KeybindingLabel { this.domNode.style.color = labelForeground; } + this.hover = this._register(setupCustomHover(getDefaultHoverDelegate('mouse'), this.domNode, '')); + this.didEverRender = false; container.appendChild(this.domNode); } @@ -102,11 +110,8 @@ export class KeybindingLabel { this.renderChord(this.domNode, chords[i], this.matches ? this.matches.chordPart : null); } const title = (this.options.disableTitle ?? false) ? undefined : this.keybinding.getAriaLabel() || undefined; - if (title !== undefined) { - this.domNode.title = title; - } else { - this.domNode.removeAttribute('title'); - } + this.hover.update(title); + this.domNode.setAttribute('aria-label', title || ''); } else if (this.options && this.options.renderUnboundKeybindings) { this.renderUnbound(this.domNode); } diff --git a/src/vs/base/browser/ui/toggle/toggle.ts b/src/vs/base/browser/ui/toggle/toggle.ts index 53e0e2b9a30..22d60e98508 100644 --- a/src/vs/base/browser/ui/toggle/toggle.ts +++ b/src/vs/base/browser/ui/toggle/toggle.ts @@ -59,6 +59,7 @@ export class ToggleActionViewItem extends BaseActionViewItem { inputActiveOptionBackground: options.toggleStyles?.inputActiveOptionBackground, inputActiveOptionBorder: options.toggleStyles?.inputActiveOptionBorder, inputActiveOptionForeground: options.toggleStyles?.inputActiveOptionForeground, + hoverDelegate: options.hoverDelegate })); this._register(this.toggle.onChange(() => this._action.checked = !!this.toggle && this.toggle.checked)); } diff --git a/src/vs/editor/contrib/find/browser/findWidget.ts b/src/vs/editor/contrib/find/browser/findWidget.ts index 424375e8cae..8d80095419d 100644 --- a/src/vs/editor/contrib/find/browser/findWidget.ts +++ b/src/vs/editor/contrib/find/browser/findWidget.ts @@ -1051,6 +1051,7 @@ export class FindWidget extends Widget implements IOverlayWidget, IVerticalSashL icon: findSelectionIcon, title: NLS_TOGGLE_SELECTION_FIND_TITLE + this._keybindingLabelFor(FIND_IDS.ToggleSearchScopeCommand), isChecked: false, + hoverDelegate: hoverDelegate, inputActiveOptionBackground: asCssVariable(inputActiveOptionBackground), inputActiveOptionBorder: asCssVariable(inputActiveOptionBorder), inputActiveOptionForeground: asCssVariable(inputActiveOptionForeground), diff --git a/src/vs/editor/contrib/inlineCompletions/browser/inlineCompletionsHintsWidget.ts b/src/vs/editor/contrib/inlineCompletions/browser/inlineCompletionsHintsWidget.ts index 30f622e01fc..e847839c042 100644 --- a/src/vs/editor/contrib/inlineCompletions/browser/inlineCompletionsHintsWidget.ts +++ b/src/vs/editor/contrib/inlineCompletions/browser/inlineCompletionsHintsWidget.ts @@ -302,7 +302,7 @@ class StatusBarViewItem extends MenuEntryActionViewItem { if (this.label) { const div = h('div.keybinding').root; - const k = new KeybindingLabel(div, OS, { disableTitle: true, ...unthemedKeybindingLabelOptions }); + const k = this._register(new KeybindingLabel(div, OS, { disableTitle: true, ...unthemedKeybindingLabelOptions })); k.set(kb); this.label.textContent = this._action.label; this.label.appendChild(div); diff --git a/src/vs/editor/contrib/inlineEdit/browser/inlineEditHintsWidget.ts b/src/vs/editor/contrib/inlineEdit/browser/inlineEditHintsWidget.ts index 73824bd5e6c..59553805863 100644 --- a/src/vs/editor/contrib/inlineEdit/browser/inlineEditHintsWidget.ts +++ b/src/vs/editor/contrib/inlineEdit/browser/inlineEditHintsWidget.ts @@ -175,7 +175,7 @@ class StatusBarViewItem extends MenuEntryActionViewItem { if (this.label) { const div = h('div.keybinding').root; - const k = new KeybindingLabel(div, OS, { disableTitle: true, ...unthemedKeybindingLabelOptions }); + const k = this._register(new KeybindingLabel(div, OS, { disableTitle: true, ...unthemedKeybindingLabelOptions })); k.set(kb); this.label.textContent = this._action.label; this.label.appendChild(div); diff --git a/src/vs/platform/actionWidget/browser/actionList.ts b/src/vs/platform/actionWidget/browser/actionList.ts index 4b0b93f695d..80f8e74664e 100644 --- a/src/vs/platform/actionWidget/browser/actionList.ts +++ b/src/vs/platform/actionWidget/browser/actionList.ts @@ -140,7 +140,7 @@ class ActionItemRenderer implements IListRenderer, IAction } disposeTemplate(_templateData: IActionMenuTemplateData): void { - // noop + _templateData.keybinding.dispose(); } } diff --git a/src/vs/platform/quickinput/browser/quickInputList.ts b/src/vs/platform/quickinput/browser/quickInputList.ts index 26e32890647..98993dd9070 100644 --- a/src/vs/platform/quickinput/browser/quickInputList.ts +++ b/src/vs/platform/quickinput/browser/quickInputList.ts @@ -277,6 +277,7 @@ class ListElementRenderer implements IListRenderer { clearNode(container); - this.renderTable(data, container); + tableDisposable.value = this.renderTable(data, container); })); } - this.renderTable(tableData.data, container); + tableDisposable.value = this.renderTable(tableData.data, container); } - private renderTable(tableData: ITableData, container: HTMLElement): void { + private renderTable(tableData: ITableData, container: HTMLElement): IDisposable { + const disposables = new DisposableStore(); append(container, $('table', undefined, $('tr', undefined, @@ -478,7 +480,7 @@ class ExtensionFeatureView extends Disposable { result.push(element); } else if (item instanceof ResolvedKeybinding) { const element = $(''); - const kbl = new KeybindingLabel(element, OS, defaultKeybindingLabelStyles); + const kbl = disposables.add(new KeybindingLabel(element, OS, defaultKeybindingLabelStyles)); kbl.set(item); result.push(element); } else if (item instanceof Color) { @@ -490,6 +492,7 @@ class ExtensionFeatureView extends Disposable { }) ); }))); + return disposables; } private renderMarkdownData(container: HTMLElement, renderer: IExtensionFeatureMarkdownRenderer): void { diff --git a/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellStatusPart.ts b/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellStatusPart.ts index b5c306d0b41..88c4252fd82 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellStatusPart.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellStatusPart.ts @@ -294,7 +294,7 @@ class CellStatusBarItem extends Disposable { this._itemDisposables.clear(); if (!this._currentItem || this._currentItem.text !== item.text) { - new SimpleIconLabel(this.container).text = item.text.replace(/\n/g, ' '); + this._itemDisposables.add(new SimpleIconLabel(this.container)).text = item.text.replace(/\n/g, ' '); } const resolveColor = (color: ThemeColor | string) => { diff --git a/src/vs/workbench/contrib/preferences/browser/keybindingWidgets.ts b/src/vs/workbench/contrib/preferences/browser/keybindingWidgets.ts index 70920e27b23..2f92489d4b8 100644 --- a/src/vs/workbench/contrib/preferences/browser/keybindingWidgets.ts +++ b/src/vs/workbench/contrib/preferences/browser/keybindingWidgets.ts @@ -141,6 +141,7 @@ export class DefineKeybindingWidget extends Widget { private _keybindingInputWidget: KeybindingsSearchWidget; private _outputNode: HTMLElement; private _showExistingKeybindingsNode: HTMLElement; + private _keybindingDisposables = this._register(new DisposableStore()); private _chords: ResolvedKeybinding[] | null = null; private _isVisible: boolean = false; @@ -238,17 +239,18 @@ export class DefineKeybindingWidget extends Widget { } private onKeybinding(keybinding: ResolvedKeybinding[] | null): void { + this._keybindingDisposables.clear(); this._chords = keybinding; dom.clearNode(this._outputNode); dom.clearNode(this._showExistingKeybindingsNode); - const firstLabel = new KeybindingLabel(this._outputNode, OS, defaultKeybindingLabelStyles); + const firstLabel = this._keybindingDisposables.add(new KeybindingLabel(this._outputNode, OS, defaultKeybindingLabelStyles)); firstLabel.set(this._chords?.[0] ?? undefined); if (this._chords) { for (let i = 1; i < this._chords.length; i++) { this._outputNode.appendChild(document.createTextNode(nls.localize('defineKeybinding.chordsTo', "chord to"))); - const chordLabel = new KeybindingLabel(this._outputNode, OS, defaultKeybindingLabelStyles); + const chordLabel = this._keybindingDisposables.add(new KeybindingLabel(this._outputNode, OS, defaultKeybindingLabelStyles)); chordLabel.set(this._chords[i]); } } diff --git a/src/vs/workbench/contrib/preferences/browser/keybindingsEditor.ts b/src/vs/workbench/contrib/preferences/browser/keybindingsEditor.ts index b673830c1e9..f9b434adf8f 100644 --- a/src/vs/workbench/contrib/preferences/browser/keybindingsEditor.ts +++ b/src/vs/workbench/contrib/preferences/browser/keybindingsEditor.ts @@ -58,6 +58,8 @@ import { IConfigurationService } from 'vs/platform/configuration/common/configur import { AccessibilityVerbositySettingId } from 'vs/workbench/contrib/accessibility/browser/accessibilityConfiguration'; import { registerNavigableContainer } from 'vs/workbench/browser/actions/widgetNavigationCommands'; import { IActionViewItemOptions } from 'vs/base/browser/ui/actionbar/actionViewItems'; +import { ICustomHover, setupCustomHover } from 'vs/base/browser/ui/hover/updatableHoverWidget'; +import { getDefaultHoverDelegate } from 'vs/base/browser/ui/hover/hoverDelegateFactory'; const $ = DOM.$; @@ -897,6 +899,7 @@ class ActionsColumnRenderer implements ITableRenderer(extensionContainer, $('a.extension-label', { tabindex: 0 })); const extensionId = new HighlightedLabel(DOM.append(extensionContainer, $('.extension-id-container.code'))); - return { sourceColumn, sourceLabel, extensionLabel, extensionContainer, extensionId, disposables: new DisposableStore() }; + return { sourceColumn, sourceColumnHover, sourceLabel, extensionLabel, extensionContainer, extensionId, disposables: new DisposableStore() }; } renderElement(keybindingItemEntry: IKeybindingItemEntry, index: number, templateData: ISourceColumnTemplateData, height: number | undefined): void { @@ -1039,14 +1049,14 @@ class SourceColumnRenderer implements ITableRenderer { this.extensionsWorkbenchService.open(extension.identifier.value); @@ -1062,6 +1072,7 @@ class SourceColumnRenderer implements ITableRenderer DOM.EventHelper.stop(e))); this._register(DOM.addDisposableListener(this.anchorElement, DOM.EventType.CLICK, e => this.onClick(e))); this._register(DOM.addDisposableListener(this.container, DOM.EventType.KEY_UP, e => this.onKeyUp(e))); @@ -145,7 +149,7 @@ export class FolderSettingsActionViewItem extends BaseActionViewItem { const workspace = this.contextService.getWorkspace(); if (this._folder) { this.labelElement.textContent = this._folder.name; - this.anchorElement.title = this._folder.name; + this.anchorElementHover.update(this._folder.name); const detailsText = this.labelWithCount(this._action.label, total); this.detailsElement.textContent = detailsText; this.dropDownElement.classList.toggle('hide', workspace.folders.length === 1 || !this._action.checked); @@ -153,7 +157,7 @@ export class FolderSettingsActionViewItem extends BaseActionViewItem { const labelText = this.labelWithCount(this._action.label, total); this.labelElement.textContent = labelText; this.detailsElement.textContent = ''; - this.anchorElement.title = this._action.label; + this.anchorElementHover.update(this._action.label); this.dropDownElement.classList.remove('hide'); } diff --git a/src/vs/workbench/contrib/preferences/browser/settingsEditorSettingIndicators.ts b/src/vs/workbench/contrib/preferences/browser/settingsEditorSettingIndicators.ts index cf98a39a26e..fd8f621714f 100644 --- a/src/vs/workbench/contrib/preferences/browser/settingsEditorSettingIndicators.ts +++ b/src/vs/workbench/contrib/preferences/browser/settingsEditorSettingIndicators.ts @@ -135,12 +135,12 @@ export class SettingsTreeIndicatorsLabel implements IDisposable { } private createWorkspaceTrustIndicator(): SettingIndicator { + const disposables = new DisposableStore(); const workspaceTrustElement = $('span.setting-indicator.setting-item-workspace-trust'); - const workspaceTrustLabel = new SimpleIconLabel(workspaceTrustElement); + const workspaceTrustLabel = disposables.add(new SimpleIconLabel(workspaceTrustElement)); workspaceTrustLabel.text = '$(warning) ' + localize('workspaceUntrustedLabel', "Setting value not applied"); const content = localize('trustLabel', "The setting value can only be applied in a trusted workspace."); - const disposables = new DisposableStore(); const showHover = (focus: boolean) => { return this.hoverService.showHover({ ...this.defaultHoverOptions, @@ -164,23 +164,24 @@ export class SettingsTreeIndicatorsLabel implements IDisposable { } private createScopeOverridesIndicator(): SettingIndicator { + const disposables = new DisposableStore(); // Don't add .setting-indicator class here, because it gets conditionally added later. const otherOverridesElement = $('span.setting-item-overrides'); - const otherOverridesLabel = new SimpleIconLabel(otherOverridesElement); + const otherOverridesLabel = disposables.add(new SimpleIconLabel(otherOverridesElement)); return { element: otherOverridesElement, label: otherOverridesLabel, - disposables: new DisposableStore() + disposables }; } private createSyncIgnoredIndicator(): SettingIndicator { + const disposables = new DisposableStore(); const syncIgnoredElement = $('span.setting-indicator.setting-item-ignored'); - const syncIgnoredLabel = new SimpleIconLabel(syncIgnoredElement); + const syncIgnoredLabel = disposables.add(new SimpleIconLabel(syncIgnoredElement)); syncIgnoredLabel.text = localize('extensionSyncIgnoredLabel', 'Not synced'); const syncIgnoredHoverContent = localize('syncIgnoredTitle', "This setting is ignored during sync"); - const disposables = new DisposableStore(); const showHover = (focus: boolean) => { return this.hoverService.showHover({ ...this.defaultHoverOptions, @@ -193,19 +194,20 @@ export class SettingsTreeIndicatorsLabel implements IDisposable { return { element: syncIgnoredElement, label: syncIgnoredLabel, - disposables: new DisposableStore() + disposables }; } private createDefaultOverrideIndicator(): SettingIndicator { + const disposables = new DisposableStore(); const defaultOverrideIndicator = $('span.setting-indicator.setting-item-default-overridden'); - const defaultOverrideLabel = new SimpleIconLabel(defaultOverrideIndicator); + const defaultOverrideLabel = disposables.add(new SimpleIconLabel(defaultOverrideIndicator)); defaultOverrideLabel.text = localize('defaultOverriddenLabel', "Default value changed"); return { element: defaultOverrideIndicator, label: defaultOverrideLabel, - disposables: new DisposableStore() + disposables }; } diff --git a/src/vs/workbench/contrib/preferences/browser/settingsTree.ts b/src/vs/workbench/contrib/preferences/browser/settingsTree.ts index 8a24222eac3..9b7e714c4ce 100644 --- a/src/vs/workbench/contrib/preferences/browser/settingsTree.ts +++ b/src/vs/workbench/contrib/preferences/browser/settingsTree.ts @@ -69,6 +69,8 @@ import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/ import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions'; import { ISetting, ISettingsGroup, SettingValueType } from 'vs/workbench/services/preferences/common/preferences'; import { getInvalidTypeError } from 'vs/workbench/services/preferences/common/preferencesValidation'; +import { setupCustomHover } from 'vs/base/browser/ui/hover/updatableHoverWidget'; +import { getDefaultHoverDelegate } from 'vs/base/browser/ui/hover/hoverDelegateFactory'; const $ = DOM.$; @@ -796,13 +798,13 @@ export abstract class AbstractSettingRenderer extends Disposable implements ITre const labelCategoryContainer = DOM.append(titleElement, $('.setting-item-cat-label-container')); const categoryElement = DOM.append(labelCategoryContainer, $('span.setting-item-category')); const labelElementContainer = DOM.append(labelCategoryContainer, $('span.setting-item-label')); - const labelElement = new SimpleIconLabel(labelElementContainer); + const labelElement = toDispose.add(new SimpleIconLabel(labelElementContainer)); const indicatorsLabel = this._instantiationService.createInstance(SettingsTreeIndicatorsLabel, titleElement); toDispose.add(indicatorsLabel); const descriptionElement = DOM.append(container, $('.setting-item-description')); const modifiedIndicatorElement = DOM.append(container, $('.setting-item-modified-indicator')); - modifiedIndicatorElement.title = localize('modified', "The setting has been configured in the current scope."); + toDispose.add(setupCustomHover(getDefaultHoverDelegate('mouse'), modifiedIndicatorElement, () => localize('modified', "The setting has been configured in the current scope."))); const valueElement = DOM.append(container, $('.setting-item-value')); const controlElement = DOM.append(valueElement, $('div.setting-item-control')); @@ -889,7 +891,7 @@ export abstract class AbstractSettingRenderer extends Disposable implements ITre const titleTooltip = setting.key + (element.isConfigured ? ' - Modified' : ''); template.categoryElement.textContent = element.displayCategory ? (element.displayCategory + ': ') : ''; - template.categoryElement.title = titleTooltip; + template.elementDisposables.add(setupCustomHover(getDefaultHoverDelegate('mouse'), template.categoryElement, titleTooltip)); template.labelElement.text = element.displayLabel; template.labelElement.title = titleTooltip; @@ -1817,24 +1819,25 @@ export class SettingBoolRenderer extends AbstractSettingRenderer implements ITre _container.classList.add('setting-item'); _container.classList.add('setting-item-bool'); + const toDispose = new DisposableStore(); + const container = DOM.append(_container, $(AbstractSettingRenderer.CONTENTS_SELECTOR)); container.classList.add('settings-row-inner-container'); const titleElement = DOM.append(container, $('.setting-item-title')); const categoryElement = DOM.append(titleElement, $('span.setting-item-category')); const labelElementContainer = DOM.append(titleElement, $('span.setting-item-label')); - const labelElement = new SimpleIconLabel(labelElementContainer); + const labelElement = toDispose.add(new SimpleIconLabel(labelElementContainer)); const indicatorsLabel = this._instantiationService.createInstance(SettingsTreeIndicatorsLabel, titleElement); const descriptionAndValueElement = DOM.append(container, $('.setting-item-value-description')); const controlElement = DOM.append(descriptionAndValueElement, $('.setting-item-bool-control')); const descriptionElement = DOM.append(descriptionAndValueElement, $('.setting-item-description')); const modifiedIndicatorElement = DOM.append(container, $('.setting-item-modified-indicator')); - modifiedIndicatorElement.title = localize('modified', "The setting has been configured in the current scope."); + toDispose.add(setupCustomHover(getDefaultHoverDelegate('mouse'), modifiedIndicatorElement, localize('modified', "The setting has been configured in the current scope."))); const deprecationWarningElement = DOM.append(container, $('.setting-item-deprecation-message')); - const toDispose = new DisposableStore(); const checkbox = new Toggle({ icon: Codicon.check, actionClassName: 'setting-value-checkbox', isChecked: true, title: '', ...unthemedToggleStyles }); controlElement.appendChild(checkbox.domNode); toDispose.add(checkbox); diff --git a/src/vs/workbench/contrib/preferences/browser/settingsWidgets.ts b/src/vs/workbench/contrib/preferences/browser/settingsWidgets.ts index 6675881177f..3cf230fa6ee 100644 --- a/src/vs/workbench/contrib/preferences/browser/settingsWidgets.ts +++ b/src/vs/workbench/contrib/preferences/browser/settingsWidgets.ts @@ -27,6 +27,8 @@ import { ThemeIcon } from 'vs/base/common/themables'; import { settingsDiscardIcon, settingsEditIcon, settingsRemoveIcon } from 'vs/workbench/contrib/preferences/browser/preferencesIcons'; import { settingsSelectBackground, settingsSelectBorder, settingsSelectForeground, settingsSelectListBorder, settingsTextInputBackground, settingsTextInputBorder, settingsTextInputForeground } from 'vs/workbench/contrib/preferences/common/settingsEditorColorRegistry'; import { defaultButtonStyles, getInputBoxStyle, getSelectBoxStyles } from 'vs/platform/theme/browser/defaultStyles'; +import { setupCustomHover } from 'vs/base/browser/ui/hover/updatableHoverWidget'; +import { getDefaultHoverDelegate } from 'vs/base/browser/ui/hover/hoverDelegateFactory'; const $ = DOM.$; @@ -673,8 +675,8 @@ export class ListSettingWidget extends AbstractListSettingWidget : localize('listSiblingHintLabel', "List item `{0}` with sibling `${1}`", value.data, sibling); const { rowElement } = rowElementGroup; - rowElement.title = title; - rowElement.setAttribute('aria-label', rowElement.title); + this.listDisposables.add(setupCustomHover(getDefaultHoverDelegate('mouse'), rowElement, title)); + rowElement.setAttribute('aria-label', title); } protected getLocalizedStrings() { @@ -733,8 +735,8 @@ export class ExcludeSettingWidget extends ListSettingWidget { : localize('excludeSiblingHintLabel', "Exclude files matching `{0}`, only when a file matching `{1}` is present", value.data, sibling); const { rowElement } = rowElementGroup; - rowElement.title = title; - rowElement.setAttribute('aria-label', rowElement.title); + this.listDisposables.add(setupCustomHover(getDefaultHoverDelegate('mouse'), rowElement, title)); + rowElement.setAttribute('aria-label', title); } protected override getLocalizedStrings() { @@ -763,8 +765,8 @@ export class IncludeSettingWidget extends ListSettingWidget { : localize('includeSiblingHintLabel', "Include files matching `{0}`, only when a file matching `{1}` is present", value.data, sibling); const { rowElement } = rowElementGroup; - rowElement.title = title; - rowElement.setAttribute('aria-label', rowElement.title); + this.listDisposables.add(setupCustomHover(getDefaultHoverDelegate('mouse'), rowElement, title)); + rowElement.setAttribute('aria-label', title); } protected override getLocalizedStrings() { @@ -1161,10 +1163,10 @@ export class ObjectSettingDropdownWidget extends AbstractListSettingWidget { @@ -111,17 +115,20 @@ export class TOCRenderer implements ITreeRenderer, index: number, template: ITOCEntryTemplate): void { + template.elementDisposables.clear(); + const element = node.element; const count = element.count; const label = element.label; template.labelElement.textContent = label; - template.labelElement.title = label; + template.elementDisposables.add(setupCustomHover(getDefaultHoverDelegate('mouse'), template.labelElement, label)); if (count) { template.countElement.textContent = ` (${count})`; @@ -131,6 +138,7 @@ export class TOCRenderer implements ITreeRenderer Date: Wed, 28 Feb 2024 08:57:08 -0800 Subject: [PATCH 05/18] Clear terminal markers when link is opened --- .../links/browser/terminalLinkQuickpick.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/vs/workbench/contrib/terminalContrib/links/browser/terminalLinkQuickpick.ts b/src/vs/workbench/contrib/terminalContrib/links/browser/terminalLinkQuickpick.ts index bfa7bbfb687..c35df5218e3 100644 --- a/src/vs/workbench/contrib/terminalContrib/links/browser/terminalLinkQuickpick.ts +++ b/src/vs/workbench/contrib/terminalContrib/links/browser/terminalLinkQuickpick.ts @@ -154,6 +154,16 @@ export class TerminalLinkQuickpick extends DisposableStore { r(); })); disposables.add(Event.once(pick.onDidAccept)(() => { + // Restore terminal scroll state + if (this._terminalScrollStateSaved) { + const markTracker = this._instance?.xterm?.markTracker; + if (markTracker) { + markTracker.restoreScrollState(); + markTracker.clear(); + this._terminalScrollStateSaved = false; + } + } + accepted = true; const event = new TerminalLinkQuickPickEvent(EventType.CLICK); const activeItem = pick.activeItems?.[0]; From 3f78333d3f31fc631c7c74ac188c42b0404de3e4 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 28 Feb 2024 19:02:06 +0100 Subject: [PATCH 06/18] chore - update distro (#206458) --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 947ec6bd4ee..f9142db514f 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "code-oss-dev", "version": "1.88.0", - "distro": "de07f23454d5352cc3711ca34d51278767e6eb0a", + "distro": "a5b6daf94540aab9d17335c2c2533e629d750123", "author": { "name": "Microsoft Corporation" }, From 29fe7ad5294d9acdccb0b703ea606c0782b5597b Mon Sep 17 00:00:00 2001 From: meganrogge Date: Wed, 28 Feb 2024 10:10:57 -0800 Subject: [PATCH 07/18] check if event is undefined --- src/vs/workbench/contrib/tasks/common/problemCollectors.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/tasks/common/problemCollectors.ts b/src/vs/workbench/contrib/tasks/common/problemCollectors.ts index d07edd2679c..9f47ca8564e 100644 --- a/src/vs/workbench/contrib/tasks/common/problemCollectors.ts +++ b/src/vs/workbench/contrib/tasks/common/problemCollectors.ts @@ -441,7 +441,7 @@ export class WatchingProblemCollector extends AbstractProblemCollector implement }, 500, false, true)(async (markerEvent) => { markerChanged?.dispose(); markerChanged = undefined; - if (!markerEvent.includes(modelEvent.uri) || (this.markerService.read({ resource: modelEvent.uri }).length !== 0)) { + if (!markerEvent || !markerEvent.includes(modelEvent.uri) || (this.markerService.read({ resource: modelEvent.uri }).length !== 0)) { return; } const oldLines = Array.from(this.lines); From eb4e516a8a6fed349066479cb0abffbf9259c663 Mon Sep 17 00:00:00 2001 From: Michael Lively Date: Wed, 28 Feb 2024 11:28:09 -0800 Subject: [PATCH 08/18] Folded cells run-in-section (#205315) * initial functionality, needs css tweaks * update button css + add executing spinner * mutable disposable to avoid leaking listeners --- .../browser/media/notebookFolding.css | 18 +++++++ .../browser/view/cellParts/foldedCellHint.ts | 52 ++++++++++++++++++- .../browser/view/renderers/cellRenderer.ts | 7 ++- 3 files changed, 75 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/media/notebookFolding.css b/src/vs/workbench/contrib/notebook/browser/media/notebookFolding.css index 7433c9a7eb2..88bfc96b8f6 100644 --- a/src/vs/workbench/contrib/notebook/browser/media/notebookFolding.css +++ b/src/vs/workbench/contrib/notebook/browser/media/notebookFolding.css @@ -46,6 +46,8 @@ .monaco-workbench .notebookOverlay > .cell-list-container .notebook-folded-hint { position: absolute; user-select: none; + display: flex; + align-items: center; } .monaco-workbench .notebookOverlay > .cell-list-container .notebook-folded-hint-label { @@ -55,6 +57,22 @@ opacity: 0.7; } +.monaco-workbench .notebookOverlay > .cell-list-container .folded-cell-run-section-button { + position: relative; + left: 0px; + padding: 2px; + border-radius: 5px; + margin-right: 4px; + height: 16px; + width: 16px; + z-index: var(--z-index-notebook-cell-expand-part-button); +} + +.monaco-workbench .notebookOverlay > .cell-list-container .folded-cell-run-section-button:hover { + background-color: var(--vscode-editorStickyScrollHover-background); + cursor: pointer; +} + .monaco-workbench .notebookOverlay .cell-editor-container .monaco-editor .margin-view-overlays .codicon-folding-expanded, .monaco-workbench .notebookOverlay .cell-editor-container .monaco-editor .margin-view-overlays .codicon-folding-collapsed { margin-left: 0; diff --git a/src/vs/workbench/contrib/notebook/browser/view/cellParts/foldedCellHint.ts b/src/vs/workbench/contrib/notebook/browser/view/cellParts/foldedCellHint.ts index 2fe72e05af8..211e85e9a62 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/cellParts/foldedCellHint.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/cellParts/foldedCellHint.ts @@ -11,12 +11,21 @@ import { FoldingController } from 'vs/workbench/contrib/notebook/browser/control import { CellEditState, CellFoldingState, INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { CellContentPart } from 'vs/workbench/contrib/notebook/browser/view/cellPart'; import { MarkupCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/markupCellViewModel'; +import { ICellRange } from 'vs/workbench/contrib/notebook/common/notebookRange'; +import { executingStateIcon } from 'vs/workbench/contrib/notebook/browser/notebookIcons'; +import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; +import { NotebookCellExecutionState } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { MutableDisposable } from 'vs/base/common/lifecycle'; export class FoldedCellHint extends CellContentPart { + private readonly _runButtonListener = this._register(new MutableDisposable()); + private readonly _cellExecutionListener = this._register(new MutableDisposable()); + constructor( private readonly _notebookEditor: INotebookEditor, private readonly _container: HTMLElement, + @INotebookExecutionStateService private readonly _notebookExecutionStateService: INotebookExecutionStateService ) { super(); } @@ -27,20 +36,27 @@ export class FoldedCellHint extends CellContentPart { private update(element: MarkupCellViewModel) { if (!this._notebookEditor.hasModel()) { + this._cellExecutionListener.clear(); + this._runButtonListener.clear(); return; } if (element.isInputCollapsed || element.getEditState() === CellEditState.Editing) { + this._cellExecutionListener.clear(); + this._runButtonListener.clear(); DOM.hide(this._container); } else if (element.foldingState === CellFoldingState.Collapsed) { const idx = this._notebookEditor.getViewModel().getCellIndex(element); const length = this._notebookEditor.getViewModel().getFoldedLength(idx); - DOM.reset(this._container, this.getHiddenCellsLabel(length), this.getHiddenCellHintButton(element)); + + DOM.reset(this._container, this.getRunFoldedSectionButton({ start: idx, end: idx + length }), this.getHiddenCellsLabel(length), this.getHiddenCellHintButton(element)); DOM.show(this._container); const foldHintTop = element.layoutInfo.previewHeight; this._container.style.top = `${foldHintTop}px`; } else { + this._cellExecutionListener.clear(); + this._runButtonListener.clear(); DOM.hide(this._container); } } @@ -67,6 +83,40 @@ export class FoldedCellHint extends CellContentPart { return expandIcon; } + private getRunFoldedSectionButton(range: ICellRange): HTMLElement { + const runAllContainer = DOM.$('span.folded-cell-run-section-button'); + const cells = this._notebookEditor.getCellsInRange(range); + + const isRunning = cells.some(cell => { + const cellExecution = this._notebookExecutionStateService.getCellExecution(cell.uri); + return cellExecution && cellExecution.state === NotebookCellExecutionState.Executing; + }); + + const runAllIcon = isRunning ? + ThemeIcon.modify(executingStateIcon, 'spin') : + Codicon.play; + runAllContainer.classList.add(...ThemeIcon.asClassNameArray(runAllIcon)); + + this._runButtonListener.value = DOM.addDisposableListener(runAllContainer, DOM.EventType.CLICK, () => { + this._notebookEditor.executeNotebookCells(cells); + }); + + this._cellExecutionListener.value = this._notebookExecutionStateService.onDidChangeExecution(() => { + const isRunning = cells.some(cell => { + const cellExecution = this._notebookExecutionStateService.getCellExecution(cell.uri); + return cellExecution && cellExecution.state === NotebookCellExecutionState.Executing; + }); + + const runAllIcon = isRunning ? + ThemeIcon.modify(executingStateIcon, 'spin') : + Codicon.play; + runAllContainer.className = ''; + runAllContainer.classList.add('folded-cell-run-section-button', ...ThemeIcon.asClassNameArray(runAllIcon)); + }); + + return runAllContainer; + } + override updateInternalLayoutNow(element: MarkupCellViewModel) { this.update(element); } diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/cellRenderer.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/cellRenderer.ts index 25a1310af43..9f6f5b8dac5 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/cellRenderer.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/cellRenderer.ts @@ -49,6 +49,7 @@ import { CodeCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewMod import { MarkupCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/markupCellViewModel'; import { CellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModelImpl'; import { CellKind } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; const $ = DOM.$; @@ -109,6 +110,8 @@ abstract class AbstractCellRenderer { export class MarkupCellRenderer extends AbstractCellRenderer implements IListRenderer { static readonly TEMPLATE_ID = 'markdown_cell'; + private _notebookExecutionStateService: INotebookExecutionStateService; + constructor( notebookEditor: INotebookEditorDelegate, dndController: CellDragAndDropController, @@ -120,8 +123,10 @@ export class MarkupCellRenderer extends AbstractCellRenderer implements IListRen @IMenuService menuService: IMenuService, @IKeybindingService keybindingService: IKeybindingService, @INotificationService notificationService: INotificationService, + @INotebookExecutionStateService notebookExecutionStateService: INotebookExecutionStateService ) { super(instantiationService, notebookEditor, contextMenuService, menuService, configurationService, keybindingService, notificationService, contextKeyServiceProvider, 'markdown', dndController); + this._notebookExecutionStateService = notebookExecutionStateService; } get templateId() { @@ -169,7 +174,7 @@ export class MarkupCellRenderer extends AbstractCellRenderer implements IListRen templateDisposables.add(scopedInstaService.createInstance(CellChatPart, this.notebookEditor, cellChatPart)), templateDisposables.add(scopedInstaService.createInstance(CellEditorStatusBar, this.notebookEditor, container, editorPart, undefined)), templateDisposables.add(new CellFocusIndicator(this.notebookEditor, titleToolbar, focusIndicatorTop, focusIndicatorLeft, focusIndicatorRight, focusIndicatorBottom)), - templateDisposables.add(new FoldedCellHint(this.notebookEditor, DOM.append(container, $('.notebook-folded-hint')))), + templateDisposables.add(new FoldedCellHint(this.notebookEditor, DOM.append(container, $('.notebook-folded-hint')), this._notebookExecutionStateService)), templateDisposables.add(new CellDecorations(rootContainer, decorationContainer)), templateDisposables.add(scopedInstaService.createInstance(CellComments, this.notebookEditor, cellCommentPartContainer)), templateDisposables.add(new CollapsedCellInput(this.notebookEditor, cellInputCollapsedContainer)), From f99981ed56f3adca0d446e9e2ec69fdff563d2c0 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Wed, 28 Feb 2024 11:47:31 -0800 Subject: [PATCH 09/18] cli: fix compressor not draining and leading to truncated responses (#206464) * cli: fix compressor not draining and leading to truncated responses Fixes https://github.com/microsoft/vscode-remote-release/issues/9594 * fix lint --- cli/src/tunnels/socket_signal.rs | 115 +++++++++++++++++++++---------- 1 file changed, 79 insertions(+), 36 deletions(-) diff --git a/cli/src/tunnels/socket_signal.rs b/cli/src/tunnels/socket_signal.rs index 9036c6ae3f9..2227f323852 100644 --- a/cli/src/tunnels/socket_signal.rs +++ b/cli/src/tunnels/socket_signal.rs @@ -94,41 +94,42 @@ impl ServerMessageSink { async fn server_message_or_closed( &mut self, - body: Option<&[u8]>, + body_or_end: Option<&[u8]>, ) -> Result<(), mpsc::error::SendError> { let i = self.id; let mut tx = self.tx.take().unwrap(); - let msg = body - .map(|b| self.get_server_msg_content(b)) - .map(|body| RefServerMessageParams { i, body }); - let r = match &mut tx { - ServerMessageDestination::Channel(tx) => { - tx.send(SocketSignal::from_message(&ToClientRequest { - id: None, - params: match msg { - Some(msg) => ClientRequestMethod::servermsg(msg), - None => ClientRequestMethod::serverclose(ServerClosedParams { i }), - }, - })) - .await - } - ServerMessageDestination::Rpc(caller) => { - match msg { - Some(msg) => caller.notify("servermsg", msg), - None => caller.notify("serverclose", ServerClosedParams { i }), - }; - Ok(()) - } - }; + if let Some(b) = body_or_end { + let body = self.get_server_msg_content(b, false); + let r = + send_data_or_close_if_none(i, &mut tx, Some(RefServerMessageParams { i, body })) + .await; + self.tx = Some(tx); + return r; + } + let tail = self.get_server_msg_content(&[], true); + if !tail.is_empty() { + let _ = send_data_or_close_if_none( + i, + &mut tx, + Some(RefServerMessageParams { i, body: tail }), + ) + .await; + } + + let r = send_data_or_close_if_none(i, &mut tx, None).await; self.tx = Some(tx); r } - pub(crate) fn get_server_msg_content<'a: 'b, 'b>(&'a mut self, body: &'b [u8]) -> &'b [u8] { + pub(crate) fn get_server_msg_content<'a: 'b, 'b>( + &'a mut self, + body: &'b [u8], + finish: bool, + ) -> &'b [u8] { if let Some(flate) = &mut self.flate { - if let Ok(compressed) = flate.process(body) { + if let Ok(compressed) = flate.process(body, finish) { return compressed; } } @@ -137,6 +138,32 @@ impl ServerMessageSink { } } +async fn send_data_or_close_if_none( + i: u16, + tx: &mut ServerMessageDestination, + msg: Option>, +) -> Result<(), mpsc::error::SendError> { + match tx { + ServerMessageDestination::Channel(tx) => { + tx.send(SocketSignal::from_message(&ToClientRequest { + id: None, + params: match msg { + Some(msg) => ClientRequestMethod::servermsg(msg), + None => ClientRequestMethod::serverclose(ServerClosedParams { i }), + }, + })) + .await + } + ServerMessageDestination::Rpc(caller) => { + match msg { + Some(msg) => caller.notify("servermsg", msg), + None => caller.notify("serverclose", ServerClosedParams { i }), + }; + Ok(()) + } + } +} + impl Drop for ServerMessageSink { fn drop(&mut self) { self.multiplexer.remove(self.id); @@ -162,7 +189,8 @@ impl ClientMessageDecoder { pub fn decode<'a: 'b, 'b>(&'a mut self, message: &'b [u8]) -> std::io::Result<&'b [u8]> { match &mut self.dec { - Some(d) => d.process(message), + // todo@connor4312 do we ever need to actually 'finish' the client message stream? + Some(d) => d.process(message, false), None => Ok(message), } } @@ -175,6 +203,7 @@ trait FlateAlgorithm { &mut self, contents: &[u8], output: &mut [u8], + finish: bool, ) -> Result; } @@ -193,9 +222,15 @@ impl FlateAlgorithm for DecompressFlateAlgorithm { &mut self, contents: &[u8], output: &mut [u8], + finish: bool, ) -> Result { + let mode = match finish { + true => flate2::FlushDecompress::Finish, + false => flate2::FlushDecompress::None, + }; + self.0 - .decompress(contents, output, flate2::FlushDecompress::None) + .decompress(contents, output, mode) .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidInput, e)) } } @@ -215,9 +250,15 @@ impl FlateAlgorithm for CompressFlateAlgorithm { &mut self, contents: &[u8], output: &mut [u8], + finish: bool, ) -> Result { + let mode = match finish { + true => flate2::FlushCompress::Finish, + false => flate2::FlushCompress::Sync, + }; + self.0 - .compress(contents, output, flate2::FlushCompress::Sync) + .compress(contents, output, mode) .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidInput, e)) } } @@ -241,23 +282,25 @@ where } } - pub fn process(&mut self, contents: &[u8]) -> std::io::Result<&[u8]> { + pub fn process(&mut self, contents: &[u8], finish: bool) -> std::io::Result<&[u8]> { let mut out_offset = 0; let mut in_offset = 0; loop { let in_before = self.flate.total_in(); let out_before = self.flate.total_out(); - match self - .flate - .process(&contents[in_offset..], &mut self.output[out_offset..]) - { + match self.flate.process( + &contents[in_offset..], + &mut self.output[out_offset..], + finish, + ) { Ok(flate2::Status::Ok | flate2::Status::BufError) => { let processed_len = in_offset + (self.flate.total_in() - in_before) as usize; let output_len = out_offset + (self.flate.total_out() - out_before) as usize; - if processed_len < contents.len() { + if processed_len < contents.len() || output_len == self.output.len() { // If we filled the output buffer but there's more data to compress, - // extend the output buffer and keep compressing. + // or the output got filled after processing all input, extend + // the output buffer and keep compressing. out_offset = output_len; in_offset = processed_len; if output_len == self.output.len() { @@ -298,7 +341,7 @@ mod tests { // 3000 and 30000 test resizing the buffer for msg_len in [3, 30, 300, 3000, 30000] { let vals = (0..msg_len).map(|v| v as u8).collect::>(); - let compressed = sink.get_server_msg_content(&vals); + let compressed = sink.get_server_msg_content(&vals, false); assert_ne!(compressed, vals); let decompressed = decompress.decode(compressed).unwrap(); assert_eq!(decompressed.len(), vals.len()); From 275d365b86c9fecb5c813bf691b3b94cd488cae2 Mon Sep 17 00:00:00 2001 From: Henning Dieterichs Date: Wed, 28 Feb 2024 21:57:49 +0100 Subject: [PATCH 10/18] debouncedObservable2 (#206470) --- src/vs/base/common/observableInternal/base.ts | 4 +- .../base/common/observableInternal/utils.ts | 45 +++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/vs/base/common/observableInternal/base.ts b/src/vs/base/common/observableInternal/base.ts index 74b03df1438..4e738e63344 100644 --- a/src/vs/base/common/observableInternal/base.ts +++ b/src/vs/base/common/observableInternal/base.ts @@ -389,12 +389,12 @@ export class ObservableValue constructor( private readonly _owner: Owner, private readonly _debugName: string | undefined, - initialValue: T + initialValue: T, ) { super(); this._value = initialValue; } - public get(): T { + public override get(): T { return this._value; } diff --git a/src/vs/base/common/observableInternal/utils.ts b/src/vs/base/common/observableInternal/utils.ts index 5831de89add..0ac31f7f881 100644 --- a/src/vs/base/common/observableInternal/utils.ts +++ b/src/vs/base/common/observableInternal/utils.ts @@ -253,6 +253,9 @@ class ObservableSignal extends BaseObservable implements } } +/** + * @deprecated Use `debouncedObservable2` instead. + */ export function debouncedObservable(observable: IObservable, debounceMs: number, disposableStore: DisposableStore): IObservable { const debouncedObservable = observableValue('debounced', undefined); @@ -276,6 +279,48 @@ export function debouncedObservable(observable: IObservable, debounceMs: n return debouncedObservable; } +/** + * Creates an observable that debounces the input observable. + */ +export function debouncedObservable2(observable: IObservable, debounceMs: number): IObservable { + let hasValue = false; + let lastValue: T | undefined; + + let timeout: any = undefined; + + return observableFromEvent(cb => { + const d = autorun(reader => { + const value = observable.read(reader); + + if (!hasValue) { + hasValue = true; + lastValue = value; + } else { + if (timeout) { + clearTimeout(timeout); + } + timeout = setTimeout(() => { + lastValue = value; + cb(); + }, debounceMs); + } + }); + return { + dispose() { + d.dispose(); + hasValue = false; + lastValue = undefined; + }, + }; + }, () => { + if (hasValue) { + return lastValue!; + } else { + return observable.get(); + } + }); +} + export function wasEventTriggeredRecently(event: Event, timeoutMs: number, disposableStore: DisposableStore): IObservable { const observable = observableValue('triggeredRecently', false); From 435f3ce9d897e40278f6ec287cb7bd68acd37018 Mon Sep 17 00:00:00 2001 From: Henning Dieterichs Date: Wed, 28 Feb 2024 21:58:18 +0100 Subject: [PATCH 11/18] Fixes #206166 (#206472) --- .../browser/widget/multiDiffEditor/diffEditorItemTemplate.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/editor/browser/widget/multiDiffEditor/diffEditorItemTemplate.ts b/src/vs/editor/browser/widget/multiDiffEditor/diffEditorItemTemplate.ts index 84cffd7e94e..83a4a22aee7 100644 --- a/src/vs/editor/browser/widget/multiDiffEditor/diffEditorItemTemplate.ts +++ b/src/vs/editor/browser/widget/multiDiffEditor/diffEditorItemTemplate.ts @@ -285,6 +285,6 @@ function isFocused(editor: ICodeEditor): IObservable { store.add(editor.onDidBlurEditorWidget(() => h(false))); return store; }, - () => editor.hasWidgetFocus() + () => editor.hasTextFocus() ); } From 4f08e9ca03574a6731abfb78189e229e4ca7fd54 Mon Sep 17 00:00:00 2001 From: Henning Dieterichs Date: Wed, 28 Feb 2024 22:01:06 +0100 Subject: [PATCH 12/18] Fixes #206355 (#206473) --- .../browser/widget/multiDiffEditor/diffEditorItemTemplate.ts | 2 +- src/vs/editor/browser/widget/multiDiffEditor/style.css | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vs/editor/browser/widget/multiDiffEditor/diffEditorItemTemplate.ts b/src/vs/editor/browser/widget/multiDiffEditor/diffEditorItemTemplate.ts index 83a4a22aee7..c6f3ca9de9e 100644 --- a/src/vs/editor/browser/widget/multiDiffEditor/diffEditorItemTemplate.ts +++ b/src/vs/editor/browser/widget/multiDiffEditor/diffEditorItemTemplate.ts @@ -236,7 +236,7 @@ export class DiffEditorItemTemplate extends Disposable implements IPooledObject< }); } - private readonly _headerHeight = /*this._elements.header.clientHeight*/ 48; + private readonly _headerHeight = /*this._elements.header.clientHeight*/ 40; private _lastScrollTop = -1; private _isSettingScrollTop = false; diff --git a/src/vs/editor/browser/widget/multiDiffEditor/style.css b/src/vs/editor/browser/widget/multiDiffEditor/style.css index 44f0703d580..c540a46b3f1 100644 --- a/src/vs/editor/browser/widget/multiDiffEditor/style.css +++ b/src/vs/editor/browser/widget/multiDiffEditor/style.css @@ -37,7 +37,7 @@ .header-content { margin: 8px 8px 0px 8px; - padding: 8px 5px; + padding: 4px 5px; border-top: 1px solid var(--vscode-multiDiffEditor-border); border-right: 1px solid var(--vscode-multiDiffEditor-border); From 0685fc347032f4c5f244ec3601e9a8f9ec95f96c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Cie=C5=9Blak?= Date: Wed, 28 Feb 2024 22:01:41 +0100 Subject: [PATCH 13/18] Inline edit - make sure we cancel in-progress request on blur (#206430) --- .../editor/contrib/inlineEdit/browser/inlineEditController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/editor/contrib/inlineEdit/browser/inlineEditController.ts b/src/vs/editor/contrib/inlineEdit/browser/inlineEditController.ts index 8355ef312cf..646c6f20c22 100644 --- a/src/vs/editor/contrib/inlineEdit/browser/inlineEditController.ts +++ b/src/vs/editor/contrib/inlineEdit/browser/inlineEditController.ts @@ -121,7 +121,7 @@ export class InlineEditController extends Disposable { if (this._configurationService.getValue('editor.experimentalInlineEdit.keepOnBlur') || editor.getOption(EditorOption.inlineEdit).keepOnBlur) { return; } - this._currentRequestCts?.dispose(); + this._currentRequestCts?.dispose(true); this._currentRequestCts = undefined; this.clear(false); })); From 3b2b4cb5eb27b0c1656e67da26f9460d1544b4dc Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 28 Feb 2024 13:51:12 -0800 Subject: [PATCH 14/18] Forward wheel events to main xterm instance Fixes #198541 --- .../stickyScroll/browser/terminalStickyScrollOverlay.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/vs/workbench/contrib/terminalContrib/stickyScroll/browser/terminalStickyScrollOverlay.ts b/src/vs/workbench/contrib/terminalContrib/stickyScroll/browser/terminalStickyScrollOverlay.ts index 4dc0908e54e..77856d9eda1 100644 --- a/src/vs/workbench/contrib/terminalContrib/stickyScroll/browser/terminalStickyScrollOverlay.ts +++ b/src/vs/workbench/contrib/terminalContrib/stickyScroll/browser/terminalStickyScrollOverlay.ts @@ -376,6 +376,9 @@ export class TerminalStickyScrollOverlay extends Disposable { } })); + // Forward mouse events to the terminal + this._register(addStandardDisposableListener(hoverOverlay, 'wheel', e => this._xterm?.raw.element?.dispatchEvent(new WheelEvent(e.type, e)))); + // Context menu - stop propagation on mousedown because rightClickBehavior listens on // mousedown, not contextmenu this._register(addDisposableListener(hoverOverlay, 'mousedown', e => { From eea2590f508761a247ab80e082843b7bc642bd85 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 28 Feb 2024 14:00:54 -0800 Subject: [PATCH 15/18] Hide sticky scroll when the prompt is trimmed Fixes #199554 --- .../stickyScroll/browser/terminalStickyScrollOverlay.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/vs/workbench/contrib/terminalContrib/stickyScroll/browser/terminalStickyScrollOverlay.ts b/src/vs/workbench/contrib/terminalContrib/stickyScroll/browser/terminalStickyScrollOverlay.ts index 4dc0908e54e..51e88210aad 100644 --- a/src/vs/workbench/contrib/terminalContrib/stickyScroll/browser/terminalStickyScrollOverlay.ts +++ b/src/vs/workbench/contrib/terminalContrib/stickyScroll/browser/terminalStickyScrollOverlay.ts @@ -245,6 +245,12 @@ export class TerminalStickyScrollOverlay extends Disposable { return; } + // Hide sticky scroll if the prompt has been trimmed from the buffer + if (command.promptStartMarker?.line === -1) { + this._setVisible(false); + return; + } + // Determine sticky scroll line count const buffer = xterm.buffer.active; const promptRowCount = command.getPromptRowCount(); From 5332bdfead0831d51703cffcb7595e7f6052f7e2 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 28 Feb 2024 14:43:43 -0800 Subject: [PATCH 16/18] Adjust sticky scroll height based on endMarker Fixes #199305 --- .../browser/terminalStickyScrollOverlay.ts | 33 +++++++++++++------ 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/src/vs/workbench/contrib/terminalContrib/stickyScroll/browser/terminalStickyScrollOverlay.ts b/src/vs/workbench/contrib/terminalContrib/stickyScroll/browser/terminalStickyScrollOverlay.ts index 0a08eb195f0..969a81c5ad6 100644 --- a/src/vs/workbench/contrib/terminalContrib/stickyScroll/browser/terminalStickyScrollOverlay.ts +++ b/src/vs/workbench/contrib/terminalContrib/stickyScroll/browser/terminalStickyScrollOverlay.ts @@ -8,7 +8,7 @@ import type { IBufferLine, IMarker, ITerminalOptions, ITheme, Terminal as RawXte import { importAMDNodeModule } from 'vs/amdX'; import { $, addDisposableListener, addStandardDisposableListener, getWindow } from 'vs/base/browser/dom'; import { CancelablePromise, createCancelablePromise } from 'vs/base/common/async'; -import { debounce, memoize, throttle } from 'vs/base/common/decorators'; +import { memoize, throttle } from 'vs/base/common/decorators'; import { Event } from 'vs/base/common/event'; import { Disposable, MutableDisposable, combinedDisposable, toDisposable } from 'vs/base/common/lifecycle'; import { removeAnsiEscapeCodes } from 'vs/base/common/strings'; @@ -193,17 +193,18 @@ export class TerminalStickyScrollOverlay extends Disposable { if (command && this._currentStickyCommand !== command) { this._throttledRefresh(); } else { - this._debouncedRefresh(); + // If it's the same command, do not throttle as the sticky scroll overlay height may + // need to be adjusted. This would cause a flicker if throttled. + this._refreshNow(); } } - @debounce(20) - private _debouncedRefresh(): void { - this._throttledRefresh(); - } - @throttle(0) private _throttledRefresh(): void { + this._refreshNow(); + } + + private _refreshNow(): void { const command = this._commandDetection.getCommandForLine(this._xterm.raw.buffer.active.viewportY); // The command from viewportY + 1 is used because this one will not be obscured by sticky @@ -255,7 +256,7 @@ export class TerminalStickyScrollOverlay extends Disposable { const buffer = xterm.buffer.active; const promptRowCount = command.getPromptRowCount(); const commandRowCount = command.getCommandRowCount(); - const stickyScrollLineStart = startMarker.line - (promptRowCount - 1); + let stickyScrollLineStart = startMarker.line - (promptRowCount - 1); // Calculate the row offset, this is the number of rows that will be clipped from the top // of the sticky overlay because we do not want to show any content above the bounds of the @@ -264,7 +265,18 @@ export class TerminalStickyScrollOverlay extends Disposable { const isPartialCommand = !('getOutput' in command); const rowOffset = !isPartialCommand && command.endMarker ? Math.max(buffer.viewportY - command.endMarker.line + 1, 0) : 0; const maxLineCount = Math.min(this._rawMaxLineCount, Math.floor(xterm.rows * Constants.StickyScrollPercentageCap)); - const stickyScrollLineCount = Math.min(promptRowCount + commandRowCount - 1, maxLineCount) - rowOffset; + let stickyScrollLineCount = Math.min(promptRowCount + commandRowCount - 1, maxLineCount) - rowOffset; + + // TODO: This needs to not be throttled + // Adjust sticky scroll content if it would below the end of the command, obscuring the + // following command. + if (!isPartialCommand && command.endMarker && command.endMarker.line !== -1) { + if (buffer.viewportY + stickyScrollLineCount > command.endMarker.line) { + const diff = buffer.viewportY + stickyScrollLineCount - command.endMarker.line; + stickyScrollLineStart += diff; + stickyScrollLineCount -= diff; + } + } // Hide sticky scroll if it's currently on a line that contains it if (buffer.viewportY <= stickyScrollLineStart) { @@ -287,7 +299,7 @@ export class TerminalStickyScrollOverlay extends Disposable { } } - // Clear attrs, reset cursor position, clear right + // Get the line content of the command from the terminal const content = this._serializeAddon.serialize({ range: { start: stickyScrollLineStart + rowOffset, @@ -305,6 +317,7 @@ export class TerminalStickyScrollOverlay extends Disposable { // Write content if it differs if (content && this._currentContent !== content) { this._stickyScrollOverlay.resize(this._stickyScrollOverlay.cols, stickyScrollLineCount); + // Clear attrs, reset cursor position, clear right this._stickyScrollOverlay.write('\x1b[0m\x1b[H\x1b[2J'); this._stickyScrollOverlay.write(content); this._currentContent = content; From 50d6e94b3f5d55c42fdc0e2302905a7541fbc2e7 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 28 Feb 2024 14:46:59 -0800 Subject: [PATCH 17/18] Move sticky scroll endMarker clipping to element position This means we no longer need to resize the canvas which reduces flicker. Fixes #199305 --- .../browser/terminalStickyScrollOverlay.ts | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/vs/workbench/contrib/terminalContrib/stickyScroll/browser/terminalStickyScrollOverlay.ts b/src/vs/workbench/contrib/terminalContrib/stickyScroll/browser/terminalStickyScrollOverlay.ts index 969a81c5ad6..04f14751372 100644 --- a/src/vs/workbench/contrib/terminalContrib/stickyScroll/browser/terminalStickyScrollOverlay.ts +++ b/src/vs/workbench/contrib/terminalContrib/stickyScroll/browser/terminalStickyScrollOverlay.ts @@ -256,7 +256,7 @@ export class TerminalStickyScrollOverlay extends Disposable { const buffer = xterm.buffer.active; const promptRowCount = command.getPromptRowCount(); const commandRowCount = command.getCommandRowCount(); - let stickyScrollLineStart = startMarker.line - (promptRowCount - 1); + const stickyScrollLineStart = startMarker.line - (promptRowCount - 1); // Calculate the row offset, this is the number of rows that will be clipped from the top // of the sticky overlay because we do not want to show any content above the bounds of the @@ -265,18 +265,7 @@ export class TerminalStickyScrollOverlay extends Disposable { const isPartialCommand = !('getOutput' in command); const rowOffset = !isPartialCommand && command.endMarker ? Math.max(buffer.viewportY - command.endMarker.line + 1, 0) : 0; const maxLineCount = Math.min(this._rawMaxLineCount, Math.floor(xterm.rows * Constants.StickyScrollPercentageCap)); - let stickyScrollLineCount = Math.min(promptRowCount + commandRowCount - 1, maxLineCount) - rowOffset; - - // TODO: This needs to not be throttled - // Adjust sticky scroll content if it would below the end of the command, obscuring the - // following command. - if (!isPartialCommand && command.endMarker && command.endMarker.line !== -1) { - if (buffer.viewportY + stickyScrollLineCount > command.endMarker.line) { - const diff = buffer.viewportY + stickyScrollLineCount - command.endMarker.line; - stickyScrollLineStart += diff; - stickyScrollLineCount -= diff; - } - } + const stickyScrollLineCount = Math.min(promptRowCount + commandRowCount - 1, maxLineCount) - rowOffset; // Hide sticky scroll if it's currently on a line that contains it if (buffer.viewportY <= stickyScrollLineStart) { @@ -336,7 +325,18 @@ export class TerminalStickyScrollOverlay extends Disposable { const termBox = xterm.element.getBoundingClientRect(); const rowHeight = termBox.height / xterm.rows; const overlayHeight = stickyScrollLineCount * rowHeight; - this._element.style.bottom = `${termBox.height - overlayHeight + 1}px`; + + // Adjust sticky scroll content if it would below the end of the command, obscuring the + // following command. + let endMarkerOffset = 0; + if (!isPartialCommand && command.endMarker && command.endMarker.line !== -1) { + if (buffer.viewportY + stickyScrollLineCount > command.endMarker.line) { + const diff = buffer.viewportY + stickyScrollLineCount - command.endMarker.line; + endMarkerOffset = diff * rowHeight; + } + } + + this._element.style.bottom = `${termBox.height - overlayHeight + 1 + endMarkerOffset}px`; } } else { this._setVisible(false); From 1ee4bfbda26215fcbf0e89f0a132bda2ffd0ca47 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Wed, 28 Feb 2024 14:55:52 -0800 Subject: [PATCH 18/18] More aggressively clear viewport commands on Windows Fixes #198278 --- .../common/capabilities/commandDetectionCapability.ts | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/vs/platform/terminal/common/capabilities/commandDetectionCapability.ts b/src/vs/platform/terminal/common/capabilities/commandDetectionCapability.ts index df3cb4ce9ee..685bae340bb 100644 --- a/src/vs/platform/terminal/common/capabilities/commandDetectionCapability.ts +++ b/src/vs/platform/terminal/common/capabilities/commandDetectionCapability.ts @@ -515,8 +515,6 @@ class WindowsPtyHeuristics extends Disposable { private _onCursorMoveListener = this._register(new MutableDisposable()); - private _recentlyPerformedCsiJ = false; - private _tryAdjustCommandStartMarkerScheduler?: RunOnceScheduler; private _tryAdjustCommandStartMarkerScannedLineCount: number = 0; private _tryAdjustCommandStartMarkerPollCount: number = 0; @@ -530,8 +528,8 @@ class WindowsPtyHeuristics extends Disposable { super(); this._register(_terminal.parser.registerCsiHandler({ final: 'J' }, params => { + // Clear commands when the viewport is cleared if (params.length >= 1 && (params[0] === 2 || params[0] === 3)) { - this._recentlyPerformedCsiJ = true; this._hooks.clearCommandsInViewport(); } // We don't want to override xterm.js' default behavior, just augment it @@ -539,11 +537,6 @@ class WindowsPtyHeuristics extends Disposable { })); this._register(this._capability.onBeforeCommandFinished(command => { - if (this._recentlyPerformedCsiJ) { - this._recentlyPerformedCsiJ = false; - return; - } - // For older Windows backends we cannot listen to CSI J, instead we assume running clear // or cls will clear all commands in the viewport. This is not perfect but it's right // most of the time.