Merge pull request #455 from ember-tooling/issue-453-glint-comment-merge

Don't merge lines if a comment breaks them up

Author: evoactivity

src/printers.ts

@@ -70,7 +70,7 @@ function getVisitorKeys(node: NodeType, nonTraversableKeys: Set<string>) {
 function print(
   path: AstPath<NodeType>,
   options: PluginOptions<NodeType>,
-  print: (path: AstPath<NodeType>) => AST.builders.Doc,
+  print: Parameters<Printer<NodeType>['print']>[2],
   args?: unknown,
 ) {
   const { node } = path;

src/printers/ambiguity.ts

@@ -36,7 +36,7 @@ export function fixPreviousPrint(
   previousTemplatePrinted: AST.builders.Doc[],
   path: AstPath<NodeType>,
   options: PluginOptions,
-  print: (path: AstPath<NodeType>) => AST.builders.Doc,
+  print: Parameters<Printer<NodeType>['print']>[2],
   args: unknown,
 ): void {
   const printedSemiFalse = printer.print(

src/printers/print.ts

@@ -51,7 +51,7 @@ export async function printTemplateContent(
   ) => Promise<AST.builders.Doc>,
   options: PluginOptions,
 ): Promise<AST.builders.Doc> {
-  return await textToDoc(text.trim(), {
+  return textToDoc(text.trim(), {
     ...options,
     parser: 'glimmer',
     singleQuote: options.templateSingleQuote ?? options.singleQuote,
@@ -70,9 +70,33 @@ export async function printTemplateContent(
 export function printTemplateTag(
   content: AST.builders.Doc,
 ): AST.builders.Doc[] {
-  const contents = flattenDoc(content);
+  const strings: string[] = [];
 
-  const useHardline = contents.some(
+  // Single pass: collect strings for the hardline/softline decision and
+  // simultaneously force any group that contains an HBS comment
+  // ({{! ... }} or {{!-- ... --}}) to always expand. This prevents the
+  // Glimmer printer from collapsing a component tag onto one line when a
+  // Glint annotation comment appears between its attributes.
+  const processedContent = AST.utils.mapDoc(content, (node) => {
+    if (typeof node === 'string') {
+      strings.push(node);
+    } else if (
+      node &&
+      typeof node === 'object' &&
+      'type' in node &&
+      node.type === 'group'
+    ) {
+      const groupStrings = flattenDoc(
+        (node as { contents: AST.builders.Doc }).contents,
+      );
+      if (groupStrings.some((s) => s.startsWith('{{!') || s.trim() === '!')) {
+        return { ...node, break: true };
+      }
+    }
+    return node;
+  });
+
+  const useHardline = strings.some(
     (c) =>
       // contains angle bracket tag
       /<.+>/.test(c) ||
@@ -83,7 +107,7 @@ export function printTemplateTag(
 
   const doc = [
     TEMPLATE_TAG_OPEN,
-    AST.builders.indent([line, AST.builders.group(content)]),
+    AST.builders.indent([line, AST.builders.group(processedContent)]),
     line,
     TEMPLATE_TAG_CLOSE,
   ];

src/utils/doc.ts

@@ -1,10 +1,10 @@
-import type { doc } from 'prettier';
+import { doc as AST } from 'prettier';
 
 /**
  * Flattens the given doc into a string array, tossing line breaks, etc, for
  * analysis.
  */
-export function flattenDoc(doc: doc.builders.Doc): string[] {
+export function flattenDoc(doc: AST.builders.Doc): string[] {
   if (Array.isArray(doc)) {
     return doc.flatMap(flattenDoc);
   } else if (typeof doc === 'string') {

tests/cases/gts/issue-453.gts

@@ -0,0 +1,7 @@
+<template>
+  <SomeComponent
+    @foo={{false}}
+    {{! @glint-expect-error }}
+    @bar={{false}}
+  />
+</template>

tests/unit-tests/__snapshots__/format.test.ts.snap

@@ -578,6 +578,17 @@ export default class PooComponent extends Component {
 "
 `;
 
+exports[`format > config > default > it formats ../cases/gts/issue-453.gts 1`] = `
+"<template>
+  <SomeComponent
+    @foo={{false}}
+    {{! @glint-expect-error }}
+    @bar={{false}}
+  />
+</template>
+"
+`;
+
 exports[`format > config > default > it formats ../cases/gts/js-only.gts 1`] = `
 "const num: number = 1;
 "

tests/unit-tests/config/__snapshots__/semi-false.test.ts.snap

@@ -578,6 +578,17 @@ export default class PooComponent extends Component {
 "
 `;
 
+exports[`config > semi: false > it formats ../cases/gts/issue-453.gts 1`] = `
+"<template>
+  <SomeComponent
+    @foo={{false}}
+    {{! @glint-expect-error }}
+    @bar={{false}}
+  />
+</template>
+"
+`;
+
 exports[`config > semi: false > it formats ../cases/gts/js-only.gts 1`] = `
 "const num: number = 1
 "

tests/unit-tests/config/__snapshots__/template-export-default.test.ts.snap

@@ -578,6 +578,17 @@ export default class PooComponent extends Component {
 "
 `;
 
+exports[`config > templateExportDefault: true > it formats ../cases/gts/issue-453.gts 1`] = `
+"export default <template>
+  <SomeComponent
+    @foo={{false}}
+    {{! @glint-expect-error }}
+    @bar={{false}}
+  />
+</template>
+"
+`;
+
 exports[`config > templateExportDefault: true > it formats ../cases/gts/js-only.gts 1`] = `
 "const num: number = 1;
 "