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 emphasis closing by single _ (part of left-flanking run) #1351

Merged
merged 1 commit into from
Oct 17, 2018

Conversation

RomanHotsiy
Copy link
Contributor

@RomanHotsiy RomanHotsiy commented Oct 8, 2018

Marked version:

0.5.1

Markdown flavor: CommonMark

Description

Fixes CommonMark: 456, 355, 451, 354, 356

Also, the concrete use case:

Some code: <code>*name*__b</code>. [link](/url/with_underscore)

Expectation

<p>Some code: <code><em>name</em>__b</code>. <a href="/url/with_underscore">link</a></p>

CommonMark playground link)

Result

<p>Some code: <code><em>name</em><em>_b</code>. [link](/url/with</em>underscore)</p>

Marked playground link)

Contributor

  • Test(s) exist to ensure functionality and minimize regression (if no tests added, list tests covering this PR); or,
  • no tests required for this PR.
  • If submitting new feature, it has been documented in the appropriate places.

Committer

In most cases, this should be a different person than the contributor.

  • Draft GitHub release notes have been updated.
  • CI is green (no forced merge required).
  • Merge PR

fixes CommonMark: 456, 355, 451, 354, 356
@RomanHotsiy RomanHotsiy changed the title fix emphasis closing by single _ part of right-flanking run fix emphasis closing by single _ (part of left-flanking run) Oct 8, 2018
@styfle
Copy link
Member

styfle commented Oct 8, 2018

Hi @RomanGotsiy thanks for the PR!

It looks like the links to the demo have a slightly different input than what you put in this PR.

Some code: <code>*name*__b</code>. [link](/url/with_underscore)
Some code: <code>name__b</code>. [link](/url/with_underscore)

That being said, the fix the the link appears to work so I added a couple more reviewers.

@RomanHotsiy
Copy link
Contributor Author

@styfle yes, updated the comment

@styfle
Copy link
Member

styfle commented Oct 17, 2018

Thanks Roman! 🎉

@styfle styfle merged commit fb48827 into markedjs:master Oct 17, 2018
@RomanHotsiy
Copy link
Contributor Author

Thanks for merging!
Do you have any ETA for a new release?

@styfle
Copy link
Member

styfle commented Oct 17, 2018

No not yet. We could probably do a patch release soon. Keep an eye out for a release PR...it will probably be from @UziTech

@styfle styfle mentioned this pull request Nov 19, 2018
12 tasks
@kaelig
Copy link

kaelig commented Nov 20, 2018

This seems to have caused a regression.

Expected: https://spec.commonmark.org/dingus/?text=_foo_%2C%20_bar_
Actual: https://marked.js.org/demo/?text=_foo_%2C%20_bar_

Should I open an issue?

@styfle
Copy link
Member

styfle commented Nov 21, 2018

Yes please!

And if you could determine which CommonMark test this is violating that would be helpful 😃

This PR added many more passing CM spec tests so there must be one in there we were ignoring and now regressed.

@styfle
Copy link
Member

styfle commented Nov 23, 2018

@kaelig Looks like there is a ticket here: #1378

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.

4 participants