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

Em less than 3 chars #1181

Merged
merged 3 commits into from Apr 5, 2018
Merged

Em less than 3 chars #1181

merged 3 commits into from Apr 5, 2018

Conversation

UziTech
Copy link
Member

@UziTech UziTech commented Mar 28, 2018

Marked version: 8489411

Description

Contributor

  • Test(s) exist to ensure functionality and minimize regression (if no tests added, list tests covering this PR)

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

@UziTech UziTech requested a review from davisjam March 28, 2018 05:36
@UziTech
Copy link
Member Author

UziTech commented Mar 28, 2018

/cc @Feder1co5oave and @davisjam to make sure I didn't introduce any security issues

@Feder1co5oave
Copy link
Contributor

Lgtm

@UziTech
Copy link
Member Author

UziTech commented Mar 28, 2018

I added some edge cases like:

**1* => <p><em>*1</em></p>

This required adding more capturing parentheses so we could run into another problem like #1013 (comment)

@UziTech
Copy link
Member Author

UziTech commented Apr 5, 2018

@joshbruce do we want to risk a breaking change like #1059 or wait for this fix til 0.4.x?

@joshbruce
Copy link
Member

@UziTech: Think this is like #1203 and should come along with a 0.4.x release. We've done a lot of work around DevOps and even adding a couple of features. Given the number of people apparently losing Marked, we haven't received that much negative feedback re semver all things considered.

Copy link
Contributor

@davisjam davisjam left a comment

Choose a reason for hiding this comment

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

The changed regexes are safe from REDOS

@Feder1co5oave
Copy link
Contributor

Did you disable CI?

@styfle
Copy link
Member

styfle commented Apr 5, 2018

CI is running for other PRs, but I don't see this one: https://travis-ci.org/markedjs/marked/pull_requests

@UziTech
Copy link
Member Author

UziTech commented Apr 5, 2018

That's weird, I swear it was there initially. I just rebased and it looks like it came back.

@styfle styfle added this to the 0.4.0 - No known defects milestone Apr 5, 2018
@styfle styfle merged commit be20b5f into markedjs:master Apr 5, 2018
@UziTech UziTech mentioned this pull request Apr 5, 2018
6 tasks
@UziTech UziTech deleted the em-less-than-2-chars branch April 10, 2018 15:07
@re-fort
Copy link

re-fort commented Apr 15, 2018

I'm looking forward to the next release included this fix:laughing:

@styfle
Copy link
Member

styfle commented Apr 15, 2018

@re-fort Definitely! We are waiting for a couple more fixes to land before we release version 0.4.0.

zhenalexfan pushed a commit to zhenalexfan/MarkdownHan that referenced this pull request Nov 8, 2021
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.

Specific character combination not rendered correctly
6 participants