diff --git a/src/Nixfmt/Parser.hs b/src/Nixfmt/Parser.hs index ddfb558d..a4c0c6f8 100644 --- a/src/Nixfmt/Parser.hs +++ b/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 diff --git a/src/Nixfmt/Pretty.hs b/src/Nixfmt/Pretty.hs index d7905ae4..4f99661c 100644 --- a/src/Nixfmt/Pretty.hs +++ b/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 diff --git a/src/Nixfmt/Util.hs b/src/Nixfmt/Util.hs index 8bc59866..c2ae1828 100644 --- a/src/Nixfmt/Util.hs +++ b/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 diff --git a/test/diff/apply_with_lists/in.nix b/test/diff/apply_with_lists/in.nix index 86eebd24..df508ab8 100644 --- a/test/diff/apply_with_lists/in.nix +++ b/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.") ] diff --git a/test/diff/apply_with_lists/out-pure.nix b/test/diff/apply_with_lists/out-pure.nix index 6c33da70..51dedb75 100644 --- a/test/diff/apply_with_lists/out-pure.nix +++ b/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.") ] diff --git a/test/diff/apply_with_lists/out.nix b/test/diff/apply_with_lists/out.nix index bee7df43..9ed360b6 100644 --- a/test/diff/apply_with_lists/out.nix +++ b/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.") ] diff --git a/test/diff/attr_set/in.nix b/test/diff/attr_set/in.nix index e9ca7d84..abc8b304 100644 --- a/test/diff/attr_set/in.nix +++ b/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 [ + ] [ + ]; + } ] diff --git a/test/diff/attr_set/out-pure.nix b/test/diff/attr_set/out-pure.nix index f28d2e25..c4b9cf21 100644 --- a/test/diff/attr_set/out-pure.nix +++ b/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 [ ] [ ]; + } ] diff --git a/test/diff/attr_set/out.nix b/test/diff/attr_set/out.nix index 7b52a916..bd47e960 100644 --- a/test/diff/attr_set/out.nix +++ b/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 + [ + ] + [ + ]; + } ] diff --git a/test/diff/idioms_nixos_2/out-pure.nix b/test/diff/idioms_nixos_2/out-pure.nix index 8c4914b5..d0403c76 100644 --- a/test/diff/idioms_nixos_2/out-pure.nix +++ b/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} ]; diff --git a/test/diff/idioms_nixos_2/out.nix b/test/diff/idioms_nixos_2/out.nix index 444c20c1..c1f64037 100644 --- a/test/diff/idioms_nixos_2/out.nix +++ b/test/diff/idioms_nixos_2/out.nix @@ -842,11 +842,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}'),"} ], ] ''; @@ -888,9 +886,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 ], @@ -901,37 +898,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} ]; diff --git a/test/diff/string_interpol/out-pure.nix b/test/diff/string_interpol/out-pure.nix index 643949e4..f195f6ef 100644 --- a/test/diff/string_interpol/out-pure.nix +++ b/test/diff/string_interpol/out-pure.nix @@ -30,23 +30,19 @@ type nat hook prerouting priority dstnat policy accept - ${ - builtins.concatStringsSep "\n" ( - map ( - e: - "iifname \"${cfg.upstreamIface}\" tcp dport ${builtins.toString e.sourcePort} dnat to ${e.destination}" - ) tcpPortMap - ) - } + ${builtins.concatStringsSep "\n" ( + map ( + e: + "iifname \"${cfg.upstreamIface}\" tcp dport ${builtins.toString e.sourcePort} dnat to ${e.destination}" + ) tcpPortMap + )} - ${ - builtins.concatStringsSep "\n" ( - map ( - e: - "ifname \"${cfg.upstreamIface}\" udp dport ${builtins.toString e.sourcePort} dnat to ${e.destination}" - ) udpPortMap - ) - } + ${builtins.concatStringsSep "\n" ( + map ( + e: + "ifname \"${cfg.upstreamIface}\" udp dport ${builtins.toString e.sourcePort} dnat to ${e.destination}" + ) udpPortMap + )} } ''; } diff --git a/test/diff/string_interpol/out.nix b/test/diff/string_interpol/out.nix index 643949e4..f195f6ef 100644 --- a/test/diff/string_interpol/out.nix +++ b/test/diff/string_interpol/out.nix @@ -30,23 +30,19 @@ type nat hook prerouting priority dstnat policy accept - ${ - builtins.concatStringsSep "\n" ( - map ( - e: - "iifname \"${cfg.upstreamIface}\" tcp dport ${builtins.toString e.sourcePort} dnat to ${e.destination}" - ) tcpPortMap - ) - } + ${builtins.concatStringsSep "\n" ( + map ( + e: + "iifname \"${cfg.upstreamIface}\" tcp dport ${builtins.toString e.sourcePort} dnat to ${e.destination}" + ) tcpPortMap + )} - ${ - builtins.concatStringsSep "\n" ( - map ( - e: - "ifname \"${cfg.upstreamIface}\" udp dport ${builtins.toString e.sourcePort} dnat to ${e.destination}" - ) udpPortMap - ) - } + ${builtins.concatStringsSep "\n" ( + map ( + e: + "ifname \"${cfg.upstreamIface}\" udp dport ${builtins.toString e.sourcePort} dnat to ${e.destination}" + ) udpPortMap + )} } ''; }