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: add option closing-bracket-newline for html beautify #2316

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,7 @@ HTML Beautifier Options:
--unformatted_content_delimiter Keep text content together between this string [""]
--indent-empty-lines Keep indentation on empty lines
--templating List of templating languages (auto,none,django,erb,handlebars,php,smarty,angular) ["auto"] auto = none in JavaScript, all in html
--closing-bracket-newline Add a newline before tag closing brackets
```

## Directives
Expand Down
4 changes: 3 additions & 1 deletion js/src/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ var path = require('path'),
"quiet": Boolean,
"type": ["js", "css", "html"],
"config": path,
"editorconfig": Boolean
"editorconfig": Boolean,
"closing_bracket_newline": Boolean
},
// dasherizeShorthands provides { "indent-size": ["--indent_size"] }
// translation, allowing more convenient dashes in CLI arguments
Expand Down Expand Up @@ -411,6 +412,7 @@ function usage(err) {
msg.push(' -T, --content_unformatted List of tags (defaults to pre) whose content should not be reformatted');
msg.push(' -E, --extra_liners List of tags (defaults to [head,body,/html] that should have an extra newline');
msg.push(' --unformatted_content_delimiter Keep text content together between this string [""]');
msg.push(' --closing-bracket-newline Add a newline before tag closing brackets');
break;
case "css":
msg.push(' -b, --brace-style [collapse|expand] ["collapse"]');
Expand Down
5 changes: 4 additions & 1 deletion js/src/html/beautifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,10 @@ Beautifier.prototype._handle_tag_close = function(printer, raw_token, last_tag_t
} else {
if (last_tag_token.tag_start_char === '<') {
printer.set_space_before_token(raw_token.text[0] === '/', true); // space before />, no space before >
if (this._is_wrap_attributes_force_expand_multiline && last_tag_token.has_wrapped_attrs) {
// Add newline before tag closing bracket:
// 1. add newline only if 'force-expand-multipand' or 'closing-bracket-newline' is specified
zhaojjiang marked this conversation as resolved.
Show resolved Hide resolved
// 2. add newline only if tag has wrapped_attrs
if ((this._is_wrap_attributes_force_expand_multiline || this._options.closing_bracket_newline) && last_tag_token.has_wrapped_attrs) {
Copy link
Member

Choose a reason for hiding this comment

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

This makes the behavior of this new option the same as force_expand_multiline ... If that is the case you don't need the new option. I think what you want is

Suggested change
if ((this._is_wrap_attributes_force_expand_multiline || this._options.closing_bracket_newline) && last_tag_token.has_wrapped_attrs) {
if ((this._is_wrap_attributes_force_expand_multiline && last_tag_token.has_wrapped_attrs) || this._options.closing_bracket_newline) {

But that might not be it either...

Copy link
Author

@zhaojjiang zhaojjiang Nov 18, 2024

Choose a reason for hiding this comment

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

This makes the behavior of this new option the same as force_expand_multiline ... If that is the case you don't need the new option.

Yes, it is. This new option should work only with a tag which has wrapped attributes, it should not work when tag has no attributes.

printer.print_newline(false);
}
}
Expand Down
2 changes: 1 addition & 1 deletion js/src/html/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ function Options(options) {
]);
this.unformatted_content_delimiter = this._get_characters('unformatted_content_delimiter');
this.indent_scripts = this._get_selection('indent_scripts', ['normal', 'keep', 'separate']);

this.closing_bracket_newline = this._get_boolean('closing_bracket_newline');
}
Options.prototype = new BaseOptions();

Expand Down
44 changes: 44 additions & 0 deletions test/data/html/tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -4401,6 +4401,50 @@ exports.test_data = {
'</style>'
]
}]
}, {
name: "Add newline before tag closing bracket when tag has wrapped attributes",
description: "Add newline before tag closing bracket when tag has wrapped attributes",
template: "^^^ $$$",
matrix: [{
options: [
{ name: "wrap_attributes", value: "'auto'" },
{ name: "closing_bracket_newline", value: "true" }
],
closing_bracket_newline: ''
}, {
options: [
{ name: "wrap_attributes", value: "'preserve'" },
{ name: "closing_bracket_newline", value: "true" }
],
fill_indent: ' ', // fill missing indent space
closing_bracket_newline: '\n'
}, {
options: [
{ name: "closing_bracket_newline", value: "false" }
],
closing_bracket_newline: ''
}],
tests: [{
fragment: true,
unchanged: [
'<div style="display: block;" width="100" height="200">',
' <p>test content</p>',
'</div>'
]
}, {
fragment: true,
input: [
'<div',
' style="display: block;" width="100" height="200">',
' <p>test content</p>',
'</div>'
],
output: [
'<div^^^closing_bracket_newline$$$^^^fill_indent$$$ style="display: block;" width="100" height="200"^^^closing_bracket_newline$$$>',
' <p>test content</p>',
'</div>'
]
}]
}, {
name: "New Test Suite"
}]
Expand Down
Loading