Skip to content

Fix terminal approval UI missing backslashes on Windows paths #309797

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Copilot wants to merge 2 commits into from
+10 −1
Closed

Fix terminal approval UI missing backslashes on Windows paths #309797

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
022b99f
Initial plan
Copilot Apr 14, 2026
df29960
fix: properly escape string titles in confirmation widget to preserve…
Copilot Apr 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatConfirmationWidget.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,10 +335,19 @@ abstract class BaseChatConfirmationWidget<T> extends Disposable {
this._domNode = elements.root;
this._buttonsDomNode = elements.buttons;

const titleMd = new MarkdownString('', { supportThemeIcons: true });
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When title is an IMarkdownString, this constructs a brand-new MarkdownString but only copies title.value, dropping any other IMarkdownString metadata (commonly isTrusted, supportHtml, baseUri, and potentially supportThemeIcons). That can change how links/commands resolve or whether markdown is trusted. Consider initializing titleMd with options derived from the incoming title (and then force-enable theme icons to support the icon prefix), so consumer-provided markdown semantics are preserved.

Suggested change
const titleMd = new MarkdownString('', { supportThemeIcons: true });
const titleMd = typeof title === 'string'
? new MarkdownString('', { supportThemeIcons: true })
: new MarkdownString('', {
isTrusted: title.isTrusted,
supportHtml: title.supportHtml,
baseUri: title.baseUri,
supportThemeIcons: true,
});

Copilot uses AI. Check for mistakes.
if (icon) {
titleMd.appendMarkdown(`$(${icon.id}) `);
}
if (typeof title === 'string') {
titleMd.appendText(title);
} else {
titleMd.appendMarkdown(title.value);
}
Comment on lines +338 to +346
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This re-implements the same “string vs IMarkdownString” conversion logic that already exists in ChatQueryTitlePart (mentioned in the PR description via toMdString()), which risks behavior drift (escaping rules, future markdown options, etc.). Consider reusing the shared conversion helper for title and only layering the icon prefix on top, so there’s a single canonical implementation of how titles become markdown.

Suggested change
const titleMd = new MarkdownString('', { supportThemeIcons: true });
if (icon) {
titleMd.appendMarkdown(`$(${icon.id}) `);
}
if (typeof title === 'string') {
titleMd.appendText(title);
} else {
titleMd.appendMarkdown(title.value);
}
const convertedTitleMd = ChatQueryTitlePart.toMdString(title);
const titleMd = icon
? (() => {
const prefixedTitleMd = new MarkdownString('', { supportThemeIcons: true });
prefixedTitleMd.appendMarkdown(`$(${icon.id}) `);
prefixedTitleMd.appendMarkdown(convertedTitleMd.value);
return prefixedTitleMd;
})()
: convertedTitleMd;

Copilot uses AI. Check for mistakes.
Comment on lines +344 to +346
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When title is an IMarkdownString, this constructs a brand-new MarkdownString but only copies title.value, dropping any other IMarkdownString metadata (commonly isTrusted, supportHtml, baseUri, and potentially supportThemeIcons). That can change how links/commands resolve or whether markdown is trusted. Consider initializing titleMd with options derived from the incoming title (and then force-enable theme icons to support the icon prefix), so consumer-provided markdown semantics are preserved.

Copilot uses AI. Check for mistakes.
this._register(instantiationService.createInstance(
ChatQueryTitlePart,
elements.title,
new MarkdownString(icon ? `$(${icon.id}) ${typeof title === 'string' ? title : title.value}` : typeof title === 'string' ? title : title.value),
titleMd,
subtitle,
));

Expand Down
Loading