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

Update remark parse v8 #8140

Merged
merged 68 commits into from Jun 23, 2020
Merged

Update remark parse v8 #8140

merged 68 commits into from Jun 23, 2020

Conversation

sarayourfriend
Copy link
Contributor

@sarayourfriend sarayourfriend commented Apr 23, 2020

I had previously opened another branch for this, but noticed that @fisker had already done a good deal of work to resolve some of the issues with upgrading remark-parse, so based my branch off of theirs.

  • I’ve added tests to confirm my change works.
  • (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory)
  • (If the change is user-facing) I’ve added my changes to changelog_unreleased/*/pr-XXXX.md file following changelog_unreleased/TEMPLATE.md.
  • I’ve read the contributing guidelines.

Try the playground for this PR

Or try this branch locally: npm install saramarcondes/prettier#update-remark-parse-v8

@fisker
Copy link
Member

fisker commented Apr 23, 2020

Thanks for taking this over, hope you can make it work. Some markdown and css related deps are really old, very hard to update.

@@ -4317,7 +4317,7 @@ This is [an example](http://example.com 'Example') link.

[This link](http://example.com) has no title attr.

This is [an example][id] reference-style link.
This is [an example] [id] reference-style link.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change matches the expectation in the commonmark spec: https://spec.commonmark.org/dingus/?text=%5Bfoo%5D%20%5Bbar%5D%0A%0A%5Bbar%5D%3A%20%2Furl%20%22title%22%0A

[an example] and [id] appear as two separate shortcut reference links with a text node between them: https://astexplorer.net/#/gist/d69f5a227e2cbde3f43f0a639bee165c/822b531c252c162faba78e38607db73ef72886d5

Previously they would have appeared as a single full reference style link.

Copy link
Member

Choose a reason for hiding this comment

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

Will this also fix #8226 probably?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It sure does! I just pushed an update to include shortcut style links in this test case. I'll add another test case for that issue.

Copy link
Member

@thorn0 thorn0 May 7, 2020

Choose a reason for hiding this comment

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

Thanks. I'm linking that issue to this PR then.

@sarayourfriend
Copy link
Contributor Author

@thorn0 This PR will also address these issues if you'd like to link them

#4369
#7695
#6177

@thorn0
Copy link
Member

thorn0 commented May 7, 2020

The tests directory has been reorganized (#8239 and other PRs). Please rebase.

@thorn0
Copy link
Member

thorn0 commented May 30, 2020

I'll do today.

@fisker
Copy link
Member

fisker commented Jun 10, 2020

#4122 is fixed, need test

@thorn0 thorn0 linked an issue Jun 20, 2020 that may be closed by this pull request
@thorn0 thorn0 added this to the 2.1 milestone Jun 22, 2020
Copy link
Member

@sosukesuzuki sosukesuzuki left a comment

Choose a reason for hiding this comment

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

Thank you for your great work!

@thorn0
Copy link
Member

thorn0 commented Jun 23, 2020

To do after merging: #8630

@alexander-akait alexander-akait merged commit b27f844 into prettier:master Jun 23, 2020
@alexander-akait
Copy link
Member

Great work! Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.