diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index 39590f47..0ec4c9f1 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -74,5 +74,37 @@ const comp = ( ); ``` +### Handlebars: Avoid adding unwanted line breaks between text and mustaches ([#6186] by [@gavinjoyce]) + +Previously, Prettier added line breaks between text and mustaches which resulted in unwanted whitespace in rendered output. + + +```hbs +// Input +

Your username is @{{name}}

+

Hi {{firstName}} {{lastName}}

+ +// Output (Prettier stable) +

+ Your username is @ + {{name}} +

+

+ Hi + {{firstName}} + {{lastName}} +

+ +// Output (Prettier master) +

+ Your username is @{{name}} +

+

+ Hi {{firstName}} {{lastName}} +

+``` + [#6209]: https://github.com/prettier/prettier/pull/6209 +[#6186]: https://github.com/prettier/prettier/pull/6186 [@duailibe]: https://github.com/duailibe +[@gavinjoyce]: https://github.com/gavinjoyce diff --git a/src/language-handlebars/parser-glimmer.js b/src/language-handlebars/parser-glimmer.js index 163dbf32..a9b13b3b 100644 --- a/src/language-handlebars/parser-glimmer.js +++ b/src/language-handlebars/parser-glimmer.js @@ -1,32 +1,13 @@ "use strict"; const createError = require("../common/parser-create-error"); -function removeEmptyNodes(node) { - return ( - node.type !== "TextNode" || - (node.type === "TextNode" && - node.chars.replace(/^\s+/, "").replace(/\s+$/, "") !== "") - ); -} -function removeWhiteSpace() { - return { - visitor: { - Program(node) { - node.body = node.body.filter(removeEmptyNodes); - }, - ElementNode(node) { - node.children = node.children.filter(removeEmptyNodes); - } - } - }; -} function parse(text) { try { const glimmer = require("@glimmer/syntax").preprocess; return glimmer(text, { plugins: { - ast: [removeWhiteSpace] + ast: [] } }); /* istanbul ignore next */ diff --git a/src/language-handlebars/printer-glimmer.js b/src/language-handlebars/printer-glimmer.js index 50a3960e..11120806 100644 --- a/src/language-handlebars/printer-glimmer.js +++ b/src/language-handlebars/printer-glimmer.js @@ -32,6 +32,31 @@ const voidTags = [ // Formatter based on @glimmerjs/syntax's built-in test formatter: // https://github.com/glimmerjs/glimmer-vm/blob/master/packages/%40glimmer/syntax/lib/generation/print.ts +function printChildren(path, options, print) { + return concat( + path.map((childPath, childIndex) => { + const isFirstNode = childIndex === 0; + const isLastNode = + childIndex == path.getParentNode(0).children.length - 1; + const isLastNodeInMultiNodeList = isLastNode && !isFirstNode; + + if (isLastNodeInMultiNodeList) { + return concat([print(childPath, options, print)]); + } else if ( + isFirstNode || + isPreviousNodeOfSomeType(childPath, [ + "ElementNode", + "CommentStatement", + "MustacheCommentStatement" + ]) + ) { + return concat([softline, print(childPath, options, print)]); + } + return concat([print(childPath, options, print)]); + }, "children") + ); +} + function print(path, options, print) { const n = path.getValue(); @@ -83,7 +108,7 @@ function print(path, options, print) { ), group( concat([ - indent(join(softline, [""].concat(path.map(print, "children")))), + indent(printChildren(path, options, print)), ifBreak(hasChildren ? hardline : "", ""), !isVoid ? concat([""]) : "" ]) @@ -122,18 +147,16 @@ function print(path, options, print) { indent(concat([hardline, path.call(print, "program")])) ]); } - /** - * I want this boolean to be: if params are going to cause a break, - * not that it has params. - */ - const hasParams = n.params.length > 0 || n.hash.pairs.length > 0; - const hasChildren = n.program.body.length > 0; + + const hasNonWhitespaceChildren = n.program.body.some( + n => !isWhitespaceNode(n) + ); return concat([ printOpenBlock(path, print), group( concat([ indent(concat([softline, path.call(print, "program")])), - hasParams && hasChildren ? hardline : softline, + hasNonWhitespaceChildren ? hardline : softline, printCloseBlock(path, print) ]) ) @@ -193,9 +216,22 @@ function print(path, options, print) { return concat([n.key, "=", path.call(print, "value")]); } case "TextNode": { + const isWhitespaceOnly = !/\S/.test(n.chars); + + if ( + isWhitespaceOnly && + isPreviousNodeOfSomeType(path, ["MustacheStatement", "TextNode"]) + ) { + return " "; + } + let leadingSpace = ""; let trailingSpace = ""; + if (isNextNodeOfType(path, "MustacheStatement")) { + trailingSpace = " "; + } + // preserve a space inside of an attribute node where whitespace present, when next to mustache statement. const inAttrNode = path.stack.indexOf("attributes") >= 0; @@ -350,6 +386,52 @@ function printCloseBlock(path, print) { return concat(["{{/", path.call(print, "path"), "}}"]); } +function isWhitespaceNode(node) { + return node.type === "TextNode" && !/\S/.test(node.chars); +} + +function getPreviousNode(path) { + const node = path.getValue(); + const parentNode = path.getParentNode(0); + + const children = parentNode.children; + if (children) { + const nodeIndex = children.indexOf(node); + if (nodeIndex > 0) { + const previousNode = children[nodeIndex - 1]; + return previousNode; + } + } +} + +function getNextNode(path) { + const node = path.getValue(); + const parentNode = path.getParentNode(0); + + const children = parentNode.children; + if (children) { + const nodeIndex = children.indexOf(node); + if (nodeIndex < children.length) { + const nextNode = children[nodeIndex + 1]; + return nextNode; + } + } +} + +function isPreviousNodeOfSomeType(path, types) { + const previousNode = getPreviousNode(path); + + if (previousNode) { + return types.some(type => previousNode.type === type); + } + return false; +} + +function isNextNodeOfType(path, type) { + const nextNode = getNextNode(path); + return nextNode && nextNode.type === type; +} + function clean(ast, newObj) { delete newObj.loc; diff --git a/tests/glimmer/__snapshots__/jsfmt.spec.js.snap b/tests/glimmer/__snapshots__/jsfmt.spec.js.snap index 9e411519..9d526efc 100644 --- a/tests/glimmer/__snapshots__/jsfmt.spec.js.snap +++ b/tests/glimmer/__snapshots__/jsfmt.spec.js.snap @@ -119,7 +119,11 @@ printWidth: 80 }} Hello {{/block}} -{{#block}}{{#block}}hello{{/block}}{{/block}} +{{#block}} + {{#block}} + hello + {{/block}} +{{/block}} {{#block}} {{#block param}} hello @@ -130,13 +134,14 @@ printWidth: 80 hello {{/block}} {{/block}} -{{#block}}hello{{/block}} +{{#block}} + hello +{{/block}} {{firstName}} - {{firstName}} - {{lastName}} + {{firstName}} {{lastName}} ================================================================================ `; @@ -261,7 +266,11 @@ singleQuote: true }} Hello {{/block}} -{{#block}}{{#block}}hello{{/block}}{{/block}} +{{#block}} + {{#block}} + hello + {{/block}} +{{/block}} {{#block}} {{#block param}} hello @@ -272,13 +281,14 @@ singleQuote: true hello {{/block}} {{/block}} -{{#block}}hello{{/block}} +{{#block}} + hello +{{/block}} {{firstName}} - {{firstName}} - {{lastName}} + {{firstName}} {{lastName}} ================================================================================ `; @@ -525,6 +535,43 @@ singleQuote: true ================================================================================ `; +exports[`curly.hbs 1`] = ` +====================================options===================================== +parsers: ["glimmer"] +printWidth: 80 + | printWidth +=====================================input====================================== +

Your username is @{{name}}

+

Hi {{firstName}} {{lastName}}

+=====================================output===================================== +

+ Your username is @{{name}} +

+

+ Hi {{firstName}} {{lastName}} +

+================================================================================ +`; + +exports[`curly.hbs 2`] = ` +====================================options===================================== +parsers: ["glimmer"] +printWidth: 80 +singleQuote: true + | printWidth +=====================================input====================================== +

Your username is @{{name}}

+

Hi {{firstName}} {{lastName}}

+=====================================output===================================== +

+ Your username is @{{name}} +

+

+ Hi {{firstName}} {{lastName}} +

+================================================================================ +`; + exports[`element-modifier-statement.hbs 1`] = ` ====================================options===================================== parsers: ["glimmer"] @@ -735,7 +782,9 @@ printWidth: 80 A long enough string to trigger a line break that would prevent wrapping more and more.
- {{#block}}{{hello}}{{/block}} + {{#block}} + {{hello}} + {{/block}}
{{hello}} @@ -809,7 +858,9 @@ singleQuote: true A long enough string to trigger a line break that would prevent wrapping more and more.
- {{#block}}{{hello}}{{/block}} + {{#block}} + {{hello}} + {{/block}}
{{hello}} diff --git a/tests/glimmer/curly.hbs b/tests/glimmer/curly.hbs new file mode 100644 index 00000000..edeee979 --- /dev/null +++ b/tests/glimmer/curly.hbs @@ -0,0 +1,2 @@ +

Your username is @{{name}}

+

Hi {{firstName}} {{lastName}}

\ No newline at end of file diff --git a/tests/handlebars/__snapshots__/jsfmt.spec.js.snap b/tests/handlebars/__snapshots__/jsfmt.spec.js.snap index c694038d..c7ee2c91 100644 --- a/tests/handlebars/__snapshots__/jsfmt.spec.js.snap +++ b/tests/handlebars/__snapshots__/jsfmt.spec.js.snap @@ -165,8 +165,7 @@ printWidth: 80 {{title}}

- By - {{author.name}} + By {{author.name}}

{{body}}