[Glimmer] improve text/mustache formatting (#6186)

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.
+
+<!-- prettier-ignore -->
+```hbs
+// Input
+<p>Your username is @{{name}}</p>
+<p>Hi {{firstName}} {{lastName}}</p>
+
+// Output (Prettier stable)
+<p>
+  Your username is @
+  {{name}}
+</p>
+<p>
+  Hi
+  {{firstName}}
+  {{lastName}}
+</p>
+
+// Output (Prettier master)
+<p>
+  Your username is @{{name}}
+</p>
+<p>
+  Hi {{firstName}} {{lastName}}
+</p>
+```
+
 [#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

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 */

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(["</", n.tag, ">"]) : ""
           ])
@@ -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;
 

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}}
 <MyComponent as |firstName|>
   {{firstName}}
 </MyComponent>
 <MyComponent as |firstName lastName|>
-  {{firstName}}
-  {{lastName}}
+  {{firstName}} {{lastName}}
 </MyComponent>
 ================================================================================
 `;
@@ -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}}
 <MyComponent as |firstName|>
   {{firstName}}
 </MyComponent>
 <MyComponent as |firstName lastName|>
-  {{firstName}}
-  {{lastName}}
+  {{firstName}} {{lastName}}
 </MyComponent>
 ================================================================================
 `;
@@ -525,6 +535,43 @@ singleQuote: true
 ================================================================================
 `;
 
+exports[`curly.hbs 1`] = `
+====================================options=====================================
+parsers: ["glimmer"]
+printWidth: 80
+                                                                                | printWidth
+=====================================input======================================
+<p>Your username is @{{name}}</p>
+<p>Hi {{firstName}} {{lastName}}</p>
+=====================================output=====================================
+<p>
+  Your username is @{{name}}
+</p>
+<p>
+  Hi {{firstName}} {{lastName}}
+</p>
+================================================================================
+`;
+
+exports[`curly.hbs 2`] = `
+====================================options=====================================
+parsers: ["glimmer"]
+printWidth: 80
+singleQuote: true
+                                                                                | printWidth
+=====================================input======================================
+<p>Your username is @{{name}}</p>
+<p>Hi {{firstName}} {{lastName}}</p>
+=====================================output=====================================
+<p>
+  Your username is @{{name}}
+</p>
+<p>
+  Hi {{firstName}} {{lastName}}
+</p>
+================================================================================
+`;
+
 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.
 </div>
 <div>
-  {{#block}}{{hello}}{{/block}}
+  {{#block}}
+    {{hello}}
+  {{/block}}
 </div>
 <div>
   {{hello}}
@@ -809,7 +858,9 @@ singleQuote: true
   A long enough string to trigger a line break that would prevent wrapping more and more.
 </div>
 <div>
-  {{#block}}{{hello}}{{/block}}
+  {{#block}}
+    {{hello}}
+  {{/block}}
 </div>
 <div>
   {{hello}}

tests/glimmer/curly.hbs

@@ -0,0 +1,2 @@
+<p>Your username is @{{name}}</p>
+<p>Hi {{firstName}} {{lastName}}</p>

tests/handlebars/__snapshots__/jsfmt.spec.js.snap

@@ -165,8 +165,7 @@ printWidth: 80
     {{title}}
   </h1>
   <h2>
-    By
-    {{author.name}}
+    By {{author.name}}
   </h2>
   <div class="body">
     {{body}}