From 331e0aa71f789b704ab7c8564ec5328a9186d024 Mon Sep 17 00:00:00 2001 From: piegames Date: Sun, 11 Aug 2024 12:37:05 +0200 Subject: [PATCH] Don't expand lists within functions --- src/Nixfmt/Pretty.hs | 25 +++++++-- src/Nixfmt/Types.hs | 2 +- test/diff/apply/in.nix | 2 +- test/diff/apply/out-pure.nix | 16 ++---- test/diff/apply/out.nix | 2 +- test/diff/idioms_lib_3/out-pure.nix | 30 +++-------- test/diff/idioms_lib_3/out.nix | 30 +++-------- test/diff/idioms_nixos_2/out-pure.nix | 73 ++++++++------------------- test/diff/idioms_nixos_2/out.nix | 73 ++++++++------------------- 9 files changed, 84 insertions(+), 169 deletions(-) diff --git a/src/Nixfmt/Pretty.hs b/src/Nixfmt/Pretty.hs index 83b02afb..bcda8a94 100644 --- a/src/Nixfmt/Pretty.hs +++ b/src/Nixfmt/Pretty.hs @@ -375,15 +375,34 @@ 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 -- This is very hacky, but selections shouldn't be in a priority group, + let -- Walk the function call chain + absorbApp :: 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 a') - absorbApp (Application f' a') = group' Transparent (absorbApp f') <> line <> nest (group' Priority a') + 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') + -- 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 + -- This is just a copy of the list rendering code, but with `sepBy line` instead of `sepBy hardline` + 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 index 262a4acb..93dd1d02 100644 --- 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 index d8c22eb0..0a639697 100644 --- 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 index 42497b7f..99114616 100644 --- 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 index a30e59ad..6692fd4e 100644 --- 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 index 1115ae8b..387e9df3 100644 --- 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 index 4a1b2843..469184e6 100644 --- 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 index e7d3e5d2..a2743768 100644 --- 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 index 7502b9f9..b63a9379 100644 --- 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 = {