From eecfc4932d67cbf3f32353b8a3802c8562ca9ce5 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Sun, 15 Feb 2026 07:31:28 +0000 Subject: [PATCH] Avoid listener leak warnings (#295402) * Fix leak in ChatProgressContentPart * Optimize inline anchor widget * And thsi --- .../attachments/chatAttachmentWidgets.ts | 62 +++++++++++++------ .../chatInlineAnchorWidget.ts | 31 ++++------ .../chatMarkdownDecorationsRenderer.ts | 38 ++++++++++++ .../chatProgressContentPart.ts | 8 ++- .../chat/browser/widget/media/chat.css | 6 +- 5 files changed, 104 insertions(+), 41 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/attachments/chatAttachmentWidgets.ts b/src/vs/workbench/contrib/chat/browser/attachments/chatAttachmentWidgets.ts index e0df9ffe04f..7744f6a9e9f 100644 --- a/src/vs/workbench/contrib/chat/browser/attachments/chatAttachmentWidgets.ts +++ b/src/vs/workbench/contrib/chat/browser/attachments/chatAttachmentWidgets.ts @@ -614,8 +614,7 @@ export class DefaultChatAttachmentWidget extends AbstractChatAttachmentWidget { } if (attachment.kind === 'symbol') { - const scopedContextKeyService = this._register(this.contextKeyService.createScoped(this.element)); - this._register(this.instantiationService.invokeFunction(hookUpSymbolAttachmentDragAndContextMenu, this.element, scopedContextKeyService, { ...attachment, kind: attachment.symbolKind }, MenuId.ChatInputSymbolAttachmentContext)); + this._register(this.instantiationService.invokeFunction(hookUpSymbolAttachmentDragAndContextMenu, this.element, this.contextKeyService, { ...attachment, kind: attachment.symbolKind }, MenuId.ChatInputSymbolAttachmentContext)); } // Handle click for string context attachments with context commands @@ -1115,19 +1114,15 @@ export function hookUpResourceAttachmentDragAndContextMenu(accessor: ServicesAcc return store; } -export function hookUpSymbolAttachmentDragAndContextMenu(accessor: ServicesAccessor, widget: HTMLElement, scopedContextKeyService: IScopedContextKeyService, attachment: { name: string; value: Location; kind: SymbolKind }, contextMenuId: MenuId): IDisposable { +export function hookUpSymbolAttachmentDragAndContextMenu(accessor: ServicesAccessor, widget: HTMLElement, parentContextKeyService: IContextKeyService, attachment: { name: string; value: Location; kind: SymbolKind }, contextMenuId: MenuId): IDisposable { const instantiationService = accessor.get(IInstantiationService); const languageFeaturesService = accessor.get(ILanguageFeaturesService); const textModelService = accessor.get(ITextModelService); + const contextMenuService = accessor.get(IContextMenuService); + const menuService = accessor.get(IMenuService); const store = new DisposableStore(); - // Context - store.add(setResourceContext(accessor, scopedContextKeyService, attachment.value.uri)); - - const chatResourceContext = chatAttachmentResourceContextKey.bindTo(scopedContextKeyService); - chatResourceContext.set(attachment.value.uri.toString()); - // Drag and drop widget.draggable = true; store.add(dom.addDisposableListener(widget, 'dragstart', e => { @@ -1143,26 +1138,57 @@ export function hookUpSymbolAttachmentDragAndContextMenu(accessor: ServicesAcces e.dataTransfer?.setDragImage(widget, 0, 0); })); - // Context menu - const providerContexts: ReadonlyArray<[IContextKey, LanguageFeatureRegistry]> = [ - [EditorContextKeys.hasDefinitionProvider.bindTo(scopedContextKeyService), languageFeaturesService.definitionProvider], - [EditorContextKeys.hasReferenceProvider.bindTo(scopedContextKeyService), languageFeaturesService.referenceProvider], - [EditorContextKeys.hasImplementationProvider.bindTo(scopedContextKeyService), languageFeaturesService.implementationProvider], - [EditorContextKeys.hasTypeDefinitionProvider.bindTo(scopedContextKeyService), languageFeaturesService.typeDefinitionProvider], - ]; + // Context menu (context key service created eagerly for keybinding preconditions, + // but resource context and provider contexts are initialized lazily on first use) + const scopedContextKeyService = store.add(parentContextKeyService.createScoped(widget)); + chatAttachmentResourceContextKey.bindTo(scopedContextKeyService).set(attachment.value.uri.toString()); + store.add(setResourceContext(accessor, scopedContextKeyService, attachment.value.uri)); + + let providerContexts: ReadonlyArray<[IContextKey, LanguageFeatureRegistry]> | undefined; + + const ensureProviderContexts = () => { + if (!providerContexts) { + providerContexts = [ + [EditorContextKeys.hasDefinitionProvider.bindTo(scopedContextKeyService), languageFeaturesService.definitionProvider], + [EditorContextKeys.hasReferenceProvider.bindTo(scopedContextKeyService), languageFeaturesService.referenceProvider], + [EditorContextKeys.hasImplementationProvider.bindTo(scopedContextKeyService), languageFeaturesService.implementationProvider], + [EditorContextKeys.hasTypeDefinitionProvider.bindTo(scopedContextKeyService), languageFeaturesService.typeDefinitionProvider], + ]; + } + }; const updateContextKeys = async () => { + ensureProviderContexts(); const modelRef = await textModelService.createModelReference(attachment.value.uri); try { const model = modelRef.object.textEditorModel; - for (const [contextKey, registry] of providerContexts) { + for (const [contextKey, registry] of providerContexts!) { contextKey.set(registry.has(model)); } } finally { modelRef.dispose(); } }; - store.add(addBasicContextMenu(accessor, widget, scopedContextKeyService, contextMenuId, attachment.value, updateContextKeys)); + + store.add(dom.addDisposableListener(widget, dom.EventType.CONTEXT_MENU, async domEvent => { + const event = new StandardMouseEvent(dom.getWindow(domEvent), domEvent); + dom.EventHelper.stop(domEvent, true); + + try { + await updateContextKeys(); + } catch (e) { + console.error(e); + } + + contextMenuService.showContextMenu({ + contextKeyService: scopedContextKeyService, + getAnchor: () => event, + getActions: () => { + const menu = menuService.getMenuActions(contextMenuId, scopedContextKeyService, { arg: attachment.value }); + return getFlatContextMenuActions(menu); + }, + }); + })); return store; } diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatInlineAnchorWidget.ts b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatInlineAnchorWidget.ts index d3cd2bd310e..d499fda339e 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatInlineAnchorWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatInlineAnchorWidget.ts @@ -23,7 +23,7 @@ import { getFlatContextMenuActions } from '../../../../../../platform/actions/br import { Action2, IMenuService, MenuId, registerAction2 } from '../../../../../../platform/actions/common/actions.js'; import { IClipboardService } from '../../../../../../platform/clipboard/common/clipboardService.js'; import { ICommandService } from '../../../../../../platform/commands/common/commands.js'; -import { IContextKey, IContextKeyService } from '../../../../../../platform/contextkey/common/contextkey.js'; +import { IContextKeyService } from '../../../../../../platform/contextkey/common/contextkey.js'; import { IContextMenuService } from '../../../../../../platform/contextview/browser/contextView.js'; import { IResourceStat } from '../../../../../../platform/dnd/browser/dnd.js'; import { ITextResourceEditorInput } from '../../../../../../platform/editor/common/editor.js'; @@ -125,8 +125,6 @@ export class InlineAnchorWidget extends Disposable { public static readonly className = 'chat-inline-anchor-widget'; - private readonly _chatResourceContext: IContextKey; - readonly data: ContentRefData; constructor( @@ -158,9 +156,6 @@ export class InlineAnchorWidget extends Disposable { ? { kind: 'symbol', symbol: inlineReference.inlineReference } : { uri: inlineReference.inlineReference }; - const contextKeyService = this._register(originalContextKeyService.createScoped(element)); - this._chatResourceContext = chatAttachmentResourceContextKey.bindTo(contextKeyService); - element.classList.add(InlineAnchorWidget.className, 'show-file-icons'); let iconText: Array; @@ -168,7 +163,6 @@ export class InlineAnchorWidget extends Disposable { let location: { readonly uri: URI; readonly range?: IRange }; - let updateContextKeys: (() => Promise) | undefined; if (this.data.kind === 'symbol') { const symbol = this.data.symbol; @@ -176,7 +170,7 @@ export class InlineAnchorWidget extends Disposable { iconText = [this.data.symbol.name]; iconClasses = ['codicon', ...getIconClasses(modelService, languageService, undefined, undefined, SymbolKinds.toIcon(symbol.kind))]; - this._store.add(instantiationService.invokeFunction(accessor => hookUpSymbolAttachmentDragAndContextMenu(accessor, element, contextKeyService, { value: symbol.location, name: symbol.name, kind: symbol.kind }, MenuId.ChatInlineSymbolAnchorContext))); + this._store.add(instantiationService.invokeFunction(accessor => hookUpSymbolAttachmentDragAndContextMenu(accessor, element, originalContextKeyService, { value: symbol.location, name: symbol.name, kind: symbol.kind }, MenuId.ChatInlineSymbolAnchorContext))); } else { location = this.data; @@ -209,10 +203,10 @@ export class InlineAnchorWidget extends Disposable { refreshIconClasses(); })); - const isFolderContext = ExplorerFolderContext.bindTo(contextKeyService); + let isDirectory = false; fileService.stat(location.uri) .then(stat => { - isFolderContext.set(stat.isDirectory); + isDirectory = stat.isDirectory; if (stat.isDirectory) { fileKind = FileKind.FOLDER; refreshIconClasses(); @@ -221,15 +215,20 @@ export class InlineAnchorWidget extends Disposable { .catch(() => { }); // Context menu + const contextKeyService = this._register(originalContextKeyService.createScoped(element)); + chatAttachmentResourceContextKey.bindTo(contextKeyService).set(location.uri.toString()); + const isFolderContext = ExplorerFolderContext.bindTo(contextKeyService); + let contextMenuInitialized = false; this._register(dom.addDisposableListener(element, dom.EventType.CONTEXT_MENU, async domEvent => { const event = new StandardMouseEvent(dom.getWindow(domEvent), domEvent); dom.EventHelper.stop(domEvent, true); - try { - await updateContextKeys?.(); - } catch (e) { - console.error(e); + if (!contextMenuInitialized) { + contextMenuInitialized = true; + const resourceContextKey = new StaticResourceContextKey(contextKeyService, fileService, languageService, modelService); + resourceContextKey.set(location.uri); } + isFolderContext.set(isDirectory); if (this._store.isDisposed) { return; @@ -255,10 +254,6 @@ export class InlineAnchorWidget extends Disposable { } } - const resourceContextKey = new StaticResourceContextKey(contextKeyService, fileService, languageService, modelService); - resourceContextKey.set(location.uri); - this._chatResourceContext.set(location.uri.toString()); - const iconEl = dom.$('span.icon'); iconEl.classList.add(...iconClasses); element.replaceChildren(iconEl, dom.$('span.icon-label', {}, ...iconText)); diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatMarkdownDecorationsRenderer.ts b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatMarkdownDecorationsRenderer.ts index cd6169b793e..12dbdf346e3 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatMarkdownDecorationsRenderer.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatMarkdownDecorationsRenderer.ts @@ -260,6 +260,44 @@ export class ChatMarkdownDecorationsRenderer { return container; } + /** + * Renders a parsed chat request as plain text DOM elements with inline decoration spans + * for agents, slash commands, and other special parts. No markdown rendering is used. + */ + renderParsedRequestToPlainText(parsedRequest: IParsedChatRequest, store: DisposableStore): HTMLElement { + const container = dom.$('span'); + container.style.whiteSpace = 'pre-wrap'; + for (const part of parsedRequest.parts) { + if (part instanceof ChatRequestTextPart) { + container.appendChild(document.createTextNode(part.text)); + } else if (part instanceof ChatRequestAgentPart) { + const widget = this.renderResourceWidget(`${chatAgentLeader}${part.agent.name}`, undefined, store); + const hover: Lazy = new Lazy(() => store.add(this.instantiationService.createInstance(ChatAgentHover))); + store.add(this.hoverService.setupManagedHover(getDefaultHoverDelegate('element'), widget, () => { + hover.value.setAgent(part.agent.id); + return hover.value.domNode; + }, getChatAgentHoverOptions(() => part.agent, this.commandService))); + container.appendChild(widget); + } else { + const title = this.getDecorationTitle(part); + const widget = this.renderResourceWidget(part.text, title ? { title } : undefined, store); + container.appendChild(widget); + } + } + return container; + } + + private getDecorationTitle(part: IParsedChatRequestPart): string | undefined { + const uri = part instanceof ChatRequestDynamicVariablePart && part.data instanceof URI ? + part.data : + undefined; + return uri ? this.labelService.getUriLabel(uri, { relative: true }) : + part instanceof ChatRequestSlashCommandPart ? part.slashCommand.detail : + part instanceof ChatRequestAgentSubcommandPart ? part.command.description : + part instanceof ChatRequestSlashPromptPart ? part.name : + part instanceof ChatRequestToolPart ? (this.toolsService.getTool(part.toolId)?.userDescription) : + undefined; + } private injectKeybindingHint(a: HTMLAnchorElement, href: string, keybindingService: IKeybindingService): void { const command = href.match(/command:([^\)]+)/)?.[1]; diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatProgressContentPart.ts b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatProgressContentPart.ts index 28d1f62c1c1..ad5bf6fee22 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatProgressContentPart.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatProgressContentPart.ts @@ -7,7 +7,7 @@ import { $, append } from '../../../../../../base/browser/dom.js'; import { alert } from '../../../../../../base/browser/ui/aria/aria.js'; import { Codicon } from '../../../../../../base/common/codicons.js'; import { MarkdownString, type IMarkdownString } from '../../../../../../base/common/htmlContent.js'; -import { Disposable, MutableDisposable } from '../../../../../../base/common/lifecycle.js'; +import { Disposable, DisposableStore, MutableDisposable } from '../../../../../../base/common/lifecycle.js'; import { ThemeIcon } from '../../../../../../base/common/themables.js'; import { IMarkdownRenderer } from '../../../../../../platform/markdown/browser/markdownRenderer.js'; import { IRenderedMarkdown } from '../../../../../../base/browser/markdownRenderer.js'; @@ -33,6 +33,7 @@ export class ChatProgressContentPart extends Disposable implements IChatContentP private readonly showSpinner: boolean; private readonly isHidden: boolean; private readonly renderedMessage = this._register(new MutableDisposable()); + private readonly _fileWidgetStore = this._register(new DisposableStore()); private currentContent: IMarkdownString; constructor( @@ -67,7 +68,7 @@ export class ChatProgressContentPart extends Disposable implements IChatContentP const codicon = icon ? icon : this.showSpinner ? ThemeIcon.modify(Codicon.loading, 'spin') : Codicon.check; const result = this.chatContentMarkdownRenderer.render(progress.content); result.element.classList.add('progress-step'); - renderFileWidgets(result.element, this.instantiationService, this.chatMarkdownAnchorService, this._store); + renderFileWidgets(result.element, this.instantiationService, this.chatMarkdownAnchorService, this._fileWidgetStore); const tooltip: IMarkdownString | undefined = this.createApprovalMessage(); const progressPart = this._register(instantiationService.createInstance(ChatProgressSubPart, result.element, codicon, tooltip)); @@ -83,7 +84,8 @@ export class ChatProgressContentPart extends Disposable implements IChatContentP // Render the new message const result = this._register(this.chatContentMarkdownRenderer.render(content)); result.element.classList.add('progress-step'); - renderFileWidgets(result.element, this.instantiationService, this.chatMarkdownAnchorService, this._store); + this._fileWidgetStore.clear(); + renderFileWidgets(result.element, this.instantiationService, this.chatMarkdownAnchorService, this._fileWidgetStore); // Replace the old message container with the new one if (this.renderedMessage.value) { diff --git a/src/vs/workbench/contrib/chat/browser/widget/media/chat.css b/src/vs/workbench/contrib/chat/browser/widget/media/chat.css index ff8f0fc8f16..f763f219b8c 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/media/chat.css +++ b/src/vs/workbench/contrib/chat/browser/widget/media/chat.css @@ -457,12 +457,14 @@ } /* not ideal but I cannot query the last div with this class... */ - .rendered-markdown:last-of-type > P > SPAN:empty { + .rendered-markdown:last-of-type > P > SPAN:empty, + .rendered-markdown:last-of-type > SPAN:empty { display: inline-block; width: 11px; } - .rendered-markdown:last-of-type > P > SPAN:empty::after { + .rendered-markdown:last-of-type > P > SPAN:empty::after, + .rendered-markdown:last-of-type > SPAN:empty::after { content: ''; white-space: nowrap; overflow: hidden;