From a7005b0c500432549e2aa94fd817f5ff9b529f05 Mon Sep 17 00:00:00 2001
From: Rebecca Turner <rbt@sent.as>
Date: Tue, 21 Feb 2023 17:48:35 -0800
Subject: [PATCH] Don't add blank lines after comments in `inherit`

Fixes #376

Note that handling of extra newlines in `inherit` statements is still a
little janky; usually blank lines will be removed from formatted output,
but in some cases (e.g. between an end-of-line comment and a full-line
comment) they will be preserved. See the tests added for a few examples
of this.
---
 src/alejandra/src/rules/inherit.rs            | 22 ++++++++++--
 src/alejandra/tests/cases/inherit/out         |  4 +--
 .../tests/cases/inherit_blank_trailing/in     | 34 +++++++++++++++++++
 .../tests/cases/inherit_blank_trailing/out    | 31 +++++++++++++++++
 src/alejandra/tests/cases/inherit_comment/in  |  7 ++++
 src/alejandra/tests/cases/inherit_comment/out |  7 ++++
 6 files changed, 101 insertions(+), 4 deletions(-)
 create mode 100644 src/alejandra/tests/cases/inherit_blank_trailing/in
 create mode 100644 src/alejandra/tests/cases/inherit_blank_trailing/out
 create mode 100644 src/alejandra/tests/cases/inherit_comment/in
 create mode 100644 src/alejandra/tests/cases/inherit_comment/out

diff --git a/src/alejandra/src/rules/inherit.rs b/src/alejandra/src/rules/inherit.rs
index 981ae8a..4171618 100644
--- a/src/alejandra/src/rules/inherit.rs
+++ b/src/alejandra/src/rules/inherit.rs
@@ -53,7 +53,15 @@ pub(crate) fn rule(
                 steps.push_back(crate::builder::Step::Whitespace);
                 steps.push_back(crate::builder::Step::Comment(text));
                 steps.push_back(crate::builder::Step::NewLine);
-                steps.push_back(crate::builder::Step::Pad);
+                // Only add padding if there are no `trivialities` (that is, there's no extra
+                // `Newlines(_)` to be added) or if the first one is a comment (that is, it'll need
+                // to be indented to match the content).
+                if matches!(
+                    child.trivialities.front(),
+                    None | Some(crate::children2::Trivia::Comment(_))
+                ) {
+                    steps.push_back(crate::builder::Step::Pad);
+                }
             } else if (not_last_child && !child.has_trivialities)
                 || matches!(
                     child.trivialities.front(),
@@ -64,10 +72,20 @@ pub(crate) fn rule(
                 steps.push_back(crate::builder::Step::Pad);
             }
 
-            for trivia in child.trivialities {
+            let mut trivia_iter = child.trivialities.into_iter().peekable();
+            while let Some(trivia) = trivia_iter.next() {
                 match trivia {
                     crate::children2::Trivia::Comment(text) => {
                         steps.push_back(crate::builder::Step::Comment(text));
+                        // If the next `trivia` is a newline, don't add newlines and padding at the
+                        // end of this iteration, as it will lead to a new blank line in the
+                        // output.
+                        if matches!(
+                            trivia_iter.peek(),
+                            Some(crate::children2::Trivia::Newlines(_))
+                        ) {
+                            continue;
+                        }
                     }
                     crate::children2::Trivia::Newlines(_) => {}
                 }
diff --git a/src/alejandra/tests/cases/inherit/out b/src/alejandra/tests/cases/inherit/out
index 4277bd1..c3b983d 100644
--- a/src/alejandra/tests/cases/inherit/out
+++ b/src/alejandra/tests/cases/inherit/out
@@ -89,11 +89,11 @@
   {
     inherit # test
       a # test
-      
+
       b # test
       c # test
       d # test
-      
+
       e
       f
       g
diff --git a/src/alejandra/tests/cases/inherit_blank_trailing/in b/src/alejandra/tests/cases/inherit_blank_trailing/in
new file mode 100644
index 0000000..a4c6f1a
--- /dev/null
+++ b/src/alejandra/tests/cases/inherit_blank_trailing/in
@@ -0,0 +1,34 @@
+[
+  {
+    inherit # test
+      a # test
+
+      b # test
+      c # test
+      d # test
+
+      e
+      f
+
+      g
+      h
+      ;
+  }
+  {
+    inherit
+      a # mixed trivialities
+
+      # comment 1
+      # comment 2
+
+      # comment 3 after blanks
+      b # multiple newlines
+
+
+      c # multiple comments
+      # comment 1
+      # comment 2
+      # comment 3
+      ;
+  }
+]
diff --git a/src/alejandra/tests/cases/inherit_blank_trailing/out b/src/alejandra/tests/cases/inherit_blank_trailing/out
new file mode 100644
index 0000000..1299d9a
--- /dev/null
+++ b/src/alejandra/tests/cases/inherit_blank_trailing/out
@@ -0,0 +1,31 @@
+[
+  {
+    inherit # test
+      a # test
+
+      b # test
+      c # test
+      d # test
+
+      e
+      f
+      g
+      h
+      ;
+  }
+  {
+    inherit
+      a # mixed trivialities
+
+      # comment 1
+      # comment 2
+      # comment 3 after blanks
+      b # multiple newlines
+
+      c # multiple comments
+      # comment 1
+      # comment 2
+      # comment 3
+      ;
+  }
+]
diff --git a/src/alejandra/tests/cases/inherit_comment/in b/src/alejandra/tests/cases/inherit_comment/in
new file mode 100644
index 0000000..1d473f2
--- /dev/null
+++ b/src/alejandra/tests/cases/inherit_comment/in
@@ -0,0 +1,7 @@
+{
+  inherit # eeby deeby
+    a
+    # b
+    c
+    ;
+}
diff --git a/src/alejandra/tests/cases/inherit_comment/out b/src/alejandra/tests/cases/inherit_comment/out
new file mode 100644
index 0000000..1d473f2
--- /dev/null
+++ b/src/alejandra/tests/cases/inherit_comment/out
@@ -0,0 +1,7 @@
+{
+  inherit # eeby deeby
+    a
+    # b
+    c
+    ;
+}