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

feat(format/html): implement suppression comments #5356

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dyc3
Copy link
Contributor

@dyc3 dyc3 commented Mar 14, 2025

Summary

This reverts #4056 to make HTML comments actual comment trivia. It also attempts to fix the formatting for comments as well, trying to preserve whitespace in the same places that prettier does.

This PR is also the result of a cascading clusterfuck of issues that appeared as a result of making this change, so it may seem a little scatterbrained, and a little difficult to review.

Test Plan

CI should pass. Added new tests, and updated snapshots.

@github-actions github-actions bot added A-Parser Area: parser A-Formatter Area: formatter A-Tooling Area: internal tools L-HTML Language: HTML labels Mar 14, 2025
Copy link
Contributor

Parser conformance results on

js/262

Test result main count This PR count Difference
Total 50412 50412 0
Passed 49097 49097 0
Failed 1315 1315 0
Panics 0 0 0
Coverage 97.39% 97.39% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 40 40 0
Passed 37 37 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.50% 92.50% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 6644 6644 0
Passed 2229 2229 0
Failed 4415 4415 0
Panics 0 0 0
Coverage 33.55% 33.55% 0.00%

ts/babel

Test result main count This PR count Difference
Total 798 798 0
Passed 706 706 0
Failed 92 92 0
Panics 0 0 0
Coverage 88.47% 88.47% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 18661 18661 0
Passed 14312 14312 0
Failed 4349 4349 0
Panics 0 0 0
Coverage 76.69% 76.69% 0.00%

Copy link

codspeed-hq bot commented Mar 14, 2025

CodSpeed Performance Report

Merging #5356 will degrade performances by 14.35%

Comparing html-suppression-comments (89290e1) with main (850f496)

Summary

❌ 1 regressions
✅ 94 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
html_formatter[index_1033418810622582172.html] 475.8 µs 555.5 µs -14.35%

@dyc3 dyc3 force-pushed the html-suppression-comments branch from 9700f3a to 89290e1 Compare March 14, 2025 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Formatter Area: formatter A-Parser Area: parser A-Tooling Area: internal tools L-HTML Language: HTML
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant