Skip to content

Commit

Permalink
incorporate review comments
Browse files Browse the repository at this point in the history
- revert model changes
- create custom hook for rendering
  • Loading branch information
eneufeld committed Sep 25, 2024
1 parent 199e634 commit 0a6f815
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,21 +39,6 @@ export class MarkdownPartRenderer implements ChatResponsePartRenderer<MarkdownCh
return -1;
}
render(response: MarkdownChatResponseContent | InformationalChatResponseContent): ReactNode {
// // eslint-disable-next-line no-null/no-null
// const ref: React.MutableRefObject<HTMLDivElement | null> = useRef(null);

// useEffect(() => {
// const host = document.createElement('div');
// const html = this.markdownIt.render(response.content);
// host.innerHTML = DOMPurify.sanitize(html, {
// ALLOW_UNKNOWN_PROTOCOLS: true // DOMPurify usually strips non http(s) links from hrefs
// });
// while (ref?.current?.firstChild) {
// ref.current.removeChild(ref.current.firstChild);
// }

// ref?.current?.appendChild(host);
// }, [response.content]);
// TODO let the user configure whether they want to see informational content
if (InformationalChatResponseContent.is(response)) {
// null is valid in React
Expand All @@ -69,11 +54,28 @@ export class MarkdownPartRenderer implements ChatResponsePartRenderer<MarkdownCh

const MarkdownRender = ({ response }: { response: MarkdownChatResponseContent | InformationalChatResponseContent }) => {
// eslint-disable-next-line no-null/no-null
const ref: React.MutableRefObject<HTMLDivElement | null> = useRef(null);
const ref = useMarkdownRendering(response.content.value);

return <div ref={ref}></div>;
};

/**
* This hook uses markdown-it directly to render markdown.
* The reason to use markdown-it directly is that the MarkdownRenderer is
* overriden by theia with a monaco version. This monaco version strips all html
* tags from the markdown with empty content.
* This leads to unexpected behavior when rendering markdown with html tags.
*
* @param markdown the string to render as markdown
* @returns the ref to use in an element to render the markdown
*/
export const useMarkdownRendering = (markdown: string) => {
// eslint-disable-next-line no-null/no-null
const ref = useRef<HTMLDivElement | null>(null);
useEffect(() => {
const markdownIt = markdownit();
const host = document.createElement('div');
const html = markdownIt.render(response.content);
const html = markdownIt.render(markdown);
host.innerHTML = DOMPurify.sanitize(html, {
ALLOW_UNKNOWN_PROTOCOLS: true // DOMPurify usually strips non http(s) links from hrefs
});
Expand All @@ -82,7 +84,7 @@ const MarkdownRender = ({ response }: { response: MarkdownChatResponseContent |
}

ref?.current?.appendChild(host);
}, [response.content]);
}, [markdown]);

return <div ref={ref}></div>;
return ref;
};
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,7 @@ import * as React from '@theia/core/shared/react';

import { ChatNodeToolbarActionContribution } from '../chat-node-toolbar-action-contribution';
import { ChatResponsePartRenderer } from '../chat-response-part-renderer';
import * as markdownit from '@theia/core/shared/markdown-it';
import * as DOMPurify from '@theia/core/shared/dompurify';
import { useEffect, useRef } from '@theia/core/shared/react';
import { useMarkdownRendering } from '../chat-response-renderer/markdown-part-renderer';

// TODO Instead of directly operating on the ChatRequestModel we could use an intermediate view model
export interface RequestNode extends TreeNode {
Expand Down Expand Up @@ -378,22 +376,8 @@ export class ChatViewTreeWidget extends TreeWidget {
}

const ChatRequestRender = ({ node }: { node: RequestNode }) => {
// eslint-disable-next-line no-null/no-null
const ref: React.MutableRefObject<HTMLDivElement | null> = useRef(null);
useEffect(() => {
const markdownIt = markdownit();
const text = node.request.request.displayText ?? node.request.request.text;
const host = document.createElement('div');
const html = markdownIt.render(text);
host.innerHTML = DOMPurify.sanitize(html, {
ALLOW_UNKNOWN_PROTOCOLS: true // DOMPurify usually strips non http(s) links from hrefs
});
while (ref?.current?.firstChild) {
ref.current.removeChild(ref.current.firstChild);
}

ref?.current?.appendChild(host);
}, [node.request]);
const text = node.request.request.displayText ?? node.request.request.text;
const ref = useMarkdownRendering(text);

return <div className={'theia-RequestNode'} ref={ref}></div>;
};
Expand Down
27 changes: 14 additions & 13 deletions packages/ai-chat/src/common/chat-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
// Partially copied from https://github.com/microsoft/vscode/blob/a2cab7255c0df424027be05d58e1b7b941f4ea60/src/vs/workbench/contrib/chat/common/chatModel.ts

import { Command, Emitter, Event, generateUuid, URI } from '@theia/core';
import { MarkdownString, MarkdownStringImpl } from '@theia/core/lib/common/markdown-rendering';
import { Position } from '@theia/core/shared/vscode-languageserver-protocol';
import { ChatAgentLocation } from './chat-agents';
import { ParsedChatRequest } from './parsed-chat-request';
Expand Down Expand Up @@ -127,7 +128,7 @@ export interface ErrorChatResponseContent extends ChatResponseContent {
export interface MarkdownChatResponseContent
extends Required<ChatResponseContent> {
kind: 'markdownContent';
content: string;
content: MarkdownString;
}

export interface CodeChatResponseContent
Expand Down Expand Up @@ -186,7 +187,7 @@ export interface CommandChatResponseContent extends ChatResponseContent {
*/
export interface InformationalChatResponseContent extends ChatResponseContent {
kind: 'informational';
content: string;
content: MarkdownString;
}

export namespace TextChatResponseContent {
Expand All @@ -206,7 +207,7 @@ export namespace MarkdownChatResponseContent {
ChatResponseContent.is(obj) &&
obj.kind === 'markdownContent' &&
'content' in obj &&
typeof (obj as { content: unknown }).content === 'string'
MarkdownString.is((obj as { content: unknown }).content)
);
}
}
Expand All @@ -217,7 +218,7 @@ export namespace InformationalChatResponseContent {
ChatResponseContent.is(obj) &&
obj.kind === 'informational' &&
'content' in obj &&
typeof (obj as { content: unknown }).content === 'string'
MarkdownString.is((obj as { content: unknown }).content)
);
}
}
Expand Down Expand Up @@ -410,35 +411,35 @@ export class TextChatResponseContentImpl implements TextChatResponseContent {

export class MarkdownChatResponseContentImpl implements MarkdownChatResponseContent {
readonly kind = 'markdownContent';
protected _content: string;
protected _content: MarkdownStringImpl = new MarkdownStringImpl();

constructor(content: string) {
this._content = content;
this._content.appendMarkdown(content);
}

get content(): string {
get content(): MarkdownString {
return this._content;
}

asString(): string {
return this._content;
return this._content.value;
}

merge(nextChatResponseContent: MarkdownChatResponseContent): boolean {
this._content += nextChatResponseContent.content;
this._content.appendMarkdown(nextChatResponseContent.content.value);
return true;
}
}

export class InformationalChatResponseContentImpl implements InformationalChatResponseContent {
readonly kind = 'informational';
protected _content: string;
protected _content: MarkdownStringImpl;

constructor(content: string) {
this._content = content;
this._content = new MarkdownStringImpl(content);
}

get content(): string {
get content(): MarkdownString {
return this._content;
}

Expand All @@ -447,7 +448,7 @@ export class InformationalChatResponseContentImpl implements InformationalChatRe
}

merge(nextChatResponseContent: InformationalChatResponseContent): boolean {
this._content += nextChatResponseContent.content;
this._content.appendMarkdown(nextChatResponseContent.content.value);
return true;
}
}
Expand Down

0 comments on commit 0a6f815

Please sign in to comment.