Better multi-list argument formatting

NixOS · Oct 1, 2024 · 80240ea · 80240ea
1 parent 228f526
commit 80240ea
Show file tree

Hide file tree

Showing 6 changed files with 54 additions and 45 deletions.
diff --git a/src/Nixfmt/Pretty.hs b/src/Nixfmt/Pretty.hs
@@ -378,18 +378,24 @@ instance Pretty Parameter where
 prettyApp :: Bool -> Doc -> Bool -> Expression -> Expression -> Doc
 prettyApp indentFunction pre hasPost f a =
   let -- Walk the function call chain
-      absorbApp :: Expression -> Doc
+      absorbApp :: Bool -> Expression -> Doc
       -- This is very hacky, but selections shouldn't be in a priority group,
       -- because if they get expanded before anything else,
       -- only the `.`-and-after part gets to a new line, which looks very odd
-      absorbApp (Application f' a'@(Term Selection{})) = group' Transparent (absorbApp f') <> line <> nest (group' RegularG $ absorbInner a')
-      absorbApp (Application f' a') = group' Transparent (absorbApp f') <> line <> nest (group' Priority $ absorbInner a')
+      absorbApp _ (Application f' a'@(Term Selection{})) =
+        group' Transparent (absorbApp False f') <> line <> nest (group' RegularG $ absorbInner a')
+      absorbApp nextIsAList (Application f' a'@(Term List{})) =
+        group' Transparent (absorbApp True f') <> sep <> nest (group' Priority $ absorbInner a')
+        where
+          sep = if nextIsAList then hardline else line
+      absorbApp _ (Application f' a') =
+        group' Transparent (absorbApp False f') <> line <> nest (group' Priority $ absorbInner a')
       -- First argument
-      absorbApp expr
+      absorbApp _ expr
         | indentFunction && null comment' = nest $ group' RegularG $ line' <> pretty expr
         | otherwise = pretty expr
 
-      isSimpleItems items = length (unItems items) <= 4 && all (isSimple . Term) items
+      isSimpleItems = all (isSimple . Term)
 
       -- Render the inner arguments of a function call
       absorbInner :: Expression -> Doc
@@ -400,14 +406,14 @@ prettyApp indentFunction pre hasPost f a =
       absorbInner expr = pretty expr
 
       -- Render the last argument of a function call
-      absorbLast :: Expression -> Doc
+      absorbLast :: Expression -> (Bool, Doc)
       -- If the list is simple, try to render it single-line instead of expanding
       absorbLast (Term (List paropen items parclose))
         | isSimpleItems items =
-            group' Priority $ nest $ prettyList line paropen items parclose
+            (True, group' Priority $ nest $ prettyList line paropen items parclose)
       absorbLast (Term t)
         | isAbsorbable t =
-            group' Priority $ nest $ prettyTerm t
+            (False, group' Priority $ nest $ prettyTerm t)
       -- Special case: Absorb parenthesized function declaration with absorbable body
       absorbLast
         ( Term
@@ -418,14 +424,16 @@ prettyApp indentFunction pre hasPost f a =
               )
           )
           | isAbsorbableTerm body && not (any hasTrivia [open, name, colon]) =
-              group' Priority $
-                nest $
-                  pretty open
-                    <> pretty name
-                    <> pretty colon
-                    <> hardspace
-                    <> prettyTermWide body
-                    <> pretty close
+              ( False,
+                group' Priority $
+                  nest $
+                    pretty open
+                      <> pretty name
+                      <> pretty colon
+                      <> hardspace
+                      <> prettyTermWide body
+                      <> pretty close
+              )
       -- Special case: Absorb parenthesized function application with absorbable body
       absorbLast
         ( Term
@@ -436,31 +444,35 @@ prettyApp indentFunction pre hasPost f a =
               )
           )
           | isAbsorbableTerm body && not (any hasTrivia [open, ident, close]) =
-              group' Priority $
-                nest $
-                  pretty open
-                    <> pretty fn
-                    <> hardspace
-                    <> prettyTermWide body
-                    <> pretty close
+              ( False,
+                group' Priority $
+                  nest $
+                    pretty open
+                      <> pretty fn
+                      <> hardspace
+                      <> prettyTermWide body
+                      <> pretty close
+              )
       absorbLast (Term (Parenthesized open expr close)) =
-        absorbParen open expr close
-      absorbLast arg = group' RegularG $ nest $ pretty arg
+        (False, absorbParen open expr close)
+      absorbLast arg = (False, group' RegularG $ nest $ pretty arg)
 
       -- Extract comment before the first function and move it out, to prevent functions being force-expanded
       (fWithoutComment, comment') =
         mapFirstToken'
           ((\a'@Ann{preTrivia} -> (a'{preTrivia = []}, preTrivia)) . moveTrailingCommentUp)
           f
 
-      renderedF = pre <> group' Transparent (absorbApp fWithoutComment)
+      renderedF = pre <> group' Transparent (absorbApp endsWithAList fWithoutComment)
       renderedFUnexpanded = unexpandSpacing' Nothing renderedF
 
       post = if hasPost then line' else mempty
+
+      (endsWithAList, lastAbsorbed) = absorbLast a
   in pretty comment'
       <> ( if isSimple (Application f a) && isJust renderedFUnexpanded
-            then group' RegularG $ fromJust renderedFUnexpanded <> hardspace <> absorbLast a
-            else group' RegularG $ renderedF <> line <> absorbLast a <> post
+            then group' RegularG $ fromJust renderedFUnexpanded <> hardspace <> lastAbsorbed
+            else group' RegularG $ renderedF <> line <> lastAbsorbed <> post
          )
       <> (if hasPost && not (null comment') then hardline else mempty)
 

diff --git a/test/diff/apply/out-pure.nix b/test/diff/apply/out-pure.nix
@@ -156,7 +156,10 @@
   }
   {
     escapeSingleline = libStr.escape [ "\\" ''"'' "\${" ];
-    escapeMultiline = libStr.replaceStrings [ "\${" "''" ] [ "''\${" "'''" ];
+    escapeMultiline =
+      libStr.replaceStrings
+        [ "\${" "''" ]
+        [ "''\${" "'''" ];
     test =
       foo
         [

diff --git a/test/diff/idioms_lib_3/out-pure.nix b/test/diff/idioms_lib_3/out-pure.nix
@@ -362,7 +362,10 @@ rec {
           let
             lines = filter (v: !isList v) (builtins.split "\n" v);
             escapeSingleline = libStr.escape [ "\\" ''"'' "\${" ];
-            escapeMultiline = libStr.replaceStrings [ "\${" "''" ] [ "''\${" "'''" ];
+            escapeMultiline =
+              libStr.replaceStrings
+                [ "\${" "''" ]
+                [ "''\${" "'''" ];
             singlelineResult =
               ''"'' + concatStringsSep "\\n" (map escapeSingleline lines) + ''"'';
             multilineResult =

diff --git a/test/diff/idioms_lib_3/out.nix b/test/diff/idioms_lib_3/out.nix
@@ -375,7 +375,10 @@ rec {
           let
             lines = filter (v: !isList v) (builtins.split "\n" v);
             escapeSingleline = libStr.escape [ "\\" ''"'' "\${" ];
-            escapeMultiline = libStr.replaceStrings [ "\${" "''" ] [ "''\${" "'''" ];
+            escapeMultiline =
+              libStr.replaceStrings
+                [ "\${" "''" ]
+                [ "''\${" "'''" ];
             singlelineResult =
               ''"'' + concatStringsSep "\\n" (map escapeSingleline lines) + ''"'';
             multilineResult =

diff --git a/test/diff/idioms_lib_4/out-pure.nix b/test/diff/idioms_lib_4/out-pure.nix
@@ -782,13 +782,7 @@ rec {
         # cpu-vendor-os
         else if
           elemAt l 1 == "apple"
-          || elem (elemAt l 2) [
-            "wasi"
-            "redox"
-            "mmixware"
-            "ghcjs"
-            "mingw32"
-          ]
+          || elem (elemAt l 2) [ "wasi" "redox" "mmixware" "ghcjs" "mingw32" ]
           || hasPrefix "freebsd" (elemAt l 2)
           || hasPrefix "netbsd" (elemAt l 2)
           || hasPrefix "genode" (elemAt l 2)

diff --git a/test/diff/idioms_lib_4/out.nix b/test/diff/idioms_lib_4/out.nix
@@ -782,13 +782,7 @@ rec {
         # cpu-vendor-os
         else if
           elemAt l 1 == "apple"
-          || elem (elemAt l 2) [
-            "wasi"
-            "redox"
-            "mmixware"
-            "ghcjs"
-            "mingw32"
-          ]
+          || elem (elemAt l 2) [ "wasi" "redox" "mmixware" "ghcjs" "mingw32" ]
           || hasPrefix "freebsd" (elemAt l 2)
           || hasPrefix "netbsd" (elemAt l 2)
           || hasPrefix "genode" (elemAt l 2)