Skip to content

Example app entries#205

Open
kostub wants to merge 3 commits into
feature/cfrac-iint-pr3from
feature/cfrac-iint-pr4
Open

Example app entries#205
kostub wants to merge 3 commits into
feature/cfrac-iint-pr3from
feature/cfrac-iint-pr4

Conversation

@kostub
Copy link
Copy Markdown
Owner

@kostub kostub commented May 26, 2026

Summary

Adds visual gallery entries for all 13 new commands (5 fraction macros + 8 multi-integrals) to MathExamples.h. This is PR 4 of 4 — the final PR in the AMSMath fractions + multi-integral stack.

Plan: docs/plans/2026-05-25-amsmath-fractions-and-multi-integrals.md
LLD: docs/lld/2026-05-25-amsmath-fractions-and-multi-integrals.md

Goal

Add visual gallery entries to MathDemoFormulas() and MathTestFormulas() in MathExamples.h so reviewers can eyeball the rendering of all new commands across the example apps. No iosMath source changes.

Commits

  • [item 19] Add \dfrac/\tfrac/\cfrac/\dbinom/\tbinom examples to gallery
  • [item 20] Add \iint/\iiint/\iiiint/\oiint/\oiiint/\fint/contour examples

Stack

Example strings added

Fraction macros:

  • \tfrac{1}{2} + \dfrac{1}{2} = \cfrac{1}{1+\cfrac{1}{1}}
  • x = \cfrac{1}{1+\cfrac{1}{x+\cfrac{1}{x+\cfrac{1}{x}}}}
  • \cfrac[l]{1}{1+x} \quad \cfrac[c]{1}{1+x} \quad \cfrac[r]{1}{1+x}
  • \dbinom{n}{k} \quad \tbinom{n}{k}

Multi-integrals:

  • \iint_S f \, dA = \iiint_V g \, dV = \iiiint_{\mathbb{R}^4} h \, dV
  • \oiint_{\partial V} \vec{F} \cdot d\vec{A} = \iiint_V (\nabla\cdot\vec{F}) \, dV
  • \varointclockwise \, \ointctrclockwise \, \fint

Test plan

  • 222 tests pass (verified by item 20 full-suite run)
  • Example apps render all new formula strings without crashing

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds several AMS fraction macros (such as \tfrac, \dfrac, \cfrac, \dbinom, \tbinom) and AMS multi-integrals (such as \iint, \iiint, \iiiint, \oiint, \varointclockwise, \ointctrclockwise, \fint) to both the MathDemoFormulas and MathTestFormulas arrays in MathExamples.h. There are no review comments to address, and I have no additional feedback to provide.

@kostub
Copy link
Copy Markdown
Owner Author

kostub commented May 26, 2026

Code Review — PR 205: Example app entries

Overall this is a clean, low-risk PR that correctly adds the new LaTeX gallery strings to both MathDemoFormulas() and MathTestFormulas(). The implementation matches the plan spec for items 19 and 20. A few things worth noting:


Issues

1. \oiiint has no gallery entry (missing command coverage)

The PR description and commit message both claim "8 multi-integrals", but only 7 are exercised in the gallery. \oiiint (U+2230, ∰) is registered in MTMathAtomFactory.m (line 818), has a parser test in MTMathListBuilderTest.m (line 969), yet neither multi-integral line in the gallery calls it. The third line only shows \varointclockwise, \ointctrclockwise, and \fint.

Suggested fix — replace the third gallery line with one that covers all three missing symbols at once:

@"\\oiiint \\, \\varointclockwise \\, \\ointctrclockwise \\, \\fint",

Or keep the existing third line and add a fourth:

@"\\oiiint_V h \\, dV",

Without this, a visual regression in \oiiint rendering would go undetected from the gallery.


2. Plan-specified inline comments were dropped (minor)

The plan (docs/plans/…, item 19 step 2) specified per-entry comments:

// dfrac vs tfrac vs cfrac size comparison
// Nested continued fraction
// cfrac alignment showcase
// dbinom vs tbinom

The implementation uses only a single section-header comment (// AMS fraction macros) and omits per-line annotations. The existing entries in both arrays consistently carry per-line // N: description comments. While the gallery items appended here are unnumbered (they extend beyond the numbered range), dropping the inline descriptions makes the intent of each line harder to read at a glance. Suggest adding at least brief inline comments matching the plan's labels.


Nits

3. MathDemoFormulas array numbering is now stale in comments

Items 0–30 are numbered in comments. The new entries at lines 111–118 have no numbers assigned. That is fine for unnumbered continuation, but the comment on line 21 of the file (/// Gallery demo formulae) doesn't mention the total count. Not a bug, but if the file convention were to keep all entries numbered, 6 lines are now anonymous. Low priority.

4. Duplication between MathDemoFormulas and MathTestFormulas

All 7 new strings are identical copies in both arrays. That is consistent with how existing entries that appear in both are handled (e.g., entries 6 and 7 are identical), so this is not a new pattern problem — but worth flagging that the two arrays now both carry the same visual-demo strings, which may not be the intent for MathTestFormulas (which per its doc comment is for "specific typesetter features and edge cases"). The fraction/integral strings are meaningful typesetter stress tests, so the duplication is defensible; just making sure it was intentional.


Confirmed correct

  • All 5 fraction macros (\tfrac, \dfrac, \cfrac, \cfrac[l/c/r], \dbinom, \tbinom) have gallery strings.
  • 7 of the 8 multi-integral operators are visually exercised.
  • Both MathDemoFormulas() and MathTestFormulas() were updated (plan requirement satisfied).
  • No iosMath source files were modified — scope is correctly limited.
  • The \clang diagnostic push/pop pragma block that suppresses -Wobjc-string-concatenation already wraps the entire file, so the adjacent-string-literal pattern used in other entries remains valid here too.
  • \nabla, \vec, \partial, \mathbb in the oiint divergence-theorem line are all pre-existing supported commands, so that formula should not crash.

@kostub kostub force-pushed the feature/cfrac-iint-pr3 branch from c32649f to 68c9a6f Compare May 26, 2026 17:06
@kostub kostub force-pushed the feature/cfrac-iint-pr4 branch from 3961abb to 727192a Compare May 26, 2026 17:06
@kostub kostub force-pushed the feature/cfrac-iint-pr3 branch from 68c9a6f to fb586ae Compare May 26, 2026 17:47
@kostub kostub force-pushed the feature/cfrac-iint-pr4 branch from 727192a to b992fd9 Compare May 26, 2026 17:47
@kostub kostub force-pushed the feature/cfrac-iint-pr3 branch from fb586ae to 1bea734 Compare May 26, 2026 18:15
kostub and others added 3 commits May 26, 2026 23:59
Follow-up to dropping \fint from supportedLatexSymbols (PR 204): the
example arrays in MathExamples.h referenced \fint, which would now
render as .notdef. Remove it from the multi-integral example line.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@kostub kostub force-pushed the feature/cfrac-iint-pr4 branch from b992fd9 to a00242a Compare May 26, 2026 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant