-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add '$$' at the beginning and end of empty latex equations to have them render correctly #8213
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
Conversation
equations to have them render correctly.
Codecov Report
@@ Coverage Diff @@
## master #8213 +/- ##
==========================================
- Coverage 58.79% 58.73% -0.07%
==========================================
Files 504 505 +1
Lines 23090 23109 +19
Branches 3723 3731 +8
==========================================
- Hits 13576 13572 -4
- Misses 8645 8656 +11
- Partials 869 881 +12
Continue to review full report at Codecov.
|
return input; | ||
} | ||
|
||
private getAllIndexesOf(arr: string, value: string): number[] { |
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.
getAllIndexesOf [](start = 12, length = 15)
I think you could likely do this with regexs?
const beginReg = /\begin/g;
const endReg = /\end/g;
while (result = beginReg.exec(input)) {
result.index is index of match
beginReg.lastIndex is next match
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.
Did you take a look at how Jupyter does this? The equation matching looks more complex to me.
https://github.com/jupyter/notebook/blob/ff29ba8ab006e59bc264e0ea33084755533535c8/docs/source/examples/Notebook/Typesetting%20Equations.ipynb
In reply to: 339142495 [](ancestors = 339142495)
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.
Here is what you want.
https://github.com/jupyter/notebook/blob/d60445e92666a0c5651a33e180c92051d94a7d44/notebook/static/notebook/js/mathjaxutils.js
Looks like we might need to check licensing as well.
In reply to: 339245020 [](ancestors = 339245020,339142495)
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.
🕐
Started using Regular Expressions instead of indexOf Added unit tests
const beginIndexes = getAllIndexesOfRegex(input, /\\begin/g); | ||
const endIndexes = getAllIndexesOfRegex(input, /\\end/g); | ||
|
||
for (let i = 0; i < beginIndexes.length; i += 1) { |
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.
I think this would have issues with mismatched begin / end pairs. The code is assuming that an equal number of \begin and \end matches were found, but you could have valid markdown with just those strings in it.
I think it's worth taking a look at this regex that I linked to see if it breaks out latex bits from a markdown block.
it now includes the opening anc closing brackets, with whatever name the user chooses. Also, the code will run only if the amount of indexes are the same.
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT License. | ||
// Adds '$$' to latex formulas that don't have a '$', allowing users to input the formula directly. | ||
function fixLatexEquations(input: string): string { |
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.
function fixLatexEquations(input: string): string { | |
export function fixLatexEquations(input: string): string { |
return indexes; | ||
} | ||
|
||
export { fixLatexEquations }; |
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.
This is not required in typescript.
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.
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.
For #7992
package-lock.json
has been regenerated by runningnpm install
(if dependencies have changed)