absorbRHS: Unindent concatenations

Author: piegamesde

src/Nixfmt/Pretty.hs

@@ -502,6 +502,34 @@ prettyApp indentFunction pre hasPost f a =
             (\fRendered -> group' RegularG $ fRendered <> line <> absorbLast a <> post)
       <> (if hasPost && not (null comment') then hardline else mempty)
 
+prettyOp :: Bool -> Expression -> Leaf -> Doc
+prettyOp forceFirstTermWide operation op =
+  let -- Walk the operation tree and put a list of things on the same level.
+      -- We still need to keep the operators around because they might have comments attached to them.
+      -- An operator is put together with its succeeding expression. Only the first operand has none.
+      flatten :: Maybe Leaf -> Expression -> [(Maybe Leaf, Expression)]
+      flatten opL (Operation a opR b) | opR == op = flatten opL a ++ flatten (Just opR) b
+      flatten opL x = [(opL, x)]
+
+      -- Called on every operand except the first one (a.k.a. RHS)
+      absorbOperation :: Expression -> Doc
+      absorbOperation (Term t) | isAbsorbable t = hardspace <> pretty t
+      -- Force nested operations to start on a new line
+      absorbOperation x@(Operation{}) = group' RegularG $ line <> pretty x
+      -- Force applications to start on a new line if more than the last argument is multiline
+      absorbOperation (Application f a) = group $ prettyApp False line False f a
+      absorbOperation x = hardspace <> pretty x
+
+      prettyOperation :: (Maybe Leaf, Expression) -> Doc
+      -- First element
+      prettyOperation (Nothing, Term t) | isAbsorbableTerm t && forceFirstTermWide = prettyTermWide t
+      prettyOperation (Nothing, expr) = pretty expr
+      -- The others
+      prettyOperation (Just op', expr) =
+        line <> pretty (moveTrailingCommentUp op') <> nest (absorbOperation expr)
+  in group' RegularG $
+      (concatMap prettyOperation . flatten Nothing) operation
+
 prettyWith :: Bool -> Expression -> Doc
 -- absorb the body
 prettyWith True (With with expr0 semicolon (Term expr1)) =
@@ -608,15 +636,14 @@ absorbRHS expr = case expr of
   -- Absorb if all arguments except the last fit into the line, start on new line otherwise
   (Application f a) -> nest $ prettyApp False line False f a
   (With{}) -> nest $ group' RegularG $ line <> pretty expr
-  -- Special case `//` and `++` operations to be more compact in some cases
-  -- Case 1: two arguments, LHS is absorbable term, RHS fits onto the last line
-  (Operation (Term t) (LoneAnn op) b)
-    | isAbsorbable t
-        && isUpdateOrConcat op
-        -- Exclude further operations on the RHS
-        -- Hotfix for https://github.com/NixOS/nixfmt/issues/198
-        && case b of (Operation{}) -> False; _ -> True ->
-        nest $ group' RegularG $ line <> group' Priority (prettyTermWide t) <> line <> pretty op <> hardspace <> pretty b
+  -- Special case `//` and `++` and `+` operations to be more compact in some cases
+  -- Case 1: LHS is absorbable term, unindent concatenations
+  -- https://github.com/NixOS/nixfmt/issues/228
+  (Operation (Term t) op@(Ann{value}) _)
+    | isAbsorbableTerm t
+        && matchFirstToken (\Ann{preTrivia} -> preTrivia == []) t
+        && elem value [TUpdate, TConcat, TPlus] ->
+        hardspace <> prettyOp True expr op
   -- Case 2a: LHS fits onto first line, RHS is an absorbable term
   (Operation l (LoneAnn op) (Term t))
     | isAbsorbable t && isUpdateOrConcat op ->
@@ -715,31 +742,7 @@ instance Pretty Expression where
     | op' == TLess || op' == TGreater || op' == TLessEqual || op' == TGreaterEqual || op' == TEqual || op' == TUnequal =
         pretty a <> softline <> pretty op <> hardspace <> pretty b
   -- all other operators
-  pretty operation@(Operation _ op _) =
-    let -- Walk the operation tree and put a list of things on the same level.
-        -- We still need to keep the operators around because they might have comments attached to them.
-        -- An operator is put together with its succeeding expression. Only the first operand has none.
-        flatten :: Maybe Leaf -> Expression -> [(Maybe Leaf, Expression)]
-        flatten opL (Operation a opR b) | opR == op = flatten opL a ++ flatten (Just opR) b
-        flatten opL x = [(opL, x)]
-
-        -- Called on every operand except the first one (a.k.a. RHS)
-        absorbOperation :: Expression -> Doc
-        absorbOperation (Term t) | isAbsorbable t = hardspace <> pretty t
-        -- Force nested operations to start on a new line
-        absorbOperation x@(Operation{}) = group' RegularG $ line <> pretty x
-        -- Force applications to start on a new line if more than the last argument is multiline
-        absorbOperation (Application f a) = group $ prettyApp False line False f a
-        absorbOperation x = hardspace <> pretty x
-
-        prettyOperation :: (Maybe Leaf, Expression) -> Doc
-        -- First element
-        prettyOperation (Nothing, expr) = pretty expr
-        -- The others
-        prettyOperation (Just op', expr) =
-          line <> pretty (moveTrailingCommentUp op') <> nest (absorbOperation expr)
-    in group' RegularG $
-        (concatMap prettyOperation . flatten Nothing) operation
+  pretty operation@(Operation _ op _) = prettyOp False operation op
   pretty (MemberCheck expr qmark sel) =
     pretty expr
       <> softline

standard.md

@@ -1282,6 +1282,27 @@ Notable special cases are:
       dependencies
     ];
   ```
+- Operator chains (specifically, `++`, `//` and `+`) where the first element is absorbable are treated specially to avoid diff churn. Assuming the following input:
+  ```nix
+  foo = [
+    bar
+  ];
+  ```
+  Normally (see "otherwise newline and indent" above), concatenating items onto that list would be formatted like this, causing the entire list to re-indent:
+  ```nix
+  foo =
+    [
+      bar
+    ]
+    ++ baz;
+  ```
+  Instead, we format it as such:
+  ```nix
+  foo = [
+    bar
+  ]
+  ++ baz;
+  ```
 
 **Alternatives**
 

test/diff/attr_set/out-pure.nix

@@ -131,22 +131,21 @@
           # several
           items
         ];
-    a =
-      [
-        some
-        flags # multiline
-      ]
-      ++ [ short ]
-      ++ [
-        more
-        stuff # multiline
-      ]
-      ++ (if foo then [ bar ] else [ baz ])
-      ++ [ ]
-      ++ (optionals condition [
-        more
-        items
-      ]);
+    a = [
+      some
+      flags # multiline
+    ]
+    ++ [ short ]
+    ++ [
+      more
+      stuff # multiline
+    ]
+    ++ (if foo then [ bar ] else [ baz ])
+    ++ [ ]
+    ++ (optionals condition [
+      more
+      items
+    ]);
     b = with pkgs; [
       a
       lot
@@ -276,34 +275,32 @@
           "${host_name}.lo.m-0.eu"
         ];
       }) secret-config.ssh-hosts;
-    programs.ssh.knownHosts9 =
-      {
-        multi = 1;
-        line = 2;
-      }
-      // lib.mapAttrs (
-        host_name: publicKey: {
-          inherit publicKey;
-          extraHostNames = [
-            "${host_name}.m-0.eu"
-            "${host_name}.vpn.m-0.eu"
-            "${host_name}.lo.m-0.eu"
-          ];
-        }
-      );
-    programs.ssh.knownHosts10 =
-      {
-        multi = 1;
-        line = 2;
-      }
-      // lib.mapAttrs (host_name: publicKey: {
+    programs.ssh.knownHosts9 = {
+      multi = 1;
+      line = 2;
+    }
+    // lib.mapAttrs (
+      host_name: publicKey: {
         inherit publicKey;
         extraHostNames = [
           "${host_name}.m-0.eu"
           "${host_name}.vpn.m-0.eu"
           "${host_name}.lo.m-0.eu"
         ];
-      }) secret-config.ssh-hosts;
+      }
+    );
+    programs.ssh.knownHosts10 = {
+      multi = 1;
+      line = 2;
+    }
+    // lib.mapAttrs (host_name: publicKey: {
+      inherit publicKey;
+      extraHostNames = [
+        "${host_name}.m-0.eu"
+        "${host_name}.vpn.m-0.eu"
+        "${host_name}.lo.m-0.eu"
+      ];
+    }) secret-config.ssh-hosts;
   }
 
   # Parentheses
@@ -395,17 +392,19 @@
       + ccccccccccccccccccccccccccccc
       + ddddddddddddddddddddddddddddd;
 
-    boot.kernelParams =
-      [ aaaaaaaaaaaaaa ]
-      ++ optionals config.boot.vesa [
-        "vga=0x317"
-        "nomodeset"
-      ];
+    boot.kernelParams = [
+      aaaaaaaaaaaaaa
+    ]
+    ++ optionals config.boot.vesa [
+      "vga=0x317"
+      "nomodeset"
+    ];
 
-    foo2 =
-      [ bar ]
-      ++ baz # newline!
-      ++ meow;
+    foo2 = [
+      bar
+    ]
+    ++ baz # newline!
+    ++ meow;
 
     foo3 =
       some function application that kinda is long # and
@@ -422,17 +421,21 @@
         a
         b
         c
-      ] ++ more stuff;
+      ]
+      ++ more stuff;
 
     foo4 # nasty
-      =  # comments
-        [ bar ]
-        ++ baz # newline!
-        ++ meow;
+      = # comments
+      [
+        bar
+      ]
+      ++ baz # newline!
+      ++ meow;
 
     environment.systemPackages =
       # Include the PAM modules in the system path mostly for the manpages.
-      [ package ] ++ lib.optional config.users.ldap.enable pam_ldap;
+      [ package ]
+      ++ lib.optional config.users.ldap.enable pam_ldap;
 
     environment.systemPackages2 =
       # Include the PAM modules in the system path mostly for the manpages.