fix(css_formatter): don't wrap selector identation after css comments

biomejs · Nov 22, 2024 · b171329 · b171329
1 parent 4848994
commit b171329
Show file tree

Hide file tree

Showing 7 changed files with 213 additions and 101 deletions.
diff --git a/crates/biome_css_formatter/src/css/lists/selector_list.rs b/crates/biome_css_formatter/src/css/lists/selector_list.rs
@@ -24,6 +24,37 @@ impl FormatRule<CssSelectorList> for FormatCssSelectorList {
         let mut joiner = f.join_with(&separator);
 
         for formatted in node.format_separated(",") {
+            let computed_selector =
+                formatted
+                    .node()?
+                    .as_css_complex_selector()
+                    .and_then(|complex_selector| {
+                        complex_selector
+                            .left()
+                            .ok()?
+                            .as_css_compound_selector()
+                            .cloned()
+                    });
+
+            if let Some(computed_selector) = computed_selector {
+                let simple_selector_has_leading_comments = computed_selector
+                    .simple_selector()
+                    .and_then(|simple_selector| simple_selector.as_css_type_selector().cloned())
+                    .and_then(|type_selector| type_selector.ident().ok()?.value_token().ok())
+                    .is_some_and(|value_token| value_token.has_leading_comments());
+
+                let sub_selector_has_leading_comments = computed_selector
+                    .sub_selectors()
+                    .first()
+                    .and_then(|sub_selector| sub_selector.as_css_class_selector().cloned())
+                    .and_then(|class_selector| class_selector.dot_token().ok())
+                    .is_some_and(|value_token| value_token.has_leading_comments());
+
+                if simple_selector_has_leading_comments || sub_selector_has_leading_comments {
+                    joiner.entry(&group(&formatted));
+                    continue;
+                }
+            }
             // Each selector gets `indent` added in case it breaks over multiple
             // lines. The break is added here rather than in each selector both
             // for simplicity and to avoid recursively adding indents when

diff --git a/crates/biome_css_formatter/src/css/selectors/complex_selector.rs b/crates/biome_css_formatter/src/css/selectors/complex_selector.rs
@@ -24,15 +24,42 @@ impl FormatNodeRule<CssComplexSelector> for FormatCssComplexSelector {
             }
         });
 
-        write!(
-            f,
-            [
-                left.format(),
-                soft_line_break_or_space(),
-                formatted_combinator,
-                space(),
-                right.format()
-            ]
-        )
+        let mut has_leading_comments = false;
+
+        if let Some(computed_selector) = left.clone()?.as_css_compound_selector() {
+            let simple_selector_has_leading_comments = computed_selector
+                .simple_selector()
+                .and_then(|simple_selector| simple_selector.as_css_type_selector().cloned())
+                .and_then(|type_selector| type_selector.ident().ok()?.value_token().ok())
+                .is_some_and(|value_token| value_token.has_leading_comments());
+
+            let sub_selector_has_leading_comments = computed_selector
+                .sub_selectors()
+                .first()
+                .and_then(|sub_selector| sub_selector.as_css_class_selector().cloned())
+                .and_then(|class_selector| class_selector.dot_token().ok())
+                .is_some_and(|value_token| value_token.has_leading_comments());
+
+            has_leading_comments =
+                simple_selector_has_leading_comments || sub_selector_has_leading_comments;
+        }
+
+        if has_leading_comments {
+            write!(
+                f,
+                [left.format(), formatted_combinator, space(), right.format()]
+            )
+        } else {
+            write!(
+                f,
+                [
+                    left.format(),
+                    soft_line_break_or_space(),
+                    formatted_combinator,
+                    space(),
+                    right.format()
+                ]
+            )
+        }
     }
 }
diff --git a/crates/biome_css_formatter/tests/quick_test.rs b/crates/biome_css_formatter/tests/quick_test.rs
@@ -13,13 +13,12 @@ mod language {
 // use this test check if your snippet prints as you wish, without using a snapshot
 fn quick_test() {
     let src = r#"
-#grid {
-	grid-template-columns:
-	  1fr /* first comment */
-	  auto /* second comment */
-      60px /* fourth comment */
-	  2fr /* third comment */  
-  }
+/* 1some medium long comment */
+.line1 selector,
+/* 2some medium long comment */
+.line1 selector {
+  background: red;
+}
 "#;
     let parse = parse_css(src, CssParserOptions::default());
     println!("{parse:#?}");

diff --git a/crates/biome_css_formatter/tests/specs/css/issue_3229.css.snap b/crates/biome_css_formatter/tests/specs/css/issue_3229.css.snap
@@ -1,7 +1,6 @@
 ---
 source: crates/biome_formatter_test/src/snapshot_builder.rs
 info: css/issue_3229.css
-snapshot_kind: text
 ---
 # Input
 
@@ -83,18 +82,16 @@ foo
 }
 
 /* input */
-foo
-	/* a comment */
+foo /* a comment */
 
-	.aRule {
+.aRule {
 	color: red;
 }
 
 /* first format */
-foo
-	/* a comment */
+foo /* a comment */
 
-	.aRule {
+.aRule {
 	color: red;
 }
 ```
diff --git a/crates/biome_css_formatter/tests/specs/css/selectors/selector_lists.css b/crates/biome_css_formatter/tests/specs/css/selectors/selector_lists.css
@@ -14,4 +14,28 @@ div.with.really.long#selector#content, div.another.really.long#selector.that.goe
 h1, h2     , h3
 
 
-, h4,    h5,      h6 {}
+, h4,    h5,      h6 {}
+
+
+/* 1some medium long comment */
+.line1 selector,
+/* 2some medium long comment */
+.line1 selector {
+  background: red;
+}
+
+/* 1some medium long comment */
+.line1 selector,
+/* 2some medium long comment */
+div selector {
+  background: blue;
+}
+
+/* 1some medium long comment */
+.line1 selector,
+/* 2some medium long comment */
+.line1 selector 
+/* 3some medium long comment */
+div selector {
+  background: green;
+}
diff --git a/crates/biome_css_formatter/tests/specs/css/selectors/selector_lists.css.snap b/crates/biome_css_formatter/tests/specs/css/selectors/selector_lists.css.snap
@@ -2,7 +2,6 @@
 source: crates/biome_formatter_test/src/snapshot_builder.rs
 info: css/selectors/selector_lists.css
 ---
-
 # Input
 
 ```css
@@ -23,6 +22,31 @@ h1, h2     , h3
 
 
 , h4,    h5,      h6 {}
+
+
+/* 1some medium long comment */
+.line1 selector,
+/* 2some medium long comment */
+.line1 selector {
+  background: red;
+}
+
+/* 1some medium long comment */
+.line1 selector,
+/* 2some medium long comment */
+div selector {
+  background: blue;
+}
+
+/* 1some medium long comment */
+.line1 selector,
+/* 2some medium long comment */
+.line1 selector 
+/* 3some medium long comment */
+div selector {
+  background: green;
+}
+
 ```
 
 
@@ -62,11 +86,33 @@ h4,
 h5,
 h6 {
 }
+
+/* 1some medium long comment */
+.line1 selector,
+/* 2some medium long comment */
+.line1 selector {
+	background: red;
+}
+
+/* 1some medium long comment */
+.line1 selector,
+/* 2some medium long comment */
+div selector {
+	background: blue;
+}
+
+/* 1some medium long comment */
+.line1 selector,
+/* 2some medium long comment */
+	.line1 selector
+	/* 3some medium long comment */
+	div
+	selector {
+	background: green;
+}
 ```
 
 # Lines exceeding max width of 80 characters
 ```
    12: div.another.really.long#selector.that.goes.past.the.line.length.with.a.single.selector {
 ```
-
-