From ebb3b25068e22b0fa13df77dae69fd1b1c9115cc Mon Sep 17 00:00:00 2001 From: piegames Date: Mon, 2 Dec 2024 23:33:56 +0100 Subject: [PATCH] Don't move trailing comments in let anymore While this was initially introduced for good reasons, as can also be seen in the diff, that code was written with documentation comments in mind and not code comments. As per the bug report, the downsides were deemed to high to keep it in Fixes #259 --- src/Nixfmt/Pretty.hs | 22 ++-------------------- test/diff/comment/out-pure.nix | 2 +- test/diff/comment/out.nix | 2 +- test/diff/idioms_lib_3/out-pure.nix | 2 +- test/diff/idioms_lib_3/out.nix | 2 +- test/diff/let_in/out-pure.nix | 4 ++-- test/diff/let_in/out.nix | 4 ++-- 7 files changed, 10 insertions(+), 28 deletions(-) diff --git a/src/Nixfmt/Pretty.hs b/src/Nixfmt/Pretty.hs index d7905ae4..b5ee6577 100644 --- a/src/Nixfmt/Pretty.hs +++ b/src/Nixfmt/Pretty.hs @@ -632,31 +632,13 @@ instance Pretty Expression where convertTrailing Nothing = [] convertTrailing (Just (TrailingComment t)) = [LineComment (" " <> t)] - -- Extract detached comments at the bottom. - -- This uses a custom variant of span/spanJust/spanMaybe. - -- Note that this is a foldr which walks from the bottom, but the lists - -- are constructed in a way that they end up correct again. - (binderComments, bindersWithoutComments) = - foldr - ( \item (start, rest) -> case item of - (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 (Items bindersWithoutComments) + letBody = nest $ prettyItems binders inPart = group $ pretty in_ <> hardline - -- Take our trailing and inject it between `in` and body - <> pretty (concat binderComments ++ preTrivia ++ convertTrailing trailComment) + <> pretty (preTrivia ++ convertTrailing trailComment) <> pretty expr pretty (Assert assert cond semicolon expr) = group $ diff --git a/test/diff/comment/out-pure.nix b/test/diff/comment/out-pure.nix index 138f59c0..c8d87b6c 100644 --- a/test/diff/comment/out-pure.nix +++ b/test/diff/comment/out-pure.nix @@ -146,8 +146,8 @@ #6 d = 1; + #7 in - #7 d ) diff --git a/test/diff/comment/out.nix b/test/diff/comment/out.nix index 138f59c0..c8d87b6c 100644 --- a/test/diff/comment/out.nix +++ b/test/diff/comment/out.nix @@ -146,8 +146,8 @@ #6 d = 1; + #7 in - #7 d ) diff --git a/test/diff/idioms_lib_3/out-pure.nix b/test/diff/idioms_lib_3/out-pure.nix index 2c9c796e..d704f9a5 100644 --- a/test/diff/idioms_lib_3/out-pure.nix +++ b/test/diff/idioms_lib_3/out-pure.nix @@ -148,8 +148,8 @@ rec { [${mkSectionName sectName}] '' + toKeyValue { inherit mkKeyValue listsAsDuplicateKeys; } sectValues; + # map input to ini sections in - # map input to ini sections mapAttrsToStringsSep "\n" mkSection attrsOfAttrs; # Generate an INI-style config file from an attrset diff --git a/test/diff/idioms_lib_3/out.nix b/test/diff/idioms_lib_3/out.nix index 53474a65..2686bade 100644 --- a/test/diff/idioms_lib_3/out.nix +++ b/test/diff/idioms_lib_3/out.nix @@ -151,8 +151,8 @@ rec { [${mkSectionName sectName}] '' + toKeyValue { inherit mkKeyValue listsAsDuplicateKeys; } sectValues; + # map input to ini sections in - # map input to ini sections mapAttrsToStringsSep "\n" mkSection attrsOfAttrs; # Generate an INI-style config file from an attrset diff --git a/test/diff/let_in/out-pure.nix b/test/diff/let_in/out-pure.nix index 6870e5b3..c0774c93 100644 --- a/test/diff/let_in/out-pure.nix +++ b/test/diff/let_in/out-pure.nix @@ -65,9 +65,9 @@ let let b = 0; + # foo + # bar in - # foo - # bar # baz # qux null; diff --git a/test/diff/let_in/out.nix b/test/diff/let_in/out.nix index 6870e5b3..c0774c93 100644 --- a/test/diff/let_in/out.nix +++ b/test/diff/let_in/out.nix @@ -65,9 +65,9 @@ let let b = 0; + # foo + # bar in - # foo - # bar # baz # qux null;