Skip to content

fix: spaces on a newline after a table#2319

Merged
UziTech merged 10 commits intomarkedjs:masterfrom
cyanxiao:master
Dec 19, 2021
Merged

fix: spaces on a newline after a table#2319
UziTech merged 10 commits intomarkedjs:masterfrom
cyanxiao:master

Conversation

@cyanxiao
Copy link
Copy Markdown
Contributor

@cyanxiao cyanxiao commented Dec 11, 2021

Marked version: 4.0.7

Markdown flavor: Markdown.pl|CommonMark|GitHub Flavored Markdown|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
Copy Markdown

vercel bot commented Dec 11, 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/8R4yxox13gcfwaQktGJmJxcEEPoW
✅ Preview: https://markedjs-git-fork-imchell-master-markedjs.vercel.app

@UziTech
Copy link
Copy Markdown
Member

UziTech commented Dec 15, 2021

does this fix #2278 as well?

@cyanxiao
Copy link
Copy Markdown
Contributor Author

cyanxiao commented Dec 16, 2021 •
edited
Loading

No, it is for #2249 solely. #2278 is getting TypeError exception for a similar reason, but the parsing result is not the same as commonmark.js even the exception is fixed. I will look into that.

@cyanxiao
Copy link
Copy Markdown
Contributor Author

cyanxiao commented Dec 17, 2021

There are no exceptions both for #2249 and #2278 now, but the parsing result of #2278 may differ from commonmark.js because of the ambiguity between gfm table and setext heading ( #1499 ).

@UziTech
Copy link
Copy Markdown
Member

UziTech commented Dec 17, 2021 •
edited
Loading

after a little bit of experimenting it looks like a better fix would be to change the line:

-        rows: cap[3] ? cap[3].replace(/\n$/, '').split('\n') : []
+        rows: cap[3] ? cap[3].replace(/\n[ \t]*$/, '').split('\n') : []

basically removing any trailing space characters before creating rows.

This would allow removing more than 4 spaces without affecting fenced code blocks inside the table.

@cyanxiao
Copy link
Copy Markdown
Contributor Author

cyanxiao commented Dec 17, 2021

Thanks. It's a better solution.

@UziTech
Copy link
Copy Markdown
Member

UziTech commented Dec 18, 2021

@imchell can you make that change and leave the test to make sure it works?

@cyanxiao
Copy link
Copy Markdown
Contributor Author

cyanxiao commented Dec 18, 2021

@UziTech updated 👀

UziTech
UziTech approved these changes Dec 18, 2021
View reviewed changes
Copy link
Copy Markdown
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! 💯

@UziTech UziTech merged commit f82ea2c into markedjs:master Dec 19, 2021
github-actions bot pushed a commit that referenced this pull request Dec 19, 2021
@semantic-release-bot
chore(release): 4.0.8 [skip ci]
59375fb 
## [4.0.8](v4.0.7...v4.0.8) (2021-12-19)

### Bug Fixes

* spaces on a newline after a table ([#2319](#2319)) ([f82ea2c](f82ea2c))
@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 19, 2021

🎉 This PR is included in version 4.0.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

@sync-by-unito sync-by-unito bot mentioned this pull request Jan 22, 2022
Closed
@snyk-bot snyk-bot mentioned this pull request Dec 29, 2022
Open
Logiclayer1111 pushed a commit to Logiclayer1111/marked that referenced this pull request Apr 20, 2026
@semantic-release-bot
chore(release): 4.0.8 [skip ci]
0d1de1a 
## [4.0.8](markedjs/marked@v4.0.7...v4.0.8) (2021-12-19)

### Bug Fixes

* spaces on a newline after a table ([#2319](markedjs/marked#2319)) ([280d3d1](markedjs/marked@280d3d1))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@UziTech UziTech UziTech approved these changes

@calculuschild calculuschild calculuschild approved these changes

@davisjam davisjam Awaiting requested review from davisjam

@joshbruce joshbruce Awaiting requested review from joshbruce

@styfle styfle Awaiting requested review from styfle

Assignees

No one assigned

Labels

released

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Error thrown if there's a space on a newline after a table

3 participants

@cyanxiao @UziTech @calculuschild