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

HTML: fix pre's incomplete description #37540

Merged
merged 4 commits into from
Jan 13, 2025
Merged

Conversation

PassionPenguin
Copy link
Contributor

Description

<pre> will drop the first blank line (if exists) immediately following its start tag.

Motivation

per @Josh-Cena's comment #37384 (comment)

Additional details

<pre>'s HTML documentation: https://html.spec.whatwg.org/multipage/grouping-content.html#the-pre-element

@PassionPenguin PassionPenguin requested a review from a team as a code owner January 7, 2025 04:51
@PassionPenguin PassionPenguin requested review from chrisdavidmills and removed request for a team January 7, 2025 04:51
@github-actions github-actions bot added Content:HTML Hypertext Markup Language docs size/s [PR only] 6-50 LoC changed labels Jan 7, 2025
Copy link
Contributor

github-actions bot commented Jan 7, 2025

Preview URLs

Flaws (23)

URL: /en-US/docs/Web/HTML/Element/pre
Title: <pre>: The Preformatted Text element
Flaw count: 23

  • macros:
    • Wrong xref macro used (consider changing which macro you use). Error processing path /en-US/docs/Learn/HTML
    • Wrong xref macro used (consider changing which macro you use). Error processing path /en-US/docs/Learn/Getting_started_with_the_web/HTML_basics
    • Wrong xref macro used (consider changing which macro you use). Error processing path /en-US/docs/Learn/HTML/Introduction_to_HTML
    • Wrong xref macro used (consider changing which macro you use). Error processing path /en-US/docs/Learn/HTML/Introduction_to_HTML/Getting_started
    • Wrong xref macro used (consider changing which macro you use). Error processing path /en-US/docs/Learn/HTML/Introduction_to_HTML/The_head_metadata_in_HTML
    • and 18 more flaws omitted
External URLs (1)

URL: /en-US/docs/Web/HTML/Element/pre
Title: <pre>: The Preformatted Text element

(comment last updated: 2025-01-13 10:42:18)

Copy link
Contributor

@chrisdavidmills chrisdavidmills 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 the great work, as always, @PassionPenguin!

So, for this one, I think your first conflict concern is OK — the wording provided is "represents preformatted text which is to be presented exactly as written". I think the stripping of the first blank line is intended to help with this — authors will appreciate being able to write their element content on separate lines to the opening and closing tags without having to worry about the leading newline character.

Also, note that the text following this line qualifies the exact behavior, so I think we are all good here. If you are still worried, you could maybe update "represents preformatted text which is to be presented exactly as written" to "represents preformatted text intended to be presented exactly as written", to signify that this is what the intent is, rather than the exact behavior?

For your second concern, I think this is valid — the whitespace line looks to be in conflict with your addition. To fix this, I would put the whitespace line into its own paragraph and combine it with your note, which I don't think needs to be a note.

So the second paragraph would read:

Whitespace inside this element is displayed as written, with one exception. If one or more leading newline characters are included immediately following the opening <pre> tag, the first newline character is stripped.

Let me know what you think. Thanks again!

@github-actions github-actions bot added size/xs [PR only] 0-5 LoC changed and removed size/s [PR only] 6-50 LoC changed labels Jan 10, 2025
@PassionPenguin
Copy link
Contributor Author

Thanks for the great work, as always, @PassionPenguin!

So, for this one, I think your first conflict concern is OK — the wording provided is "represents preformatted text which is to be presented exactly as written". I think the stripping of the first blank line is intended to help with this — authors will appreciate being able to write their element content on separate lines to the opening and closing tags without having to worry about the leading newline character.

Also, note that the text following this line qualifies the exact behavior, so I think we are all good here. If you are still worried, you could maybe update "represents preformatted text which is to be presented exactly as written" to "represents preformatted text intended to be presented exactly as written", to signify that this is what the intent is, rather than the exact behavior?

For your second concern, I think this is valid — the whitespace line looks to be in conflict with your addition. To fix this, I would put the whitespace line into its own paragraph and combine it with your note, which I don't think needs to be a note.

So the second paragraph would read:

Whitespace inside this element is displayed as written, with one exception. If one or more leading newline characters are included immediately following the opening \<pre\> tag, the first newline character is stripped.

Let me know what you think. Thanks again!x

Your suggestions are reasonable and awesome, and I'd updated the content to go with your suggestion!

Thanks for your review~!

Copy link
Contributor

@chrisdavidmills chrisdavidmills left a comment

Choose a reason for hiding this comment

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

@PassionPenguin Perfect; let's get this merged. Thanks again!

@chrisdavidmills chrisdavidmills merged commit 6ca9542 into mdn:main Jan 13, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:HTML Hypertext Markup Language docs size/xs [PR only] 0-5 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants