From db4938fa5099ed24e8e88935c7a3609952ece61f Mon Sep 17 00:00:00 2001 From: Matt Bierner <12821956+mjbvz@users.noreply.github.com> Date: Thu, 9 Oct 2025 09:45:04 -0700 Subject: [PATCH] Avoid using custom markdown actionHandlers in a few cases Tries to use the standard markdown actionHandler in more cases As part of this, switches some callers from the low level `renderMarkdown` function to the `IMarkdownRendererService` --- .../services/hoverService/hoverWidget.ts | 12 ++---- .../markdown/browser/markdownRenderer.ts | 20 +++++++--- .../browser/extensionFeaturesTab.ts | 37 +++++++------------ .../extensions/browser/extensionsWidgets.ts | 11 ++---- .../contrib/mcp/browser/mcpServerWidgets.ts | 11 ++---- .../contrib/mcp/browser/mcpServersView.ts | 23 +++++------- 6 files changed, 48 insertions(+), 66 deletions(-) diff --git a/src/vs/editor/browser/services/hoverService/hoverWidget.ts b/src/vs/editor/browser/services/hoverService/hoverWidget.ts index 5db8f80e381..a734f50acba 100644 --- a/src/vs/editor/browser/services/hoverService/hoverWidget.ts +++ b/src/vs/editor/browser/services/hoverService/hoverWidget.ts @@ -13,8 +13,7 @@ import { IConfigurationService } from '../../../../platform/configuration/common import { HoverAction, HoverPosition, HoverWidget as BaseHoverWidget, getHoverAccessibleViewHint } from '../../../../base/browser/ui/hover/hoverWidget.js'; import { Widget } from '../../../../base/browser/ui/widget.js'; import { AnchorPosition } from '../../../../base/browser/ui/contextview/contextview.js'; -import { IOpenerService } from '../../../../platform/opener/common/opener.js'; -import { IMarkdownRendererService, openLinkFromMarkdown } from '../../../../platform/markdown/browser/markdownRenderer.js'; +import { IMarkdownRendererService } from '../../../../platform/markdown/browser/markdownRenderer.js'; import { isMarkdownString } from '../../../../base/common/htmlContent.js'; import { localize } from '../../../../nls.js'; import { isMacintosh } from '../../../../base/common/platform.js'; @@ -49,7 +48,7 @@ export class HoverWidget extends Widget implements IHoverWidget { private readonly _hoverPointer: HTMLElement | undefined; private readonly _hoverContainer: HTMLElement; private readonly _target: IHoverTarget; - private readonly _linkHandler: (url: string) => void; + private readonly _linkHandler: ((url: string) => void) | undefined; private _isDisposed: boolean = false; private _hoverPosition: HoverPosition; @@ -98,15 +97,12 @@ export class HoverWidget extends Widget implements IHoverWidget { options: IHoverOptions, @IKeybindingService private readonly _keybindingService: IKeybindingService, @IConfigurationService private readonly _configurationService: IConfigurationService, - @IOpenerService private readonly _openerService: IOpenerService, @IMarkdownRendererService private readonly _markdownRenderer: IMarkdownRendererService, @IAccessibilityService private readonly _accessibilityService: IAccessibilityService ) { super(); - this._linkHandler = options.linkHandler || (url => { - return openLinkFromMarkdown(this._openerService, url, isMarkdownString(options.content) ? options.content.isTrusted : undefined); - }); + this._linkHandler = options.linkHandler; this._target = 'targetElements' in options.target ? options.target : new ElementHoverTarget(options.target); @@ -166,7 +162,7 @@ export class HoverWidget extends Widget implements IHoverWidget { const markdown = options.content; const { element, dispose } = this._markdownRenderer.render(markdown, { - actionHandler: (content) => this._linkHandler(content), + actionHandler: this._linkHandler, asyncRenderCallback: () => { contentsElement.classList.add('code-hover-contents'); this.layout(); diff --git a/src/vs/platform/markdown/browser/markdownRenderer.ts b/src/vs/platform/markdown/browser/markdownRenderer.ts index fed8adf5d36..1217633b485 100644 --- a/src/vs/platform/markdown/browser/markdownRenderer.ts +++ b/src/vs/platform/markdown/browser/markdownRenderer.ts @@ -62,13 +62,21 @@ export class MarkdownRendererService implements IMarkdownRendererService { ) { } render(markdown: IMarkdownString, options?: MarkdownRenderOptions & IMarkdownRendererExtraOptions, outElement?: HTMLElement): IRenderedMarkdown { - const rendered = renderMarkdown(markdown, { - codeBlockRenderer: (alias, value) => this._defaultCodeBlockRenderer?.renderCodeBlock(alias, value, options ?? {}) ?? Promise.resolve(document.createElement('span')), - actionHandler: (link, mdStr) => { + const resolvedOptions = { ...options }; + + if (!resolvedOptions.actionHandler) { + resolvedOptions.actionHandler = (link, mdStr) => { return openLinkFromMarkdown(this._openerService, link, mdStr.isTrusted); - }, - ...options, - }, outElement); + }; + } + + if (!resolvedOptions.codeBlockRenderer) { + resolvedOptions.codeBlockRenderer = (alias, value) => { + return this._defaultCodeBlockRenderer?.renderCodeBlock(alias, value, resolvedOptions ?? {}) ?? Promise.resolve(document.createElement('span')); + }; + } + + const rendered = renderMarkdown(markdown, resolvedOptions, outElement); rendered.element.classList.add('rendered-markdown'); return rendered; } diff --git a/src/vs/workbench/contrib/extensions/browser/extensionFeaturesTab.ts b/src/vs/workbench/contrib/extensions/browser/extensionFeaturesTab.ts index 6ae42e60fc3..3cd48a5bd37 100644 --- a/src/vs/workbench/contrib/extensions/browser/extensionFeaturesTab.ts +++ b/src/vs/workbench/contrib/extensions/browser/extensionFeaturesTab.ts @@ -17,9 +17,7 @@ import { getExtensionId } from '../../../../platform/extensionManagement/common/ import { IListRenderer, IListVirtualDelegate } from '../../../../base/browser/ui/list/list.js'; import { Button } from '../../../../base/browser/ui/button/button.js'; import { defaultButtonStyles, defaultKeybindingLabelStyles } from '../../../../platform/theme/browser/defaultStyles.js'; -import { renderMarkdown } from '../../../../base/browser/markdownRenderer.js'; -import { getErrorMessage, onUnexpectedError } from '../../../../base/common/errors.js'; -import { IOpenerService } from '../../../../platform/opener/common/opener.js'; +import { getErrorMessage } from '../../../../base/common/errors.js'; import { PANEL_SECTION_BORDER } from '../../../common/theme.js'; import { IThemeService, Themable } from '../../../../platform/theme/common/themeService.js'; import { DomScrollableElement } from '../../../../base/browser/ui/scrollbar/scrollableElement.js'; @@ -39,6 +37,7 @@ import { ResolvedKeybinding } from '../../../../base/common/keybindings.js'; import { asCssVariable } from '../../../../platform/theme/common/colorUtils.js'; import { foreground, chartAxis, chartGuide, chartLine } from '../../../../platform/theme/common/colorRegistry.js'; import { IHoverService } from '../../../../platform/hover/browser/hover.js'; +import { IMarkdownRendererService } from '../../../../platform/markdown/browser/markdownRenderer.js'; interface IExtensionFeatureElementRenderer extends IExtensionFeatureRenderer { type: 'element'; @@ -52,9 +51,9 @@ class RuntimeStatusMarkdownRenderer extends Disposable implements IExtensionFeat constructor( @IExtensionService private readonly extensionService: IExtensionService, - @IOpenerService private readonly openerService: IOpenerService, @IHoverService private readonly hoverService: IHoverService, @IExtensionFeaturesManagementService private readonly extensionFeaturesManagementService: IExtensionFeaturesManagementService, + @IMarkdownRendererService private readonly markdownRendererService: IMarkdownRendererService, ) { super(); } @@ -152,15 +151,11 @@ class RuntimeStatusMarkdownRenderer extends Disposable implements IExtensionFeat } private renderMarkdown(markdown: IMarkdownString, container: HTMLElement, disposables: DisposableStore): void { - const { element } = disposables.add(renderMarkdown( - { - value: markdown.value, - isTrusted: markdown.isTrusted, - supportThemeIcons: true - }, - { - actionHandler: (content) => this.openerService.open(content, { allowCommands: !!markdown.isTrusted }).catch(onUnexpectedError), - })); + const { element } = disposables.add(this.markdownRendererService.render({ + value: markdown.value, + isTrusted: markdown.isTrusted, + supportThemeIcons: true + })); append(container, element); } @@ -547,10 +542,10 @@ class ExtensionFeatureView extends Disposable { private readonly extensionId: ExtensionIdentifier, private readonly manifest: IExtensionManifest, readonly feature: IExtensionFeatureDescriptor, - @IOpenerService private readonly openerService: IOpenerService, @IInstantiationService private readonly instantiationService: IInstantiationService, @IExtensionFeaturesManagementService private readonly extensionFeaturesManagementService: IExtensionFeaturesManagementService, @IDialogService private readonly dialogService: IDialogService, + @IMarkdownRendererService private readonly markdownRendererService: IMarkdownRendererService, ) { super(); @@ -702,15 +697,11 @@ class ExtensionFeatureView extends Disposable { } private renderMarkdown(markdown: IMarkdownString, container: HTMLElement): void { - const { element } = this._register(renderMarkdown( - { - value: markdown.value, - isTrusted: markdown.isTrusted, - supportThemeIcons: true - }, - { - actionHandler: (content) => this.openerService.open(content, { allowCommands: !!markdown.isTrusted }).catch(onUnexpectedError), - })); + const { element } = this._register(this.markdownRendererService.render({ + value: markdown.value, + isTrusted: markdown.isTrusted, + supportThemeIcons: true + })); append(container, element); } diff --git a/src/vs/workbench/contrib/extensions/browser/extensionsWidgets.ts b/src/vs/workbench/contrib/extensions/browser/extensionsWidgets.ts index e3863b16ae0..012c6840c2c 100644 --- a/src/vs/workbench/contrib/extensions/browser/extensionsWidgets.ts +++ b/src/vs/workbench/contrib/extensions/browser/extensionsWidgets.ts @@ -32,9 +32,7 @@ import { IExtensionService } from '../../../services/extensions/common/extension import { areSameExtensions } from '../../../../platform/extensionManagement/common/extensionManagementUtil.js'; import Severity from '../../../../base/common/severity.js'; import { Color } from '../../../../base/common/color.js'; -import { renderMarkdown } from '../../../../base/browser/markdownRenderer.js'; import { IOpenerService } from '../../../../platform/opener/common/opener.js'; -import { onUnexpectedError } from '../../../../base/common/errors.js'; import { renderIcon } from '../../../../base/browser/ui/iconLabel/iconLabels.js'; import { StandardKeyboardEvent } from '../../../../base/browser/keyboardEvent.js'; import { KeyCode } from '../../../../base/common/keyCodes.js'; @@ -51,6 +49,7 @@ import { IExplorerService } from '../../files/browser/files.js'; import { IViewsService } from '../../../services/views/common/viewsService.js'; import { VIEW_ID as EXPLORER_VIEW_ID } from '../../files/common/files.js'; import { IExtensionGalleryManifest, IExtensionGalleryManifestService } from '../../../../platform/extensionManagement/common/extensionGalleryManifest.js'; +import { IMarkdownRendererService } from '../../../../platform/markdown/browser/markdownRenderer.js'; export abstract class ExtensionWidget extends Disposable implements IExtensionContainer { private _extension: IExtension | null = null; @@ -998,7 +997,7 @@ export class ExtensionStatusWidget extends ExtensionWidget { constructor( private readonly container: HTMLElement, private readonly extensionStatusAction: ExtensionStatusAction, - @IOpenerService private readonly openerService: IOpenerService, + @IMarkdownRendererService private readonly markdownRendererService: IMarkdownRendererService, ) { super(); this.render(); @@ -1023,11 +1022,7 @@ export class ExtensionStatusWidget extends ExtensionWidget { markdown.appendText(`\n`); } } - const rendered = disposables.add(renderMarkdown(markdown, { - actionHandler: (content) => { - this.openerService.open(content, { allowCommands: true }).catch(onUnexpectedError); - }, - })); + const rendered = disposables.add(this.markdownRendererService.render(markdown)); append(this.container, rendered.element); } this._onDidRender.fire(); diff --git a/src/vs/workbench/contrib/mcp/browser/mcpServerWidgets.ts b/src/vs/workbench/contrib/mcp/browser/mcpServerWidgets.ts index 6761a41c515..47dbe902e79 100644 --- a/src/vs/workbench/contrib/mcp/browser/mcpServerWidgets.ts +++ b/src/vs/workbench/contrib/mcp/browser/mcpServerWidgets.ts @@ -24,14 +24,13 @@ import { McpServerStatusAction } from './mcpServerActions.js'; import { reset } from '../../../../base/browser/dom.js'; import { mcpLicenseIcon, mcpServerIcon, mcpServerRemoteIcon, mcpServerWorkspaceIcon, mcpStarredIcon } from './mcpServerIcons.js'; import { MarkdownString } from '../../../../base/common/htmlContent.js'; -import { renderMarkdown } from '../../../../base/browser/markdownRenderer.js'; -import { onUnexpectedError } from '../../../../base/common/errors.js'; import { ExtensionHoverOptions, ExtensionIconBadge } from '../../extensions/browser/extensionsWidgets.js'; import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js'; import { LocalMcpServerScope } from '../../../services/mcp/common/mcpWorkbenchManagementService.js'; import { IInstantiationService } from '../../../../platform/instantiation/common/instantiation.js'; import { registerColor } from '../../../../platform/theme/common/colorUtils.js'; import { textLinkForeground } from '../../../../platform/theme/common/colorRegistry.js'; +import { IMarkdownRendererService } from '../../../../platform/markdown/browser/markdownRenderer.js'; export abstract class McpServerWidget extends Disposable implements IMcpServerContainer { private _mcpServer: IWorkbenchMcpServer | null = null; @@ -451,7 +450,7 @@ export class McpServerStatusWidget extends McpServerWidget { constructor( private readonly container: HTMLElement, private readonly extensionStatusAction: McpServerStatusAction, - @IOpenerService private readonly openerService: IOpenerService, + @IMarkdownRendererService private readonly markdownRendererService: IMarkdownRendererService, ) { super(); this.render(); @@ -476,11 +475,7 @@ export class McpServerStatusWidget extends McpServerWidget { markdown.appendText(`\n`); } } - const rendered = disposables.add(renderMarkdown(markdown, { - actionHandler: (content) => { - this.openerService.open(content, { allowCommands: true }).catch(onUnexpectedError); - } - })); + const rendered = disposables.add(this.markdownRendererService.render(markdown)); dom.append(this.container, rendered.element); } this._onDidRender.fire(); diff --git a/src/vs/workbench/contrib/mcp/browser/mcpServersView.ts b/src/vs/workbench/contrib/mcp/browser/mcpServersView.ts index 5a12c9d54f3..38eaf863569 100644 --- a/src/vs/workbench/contrib/mcp/browser/mcpServersView.ts +++ b/src/vs/workbench/contrib/mcp/browser/mcpServersView.ts @@ -32,7 +32,6 @@ import { PublisherWidget, StarredWidget, McpServerIconWidget, McpServerHoverWidg import { ActionRunner, IAction, Separator } from '../../../../base/common/actions.js'; import { IActionViewItemOptions } from '../../../../base/browser/ui/actionbar/actionViewItems.js'; import { IAllowedMcpServersService, mcpGalleryServiceEnablementConfig, mcpGalleryServiceUrlConfig } from '../../../../platform/mcp/common/mcpManagement.js'; -import { URI } from '../../../../base/common/uri.js'; import { ThemeIcon } from '../../../../base/common/themables.js'; import { alert } from '../../../../base/browser/ui/aria/aria.js'; import { Registry } from '../../../../platform/registry/common/platform.js'; @@ -40,7 +39,6 @@ import { IWorkbenchContribution } from '../../../common/contributions.js'; import { SyncDescriptor } from '../../../../platform/instantiation/common/descriptors.js'; import { DefaultViewsContext, SearchMcpServersContext } from '../../extensions/common/extensions.js'; import { VIEW_CONTAINER } from '../../extensions/browser/extensions.contribution.js'; -import { renderMarkdown } from '../../../../base/browser/markdownRenderer.js'; import { ChatContextKeys } from '../../chat/common/chatContextKeys.js'; import { Button } from '../../../../base/browser/ui/button/button.js'; import { defaultButtonStyles } from '../../../../platform/theme/browser/defaultStyles.js'; @@ -53,6 +51,7 @@ import { IPagedRenderer } from '../../../../base/browser/ui/list/listPaging.js'; import { IMcpGalleryManifestService, McpGalleryManifestStatus } from '../../../../platform/mcp/common/mcpGalleryManifest.js'; import { ProductQualityContext } from '../../../../platform/contextkey/common/contextkeys.js'; import { SeverityIcon } from '../../../../base/browser/ui/severityIcon/severityIcon.js'; +import { IMarkdownRendererService } from '../../../../platform/markdown/browser/markdownRenderer.js'; export interface McpServerListViewOptions { showWelcome?: boolean; @@ -100,6 +99,7 @@ export class McpServersListView extends AbstractExtensionsListView { - this.openerService.open(URI.parse(content), { allowCommands: ['workbench.action.openSettings'] }); - } - })); + const markdownResult = this._register(this.markdownRendererService.render( + new MarkdownString( + localize('mcp.welcome.descriptionWithLink', "Browse and install [Model Context Protocol (MCP) servers](https://code.visualstudio.com/docs/copilot/customization/mcp-servers) directly from VS Code to extend agent mode with extra tools for connecting to databases, invoking APIs and performing specialized tasks."), + { isTrusted: { enabledCommands: ['workbench.action.openSettings'] } }, + ) + .appendMarkdown('\n\n') + .appendMarkdown(localize('mcp.gallery.enableDialog.setting', "This feature is currently in preview. You can disable it anytime using the setting {0}.", settingsCommandLink)), + )); description.appendChild(markdownResult.element); const buttonContainer = dom.append(welcomeContent, dom.$('.mcp-welcome-button-container'));