Improved handling of tag closing errors #147
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Wasn’t returning the promise, so failed without reporting failure * Also was asserting the wrong thing Nice.
Problems with closing HTML tags can look like a variety of things to our HTML linters and parsers. More confusingly, these things can overlap. This pull improves detection of unclosed tags by handling `htmllint`’s `tag-close` error, and uses suppression to avoid confusing, overlapping error messages. Problem 1: Close tag is missing ```html <!doctype html> <html> <head><title>My Page</title></head> <body> <div>Page content </body> </html> ``` This error seems straightforward: there is an opening `<div>` tag that doesn’t have a corresponding closing tag. However, it can also be interpreted as a *mismatched* close tag: namely, you’re trying to close the `<div>` with a `</body>`. Intuitively, we want to report the first error, and now we do. Problem 2: Mismatched opening and closing tags ```html <!doctype html> <html> <head><title>My Page</title></head> <body> <div>Page content</span> </body> </html> ``` In this case, you didn’t necessarily forget to close the `<div>`; you’re just closing it with the wrong closing tag. Unfortunately, from the parsers’ standpoint, this looks the same as Problem 1: you have an opening tag without a closing tag, and you also have a mismatched closing tag where the `<div>` should be closed. Absent a great way to tell the difference between this situation and the previous one, we still report this as a missing closing tag for the `<div>`. The error message, “Your `<div>` tag needs to be closed by a `</div>` tag”, is clear enough for this situation. Problem 3: Incomplete closing tag ```html <!doctype html> <html> <head><title>My Page</title></head> <body> <div>Page content</div </body> </html> ``` The behavior we want here is also straightforward: point out that the `</div>` tag is missing the closing angle bracket. However, the error in the `</div>` means that the `</body>` also isn’t parsed correctly, so the linters also catch a missing `</body>` tag. The latter error isn’t directly actionable, so we suppress it. Problem 4: Closing tag with no corresponding opening tag ```html <!doctype html> <html> <head><title>My Page</title></head> <body> <div>Page content</div></span> </body> </html> ``` In this case, the `</span>` is unambiguously a mismatched closing tag, and we can report it as such.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problems with closing HTML tags can look like a variety of things to our HTML linters and parsers. More confusingly, these things can overlap.
This pull improves detection of unclosed tags by handling
htmllint’stag-closeerror, and uses suppression to avoid confusing, overlapping error messages.Problem 1: Close tag is missing
This error seems straightforward: there is an opening
<div>tag that doesn’t have a corresponding closing tag. However, it can also be interpreted as a mismatched close tag: namely, you’re trying to close the<div>with a</body>.Intuitively, we want to report the first error, and now we do.
Problem 2: Mismatched opening and closing tags
In this case, you didn’t necessarily forget to close the
<div>; you’re just closing it with the wrong closing tag. Unfortunately, from the parsers’ standpoint, this looks the same as Problem 1: you have an opening tag without a closing tag, and you also have a mismatched closing tag where the<div>should be closed.Absent a great way to tell the difference between this situation and the previous one, we still report this as a missing closing tag for the
<div>. The error message, “Your<div>tag needs to be closed by a</div>tag”, is clear enough for this situation.Problem 3: Incomplete closing tag
The behavior we want here is also straightforward: point out that the
</div>tag is missing the closing angle bracket. However, the error in the</div>means that the</body>also isn’t parsed correctly, so the linters also catch a missing</body>tag. The latter error isn’t directly actionable, so we suppress it.Problem 4: Closing tag with no corresponding opening tag
In this case, the
</span>is unambiguously a mismatched closing tag, and we can report it as such.