HTML formatter does not send comments to a new line when they start with non-whitespace character #1915
Conversation
bitwiseman
changed the title
There was a problem hiding this comment.
Here is where tag_name get calculated:
https://github.com/beautify-web/js-beautify/blob/bc48f5b54556bfcb3d2baefb2902ccb44958c9ed/js/src/html/beautifier.js#L607
That is what is causing the bug - it is using whitespace as the delimiter. That is reasonable for regular elements but not for comments. You probably want to do a special case right there for TOKEN.COMMENT and check for whether you are dealing with !-- or CDATA.
It is interesting that the change you did make did not break any tests.
Speaking of which, please add tests to this PR to cover the new behavior.
Thanks!
| @@ -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 && | |||
There was a problem hiding this comment.
When does the expression (parser_token.tag_name !== '!--' || parser_token.tag_name === '!--') not evaluate to true? What happens if you remove this expression?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
How would you like me to add tests?
There was a problem hiding this comment.
Tests: See #1844. You only the to update the test data file. The generated files are generated by the build.
There was a problem hiding this comment.
@carolinekistler
Please fetch and merge the latest changes from main. I've just removed the generated files from tracking to improve clarity.
|
@carolinekistler |
Description
main)Fixed if statement here: https://github.com/beautify-web/js-beautify/blob/master/js/src/html/beautifier.js#L698
Fixes Issue:
Fixes #1823 HTML formatter sends comments to a new line when the comment does not include a space as the first character
Before Merge Checklist
These items can be completed after PR is created.
(Check any items that are not applicable (NA) for this PR)