-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Render line breaks in our markdown block #7857
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution! I've reviewed the changes and have some suggestions for improvement. The implementation looks clean and follows the existing pattern for adding remark plugins.
| import rehypeKatex from "rehype-katex" | ||
| import remarkMath from "remark-math" | ||
| import remarkGfm from "remark-gfm" | ||
| import remarkBreaks from "remark-breaks" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a brief comment explaining why this plugin was added, for future maintainers:
| import remarkBreaks from "remark-breaks" | |
| import remarkBreaks from "remark-breaks" // Converts line breaks in markdown to <br> tags for better rendering in thinking output |
| remarkPlugins={[ | ||
| remarkGfm, | ||
| remarkMath, | ||
| remarkBreaks, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a test case for line break rendering in the existing test file at webview-ui/src/components/common/__tests__/MarkdownBlock.spec.tsx. This would help ensure the remark-breaks plugin works as expected and prevent regressions.
For example:
it('should render line breaks correctly with remark-breaks', async () => {
const markdown = 'First line\nSecond line\nThird line';
const { container } = render(<MarkdownBlock markdown={markdown} />);
// Wait for content to be processed
await screen.findByText(/First line/, { exact: false });
// Check that line breaks are rendered as <br> tags
const brElements = container.querySelectorAll('br');
expect(brElements.length).toBe(2);
});Also, I noticed that the p tag styling already has white-space: pre-wrap (line 119), which preserves line breaks. Could this potentially conflict with or duplicate the behavior of remark-breaks? It might be worth verifying that these two approaches work well together.
Saw this show up in the new thinking output
Important
Add
remark-breaksplugin to render line breaks in markdown content inMarkdownBlock.tsx.remark-breakstoremarkPluginsinMarkdownBlock.tsxto render line breaks in markdown content.remark-breakstodependenciesinpackage.json.This description was created by
for 4941b92. You can customize this summary. It will automatically update as commits are pushed.