Skip to content

fix emphasis closing by single _ (part of left-flanking run) - #1351

Merged
styfle merged 1 commit into
markedjs:masterfrom
RomanHotsiy:master
Oct 17, 2018
Merged

fix emphasis closing by single _ (part of left-flanking run)#1351
styfle merged 1 commit into
markedjs:masterfrom
RomanHotsiy:master

Conversation

@RomanHotsiy

@RomanHotsiyRomanHotsiy commented Oct 8, 2018

Copy link
Copy Markdown
Contributor

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>. <ahref="/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
@RomanHotsiyRomanHotsiy changed the title fix emphasis closing by single _ part of right-flanking runfix emphasis closing by single _ (part of left-flanking run)Oct 8, 2018
@styfle
styfle requested review from UziTech and davisjamOctober 8, 2018 12:59
@styfle

Copy link
Copy Markdown
Member

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
Copy Markdown
ContributorAuthor

@styfle yes, updated the comment

@styfle

Copy link
Copy Markdown
Member

Thanks Roman! 🎉

@styfle
styfle merged commit fb48827 into markedjs:masterOct 17, 2018
@RomanHotsiy

Copy link
Copy Markdown
ContributorAuthor

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

@styfle

Copy link
Copy Markdown
Member

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

@styflestyfle mentioned this pull request Nov 19, 2018
12 tasks
@kaelig

Copy link
Copy Markdown

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
Copy Markdown
Member

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
Copy Markdown
Member

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

Sign up for freeto 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

@RomanHotsiy@styfle@kaelig@UziTech