Merge pull request #159 from Shopify/fix/158-line-breaks-btn-attribtues

Maintain at most 1 newline between tag attributes

Author: charlespwd

src/printer/print/tag.ts

@@ -28,6 +28,7 @@ import {
   last,
   first,
   isPrettierIgnoreAttributeNode,
+  hasMoreThanOneNewLineBetweenNodes,
 } from '~/printer/utils';
 
 const {
@@ -271,10 +272,22 @@ function printAttributes(
 
   const prettierIgnoreAttributes = isPrettierIgnoreAttributeNode(node.prev);
 
-  const printedAttributes = path.map(
-    (attr) => print(attr, { trailingSpaceGroupId: attrGroupId }),
-    'attributes',
-  );
+  const printedAttributes = path.map((attr) => {
+    const attrNode = attr.getValue();
+    let extraNewline: Doc = '';
+    if (
+      attrNode.prev &&
+      hasMoreThanOneNewLineBetweenNodes(
+        attrNode.source,
+        attrNode.prev,
+        attrNode,
+      )
+    ) {
+      extraNewline = hardline;
+    }
+    const printed = print(attr, { trailingSpaceGroupId: attrGroupId });
+    return [extraNewline, printed];
+  }, 'attributes');
 
   const forceBreakAttrContent = node.source
     .slice(node.blockStartPosition.start, last(node.attributes).position.end)

src/printer/utils/string.ts

@@ -1,4 +1,9 @@
-import { LiquidAstPath, LiquidHtmlNode, LiquidParserOptions } from '~/types';
+import {
+  LiquidAstPath,
+  LiquidHtmlNode,
+  LiquidParserOptions,
+  Position,
+} from '~/types';
 
 export function isWhitespace(source: string, loc: number): boolean {
   if (loc < 0 || loc >= source.length) return false;
@@ -49,3 +54,14 @@ export function hasLineBreakInRange(
   const indexOfNewLine = source.indexOf('\n', locStart);
   return 0 <= indexOfNewLine && indexOfNewLine < locEnd;
 }
+
+export function hasMoreThanOneNewLineBetweenNodes(
+  source: string,
+  prev: { position: Position } | undefined,
+  next: { position: Position } | undefined,
+): boolean {
+  if (!prev || !next) return false;
+  const between = source.slice(prev.position.end, next.position.start);
+  const count = between.match(/\n/g)?.length || 0;
+  return count > 1;
+}

test/issue-158/fixed.liquid

@@ -0,0 +1,20 @@
+It should maintain at most 1 newline between attributes, and none at the end
+<label class="block relative">
+  <input
+    class="absolute inset-0 opacity-0 pointer-events-none peer"
+    type="radio"
+    name="id"
+
+    id="variant-{{ variant.id }}"
+    value="{{ variant.id }}"
+    required
+
+    {% unless variant.available %}
+      disabled
+    {% endunless %}
+
+    {% if product.selected_variant == variant %}
+      checked
+    {% endif %}
+  >
+</label>

test/issue-158/index.liquid

@@ -0,0 +1,22 @@
+It should maintain at most 1 newline between attributes, and none at the end
+<label class="block relative">
+  <input
+    class="absolute inset-0 opacity-0 pointer-events-none peer"
+    type="radio"
+    name="id"
+
+
+    id="variant-{{ variant.id }}"
+    value="{{ variant.id }}"
+    required
+
+    {% unless variant.available %}
+      disabled
+    {% endunless %}
+
+    {% if product.selected_variant == variant %}
+      checked
+    {% endif %}
+
+  >
+</label>

test/issue-158/index.spec.ts

@@ -0,0 +1,6 @@
+import { assertFormattedEqualsFixed } from '../test-helpers';
+import * as path from 'path';
+
+describe(`Unit: ${path.basename(__dirname)}`, () => {
+  assertFormattedEqualsFixed(__dirname);
+});