Merge pull request #201 from NixOS/preserve-more-newlines

Preserve more newlines

src/Nixfmt/Parser.hs

@@ -341,26 +341,17 @@ term = label "term" $ do
     _ -> Selection t sel def
 
 items :: Parser a -> Parser (Items a)
-items p = Items <$> many (item p) <> (toList <$> optional lastItem)
+items p = Items <$> many (item p) <> (toList <$> optional itemComment)
 
 item :: Parser a -> Parser (Item a)
-item p = detachedComment <|> CommentedItem <$> takeTrivia <*> p
+item p = itemComment <|> Item <$> p
 
-lastItem :: Parser (Item a)
-lastItem = do
+itemComment :: Parser (Item a)
+itemComment = do
   trivia <- takeTrivia
   case trivia of
     [] -> empty
-    _ -> pure $ DetachedComments trivia
-
-detachedComment :: Parser (Item a)
-detachedComment = do
-  trivia <- takeTrivia
-  case break (== EmptyLine) trivia of
-    -- Return a set of comments that don't annotate the next item
-    (detached, EmptyLine : trivia') -> pushTrivia trivia' >> pure (DetachedComments detached)
-    -- The remaining trivia annotate the next item
-    _ -> pushTrivia trivia >> empty
+    _ -> pure $ Comments trivia
 
 -- ABSTRACTIONS
 

src/Nixfmt/Pretty.hs

@@ -91,23 +91,12 @@ instance Pretty Trivium where
         | otherwise = comment l <> hardline
 
 instance (Pretty a) => Pretty (Item a) where
-  pretty (DetachedComments trivia) = pretty trivia
-  pretty (CommentedItem trivia x) = pretty trivia <> group x
+  pretty (Comments trivia) = pretty trivia
+  pretty (Item x) = group x
 
 -- For lists, attribute sets and let bindings
-prettyItems :: (Pretty a) => Doc -> Items a -> Doc
--- Special case: Preserve an empty line with no items
--- usually, trailing newlines after the last element are not preserved
-prettyItems _ (Items [DetachedComments []]) = emptyline
-prettyItems sep items = prettyItems' $ unItems items
-  where
-    prettyItems' :: (Pretty a) => [Item a] -> Doc
-    prettyItems' [] = mempty
-    prettyItems' [item] = pretty item
-    prettyItems' (item : xs) =
-      pretty item
-        <> case item of CommentedItem _ _ -> sep; DetachedComments _ -> emptyline
-        <> prettyItems' xs
+prettyItems :: (Pretty a) => Items a -> Doc
+prettyItems (Items items) = sepBy hardline items
 
 instance Pretty [Trivium] where
   pretty [] = mempty
@@ -170,7 +159,7 @@ prettySet _ (krec, Ann [] paropen Nothing, Items [], parclose@(Ann [] _ _)) =
 prettySet wide (krec, Ann pre paropen post, binders, parclose) =
   pretty (fmap (,hardspace) krec)
     <> pretty (Ann pre paropen Nothing)
-    <> surroundWith sep (nest $ pretty post <> prettyItems hardline binders)
+    <> surroundWith sep (nest $ pretty post <> prettyItems binders)
     <> pretty parclose
   where
     sep = if wide && not (null (unItems binders)) then hardline else line
@@ -207,7 +196,7 @@ prettyTerm (List (Ann leading paropen Nothing) (Items []) (Ann [] parclose trail
 -- Always expand if len > 1
 prettyTerm (List (Ann pre paropen post) items parclose) =
   pretty (Ann pre paropen Nothing)
-    <> surroundWith line (nest $ pretty post <> prettyItems hardline items)
+    <> surroundWith line (nest $ pretty post <> prettyItems items)
     <> pretty parclose
 prettyTerm (Set krec paropen items parclose) = prettySet False (krec, paropen, items, parclose)
 -- Parentheses
@@ -561,14 +550,18 @@ instance Pretty Expression where
       (binderComments, bindersWithoutComments) =
         foldr
           ( \item (start, rest) -> case item of
-              (DetachedComments inner) | null rest -> (inner : start, rest)
+              (Comments inner)
+                | null rest ->
+                    -- Only move all non-empty-line trivia below the `in`
+                    let (comments, el) = break (== EmptyLine) (reverse inner)
+                    in (reverse comments : start, Comments (reverse el) : rest)
               _ -> (start, item : rest)
           )
           ([], [])
           (unItems binders)
 
       letPart = group $ pretty let_ <> hardline <> letBody
-      letBody = nest $ prettyItems hardline (Items bindersWithoutComments)
+      letBody = nest $ prettyItems (Items bindersWithoutComments)
       inPart =
         group $
           pretty (Ann [] in_ Nothing)

src/Nixfmt/Types.hs

@@ -58,12 +58,10 @@ instance (Eq a) => Eq (Ann a) where
 --    show (Ann _ a _) = show a
 
 data Item a
-  = -- | An item with a list of line comments that apply to it. There is no
-    -- empty line between the comments and the stuff it applies to.
-    CommentedItem Trivia a
-  | -- | A list of line comments not associated with any item. Followed by an
-    -- empty line unless they're the last comments in a set or list.
-    DetachedComments Trivia
+  = -- | An item
+    Item a
+  | -- | Trivia interleaved in items
+    Comments Trivia
   deriving (Foldable, Show)
 
 newtype Items a = Items {unItems :: [Item a]}
@@ -279,33 +277,33 @@ instance LanguageElement Term where
   walkSubprograms = \case
     -- Map each item to a singleton list, then handle that
     (List _ items _) | Prelude.length (unItems items) == 1 -> case Prelude.head (unItems items) of
-      (CommentedItem c item) -> [emptySet c, Term item]
-      (DetachedComments _) -> []
+      (Item item) -> [Term item]
+      (Comments _) -> []
     (List _ items _) ->
       unItems items >>= \case
-        CommentedItem comment item ->
-          [Term (List (ann TBrackOpen) (Items [CommentedItem comment item]) (ann TBrackClose))]
-        DetachedComments c ->
-          [Term (List (ann TBrackOpen) (Items [DetachedComments c]) (ann TBrackClose))]
+        Item item ->
+          [Term (List (ann TBrackOpen) (Items [Item item]) (ann TBrackClose))]
+        Comments c ->
+          [Term (List (ann TBrackOpen) (Items [Comments c]) (ann TBrackClose))]
     (Set _ _ items _) | Prelude.length (unItems items) == 1 -> case Prelude.head (unItems items) of
-      (CommentedItem c (Inherit _ from sels _)) ->
-        (Term <$> maybeToList from) ++ concatMap walkSubprograms sels ++ [emptySet c]
-      (CommentedItem c (Assignment sels _ expr _)) ->
-        expr : concatMap walkSubprograms sels ++ [emptySet c]
-      (DetachedComments _) -> []
+      (Item (Inherit _ from sels _)) ->
+        (Term <$> maybeToList from) ++ concatMap walkSubprograms sels
+      (Item (Assignment sels _ expr _)) ->
+        expr : concatMap walkSubprograms sels
+      (Comments _) -> []
     (Set _ _ items _) ->
       unItems items >>= \case
         -- Map each binding to a singleton set
-        (CommentedItem comment item) ->
-          [Term (Set Nothing (ann TBraceOpen) (Items [CommentedItem comment item]) (ann TBraceClose))]
-        (DetachedComments c) -> [emptySet c]
+        (Item item) ->
+          [Term (Set Nothing (ann TBraceOpen) (Items [Item item]) (ann TBraceClose))]
+        (Comments c) -> [emptySet c]
     (Selection term sels Nothing) -> Term term : (sels >>= walkSubprograms)
     (Selection term sels (Just (_, def))) -> Term term : (sels >>= walkSubprograms) ++ [Term def]
     (Parenthesized _ expr _) -> [expr]
     -- The others are already minimal
     _ -> []
     where
-      emptySet c = Term (Set Nothing (ann TBraceOpen) (Items [DetachedComments c]) (ann TBraceClose))
+      emptySet c = Term (Set Nothing (ann TBraceOpen) (Items [Comments c]) (ann TBraceClose))
 
 instance LanguageElement Expression where
   mapFirstToken' f = \case
@@ -342,8 +340,8 @@ instance LanguageElement Expression where
       body
         : ( unItems items >>= \case
               -- Map each binding to a singleton set
-              (CommentedItem _ item) -> [Term (Set Nothing (ann TBraceOpen) (Items [CommentedItem [] item]) (ann TBraceClose))]
-              (DetachedComments _) -> []
+              (Item item) -> [Term (Set Nothing (ann TBraceOpen) (Items [Item item]) (ann TBraceClose))]
+              (Comments _) -> []
           )
     (Assert _ cond _ body) -> [cond, body]
     (If _ expr0 _ expr1 _ expr2) -> [expr0, expr1, expr2]

test/diff/attr_set/out.nix

@@ -87,6 +87,7 @@
     c = 1;
 
     e = 1;
+
   }
 
   rec
@@ -102,6 +103,7 @@
     e = 1;
 
     # f
+
   }
   {
     x =

test/diff/idioms_lib_3/out.nix

@@ -19,6 +19,7 @@ let
   libAttr = lib.attrsets;
 
   inherit (lib) isFunction;
+
 in
 rec {
 
@@ -528,6 +529,7 @@ rec {
             ) x
           )
         );
+
     in
     ''
       <?xml version="1.0" encoding="UTF-8"?>

test/diff/idioms_lib_4/out.nix

@@ -31,6 +31,7 @@ let
       assert type.check value;
       setType type.name ({ inherit name; } // value)
     );
+
 in
 
 rec {
@@ -887,6 +888,7 @@ rec {
           else
             abis.unknown;
       };
+
     in
     mkSystem parsed;
 
@@ -927,4 +929,5 @@ rec {
     "${cpu.name}-${vendor.name}-${kernelName kernel}${optExecFormat}${optAbi}";
 
   ################################################################################
+
 }

test/diff/idioms_lib_5/out.nix

@@ -604,7 +604,9 @@ let
           yes = true;
         }
         .${validity.valid};
+
     };
+
 in
 {
   inherit assertValidity commonMeta;

test/diff/idioms_nixos_1/out.nix

@@ -16,6 +16,7 @@ let
   kernelModulesConf = pkgs.writeText "nixos.conf" ''
     ${concatStringsSep "\n" config.boot.kernelModules}
   '';
+
 in
 
 {
@@ -214,6 +215,7 @@ in
         lib.kernelConfig functions to build list elements.
       '';
     };
+
   };
 
   ###### implementation
@@ -262,6 +264,7 @@ in
               "hid_logitech_hidpp"
               "hid_logitech_dj"
               "hid_microsoft"
+
             ]
             ++ optionals pkgs.stdenv.hostPlatform.isx86 [
               # Misc. x86 keyboard stuff.
@@ -381,6 +384,9 @@ in
             assertion = attrs.assertion cfg;
             inherit (attrs) message;
           }) config.system.requiredKernelConfig;
+
     })
+
   ];
+
 }

test/diff/idioms_nixos_2/out.nix

@@ -67,6 +67,7 @@ let
   '';
 
   inherit (config.system) stateVersion;
+
 in
 {
 
@@ -383,6 +384,7 @@ in
           is used to also support MariaDB version >= 10.6.
         '';
       };
+
     };
 
     config = {
@@ -762,6 +764,7 @@ in
             The package can be upgraded by explicitly declaring the service-option
             `services.nextcloud.package`.
           '';
+
         in
         (optional (cfg.poolConfig != null) ''
           Using config.services.nextcloud.poolConfig is deprecated and will become unsupported in a future release.
@@ -1016,6 +1019,7 @@ in
                   ${toString i} --value="${toString v}"
               '') ([ cfg.hostName ] ++ cfg.config.extraTrustedDomains)
             );
+
           in
           {
             wantedBy = [ "multi-user.target" ];

test/diff/idioms_pkgs_3/out.nix

@@ -228,6 +228,7 @@ let
       '') defaultPrefs
     )
   );
+
 in
 
 buildStdenv.mkDerivation ({

test/diff/idioms_pkgs_4/out.nix

@@ -145,6 +145,7 @@ let
         config
         ;
     };
+
 in
 
 [
@@ -189,6 +190,7 @@ in
         # Curl should be in /usr/bin or so.
         curl = null;
       };
+
     }
   )
 
@@ -212,4 +214,5 @@ in
         if localSystem.isLinux then [ prevStage.patchelf ] else [ ];
     };
   })
+
 ]

test/diff/idioms_pkgs_5/out.nix

@@ -90,6 +90,7 @@ let
         );
 
       finalPackage = mkDerivationSimple overrideAttrs args;
+
     in
     finalPackage;
 
@@ -727,6 +728,7 @@ let
               "The ‘env’ attribute set can only contain derivation, string, boolean or integer attributes. The ‘${n}’ attribute is of type ${builtins.typeOf v}.";
             v
           ) env;
+
       in
 
       extendDerivation validity.handled (
@@ -785,6 +787,7 @@ let
           # derivation (e.g., in assertions).
           passthru
       ) (derivation (derivationArg // optionalAttrs envIsExportable checkedEnv));
+
 in
 fnOrAttrs:
 if builtins.isFunction fnOrAttrs then

test/diff/key_value/out.nix

@@ -67,4 +67,5 @@ rec {
   ;
 
   p = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa { } a;
+
 }

test/diff/let_in/in.nix

@@ -31,6 +31,17 @@ let
     2
   ];
 
+  a =
+    let
+      b = 0;
+
+
+      # foo
+      # bar
+    in # baz
+    # qux
+    null;
+
 in