Skip to content
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

Fix crash if non-ASCII character precedes $ math #239

Merged
merged 2 commits into from
Sep 29, 2019

Conversation

TotalVerb
Copy link
Contributor

Previously, if a sequence like “$x$” occurs (where the quotes are fancy quotes), then there would be a StringIndexError. This fixes the problem by calling prevind instead of subtracting 1.

Since prevind with the default n=1 can't return a negative number, I've also removed the else branch here since it's only setting 0 to 0.

@TotalVerb
Copy link
Contributor Author

The second commit fixes a related issue, but during the prerendering KaTeX step. I split up the prerendering test so that I could test it locally, since I haven't been able to figure out node/highlight.js yet.

@TotalVerb
Copy link
Contributor Author

TotalVerb commented Sep 29, 2019

The Travis 1.0 failure is real, but preexisting (as redirecting stderr/stdout to a IOBuffer is a 1.1 feature). I guess Travis wasn't testing this codepath because it's missing highlight.js; this change made the failure visible by removing that check for this branch.

Maybe upgrading the minimum Julia version to 1.1 is reasonable?

@tlienart tlienart changed the base branch from master to wip3 September 29, 2019 16:37
@tlienart tlienart merged commit dd4d5a0 into tlienart:wip3 Sep 29, 2019
@tlienart tlienart mentioned this pull request Sep 29, 2019
tlienart added a commit that referenced this pull request Sep 29, 2019
* Fix crash if non-ASCII character precedes $ math (#239)

* Fix crash if non-ASCII character precedes $ math

* Fix similar crash in prerendering step

* adding a nextind and restricting to 1.1

* fixing travis
@tlienart tlienart mentioned this pull request Sep 29, 2019
67 tasks
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.

2 participants