Matrix: render LaTeX for Element Desktop by emitting data-mx-maths in formatted_body#20286
Open
eloklam wants to merge 3 commits intoopenclaw:mainfrom
Open
Matrix: render LaTeX for Element Desktop by emitting data-mx-maths in formatted_body#20286eloklam wants to merge 3 commits intoopenclaw:mainfrom
eloklam wants to merge 3 commits intoopenclaw:mainfrom
Conversation
Comment on lines
+40
to
+57
| function extractMath(markdown: string): { text: string; blocks: string[]; inlines: string[] } { | ||
| const blocks: string[] = []; | ||
| const inlines: string[] = []; | ||
| let text = markdown; | ||
|
|
||
| text = text.replace(/\$\$([\s\S]+?)\$\$/g, (_m, latex) => { | ||
| const id = blocks.length; | ||
| blocks.push(String(latex)); | ||
| return `OC_MATH_BLOCK_${id}`; | ||
| }); | ||
|
|
||
| text = text.replace(/\$([^\n$]+?)\$/g, (_m, latex) => { | ||
| const id = inlines.length; | ||
| inlines.push(String(latex)); | ||
| return `OC_MATH_INLINE_${id}`; | ||
| }); | ||
|
|
||
| return { text, blocks, inlines }; |
Contributor
There was a problem hiding this comment.
No tests for the new math extraction/injection logic
The existing format.test.ts covers markdownToMatrixHtml but there are no tests for extractMath, injectMath, or the updated applyMatrixFormatting. Given the regex edge cases involved (currency false positives, escaped dollars, mixed inline/block), this logic needs test coverage. Consider adding tests for at least:
- Basic inline math:
$x^2$ - Basic block math:
$$E = mc^2$$ - Currency text not treated as math:
costs $10 and $20 - Mixed content:
The equation $a+b$ and the formula $$c+d$$ - No math (passthrough):
Hello world
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/matrix/src/matrix/send/formatting.ts
Line: 40:57
Comment:
**No tests for the new math extraction/injection logic**
The existing `format.test.ts` covers `markdownToMatrixHtml` but there are no tests for `extractMath`, `injectMath`, or the updated `applyMatrixFormatting`. Given the regex edge cases involved (currency false positives, escaped dollars, mixed inline/block), this logic needs test coverage. Consider adding tests for at least:
- Basic inline math: `$x^2$`
- Basic block math: `$$E = mc^2$$`
- Currency text not treated as math: `costs $10 and $20`
- Mixed content: `The equation $a+b$ and the formula $$c+d$$`
- No math (passthrough): `Hello world`
How can I resolve this? If you propose a fix, please make it concise.Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
|
This pull request has been automatically marked as stale due to inactivity. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.........,.........) were sent as plain Markdown HTML (<p>.........</p>), which Element does not render as math.extensions/matrix/src/matrix/send/formatting.tsto detect inline/block LaTeX and emit Element-compatibledata-mx-mathsmarkup informatted_body.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
User-visible / Behavior Changes
$$x^2$$→<span data-mx-maths="x^2"><code>x^2</code></span>E=mc2E = mc^2E=mc2→<div data-mx-maths="E = mc^2"><code>E = mc^2</code></div>Security Impact (required)
No)No)No)No)No)Repro + Verification
Environment
org.matrix.custom.htmlmessage formatting pathSteps
E=mc2E = mc^2E=mc2.content.formatted_body.$$x^2$$embedded in normal text.Expected
formatted_bodycontainsdata-mx-mathsmarkup for detected math.$delimiters.Actual
formatted_bodycontained<p>E=mc2E = mc^2E=mc2</p>(raw text shown in Element).formatted_bodycontainsdata-mx-mathsHTML and renders correctly.Evidence
Attach at least one:
Human Verification (required)
What you personally verified (not just CI), and how:
Verified scenarios:
Display math-only message renders in Element.
Inline math in mixed text renders.
Normal non-math markdown remains unchanged.
Edge cases checked:
Multiple math expressions in one message.
Multiline display-math capture with
..........What you did not verify:
Full escaped-dollar edge cases (e.g.
\$) across all clients.Behavior in non-Element Matrix clients.
Compatibility / Migration
Yes)No)No)Failure Recovery (if this breaks)
How to disable/revert this change quickly:
Revert commit affecting
extensions/matrix/src/matrix/send/formatting.ts.Files/config to restore:
extensions/matrix/src/matrix/send/formatting.tsKnown bad symptoms reviewers should watch for:
Raw placeholders like
OC_MATH_BLOCK_*appearing in chat.Missing or malformed
formatted_body.Regressions in non-math markdown formatting.
Risks and Mitigations
List only real risks for this PR. Add/remove entries as needed. If none, write
None.None
Greptile Summary
This PR adds LaTeX math rendering support for Matrix messages in Element by detecting
$...$(inline) and$$...$$(block) delimiters and converting them to Element-compatibledata-mx-mathsHTML markup. The implementation uses a placeholder-based approach: math expressions are extracted before markdown rendering, then re-injected as proper HTML elements afterward.\$([^\n$]+?)\$will match text like$10 and $20as a single math expression, corrupting non-math messages. This needs word-boundary guards (e.g.(?<!\w)\$...\$(?!\w)) to avoid treating dollar amounts as LaTeX delimiters.extractMath/injectMathlogic — the existing test file only coversmarkdownToMatrixHtml. Given the regex edge cases, tests are important for confidence in this change.escapeHtmlusage correctly prevents injection in thedata-mx-mathsattribute.Confidence Score: 2/5
extensions/matrix/src/matrix/send/formatting.ts— the inline math regex at line 51 needs word-boundary guards to prevent false positives on currency textLast reviewed commit: 3e845bd
(2/5) Greptile learns from your feedback when you react with thumbs up/down!