Skip to content

Fix crash on lone \sqrt at end of input (#188)#207

Merged
kostub merged 3 commits into
masterfrom
fix/issue-188-sqrt-crash
May 29, 2026
Merged

Fix crash on lone \sqrt at end of input (#188)#207
kostub merged 3 commits into
masterfrom
fix/issue-188-sqrt-crash

Conversation

@kostub
Copy link
Copy Markdown
Owner

@kostub kostub commented May 29, 2026

Summary

Fixes #188. A lone \sqrt at the end of the input crashed the parser with an NSInternalInconsistencyException.

The sqrt branch in MTMathListBuilder called getNextCharacter unconditionally to peek for an optional [degree] argument. When \sqrt was the last token, that call tripped the NSAssert([self hasCharacters]) in getNextCharacter and crashed.

Fix

Guard the lookahead with [self hasCharacters]. When no argument follows, build an empty radicand — the same result \sqrt{} already produces. This matches the fix proposed by the issue author.

Testing

  • Added testSqrtAtEnd: asserts \sqrt parses to a radical with an empty radicand and round-trips to \sqrt{}. Confirmed it crashed before the fix and passes after.
  • Full suite green: 264 tests, 0 failures.

🤖 Generated with Claude Code

The sqrt branch in MTMathListBuilder read the next character
unconditionally. When \sqrt was the last token in the input, this hit
the NSAssert in getNextCharacter and crashed with an
NSInternalInconsistencyException.

Guard the lookahead with hasCharacters; when no argument follows, build
an empty radicand, matching the existing behavior of \sqrt{}.

Adds testSqrtAtEnd covering the regression.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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 fixes a crash that occurs when a lone \sqrt command is parsed at the end of the input by ensuring characters are available before reading. A unit test was also added to prevent regression. The reviewer suggested simplifying the nested conditional logic in MTMathListBuilder.m by pulling the common rad.radicand assignment out of the conditional branches to reduce code duplication.

Comment on lines +749 to 763
if ([self hasCharacters]) {
unichar ch = [self getNextCharacter];
if (ch == '[') {
// special handling for sqrt[degree]{radicand}
rad.degree = [self buildInternal:false stopChar:']'];
rad.radicand = [self buildInternal:true];
} else {
[self unlookCharacter];
rad.radicand = [self buildInternal:true];
}
} else {
[self unlookCharacter];
// No argument follows (e.g. a lone "\sqrt" at the end of input).
// Build an empty radicand rather than reading past the end.
rad.radicand = [self buildInternal:true];
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The nested conditional logic can be simplified by pulling the common rad.radicand = [self buildInternal:true]; call out of the conditional branches. This eliminates redundant code duplication and improves readability.

        if ([self hasCharacters]) {
            unichar ch = [self getNextCharacter];
            if (ch == '[') {
                // special handling for sqrt[degree]{radicand}
                rad.degree = [self buildInternal:false stopChar:']'];
            } else {
                [self unlookCharacter];
            }
        }
        rad.radicand = [self buildInternal:true];

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed. Pulled the common rad.radicand = [self buildInternal:true] out of all three branches into a single call after the optional degree parse. Kept a brief comment noting the lone-\sqrt guard. All 119 tests pass.

kostub and others added 2 commits May 29, 2026 23:07
Covers a lone \sqrt inside a group ({\sqrt}), which reaches an empty
radicand via the oneCharOnly stop-char guard — a different path than the
end-of-input case. Suggested in code review.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Pull the common radicand build out of all three branches into a
single call after the optional degree parse, per review on #207.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@kostub kostub merged commit da9abdd into master May 29, 2026
1 check passed
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.

Failure on rendering \sqrt{} command

1 participant