-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Docgen: Replace line breaks with spaces and remove line-ending hyphens #49635
Conversation
Size Change: 0 B Total Size: 1.37 MB ℹ️ View Unchanged
|
description: 'My declaration example.', | ||
description: `My declaration example. | ||
Hyphen- | ||
ation. | ||
|
||
Code: | ||
|
||
\`\`\`js | ||
const myCode = 'code'; | ||
\`\`\` | ||
|
||
Unordered Lists: | ||
|
||
- List Item. | ||
- List Item. | ||
List Item. | ||
|
||
Ordered Lists: | ||
|
||
1. List Item. | ||
2. List Item. | ||
List Item. | ||
`, |
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.
Updated strings to confirm that the intended parsing occurs.
Flaky tests detected in 8ba9724. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4649073152
|
Block validity is a function of blocks state (at the point of a | ||
reset) and the template setting. As a compromise to its placement | ||
across distinct parts of state, it is implemented here as a side- | ||
effect of the block reset action. | ||
Block validity is a function of blocks state (at the point of a reset) and the template setting. As a compromise to its placement across distinct parts of state, it is implemented here as a sideeffect of the block reset action. |
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.
This is where the hyphenation is removed. Perhaps it should be replaced with a single-byte space.
// From
side-
effect
// To
sideeffect
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, as a 30+ year native english speaker, it should be "side effect"
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.
Thanks for the advice. As you said, I don't think this is the hyphenation, so I fixed the JSDoc comment and regenerated the document in 8ba9724.
@t-hamano thanks for your work on resolving this. There's a part of me that wonders though if this is more eliminating the symptom of a broken/undesirable Markdown parser and less a fix of the core issue, because it seems like we're breaking the formatting of those docs in the code itself in order to avoid a case in the documentation generation that doesn't wrap properly. Did you examine any changes to how the documentation is generated that would resolve the issue without hiding the long JSDoc comments in the source code? and by hiding I mean that previously they were line-wrapped so they would be visible in any code editor, but after this change they will run on past the view and someone will need to scroll their editor to read the full text. |
Is this about the scrolling that occurs when looking at auto-generated markdown files if a user has a setting like word wrap turned off in the code editor? For example, the following behavior: 821dfbebcf26f23f5b165a8eb998cf3d.mp4If this is about this, I think it could be about all other documents that are not auto-generated: 7d8a8e2c0c0195dae643a3859a3b9a6d.mp4If this is what you are concerned about, these wraps can be changed in the code editor settings. I think it is more important to fix the unintended folds in the Block Editor Handbook that will be published on the web. I may not be understanding what you are saying correctly, so I would appreciate it if you could tell me if I am different. |
transformer( node ); | ||
} | ||
if ( node.type === 'text' && node.value ) { | ||
// Replace line breaks with spaces and remove hyphens from hyphenated words. |
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.
the code is right here, but the comment is misleading the way I interpret it.
Replace line breaks with spaces and remove line-ending hyphens.
Based on the comment I expected "client-side" to be replaced by "clientside" though it was me being wrong with that assumption. It could help to have an example in the test code that shows hyphens that aren't replaced.
Client-side hyphen-
ation.
My declaration example. Client-side hyphenation.
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.
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.
aha, it was me who misunderstood, as you are doing exactly what I was asking for and there are no .js
changes in here 😄
left a comment about the comment in the transformer, but apart from that this seems like a solid change. thanks for clearing it up.
Yes, this PR removes the line breaks in the auto-generated markdown file and does nothing to the JSDOC in the original I also merged the latest trunk and built it again and all markdown files appear to be handled properly. |
Removed label |
Related to: #44768
Note: Sorry for the big change. But almost all files are changes due to auto-generation of documents, and the actual updates are the following two:
What?
This PR replaces line breaks in the JS Doc description text with single-byte spaces in the document file generated by the
docs:build
command.Why?
The reference guide in the block editor handbook and some contents of the package README are generated by parsing JSDoc by the
docgen
package. In particular, JSDoc's@description
is broken into appropriate lines to prevent a single line from becoming too long. The current document generation logic converts this tag directly to markdown, which may result in unnatural line breaks in the block editor handbook.Below are examples of a section with unnatural line breaks:
How?
Since a description may contain lists or code blocks, it cannot simply replace line breaks with spaces. Fortunately, there was already remark package that handled text in markdown format, so I replaced only the line breaks in the text node values in the AST tree obtained by parsing.
In addition, hyphens are removed where they appear to be line breaks due to end-of-line hyphenation.
Testing Instructions
There is no impact on the code, as I only update the logic of the automatically generated markdown.