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 paragraph type #1248

Merged
merged 4 commits into from
May 2, 2018
Merged

fix paragraph type #1248

merged 4 commits into from
May 2, 2018

Conversation

UziTech
Copy link
Member

@UziTech UziTech commented May 2, 2018

Marked version: 37c0324

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
Copy link
Member Author

UziTech commented May 2, 2018

@davisjam not sure if this makes the paragraph regex vulnerable. It doesn't looks like the eslint plugin marked it as vulnerable.

@UziTech UziTech requested review from styfle, joshbruce and davisjam May 2, 2018 13:54
@UziTech UziTech changed the title [wip] fix paragraph type fix paragraph type May 2, 2018

const tokens = marked.lexer(md);

expect(tokens[0].type).toBe('paragraph');
Copy link
Member

Choose a reason for hiding this comment

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

Think this makes more sense to be in the unit tests, yeah? The construction seems similar enough and this isn't, technically speaking, checking a specific spec or the output as a whole.

@davisjam
Copy link
Contributor

davisjam commented May 2, 2018

@UziTech Looks OK.

marked.parse('Hello World!');
expect(marked.parse).toHaveBeenCalled();
});

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed this test as well since it didn't really do anything. and was just there initially to have a test

@joshbruce
Copy link
Member

joshbruce commented May 2, 2018

@UziTech - looks like - is this a false positive in security check? The regexes don't seem to match what was changed.

@UziTech
Copy link
Member Author

UziTech commented May 2, 2018

travis is just hung up on the security scan. this PR is good to go

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

4 participants