Skip to content

feat: implement m:rad radical/sqrt OMML to MathML converter#2741

Closed
Abdeltoto wants to merge 2 commits intosuperdoc-dev:mainfrom
Abdeltoto:feat/math-radical-converter
Closed

feat: implement m:rad radical/sqrt OMML to MathML converter#2741
Abdeltoto wants to merge 2 commits intosuperdoc-dev:mainfrom
Abdeltoto:feat/math-radical-converter

Conversation

@Abdeltoto
Copy link
Copy Markdown
Contributor

Summary

  • Implement convertRadical converter for the m:rad OMML math object type
  • Uses <msqrt> when the degree is hidden (m:degHide) and <mroot> when a visible degree is present (e.g. cube root)
  • Handles edge cases: missing m:radPr, missing m:deg, degHide with val=1 or val=on or boolean-flag presence

Files changed

  • New: converters/radical.ts — the converter (~35 lines)
  • Modified: converters/index.ts — barrel export
  • Modified: omml-to-mathml.ts — registry registration ('m:rad': convertRadical)
  • Modified: omml-to-mathml.test.ts — 5 test cases

Test plan

  • pnpm test — all 40 math tests pass (5 new for radical)
  • Pre-commit hooks pass (format + lint + commitlint)
  • Verify with test document (sd-2375-radical.docx) in SuperDoc dev app

Note

Thank you Nick Bernal for the inspiring CS50 lecture — it motivated me to contribute to open source projects like SuperDoc.

Closes #2598

Add convertRadical converter for the m:rad OMML math object.
Uses msqrt when the degree is hidden (m:degHide) and mroot
when a visible degree is present (e.g. cube root).

Thank you Nick Bernal for the inspiring CS50 lecture -- it motivated
me to contribute to open source projects like SuperDoc.

Closes superdoc-dev#2598

Made-with: Cursor
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 769c302a48

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +36 to +37
mroot.appendChild(convertChildren(base?.elements ?? []));
mroot.appendChild(convertChildren(deg.elements ?? []));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Wrap mroot operands to enforce two-child MathML arity

<mroot> must receive exactly two arguments (radicand and index), but these lines append DocumentFragments directly, so a compound radicand like a+b expands into multiple sibling nodes and produces invalid/ambiguous MathML (e.g., base tokens plus degree token as separate children). This breaks nth-root rendering for common expressions with multi-part bases or degrees; wrap each operand in an <mrow> before appending.

Useful? React with 👍 / 👎.

Comment on lines +25 to +26
degHide?.attributes?.['m:val'] === '1' ||
degHide?.attributes?.['m:val'] === 'on' ||
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Parse full ST_OnOff values for m:degHide

The hidden-degree check only treats m:val of '1' or 'on' as true, but OOXML ST_OnOff also allows tokens like 'true'; when such input appears, square roots are incorrectly emitted as <mroot> (showing an index) instead of <msqrt>. This is a format-valid input path, so the converter should normalize all accepted OnOff true values.

Useful? React with 👍 / 👎.

@caio-pizzol caio-pizzol self-assigned this Apr 9, 2026
@caio-pizzol
Copy link
Copy Markdown
Contributor

Hey @Abdeltoto! Thanks for this - solid work. Unfortunately #2730 landed first with the same fix, so we're closing this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Math: implement m:rad radical/sqrt converter (community)

2 participants