Skip to content

Comments

In-text equations containing dollar signs won't render#1

Merged
tatums merged 8 commits intomasterfrom
subscript-fix
Apr 22, 2020
Merged

In-text equations containing dollar signs won't render#1
tatums merged 8 commits intomasterfrom
subscript-fix

Conversation

@tatums
Copy link

@tatums tatums commented Apr 21, 2020

Problem

When using inline math delimiters $ and you have dollars as content our markdown-it-mathjax plugin doesn't work like it should.

I created an issue with the maintainer: classeur#10

Changes

Follow up

I'll spread this around to our other tooling.

@tatums
Copy link
Author

tatums commented Apr 21, 2020

@tatums tatums changed the title Subscript fix In-text equations containing dollar signs won't render Apr 21, 2020
})
it('can handle multiple dollors in formulas', function () {
md.render('if $a^{$2.00}$ then $b^{$3.00}$').should.eql('<p>if \\(a^{$2.00}\\) then \\(b^{$3.00}\\)</p>\n')
})

Choose a reason for hiding this comment

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

always nice to see !

Copy link
Member

Choose a reason for hiding this comment

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

@tatums you spelled "dollars" incorrectly.

Copy link
Member

Choose a reason for hiding this comment

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

misspelled "dollars"

Copy link
Author

Choose a reason for hiding this comment

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

Oooppps!

@georgediaz88
Copy link

LGTM! Thx for following up w/the maintainer too.

@raquelcps
Copy link

LGTM

for (var i = 0; i < state.src.length; i++) {
var marker = state.src[i]
var afterMarker = state.src[ i + 1 ]
if (marker === endMarker && !!/[^\d]/.test(afterMarker) && (i + 1) > startMathPos) {
Copy link
Member

Choose a reason for hiding this comment

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

I'd assume the !! is not necessary since test already returns true / false?

Although, honestly, who cares. Just thought I'd mention it.

Copy link
Author

Choose a reason for hiding this comment

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

Oh right of course! I thought I needed to coerce it but you're right. It works with null and even undefined. Thanks I'll update.

@coryschires
Copy link
Member

coryschires commented Apr 22, 2020

Love it!

Recommendation: Maybe update your issue to mention that you have a fork which fixes this issue. That may make them more likely to merge your changes. That said, probably doesn't matter. I agree, this project looks dead.

LGTM

@robertwalsh0
Copy link
Member

I left a couple notes on spelling otherwise lgtm

@tatums tatums merged commit 231b3f8 into master Apr 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants