-
-
Notifications
You must be signed in to change notification settings - Fork 23
Fix equation rendering #137
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
base: main
Are you sure you want to change the base?
Fix equation rendering #137
Conversation
Reviewer's GuideAdds support for parsing and rendering mathematical equations by introducing new Markdown node types for inline and block LaTeX formulas and updating the XML parser to detect and handle tags appropriately. Sequence Diagram: Processing of Tags by XmlParsersequenceDiagram
participant Caller
participant P as XmlParser
participant E_item as "item: Element"
participant ME_Block as MdBlockEquation
participant ME_Inline as MdInlineEquation
Caller->>P: paras(p, italic)
activate P
loop For each item in p elements
P->>E_item: Access item.tag
alt If item.tag is "formula"
P->>E_item: Access item.text (to get equation)
P->>P: Determine if block or inline (based on p and item.tail)
alt If block equation
P->>ME_Block: __init__(equation)
activate ME_Block
ME_Block-->>P: mdBlockEquation
deactivate ME_Block
P->>P: Add mdBlockEquation to result list
else Else (inline equation)
P->>ME_Inline: __init__(equation)
activate ME_Inline
ME_Inline-->>P: mdInlineEquation
deactivate ME_Inline
P->>P: Add mdInlineEquation to result list
end
end
end
P-->>Caller: List of Md objects
deactivate P
Class Diagram: New Markdown Equation Types and Parser IntegrationclassDiagram
class Md {
<<Abstract>>
+render(f: MdRenderer, indent: str) void
}
class MdInlineEquation {
-equation: str
+__init__(equation: str)
+render(f: MdRenderer, indent: str) void
}
class MdBlockEquation {
-equation: str
+__init__(equation: str)
+render(f: MdRenderer, indent: str) void
}
class XmlParser {
+paras(p: Element, italic: bool) list~Md~
}
class MdRenderer {
+write(text: str) void
}
class Element {
+tag: str
+text: str
+tail: str
}
Md <|-- MdInlineEquation
Md <|-- MdBlockEquation
XmlParser ..> MdInlineEquation : "creates"
XmlParser ..> MdBlockEquation : "creates"
XmlParser ..> Element : "uses"
MdInlineEquation ..> MdRenderer : "uses"
MdBlockEquation ..> MdRenderer : "uses"
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
714935a
to
5dcbf02
Compare
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.
Hey @EmilyBourne - I've reviewed your changes - here's some feedback:
- The block vs inline equation detection using
len(p) == 1 && item.tail is None
seems fragile—consider using an explicit context flag or attribute instead of relying on that heuristic. - The equation trimming logic
item.text.strip("$ ")
may remove valid dollar signs inside your equation; use a more precise regex or logic to strip only the outer delimiters. - In
MdBlockEquation.render
, theindent
parameter isn’t used and blank‐line handling may be inconsistent—ensure block equations respect indentation and proper Markdown spacing.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Fix equation rendering by saving strings from a
<formula>
tag into aMdInlineEquation
orMdBlockEquation
. Fixes #133Summary by Sourcery
Fix equation rendering by introducing inline and block math nodes and mapping
<formula>
tags to them in the XML parser.Bug Fixes:
<formula>
tags into math nodes.Enhancements: