-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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 relative links in main package READMEs so that they work in npm #38609
Conversation
@@ -1,6 +1,6 @@ | |||
# Block Serialization Default Parser | |||
|
|||
This library contains the default block serialization parser implementations for WordPress documents. It provides native PHP and JavaScript parsers that implement the [specification](/docs/contributors/grammar.md) from [`@wordpress/block-serialization-spec-parser`](https://github.com/WordPress/gutenberg/tree/HEAD/packages/block-serialization-spec-parser/README.md) and which normally operates on the document stored in `post_content`. | |||
This library contains the default block serialization parser implementations for WordPress documents. It provides native PHP and JavaScript parsers that implement the [specification](https://github.com/WordPress/gutenberg/tree/HEAD/docs/contributors/grammar.md) from [`@wordpress/block-serialization-spec-parser`](https://github.com/WordPress/gutenberg/tree/HEAD/packages/block-serialization-spec-parser/README.md) and which normally operates on the document stored in `post_content`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, this seems to have been a broken link anyway. I'll try to fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is a very annoying problem. It has two downsides when using full links:
- you can't test new links in PRs (branches)
- when browsing the Block Editor Handbook links will go to GitHub rather than be internal as they used to
Thanks for the quick review!
Hmm, that's a good point and not ideal. I guess we might be able to rewrite these links when publishing to the handbook, but I'm not sure where that change would be made. |
Yes, it might be possible if always use the same URL. It is now standardized to https://github.com/WordPress/gutenberg/tree/HEAD/ so we can update the regular expression that is used to replace URLs with internal links. I only forgot which repository is used to sync the docs with the dev portal. I'll try to find it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work 👍 !
I think I've found the importer: It should be as simple as ensuring that the regular expressions cover also the case with URLs. Maybe they do already? 😅 |
Size Change: +36 B (0%) Total Size: 1.14 MB
ℹ️ View Unchanged
|
Description
Follow up to #38605.
In that PR, I discovered when npm displays the README for the package, it interprets relative links incorrectly resulting in a broken URL. This is probably an npm bug, but I haven't figured out where to report it yet (one of these - https://github.com/orgs/npm/repositories?type=all).
This PR updates some of the relative links I found using find/replace, changing them to absolute links.
Testing Instructions
To reproduce the bug, try the links in the first paragraph of the core-data package in npm - https://www.npmjs.com/package/@wordpress/core-data.
You can see they're broken but they work ok in github - https://github.com/WordPress/gutenberg/tree/trunk/packages/core-data
If you try an absolute link in npm, like the one in the second 'Installation' section for core-data, that seems to work fine - https://www.npmjs.com/package/@wordpress/core-data.
So it seems like changing to absolute URLs is the best thing to do right now.
Types of changes
Docs