Merge pull request #269 from piegamesde/final-polish

Final polish

src/Nixfmt/Parser.hs

@@ -292,6 +292,7 @@ stripIndentation parts = case commonIndentation $ mapMaybe lineHead parts of
   Nothing -> map (const []) parts
   Just indentation -> map (stripParts indentation) parts
 
+-- Merge adjacent string parts which resulted as parsing artifacts
 normalizeLine :: [StringPart] -> [StringPart]
 normalizeLine [] = []
 normalizeLine (TextPart "" : xs) = normalizeLine xs

src/Nixfmt/Pretty.hs

@@ -62,6 +62,7 @@ import Nixfmt.Types (
   mapLastToken',
   tokenText,
  )
+import Nixfmt.Util (isSpaces)
 import Prelude hiding (String)
 
 toLineComment :: TrailingComment -> Trivium
@@ -412,7 +413,10 @@ prettyApp indentFunction pre hasPost f a =
               <> pretty parclose
         where
           -- If the brackets are on different lines, keep them like that
-          sur = if sourceLine paropen /= sourceLine parclose then hardline else line
+          sur
+            | sourceLine paropen /= sourceLine parclose = hardline
+            | null $ unItems items = hardspace
+            | otherwise = line
       absorbInner expr = pretty expr
 
       -- Render the last argument of a function call
@@ -536,6 +540,12 @@ isAbsorbable (Path _) = True
 -- Non-empty sets and lists
 isAbsorbable (Set _ _ (Items (_ : _)) _) = True
 isAbsorbable (List _ (Items (_ : _)) _) = True
+-- Empty sets and lists if they have a line break
+-- https://github.com/NixOS/nixfmt/issues/253
+isAbsorbable (Set _ (Ann{sourceLine = line1}) (Items []) (Ann{sourceLine = line2}))
+  | line1 /= line2 = True
+isAbsorbable (List (Ann{sourceLine = line1}) (Items []) (Ann{sourceLine = line2}))
+  | line1 /= line2 = True
 isAbsorbable (Parenthesized (LoneAnn _) (Term t) _) = isAbsorbable t
 isAbsorbable _ = False
 
@@ -599,7 +609,11 @@ absorbRHS expr = case expr of
   -- 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 ->
+    | 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 ->
         group' RegularG $ line <> group' Priority (prettyTermWide t) <> line <> pretty op <> hardspace <> pretty b
   -- Case 2a: LHS fits onto first line, RHS is an absorbable term
   (Operation l (LoneAnn op) (Term t))
@@ -768,6 +782,9 @@ isSimple (Term (SimpleString (LoneAnn _))) = True
 isSimple (Term (IndentedString (LoneAnn _))) = True
 isSimple (Term (Path (LoneAnn _))) = True
 isSimple (Term (Token (LoneAnn (Identifier _)))) = True
+isSimple (Term (Token (LoneAnn (Integer _)))) = True
+isSimple (Term (Token (LoneAnn (Float _)))) = True
+isSimple (Term (Token (LoneAnn (EnvPath _)))) = True
 isSimple (Term (Selection t selectors def)) =
   isSimple (Term t) && all isSimpleSelector selectors && isNothing def
 isSimple (Term (Parenthesized (LoneAnn _) e (LoneAnn _))) = isSimple e
@@ -805,11 +822,13 @@ instance Pretty StringPart where
           (unexpandSpacing' (Just 30) whole')
 
 instance Pretty [StringPart] where
-  -- When the interpolation is the only thing on the string line,
+  -- When the interpolation is the only thing on the string line (ignoring leading whitespace),
   -- then absorb the content (i.e. don't surround with line').
   -- Only do this when there are no comments
-  pretty [Interpolation (Whole expr [])] =
-    group $ text "${" <> nest inner <> text "}"
+  pretty [Interpolation (Whole expr [])] = pretty [TextPart "", Interpolation (Whole expr [])]
+  pretty [TextPart pre, Interpolation (Whole expr [])]
+    | isSpaces pre =
+        text pre <> offset (textWidth pre) (group $ text "${" <> nest inner <> text "}")
     where
       -- Code copied over from parentheses. Could be factored out into a common function one day
       inner = case expr of

src/Nixfmt/Util.hs

@@ -12,7 +12,7 @@ module Nixfmt.Util (
 )
 where
 
-import Data.Char (isAlpha, isDigit, isSpace)
+import Data.Char (isAlpha, isDigit)
 import Data.Text as Text (
   Text,
   all,
@@ -73,7 +73,7 @@ commonPrefix a b =
 -- prefix of zero texts is infinite, represented as Nothing.
 commonIndentation :: [Text] -> Maybe Text
 commonIndentation [] = Nothing
-commonIndentation [x] = Just $ Text.takeWhile isSpace x
+commonIndentation [x] = Just $ Text.takeWhile (== ' ') x
 commonIndentation (x : y : xs) = commonIndentation (commonPrefix x y : xs)
 
 isSpaces :: Text -> Bool

test/diff/apply_with_lists/in.nix

@@ -75,4 +75,7 @@
       out = "20170512";
     }
   ] null)
+  # https://github.com/NixOS/nixfmt/issues/268 regression test. [] and {} should be treated the same
+  (defaultNullOpts.mkNullable (with types; either str (listOf str)) [] "Some example long text that causes the line to be too long.")
+  (defaultNullOpts.mkNullable (with types; either str (listOf str)) {} "Some example long text that causes the line to be too long.")
 ]

test/diff/apply_with_lists/out-pure.nix

@@ -19,18 +19,9 @@
     name
   )
   (replaceStrings [ "@" ":" "\\" "[" "]" ] [ "-" "-" "-" "" "" ])
-  (lists.removePrefix
-    [
-      1
-      2
-    ]
-    [ ]
-  )
+  (lists.removePrefix [ 1 2 ] [ ])
   (lists.removePrefix aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
-    [
-      1
-      2
-    ]
+    [ 1 2 ]
     [ ]
   )
   (builtins.replaceStrings [ "@NIX_STORE_VERITY@" ] [ partitionTypes.usr-verity ]
@@ -107,4 +98,11 @@
     ]
     null
   )
+  # https://github.com/NixOS/nixfmt/issues/268 regression test. [] and {} should be treated the same
+  (defaultNullOpts.mkNullable (
+    with types; either str (listOf str)
+  ) [ ] "Some example long text that causes the line to be too long.")
+  (defaultNullOpts.mkNullable (
+    with types; either str (listOf str)
+  ) { } "Some example long text that causes the line to be too long.")
 ]

test/diff/apply_with_lists/out.nix

@@ -19,18 +19,9 @@
     name
   )
   (replaceStrings [ "@" ":" "\\" "[" "]" ] [ "-" "-" "-" "" "" ])
-  (lists.removePrefix
-    [
-      1
-      2
-    ]
-    [ ]
-  )
+  (lists.removePrefix [ 1 2 ] [ ])
   (lists.removePrefix aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
-    [
-      1
-      2
-    ]
+    [ 1 2 ]
     [ ]
   )
   (builtins.replaceStrings [ "@NIX_STORE_VERITY@" ] [ partitionTypes.usr-verity ]
@@ -117,4 +108,11 @@
     ]
     null
   )
+  # https://github.com/NixOS/nixfmt/issues/268 regression test. [] and {} should be treated the same
+  (defaultNullOpts.mkNullable (
+    with types; either str (listOf str)
+  ) [ ] "Some example long text that causes the line to be too long.")
+  (defaultNullOpts.mkNullable (
+    with types; either str (listOf str)
+  ) { } "Some example long text that causes the line to be too long.")
 ]

test/diff/attr_set/in.nix

@@ -274,4 +274,21 @@
         pkgs.xorg.fontadobe75dpi
       ];
   }
+  # Regression https://github.com/NixOS/nixfmt/issues/253
+  {
+    foo1 = {
+    };
+    foo2 = bar {
+    };
+    foo3 = bar {
+    } {
+    };
+    foo4 = [
+    ];
+    foo5 = bar [
+    ];
+    foo6 = bar [
+    ] [
+    ];
+  }
 ]

test/diff/attr_set/out-pure.nix

@@ -378,4 +378,13 @@
         pkgs.xorg.fontadobe75dpi
       ];
   }
+  # Regression https://github.com/NixOS/nixfmt/issues/253
+  {
+    foo1 = { };
+    foo2 = bar { };
+    foo3 = bar { } { };
+    foo4 = [ ];
+    foo5 = bar [ ];
+    foo6 = bar [ ] [ ];
+  }
 ]

test/diff/attr_set/out.nix

@@ -381,4 +381,27 @@
         pkgs.xorg.fontadobe75dpi
       ];
   }
+  # Regression https://github.com/NixOS/nixfmt/issues/253
+  {
+    foo1 = {
+    };
+    foo2 = bar {
+    };
+    foo3 =
+      bar
+        {
+        }
+        {
+        };
+    foo4 = [
+    ];
+    foo5 = bar [
+    ];
+    foo6 =
+      bar
+        [
+        ]
+        [
+        ];
+  }
 ]

test/diff/idioms_nixos_2/out-pure.nix

@@ -51,7 +51,8 @@ let
       post_max_size = cfg.maxUploadSize;
       memory_limit = cfg.maxUploadSize;
     }
-    // cfg.phpOptions // optionalAttrs cfg.caching.apcu { "apc.enable_cli" = "1"; };
+    // cfg.phpOptions
+    // optionalAttrs cfg.caching.apcu { "apc.enable_cli" = "1"; };
 
   occ = pkgs.writeScriptBin "nextcloud-occ" ''
     #! ${pkgs.runtimeShell}
@@ -839,11 +840,9 @@ in
                     'use_ssl' => ${boolToString s3.useSsl},
                     ${optionalString (s3.region != null) "'region' => '${s3.region}',"}
                     'use_path_style' => ${boolToString s3.usePathStyle},
-                    ${
-                      optionalString (
-                        s3.sseCKeyFile != null
-                      ) "'sse_c_key' => nix_read_secret('${s3.sseCKeyFile}'),"
-                    }
+                    ${optionalString (
+                      s3.sseCKeyFile != null
+                    ) "'sse_c_key' => nix_read_secret('${s3.sseCKeyFile}'),"}
                   ],
                 ]
               '';
@@ -885,9 +884,8 @@ in
               }
               $CONFIG = [
                 'apps_paths' => [
-                  ${
-                    optionalString (cfg.extraApps != { })
-                      "[ 'path' => '${cfg.home}/nix-apps', 'url' => '/nix-apps', 'writable' => false ],"
+                  ${optionalString (cfg.extraApps != { })
+                    "[ 'path' => '${cfg.home}/nix-apps', 'url' => '/nix-apps', 'writable' => false ],"
                   }
                   [ 'path' => '${cfg.home}/apps', 'url' => '/apps', 'writable' => false ],
                   [ 'path' => '${cfg.home}/store-apps', 'url' => '/store-apps', 'writable' => true ],
@@ -898,37 +896,29 @@ in
                 ${optionalString cfg.caching.apcu "'memcache.local' => '\\OC\\Memcache\\APCu',"}
                 'log_type' => '${cfg.logType}',
                 'loglevel' => '${builtins.toString cfg.logLevel}',
-                ${
-                  optionalString (
-                    c.overwriteProtocol != null
-                  ) "'overwriteprotocol' => '${c.overwriteProtocol}',"
-                }
+                ${optionalString (
+                  c.overwriteProtocol != null
+                ) "'overwriteprotocol' => '${c.overwriteProtocol}',"}
                 ${optionalString (c.dbname != null) "'dbname' => '${c.dbname}',"}
                 ${optionalString (c.dbhost != null) "'dbhost' => '${c.dbhost}',"}
                 ${optionalString (c.dbport != null) "'dbport' => '${toString c.dbport}',"}
                 ${optionalString (c.dbuser != null) "'dbuser' => '${c.dbuser}',"}
-                ${
-                  optionalString (
-                    c.dbtableprefix != null
-                  ) "'dbtableprefix' => '${toString c.dbtableprefix}',"
-                }
-                ${
-                  optionalString (c.dbpassFile != null) ''
-                    'dbpassword' => nix_read_secret(
-                      "${c.dbpassFile}"
-                    ),
-                  ''
-                }
+                ${optionalString (
+                  c.dbtableprefix != null
+                ) "'dbtableprefix' => '${toString c.dbtableprefix}',"}
+                ${optionalString (c.dbpassFile != null) ''
+                  'dbpassword' => nix_read_secret(
+                    "${c.dbpassFile}"
+                  ),
+                ''}
                 'dbtype' => '${c.dbtype}',
                 'trusted_domains' => ${
                   writePhpArray ([ cfg.hostName ] ++ c.extraTrustedDomains)
                 },
                 'trusted_proxies' => ${writePhpArray (c.trustedProxies)},
-                ${
-                  optionalString (
-                    c.defaultPhoneRegion != null
-                  ) "'default_phone_region' => '${c.defaultPhoneRegion}',"
-                }
+                ${optionalString (
+                  c.defaultPhoneRegion != null
+                ) "'default_phone_region' => '${c.defaultPhoneRegion}',"}
                 ${optionalString (nextcloudGreaterOrEqualThan "23") "'profile.enabled' => ${boolToString cfg.globalProfiles},"}
                 ${objectstoreConfig}
               ];