Skip to content

Commit ef35ae4

Browse files
committed
Fix chat MarkdownString merging
1 parent d28dc16 commit ef35ae4

File tree

4 files changed

+36
-26
lines changed

4 files changed

+36
-26
lines changed

src/vs/workbench/contrib/chat/browser/chatListRenderer.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -953,14 +953,6 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch
953953
private renderMarkdown(markdown: IMarkdownString, element: ChatTreeItem, templateData: IChatListItemTemplate, fillInIncompleteTokens = false): IMarkdownRenderResult {
954954
const disposables = new DisposableStore();
955955

956-
markdown = new MarkdownString(markdown.value, {
957-
isTrusted: {
958-
// Disable all other config options except isTrusted
959-
enabledCommands: typeof markdown.isTrusted === 'object' ? markdown.isTrusted?.enabledCommands : [] ?? []
960-
},
961-
supportHtml: true
962-
});
963-
964956
// We release editors in order so that it's more likely that the same editor will be assigned if this element is re-rendered right away, like it often is during progressive rendering
965957
const orderedDisposablesList: IDisposable[] = [];
966958
const codeblocks: IChatCodeBlockInfo[] = [];

src/vs/workbench/contrib/chat/common/annotations.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { MarkdownString } from 'vs/base/common/htmlContent';
66
import { basename } from 'vs/base/common/resources';
77
import { URI } from 'vs/base/common/uri';
88
import { IRange } from 'vs/editor/common/core/range';
9-
import { IChatProgressRenderableResponseContent, IChatProgressResponseContent } from 'vs/workbench/contrib/chat/common/chatModel';
9+
import { IChatProgressRenderableResponseContent, IChatProgressResponseContent, canMergeMarkdownStrings } from 'vs/workbench/contrib/chat/common/chatModel';
1010
import { IChatAgentMarkdownContentWithVulnerability, IChatAgentVulnerabilityDetails, IChatContentInlineReference, IChatMarkdownContent } from 'vs/workbench/contrib/chat/common/chatService';
1111

1212
export const contentRefUrl = 'http://_vscodecontentref_'; // must be lowercase for URI
@@ -20,16 +20,17 @@ export function annotateSpecialMarkdownContent(response: ReadonlyArray<IChatProg
2020
const printUri = URI.parse(contentRefUrl).with({ fragment: JSON.stringify(location) });
2121
const markdownText = `[${item.name || basename(location.uri)}](${printUri.toString()})`;
2222
if (previousItem?.kind === 'markdownContent') {
23-
result[result.length - 1] = { content: new MarkdownString(previousItem.content.value + markdownText, { isTrusted: previousItem.content.isTrusted }), kind: 'markdownContent' };
23+
result[result.length - 1] = { content: new MarkdownString(previousItem.content.value + markdownText), kind: 'markdownContent' };
2424
} else {
2525
result.push({ content: new MarkdownString(markdownText), kind: 'markdownContent' });
2626
}
27-
} else if (item.kind === 'markdownContent' && previousItem?.kind === 'markdownContent') {
28-
result[result.length - 1] = { content: new MarkdownString(previousItem.content.value + item.content.value, { isTrusted: previousItem.content.isTrusted }), kind: 'markdownContent' };
27+
} else if (item.kind === 'markdownContent' && previousItem?.kind === 'markdownContent' && canMergeMarkdownStrings(previousItem.content, item.content)) {
28+
result[result.length - 1] = { content: new MarkdownString(previousItem.content.value + item.content.value), kind: 'markdownContent' };
2929
} else if (item.kind === 'markdownVuln') {
3030
const vulnText = encodeURIComponent(JSON.stringify(item.vulnerabilities));
3131
const markdownText = `<vscode_annotation details='${vulnText}'>${item.content.value}</vscode_annotation>`;
3232
if (previousItem?.kind === 'markdownContent') {
33+
// Since this is inside a codeblock, it needs to be merged into the previous markdown content.
3334
result[result.length - 1] = { content: new MarkdownString(previousItem.content.value + markdownText, { isTrusted: previousItem.content.isTrusted }), kind: 'markdownContent' };
3435
} else {
3536
result.push({ content: new MarkdownString(markdownText), kind: 'markdownContent' });
@@ -101,4 +102,3 @@ export function extractVulnerabilitiesFromText(text: string): { newText: string;
101102

102103
return { newText, vulnerabilities };
103104
}
104-

src/vs/workbench/contrib/chat/common/chatModel.ts

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { Emitter, Event } from 'vs/base/common/event';
99
import { IMarkdownString, MarkdownString, isMarkdownString } from 'vs/base/common/htmlContent';
1010
import { Disposable } from 'vs/base/common/lifecycle';
1111
import { revive } from 'vs/base/common/marshalling';
12+
import { equals } from 'vs/base/common/objects';
1213
import { basename, isEqual } from 'vs/base/common/resources';
1314
import { ThemeIcon } from 'vs/base/common/themables';
1415
import { URI, UriComponents, UriDto, isUriComponents } from 'vs/base/common/uri';
@@ -164,22 +165,19 @@ export class Response implements IResponse {
164165
const responsePartLength = this._responseParts.length - 1;
165166
const lastResponsePart = this._responseParts[responsePartLength];
166167

167-
if (!lastResponsePart || lastResponsePart.kind !== 'markdownContent') {
168-
// The last part can't be merged with
168+
if (!lastResponsePart || lastResponsePart.kind !== 'markdownContent' || !canMergeMarkdownStrings(lastResponsePart.content, progress.content)) {
169+
// The last part can't be merged with- not markdown, or markdown with different permissions
169170
this._responseParts.push(progress);
170171
} else {
171-
// Merge all enabled commands TODO
172-
const lastPartEnabledCommands = typeof lastResponsePart.content.isTrusted === 'object' ?
173-
lastResponsePart.content.isTrusted.enabledCommands :
174-
[];
175-
const thisPartEnabledCommands = typeof progress.content.isTrusted === 'object' ?
176-
progress.content.isTrusted.enabledCommands :
177-
[];
178-
const enabledCommands = [...lastPartEnabledCommands, ...thisPartEnabledCommands];
179-
this._responseParts[responsePartLength] = { content: new MarkdownString(lastResponsePart.content.value + progress.content.value, { isTrusted: { enabledCommands } }), kind: 'markdownContent' };
172+
lastResponsePart.content = {
173+
value: lastResponsePart.content.value + progress.content.value,
174+
isTrusted: lastResponsePart.content.isTrusted,
175+
supportThemeIcons: lastResponsePart.content.supportThemeIcons,
176+
supportHtml: lastResponsePart.content.supportHtml,
177+
baseUri: lastResponsePart.content.baseUri
178+
} satisfies IMarkdownString;
180179
}
181180
this._updateRepr(quiet);
182-
183181
} else if (progress.kind === 'textEdit') {
184182
if (progress.edits.length > 0) {
185183
// merge text edits for the same file no matter when they come in
@@ -936,3 +934,22 @@ export function updateRanges(variableData: IChatRequestVariableData, diff: numbe
936934
}))
937935
};
938936
}
937+
938+
export function canMergeMarkdownStrings(md1: IMarkdownString, md2: IMarkdownString): boolean {
939+
if (md1.baseUri && md2.baseUri) {
940+
const baseUriEquals = md1.baseUri.scheme === md2.baseUri.scheme
941+
&& md1.baseUri.authority === md2.baseUri.authority
942+
&& md1.baseUri.path === md2.baseUri.path
943+
&& md1.baseUri.query === md2.baseUri.query
944+
&& md1.baseUri.fragment === md2.baseUri.fragment;
945+
if (!baseUriEquals) {
946+
return false;
947+
}
948+
} else if (md1.baseUri || md2.baseUri) {
949+
return false;
950+
}
951+
952+
return equals(md1.isTrusted, md2.isTrusted) &&
953+
md1.supportHtml === md2.supportHtml &&
954+
md1.supportThemeIcons === md2.supportThemeIcons;
955+
}

src/vscode-dts/vscode.proposed.chatParticipant.d.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ declare module 'vscode' {
309309
* `push(new ChatResponseMarkdownPart(value))`.
310310
*
311311
* @see {@link ChatResponseStream.push}
312-
* @param value A markdown string or a string that should be interpreted as markdown.
312+
* @param value A markdown string or a string that should be interpreted as markdown. The boolean form of {@link MarkdownString.isTrusted} is NOT supported.
313313
* @returns This stream.
314314
*/
315315
markdown(value: string | MarkdownString): ChatResponseStream;
@@ -373,6 +373,7 @@ declare module 'vscode' {
373373
}
374374

375375
export class ChatResponseMarkdownPart {
376+
/** Note: The boolean form of {@link MarkdownString.isTrusted} is NOT supported. */
376377
value: MarkdownString;
377378
constructor(value: string | MarkdownString);
378379
}

0 commit comments

Comments
 (0)