Don't expand lists within functions

NixOS · Aug 17, 2024 · 9b11e90 · 9b11e90
1 parent 2d79e0f
commit 9b11e90
Show file tree

Hide file tree

Showing 9 changed files with 81 additions and 167 deletions.
diff --git a/src/Nixfmt/Pretty.hs b/src/Nixfmt/Pretty.hs
@@ -375,11 +375,29 @@ instance Pretty Parameter where
 --    then start on a new line instead".
 prettyApp :: Bool -> Doc -> Bool -> Expression -> Expression -> Doc
 prettyApp indentFunction pre hasPost f a =
-  let absorbApp (Application f' a') = group' Transparent (absorbApp f') <> line <> nest (group' Priority a')
+  let -- Walk the function call chain
+      absorbApp :: Expression -> Doc
+      absorbApp (Application f' a') = group' Transparent (absorbApp f') <> line <> nest (group' Priority $ absorbInner a')
+      -- First argument
       absorbApp expr
         | indentFunction && null comment' = nest $ group' RegularG $ line' <> pretty expr
         | otherwise = pretty expr
 
+      -- Render the inner arguments of a function call
+      absorbInner :: Expression -> Doc
+      -- If lists have only simple items, try to render them single-line instead of expanding
+      absorbInner (Term (List paropen@Ann{trailComment = post'} items parclose))
+        | length (unItems items) <= 4 && all isSimple (Term <$> items) =
+            pretty (paropen{trailComment = Nothing})
+              <> surroundWith sur (nest $ pretty post' <> sepBy line (unItems items))
+              <> pretty parclose
+        where
+          -- If the brackets are on different lines, keep them like that
+          sur = if sourceLine paropen /= sourceLine parclose then hardline else line
+      absorbInner expr = pretty expr
+
+      -- Render the last argument of a function call
+      absorbLast :: Expression -> Doc
       absorbLast (Term t)
         | isAbsorbable t =
             group' Priority $ nest $ prettyTerm t

diff --git a/src/Nixfmt/Types.hs b/src/Nixfmt/Types.hs
@@ -122,7 +122,7 @@ data Item a
     Comments Trivia
   deriving (Foldable, Show, Functor)
 
-newtype Items a = Items {unItems :: [Item a]} deriving (Functor)
+newtype Items a = Items {unItems :: [Item a]} deriving (Functor, Foldable)
 
 instance (Eq a) => Eq (Items a) where
   (==) = (==) `on` concatMap Data.Foldable.toList . unItems

diff --git a/test/diff/apply/in.nix b/test/diff/apply/in.nix
@@ -65,7 +65,7 @@
       mapAttrsToStringsSep "\n" mkSection attrsOfAttrs;
   }
   [
-    (mapAttrsToStringsSep [force long] "\n" mkSection attrsOfAttrs)
+    (mapAttrsToStringsSep [force /* meow */ long] "\n" mkSection attrsOfAttrs)
   ]
   (a
   b)

diff --git a/test/diff/apply/out-pure.nix b/test/diff/apply/out-pure.nix
@@ -71,7 +71,7 @@
   }
   [
     (mapAttrsToStringsSep [
-      force
+      force # meow
       long
     ] "\n" mkSection attrsOfAttrs)
   ]
@@ -160,16 +160,10 @@
       ''"''
       "\${"
     ];
-    escapeMultiline =
-      libStr.replaceStrings
-        [
-          "\${"
-          "''"
-        ]
-        [
-          "''\${"
-          "'''"
-        ];
+    escapeMultiline = libStr.replaceStrings [ "\${" "''" ] [
+      "''\${"
+      "'''"
+    ];
     test =
       foo
         [

diff --git a/test/diff/apply/out.nix b/test/diff/apply/out.nix
@@ -71,7 +71,7 @@
   }
   [
     (mapAttrsToStringsSep [
-      force
+      force # meow
       long
     ] "\n" mkSection attrsOfAttrs)
   ]

diff --git a/test/diff/idioms_lib_3/out-pure.nix b/test/diff/idioms_lib_3/out-pure.nix
@@ -130,13 +130,7 @@ rec {
   toINI =
     {
       # apply transformations (e.g. escapes) to section names
-      mkSectionName ? (
-        name:
-        libStr.escape [
-          "["
-          "]"
-        ] name
-      ),
+      mkSectionName ? (name: libStr.escape [ "[" "]" ] name),
       # format a setting line from key and value
       mkKeyValue ? mkKeyValueDefault { } "=",
       # allow lists as values for duplicate keys
@@ -191,13 +185,7 @@ rec {
   toINIWithGlobalSection =
     {
       # apply transformations (e.g. escapes) to section names
-      mkSectionName ? (
-        name:
-        libStr.escape [
-          "["
-          "]"
-        ] name
-      ),
+      mkSectionName ? (name: libStr.escape [ "[" "]" ] name),
       # format a setting line from key and value
       mkKeyValue ? mkKeyValueDefault { } "=",
       # allow lists as values for duplicate keys
@@ -378,16 +366,10 @@ rec {
               ''"''
               "\${"
             ];
-            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
@@ -133,13 +133,7 @@ rec {
   toINI =
     {
       # apply transformations (e.g. escapes) to section names
-      mkSectionName ? (
-        name:
-        libStr.escape [
-          "["
-          "]"
-        ] name
-      ),
+      mkSectionName ? (name: libStr.escape [ "[" "]" ] name),
       # format a setting line from key and value
       mkKeyValue ? mkKeyValueDefault { } "=",
       # allow lists as values for duplicate keys
@@ -194,13 +188,7 @@ rec {
   toINIWithGlobalSection =
     {
       # apply transformations (e.g. escapes) to section names
-      mkSectionName ? (
-        name:
-        libStr.escape [
-          "["
-          "]"
-        ] name
-      ),
+      mkSectionName ? (name: libStr.escape [ "[" "]" ] name),
       # format a setting line from key and value
       mkKeyValue ? mkKeyValueDefault { } "=",
       # allow lists as values for duplicate keys
@@ -391,16 +379,10 @@ rec {
               ''"''
               "\${"
             ];
-            escapeMultiline =
-              libStr.replaceStrings
-                [
-                  "\${"
-                  "''"
-                ]
-                [
-                  "''\${"
-                  "'''"
-                ];
+            escapeMultiline = libStr.replaceStrings [ "\${" "''" ] [
+              "''\${"
+              "'''"
+            ];
             singlelineResult =
               ''"'' + concatStringsSep "\\n" (map escapeSingleline lines) + ''"'';
             multilineResult =

diff --git a/test/diff/idioms_nixos_2/out-pure.nix b/test/diff/idioms_nixos_2/out-pure.nix
@@ -72,58 +72,27 @@ in
 {
 
   imports = [
-    (mkRemovedOptionModule
-      [
-        "services"
-        "nextcloud"
-        "config"
-        "adminpass"
-      ]
-      ''
-        Please use `services.nextcloud.config.adminpassFile' instead!
-      ''
-    )
-    (mkRemovedOptionModule
-      [
-        "services"
-        "nextcloud"
-        "config"
-        "dbpass"
-      ]
-      ''
-        Please use `services.nextcloud.config.dbpassFile' instead!
-      ''
-    )
-    (mkRemovedOptionModule
-      [
-        "services"
-        "nextcloud"
-        "nginx"
-        "enable"
-      ]
-      ''
-        The nextcloud module supports `nginx` as reverse-proxy by default and doesn't
-        support other reverse-proxies officially.
-
-        However it's possible to use an alternative reverse-proxy by
-
-          * disabling nginx
-          * setting `listen.owner` & `listen.group` in the phpfpm-pool to a different value
-
-        Further details about this can be found in the `Nextcloud`-section of the NixOS-manual
-        (which can be opened e.g. by running `nixos-help`).
-      ''
-    )
-    (mkRemovedOptionModule
-      [
-        "services"
-        "nextcloud"
-        "disableImagemagick"
-      ]
-      ''
-        Use services.nextcloud.enableImagemagick instead.
-      ''
-    )
+    (mkRemovedOptionModule [ "services" "nextcloud" "config" "adminpass" ] ''
+      Please use `services.nextcloud.config.adminpassFile' instead!
+    '')
+    (mkRemovedOptionModule [ "services" "nextcloud" "config" "dbpass" ] ''
+      Please use `services.nextcloud.config.dbpassFile' instead!
+    '')
+    (mkRemovedOptionModule [ "services" "nextcloud" "nginx" "enable" ] ''
+      The nextcloud module supports `nginx` as reverse-proxy by default and doesn't
+      support other reverse-proxies officially.
+
+      However it's possible to use an alternative reverse-proxy by
+
+        * disabling nginx
+        * setting `listen.owner` & `listen.group` in the phpfpm-pool to a different value
+
+      Further details about this can be found in the `Nextcloud`-section of the NixOS-manual
+      (which can be opened e.g. by running `nixos-help`).
+    '')
+    (mkRemovedOptionModule [ "services" "nextcloud" "disableImagemagick" ] ''
+      Use services.nextcloud.enableImagemagick instead.
+    '')
   ];
 
   options.services.nextcloud = {

diff --git a/test/diff/idioms_nixos_2/out.nix b/test/diff/idioms_nixos_2/out.nix
@@ -75,58 +75,27 @@ in
 {
 
   imports = [
-    (mkRemovedOptionModule
-      [
-        "services"
-        "nextcloud"
-        "config"
-        "adminpass"
-      ]
-      ''
-        Please use `services.nextcloud.config.adminpassFile' instead!
-      ''
-    )
-    (mkRemovedOptionModule
-      [
-        "services"
-        "nextcloud"
-        "config"
-        "dbpass"
-      ]
-      ''
-        Please use `services.nextcloud.config.dbpassFile' instead!
-      ''
-    )
-    (mkRemovedOptionModule
-      [
-        "services"
-        "nextcloud"
-        "nginx"
-        "enable"
-      ]
-      ''
-        The nextcloud module supports `nginx` as reverse-proxy by default and doesn't
-        support other reverse-proxies officially.
-
-        However it's possible to use an alternative reverse-proxy by
-
-          * disabling nginx
-          * setting `listen.owner` & `listen.group` in the phpfpm-pool to a different value
-
-        Further details about this can be found in the `Nextcloud`-section of the NixOS-manual
-        (which can be opened e.g. by running `nixos-help`).
-      ''
-    )
-    (mkRemovedOptionModule
-      [
-        "services"
-        "nextcloud"
-        "disableImagemagick"
-      ]
-      ''
-        Use services.nextcloud.enableImagemagick instead.
-      ''
-    )
+    (mkRemovedOptionModule [ "services" "nextcloud" "config" "adminpass" ] ''
+      Please use `services.nextcloud.config.adminpassFile' instead!
+    '')
+    (mkRemovedOptionModule [ "services" "nextcloud" "config" "dbpass" ] ''
+      Please use `services.nextcloud.config.dbpassFile' instead!
+    '')
+    (mkRemovedOptionModule [ "services" "nextcloud" "nginx" "enable" ] ''
+      The nextcloud module supports `nginx` as reverse-proxy by default and doesn't
+      support other reverse-proxies officially.
+
+      However it's possible to use an alternative reverse-proxy by
+
+        * disabling nginx
+        * setting `listen.owner` & `listen.group` in the phpfpm-pool to a different value
+
+      Further details about this can be found in the `Nextcloud`-section of the NixOS-manual
+      (which can be opened e.g. by running `nixos-help`).
+    '')
+    (mkRemovedOptionModule [ "services" "nextcloud" "disableImagemagick" ] ''
+      Use services.nextcloud.enableImagemagick instead.
+    '')
   ];
 
   options.services.nextcloud = {