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.

Sign up for GitHub

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

Jump to bottom

fix: Fix indented markdown in html #2052

Merged
merged 5 commits into from
May 20, 2021
Merged

fix: Fix indented markdown in html #2052

merged 5 commits into from
May 20, 2021

Conversation

krassowski
Copy link
Contributor

@krassowski krassowski commented May 14, 2021 •
edited
Loading

Marked version: 1.2.8-2.0.3

Markdown flavor: n/a

Description

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.

@vercel
Copy link

vercel bot commented May 14, 2021 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/markedjs/markedjs/BUgTvEdPFRDvwkiPPmi1MYWZccbi
✅ Preview: https://markedjs-git-fork-krassowski-fix-indented-markdown-953ce6.vercel.app

@krassowski krassowski force-pushed the fix-indented-markdown-in-html branch from 000a6f3 to 91d4185 Compare May 14, 2021 22:09
@krassowski krassowski mentioned this pull request May 15, 2021
Closed
@UziTech
Copy link
Member

UziTech commented May 17, 2021

I don't think that actually fixes it

according to this demo the test you added should pass without changing anything in marked.

@krassowski
Copy link
Contributor Author

I don't think that actually fixes it
according to this demo the test you added should pass without changing anything in marked.

It does. You copied my test without space and that's because the demo works. Probably because spaces are not rendered in the diff in GitHub interface.

it-does

UziTech
UziTech requested changes May 17, 2021
View reviewed changes
Copy link
Member

@UziTech UziTech left a comment

Choose a reason for hiding this comment

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

This looks like it could work.

src/rules.js Outdated Show resolved Hide resolved
test/specs/new/indented_details.md Show resolved Hide resolved
test/specs/new/indented_details.html Show resolved Hide resolved
@krassowski @UziTech
Apply suggestions from code review
18d6ac2 
Co-authored-by: Tony Brix <tony@brix.ninja>
UziTech
UziTech approved these changes May 17, 2021
View reviewed changes
Copy link
Member

@UziTech UziTech left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! 💯

calculuschild
calculuschild requested changes May 17, 2021
View reviewed changes
src/rules.js Outdated Show resolved Hide resolved
styfle
styfle approved these changes May 19, 2021
View reviewed changes
Copy link
Member

@styfle styfle left a comment

Choose a reason for hiding this comment

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

Thanks!

@UziTech UziTech changed the title Fix indented markdown in html fix: Fix indented markdown in html May 20, 2021
@UziTech UziTech merged commit 6435ac9 into markedjs:master May 20, 2021
github-actions bot pushed a commit that referenced this pull request May 20, 2021
@semantic-release-bot
chore(release): 2.0.4 [skip ci]
29a301b 
## [2.0.4](v2.0.3...v2.0.4) (2021-05-20)

### Bug Fixes

* Fix indented markdown in html ([#2052](#2052)) ([6435ac9](6435ac9))
@github-actions
Copy link

🎉 This PR is included in version 2.0.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

@krassowski krassowski mentioned this pull request May 20, 2021
Open
davisjam
davisjam reviewed May 21, 2021
View reviewed changes
src/rules.js
+ '|</(?!script|pre|style)[a-z][\\w-]*\\s*>(?=[ \\t]*(?:\\n|$))[\\s\\S]*?(?:\\n{2,}|$)' // (7) closing tag
+ '|</?(tag)(?: +|\\n|/?>)[\\s\\S]*?(?:(?:\\n *)+\\n|$)' // (6)
+ '|<(?!script|pre|style)([a-z][\\w-]*)(?:attribute)*? */?>(?=[ \\t]*(?:\\n|$))[\\s\\S]*?(?:(?:\\n *)+\\n|$)' // (7) open tag
+ '|</(?!script|pre|style)[a-z][\\w-]*\\s*>(?=[ \\t]*(?:\\n|$))[\\s\\S]*?(?:(?:\\n *)+\\n|$)' // (7) closing tag
Copy link
Contributor

Choose a reason for hiding this comment

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

No REDOS issues in this change.

@chlafreniere chlafreniere mentioned this pull request Sep 3, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davisjam davisjam davisjam left review comments

@UziTech UziTech UziTech approved these changes

@calculuschild calculuschild calculuschild requested changes

@styfle styfle styfle approved these changes

@joshbruce joshbruce Awaiting requested review from joshbruce

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

HTML inside markdown will cause it fail to parse.
5 participants
@krassowski @UziTech @styfle @calculuschild @davisjam