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.

Sign up for GitHub

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

Jump to bottom

HTML formatter does not send comments to a new line when they start with non-whitespace character #1915

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

HTML formatter does not send comments to a new line when they start with non-whitespace character #1915

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6e8955c
fixed issue 1823
carolinekistler Apr 14, 2021
d569ebb
Merge branch 'main' into issue1823
bitwiseman Apr 15, 2021
4a25303
Merge branch 'main' into issue1823
carolinekistler Apr 18, 2021
3d58277
Merge branch 'main' into issue1823
bitwiseman May 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion js/src/html/beautifier.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@ Beautifier.prototype._set_tag_position = function(printer, raw_token, parser_tok
}

// Don't add a newline before elements that should remain where they are.
if (parser_token.tag_name === '!--' && last_token.type === TOKEN.TAG_CLOSE &&
if ((parser_token.tag_name !== '!--' || parser_token.tag_name === '!--') && last_token.type === TOKEN.TAG_CLOSE &&
Copy link
Member

Choose a reason for hiding this comment

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

When does the expression (parser_token.tag_name !== '!--' || parser_token.tag_name === '!--') not evaluate to true? What happens if you remove this expression?

Copy link
Author

Choose a reason for hiding this comment

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

It has to deal with the spacing issue that it is reading when reading the comments. It is reading the tag name that does not have a space after as the tag name is not equal to '!== 'and vice versa. This expression covers both edge cases.

Copy link
Author

Choose a reason for hiding this comment

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

How would you like me to add tests?

Copy link
Member

Choose a reason for hiding this comment

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

Tests: See #1844. You only the to update the test data file. The generated files are generated by the build.

Copy link
Member

Choose a reason for hiding this comment

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

@carolinekistler
Please fetch and merge the latest changes from main. I've just removed the generated files from tracking to improve clarity.

last_tag_token.is_end_tag && parser_token.text.indexOf('\n') === -1) {
//Do nothing. Leave comments on same line.
} else {
Expand Down